From 82fa485359745e5bda9eadc9c080bb84f98a4649 Mon Sep 17 00:00:00 2001 From: Zhongyang Wu Date: Thu, 30 May 2024 10:04:08 -0700 Subject: [PATCH] feat: clean up trace SDK APIs (#1755) Co-authored-by: Cijo Thomas --- examples/tracing-jaeger/src/main.rs | 2 +- opentelemetry-appender-tracing/src/layer.rs | 7 ++++--- .../tests/integration_test/tests/traces.rs | 2 +- opentelemetry-sdk/CHANGELOG.md | 4 ++++ opentelemetry-sdk/benches/context.rs | 7 +++++-- opentelemetry-sdk/benches/log.rs | 5 +++-- opentelemetry-sdk/benches/span_builder.rs | 2 +- opentelemetry-sdk/benches/trace.rs | 4 ++-- opentelemetry-sdk/src/trace/config.rs | 2 ++ opentelemetry-sdk/src/trace/provider.rs | 4 ++-- opentelemetry-sdk/src/trace/tracer.rs | 4 ++-- opentelemetry-zipkin/src/exporter/mod.rs | 11 ++--------- scripts/integration_tests.sh | 4 ---- stress/src/traces.rs | 2 +- 14 files changed, 30 insertions(+), 30 deletions(-) diff --git a/examples/tracing-jaeger/src/main.rs b/examples/tracing-jaeger/src/main.rs index 5ddb5a843c..e6c3dfdb2b 100644 --- a/examples/tracing-jaeger/src/main.rs +++ b/examples/tracing-jaeger/src/main.rs @@ -19,7 +19,7 @@ fn init_tracer_provider() -> Result Result { .tracing() .with_exporter(opentelemetry_otlp::new_exporter().tonic()) .with_trace_config( - sdktrace::config().with_resource(Resource::new(vec![KeyValue::new( + sdktrace::Config::default().with_resource(Resource::new(vec![KeyValue::new( opentelemetry_semantic_conventions::resource::SERVICE_NAME, "basic-otlp-tracing-example", )])), diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index ccd7fd1e36..25c4775284 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -27,6 +27,10 @@ reference or owned`LogData`. If the exporter needs to process the log data asynchronously, it should clone the log data to ensure it can be safely processed without lifetime issues. +- Clean up public methods in SDK. + - [`TracerProvider::span_processors`] and [`TracerProvider::config`] was removed as it's not part of the spec. + - Added `non_exhaustive` annotation to [`trace::Config`]. Marked [`config`] as deprecated since it's only a wrapper for `Config::default` + - Removed [`Tracer::tracer_provder`] and [`Tracer::instrument_libraries`] as it's not part of the spec. - **Breaking** [#1830](https://github.com/open-telemetry/opentelemetry-rust/pull/1830/files) [Traces SDK] Improves performance by sending Resource information to processors (and exporters) once, instead of sending with every log. If you are an author diff --git a/opentelemetry-sdk/benches/context.rs b/opentelemetry-sdk/benches/context.rs index f5a1f7e2df..3ef494990c 100644 --- a/opentelemetry-sdk/benches/context.rs +++ b/opentelemetry-sdk/benches/context.rs @@ -10,7 +10,8 @@ use opentelemetry::{ }; use opentelemetry_sdk::{ export::trace::{ExportResult, SpanData, SpanExporter}, - trace::{config, Sampler, TracerProvider}, + trace, + trace::{Sampler, TracerProvider}, }; #[cfg(not(target_os = "windows"))] use pprof::criterion::{Output, PProfProfiler}; @@ -126,7 +127,9 @@ impl Display for Environment { fn parent_sampled_tracer(inner_sampler: Sampler) -> (TracerProvider, BoxedTracer) { let provider = TracerProvider::builder() - .with_config(config().with_sampler(Sampler::ParentBased(Box::new(inner_sampler)))) + .with_config( + trace::Config::default().with_sampler(Sampler::ParentBased(Box::new(inner_sampler))), + ) .with_simple_exporter(NoopExporter) .build(); let tracer = provider.tracer(module_path!()); diff --git a/opentelemetry-sdk/benches/log.rs b/opentelemetry-sdk/benches/log.rs index 72f205c5e0..21b807cbf7 100644 --- a/opentelemetry-sdk/benches/log.rs +++ b/opentelemetry-sdk/benches/log.rs @@ -17,7 +17,8 @@ use opentelemetry::trace::TracerProvider as _; use opentelemetry::Key; use opentelemetry_sdk::export::logs::{LogData, LogExporter}; use opentelemetry_sdk::logs::{Logger, LoggerProvider}; -use opentelemetry_sdk::trace::{config, Sampler, TracerProvider}; +use opentelemetry_sdk::trace; +use opentelemetry_sdk::trace::{Sampler, TracerProvider}; #[derive(Debug)] struct VoidExporter; @@ -51,7 +52,7 @@ fn log_benchmark_group(c: &mut Criterion, name: &str, f: F) { // setup tracing as well. let tracer_provider = TracerProvider::builder() - .with_config(config().with_sampler(Sampler::AlwaysOn)) + .with_config(trace::Config::default().with_sampler(Sampler::AlwaysOn)) .build(); let tracer = tracer_provider.tracer("bench-tracer"); diff --git a/opentelemetry-sdk/benches/span_builder.rs b/opentelemetry-sdk/benches/span_builder.rs index de5c1fd235..10fd4addfe 100644 --- a/opentelemetry-sdk/benches/span_builder.rs +++ b/opentelemetry-sdk/benches/span_builder.rs @@ -54,7 +54,7 @@ fn span_builder_benchmark_group(c: &mut Criterion) { fn not_sampled_provider() -> (sdktrace::TracerProvider, sdktrace::Tracer) { let provider = sdktrace::TracerProvider::builder() - .with_config(sdktrace::config().with_sampler(sdktrace::Sampler::AlwaysOff)) + .with_config(sdktrace::Config::default().with_sampler(sdktrace::Sampler::AlwaysOff)) .with_simple_exporter(NoopExporter) .build(); let tracer = provider.tracer("not-sampled"); diff --git a/opentelemetry-sdk/benches/trace.rs b/opentelemetry-sdk/benches/trace.rs index 93bdf3859a..ed4d3add82 100644 --- a/opentelemetry-sdk/benches/trace.rs +++ b/opentelemetry-sdk/benches/trace.rs @@ -70,7 +70,7 @@ fn trace_benchmark_group(c: &mut Criterion, name: &str group.bench_function("always-sample", |b| { let provider = sdktrace::TracerProvider::builder() - .with_config(sdktrace::config().with_sampler(sdktrace::Sampler::AlwaysOn)) + .with_config(sdktrace::Config::default().with_sampler(sdktrace::Sampler::AlwaysOn)) .with_simple_exporter(VoidExporter) .build(); let always_sample = provider.tracer("always-sample"); @@ -80,7 +80,7 @@ fn trace_benchmark_group(c: &mut Criterion, name: &str group.bench_function("never-sample", |b| { let provider = sdktrace::TracerProvider::builder() - .with_config(sdktrace::config().with_sampler(sdktrace::Sampler::AlwaysOff)) + .with_config(sdktrace::Config::default().with_sampler(sdktrace::Sampler::AlwaysOff)) .with_simple_exporter(VoidExporter) .build(); let never_sample = provider.tracer("never-sample"); diff --git a/opentelemetry-sdk/src/trace/config.rs b/opentelemetry-sdk/src/trace/config.rs index 88aa925286..368f69dacd 100644 --- a/opentelemetry-sdk/src/trace/config.rs +++ b/opentelemetry-sdk/src/trace/config.rs @@ -10,12 +10,14 @@ use std::env; use std::str::FromStr; /// Default trace configuration +#[deprecated(since = "0.23.0", note = "Use Config::default() instead")] pub fn config() -> Config { Config::default() } /// Tracer configuration #[derive(Debug)] +#[non_exhaustive] pub struct Config { /// The sampler that the sdk should use pub sampler: Box, diff --git a/opentelemetry-sdk/src/trace/provider.rs b/opentelemetry-sdk/src/trace/provider.rs index 560d37dae9..701a784f90 100644 --- a/opentelemetry-sdk/src/trace/provider.rs +++ b/opentelemetry-sdk/src/trace/provider.rs @@ -62,12 +62,12 @@ impl TracerProvider { } /// Span processors associated with this provider - pub fn span_processors(&self) -> &[Box] { + pub(crate) fn span_processors(&self) -> &[Box] { &self.inner.processors } /// Config associated with this tracer - pub fn config(&self) -> &crate::trace::Config { + pub(crate) fn config(&self) -> &crate::trace::Config { &self.inner.config } diff --git a/opentelemetry-sdk/src/trace/tracer.rs b/opentelemetry-sdk/src/trace/tracer.rs index 2ca1af30c0..0749937cd7 100644 --- a/opentelemetry-sdk/src/trace/tracer.rs +++ b/opentelemetry-sdk/src/trace/tracer.rs @@ -55,12 +55,12 @@ impl Tracer { } /// TracerProvider associated with this tracer. - pub fn provider(&self) -> Option { + pub(crate) fn provider(&self) -> Option { self.provider.upgrade().map(TracerProvider::new) } /// Instrumentation library information of this tracer. - pub fn instrumentation_library(&self) -> &InstrumentationLibrary { + pub(crate) fn instrumentation_library(&self) -> &InstrumentationLibrary { &self.instrumentation_lib } diff --git a/opentelemetry-zipkin/src/exporter/mod.rs b/opentelemetry-zipkin/src/exporter/mod.rs index 0bd2d19016..88edcfb025 100644 --- a/opentelemetry-zipkin/src/exporter/mod.rs +++ b/opentelemetry-zipkin/src/exporter/mod.rs @@ -106,10 +106,7 @@ impl ZipkinPipelineBuilder { )); cfg } else { - Config { - resource: Cow::Owned(Resource::empty()), - ..Default::default() - } + Config::default().with_resource(Resource::empty()) }; (config, Endpoint::new(service_name, self.service_addr)) } else { @@ -119,11 +116,7 @@ impl ZipkinPipelineBuilder { .unwrap() .to_string(); ( - Config { - // use a empty resource to prevent TracerProvider to assign a service name. - resource: Cow::Owned(Resource::empty()), - ..Default::default() - }, + Config::default().with_resource(Resource::empty()), Endpoint::new(service_name, self.service_addr), ) } diff --git a/scripts/integration_tests.sh b/scripts/integration_tests.sh index cff6834f84..361098a5fc 100755 --- a/scripts/integration_tests.sh +++ b/scripts/integration_tests.sh @@ -1,5 +1 @@ -#COMPOSE_FILE=./opentelemetry-jaeger/tests/docker-compose.yaml -#docker-compose -f $COMPOSE_FILE down -v && -#docker-compose -f $COMPOSE_FILE up --build --abort-on-container-exit --exit-code-from opentelemetry-jaeger - cargo test ./opentelemetry-otlp/tests/integration_test/tests -- --ignored diff --git a/stress/src/traces.rs b/stress/src/traces.rs index 9f9065d1a5..3924dc9e2b 100644 --- a/stress/src/traces.rs +++ b/stress/src/traces.rs @@ -20,7 +20,7 @@ mod throughput; lazy_static! { static ref PROVIDER: sdktrace::TracerProvider = sdktrace::TracerProvider::builder() - .with_config(sdktrace::config().with_sampler(sdktrace::Sampler::AlwaysOn)) + .with_config(sdktrace::Config::default().with_sampler(sdktrace::Sampler::AlwaysOn)) .with_span_processor(NoOpSpanProcessor {}) .build(); static ref TRACER: sdktrace::Tracer = PROVIDER.tracer("stress");