feat(telemetry): add the ability to override span name with otel.name…

… attribute (#5365)

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

.changesets/feat_bnjjj_feat_otel_name_attribute.md

@@ -0,0 +1,16 @@
+### Override tracing span names using custom span selectors ([Issue #5261](https://github.com/apollographql/router/issues/5261))
+
+It gives you the ability to override the span name by using custom telemetry with any [selectors](https://www.apollographql.com/docs/router/configuration/telemetry/instrumentation/selectors/) you want just by setting the `otel.name` attribute.
+
+Example:
+
+```yaml
+telemetry:
+  instrumentation:
+    spans:
+      router:
+        otel.name:
+           static: router # Override the span name to router 
+```
+
+By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/5365

apollo-router/src/plugins/telemetry/config_new/spans.rs

@@ -547,6 +547,16 @@ mod test {
                 value: Arc::new(Default::default()),
             },
         );
+        spans.attributes.custom.insert(
+            "otel.name".to_string(),
+            Conditional {
+                selector: RouterSelector::StaticField {
+                    r#static: String::from("new_name").into(),
+                },
+                condition: None,
+                value: Arc::new(Default::default()),
+            },
+        );
         let values = spans.attributes.on_response(
             &router::Response::fake_builder()
                 .header("my-header", "test_val")
@@ -556,6 +566,10 @@ mod test {
         assert!(values
             .iter()
             .any(|key_val| key_val.key == opentelemetry::Key::from_static_str("test")));
+
+        assert!(values.iter().any(|key_val| key_val.key
+            == opentelemetry::Key::from_static_str("otel.name")
+            && key_val.value == opentelemetry::Value::String(String::from("new_name").into())));
     }
 
     #[test]

apollo-router/src/plugins/telemetry/dynamic_attribute.rs

@@ -194,7 +194,7 @@ impl SpanDynAttribute for ::tracing::Span {
 
 fn update_otel_data(otel_data: &mut OtelData, key: &Key, value: &opentelemetry::Value) {
     match key.as_str() {
-        SPAN_NAME_FIELD => otel_data.builder.name = format!("{:?}", value).into(),
+        SPAN_NAME_FIELD => otel_data.forced_span_name = Some(value.to_string()),
         SPAN_KIND_FIELD => otel_data.builder.span_kind = str_to_span_kind(&value.as_str()),
         SPAN_STATUS_CODE_FIELD => otel_data.forced_status = str_to_status(&value.as_str()).into(),
         SPAN_STATUS_MESSAGE_FIELD => {

apollo-router/src/plugins/telemetry/otel/layer.rs

@@ -709,6 +709,7 @@ where
             parent_cx,
             event_attributes: None,
             forced_status: None,
+            forced_span_name: None,
         });
     }
 
@@ -884,6 +885,7 @@ where
             mut builder,
             parent_cx,
             forced_status,
+            forced_span_name,
             ..
         }) = extensions.remove::<OtelData>()
         {
@@ -903,6 +905,9 @@ where
             if let Some(forced_status) = forced_status {
                 builder.status = forced_status;
             }
+            if let Some(forced_span_name) = forced_span_name {
+                builder.name = forced_span_name.into();
+            }
 
             // Assign end time, build and start span, drop span to export
             builder
@@ -964,8 +969,11 @@ mod tests {
     use opentelemetry::trace::TraceFlags;
     use opentelemetry::StringValue;
     use tracing_subscriber::prelude::*;
+    use tracing_subscriber::Registry;
 
     use super::*;
+    use crate::plugins::telemetry::dynamic_attribute::SpanDynAttribute;
+    use crate::plugins::telemetry::reload::SampledSpan;
 
     #[derive(Debug, Clone)]
     struct TestTracer(Arc<Mutex<Option<OtelData>>>);
@@ -993,6 +1001,7 @@ mod tests {
                 parent_cx: parent_cx.clone(),
                 event_attributes: None,
                 forced_status: None,
+                forced_span_name: None,
             });
             noop::NoopSpan::new()
         }
@@ -1089,6 +1098,41 @@ mod tests {
         assert_eq!(recorded_name, Some(dynamic_name.into()))
     }
 
+    #[test]
+    fn forced_dynamic_span_names() {
+        let dynamic_name = "GET http://example.com".to_string();
+        let forced_dynamic_name = "OVERRIDE GET http://example.com".to_string();
+        let tracer = TestTracer(Arc::new(Mutex::new(None)));
+        let subscriber = tracing_subscriber::registry().with(layer().with_tracer(tracer.clone()));
+
+        tracing::subscriber::with_default(subscriber, || {
+            let span = tracing::debug_span!("static_name", otel.name = dynamic_name.as_str());
+            span.with_subscriber(move |(id, dispatch)| {
+                if let Some(reg) = dispatch.downcast_ref::<Registry>() {
+                    match reg.span(id) {
+                        Some(s) => {
+                            s.extensions_mut().insert(SampledSpan {});
+                        }
+                        None => panic!("should not happen"),
+                    }
+                }
+            });
+            let _entered = span.enter();
+            span.set_span_dyn_attribute(
+                Key::from_static_str("otel.name"),
+                opentelemetry::Value::String(forced_dynamic_name.clone().into()),
+            );
+        });
+
+        let recorded_name = tracer
+            .0
+            .lock()
+            .unwrap()
+            .as_ref()
+            .map(|b| b.builder.name.clone());
+        assert_eq!(recorded_name, Some(Cow::Owned(forced_dynamic_name)))
+    }
+
     #[test]
     fn span_kind() {
         let tracer = TestTracer(Arc::new(Mutex::new(None)));

apollo-router/src/plugins/telemetry/otel/mod.rs

@@ -29,4 +29,7 @@ pub(crate) struct OtelData {
 
     /// Forced status in case it's coming from the custom attributes
     pub(crate) forced_status: Option<opentelemetry::trace::Status>,
+
+    /// Forced span name in case it's coming from the custom attributes
+    pub(crate) forced_span_name: Option<String>,
 }

apollo-router/src/plugins/telemetry/otel/tracer.rs

@@ -188,6 +188,7 @@ mod tests {
             parent_cx,
             event_attributes: None,
             forced_status: None,
+            forced_span_name: None,
         });
         let span = cx.span();
         let span_context = span.span_context();
@@ -232,6 +233,7 @@ mod tests {
                 parent_cx,
                 event_attributes: None,
                 forced_status: None,
+                forced_span_name: None,
             });
 
             assert_eq!(

apollo-router/src/plugins/telemetry/reload.rs

@@ -269,7 +269,7 @@ where
     }
 }
 
