Skip to content
This repository has been archived by the owner on Oct 26, 2020. It is now read-only.

Add accessors to deconstruct Span #54

Merged

Conversation

leostera
Copy link
Contributor

When trying to link traces across separate systems (either linked via
HTTP calls or a message queue), having direct access to the TraceID and the
SpanID make it easier to serialize this properly and recreate a Link on
the receiving end.

This PR adds support for deconstructing a Span Context to get access to
these values.

I've added a couple of tests but they aren't all passing, so could use a
hand getting the test for establishing links to parent trace in shape.

When trying to link traces across separate systems (either linked via
HTTP calls or a message queue), having direct access to the TraceID and the
SpanID make it easier to serialize this properly and recreate a Link on
the receiving end.

This PR adds support for deconstructing a Span Context to get access to
these values.

I've added a couple of tests but they aren't all passing, so could use a
hand getting the test for establishing links to parent trace in shape.
@leostera leostera requested a review from a team as a code owner May 27, 2020 14:22
span = Tracer.current_span_ctx()

assert parent_trace_id == Span.trace_id(span)
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tsloughter this is currently failing with:

  1) test can establish link to parent trace (OpenTelemetryTest)
     test/open_telemetry_test.exs:46
     Assertion with == failed
     code:  assert parent_trace_id == Span.trace_id(span)
     left:  1
     right: 0
     stacktrace:
       test/open_telemetry_test.exs:49: (test)

Which makes me think that either the test is testing the wrong thing, or that with_span/2 isn't honoring the links as I had expected it to. Considering there's no way of manually building a OpenTelemetry.span_ctx(), I'd expect it to treat links as such.

Copy link
Member

Choose a reason for hiding this comment

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

I don't remember writing this test. You are right that it looks wrong. There is no reason the new span in with_span should have the same parent trace id as is used in the link.

Copy link
Member

Choose a reason for hiding this comment

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

Oh wait, this is a test you added?

Copy link
Member

Choose a reason for hiding this comment

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

probably just don't test this here, we'll want a test for it in the SDK where we can pull the links from the span to verify.

@doc """
Get the SpanId of a Span.
"""
@spec span_id(OpenTelemetry.span()) :: OpenTelemetry.span_id()
Copy link
Member

Choose a reason for hiding this comment

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

I think you meant span context here, not span?

@tsloughter tsloughter merged commit e955e27 into open-telemetry:master May 27, 2020
@leostera leostera deleted the span/accessors-for-trace-and-span-id branch May 27, 2020 16:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants