Fix config metrics attribute types

apollographql · Sep 4, 2023 · 723d5dd · 723d5dd
1 parent decb7e9
commit 723d5dd
Show file tree

Hide file tree

Showing 11 changed files with 99 additions and 59 deletions.
diff --git a/apollo-router/src/configuration/metrics.rs b/apollo-router/src/configuration/metrics.rs
@@ -5,6 +5,7 @@ use std::time::Duration;
 
 use jsonpath_rust::JsonPathInst;
 use paste::paste;
+use serde::Serialize;
 use serde_json::Value;
 use tokio::sync::OwnedSemaphorePermit;
 
@@ -16,7 +17,39 @@ pub(crate) struct MetricsHandle {
 
 pub(crate) struct Metrics {
     yaml: Value,
-    metrics: HashMap<String, (u64, HashMap<String, String>)>,
+    metrics: HashMap<String, (u64, HashMap<String, AttributeValue>)>,
+}
+
+enum AttributeValue {
+    Bool(bool),
+    U64(u64),
+    I64(i64),
+    F64(f64),
+    String(String),
+}
+
+impl Serialize for AttributeValue {
+    fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
+        match self {
+            AttributeValue::Bool(value) => serializer.serialize_bool(*value),
+            AttributeValue::U64(value) => serializer.serialize_u64(*value),
+            AttributeValue::I64(value) => serializer.serialize_i64(*value),
+            AttributeValue::F64(value) => serializer.serialize_f64(*value),
+            AttributeValue::String(value) => serializer.serialize_str(value),
+        }
+    }
+}
+
+impl AttributeValue {
+    fn dyn_value(self: &AttributeValue) -> &dyn tracing::Value {
+        match self {
+            AttributeValue::Bool(value) => value as &dyn tracing::Value,
+            AttributeValue::U64(value) => value as &dyn tracing::Value,
+            AttributeValue::I64(value) => value as &dyn tracing::Value,
+            AttributeValue::F64(value) => value as &dyn tracing::Value,
+            AttributeValue::String(value) => value as &dyn tracing::Value,
+        }
+    }
 }
 
 impl Metrics {
@@ -98,12 +131,19 @@ impl Metrics {
                             let attr_name = stringify!([<$($attr __ )+>]).to_string();
                             match JsonPathInst::from_str($attr_path).expect("json path must be valid").find_slice(value).into_iter().next().as_deref() {
                                 // If the value is an object we can only state that it is set, but not what it is set to.
-                                Some(Value::Object(_value)) => {attributes.insert(attr_name, "true".to_string());},
-                                Some(Value::Array(value)) if !value.is_empty() => {attributes.insert(attr_name, "true".to_string());},
+                                Some(Value::Object(_value)) => {attributes.insert(attr_name, AttributeValue::Bool(true));},
+                                Some(Value::Array(value)) if !value.is_empty() => {attributes.insert(attr_name, AttributeValue::Bool(true));},
                                 // Scalars can be logged as is.
-                                Some(value) => {attributes.insert(attr_name, value.to_string());},
+                                Some(Value::Number(value)) if value.is_f64() => {attributes.insert(attr_name, AttributeValue::F64(value.as_f64().expect("checked, qed")));},
+                                Some(Value::Number(value)) if value.is_i64() => {attributes.insert(attr_name, AttributeValue::I64(value.as_i64().expect("checked, qed")));},
+                                Some(Value::Number(value)) => {attributes.insert(attr_name, AttributeValue::U64(value.as_u64().expect("checked, qed")));},
+                                Some(Value::String(value)) => {attributes.insert(attr_name, AttributeValue::String(value.clone()));},
+                                Some(Value::Bool(value)) => {attributes.insert(attr_name, AttributeValue::Bool(*value));},
+
                                 // If the value is not set we don't specify the attribute.
-                                None => {attributes.insert(attr_name, "false".to_string());},
+                                None => {attributes.insert(attr_name, AttributeValue::Bool(false));},
+
+                                _ => {},
                             };)+
                             (1, attributes)
                         }
@@ -113,7 +153,7 @@ impl Metrics {
                             let mut attributes = HashMap::new();
                             $(
                                 let attr_name = stringify!([<$($attr __ )+>]).to_string();
-                                attributes.insert(attr_name, "false".to_string());
+                                attributes.insert(attr_name, AttributeValue::Bool(false));
                             )+
                             (0, attributes)
                         }
