Deprecate JaegerPropagator in OpenTelemetry.Extensions.Propagators#6819
Conversation
- Add [Obsolete] attributes to JaegerPropagator class and all public members - Update README.md with deprecation notice and migration guidance - Follows OpenTelemetry specification change (opentelemetry-specification#4827) Fixes
|
Hi @Kielek, I've implemented the deprecation of
I have a few questions:
This is my first contribution here, so I'd appreciate any feedback or guidance. Happy to make adjustments as needed! Thanks! |
|
I've done this |
|
the PR is now updated and ready for your review. Please let me know if there's anything else that needs to be adjusted. |
|
There's a bunch of compiler errors that need addressing. |
|
@martincostello I've fixed the compiler errors. CI completed but shows 35 failing checks. I'm reviewing the error messages to see if they're related to my changes or infrastructure issues. Could you help clarify which failures (if any) are related to the deprecation changes? I want to make sure I address the right issues. Thanks! |
|
They're all related to the changes you've made. |
I'm reviewing the error logs now. I'll fix all issues related to my changes. Should I focus on any particular checks first (markdown linting, sanity check, build errors, etc.)? |
|
The order doesn't really matter. They'd all have to be resolved for merge. |
|
I'm having trouble seeing the specific error messages from the failing CI checks. The build is clean locally (0 warnings/errors in Release, all 54 tests pass on net10.0), but CI shows 35 failing checks. Could you help me understand:
I want to fix all the issues but need to see the actual errors to address them. Thanks for your help! @martincostello |
|
Also note that in CI all warnings as treated as errors, so if you have any warnings in your build locally, those also need to be fixed. |
The bare minimum local check is to execute solution build in the Release mode. If it fails locally, it will fail also in the CI. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6819 +/- ##
==========================================
+ Coverage 86.91% 86.99% +0.08%
==========================================
Files 262 263 +1
Lines 12355 12490 +135
==========================================
+ Hits 10738 10866 +128
- Misses 1617 1624 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
All CI checks have passed now.. Fixed the CS0809 error by adding pragma warnings around the The PR is ready for review. Thanks! |
Kielek
left a comment
There was a problem hiding this comment.
LGTM. Thanks.
Note for myseld/other approvers: Jaeger propagator was never part of OpenTelemetry.Context.Propagation namespace. It was implemented only in this package.
|
Thanks for the review and approval.. If everything looks good here, could you please merge this PR? cc: @martincostello , @Kielek |
|
@Kielek Merging is blocked due to validation failure. Have you seen this earlier?
|
Nevermind, it took few minutes to show merge button. |
|
Thanks for the review and merge! Appreciate it 🙌 |


Description
This PR deprecates the
JaegerPropagatorclass and all its public members in theOpenTelemetry.Extensions.Propagatorspackage, following the OpenTelemetry specification change (opentelemetry-specification#4827) and Jaeger's migration guide.Changes
[Obsolete]attributes toJaegerPropagatorclass[Obsolete]attributes to all public members (Fields,Extract,Inject)#pragma warning disable CS0809to handle obsolete member overridesRelated Issues
Fixes #6814
Migration Path
Users should migrate to
TraceContextPropagatorfrom theOpenTelemetrypackage instead. See:Testing
JaegerPropagatorScreenshot: