feat(telemetry): add description and units to standard instruments (#…

…5407) Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
apollographql · Jun 12, 2024 · dcc9cbe · dcc9cbe
1 parent e178aaa
commit dcc9cbe
Show file tree

Hide file tree

Showing 10 changed files with 89 additions and 37 deletions.
diff --git a/.changesets/feat_bnjjj_add_missing_units_desc_instruments.md b/.changesets/feat_bnjjj_add_missing_units_desc_instruments.md
@@ -0,0 +1,5 @@
+### Add description and units to standard instruments ([PR #5407](https://github.com/apollographql/router/pull/5407))
+
+This PR adds description and units to standard instruments available in the router. These descriptions and units have been copy pasted directly from the [OpenTelemetry semantic conventions](https://opentelemetry.io/docs/specs/semconv/http/http-metrics/) and are needed for better integrations with APMs.
+
+By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/5407
diff --git a/...router/src/plugins/telemetry/config_new/fixtures/router/attribute.error.type/metrics.snap b/...router/src/plugins/telemetry/config_new/fixtures/router/attribute.error.type/metrics.snap
@@ -1,6 +1,6 @@
 ---
 source: apollo-router/src/plugins/telemetry/config_new/instruments.rs
-description: Custom counter
+description: error.type attribute
 expression: "&metrics.all()"
 info:
   telemetry:
@@ -13,6 +13,8 @@ info:
               error.type: true
 ---
 - name: http.server.request.duration
+  description: Duration of HTTP server requests.
+  unit: s
   data:
     datapoints:
       - sum: 0.1

diff --git a/.../src/plugins/telemetry/config_new/fixtures/router/attribute.on_graphql_error/metrics.snap b/.../src/plugins/telemetry/config_new/fixtures/router/attribute.on_graphql_error/metrics.snap
@@ -14,6 +14,8 @@ info:
                 on_graphql_error: true
 ---
 - name: http.server.request.duration
+  description: Duration of HTTP server requests.
+  unit: s
   data:
     datapoints:
       - sum: 0.1

diff --git a/...src/plugins/telemetry/config_new/fixtures/router/http.server.active_requests/metrics.snap b/...src/plugins/telemetry/config_new/fixtures/router/http.server.active_requests/metrics.snap
@@ -1,6 +1,6 @@
 ---
 source: apollo-router/src/plugins/telemetry/config_new/instruments.rs
-description: Test standard router metrics
+description: Active requests metrics
 expression: "&metrics.all()"
 info:
   telemetry:
@@ -9,8 +9,12 @@ info:
         router:
           http.server.active_requests: true
           http.server.request.duration: false
+        subgraph:
+          http.client.request.duration: false
 ---
 - name: http.server.active_requests
+  description: Number of active HTTP server requests.
+  unit: request
   data:
     datapoints:
       - value: 0

diff --git a/...c/plugins/telemetry/config_new/fixtures/router/http.server.request.body.size/metrics.snap b/...c/plugins/telemetry/config_new/fixtures/router/http.server.request.body.size/metrics.snap
@@ -1,6 +1,6 @@
 ---
 source: apollo-router/src/plugins/telemetry/config_new/instruments.rs
-description: Test server request body size metrics
+description: Server request body size metrics
 expression: "&metrics.all()"
 info:
   telemetry:
@@ -12,6 +12,8 @@ info:
           http.server.request.body.size: true
 ---
 - name: http.server.request.body.size
+  description: Size of HTTP server request bodies.
+  unit: By
   data:
     datapoints:
       - sum: 35

diff --git a/...fig_new/fixtures/router/http.server.request.body.size_with_custom_attributes/metrics.snap b/...fig_new/fixtures/router/http.server.request.body.size_with_custom_attributes/metrics.snap
@@ -1,6 +1,6 @@
 ---
 source: apollo-router/src/plugins/telemetry/config_new/instruments.rs
-description: Test server request body size metrics
+description: Server request body size metrics
 expression: "&metrics.all()"
 info:
   telemetry:
@@ -12,6 +12,8 @@ info:
           http.server.request.body.size: true
 ---
 - name: http.server.request.body.size
+  description: Size of HTTP server request bodies.
+  unit: By
   data:
     datapoints:
       - sum: 35

diff --git a/...rc/plugins/telemetry/config_new/fixtures/router/http.server.request.duration/metrics.snap b/...rc/plugins/telemetry/config_new/fixtures/router/http.server.request.duration/metrics.snap
@@ -11,6 +11,8 @@ info:
           http.server.request.duration: true
 ---
 - name: http.server.request.duration
+  description: Duration of HTTP server requests.
+  unit: s
   data:
     datapoints:
       - sum: 0.1

diff --git a/...nfig_new/fixtures/router/http.server.request.duration_with_custom_attributes/metrics.snap b/...nfig_new/fixtures/router/http.server.request.duration_with_custom_attributes/metrics.snap
@@ -14,6 +14,8 @@ info:
                 request_method: true
 ---
 - name: http.server.request.duration
+  description: Duration of HTTP server requests.
+  unit: s
   data:
     datapoints:
       - sum: 0.1

diff --git a/.../plugins/telemetry/config_new/fixtures/router/http.server.response.body.size/metrics.snap b/.../plugins/telemetry/config_new/fixtures/router/http.server.response.body.size/metrics.snap
@@ -1,6 +1,6 @@
 ---
 source: apollo-router/src/plugins/telemetry/config_new/instruments.rs
-description: Test server request body size metrics
+description: Server request body size metrics
 expression: "&metrics.all()"
 info:
   telemetry:
@@ -12,6 +12,8 @@ info:
           http.server.response.body.size: true
 ---
 - name: http.server.response.body.size
+  description: Size of HTTP server response bodies.
+  unit: By
   data:
     datapoints:
       - sum: 35

diff --git a/apollo-router/src/plugins/telemetry/config_new/instruments.rs b/apollo-router/src/plugins/telemetry/config_new/instruments.rs
@@ -93,7 +93,13 @@ impl InstrumentsConfig {
                 inner: Mutex::new(CustomHistogramInner {
                     increment: Increment::Duration(Instant::now()),
                     condition: Condition::True,
-                    histogram: Some(meter.f64_histogram("http.server.request.duration").init()),
+                    histogram: Some(
+                        meter
+                            .f64_histogram("http.server.request.duration")
+                            .with_unit(Unit::new("s"))
+                            .with_description("Duration of HTTP server requests.")
+                            .init(),
+                    ),
                     attributes: Vec::new(),
                     selector: None,
                     selectors: match &self.router.attributes.http_server_request_duration {
@@ -126,7 +132,11 @@ impl InstrumentsConfig {
                             increment: Increment::Custom(None),
                             condition: Condition::True,
                             histogram: Some(
-                                meter.f64_histogram("http.server.request.body.size").init(),
+                                meter
+                                    .f64_histogram("http.server.request.body.size")
+                                    .with_unit(Unit::new("By"))
+                                    .with_description("Size of HTTP server request bodies.")
+                                    .init(),
                             ),
                             attributes: Vec::with_capacity(nb_attributes),
                             selector: Some(Arc::new(RouterSelector::RequestHeader {
@@ -160,7 +170,11 @@ impl InstrumentsConfig {
                             increment: Increment::Custom(None),
                             condition: Condition::True,
                             histogram: Some(
-                                meter.f64_histogram("http.server.response.body.size").init(),
+                                meter
+                                    .f64_histogram("http.server.response.body.size")
+                                    .with_unit(Unit::new("By"))
+                                    .with_description("Size of HTTP server response bodies.")
+                                    .init(),
                             ),
                             attributes: Vec::with_capacity(nb_attributes),
                             selector: Some(Arc::new(RouterSelector::ResponseHeader {
@@ -183,6 +197,8 @@ impl InstrumentsConfig {
                     counter: Some(
                         meter
                             .i64_up_down_counter("http.server.active_requests")
+                            .with_unit(Unit::new("request"))
+                            .with_description("Number of active HTTP server requests.")
                             .init(),
                     ),
                     attrs_config: match &self.router.attributes.http_server_active_requests {
@@ -213,34 +229,39 @@ impl InstrumentsConfig {
 
     pub(crate) fn new_subgraph_instruments(&self) -> SubgraphInstruments {
         let meter = metrics::meter_provider().meter(METER_NAME);
-        let http_client_request_duration = self
-            .subgraph
-            .attributes
-            .http_client_request_duration
-            .is_enabled()
-            .then(|| {
-                let mut nb_attributes = 0;
-                let selectors = match &self.subgraph.attributes.http_client_request_duration {
-                    DefaultedStandardInstrument::Bool(_) | DefaultedStandardInstrument::Unset => {
-                        None
-                    }
-                    DefaultedStandardInstrument::Extendable { attributes } => {
-                        nb_attributes = attributes.custom.len();
-                        Some(attributes.clone())
+        let http_client_request_duration =
+            self.subgraph
+                .attributes
+                .http_client_request_duration
+                .is_enabled()
+                .then(|| {
+                    let mut nb_attributes = 0;
+                    let selectors = match &self.subgraph.attributes.http_client_request_duration {
+                        DefaultedStandardInstrument::Bool(_)
+                        | DefaultedStandardInstrument::Unset => None,
+                        DefaultedStandardInstrument::Extendable { attributes } => {
+                            nb_attributes = attributes.custom.len();
+                            Some(attributes.clone())
+                        }
+                    };
+                    CustomHistogram {
+                        inner: Mutex::new(CustomHistogramInner {
+                            increment: Increment::Duration(Instant::now()),
+                            condition: Condition::True,
+                            histogram: Some(
+                                meter
+                                    .f64_histogram("http.client.request.duration")
+                                    .with_unit(Unit::new("s"))
+                                    .with_description("Duration of HTTP client requests.")
+                                    .init(),
+                            ),
+                            attributes: Vec::with_capacity(nb_attributes),
+                            selector: None,
+                            selectors,
+                            updated: false,
+                        }),
                     }
-                };
-                CustomHistogram {
-                    inner: Mutex::new(CustomHistogramInner {
-                        increment: Increment::Duration(Instant::now()),
-                        condition: Condition::True,
-                        histogram: Some(meter.f64_histogram("http.client.request.duration").init()),
-                        attributes: Vec::with_capacity(nb_attributes),
-                        selector: None,
-                        selectors,
-                        updated: false,
-                    }),
-                }
-            });
+                });
         let http_client_request_body_size =
             self.subgraph
                 .attributes
@@ -261,7 +282,11 @@ impl InstrumentsConfig {
                             increment: Increment::Custom(None),
                             condition: Condition::True,
                             histogram: Some(
-                                meter.f64_histogram("http.client.request.body.size").init(),
+                                meter
+                                    .f64_histogram("http.client.request.body.size")
+                                    .with_unit(Unit::new("By"))
+                                    .with_description("Size of HTTP client request bodies.")
+                                    .init(),
                             ),
                             attributes: Vec::with_capacity(nb_attributes),
                             selector: Some(Arc::new(SubgraphSelector::SubgraphRequestHeader {
@@ -294,7 +319,11 @@ impl InstrumentsConfig {
                             increment: Increment::Custom(None),
                             condition: Condition::True,
                             histogram: Some(
-                                meter.f64_histogram("http.client.response.body.size").init(),
+                                meter
+                                    .f64_histogram("http.client.response.body.size")
+                                    .with_unit(Unit::new("By"))
+                                    .with_description("Size of HTTP client response bodies.")
+                                    .init(),
                             ),
                             attributes: Vec::with_capacity(nb_attributes),
                             selector: Some(Arc::new(SubgraphSelector::SubgraphResponseHeader {