fix(telemetry): fix supergraph query selector

.changesets/fix_bnjjj_fix_880.md

@@ -0,0 +1,53 @@
+### Fix telemetry instrumentation using supergraph query selector ([PR #6324](https://github.com/apollographql/router/pull/6324))
+
+Query selector was raising error logs like `this is a bug and should not happen`. It's now fixed.
+Now this configuration will work properly:
+```yaml title=router.yaml
+telemetry:
+  exporters:
+    metrics:
+      common:
+        views:
+          # Define a custom view because operation limits are different than the default latency-oriented view of OpenTelemetry
+          - name: oplimits.*
+            aggregation:
+              histogram:
+                buckets:
+                  - 0
+                  - 5
+                  - 10
+                  - 25
+                  - 50
+                  - 100
+                  - 500
+                  - 1000
+  instrumentation:
+    instruments:
+      supergraph:
+        oplimits.aliases:
+          value:
+            query: aliases
+          type: histogram
+          unit: number
+          description: "Aliases for an operation"
+        oplimits.depth:
+          value:
+            query: depth
+          type: histogram
+          unit: number
+          description: "Depth for an operation"
+        oplimits.height:
+          value:
+            query: height
+          type: histogram
+          unit: number
+          description: "Height for an operation"
+        oplimits.root_fields:
+          value:
+            query: root_fields
+          type: histogram
+          unit: number
+          description: "Root fields for an operation"
+```
+
+By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/6324

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

@@ -1446,7 +1446,7 @@ where
                             })
                         }
                         None => {
-                            ::tracing::error!("cannot convert static instrument into a counter, this is an error; please fill an issue on GitHub");
+                            failfast_debug!("cannot convert static instrument into a counter, this is an error; please fill an issue on GitHub");
                         }
                     }
                 }
@@ -1501,7 +1501,7 @@ where
                             });
                         }
                         None => {
-                            ::tracing::error!("cannot convert static instrument into a histogram, this is an error; please fill an issue on GitHub");
+                            failfast_debug!("cannot convert static instrument into a histogram, this is an error; please fill an issue on GitHub");
                         }
                     }
                 }

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

@@ -1069,25 +1069,6 @@ impl Selector for SupergraphSelector {
         ctx: &Context,
     ) -> Option<opentelemetry::Value> {
         match self {
-            SupergraphSelector::Query { query, .. } => {
-                let limits_opt = ctx
-                    .extensions()
-                    .with_lock(|lock| lock.get::<OperationLimits<u32>>().cloned());
-                match query {
-                    Query::Aliases => {
-                        limits_opt.map(|limits| opentelemetry::Value::I64(limits.aliases as i64))
-                    }
-                    Query::Depth => {
-                        limits_opt.map(|limits| opentelemetry::Value::I64(limits.depth as i64))
-                    }
-                    Query::Height => {
-                        limits_opt.map(|limits| opentelemetry::Value::I64(limits.height as i64))
-                    }
-                    Query::RootFields => limits_opt
-                        .map(|limits| opentelemetry::Value::I64(limits.root_fields as i64)),
-                    Query::String => None,
-                }
-            }
             SupergraphSelector::ResponseData {
                 response_data,
                 default,
@@ -3289,12 +3270,6 @@ mod test {
                 .unwrap(),
             4.into()
         );
-        assert_eq!(
-            selector
-                .on_response_event(&crate::graphql::Response::builder().build(), &context)
-                .unwrap(),
-            4.into()
-        );
     }
 
     #[test]

apollo-router/tests/integration/telemetry/fixtures/graphql.router.yaml

@@ -9,6 +9,31 @@ telemetry:
 
   instrumentation:
     instruments:
+      supergraph:
+        oplimits.aliases:
+          value:
+            query: aliases
+          type: histogram
+          unit: number
+          description: "Aliases for an operation"
+        oplimits.depth:
+          value:
+            query: depth
+          type: histogram
+          unit: number
+          description: "Depth for an operation"
+        oplimits.height:
+          value:
+            query: height
+          type: histogram
+          unit: number
+          description: "Height for an operation"
+        oplimits.root_fields:
+          value:
+            query: root_fields
+          type: histogram
+          unit: number
+          description: "Root fields for an operation"
       graphql:
         field.execution: true
         list.length: true
@@ -38,7 +63,4 @@ telemetry:
           condition:
             eq:
               - field_name: string
-              - "topProducts"
-
-
-
+              - "topProducts"

apollo-router/tests/integration/telemetry/metrics.rs

@@ -201,6 +201,27 @@ async fn test_graphql_metrics() {
     router.start().await;
     router.assert_started().await;
     router.execute_default_query().await;
+    router
+        .assert_log_not_contains("this is a bug and should not happen")
+        .await;
+    router
+        .assert_metrics_contains(
+            r#"oplimits_aliases_sum{otel_scope_name="apollo/router"} 0"#,
+            None,
+        )
+        .await;
+    router
+        .assert_metrics_contains(
+            r#"oplimits_root_fields_sum{otel_scope_name="apollo/router"} 1"#,
+            None,
+        )
+        .await;
+    router
+        .assert_metrics_contains(
+            r#"oplimits_depth_sum{otel_scope_name="apollo/router"} 2"#,
+            None,
+        )
+        .await;
     router
             .assert_metrics_contains(r#"graphql_field_list_length_sum{graphql_field_name="topProducts",graphql_field_type="Product",graphql_type_name="Query",otel_scope_name="apollo/router"} 3"#, None)
             .await;