-
Notifications
You must be signed in to change notification settings - Fork 445
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
chore: remove the global::shutdown_tracer_provider function #2369
base: main
Are you sure you want to change the base?
chore: remove the global::shutdown_tracer_provider function #2369
Conversation
this should no longer be in the spec, making consistent with logs and metrics
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2369 +/- ##
=======================================
- Coverage 79.5% 79.4% -0.1%
=======================================
Files 123 123
Lines 21479 21485 +6
=======================================
- Hits 17084 17074 -10
- Misses 4395 4411 +16 ☔ View full report in Codecov by Sentry. |
@@ -141,7 +141,9 @@ impl ZipkinPipelineBuilder { | |||
|
|||
/// Install the Zipkin trace exporter pipeline with a simple span processor. | |||
#[allow(deprecated)] | |||
pub fn install_simple(mut self) -> Result<Tracer, TraceError> { | |||
pub fn install_simple( |
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.
not related to this PR - but ZipkinPipelineBuilder
should be replaced with ZipkinBuilder
and the implementation should be made consistent to OTLP as done in #2221. The crate be only responsible to provide builder to create Zipkin exporter, and no convenience wrappers to create TracerProvider / Processors.
Thanks, nicely done. Can you also update the CHANGELOG for opentelemetry crate as this is a breaking change. CI lint failure doesn't look to be related to this PR, but coming from the latest stable toolchain v1.83 released on 28th Nov. I will have a look if no one is checking. |
I'll update as soon as I'm at my computer again 👍 |
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.
LGTM with a changelog entry for the breaking change. Also PR desc can be updated to show how users can migrate.
related to: #1961
Changes
Removing the
opentelemetry::global::shutdown_tracer_provider()
. This should no longer be in the API, making tracing consistent with logs and metrics.Merge requirement checklist
CHANGELOG.md
files updated for non-trivial, user-facing changes