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

Add Jaeger propagator #967

Closed
pavolloffay opened this issue Mar 6, 2020 · 10 comments · Fixed by #1549
Closed

Add Jaeger propagator #967

pavolloffay opened this issue Mar 6, 2020 · 10 comments · Fixed by #1549
Assignees
Labels
help wanted priority:p2 Medium priority issues and bugs. release:required-for-ga Required for 1.0 GA release
Projects

Comments

@pavolloffay
Copy link
Member

Similar to #853 but for Jaeger clients.

Does OTEL client support exporting/injecting of TraceContext/SpanContext into two formats? At the same time use W3C TraceContext and Jaeger? The use case is to allow gradual migration of existing deployments using Jaeger prop. format to W3C.

@Oberon00
Copy link
Member

Oberon00 commented Mar 6, 2020

Related #718 "No way to change HttpTextFormat"

@jarebudev
Copy link
Contributor

Hi, I can take a look into this if nobody is already working on it

@pavolloffay
Copy link
Member Author

Go ahead @jarebudev

@carlosalberto carlosalberto added the release:required-for-ga Required for 1.0 GA release label Jul 21, 2020
@carlosalberto
Copy link
Contributor

We need to support baggage in the Jaeger format to close this one.

@jarebudev
Copy link
Contributor

i can take a look at this, should have a bit more spare time to help out

@jarebudev
Copy link
Contributor

I should have a PR ready this week time permitting.
To confirm - is this for inject only, or both inject and extract?

@jkwatson
Copy link
Contributor

Unless there's some reason I don't understand, I think we'd want both inject and extract.

@jarebudev
Copy link
Contributor

Unless there's some reason I don't understand, I think we'd want both inject and extract.

Finally got back to this.. :)

I have made changes so that JaegerPropagator injects baggage items and is ready for a PR.

For extracting baggage from Jaeger, unless I've missed something, I was wondering how we'd extract a baggage item when the HTTP headers uberctx-{baggage-key} passed to the propagator are not fixed and we can only get 'known' headers (e.g. uber-trace-id) using the HttpTextFormat.Getter.get() method? Do we need an additional method on the Getter interface to return all keys?

@carlosalberto
Copy link
Contributor

@jarebudev

We need to support iteration of keys in the Getter - see this (on my queue, and it's also marked as "required-for-ga"). Hope to work on that next week, and once that is done, you will be able to complete this task ;)

@jkwatson jkwatson added the priority:p2 Medium priority issues and bugs. label Aug 12, 2020
@jkwatson jkwatson added this to To do in GA Sep 10, 2020
@jkwatson jkwatson moved this from To do to In progress in GA Oct 19, 2020
@carlosalberto
Copy link
Contributor

Assigning this to me as this will be part of the over all updates for Getter.Keys(). Hope that is fine @jarebudev 😅

@jkwatson jkwatson moved this from In progress to Review in progress in GA Oct 27, 2020
GA automation moved this from Review in progress to Done Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted priority:p2 Medium priority issues and bugs. release:required-for-ga Required for 1.0 GA release
Projects
No open projects
GA
  
Done
Development

Successfully merging a pull request may close this issue.

5 participants