Skip to content
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

Propagate valid span context in noop tracer #197

Merged
merged 1 commit into from
Aug 27, 2020

Conversation

frigus02
Copy link
Member

Change the API to propagate valid span contexts if no SDK tracer is configured and new spans are created.

Part of #181

Description

The latest specification reads:

Behavior of the API in the absence of an installed SDK

The following cases must be considered when a new Span is requested to be created, especially in relation to the requested parent SpanContext:

  • A valid SpanContext is specified as the parent of the new Span: The API MUST treat this parent context as the context for the newly created Span. [...]
  • No valid SpanContext is specified as the parent of the new Span: The API MUST create an non-valid [...] Span for use by the API caller. [...]

If I understand it correctly, this maps to the NoopTracer in this repo. Currently the NoopTracer always creates invalid spans.

I copied the logic from the sdk::Tracer about figuring out the correct parent span context. If this is correct, we could consider moving this logic into a function shared between the NoopTracer and the sdk::Tracer.

Change the API to propagate valid span contexts if no SDK tracer is
configured and new spans are created.
@frigus02 frigus02 requested a review from a team August 26, 2020 21:08
Copy link
Contributor

@TommyCpp TommyCpp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start to address #181. Thanks!

Copy link
Member

@jtescher jtescher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jtescher jtescher merged commit 4dd328c into open-telemetry:master Aug 27, 2020
@frigus02 frigus02 deleted the noop-tracer-propagates branch August 28, 2020 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants