Propagate baggage through opentracing bridge #4021
Conversation
shubham-bansal96
requested review from
jmacd,
MrAlias,
Aneurysm9,
evantorrie,
XSAM,
dashpole,
MadVikingGod,
pellared,
hanyuancheung and
dmathieu
as code owners
|
|
shubham-bansal96
changed the title
shubham-bansal96
changed the title
Signed-off-by: Shubham Bansal <shubhambansaliimtgn@gmail.com>
shubham-bansal96
force-pushed
the
issue-2793
branch
from
04d6677
to
2ec2755
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4021 +/- ##
======================================
Coverage 82.1% 82.1%
======================================
Files 175 175
Lines 12977 13082 +105
======================================
+ Hits 10655 10752 +97
- Misses 2102 2108 +6
- Partials 220 222 +2
|
| @@ -63,6 +64,11 @@ func (tc TraceContext) Inject(ctx context.Context, carrier TextMapCarrier) { | |||
| sc.SpanID(), | |||
| flags) | |||
| carrier.Set(traceparentHeader, h) | |||
|
|
|||
| bStr := baggage.FromContext(ctx).String() | |||
There was a problem hiding this comment.
There is a separate baggage propagator that should be used to propagate baggage. This propagator is only for propagating trace context in the w3c tracecontext format. A composite propagator should be used if you want to propagate both trace context and baggage and should not require any changes.
There was a problem hiding this comment.
Thanks @Aneurysm9 for letting me know, if no changes are required then could you please close this issue ?
#2793
There was a problem hiding this comment.
I'm not sure that the issue is invalid. This attempt to fix it, however, is.
There was a problem hiding this comment.
@Aneurysm9
I wanted you to close the issue, but i think you have mistakenly closed my PR. If my changes doesn't looks good to you then i can try different approach to work on this issue.
I would request you to please open this PR, so i could start working on this issue again. And it will be really helpful for me, if you could please confirm whether this is a valid issue(#2793) or not ??
There was a problem hiding this comment.
I intended to close this PR. This approach is not viable. Please open a new PR with a new approach if you would like to try again.
The linked issue appears to be valid from the user reports, but I have not personally validated it. If you present a PR that fixes it in an appropriate manner and includes tests that demonstrate it has been fixed then the issue can be resolved when that PR is merged. Alternately, a PR with tests demonstrating that the functionality is working as expected would also suffice.
Resolves #2793
These changes will allow to propagate baggage through opentracing bridge.