Clean up Span and Event APIs (#80)

This updates the Span API to reflect the fact that data access is already
synchronized behind a mutex so the additional `&mut` restrictions are
unnecessary. It also renames event's `message` to `name` to better
reflect the specification.

benches/trace.rs

@@ -10,7 +10,7 @@ fn criterion_benchmark(c: &mut Criterion) {
     });
 
     trace_benchmark_group(c, "start-end-span-4-attrs", |tracer| {
-        let mut span = tracer.start("foo", None);
+        let span = tracer.start("foo", None);
         span.set_attribute(Key::new("key1").bool(false));
         span.set_attribute(Key::new("key2").string("hello"));
         span.set_attribute(Key::new("key3").u64(123));
@@ -19,7 +19,7 @@ fn criterion_benchmark(c: &mut Criterion) {
     });
 
     trace_benchmark_group(c, "start-end-span-8-attrs", |tracer| {
-        let mut span = tracer.start("foo", None);
+        let span = tracer.start("foo", None);
         span.set_attribute(Key::new("key1").bool(false));
         span.set_attribute(Key::new("key2").string("hello"));
         span.set_attribute(Key::new("key3").u64(123));
@@ -32,7 +32,7 @@ fn criterion_benchmark(c: &mut Criterion) {
     });
 
     trace_benchmark_group(c, "start-end-span-all-attr-types", |tracer| {
-        let mut span = tracer.start("foo", None);
+        let span = tracer.start("foo", None);
         span.set_attribute(Key::new("key1").bool(false));
         span.set_attribute(Key::new("key2").string("hello"));
         span.set_attribute(Key::new("key3").i64(123));
@@ -45,7 +45,7 @@ fn criterion_benchmark(c: &mut Criterion) {
     });
 
     trace_benchmark_group(c, "start-end-span-all-attr-types-2x", |tracer| {
-        let mut span = tracer.start("foo", None);
+        let span = tracer.start("foo", None);
         span.set_attribute(Key::new("key1").bool(false));
         span.set_attribute(Key::new("key2").string("hello"));
         span.set_attribute(Key::new("key3").i64(123));

examples/report.rs

@@ -13,7 +13,7 @@ fn main() {
         let span0 = tracer.start("main", None);
         thread::sleep(Duration::from_millis(10));
         {
-            let mut span1 = tracer.start("sub", Some(span0.get_context()));
+            let span1 = tracer.start("sub", Some(span0.get_context()));
             span1.set_attribute(api::Key::new("foo").string("bar"));
             span1.add_event("something wrong".to_string());
             thread::sleep(Duration::from_millis(10));

opentelemetry-jaeger/src/lib.rs

@@ -317,9 +317,9 @@ impl Into<jaeger::Log> for api::Event {
         jaeger::Log::new(
             timestamp,
             vec![jaeger::Tag::new(
-                "event".to_string(),
+                "name".to_string(),
                 jaeger::TagType::String,
-                Some(self.message),
+                Some(self.name),
                 None,
                 None,
                 None,

opentelemetry-zipkin/src/lib.rs

@@ -187,7 +187,7 @@ impl Into<annotation::Annotation> for api::Event {
 
         annotation::Annotation::builder()
             .timestamp(timestamp)
-            .value(self.message)
+            .value(self.name)
             .build()
     }
 }

src/api/trace/event.rs

@@ -9,22 +9,22 @@ use std::time::SystemTime;
 #[cfg_attr(feature = "serialize", derive(Deserialize, PartialEq, Serialize))]
 #[derive(Clone, Debug)]
 pub struct Event {
-    /// Event message
-    pub message: String,
+    /// Event name
+    pub name: String,
     /// Event timestamp
     pub timestamp: SystemTime,
 }
 
 impl Event {
     /// Create new `Event`
-    pub fn new(message: String, timestamp: SystemTime) -> Self {
-        Event { message, timestamp }
+    pub fn new(name: String, timestamp: SystemTime) -> Self {
+        Event { name, timestamp }
     }
 
-    /// Create new `Event` for a given message.
-    pub fn from_message(message: String) -> Self {
+    /// Create new `Event` with a given name.
+    pub fn with_name(name: String) -> Self {
         Event {
-            message,
+            name,
             timestamp: SystemTime::now(),
         }
     }

src/api/trace/noop.rs

@@ -48,12 +48,12 @@ impl NoopSpan {
 
 impl api::Span for NoopSpan {
     /// Ignores all events
-    fn add_event(&mut self, _message: String) {
+    fn add_event(&self, _name: String) {
         // Ignore
     }
 
     /// Ignores all events with timestamps
-    fn add_event_with_timestamp(&mut self, _message: String, _timestamp: SystemTime) {
+    fn add_event_with_timestamp(&self, _name: String, _timestamp: SystemTime) {
         // Ignored
     }
 
@@ -68,22 +68,22 @@ impl api::Span for NoopSpan {
     }
 
     /// Ignores all attributes
-    fn set_attribute(&mut self, _attribute: api::KeyValue) {
+    fn set_attribute(&self, _attribute: api::KeyValue) {
         // Ignored
     }
 
     /// Ignores status
-    fn set_status(&mut self, _status: api::SpanStatus) {
+    fn set_status(&self, _status: api::SpanStatus) {
         // Ignored
     }
 
     /// Ignores name updates
-    fn update_name(&mut self, _new_name: String) {
+    fn update_name(&self, _new_name: String) {
         // Ignored
     }
 
     /// Ignores `Span` endings.
-    fn end(&mut self) {
+    fn end(&self) {
         // Ignored
     }
 

src/api/trace/span.rs

@@ -34,8 +34,8 @@ pub trait Span: fmt::Debug + 'static {
     /// Note that the OpenTelemetry project documents certain ["standard event names and
     /// keys"](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md)
     /// which have prescribed semantic meanings.
-    fn add_event(&mut self, message: String) {
-        self.add_event_with_timestamp(message, SystemTime::now())
+    fn add_event(&self, name: String) {
+        self.add_event_with_timestamp(name, SystemTime::now())
     }
 
     /// An API to record events at a specific time in the context of a given `Span`.
@@ -46,7 +46,7 @@ pub trait Span: fmt::Debug + 'static {
     /// Note that the OpenTelemetry project documents certain ["standard event names and
     /// keys"](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md)
     /// which have prescribed semantic meanings.
-    fn add_event_with_timestamp(&mut self, message: String, timestamp: SystemTime);
+    fn add_event_with_timestamp(&self, name: String, timestamp: SystemTime);
 
     /// Returns the `SpanContext` for the given `Span`. The returned value may be used even after
     /// the `Span is finished. The returned value MUST be the same for the entire `Span` lifetime.
@@ -83,14 +83,14 @@ pub trait Span: fmt::Debug + 'static {
     /// Note that the OpenTelemetry project documents certain ["standard
     /// attributes"](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md)
     /// that have prescribed semantic meanings.
-    fn set_attribute(&mut self, attribute: api::KeyValue);
+    fn set_attribute(&self, attribute: api::KeyValue);
 
     /// Sets the status of the `Span`. If used, this will override the default `Span`
     /// status, which is `OK`.
     ///
     /// Only the value of the last call will be recorded, and implementations are free
     /// to ignore previous calls.
-    fn set_status(&mut self, status: api::SpanStatus);
+    fn set_status(&self, status: api::SpanStatus);
 
     /// Updates the `Span`'s name. After this update, any sampling behavior based on the
     /// name will depend on the implementation.
@@ -104,7 +104,7 @@ pub trait Span: fmt::Debug + 'static {
     /// regular property. It emphasizes that this operation signifies a
     /// major change for a `Span` and may lead to re-calculation of sampling or
     /// filtering decisions made previously depending on the implementation.
-    fn update_name(&mut self, new_name: String);
+    fn update_name(&self, new_name: String);
 
     /// Finishes the `Span`.
     ///
@@ -116,7 +116,7 @@ pub trait Span: fmt::Debug + 'static {
     /// still be running and can be ended later.
     ///
     ///This API MUST be non-blocking.
-    fn end(&mut self);
+    fn end(&self);
 
     /// Used by global tracer to downcast to specific span type.
     fn as_any(&self) -> &dyn std::any::Any;

src/api/trace/tracer.rs

@@ -112,7 +112,7 @@ pub trait TracerGenerics: Tracer {
     /// It then executes the body. It closes the span before returning the execution result.
     fn with_span<T, F>(&self, name: &'static str, f: F) -> T
     where
-        F: FnOnce(&mut Self::Span) -> T;
+        F: FnOnce(&Self::Span) -> T;
 }
 
 // These functions can be implemented for all tracers to allow for convenient `with_span` syntax.
@@ -122,12 +122,12 @@ impl<S: Tracer> TracerGenerics for S {
     /// It then executes the body. It closes the span before returning the execution result.
     fn with_span<T, F>(&self, name: &'static str, f: F) -> T
     where
-        F: FnOnce(&mut Self::Span) -> T,
+        F: FnOnce(&Self::Span) -> T,
     {
-        let mut span = self.start(name, None);
+        let span = self.start(name, None);
         self.mark_span_as_active(&span);
 
-        let result = f(&mut span);
+        let result = f(&span);
         span.end();
         self.mark_span_as_inactive(span.get_context().span_id());
 

src/global.rs

@@ -82,8 +82,8 @@ impl api::Span for BoxedSpan {
     /// Note that the OpenTelemetry project documents certain ["standard event names and
     /// keys"](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md)
     /// which have prescribed semantic meanings.
-    fn add_event_with_timestamp(&mut self, message: String, timestamp: SystemTime) {
-        self.0.add_event_with_timestamp(message, timestamp)
+    fn add_event_with_timestamp(&self, name: String, timestamp: SystemTime) {
+        self.0.add_event_with_timestamp(name, timestamp)
     }
 
     /// Returns the `SpanContext` for the given `Span`.
@@ -102,23 +102,23 @@ impl api::Span for BoxedSpan {
     /// Note that the OpenTelemetry project documents certain ["standard
     /// attributes"](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md)
     /// that have prescribed semantic meanings.
-    fn set_attribute(&mut self, attribute: api::KeyValue) {
+    fn set_attribute(&self, attribute: api::KeyValue) {
         self.0.set_attribute(attribute)
     }
 
     /// Sets the status of the `Span`. If used, this will override the default `Span`
     /// status, which is `OK`.
-    fn set_status(&mut self, status: api::SpanStatus) {
+    fn set_status(&self, status: api::SpanStatus) {
         self.0.set_status(status)
     }
 
     /// Updates the `Span`'s name.
-    fn update_name(&mut self, new_name: String) {
+    fn update_name(&self, new_name: String) {
         self.0.update_name(new_name)
     }
 
     /// Finishes the span.
-    fn end(&mut self) {
+    fn end(&self) {
         self.0.end()
     }
 
@@ -127,12 +127,22 @@ impl api::Span for BoxedSpan {
         self
     }
 
-    /// Mark span as active
+    /// Mark span as currently active
+    ///
+    /// This is the _synchronous_ api. If you are using futures, you
+    /// need to use the async api via [`instrument`].
+    ///
+    /// [`instrument`]: ../api/trace/futures/trait.Instrument.html#method.instrument
     fn mark_as_active(&self) {
         self.0.mark_as_active()
     }
 
-    /// Mark span as inactive
+    /// Mark span as no longer active
+    ///
+    /// This is the _synchronous_ api. If you are using futures, you
+    /// need to use the async api via [`instrument`].
+    ///
+    /// [`instrument`]: ../api/trace/futures/trait.Instrument.html#method.instrument
     fn mark_as_inactive(&self) {
         self.0.mark_as_inactive()
     }

src/sdk/trace/span.rs

@@ -77,10 +77,10 @@ impl api::Span for Span {
     /// Note that the OpenTelemetry project documents certain ["standard event names and
     /// keys"](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md)
     /// which have prescribed semantic meanings.
-    fn add_event_with_timestamp(&mut self, message: String, timestamp: SystemTime) {
+    fn add_event_with_timestamp(&self, name: String, timestamp: SystemTime) {
         self.with_data_mut(|data| {
             data.message_events
-                .push_front(api::Event { message, timestamp })
+                .push_front(api::Event { name, timestamp })
         });
     }
 
@@ -103,29 +103,29 @@ impl api::Span for Span {
     /// Note that the OpenTelemetry project documents certain ["standard
     /// attributes"](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md)
     /// that have prescribed semantic meanings.
-    fn set_attribute(&mut self, attribute: api::KeyValue) {
+    fn set_attribute(&self, attribute: api::KeyValue) {
         self.with_data_mut(|data| {
             data.attributes.push_front(attribute);
         });
     }
 
     /// Sets the status of the `Span`. If used, this will override the default `Span`
     /// status, which is `OK`.
-    fn set_status(&mut self, status: api::SpanStatus) {
+    fn set_status(&self, status: api::SpanStatus) {
         self.with_data_mut(|data| {
             data.status = status;
         });
     }
 
     /// Updates the `Span`'s name.
-    fn update_name(&mut self, new_name: String) {
+    fn update_name(&self, new_name: String) {
         self.with_data_mut(|data| {
             data.name = new_name;
         });
     }
 
     /// Finishes the span.
-    fn end(&mut self) {
+    fn end(&self) {
         self.with_data_mut(|data| {
             data.end_time = SystemTime::now();
         });
@@ -136,12 +136,22 @@ impl api::Span for Span {
         self
     }
 
-    /// Mark span as active
+    /// Mark as currently active span.
+    ///
+    /// This is the _synchronous_ api. If you are using futures, you
+    /// need to use the async api via [`instrument`].
+    ///
+    /// [`instrument`]: ../../api/trace/futures/trait.Instrument.html#method.instrument
     fn mark_as_active(&self) {
         self.inner.tracer.mark_span_as_active(&self);
     }
 
     /// Mark span as inactive
+    ///
+    /// This is the _synchronous_ api. If you are using futures, you
+    /// need to use the async api via [`instrument`].
+    ///
+    /// [`instrument`]: ../futures/trait.Instrument.html#method.instrument
     fn mark_as_inactive(&self) {
         self.inner.tracer.mark_span_as_inactive(self.id);
     }