Local span perf improvements (#505)

Currently span data recording performance is capped because all changes are synchronized behind a mutex. This patch changes the span API to take mutable references instead, and only requires the use of mutexes when setting values through a shared reference to the active span in the context API.
open-telemetry · Mar 31, 2021 · e169d8d · e169d8d
1 parent 4af37e1
commit e169d8d
Show file tree

Hide file tree

Showing 22 changed files with 257 additions and 173 deletions.
diff --git a/examples/aws-xray/src/server.rs b/examples/aws-xray/src/server.rs
@@ -22,7 +22,7 @@ async fn handle(req: Request<Body>) -> Result<Response<Body>, Infallible> {
         .to_str()
         .unwrap();
 
-    let span = global::tracer("example/server").start_with_context("hello", parent_context);
+    let mut span = global::tracer("example/server").start_with_context("hello", parent_context);
     span.add_event(format!("Handling - {}", x_amzn_trace_id), Vec::new());
 
     Ok(Response::new(

diff --git a/examples/datadog/src/main.rs b/examples/datadog/src/main.rs
@@ -10,7 +10,7 @@ use std::time::Duration;
 
 fn bar() {
     let tracer = global::tracer("component-bar");
-    let span = tracer.start("bar");
+    let mut span = tracer.start("bar");
     span.set_attribute(Key::new("span.type").string("sql"));
     span.set_attribute(Key::new("sql.query").string("SELECT * FROM table"));
     thread::sleep(Duration::from_millis(6));

diff --git a/examples/grpc/src/server.rs b/examples/grpc/src/server.rs
@@ -47,7 +47,7 @@ impl Greeter for MyGreeter {
     ) -> Result<Response<HelloReply>, Status> {
         let parent_cx =
             global::get_text_map_propagator(|prop| prop.extract(&MetadataMap(request.metadata())));
-        let span = global::tracer("greeter").start_with_context("Processing reply", parent_cx);
+        let mut span = global::tracer("greeter").start_with_context("Processing reply", parent_cx);
         span.set_attribute(KeyValue::new("request", format!("{:?}", request)));
 
         // Return an instance of type HelloReply

diff --git a/examples/http/src/server.rs b/examples/http/src/server.rs
@@ -16,7 +16,7 @@ async fn handle(req: Request<Body>) -> Result<Response<Body>, Infallible> {
     let parent_cx = global::get_text_map_propagator(|propagator| {
         propagator.extract(&HeaderExtractor(req.headers()))
     });
-    let span = global::tracer("example/server").start_with_context("hello", parent_cx);
+    let mut span = global::tracer("example/server").start_with_context("hello", parent_cx);
     span.add_event("handling this...".to_string(), Vec::new());
 
     Ok(Response::new("Hello, World!".into()))

diff --git a/examples/zipkin/src/main.rs b/examples/zipkin/src/main.rs
@@ -6,7 +6,7 @@ use std::time::Duration;
 
 fn bar() {
     let tracer = global::tracer("component-bar");
-    let span = tracer.start("bar");
+    let mut span = tracer.start("bar");
     thread::sleep(Duration::from_millis(6));
     span.end()
 }

diff --git a/opentelemetry-aws/src/lib.rs b/opentelemetry-aws/src/lib.rs
@@ -146,7 +146,8 @@ pub mod trace {
 
     impl TextMapPropagator for XrayPropagator {
         fn inject_context(&self, cx: &Context, injector: &mut dyn Injector) {
-            let span_context = cx.span().span_context();
+            let span = cx.span();
+            let span_context = span.span_context();
             if span_context.is_valid() {
                 let xray_trace_id: XrayTraceId = span_context.trace_id().into();
 

diff --git a/opentelemetry-datadog/src/lib.rs b/opentelemetry-datadog/src/lib.rs
@@ -260,7 +260,8 @@ mod propagator {
 
     impl TextMapPropagator for DatadogPropagator {
         fn inject_context(&self, cx: &Context, injector: &mut dyn Injector) {
-            let span_context = cx.span().span_context();
+            let span = cx.span();
+            let span_context = span.span_context();
             if span_context.is_valid() {
                 injector.set(
                     DATADOG_TRACE_ID_HEADER,

diff --git a/opentelemetry-jaeger/src/lib.rs b/opentelemetry-jaeger/src/lib.rs
@@ -331,7 +331,8 @@ mod propagator {
 
     impl TextMapPropagator for Propagator {
         fn inject_context(&self, cx: &Context, injector: &mut dyn Injector) {
-            let span_context = cx.span().span_context();
+            let span = cx.span();
+            let span_context = span.span_context();
             if span_context.is_valid() {
                 let flag: u8 = if span_context.is_sampled() {
                     if span_context.is_debug() {

diff --git a/opentelemetry-otlp/tests/smoke.rs b/opentelemetry-otlp/tests/smoke.rs
@@ -74,7 +74,7 @@ async fn smoke_tracer() {
             .expect("failed to install");
 
         println!("Sending span...");
-        let span = tracer
+        let mut span = tracer
             .span_builder("my-test-span")
             .with_kind(SpanKind::Server)
             .start(&tracer);

diff --git a/opentelemetry-zipkin/src/propagator/mod.rs b/opentelemetry-zipkin/src/propagator/mod.rs
@@ -211,7 +211,8 @@ impl TextMapPropagator for Propagator {
     /// Properly encodes the values of the `Context`'s `SpanContext` and injects
     /// them into the `Injector`.
     fn inject_context(&self, context: &Context, injector: &mut dyn Injector) {
-        let span_context = context.span().span_context();
+        let span = context.span();
+        let span_context = span.span_context();
         if span_context.is_valid() {
             if self.inject_encoding.support(&B3Encoding::SingleHeader) {
                 let mut value = format!(

diff --git a/opentelemetry/benches/trace.rs b/opentelemetry/benches/trace.rs
@@ -27,15 +27,15 @@ fn criterion_benchmark(c: &mut Criterion) {
     trace_benchmark_group(c, "start-end-span", |tracer| tracer.start("foo").end());
 
     trace_benchmark_group(c, "start-end-span-4-attrs", |tracer| {
-        let span = tracer.start("foo");
+        let mut span = tracer.start("foo");
         span.set_attribute(Key::new("key1").bool(false));
         span.set_attribute(Key::new("key2").string("hello"));
         span.set_attribute(Key::new("key4").f64(123.456));
         span.end();
     });
 
     trace_benchmark_group(c, "start-end-span-8-attrs", |tracer| {
-        let span = tracer.start("foo");
+        let mut span = tracer.start("foo");
         span.set_attribute(Key::new("key1").bool(false));
         span.set_attribute(Key::new("key2").string("hello"));
         span.set_attribute(Key::new("key4").f64(123.456));
@@ -46,7 +46,7 @@ fn criterion_benchmark(c: &mut Criterion) {
     });
 
     trace_benchmark_group(c, "start-end-span-all-attr-types", |tracer| {
-        let span = tracer.start("foo");
+        let mut span = tracer.start("foo");
         span.set_attribute(Key::new("key1").bool(false));
         span.set_attribute(Key::new("key2").string("hello"));
         span.set_attribute(Key::new("key3").i64(123));
@@ -55,7 +55,7 @@ fn criterion_benchmark(c: &mut Criterion) {
     });
 
     trace_benchmark_group(c, "start-end-span-all-attr-types-2x", |tracer| {
-        let span = tracer.start("foo");
+        let mut span = tracer.start("foo");
         span.set_attribute(Key::new("key1").bool(false));
         span.set_attribute(Key::new("key2").string("hello"));
         span.set_attribute(Key::new("key3").i64(123));

diff --git a/opentelemetry/src/global/trace.rs b/opentelemetry/src/global/trace.rs
@@ -21,7 +21,7 @@ impl trace::Span for BoxedSpan {
     /// keys"](https://github.com/open-telemetry/opentelemetry-specification/tree/v0.5.0/specification/trace/semantic_conventions/README.md)
     /// which have prescribed semantic meanings.
     fn add_event_with_timestamp(
-        &self,
+        &mut self,
         name: String,
         timestamp: SystemTime,
         attributes: Vec<KeyValue>,
@@ -45,23 +45,23 @@ impl trace::Span for BoxedSpan {
     /// Note that the OpenTelemetry project documents certain ["standard
     /// attributes"](https://github.com/open-telemetry/opentelemetry-specification/tree/v0.5.0/specification/trace/semantic_conventions/README.md)
     /// that have prescribed semantic meanings.
-    fn set_attribute(&self, attribute: KeyValue) {
+    fn set_attribute(&mut self, attribute: KeyValue) {
         self.0.set_attribute(attribute)
     }
 
     /// Sets the status of the `Span`. If used, this will override the default `Span`
     /// status, which is `Unset`.
-    fn set_status(&self, code: trace::StatusCode, message: String) {
+    fn set_status(&mut self, code: trace::StatusCode, message: String) {
         self.0.set_status(code, message)
     }
 
     /// Updates the `Span`'s name.
-    fn update_name(&self, new_name: String) {
+    fn update_name(&mut self, new_name: String) {
         self.0.update_name(new_name)
     }
 
     /// Finishes the span with given timestamp.
-    fn end_with_timestamp(&self, timestamp: SystemTime) {
+    fn end_with_timestamp(&mut self, timestamp: SystemTime) {
         self.0.end_with_timestamp(timestamp);
     }
 }
@@ -128,7 +128,7 @@ pub trait GenericTracer: fmt::Debug + 'static {
 
 impl<S, T> GenericTracer for T
 where
-    S: trace::Span + Send + Sync,
+    S: trace::Span + Send + Sync + 'static,
     T: trace::Tracer<Span = S>,
 {
     /// Create a new invalid span for use in cases where there are no active spans.
@@ -165,7 +165,7 @@ pub trait GenericTracerProvider: fmt::Debug + 'static {
 
 impl<S, T, P> GenericTracerProvider for P
 where
-    S: trace::Span + Send + Sync,
+    S: trace::Span + Send + Sync + 'static,
     T: trace::Tracer<Span = S> + Send + Sync,
     P: trace::TracerProvider<Tracer = T>,
 {
@@ -193,7 +193,7 @@ impl GlobalTracerProvider {
     /// Create a new GlobalTracerProvider instance from a struct that implements `TracerProvider`.
     fn new<P, T, S>(provider: P) -> Self
     where
-        S: trace::Span + Send + Sync,
+        S: trace::Span + Send + Sync + 'static,
         T: trace::Tracer<Span = S> + Send + Sync,
         P: trace::TracerProvider<Tracer = T> + Send + Sync,
     {
@@ -258,7 +258,7 @@ pub fn tracer_with_version(name: &'static str, version: &'static str) -> BoxedTr
 /// [`TracerProvider`]: crate::trace::TracerProvider
 pub fn set_tracer_provider<P, T, S>(new_provider: P) -> GlobalTracerProvider
 where
-    S: trace::Span + Send + Sync,
+    S: trace::Span + Send + Sync + 'static,
     T: trace::Tracer<Span = S> + Send + Sync,
     P: trace::TracerProvider<Tracer = T> + Send + Sync,
 {

diff --git a/opentelemetry/src/sdk/propagation/composite.rs b/opentelemetry/src/sdk/propagation/composite.rs
@@ -138,14 +138,15 @@ mod tests {
 
     impl TextMapPropagator for TestPropagator {
         fn inject_context(&self, cx: &Context, injector: &mut dyn Injector) {
-            let span = cx.span().span_context();
+            let span = cx.span();
+            let span_context = span.span_context();
             injector.set(
                 "testheader",
                 format!(
                     "{}-{}-{}",
-                    span.trace_id().to_u128(),
-                    span.span_id().to_u64(),
-                    span.trace_flags()
+                    span_context.trace_id().to_u128(),
+                    span_context.span_id().to_u64(),
+                    span_context.trace_flags()
                 ),
             )
         }

diff --git a/opentelemetry/src/sdk/propagation/trace_context.rs b/opentelemetry/src/sdk/propagation/trace_context.rs
@@ -117,7 +117,8 @@ impl TextMapPropagator for TraceContextPropagator {
     /// Properly encodes the values of the `SpanContext` and injects them
     /// into the `Injector`.
     fn inject_context(&self, cx: &Context, injector: &mut dyn Injector) {
-        let span_context = cx.span().span_context();
+        let span = cx.span();
+        let span_context = span.span_context();
         if span_context.is_valid() {
             let header_value = format!(
                 "{:02x}-{:032x}-{:016x}-{:02x}",

diff --git a/opentelemetry/src/sdk/trace/sampler.rs b/opentelemetry/src/sdk/trace/sampler.rs
@@ -119,7 +119,8 @@ impl ShouldSample for Sampler {
                         .should_sample(parent_context, trace_id, name, span_kind, attributes, links)
                         .decision,
                     |ctx| {
-                        let parent_span_context = ctx.span().span_context();
+                        let span = ctx.span();
+                        let parent_span_context = span.span_context();
                         if parent_span_context.is_sampled() {
                             SamplingDecision::RecordAndSample
                         } else {