fix(telemetry): rename the selector to get studio operation id (#5337)

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
apollographql · Jun 5, 2024 · 37ad992 · 37ad992
1 parent 576f28f
commit 37ad992
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 42 deletions.
diff --git a/.changesets/config_bnjjj_fix_studio_selector.md b/.changesets/config_bnjjj_fix_studio_selector.md
@@ -0,0 +1,14 @@
+### Rename the telemetry selector to get studio operation id ([PR #5337](https://github.com/apollographql/router/pull/5337))
+
+We introduced a new `trace_id` selector format in `1.48.0` which has been misnamed because it's not a trace id but the Apollo Studio Operation ID. If you want to access to this selector, here is an example:
+
+```yaml
+telemetry:
+  instrumentation:
+    spans:
+      router:
+        "studio.operation.id":
+            studio_operation_id: true
+```
+
+By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/5337
diff --git a/...r/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap b/...r/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap
@@ -1,5 +1,6 @@
 ---
 source: apollo-router/src/configuration/tests.rs
+assertion_line: 26
 expression: "&schema"
 ---
 {
@@ -5028,6 +5029,20 @@ expression: "&schema"
           ],
           "type": "object"
         },
+        {
+          "additionalProperties": false,
+          "description": "Apollo Studio operation id",
+          "properties": {
+            "studio_operation_id": {
+              "description": "Apollo Studio operation id",
+              "type": "boolean"
+            }
+          },
+          "required": [
+            "studio_operation_id"
+          ],
+          "type": "object"
+        },
         {
           "additionalProperties": false,
           "description": "A value from context.",
@@ -6882,13 +6897,6 @@ expression: "&schema"
             "datadog"
           ],
           "type": "string"
-        },
-        {
-          "description": "Apollo Studio trace id",
-          "enum": [
-            "apollo"
-          ],
-          "type": "string"
         }
       ]
     },

diff --git a/apollo-router/src/plugins/telemetry/config_new/selectors.rs b/apollo-router/src/plugins/telemetry/config_new/selectors.rs
@@ -35,8 +35,6 @@ pub(crate) enum TraceIdFormat {
     OpenTelemetry,
     /// Datadog trace ID, a u64.
     Datadog,
-    /// Apollo Studio trace id
-    Apollo,
 }
 
 #[derive(Deserialize, JsonSchema, Clone, Debug)]
@@ -127,6 +125,11 @@ pub(crate) enum RouterSelector {
         /// The format of the trace ID.
         trace_id: TraceIdFormat,
     },
+    /// Apollo Studio operation id
+    StudioOperationId {
+        /// Apollo Studio operation id
+        studio_operation_id: bool,
+    },
     /// A value from context.
     ResponseContext {
         /// The response context key.
@@ -566,20 +569,13 @@ impl Selector for RouterSelector {
                 .map(opentelemetry::Value::from),
             RouterSelector::TraceId {
                 trace_id: trace_id_format,
-            } => {
-                if let TraceIdFormat::Apollo = &trace_id_format {
-                    return None;
+            } => trace_id().map(|id| {
+                match trace_id_format {
+                    TraceIdFormat::OpenTelemetry => id.to_string(),
+                    TraceIdFormat::Datadog => id.to_datadog(),
                 }
-                trace_id().map(|id| {
-                    match trace_id_format {
-                        TraceIdFormat::OpenTelemetry => id.to_string(),
-                        TraceIdFormat::Datadog => id.to_datadog(),
-                        // It happens in the response
-                        TraceIdFormat::Apollo => String::new(),
-                    }
-                    .into()
-                })
-            }
+                .into()
+            }),
             RouterSelector::Baggage {
                 baggage, default, ..
             } => get_baggage(baggage).or_else(|| default.maybe_to_otel_value()),
@@ -636,20 +632,14 @@ impl Selector for RouterSelector {
             }
             RouterSelector::Static(val) => Some(val.clone().into()),
             RouterSelector::StaticField { r#static } => Some(r#static.clone().into()),
-            RouterSelector::TraceId {
-                trace_id: trace_id_format,
-            } => {
-                if let TraceIdFormat::Apollo = &trace_id_format {
-                    response
-                        .context
-                        .get::<_, String>(APOLLO_OPERATION_ID)
-                        .ok()
-                        .flatten()
-                        .map(opentelemetry::Value::from)
-                } else {
-                    None
-                }
-            }
+            RouterSelector::StudioOperationId {
+                studio_operation_id,
+            } if *studio_operation_id => response
+                .context
+                .get::<_, String>(APOLLO_OPERATION_ID)
+                .ok()
+                .flatten()
+                .map(opentelemetry::Value::from),
             _ => None,
         }
     }
@@ -2016,8 +2006,8 @@ mod test {
 
     #[test]
     fn test_router_studio_trace_id() {
-        let selector = RouterSelector::TraceId {
-            trace_id: TraceIdFormat::Apollo,
+        let selector = RouterSelector::StudioOperationId {
+            studio_operation_id: true,
         };
         let ctx = crate::Context::new();
         let _ = ctx.insert(APOLLO_OPERATION_ID, "42".to_string()).unwrap();

diff --git a/apollo-router/tests/integration/telemetry/fixtures/jaeger-advanced.router.yaml b/apollo-router/tests/integration/telemetry/fixtures/jaeger-advanced.router.yaml
@@ -44,8 +44,8 @@ telemetry:
               eq:
                 - request_header: "head"
                 - "test"
-          studio.trace.id:
-            trace_id: apollo
+          studio.operation.id:
+            studio_operation_id: true
       supergraph:
         attributes:
           graphql.operation.name: true

diff --git a/apollo-router/tests/integration/telemetry/jaeger.rs b/apollo-router/tests/integration/telemetry/jaeger.rs
@@ -414,7 +414,7 @@ fn verify_router_span_fields(
         );
         assert_eq!(
             router_span
-                .select_path("$.tags[?(@.key == 'studio.trace.id')].value")?
+                .select_path("$.tags[?(@.key == 'studio.operation.id')].value")?
                 .first(),
             Some(&&Value::String(
                 "f60e643d7f52ecda23216f86409d7e2e5c3aa68c".to_string()

diff --git a/docs/source/configuration/telemetry/instrumentation/selectors.mdx b/docs/source/configuration/telemetry/instrumentation/selectors.mdx
@@ -32,7 +32,8 @@ The router service is the initial entrypoint for all requests. It is HTTP centri
 
 | Selector           | Defaultable | Values                                    | Description                          |
 |--------------------|-------------|-------------------------------------------|--------------------------------------|
-| `trace_id`         | Yes         | `open_telemetry`\|`datadog`\|`apollo`      | The trace ID                         |
+| `trace_id`         | Yes         | `open_telemetry`\|`datadog`              | The trace ID                         |
+| `studio_operation_id` | Yes      | `true`|`false`                            | The Apollo Studio operation id       |
 | `request_header`   | Yes         |                                           | The name of the request header       |
 | `response_header`  | Yes         |                                           | The name of a response header        |
 | `response_status`  | Yes         | `code`\|`reason`                          | The response status                  |