@@ -122,7 +162,7 @@ impl Metrics {
 
                 // Now log the metric
                 paste!{
-                    tracing::info!($($metric).+ = metric.0, $($($attr).+ = metric.1.get(stringify!([<$($attr __ )+>])).expect("attribute must be in map")),+);
+                    tracing::info!($($metric).+ = metric.0, $($($attr).+ = metric.1.get(stringify!([<$($attr __ )+>])).expect("attribute must be in map").dyn_value()),+);
                 }
             };
         }

diff --git a/...ation/snapshots/apollo_router__configuration__metrics__test__metrics@apq.router.yaml.snap b/...ation/snapshots/apollo_router__configuration__metrics__test__metrics@apq.router.yaml.snap
@@ -4,7 +4,7 @@ expression: "&metrics.metrics"
 ---
 value.apollo.router.config.apq:
   - 1
-  - opt__router__cache__in_memory__: "true"
-    opt__router__cache__redis__: "true"
-    opt__subgraph__: "true"
+  - opt__router__cache__in_memory__: true
+    opt__router__cache__redis__: true
+    opt__subgraph__: true
 
diff --git a/...shots/apollo_router__configuration__metrics__test__metrics@authorization.router.yaml.snap b/...shots/apollo_router__configuration__metrics__test__metrics@authorization.router.yaml.snap
@@ -4,6 +4,6 @@ expression: "&metrics.metrics"
 ---
 value.apollo.router.config.authorization:
   - 1
-  - opt__directives__: "false"
-    opt__require_authentication__: "true"
+  - opt__directives__: false
+    opt__require_authentication__: true
 
diff --git a/...o_router__configuration__metrics__test__metrics@authorization_directives.router.yaml.snap b/...o_router__configuration__metrics__test__metrics@authorization_directives.router.yaml.snap
@@ -4,6 +4,6 @@ expression: "&metrics.metrics"
 ---
 value.apollo.router.config.authorization:
   - 1
-  - opt__directives__: "true"
-    opt__require_authentication__: "false"
+  - opt__directives__: true
+    opt__require_authentication__: false
 
diff --git a/...apshots/apollo_router__configuration__metrics__test__metrics@coprocessor.router.yaml.snap b/...apshots/apollo_router__configuration__metrics__test__metrics@coprocessor.router.yaml.snap
@@ -4,10 +4,10 @@ expression: "&metrics.metrics"
 ---
 value.apollo.router.config.coprocessor:
   - 1
-  - opt__router__request__: "true"
-    opt__router__response__: "true"
-    opt__subgraph__request__: "true"
-    opt__subgraph__response__: "true"
-    opt__supergraph__request__: "false"
-    opt__supergraph__response__: "false"
+  - opt__router__request__: true
+    opt__router__response__: true
+    opt__subgraph__request__: true
+    opt__subgraph__response__: true
+    opt__supergraph__request__: false
+    opt__supergraph__response__: false
 
diff --git a/.../snapshots/apollo_router__configuration__metrics__test__metrics@entities.router.yaml.snap b/.../snapshots/apollo_router__configuration__metrics__test__metrics@entities.router.yaml.snap
@@ -4,15 +4,15 @@ expression: "&metrics.metrics"
 ---
 value.apollo.router.config.entities:
   - 1
-  - opt__cache__: "true"
+  - opt__cache__: true
 value.apollo.router.config.traffic_shaping:
   - 1
-  - opt__router__rate_limit__: "false"
-    opt__router__timout__: "false"
-    opt__subgraph__compression__: "false"
-    opt__subgraph__deduplicate_query__: "false"
-    opt__subgraph__http2__: "false"
-    opt__subgraph__rate_limit__: "false"
-    opt__subgraph__retry__: "false"
-    opt__subgraph__timeout__: "false"
+  - opt__router__rate_limit__: false
+    opt__router__timout__: false
+    opt__subgraph__compression__: false
+    opt__subgraph__deduplicate_query__: false
+    opt__subgraph__http2__: false
+    opt__subgraph__rate_limit__: false
+    opt__subgraph__retry__: false
+    opt__subgraph__timeout__: false
 