-struct SampledSpan;
+pub(crate) struct SampledSpan;
 
 pub(crate) trait IsSampled {
     fn is_sampled(&self) -> bool;

docs/source/configuration/telemetry/instrumentation/spans.mdx

@@ -157,14 +157,23 @@ The `mode` option will be defaulted to `spec_compliant` in a future release, and
 By default spans are marked in error only if the http status code is different than 200. If you want to mark a span in error for other reason you can override the `otel.status_code` attribute which is responsible to mark a span in error or not.
 If it's in error then `otel.status_code` = `error`, if not it will be `ok`.
 
-Here is an example if you want to mark the `router` and `supergraph` span in error if you have a graphql error in the payload.
+## Naming
+
+By default, we will use a span naming convention that aligns with the current [semantinc conventions for GraphQL server in OpenTelemetry](https://opentelemetry.io/docs/specs/semconv/graphql/graphql-spans/) which means the root span name 
+must be of format `<graphql.operation.type> <graphql.operation.name>` provided that `graphql.operation.type` and `graphql.operation.name` are available. 
+
+If you want to change the name of spans we're creating for each services you can override this value by setting the `otel.name` attribute using any selectors you want.
+
+Here is an example if you want to mark the `router` and `supergraph` span in error if you have a graphql error in the payload and you want to enforce the `router` span name to be `graphql_router`.
 
 ```yaml title="router.yaml"
 telemetry:
   instrumentation:
     spans:
       router:
         attributes:
+          otel.name:
+            static: graphql_router # Override the span name to graphql_router 
           otel.status_code:
             static: error
             condition: