Add support of instruments in configuration for telemetry

.changesets/feat_bnjjj_feat_4319.md

@@ -0,0 +1,43 @@
+### Add support of instruments in configuration for telemetry ([Issue #4319](https://github.com/apollographql/router/issues/4319))
+
+Add support for custom and standard instruments through the configuration file. You'll be able to add your own custom metrics just using the configuration file. They may:
+- be conditional
+- get values from selectors, for instance headers, context or body
+- have different types like `histogram` or `counter`.
+
+Example:
+
+```yaml title="router.yaml"
+telemetry:
+  instrumentation:
+    instruments:
+      router:
+        http.server.active_requests: true
+        acme.request.duration:
+          value: duration
+          type: counter
+          unit: kb
+          description: "my description"
+          attributes:
+            http.response.status_code: true
+            "my_attribute":
+              response_header: "x-my-header"
+
+      supergraph:
+        acme.graphql.requests:
+          value: unit
+          type: counter
+          unit: count
+          description: "supergraph requests"
+
+      subgraph:
+        acme.graphql.subgraph.errors:
+          value: unit
+          type: counter
+          unit: count
+          description: "my description"
+```
+
+[Documentation](https://www.apollographql.com/docs/router/configuration/telemetry/instrumentation/instruments)
+
+By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/4771

apollo-router/src/configuration/metrics.rs

@@ -307,6 +307,14 @@ impl InstrumentData {
             "$..events",
             opt.instruments,
             "$..instruments",
+            opt.instruments.router,
+            "$..instruments.router",
+            opt.instruments.supergraph,
+            "$..instruments.supergraph",
+            opt.instruments.subgraph,
+            "$..instruments.subgraph",
+            opt.instruments.default_attribute_requirement_level,
+            "$..instruments.default_attribute_requirement_level",
             opt.spans,
             "$..spans",
             opt.spans.mode,

apollo-router/src/configuration/snapshots/apollo_router__configuration__metrics__test__metrics@telemetry.router.yaml.snap

@@ -8,7 +8,11 @@ expression: "&metrics.non_zero()"
       - value: 1
         attributes:
           opt.events: false
-          opt.instruments: false
+          opt.instruments: true
+          opt.instruments.default_attribute_requirement_level: false
+          opt.instruments.router: true
+          opt.instruments.subgraph: true
+          opt.instruments.supergraph: true
           opt.logging.experimental_when_header: true
           opt.metrics.otlp: true
           opt.metrics.prometheus: true

apollo-router/src/configuration/testdata/metrics/telemetry.router.yaml

@@ -44,3 +44,58 @@ telemetry:
       subgraph:
         attributes:
           subgraph.graphql.document: true
+    instruments:
+      router:
+        http.server.request.body.size:
+          attributes:
+            # Standard attributes
+            http.response.status_code: true
+            "my_attribute":
+              response_header: "content-type"
+        http.server.request.duration:
+          attributes:
+            # Standard attributes
+            http.response.status_code: true
+            http.request.method: true
+            # Custom attribute
+            "my_attribute":
+              response_header: "content-type"
+        my.request.duration: # The name of your custom instrument/metric
+          value: duration
+          type: counter
+          unit: s
+          description: "my description"
+        acme.request.size: # The name of your custom instrument/metric
+          value:
+           request_header: "content-length"
+          type: counter
+          unit: s
+          description: "my description" 
+
+        acme.request.length: # The name of your custom instrument/metric
+          value:
+           request_header: "content-length"
+          type: histogram
+          unit: s
+          description: "my description"
+      supergraph:
+        acme.graphql.requests:
+          value: unit
+          type: counter
+          unit: request
+          description: "supergraph requests"
+          attributes:
+            static: hello
+            graphql_operation_kind:
+              operation_kind: string
+      subgraph:
+        request_including_price1:
+          value: unit
+          type: counter
+          unit: request
+          description: "supergraph requests"
+          condition:
+            exists:
+             subgraph_response_data: "$.products[*].price1"
+          attributes:
+            subgraph.name: true

apollo-router/src/metrics/mod.rs

@@ -185,27 +185,27 @@ pub(crate) mod test_utils {
         ) -> bool {
             let attributes = AttributeSet::from(attributes);
             if let Some(value) = value.to_u64() {
-                if self.metric_exists(name, &ty, value, &attributes) {
+                if self.metric_matches(name, &ty, value, &attributes) {
                     return true;
                 }
             }
 
             if let Some(value) = value.to_i64() {
-                if self.metric_exists(name, &ty, value, &attributes) {
+                if self.metric_matches(name, &ty, value, &attributes) {
                     return true;
                 }
             }
 
             if let Some(value) = value.to_f64() {
-                if self.metric_exists(name, &ty, value, &attributes) {
+                if self.metric_matches(name, &ty, value, &attributes) {
                     return true;
                 }
             }
 
             false
         }
 
-        fn metric_exists<T: Debug + PartialEq + Display + ToPrimitive + 'static>(
+        pub(crate) fn metric_matches<T: Debug + PartialEq + Display + ToPrimitive + 'static>(
             &self,
             name: &str,
             ty: &MetricType,
@@ -231,11 +231,47 @@ pub(crate) mod test_utils {
                 } else if let Some(histogram) = metric.data.as_any().downcast_ref::<Histogram<T>>()
                 {
                     if matches!(ty, MetricType::Histogram) {
-                        if let Some(value) = value.to_u64() {
-                            return histogram.data_points.iter().any(|datapoint| {
-                                datapoint.attributes == *attributes && datapoint.count == value
-                            });
-                        }
+                        return histogram.data_points.iter().any(|datapoint| {
+                            datapoint.attributes == *attributes && datapoint.sum == value
+                        });
+                    }
+                }
+            }
+            false
+        }
+
+        pub(crate) fn metric_exists<T: Debug + PartialEq + Display + ToPrimitive + 'static>(
+            &self,
+            name: &str,
+            ty: MetricType,
+            attributes: &[KeyValue],
+        ) -> bool {
+            let attributes = AttributeSet::from(attributes);
+            if let Some(metric) = self.find(name) {
+                // Try to downcast the metric to each type of aggregation and assert that the value is correct.
+                if let Some(gauge) = metric.data.as_any().downcast_ref::<Gauge<T>>() {
+                    // Find the datapoint with the correct attributes.
+                    if matches!(ty, MetricType::Gauge) {
+                        return gauge
+                            .data_points
+                            .iter()
+                            .any(|datapoint| datapoint.attributes == attributes);
+                    }
+                } else if let Some(sum) = metric.data.as_any().downcast_ref::<Sum<T>>() {
+                    // Note that we can't actually tell if the sum is monotonic or not, so we just check if it's a sum.
+                    if matches!(ty, MetricType::Counter | MetricType::UpDownCounter) {
+                        return sum
+                            .data_points
+                            .iter()
+                            .any(|datapoint| datapoint.attributes == attributes);
+                    }
+                } else if let Some(histogram) = metric.data.as_any().downcast_ref::<Histogram<T>>()
+                {
+                    if matches!(ty, MetricType::Histogram) {
+                        return histogram
+                            .data_points
+                            .iter()
+                            .any(|datapoint| datapoint.attributes == attributes);
                     }
                 }
             }
@@ -905,7 +941,7 @@ macro_rules! assert_gauge {
 }
 
 #[cfg(test)]
-macro_rules! assert_histogram {
+macro_rules! assert_histogram_sum {
 
     ($($name:ident).+, $value: expr, $($attr_key:literal = $attr_value:expr),+) => {
         let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
@@ -932,11 +968,77 @@ macro_rules! assert_histogram {
     };
 
     ($name:literal, $value: expr) => {
-        let result = crate::metrics::collect_metrics().assert($name, $value, &[]);
+        let result = crate::metrics::collect_metrics().assert($name, crate::metrics::test_utils::MetricType::Histogram, $value, &[]);
         assert_metric!(result, $name, None, Some($value.into()), &[]);
     };
 }
 
+#[cfg(test)]
+macro_rules! assert_histogram_exists {
+
+    ($($name:ident).+, $value: ty, $($attr_key:literal = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
+        let result = crate::metrics::collect_metrics().metric_exists::<$value>(stringify!($($name).+), crate::metrics::test_utils::MetricType::Histogram, &attributes);
+        assert_metric!(result, $name, None, None, &attributes);
+    };
+
+    ($($name:ident).+, $value: ty, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        let result = crate::metrics::collect_metrics().metric_exists::<$value>(stringify!($($name).+), crate::metrics::test_utils::MetricType::Histogram, &attributes);
+        assert_metric!(result, $name, None, None, &attributes);
+    };
+
+    ($name:literal, $value: ty, $($attr_key:literal = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
+        let result = crate::metrics::collect_metrics().metric_exists::<$value>($name, crate::metrics::test_utils::MetricType::Histogram, &attributes);
+        assert_metric!(result, $name, None, None, &attributes);
+    };
+
+    ($name:literal, $value: ty, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        let result = crate::metrics::collect_metrics().metric_exists::<$value>($name, crate::metrics::test_utils::MetricType::Histogram, &attributes);
+        assert_metric!(result, $name, None, None, &attributes);
+    };
+
+    ($name:literal, $value: ty) => {
+        let result = crate::metrics::collect_metrics().metric_exists::<$value>($name, crate::metrics::test_utils::MetricType::Histogram, &[]);
+        assert_metric!(result, $name, None, None, &[]);
+    };
+}
+
+#[cfg(test)]
+macro_rules! assert_histogram_not_exists {
+
+    ($($name:ident).+, $value: ty, $($attr_key:literal = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
+        let result = crate::metrics::collect_metrics().metric_exists::<$value>(stringify!($($name).+), crate::metrics::test_utils::MetricType::Histogram, &attributes);
+        assert_metric!(!result, $name, None, None, &attributes);
+    };
+
+    ($($name:ident).+, $value: ty, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        let result = crate::metrics::collect_metrics().metric_exists::<$value>(stringify!($($name).+), crate::metrics::test_utils::MetricType::Histogram, &attributes);
+        assert_metric!(!result, $name, None, None, &attributes);
+    };
+
+    ($name:literal, $value: ty, $($attr_key:literal = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
+        let result = crate::metrics::collect_metrics().metric_exists::<$value>($name, crate::metrics::test_utils::MetricType::Histogram, &attributes);
+        assert_metric!(!result, $name, None, None, &attributes);
+    };
+
+    ($name:literal, $value: ty, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        let result = crate::metrics::collect_metrics().metric_exists::<$value>($name, crate::metrics::test_utils::MetricType::Histogram, &attributes);
+        assert_metric!(!result, $name, None, None, &attributes);
+    };
+
+    ($name:literal, $value: ty) => {
+        let result = crate::metrics::collect_metrics().metric_exists::<$value>($name, crate::metrics::test_utils::MetricType::Histogram, &[]);
+        assert_metric!(!result, $name, None, None, &[]);
+    };
+}
+
 #[cfg(test)]
 #[allow(unused_macros)]
 macro_rules! assert_metrics_snapshot {
@@ -1143,7 +1245,7 @@ mod test {
     async fn test_u64_histogram() {
         async {
             u64_histogram!("test", "test description", 1, "attr" = "val");
-            assert_histogram!("test", 1, "attr" = "val");
+            assert_histogram_sum!("test", 1, "attr" = "val");
         }
         .with_metrics()
         .await;
@@ -1153,7 +1255,7 @@ mod test {
     async fn test_i64_histogram() {
         async {
             i64_histogram!("test", "test description", 1, "attr" = "val");
-            assert_histogram!("test", 1, "attr" = "val");
+            assert_histogram_sum!("test", 1, "attr" = "val");
         }
         .with_metrics()
         .await;
@@ -1163,7 +1265,7 @@ mod test {
     async fn test_f64_histogram() {
         async {
             f64_histogram!("test", "test description", 1.0, "attr" = "val");
-            assert_histogram!("test", 1, "attr" = "val");
+            assert_histogram_sum!("test", 1, "attr" = "val");
         }
         .with_metrics()
         .await;
@@ -1185,7 +1287,7 @@ mod test {
     async fn test_type_counter() {
         async {
             f64_counter!("test", "test description", 1.0, "attr" = "val");
-            assert_histogram!("test", 1, "attr" = "val");
+            assert_histogram_sum!("test", 1, "attr" = "val");
         }
         .with_metrics()
         .await;
@@ -1196,7 +1298,7 @@ mod test {
     async fn test_type_up_down_counter() {
         async {
             f64_up_down_counter!("test", "test description", 1.0, "attr" = "val");
-            assert_histogram!("test", 1, "attr" = "val");
+            assert_histogram_sum!("test", 1, "attr" = "val");
         }
         .with_metrics()
         .await;
@@ -1211,7 +1313,7 @@ mod test {
                 .u64_observable_gauge("test")
                 .with_callback(|m| m.observe(5, &[]))
                 .init();
-            assert_histogram!("test", 1, "attr" = "val");
+            assert_histogram_sum!("test", 1, "attr" = "val");
         }
         .with_metrics()
         .await;

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

@@ -88,9 +88,8 @@ pub(crate) struct Instrumentation {
     pub(crate) events: config_new::events::Events,
     /// Span configuration
     pub(crate) spans: config_new::spans::Spans,
-    #[serde(skip)]
     /// Instrument configuration
-    pub(crate) instruments: config_new::instruments::Instruments,
+    pub(crate) instruments: config_new::instruments::InstrumentsConfig,
 }
 
 /// Metrics configuration
@@ -168,14 +167,14 @@ impl TryInto<Box<dyn View>> for MetricView {
                     record_min_max: true,
                 },
             );
-        let mut instrument = Instrument::new().name(self.name);
+        let instrument = Instrument::new().name(self.name);
+        let mut mask = Stream::new();
         if let Some(desc) = self.description {
-            instrument = instrument.description(desc);
+            mask = mask.description(desc);
         }
         if let Some(unit) = self.unit {
-            instrument = instrument.unit(Unit::new(unit));
+            mask = mask.unit(Unit::new(unit));
         }
-        let mut mask = Stream::new();
         if let Some(aggregation) = aggregation {
             mask = mask.aggregation(aggregation);
         }