opentelemetry: enforce event_location for span tags (#2124)

The `with_event_location(false)` method will now properly omit `code.*`
tags from spans.

## Motivation

Recently, I attempted to remove the `code.*` tags from opentelemetry
tracing spans utilizing the [`with_event_location`] of
OpenTelemetrySubscriber. This did not work as expected because the
[`on_new_span`] doesn't account for the `event_location` boolean similar
to how [`on_event`] does.

## Solution

The change presented will expand the use of the `location` field to
check before adding the `code.*` KeyValue attributes in `on_new_span`.

In addition, `with_event_location` was renamed to `with_location`, as it
now controls both span *and* event locations, and the
`with_event_location` method has been deprecated.

## Testing

Additional unit tests are included in
[tracing-opentelemetry/src/subscriber.rs] to cover both boolean cases of
`with_location`.

[`with_event_location`]: https://github.com/tokio-rs/tracing/blob/master/tracing-opentelemetry/src/subscriber.rs#L343
[`on_new_span`]: https://github.com/tokio-rs/tracing/blob/master/tracing-opentelemetry/src/subscriber.rs#L448-L460
[`on_event`]: https://github.com/tokio-rs/tracing/blob/master/tracing-opentelemetry/src/subscriber.rs#L582
[tracing-opentelemetry/src/subscriber.rs]: https://github.com/tokio-rs/tracing/pull/2124/files#diff-69011e8b23dffcbe19b9b72e5ac54330a7871f424a90700ed7f5c5686daf431bR911-R975)

Author: DevinCarr

tracing-opentelemetry/src/layer.rs

@@ -28,7 +28,7 @@ const SPAN_STATUS_MESSAGE_FIELD: &str = "otel.status_message";
 /// [tracing]: https://github.com/tokio-rs/tracing
 pub struct OpenTelemetryLayer<S, T> {
     tracer: T,
-    event_location: bool,
+    location: bool,
     tracked_inactivity: bool,
     get_context: WithContext,
     _registry: marker::PhantomData<S>,
@@ -312,7 +312,7 @@ where
     pub fn new(tracer: T) -> Self {
         OpenTelemetryLayer {
             tracer,
-            event_location: true,
+            location: true,
             tracked_inactivity: true,
             get_context: WithContext(Self::get_context),
             _registry: marker::PhantomData,
@@ -351,20 +351,32 @@ where
     {
         OpenTelemetryLayer {
             tracer,
-            event_location: self.event_location,
+            location: self.location,
             tracked_inactivity: self.tracked_inactivity,
             get_context: WithContext(OpenTelemetryLayer::<S, Tracer>::get_context),
             _registry: self._registry,
         }
     }
 
+    /// Sets whether or not span and event metadata should include detailed
+    /// location  information, such as the file, module and line number.
+    ///
+    /// By default, locations are enabled.
+    pub fn with_location(self, location: bool) -> Self {
+        Self { location, ..self }
+    }
+
     /// Sets whether or not event span's metadata should include detailed location
     /// information, such as the file, module and line number.
     ///
     /// By default, event locations are enabled.
+    #[deprecated(
+        since = "0.17.3",
+        note = "renamed to `OpenTelemetrySubscriber::with_location`"
+    )]
     pub fn with_event_location(self, event_location: bool) -> Self {
         Self {
-            event_location,
+            location: event_location,
             ..self
         }
     }
@@ -467,18 +479,20 @@ where
             .attributes
             .get_or_insert(Vec::with_capacity(attrs.fields().len() + 3));
 
-        let meta = attrs.metadata();
+        if self.location {
+            let meta = attrs.metadata();
 
-        if let Some(filename) = meta.file() {
-            builder_attrs.push(KeyValue::new("code.filepath", filename));
-        }
+            if let Some(filename) = meta.file() {
+                builder_attrs.push(KeyValue::new("code.filepath", filename));
+            }
 
-        if let Some(module) = meta.module_path() {
-            builder_attrs.push(KeyValue::new("code.namespace", module));
-        }
+            if let Some(module) = meta.module_path() {
+                builder_attrs.push(KeyValue::new("code.namespace", module));
+            }
 
-        if let Some(line) = meta.line() {
-            builder_attrs.push(KeyValue::new("code.lineno", line as i64));
+            if let Some(line) = meta.line() {
+                builder_attrs.push(KeyValue::new("code.lineno", line as i64));
+            }
         }
 
         attrs.record(&mut SpanAttributeVisitor(&mut builder));
@@ -601,7 +615,7 @@ where
                     builder.status_code = Some(otel::StatusCode::Error);
                 }
 
-                if self.event_location {
+                if self.location {
                     #[cfg(not(feature = "tracing-log"))]
                     let normalized_meta: Option<tracing_core::Metadata<'_>> = None;
                     let (file, module) = match &normalized_meta {
@@ -999,4 +1013,64 @@ mod tests {
             )
         );
     }
+
+    #[test]
+    fn includes_span_location() {
+        let tracer = TestTracer(Arc::new(Mutex::new(None)));
+        let subscriber = tracing_subscriber::registry()
+            .with(layer().with_tracer(tracer.clone()).with_location(true));
+
+        tracing::subscriber::with_default(subscriber, || {
+            tracing::debug_span!("request");
+        });
+
+        let attributes = tracer
+            .0
+            .lock()
+            .unwrap()
+            .as_ref()
+            .unwrap()
+            .builder
+            .attributes
+            .as_ref()
+            .unwrap()
+            .clone();
+        let keys = attributes
+            .iter()
+            .map(|attr| attr.key.as_str())
+            .collect::<Vec<&str>>();
+        assert!(keys.contains(&"code.filepath"));
+        assert!(keys.contains(&"code.namespace"));
+        assert!(keys.contains(&"code.lineno"));
+    }
+
+    #[test]
+    fn excludes_span_location() {
+        let tracer = TestTracer(Arc::new(Mutex::new(None)));
+        let subscriber = tracing_subscriber::registry()
+            .with(layer().with_tracer(tracer.clone()).with_location(false));
+
+        tracing::subscriber::with_default(subscriber, || {
+            tracing::debug_span!("request");
+        });
+
+        let attributes = tracer
+            .0
+            .lock()
+            .unwrap()
+            .as_ref()
+            .unwrap()
+            .builder
+            .attributes
+            .as_ref()
+            .unwrap()
+            .clone();
+        let keys = attributes
+            .iter()
+            .map(|attr| attr.key.as_str())
+            .collect::<Vec<&str>>();
+        assert!(!keys.contains(&"code.filepath"));
+        assert!(!keys.contains(&"code.namespace"));
+        assert!(!keys.contains(&"code.lineno"));
+    }
 }