diff --git a/.changesets/feat_convertNonSecondTimeUnits.md b/.changesets/feat_convertNonSecondTimeUnits.md new file mode 100644 index 0000000000..41fadec5f1 --- /dev/null +++ b/.changesets/feat_convertNonSecondTimeUnits.md @@ -0,0 +1,30 @@ +### Automatic unit conversion for duration instruments with non-second units + +The Router now automatically converts duration measurements to match the configured unit for telemetry instruments. +Previously, duration instruments always recorded values in seconds regardless of the configured `unit` field. +With this enhancement, when you specify units like `"ms"` (milliseconds), `"us"` (microseconds), or `"ns"` (nanoseconds), +the Router will automatically convert the measured duration to the appropriate scale. + +**Supported units:** +- `"s"` - seconds (default) +- `"ms"` - milliseconds +- `"us"` - microseconds +- `"ns"` - nanoseconds + +Important Note: This should only be used in cases where you are required to integrate with an observability platform that does not properly translate from source timeunit into the necessary target timeunit (ie: seconds to milliseconds). In all other cases, +customers should follow the OLTP convention indicating you "SHOULD" use seconds as the unit. + +**Example:** +```yaml title="router.yaml" +telemetry: + instrumentation: + instruments: + subgraph: + acme.request.duration: + value: duration + type: histogram + unit: ms # Values are now automatically converted to milliseconds + description: "Metric to get the request duration in milliseconds" +``` + +By [Jon Christiansen](https://github.com/theJC) in https://github.com/apollographql/router/pull/8415 diff --git a/apollo-router/src/plugins/telemetry/config_new/apollo/instruments.rs b/apollo-router/src/plugins/telemetry/config_new/apollo/instruments.rs index 0e33707bca..4196811d41 100644 --- a/apollo-router/src/plugins/telemetry/config_new/apollo/instruments.rs +++ b/apollo-router/src/plugins/telemetry/config_new/apollo/instruments.rs @@ -122,7 +122,7 @@ impl ApolloSubgraphInstruments { let apollo_router_operations_fetch_duration = apollo_config.subgraph_metrics.then(|| { CustomHistogram::builder() - .increment(Increment::Duration(Instant::now())) + .increment(Increment::Duration(Instant::now(), "s".to_string())) .attributes(Vec::with_capacity(attribute_count)) .selectors(Arc::new(selectors)) .histogram(static_instruments @@ -246,7 +246,7 @@ impl ApolloConnectorInstruments { let apollo_router_operations_fetch_duration = apollo_config.subgraph_metrics.then(|| { CustomHistogram::builder() - .increment(Increment::Duration(Instant::now())) + .increment(Increment::Duration(Instant::now(), "s".to_string())) .attributes(Vec::with_capacity(attribute_count)) .selectors(Arc::new(selectors)) .histogram(static_instruments diff --git a/apollo-router/src/plugins/telemetry/config_new/connector/instruments.rs b/apollo-router/src/plugins/telemetry/config_new/connector/instruments.rs index c58b0cbad4..4b3c451a98 100644 --- a/apollo-router/src/plugins/telemetry/config_new/connector/instruments.rs +++ b/apollo-router/src/plugins/telemetry/config_new/connector/instruments.rs @@ -123,7 +123,7 @@ impl ConnectorInstruments { }; CustomHistogram { inner: Mutex::new(CustomHistogramInner { - increment: Increment::Duration(Instant::now()), + increment: Increment::Duration(Instant::now(), "s".to_string()), condition: Condition::True, histogram: Some(static_instruments .get(HTTP_CLIENT_REQUEST_DURATION_METRIC) diff --git a/apollo-router/src/plugins/telemetry/config_new/instruments.rs b/apollo-router/src/plugins/telemetry/config_new/instruments.rs index ea5dfcb831..53bd4dd2d0 100644 --- a/apollo-router/src/plugins/telemetry/config_new/instruments.rs +++ b/apollo-router/src/plugins/telemetry/config_new/instruments.rs @@ -292,7 +292,7 @@ impl InstrumentsConfig { .is_enabled() .then(|| CustomHistogram { inner: Mutex::new(CustomHistogramInner { - increment: Increment::Duration(Instant::now()), + increment: Increment::Duration(Instant::now(), "s".to_string()), condition: Condition::True, histogram: Some( static_instruments @@ -605,7 +605,7 @@ impl InstrumentsConfig { }; CustomHistogram { inner: Mutex::new(CustomHistogramInner { - increment: Increment::Duration(Instant::now()), + increment: Increment::Duration(Instant::now(), "s".to_string()), condition: Condition::True, histogram: Some(static_instruments .get(HTTP_CLIENT_REQUEST_DURATION_METRIC) @@ -1440,7 +1440,9 @@ where let (selector, increment) = match (&instrument.value).into() { InstrumentValue::Standard(incr) => { let incr = match incr { - Standard::Duration => Increment::Duration(Instant::now()), + Standard::Duration => { + Increment::Duration(Instant::now(), instrument.unit.clone()) + } Standard::Unit => Increment::Unit, }; (None, incr) @@ -1449,7 +1451,10 @@ where (Some(Arc::new(selector)), Increment::Custom(None)) } InstrumentValue::Chunked(incr) => match incr { - Event::Duration => (None, Increment::EventDuration(Instant::now())), + Event::Duration => ( + None, + Increment::EventDuration(Instant::now(), instrument.unit.clone()), + ), Event::Unit => (None, Increment::EventUnit), Event::Custom(selector) => { (Some(Arc::new(selector)), Increment::EventCustom(None)) @@ -1496,7 +1501,9 @@ where let (selector, increment) = match (&instrument.value).into() { InstrumentValue::Standard(incr) => { let incr = match incr { - Standard::Duration => Increment::Duration(Instant::now()), + Standard::Duration => { + Increment::Duration(Instant::now(), instrument.unit.clone()) + } Standard::Unit => Increment::Unit, }; (None, incr) @@ -1505,7 +1512,10 @@ where (Some(Arc::new(selector)), Increment::Custom(None)) } InstrumentValue::Chunked(incr) => match incr { - Event::Duration => (None, Increment::EventDuration(Instant::now())), + Event::Duration => ( + None, + Increment::EventDuration(Instant::now(), instrument.unit.clone()), + ), Event::Unit => (None, Increment::EventUnit), Event::Custom(selector) => { (Some(Arc::new(selector)), Increment::EventCustom(None)) @@ -1660,8 +1670,8 @@ pub(crate) enum Increment { Unit, EventUnit, FieldUnit, - Duration(Instant), - EventDuration(Instant), + Duration(Instant, String), + EventDuration(Instant, String), Custom(Option), EventCustom(Option), FieldCustom(Option), @@ -1677,6 +1687,18 @@ fn to_i64(value: opentelemetry::Value) -> Option { } } +/// Convert a duration to f64 based on the specified unit. +/// Supported units: "s" (seconds), "ms" (milliseconds), "us" (microseconds), "ns" (nanoseconds) +/// Defaults to seconds for any other unit string. +fn duration_to_f64(duration: std::time::Duration, unit: &str) -> f64 { + match unit { + "ms" => duration.as_secs_f64() * 1000.0, + "us" => duration.as_micros() as f64, + "ns" => duration.as_nanos() as f64, + _ => duration.as_secs_f64(), // Default to seconds for "s" or any other unit + } +} + pub(crate) struct CustomCounter where A: Selectors + Default, @@ -1777,7 +1799,7 @@ where if !matches!( &inner.increment, Increment::EventCustom(_) - | Increment::EventDuration(_) + | Increment::EventDuration(_, _) | Increment::EventUnit | Increment::FieldCustom(_) | Increment::FieldUnit @@ -1814,13 +1836,13 @@ where let increment = match inner.increment { Increment::Unit => 1f64, - Increment::Duration(instant) => instant.elapsed().as_secs_f64(), + Increment::Duration(instant, ref unit) => duration_to_f64(instant.elapsed(), unit), Increment::Custom(val) => match val { Some(incr) => incr as f64, None => 0f64, }, Increment::EventUnit - | Increment::EventDuration(_) + | Increment::EventDuration(_, _) | Increment::EventCustom(_) | Increment::FieldUnit | Increment::FieldCustom(_) => { @@ -1873,8 +1895,8 @@ where let increment = match &mut inner.increment { Increment::EventUnit => 1f64, - Increment::EventDuration(instant) => { - let incr = instant.elapsed().as_secs_f64(); + Increment::EventDuration(instant, unit) => { + let incr = duration_to_f64(instant.elapsed(), unit); // Set it to new instant for the next event *instant = Instant::now(); incr @@ -1912,8 +1934,9 @@ where let increment = match inner.increment { Increment::Unit | Increment::EventUnit | Increment::FieldUnit => 1f64, - Increment::Duration(instant) | Increment::EventDuration(instant) => { - instant.elapsed().as_secs_f64() + Increment::Duration(instant, ref unit) + | Increment::EventDuration(instant, ref unit) => { + duration_to_f64(instant.elapsed(), unit) } Increment::Custom(val) | Increment::EventCustom(val) | Increment::FieldCustom(val) => { match val { @@ -1970,9 +1993,9 @@ where incr } Increment::Unit - | Increment::Duration(_) + | Increment::Duration(_, _) | Increment::Custom(_) - | Increment::EventDuration(_) + | Increment::EventDuration(_, _) | Increment::EventCustom(_) | Increment::EventUnit => { // Nothing to do because we're incrementing on fields @@ -2018,8 +2041,9 @@ where if let Some(counter) = inner.counter.take() { let incr: f64 = match &inner.increment { Increment::Unit | Increment::EventUnit => 1f64, - Increment::Duration(instant) | Increment::EventDuration(instant) => { - instant.elapsed().as_secs_f64() + Increment::Duration(instant, unit) + | Increment::EventDuration(instant, unit) => { + duration_to_f64(instant.elapsed(), unit) } Increment::Custom(val) | Increment::EventCustom(val) => match val { Some(incr) => *incr as f64, @@ -2216,7 +2240,7 @@ where if !matches!( &inner.increment, Increment::EventCustom(_) - | Increment::EventDuration(_) + | Increment::EventDuration(_, _) | Increment::EventUnit | Increment::FieldCustom(_) | Increment::FieldUnit @@ -2252,10 +2276,12 @@ where let increment = match inner.increment { Increment::Unit => Some(1f64), - Increment::Duration(instant) => Some(instant.elapsed().as_secs_f64()), + Increment::Duration(instant, ref unit) => { + Some(duration_to_f64(instant.elapsed(), unit)) + } Increment::Custom(val) => val.map(|incr| incr as f64), Increment::EventUnit - | Increment::EventDuration(_) + | Increment::EventDuration(_, _) | Increment::EventCustom(_) | Increment::FieldUnit | Increment::FieldCustom(_) => { @@ -2307,8 +2333,8 @@ where let increment: Option = match &mut inner.increment { Increment::EventUnit => Some(1f64), - Increment::EventDuration(instant) => { - let incr = Some(instant.elapsed().as_secs_f64()); + Increment::EventDuration(instant, unit) => { + let incr = Some(duration_to_f64(instant.elapsed(), unit)); // Need a new instant for the next event *instant = Instant::now(); incr @@ -2320,7 +2346,7 @@ where incr } Increment::Unit - | Increment::Duration(_) + | Increment::Duration(_, _) | Increment::Custom(_) | Increment::FieldUnit | Increment::FieldCustom(_) => { @@ -2345,8 +2371,9 @@ where let increment = match inner.increment { Increment::Unit | Increment::EventUnit | Increment::FieldUnit => Some(1f64), - Increment::Duration(instant) | Increment::EventDuration(instant) => { - Some(instant.elapsed().as_secs_f64()) + Increment::Duration(instant, ref unit) + | Increment::EventDuration(instant, ref unit) => { + Some(duration_to_f64(instant.elapsed(), unit)) } Increment::Custom(val) | Increment::EventCustom(val) | Increment::FieldCustom(val) => { val.map(|incr| incr as f64) @@ -2400,9 +2427,9 @@ where incr } Increment::Unit - | Increment::Duration(_) + | Increment::Duration(_, _) | Increment::Custom(_) - | Increment::EventDuration(_) + | Increment::EventDuration(_, _) | Increment::EventCustom(_) | Increment::EventUnit => { // Nothing to do because we're incrementing on fields @@ -2447,8 +2474,9 @@ where if let Some(histogram) = inner.histogram.take() { let increment = match &inner.increment { Increment::Unit | Increment::EventUnit => Some(1f64), - Increment::Duration(instant) | Increment::EventDuration(instant) => { - Some(instant.elapsed().as_secs_f64()) + Increment::Duration(instant, unit) + | Increment::EventDuration(instant, unit) => { + Some(duration_to_f64(instant.elapsed(), unit)) } Increment::Custom(val) | Increment::EventCustom(val) => { val.map(|incr| incr as f64) @@ -3840,4 +3868,44 @@ mod tests { .with_metrics() .await; } + + #[test] + fn test_duration_to_f64_seconds() { + let duration = std::time::Duration::from_secs(2); + assert_eq!(duration_to_f64(duration, "s"), 2.0); + assert_eq!(duration_to_f64(duration, ""), 2.0); // empty unit defaults to seconds + assert_eq!(duration_to_f64(duration, "unknown"), 2.0); // unknown unit defaults to seconds + } + + #[test] + fn test_duration_to_f64_milliseconds() { + let duration = std::time::Duration::from_millis(1500); + assert_eq!(duration_to_f64(duration, "ms"), 1500.0); + } + + #[test] + fn test_duration_to_f64_microseconds() { + let duration = std::time::Duration::from_micros(500); + assert_eq!(duration_to_f64(duration, "us"), 500.0); + } + + #[test] + fn test_duration_to_f64_nanoseconds() { + let duration = std::time::Duration::from_nanos(1234567); + assert_eq!(duration_to_f64(duration, "ns"), 1234567.0); + } + + #[test] + fn test_duration_to_f64_fractional_seconds() { + let duration = std::time::Duration::from_millis(1500); + // When unit is "s", 1500ms should be 1.5 seconds + assert_eq!(duration_to_f64(duration, "s"), 1.5); + } + + #[test] + fn test_duration_to_f64_fractional_milliseconds() { + let duration = std::time::Duration::from_micros(1500); + // When unit is "ms", 1500us should be 1.5 milliseconds + assert_eq!(duration_to_f64(duration, "ms"), 1.5); + } } diff --git a/docs/source/routing/observability/router-telemetry-otel/enabling-telemetry/instruments.mdx b/docs/source/routing/observability/router-telemetry-otel/enabling-telemetry/instruments.mdx index 3848f7d586..08476022da 100644 --- a/docs/source/routing/observability/router-telemetry-otel/enabling-telemetry/instruments.mdx +++ b/docs/source/routing/observability/router-telemetry-otel/enabling-telemetry/instruments.mdx @@ -400,11 +400,48 @@ telemetry: instrumentation: instruments: router: - acme.metric: + acme.metric: # ... unit: s # seconds ``` +##### Duration unit conversion + +For instruments with `value: duration` or `value: event_duration`, the router automatically converts the measured duration to match the configured `unit`. This feature supports the following time units: + +- `s` - seconds (default, recommended) +- `ms` - milliseconds +- `us` - microseconds +- `ns` - nanoseconds + + + +Best practice: Follow the [OpenTelemetry semantic conventions](https://opentelemetry.io/docs/specs/semconv/general/metrics/#instrument-units) and use seconds (`s`) as the unit for duration measurements. + +Only use non-second units (`ms`, `us`, `ns`) when integrating with an observability platform that requires a specific unit and doesn't automatically convert time units. + + + +Example: + +```yaml title="router.yaml" +telemetry: + instrumentation: + instruments: + router: + # Recommended: use seconds (values recorded as seconds) + acme.request.duration: + value: duration + type: histogram + unit: s + + # Only if required by your observability platform + otheracme.request.duration: + value: duration + type: histogram + unit: ms # Values automatically converted to milliseconds +``` + #### `description` A free format description of the instrument that will be displayed in your APM.