Skip to content

Provide unit conversion for common non-second duration instruments (TSH-20621) #8415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
theJC wants to merge 8 commits into from
Closed

Provide unit conversion for common non-second duration instruments (TSH-20621) #8415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4bb451f
Provide unit conversion for common non-second duration instruments
theJC Oct 14, 2025
70e5df5
Add changeset
theJC Oct 14, 2025
093de82
Update documentation
theJC Oct 14, 2025
6486219
Update doc to incorporate suggested changes from Apollo librarian (AI…
theJC Oct 14, 2025
0186eb9
Apply lint formatting fixes
theJC Oct 14, 2025
3e997f1
Update doc to incorporate suggested changes from Apollo librarian (AI…
theJC Oct 15, 2025
2a688cc
Update changeset content per MR feedback
theJC Oct 15, 2025
e1951ce
Update .changesets/feat_convertNonSecondTimeUnits.md
theJC Oct 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .changesets/feat_convertNonSecondTimeUnits.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions apollo-router/src/plugins/telemetry/config_new/apollo/instruments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion apollo-router/src/plugins/telemetry/config_new/connector/instruments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
130 changes: 99 additions & 31 deletions apollo-router/src/plugins/telemetry/config_new/instruments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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))
Expand Down Expand Up @@ -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)
Expand All @@ -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))
Expand Down Expand Up @@ -1660,8 +1670,8 @@ pub(crate) enum Increment {
Unit,
EventUnit,
FieldUnit,
Duration(Instant),
EventDuration(Instant),
Duration(Instant, String),
EventDuration(Instant, String),
Custom(Option<i64>),
EventCustom(Option<i64>),
FieldCustom(Option<i64>),
Expand All @@ -1677,6 +1687,18 @@ fn to_i64(value: opentelemetry::Value) -> Option<i64> {
}
}

/// 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,
Copy link
Copy Markdown
Contributor

@BrynCooke BrynCooke Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not as_millis() as f64 for consistency with the other units?

Copy link
Copy Markdown
Contributor Author

@theJC theJC Oct 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good 👀. I'm using as_secs_f64() * 1000 here because:

"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<Request, Response, EventResponse, A, T>
where
A: Selectors<Request, Response, EventResponse> + Default,
Expand Down Expand Up @@ -1777,7 +1799,7 @@ where
if !matches!(
&inner.increment,
Increment::EventCustom(_)
| Increment::EventDuration(_)
| Increment::EventDuration(_, _)
| Increment::EventUnit
| Increment::FieldCustom(_)
| Increment::FieldUnit
Expand Down Expand Up @@ -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(_) => {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -2216,7 +2240,7 @@ where
if !matches!(
&inner.increment,
Increment::EventCustom(_)
| Increment::EventDuration(_)
| Increment::EventDuration(_, _)
| Increment::EventUnit
| Increment::FieldCustom(_)
| Increment::FieldUnit
Expand Down Expand Up @@ -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(_) => {
Expand Down Expand Up @@ -2307,8 +2333,8 @@ where

let increment: Option<f64> = 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
Expand All @@ -2320,7 +2346,7 @@ where
incr
}
Increment::Unit
| Increment::Duration(_)
| Increment::Duration(_, _)
| Increment::Custom(_)
| Increment::FieldUnit
| Increment::FieldCustom(_) => {
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
}
}
39 changes: 38 additions & 1 deletion .../routing/observability/router-telemetry-otel/enabling-telemetry/instruments.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,11 +400,48 @@
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:

Check notice on line 410 in docs/source/routing/observability/router-telemetry-otel/enabling-telemetry/instruments.mdx

View check run for this annotation

Apollo Librarian / AI Style Review

docs/source/routing/observability/router-telemetry-otel/enabling-telemetry/instruments.mdx#L410 

Adopt a more reader-centric voice by using 'you' to address the user directly.

```suggestion
For instruments with <code>value: duration</code> or <code>value: event_duration</code>, the router automatically converts the measured duration to match the configured <code>unit</code>. You can use any of the following time units:
```

- `s` - seconds (default, recommended)

Check notice on line 412 in docs/source/routing/observability/router-telemetry-otel/enabling-telemetry/instruments.mdx

View check run for this annotation

Apollo Librarian / AI Style Review

docs/source/routing/observability/router-telemetry-otel/enabling-telemetry/instruments.mdx#L412 

The following note already states that using seconds is the recommended best practice. This change removes the redundant word 'recommended'.

```suggestion
- <code>s</code> - seconds (default)
```
- `ms` - milliseconds
- `us` - microseconds
- `ns` - nanoseconds

<Note>

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.

</Note>

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.
Expand Down