diff --git a/...on/snapshots/apollo_router__configuration__metrics__test__metrics@limits.router.yaml.snap b/...on/snapshots/apollo_router__configuration__metrics__test__metrics@limits.router.yaml.snap
@@ -4,12 +4,12 @@ expression: "&metrics.metrics"
 ---
 value.apollo.router.config.limits:
   - 1
-  - opt__operation__max_aliases__: "true"
-    opt__operation__max_depth__: "true"
-    opt__operation__max_height__: "true"
-    opt__operation__max_root_fields__: "true"
-    opt__operation__warn_only__: "true"
-    opt__parser__max_recursion__: "true"
-    opt__parser__max_tokens__: "true"
-    opt__request__max_size__: "true"
+  - opt__operation__max_aliases__: true
+    opt__operation__max_depth__: true
+    opt__operation__max_height__: true
+    opt__operation__max_root_fields__: true
+    opt__operation__warn_only__: true
+    opt__parser__max_recursion__: true
+    opt__parser__max_tokens__: true
+    opt__request__max_size__: true
 
diff --git a/...s/apollo_router__configuration__metrics__test__metrics@persisted_queries.router.yaml.snap b/...s/apollo_router__configuration__metrics__test__metrics@persisted_queries.router.yaml.snap
@@ -4,7 +4,7 @@ expression: "&metrics.metrics"
 ---
 value.apollo.router.config.persisted_queries:
   - 1
-  - opt__log_unknown__: "true"
-    opt__safelist__enabled__: "true"
-    opt__safelist__require_id__: "true"
+  - opt__log_unknown__: true
+    opt__safelist__enabled__: true
+    opt__safelist__require_id__: true
 
diff --git a/...shots/apollo_router__configuration__metrics__test__metrics@subscriptions.router.yaml.snap b/...shots/apollo_router__configuration__metrics__test__metrics@subscriptions.router.yaml.snap
@@ -4,9 +4,9 @@ expression: "&metrics.metrics"
 ---
 value.apollo.router.config.subscriptions:
   - 1
-  - opt__deduplication__: "false"
-    opt__max_opened__: "true"
-    opt__mode__callback__: "true"
-    opt__mode__passthrough__: "true"
-    opt__queue_capacity__: "true"
+  - opt__deduplication__: false
+    opt__max_opened__: true
+    opt__mode__callback__: true
+    opt__mode__passthrough__: true
+    opt__queue_capacity__: true
 
diff --git a/...snapshots/apollo_router__configuration__metrics__test__metrics@telemetry.router.yaml.snap b/...snapshots/apollo_router__configuration__metrics__test__metrics@telemetry.router.yaml.snap
@@ -4,10 +4,10 @@ expression: "&metrics.metrics"
 ---
 value.apollo.router.config.telemetry:
   - 1
-  - opt__metrics__otlp__: "true"
-    opt__metrics__prometheus__: "true"
-    opt__tracing__datadog__: "true"
-    opt__tracing__jaeger__: "true"
-    opt__tracing__otlp__: "true"
-    opt__tracing__zipkin__: "true"
+  - opt__metrics__otlp__: true
+    opt__metrics__prometheus__: true
+    opt__tracing__datadog__: true
+    opt__tracing__jaeger__: true
+    opt__tracing__otlp__: true
+    opt__tracing__zipkin__: true
 
diff --git a/...ots/apollo_router__configuration__metrics__test__metrics@traffic_shaping.router.yaml.snap b/...ots/apollo_router__configuration__metrics__test__metrics@traffic_shaping.router.yaml.snap
@@ -4,12 +4,12 @@ expression: "&metrics.metrics"
 ---
 value.apollo.router.config.traffic_shaping:
   - 1
-  - opt__router__rate_limit__: "true"
-    opt__router__timout__: "true"
-    opt__subgraph__compression__: "true"
-    opt__subgraph__deduplicate_query__: "true"
-    opt__subgraph__http2__: "true"
-    opt__subgraph__rate_limit__: "true"
-    opt__subgraph__retry__: "true"
-    opt__subgraph__timeout__: "true"
+  - opt__router__rate_limit__: true
+    opt__router__timout__: true
+    opt__subgraph__compression__: true
+    opt__subgraph__deduplicate_query__: true
+    opt__subgraph__http2__: true
+    opt__subgraph__rate_limit__: true
+    opt__subgraph__retry__: true
+    opt__subgraph__timeout__: true