[OpenTracing] Fix context propagation between OpenTracing and Datadog#518
Merged
delner merged 2 commits intoAug 24, 2018
Merged
Conversation
pawelchcki
approved these changes
Aug 22, 2018
delner
commented
Aug 22, 2018
Contributor
Author
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
August 22, 2018 18:58
3d223ff to
fab0f89
Compare
delner
commented
Aug 22, 2018
Contributor
Author
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
Contributor
Author
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
August 22, 2018 19:06
fab0f89 to
0612ae0
Compare
Contributor
Author
|
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
Kyle-Verhoog
left a comment
Kyle-Verhoog
left a comment
Member
There was a problem hiding this comment.
Looks good to me, I had a nearly identical approach with the Python tracer.
| 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 |
| 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.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.