-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow for alternative context propagation headers #4
Comments
Hey @felixbarny, there is currently no appropriate way/file to do this. How about we add an option to WDYT?
The compilation result of weasel must be as small as possible. I'd prefer not to ship optional code paths for runtime. |
How can |
You replace it at build time. |
I see. Then yes, it's probably the best place to specify the header names. |
Do you want to contribute this? |
I really want but currently I'm quite involved in a lot of stuff. But I'll ask my colleague @trampi who implemented the eum stuff in stagemonitor. |
@trampi: Do you think this approach is feasible or should we rather stick to directly modifying the headers in |
It seems reasonable to me to override them via vars.js. @bripkens: you are not using CORS because you want to support all browsers with minimum overhead, right? |
The thing is, that you cannot set tracing headers by default. Only for the same origin you can be sure that this is possible. All other origins need to explicitly allow these custom headers. By always setting these headers, we could break a lot of monitored websites which access third parties (which is basically every large website). To make this work within weasel, I thought about adding a user-configurable domain whitelist. Users could add domains for which they know that servers send correct |
Suppose in the not too distant future we (as in the tracing community) come up with a common distributed trace correlation header, then we could talk to browser vendors to allow this header even for cross-origin requests. That would be great to have. |
To give a little more background: we're currently implementing end user monitoring support in stagemonitor and use weasel to report the end user timings to a custom stagemonitor endpoint. The endpoint is either same origin or cross origin, in the latter case stagemonitor will send the correct CORS-headers.
As far as I understand, only the server specified in
Weasel could try to send a XHR with the correlation headers and fall back to sending an XHR without them, if the first XHR fails because of CORS being configured incorrectly.
Yes, that would be awesome! |
Sorry, was thinking about beacons, not XHR-Requests to other hosts for propagation. I now understand the problem. |
As far as I know, the client side is not informed about the concrete XHR rejection reason. If it is, this might be one way to do it. Though I'd prefer to give users control over this as it is incredibly unlikely that APIs send the correct |
In https://github.com/instana/weasel/blob/master/lib/hooks/XMLHttpRequest.js#L172, Instana specific context propagation headers are used. It would be nice if it was configurable which headers should be used. Maybe even by just specifying the "propagation style" like
instana
orb3
.Where would be the appropriate place to do that?
As a workaround, I'm currently replacing the Instana headers with B3 headers in the build.
The text was updated successfully, but these errors were encountered: