Add external_propagation_context support #5051
Conversation
sl0thentr0py force-pushed the neel/external-prop-contex branch from 77c1d24 to 5c675fd Compare 
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@ ## master #5051 +/- ## ========================================== - Coverage 83.93% 83.91% -0.03% ========================================== Files 179 179 Lines 17887 17894 +7 Branches 3179 3177 -2 ========================================== + Hits 15013 15015 +2 - Misses 1906 1908 +2 - Partials 968 971 +3
|
sl0thentr0py force-pushed the neel/external-prop-contex branch 4 times, most recently from e902c81 to 3763d47 Compare 
If we are on an external tracing system like otel, we allow registering a new source of `trace_id/span_id` that takes precedence over the scope's propagation_context. * Also reworked logs and metrics to use `get_trace_context` * Cleaned up handling of `get_trace_context` that is still messy but a bit more clearer now regarding which underlying `propagation_context` is used
sl0thentr0py force-pushed the neel/external-prop-contex branch from 3763d47 to 203d4d1 Compare 
| | ||
| # Add "trace" context | ||
| if contexts.get("trace") is None: | ||
| if has_tracing_enabled(options) and self._span is not None: |
There was a problem hiding this comment.
moved these into get_trace_context for consistency
| | ||
| return trace_context | ||
| propagation_context = self.get_active_propagation_context() | ||
| if propagation_context is None: |
There was a problem hiding this comment.
propagation_context is actually never None, but this implementation is whack so we need this redundant check...
There was a problem hiding this comment.
for some future cleanup, if you see get_active_propagation_context that's also really shady. I generally dislike that so many instance methods on this scope call and fetch other scopes themselves, it's entirely spaghetti but somehow works in the common code paths.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
sentrivana left a comment
sentrivana left a comment There was a problem hiding this comment.
Looking good as far as I can tell. Thanks for centralizing the logic more, def an improvement to directly accessing the propagation context from random places 😢
If we are on an external tracing system like otel, we allow registering a new source of
trace_id/span_idthat takes precedence over the scope's propagation_context.get_trace_contextget_trace_contextthat is still messy but a bit more clearer now regarding which underlyingpropagation_contextis used