chore(tracer): clean up span builder api

Author: bantonsson

opentelemetry-sdk/CHANGELOG.md

@@ -2,8 +2,11 @@
 
 ## vNext
 
-### Added
 - Added `Resource::get_ref(&self, key: &Key) -> Option<&Value>` to allow retrieving a reference to a resource value without cloning.
+- **Breaking** Removed the following public hidden methods from the `SdkTracer` [#3227][3227]:
+  - `id_generator`, `should_sample`
+
+[3227]: https://github.com/open-telemetry/opentelemetry-rust/pull/3227
 
 ## 0.31.0
 

opentelemetry-sdk/src/trace/tracer.rs

@@ -10,11 +10,11 @@
 use crate::trace::{
     provider::SdkTracerProvider,
     span::{Span, SpanData},
-    IdGenerator, ShouldSample, SpanEvents, SpanLimits, SpanLinks,
+    SpanEvents, SpanLimits, SpanLinks,
 };
 use opentelemetry::{
     trace::{
-        SamplingDecision, Span as _, SpanBuilder, SpanContext, SpanKind, TraceContextExt,
+        SamplingDecision, Span as _, SpanBuilder, SpanContext, SpanKind, Status, TraceContextExt,
         TraceFlags,
     },
     Context, InstrumentationScope, KeyValue,
@@ -105,14 +105,11 @@ impl SdkTracer {
         let SpanBuilder {
             name,
             start_time,
-            end_time,
             events,
-            status,
             ..
         } = builder;
 
         let start_time = start_time.unwrap_or_else(opentelemetry::time::now);
-        let end_time = end_time.unwrap_or(start_time);
         let spans_events_limit = span_limits.max_events_per_span as usize;
         let span_events: SpanEvents = if let Some(mut events) = events {
             let dropped_count = events.len().saturating_sub(spans_events_limit);
@@ -141,33 +138,17 @@ impl SdkTracer {
                 span_kind: builder.span_kind.take().unwrap_or(SpanKind::Internal),
                 name,
                 start_time,
-                end_time,
+                end_time: start_time,
                 attributes: attribute_options,
                 dropped_attributes_count,
                 events: span_events,
                 links: span_links,
-                status,
+                status: Status::default(),
             }),
             self.clone(),
             span_limits,
         )
     }
-
-    /// The [`IdGenerator`] associated with this tracer.
-    ///
-    // Note: this is necessary for tracing-opentelemetry's `PreSampledTracer`.
-    #[doc(hidden)]
-    pub fn id_generator(&self) -> &dyn IdGenerator {
-        &*self.provider.config().id_generator
-    }
-
-    /// The [`ShouldSample`] associated with this tracer.
-    ///
-    // Note: this is necessary for tracing-opentelemetry's `PreSampledTracer`.
-    #[doc(hidden)]
-    pub fn should_sample(&self) -> &dyn ShouldSample {
-        &*self.provider.config().sampler
-    }
 }
 
 impl opentelemetry::trace::Tracer for SdkTracer {
@@ -181,7 +162,7 @@ impl opentelemetry::trace::Tracer for SdkTracer {
     /// trace. A span is said to be a _root span_ if it does not have a parent. Each
     /// trace includes a single root span, which is the shared ancestor of all other
     /// spans in the trace.
-    fn build_with_context(&self, mut builder: SpanBuilder, parent_cx: &Context) -> Self::Span {
+    fn build_with_context(&self, builder: SpanBuilder, parent_cx: &Context) -> Self::Span {
         if parent_cx.is_telemetry_suppressed() {
             return Span::new(
                 SpanContext::empty_context(),
@@ -203,10 +184,7 @@ impl opentelemetry::trace::Tracer for SdkTracer {
         }
 
         let config = provider.config();
-        let span_id = builder
-            .span_id
-            .take()
-            .unwrap_or_else(|| config.id_generator.new_span_id());
+        let span_id = config.id_generator.new_span_id();
         let trace_id;
         let mut psc = &SpanContext::empty_context();
 
@@ -221,25 +199,17 @@ impl opentelemetry::trace::Tracer for SdkTracer {
             trace_id = sc.trace_id();
             psc = sc;
         } else {
-            trace_id = builder
-                .trace_id
-                .unwrap_or_else(|| config.id_generator.new_trace_id());
+            trace_id = config.id_generator.new_trace_id();
         };
 
-        // In order to accommodate use cases like `tracing-opentelemetry` we there is the ability
-        // to use pre-sampling. Otherwise, the standard method of sampling is followed.
-        let samplings_result = if let Some(sr) = builder.sampling_result.take() {
-            sr
-        } else {
-            config.sampler.should_sample(
-                Some(parent_cx),
-                trace_id,
-                &builder.name,
-                builder.span_kind.as_ref().unwrap_or(&SpanKind::Internal),
-                builder.attributes.as_ref().unwrap_or(&Vec::new()),
-                builder.links.as_deref().unwrap_or(&[]),
-            )
-        };
+        let samplings_result = config.sampler.should_sample(
+            Some(parent_cx),
+            trace_id,
+            &builder.name,
+            builder.span_kind.as_ref().unwrap_or(&SpanKind::Internal),
+            builder.attributes.as_ref().unwrap_or(&Vec::new()),
+            builder.links.as_deref().unwrap_or(&[]),
+        );
 
         let trace_flags = parent_cx.span().span_context().trace_flags();
         let trace_state = samplings_result.trace_state;

opentelemetry/CHANGELOG.md

@@ -3,6 +3,11 @@
 ## vNext
 
 - Add `reserve` method to `opentelemetry::propagation::Injector` to hint at the number of elements that will be added to avoid multiple resize operations of the underlying data structure. Has an empty default implementation.
+- **Breaking** Removed the following public fields and methods from the `SpanBuilder` [#3227][3227]:
+  - `trace_id`, `span_id`, `end_time`, `status`, `sampling_result`
+  - `with_trace_id`, `with_span_id`, `with_end_time`, `with_status`, `with_sampling_result`
+
+[3227]: https://github.com/open-telemetry/opentelemetry-rust/pull/3227
 
 ## v0.31.0
 

opentelemetry/src/trace/tracer.rs

@@ -1,6 +1,6 @@
 use crate::{
-    trace::{Event, Link, Span, SpanKind, Status, TraceContextExt, TraceState},
-    Context, KeyValue, SpanId, TraceId,
+    trace::{Event, Link, Span, SpanKind, TraceContextExt, TraceState},
+    Context, KeyValue,
 };
 use std::borrow::Cow;
 use std::time::SystemTime;
@@ -240,12 +240,6 @@ pub trait Tracer {
 /// ```
 #[derive(Clone, Debug, Default)]
 pub struct SpanBuilder {
-    /// Trace id, useful for integrations with external tracing systems.
-    pub trace_id: Option<TraceId>,
-
-    /// Span id, useful for integrations with external tracing systems.
-    pub span_id: Option<SpanId>,
-
     /// Span kind
     pub span_kind: Option<SpanKind>,
 
@@ -255,9 +249,6 @@ pub struct SpanBuilder {
     /// Span start time
     pub start_time: Option<SystemTime>,
 
-    /// Span end time
-    pub end_time: Option<SystemTime>,
-
     /// Span attributes that are provided at the span creation time.
     /// More attributes can be added afterwards.
     /// Providing duplicate keys will result in multiple attributes
@@ -269,12 +260,6 @@ pub struct SpanBuilder {
 
     /// Span Links
     pub links: Option<Vec<Link>>,
-
-    /// Span status
-    pub status: Status,
-
-    /// Sampling result
-    pub sampling_result: Option<SamplingResult>,
 }
 
 /// SpanBuilder methods
@@ -287,22 +272,6 @@ impl SpanBuilder {
         }
     }
 
-    /// Specify trace id to use if no parent context exists
-    pub fn with_trace_id(self, trace_id: TraceId) -> Self {
-        SpanBuilder {
-            trace_id: Some(trace_id),
-            ..self
-        }
-    }
-
-    /// Assign span id
-    pub fn with_span_id(self, span_id: SpanId) -> Self {
-        SpanBuilder {
-            span_id: Some(span_id),
-            ..self
-        }
-    }
-
     /// Assign span kind
     pub fn with_kind(self, span_kind: SpanKind) -> Self {
         SpanBuilder {
@@ -319,14 +288,6 @@ impl SpanBuilder {
         }
     }
 
-    /// Assign span end time
-    pub fn with_end_time<T: Into<SystemTime>>(self, end_time: T) -> Self {
-        SpanBuilder {
-            end_time: Some(end_time.into()),
-            ..self
-        }
-    }
-
     /// Assign span attributes from an iterable.
     /// Providing duplicate keys will result in multiple attributes
     /// with the same key, as there is no de-duplication performed.    
@@ -357,19 +318,6 @@ impl SpanBuilder {
         }
     }
 
-    /// Assign status code
-    pub fn with_status(self, status: Status) -> Self {
-        SpanBuilder { status, ..self }
-    }
-
-    /// Assign sampling result
-    pub fn with_sampling_result(self, sampling_result: SamplingResult) -> Self {
-        SpanBuilder {
-            sampling_result: Some(sampling_result),
-            ..self
-        }
-    }
-
     /// Builds a span with the given tracer from this configuration.
     pub fn start<T: Tracer>(self, tracer: &T) -> T::Span {
         Context::map_current(|cx| tracer.build_with_context(self, cx))