-
Notifications
You must be signed in to change notification settings - Fork 825
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
plugin(grpc): use HttpTextFormat instead of Binary propagator #762
Conversation
@mayurkale22 the linked spec issue is merged now |
Removed the onHold label, will create separate PR to remove |
9a344a1
to
c2c8e32
Compare
setExtractedSpanContext(Context.ROOT_CONTEXT, spanContext), | ||
carrier | ||
); | ||
for (const [k, v] of Object.entries(carrier)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to approve this for now, but we should be using the new setter
interface for propagators to handle this. https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/api-propagators.md#setter-argument
Co-Authored-By: Daniel Dyla <dyladan@users.noreply.github.com>
Bummer that this will be broken/incompatible if you call an older version of ourselves now, but I suppose it had to be done |
Codecov Report
@@ Coverage Diff @@
## master #762 +/- ##
===========================================
- Coverage 92.99% 70.86% -22.14%
===========================================
Files 244 89 -155
Lines 10861 2313 -8548
Branches 1058 317 -741
===========================================
- Hits 10100 1639 -8461
+ Misses 761 674 -87
|
Co-authored-by: Valentin Marchaud <contact@vmarchaud.fr>
Which problem is this PR solving?
This is inline with Remove binary format propagator opentelemetry-specification#426
This is not compatible with OpenCensus, I have added
onHold
label, waiting for #opentelemetry-specification/426 to merge.Will open follow-up PR to remove Binary propagator completely.
Feel free to add reviews
/cc @markwolff @dyladan