[OpenTracing] Fix context propagation between OpenTracing and Datadog #518
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pawelchcki
approved these changes
Aug 22, 2018
delner
commented
Aug 22, 2018
| # effectively ends up "assigned to a scope", by virtue of being added to the | ||
| # Context. Hence, it would behave more like an active span, which is why it | ||
| # should only be here. | ||
| unless child_of || ignore_active_scope |
There was a problem hiding this comment.
This block is responsible for allowing OpenTracer spans to append to traces that were created by Datadog auto-instrumentation. e.g. datadog.span --> open_tracer.span.
delner
force-pushed
the
bugfix/opentracing_context_between_tracers
branch
from
3d223ff to
fab0f89
Compare
delner
commented
Aug 22, 2018
| # This is mostly for the benefit of any out-of-the-box tracing from Datadog, | ||
| # such that spans generated by that tracing will be attached to the OpenTracer | ||
| # parent span. | ||
| datadog_tracer.provider.context = span.datadog_span.context |
There was a problem hiding this comment.
This line is responsible for allowing Datadog auto-instrumentation spans to append to traces that were created by OpenTracer. e.g. open_tracer.span --> datadog.span .
delner
commented
Aug 22, 2018
|
|
||
| private | ||
|
|
||
| def inherited_span_context(parent, ignore_active_scope: false) |
There was a problem hiding this comment.
This is just a refactor from #start_span.
delner
force-pushed
the
bugfix/opentracing_context_between_tracers
branch
from
fab0f89 to
0612ae0
Compare
|
Made some significant changes to how context is resolved and written to. This should be ready for review. |
pawelchcki
reviewed
Aug 23, 2018
| child_of = if scope_manager.active | ||
| scope_manager.active.span.context | ||
| else | ||
| SpanContextFactory.build(datadog_context: datadog_tracer.call_context) |
delner
changed the title
Merged
Kyle-Verhoog
approved these changes
Aug 23, 2018
| @@ -204,6 +228,58 @@ def current_trace_for(object) | |||
| end | |||
| end | |||
| end | |||
|
|
|||
| context 'preceded by a Datadog span' do | |||
| it { expect(child_datadog_span.trace_id).to eq(parent_datadog_span.trace_id) } | ||
| end | ||
|
|
||
| context 'followed by a Datadog span' do |
| @@ -51,6 +51,30 @@ def current_trace_for(object) | |||
| end | |||
|
|
|||
| context 'for a nested span' do | |||
| context 'when there is no active scope' do | |||
delner
added a commit
that referenced
this pull request
Sep 13, 2018
…#518) * Fixed: OpenTracer::Tracer not using child_of SpanContext properly. * Changed: OpenTracer to append its spans to Datadog auto-instrumentation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When using
ddtraceOpenTracer in conjunction with an app already instrumented byruby-rack-tracer, we were seeing some disjoint traces. Spans generated byddtraceout of the box instrumentation was not ending up parented by the OpenTracing integration parent span (in this case, Rack), resulting in multiple traces that should have appeared as one.To fix this, the pull request does two things:
SpanContextobjects aschild_of. We need to make sure theDatadog::Contextobject that comes from this is set on the tracer, such that the following generated Datadog span inherits the proper trace info (trace ID, parent ID.) It was not actually using this context properly before.nilto thestart_spanmethod which would create a brand new context that would not be re-used on subsequent spans. We now make sure its passing aDatadog::Contexttostart_span.