Create new opentelemetry-jaeger-propagator library #1487

mattbodd · 2024-01-22T20:22:17Z

Description

Create opentelemetry-jaeger-propagator to prepare for opentelemetry-jaeger exporter deprecation (https://opentelemetry.io/blog/2022/jaeger-native-otlp/).

This is the first step taken from #995 (comment) towards resolving #995.

Changes

Create a new opentelemetry-jaeger-propagator library
Moves opentelemetry-jaeger::Propagator module to opentelemetry-jaeger-propagator

Merge requirement checklist

CONTRIBUTING guidelines followed
Unit tests added/updated (if applicable)
Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
Changes in public API reviewed (if applicable)

linux-foundation-easycla · 2024-01-22T20:22:22Z

The committers listed above are authorized under a signed CLA.

✅ login: mattbodd / name: Matthew Boddewyn (6b5ef34, 85edd8f, da1a544, 95506fb, 622fa08, 56f3c31, 666eac4, 63a8b50, 8a3bc31, 2927407, b8a0f82, bfc57a7, 10b78d5, e3945b3, 28b01df, 89ef555, 8008564, 9ed6e99, 766f6a2, b13f7eb, 9fd8e40, 9ae59b4, 889912a, 2993115, 951f8c1, 2cff654, d7622cd, 39178d5, 5add144, 515d084, 2326483, 6711308, 5aabbeb, 36ed1d3, 5d032ae)
✅ login: cijothomas / name: Cijo Thomas (971b553)

…er-propagator/Cargo.toml

…lemetry-jaeger

…ger-propagator module

codecov · 2024-01-23T02:56:11Z

Codecov Report

Attention: 248 lines in your changes are missing coverage. Please review.

Comparison is base (85f678a) 64.0% compared to head (5aabbeb) 63.9%.
Report is 1 commits behind head on main.

❗ Current head 5aabbeb differs from pull request most recent head 971b553. Consider uploading reports for the commit 971b553 to get more accurate results

Files	Patch %	Lines
...try-jaeger-propagator/src/testing/jaeger_api_v2.rs	0.0%	194 Missing ⚠️
opentelemetry-jaeger-propagator/src/testing/mod.rs	0.0%	53 Missing ⚠️
opentelemetry-jaeger-propagator/src/propagator.rs	99.8%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1487     +/-   ##
=======================================
- Coverage   64.0%   63.9%   -0.2%     
=======================================
  Files        142     144      +2     
  Lines      19507   20042    +535     
=======================================
+ Hits       12499   12808    +309     
- Misses      7008    7234    +226

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…ere is a Result::Err

…try_jaeger_propagator::Propagator module

…lementation of TextMapPropagator::fields()

…ELOG.md's to reflect changes

…er-propagator/README.md

mattbodd · 2024-01-23T20:30:09Z

@cijothomas please let me know if this is what you had in mind for #995 (comment). I would especially be interested in getting feedback on the style/approach I've taken (ie: need more docs, additional unit tests are too granular).

opentelemetry-jaeger-propagator/CHANGELOG.md

cijothomas · 2024-01-24T15:01:52Z

opentelemetry-jaeger/CHANGELOG.md

+## v0.21.0
+
+### Changed
+- Move opentelemetry-jaeger::Propagator logic to its own crate (opentelemetry-jaeger-propagator) to prepare for opentelemetry-jaeger exporter deprecation [#1487](https://github.com/open-telemetry/opentelemetry-rust/pull/1487)


lets be more descriptive here to smoothen migration.
This crate previously contained two functionalities - an exporter to Jaeger, and a Jaeger propagator. Going forward, Jaeger propagator is shipped as new crate "crate-name"/link. The JaegerExporter will be deprecated after the next release, and users should use OTLP exporter instead. See "linktoexample" showing OTLPExporter with Jaeger.

The JaegerExporter will be deprecated after the next release, and users should use OTLP exporter instead. See "linktoexample" showing OTLPExporter with Jaeger.

+1. Let's just call out this create will be deprecate with only exporter in it.

cijothomas · 2024-01-24T15:05:12Z

@mattbodd This matches my expectation (left some comments to better describe changelog).
@TommyCpp Could you also review, and see if we have sufficient information for users to migrate and for us to deprecate opentelemetry-jaeger.

cijothomas · 2024-01-24T15:06:19Z

opentelemetry-jaeger/README.md

@@ -41,9 +41,10 @@ exporting telemetry:
 ```rust
 use opentelemetry::global;
 use opentelemetry::trace::Tracer;
+use opentelemetry_jaeger_propagator;


this readme file needs to updated to reflect it no longer has propagator and warning about exporter being removed in the next release.

Updated the opentelemetry-jaeger readme to provide a warning about the upcoming deprecation of the Jaeger exporter and migration of the Jaeger propagator to a new crate.

https://github.com/open-telemetry/opentelemetry-rust/pull/1487/files#diff-d153afbd7102b8218509b0ce0b654c51ba1b8fee4c558c097f01226743a89870R7 need a small fix to remove propagator wording

opentelemetry-jaeger-propagator/src/lib.rs

opentelemetry-jaeger-propagator/Cargo.toml

TommyCpp · 2024-01-25T03:04:45Z

opentelemetry-jaeger/CHANGELOG.md

+## v0.21.0
+
+### Changed
+- Move opentelemetry-jaeger::Propagator logic to its own crate (opentelemetry-jaeger-propagator) to prepare for opentelemetry-jaeger exporter deprecation [#1487](https://github.com/open-telemetry/opentelemetry-rust/pull/1487)


The JaegerExporter will be deprecated after the next release, and users should use OTLP exporter instead. See "linktoexample" showing OTLPExporter with Jaeger.

+1. Let's just call out this create will be deprecate with only exporter in it.

…er-propgator as the propgator functionality is not on track for deprecation at the moment

…phasize the current change, upcoming deprecation of the Jaeger exporter, and provide additional references

… about its upcoming deprecation and the migration of the Jaeger propagator functionality to a standalone crate

…test feature for future use

opentelemetry-jaeger-propagator/README.md

opentelemetry-jaeger/README.md

Co-authored-by: Cijo Thomas <[email protected]>

cijothomas

Thanks.
We may need some more cleanups to make sure no leftovers exist that can cause confusion.

mattbodd · 2024-01-25T19:49:59Z

Thanks @cijothomas for the review and approval. Do we want to wait for TommyCpp and/or others to give a final review + approval before merging or is your approval enough.

cijothomas · 2024-01-25T20:07:31Z

Thanks @cijothomas for the review and approval. Do we want to wait for TommyCpp and/or others to give a final review + approval before merging or is your approval enough.

For trivial changes, one approval is good. This needs one more, to catch things one person may have missed! Lets wait for @TommyCpp to review.

TommyCpp

LGTM. Thanks for working on this

Create new library for opentelemetry-jaeger-propagator

6b5ef34

mattbodd added 9 commits January 22, 2024 20:38

Copy and update opentelemetry-jaeger/Cargo.toml to opentelemetry-jaeg…

85edd8f

…er-propagator/Cargo.toml

Copy files from opentelemetry-jaeger to opentelemetry-jaeger-propagator

da1a544

Reorder members to place opentelemetry-jaeger-propagator under opente…

95506fb

…lemetry-jaeger

Remove opentelemetry-jaeger exporter logic from the opentelemetry-jae…

622fa08

…ger-propagator module

Remove examples that are still housed within opentelemetry-jaeger

56f3c31

Remove exporter module from opentelemetry-jaeger-propagator module

666eac4

Remove unused opentelemetry-jaeger-propagator features

63a8b50

Remove irrelevant documentation comments

8a3bc31

Remove unused tokio and async-std runtime features from the full list

2927407

mattbodd changed the title ~~[WIP] Create new library for opentelemetry-jaeger-propagator~~ [WIP] Create new opentelemetry-jaeger-propagator library Jan 23, 2024

mattbodd added 2 commits January 23, 2024 02:29

Add opentelemetry-jaeger-propagator default to lint.sh

b8a0f82

Remove integration test

bfc57a7

mattbodd added 10 commits January 23, 2024 03:17

Add unit test for various propgator creation methods

10b78d5

Add unit test coverage for all impl Propagator methods

e3945b3

Prefer to assert value of Result::Err rather than just assert that th…

28b01df

…ere is a Result::Err

Remove opentelemetry_jaeger::Propagator module in favor of openteleme…

89ef555

…try_jaeger_propagator::Propagator module

Add test coverage for opentelemetry_jaeger_propagator::Propagator imp…

8008564

…lementation of TextMapPropagator::fields()

Test clone and debug of opentelemetry_jaeger_propagator::Propagator

9ed6e99

Update opentelemetry-jaeger and opentelemetry-jaeger-propagator CHANG…

766f6a2

…ELOG.md's to reflect changes

Remove unnecessary explicit into_iter

b13f7eb

Remove references to integration tests that don't exist

9fd8e40

Aggressively remove unused dependencies and update opentelemetry-jaeg…

9ae59b4

…er-propagator/README.md

mattbodd marked this pull request as ready for review January 23, 2024 20:26

mattbodd requested a review from a team January 23, 2024 20:26

mattbodd changed the title ~~[WIP] Create new opentelemetry-jaeger-propagator library~~ Create new opentelemetry-jaeger-propagator library Jan 23, 2024

Merge branch 'main' into deprecate_opentelemetry_jaeger_exporter

889912a

cijothomas reviewed Jan 24, 2024

View reviewed changes

opentelemetry-jaeger-propagator/CHANGELOG.md Outdated Show resolved Hide resolved

cijothomas reviewed Jan 24, 2024

View reviewed changes

TommyCpp reviewed Jan 25, 2024

View reviewed changes

mattbodd added 8 commits January 25, 2024 06:22

Remove opetelemetry-jaeger deprecation notice from opentelemetry-jaeg…

2993115

…er-propgator as the propgator functionality is not on track for deprecation at the moment

Make opentelemetry-jaeger-propagator CHANGELOG more descriptive

951f8c1

Add more of a description to the opentelemetry-jaeger CHANGELOG to em…

2cff654

…phasize the current change, upcoming deprecation of the Jaeger exporter, and provide additional references

Enrich the opentelemetry-jaeger README to contain an explicit warning…

d7622cd

… about its upcoming deprecation and the migration of the Jaeger propagator functionality to a standalone crate

Move the opentelemetry-jaeger-propagator module from to its own file

39178d5

Remove empty full and default features in favor of empty integration_…

5add144

…test feature for future use

Add back default feature set to satisfy lint script

515d084

Merge branch 'main' into deprecate_opentelemetry_jaeger_exporter

2326483

cijothomas reviewed Jan 25, 2024

View reviewed changes

opentelemetry-jaeger-propagator/README.md Outdated Show resolved Hide resolved

cijothomas reviewed Jan 25, 2024

View reviewed changes

opentelemetry-jaeger/README.md Outdated Show resolved Hide resolved

mattbodd and others added 3 commits January 25, 2024 09:51

Update opentelemetry-jaeger-propagator/README.md

6711308

Co-authored-by: Cijo Thomas <[email protected]>

Add example of exporting OTLP to Jaeger to opentelemetry-jaeger README

5aabbeb

Merge branch 'main' into deprecate_opentelemetry_jaeger_exporter

36ed1d3

cijothomas approved these changes Jan 25, 2024

View reviewed changes

Merge branch 'main' into deprecate_opentelemetry_jaeger_exporter

5d032ae

TommyCpp approved these changes Jan 26, 2024

View reviewed changes

Merge branch 'main' into deprecate_opentelemetry_jaeger_exporter

971b553

cijothomas merged commit 0101233 into open-telemetry:main Jan 26, 2024
12 of 13 checks passed

mattbodd deleted the deprecate_opentelemetry_jaeger_exporter branch January 26, 2024 17:07

hdost mentioned this pull request Feb 18, 2024

Bump versions to prepare for release v0.22 #1539

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create new opentelemetry-jaeger-propagator library #1487

Create new opentelemetry-jaeger-propagator library #1487

mattbodd commented Jan 22, 2024 •

edited

Loading

linux-foundation-easycla bot commented Jan 22, 2024 •

edited

Loading

codecov bot commented Jan 23, 2024 •

edited

Loading

mattbodd commented Jan 23, 2024

cijothomas Jan 24, 2024 •

edited

Loading

TommyCpp Jan 25, 2024

cijothomas commented Jan 24, 2024

cijothomas Jan 24, 2024

mattbodd Jan 25, 2024

cijothomas Jan 25, 2024

TommyCpp Jan 25, 2024

cijothomas left a comment

mattbodd commented Jan 25, 2024

cijothomas commented Jan 25, 2024

TommyCpp left a comment

Create new opentelemetry-jaeger-propagator library #1487

Create new opentelemetry-jaeger-propagator library #1487

Conversation

mattbodd commented Jan 22, 2024 • edited Loading

Description

Changes

Merge requirement checklist

linux-foundation-easycla bot commented Jan 22, 2024 • edited Loading

codecov bot commented Jan 23, 2024 • edited Loading

Codecov Report

mattbodd commented Jan 23, 2024

cijothomas Jan 24, 2024 • edited Loading

Choose a reason for hiding this comment

TommyCpp Jan 25, 2024

Choose a reason for hiding this comment

cijothomas commented Jan 24, 2024

cijothomas Jan 24, 2024

Choose a reason for hiding this comment

mattbodd Jan 25, 2024

Choose a reason for hiding this comment

cijothomas Jan 25, 2024

Choose a reason for hiding this comment

TommyCpp Jan 25, 2024

Choose a reason for hiding this comment

cijothomas left a comment

Choose a reason for hiding this comment

mattbodd commented Jan 25, 2024

cijothomas commented Jan 25, 2024

TommyCpp left a comment

Choose a reason for hiding this comment

mattbodd commented Jan 22, 2024 •

edited

Loading

linux-foundation-easycla bot commented Jan 22, 2024 •

edited

Loading

codecov bot commented Jan 23, 2024 •

edited

Loading

cijothomas Jan 24, 2024 •

edited

Loading