Skip to content

B3 context propagation #86

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

Merged
merged 13 commits into from
Oct 24, 2019
Merged

B3 context propagation #86

merged 13 commits into from
Oct 24, 2019

Conversation

mwear
Copy link
Contributor

@mwear mwear commented Oct 22, 2019

This PR adds support for B3 context propagation. A user is able to specify a propagator instance during tracer initialization. The default propagation format is LightStep. See the example below:

# Use default (LightStep) propagation
LightStep.configure(component_name: 'lightstep/ruby/example', access_token: 'your_access_token',)

#Specify B3 propagation
LightStep.configure(component_name: 'lightstep/ruby/example', access_token: 'your_access_token', propagator: :b3)

Currently Implemented

  • B3 Multiple Header format
  • Propagates sampling flag

Not Implemented

  • B3 debug trace flag
  • Single header version

This work might be incomplete, however, I figured I'd open this PR as a starting point for discussion as it's somewhat unclear what level of support is required. The current javascript PR implements the multiple header format, does not propagate the sampled state, or implement the debug flag or single header format. The python PR is the most complete and as it implements both multiple and single header formats, propagates the sampled state and debug trace flag.

mwear and others added 8 commits October 17, 2019 13:13
Constant lookup will search in Module.nesting before looking up
the inheritance chain, which makes constants hard to override
when subclassing. This commit prefixes constants in the
LightStep propagator with self.class in order to steer lookup
to the inheritance chain.
This commit introduces a B3 Propagator where keys are properly
injected and extracted under the proper names. Special handling
of the trace id and sampled flag will come in subsequent commits.
@@ -0,0 +1,125 @@
#frozen_string_literal: true

module LightStep
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The LightStepPropagator was extracted out of the LightStep::Tracer class.

module LightStep
# SpanContext holds the data for a span that gets inherited to child spans
class SpanContext
attr_reader :id, :trace_id, :baggage
attr_reader :id, :trace_id, :trace_id16, :sampled, :baggage
Copy link
Contributor Author

@mwear mwear Oct 22, 2019

Choose a reason for hiding this comment

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

The SpanContext maintains an 8 byte and 16 byte trace id. The 16 byte trace id is either the original, or an 8 byte id padded with zeros on the left.


def pad_id(id)
return id unless id && id.size == 16
"#{ZERO_PADDING}#{id}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm following the convention set out in the JS PR, but I am wondering if we should be padding on the right side of an 8 byte id. There was some discussion and a recent PR around this in the W3C Trace Context repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think, conventionally, we only pad the LHS of the id. I'm not sure what'd happen if you added the padding to the RHS - presumably it'd all convert to the same uint64 or whatever.

@mwear
Copy link
Contributor Author

mwear commented Oct 22, 2019

The build appears to be failing on Circle for reasons other than the code changes here. It's failing to install dependencies. I'm troubleshooting this in #87.

Update:

  • master has been failing with the same issue for 2 months. Here is when it started failing.
  • the last successful build was here

Latest update:

  • this was fixed by updating our build matrix with currently maintained ruby versions

mwear added 3 commits October 22, 2019 18:58
This updates our build matrix to test against Ruby versions that are
still under maintenance by the Ruby core team.
… them

These were switched to use the mutating versions to spare two string
allocations in a previous commit.
@mwear mwear requested a review from iredelmeier October 24, 2019 18:06
Copy link
Contributor

@austinlparker austinlparker left a comment

Choose a reason for hiding this comment

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

lgtm other than the upper/lowercase key question.


def pad_id(id)
return id unless id && id.size == 16
"#{ZERO_PADDING}#{id}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think, conventionally, we only pad the LHS of the id. I'm not sure what'd happen if you added the padding to the RHS - presumably it'd all convert to the same uint64 or whatever.

@mwear mwear merged commit 38b6701 into lightstep:master Oct 24, 2019
@mwear mwear deleted the b3_context_prop branch October 24, 2019 21:27
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.

2 participants