-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
OpenTelemetry Propagator Support #22
Comments
Is propagation of headers like tracestate still broken due to no ContextManager being registered during initialization? I was debugging some stuff around active context, but kept getting the default ROOT_CONTEXT. From the docs, it made it clear that SDKs that fail to register a context manager will always default to the ROOT_CONTEXT as active. |
@Schachte This SDK (currently) effectively doesn't have a concept of a "global" context and anything trying to use This will be fixed with #26 when I get around to it, but in the meantime this is the "real" context. |
@RichiCoder1 ah nice, I understand now. I was adapting some of these ideas into a mini test project and was confused why my active context was always empty. Great library! |
@Schachte absolutely! Hopefully the gap between the official JS and my lib will shrink over time 😄. Happy to hear any other feedback |
@RichiCoder1 Same! I assume it will, same on the Workers side. Was main issue with the http exporter from OTEL the mixed use of XHR/window that caused a lot of issues for you in CF Workers? Just getting started messing with OTEL with CF workers and feel like I probably ran into a lot of the same fun as you before I read your lib :) |
Yah, the Otel libs (reasonably) make a lot of assumptions that their either in a standard Node environment or a standard Web environment, both of which break down in Worker-like environments like Cloudflare Workers. They haven't really had the bandwidth to address that, hence this SDK as a patch fix. |
👍 sweet. Well thanks again for a great lib, was able to dissect a lot of OTEL weirdness combing through your logic. Cheers! |
I didn't implement propagator support originally as I was worried about potential global assumptions they were making, but after more research I think it may be reasonable to expose this as a config option with a default.
The text was updated successfully, but these errors were encountered: