-
Notifications
You must be signed in to change notification settings - Fork 485
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
Move 3rd party propagators and merge exporter into sdk::export #375
Conversation
Codecov Report
@@ Coverage Diff @@
## master #375 +/- ##
==========================================
- Coverage 53.93% 49.13% -4.81%
==========================================
Files 71 66 -5
Lines 6081 5326 -755
==========================================
- Hits 3280 2617 -663
+ Misses 2801 2709 -92
Continue to review full report at Codecov.
|
Hm yeah I think it may be more intuitive to have the jaeger / be propagators with the exporters in their crates rather than contrib. |
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.
Other than that I think this looks good
Also updated proto
…into move � Conflicts: � opentelemetry-otlp/src/lib.rs � opentelemetry-otlp/src/span.rs
@@ -129,3 +129,7 @@ See the [code owners](CODEOWNERS) file. | |||
|
|||
See the [community membership document in OpenTelemetry community | |||
repo](https://github.com/open-telemetry/community/blob/master/community-membership.md). | |||
|
|||
## FAQ | |||
### Where should I put third party propagators/exporters, contrib or standalone crates? |
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.
wanted to clarify the problem that where to put the stuff here. Please let me know if the below descriptions makes sense. @jtescher
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.
Seems good 👍
Fix #361
Fix #340
This PR:
opentelemetry-contrib/propagator
testing
feature so that we could share test util between crates.exporter
intosdk::export
opentelemetry-proto
Upgrade note:
aws
propagators. Addopentelemetry-contrib
in yourCargo.toml
. Choose the feature based on the propagators you need and import them viaopentelemetry_contrib::trace::propagators::<propagators name>
.exporter
module. Such asExportError
. Change the import path fromopentelemetry::exporter
intoopentelemetry::sdk::export
jaeger
propagators. Addopentelemetry-jaeger
in yourCargo.toml
. useopentelemetry_jaeger::Propagator
b3
propagators. Addopentelemetry-zipkin
in yourCargo.toml
. useopentelemetry_zipkin::Propagator
Open question(s):
Where should we put theMoved zipkin and jaeger propagators into their own crate.jaeger
andb3
propagtor? It's intutive to me that we include them as part of theopentelemetry-jaeger
oropentelemetry-zipkin
crate. But Go impl put it incontrib