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

Expose context methods for current trace and span #46

Merged
merged 3 commits into from
Jan 21, 2020

Conversation

polotek
Copy link
Contributor

@polotek polotek commented Nov 26, 2019

Expose the current trace and current span in the top level scope. These properties are useful for doing custom instrumentation.

Copy link
Contributor

@ajvondrak ajvondrak left a comment

Choose a reason for hiding this comment

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

Hey, just adding my thoughts since these APIs are relevant to my interests. 😄 (Cf. all the talking to myself in #45.)

lib/honeycomb-beeline.rb Show resolved Hide resolved
lib/honeycomb/client.rb Outdated Show resolved Hide resolved
lib/honeycomb/client.rb Show resolved Hide resolved
@grncdr
Copy link

grncdr commented Dec 5, 2019

Just want to add that since upgrading from 0.8, I couldn't find any "official" way of creating a tracing header for propagation, and ended up with this code which is using send to hack around private methods:

trace_header = Honeycomb.client.send(:context).current_span.try(&:to_trace_header)

So any PR that lets me avoid that would be most welcome 👍

Copy link
Member

@martin308 martin308 left a comment

Choose a reason for hiding this comment

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

This looks great. Would love to have a little bit more discussion on exposing the non current_span and current_trace accessors. We could split that into another PR so we can discuss those features and get current_span and current_trace released?

lib/honeycomb-beeline.rb Show resolved Hide resolved
lib/honeycomb/client.rb Outdated Show resolved Hide resolved
lib/honeycomb/client.rb Outdated Show resolved Hide resolved
@martin308
Copy link
Member

@grncdr the way I was expecting people to use the trace header stuff was that you would have a span wrapping the http request so you would end up with something like the faraday integration https://github.com/honeycombio/beeline-ruby/blob/master/lib/honeycomb/integrations/faraday.rb#L25.

@ajvondrak
Copy link
Contributor

Would love to have a little bit more discussion on exposing the non current_span and current_trace accessors. We could split that into another PR so we can discuss those features and get current_span and current_trace released?

Big 👍 on that.

@polotek
Copy link
Contributor Author

polotek commented Jan 17, 2020

This PR is super simple now. Just exposing the current_trace and current_span. I think there's a lot more to discuss. But if we can land this, it'll enable a lot of good things that aren't possible today.

@martin308 martin308 merged commit 4713124 into honeycombio:master Jan 21, 2020
@ajvondrak ajvondrak mentioned this pull request Jan 27, 2020
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.

4 participants