From 03c1f14dbafb8f2b03ca576e371fac34be3371bc Mon Sep 17 00:00:00 2001 From: lda Date: Tue, 20 Oct 2026 01:47:48 +0700 Subject: [PATCH] fix(telemetry): use seconds-based buckets for operation durations --- docs/telemetry-recipes.md | 9 +++++++++ wakey-core/src/telemetry.rs | 6 ++++++ wakey-core/tests/telemetry.rs | 31 +++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/docs/telemetry-recipes.md b/docs/telemetry-recipes.md index 7d3f014..da3d2c9 100644 --- a/docs/telemetry-recipes.md +++ b/docs/telemetry-recipes.md @@ -155,6 +155,15 @@ This needs classic histogram bucket series. Keep `le` in the grouping: it is the bucket boundary, in seconds. p95 is an estimate from those boundaries, not an exact sort of recorded durations. Few operations make percentiles noisy. +Operation-duration buckets span 100 microseconds to 60 seconds, with an overflow +bucket above that. Older binaries used SDK defaults whose first positive boundary +was 5 seconds: fast operations could therefore report an estimated p95 of 4.75 +seconds. Updating both CC and agents fixes future measurements, not historical +data. Wait until the query's window contains only new-version measurements after +rollout; mixing old and new bucket layouts makes the estimate unreliable. +With roughly one snapshot per minute, a five-minute percentile is based on only +a handful of observations and can still look stepped. Use the mean above alongside it. + CC relay duration includes waiting for the agent; agent execution duration measures its dispatch separately. Do not sum both and call that user latency. diff --git a/wakey-core/src/telemetry.rs b/wakey-core/src/telemetry.rs index 61a1216..dabcdc8 100644 --- a/wakey-core/src/telemetry.rs +++ b/wakey-core/src/telemetry.rs @@ -338,6 +338,12 @@ impl Instruments { duration: meter .f64_histogram("wakey.operation.duration") .with_unit("s") + // Measurements are seconds; SDK defaults start at 5, which + // collapses millisecond operations into one unusably wide bucket. + .with_boundaries(vec![ + 0.0001, 0.0005, 0.001, 0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1.0, 2.5, 5.0, + 10.0, 30.0, 60.0, + ]) .build(), snapshot_devices: meter.u64_counter("wakey.snapshot.devices").build(), terminal_active: meter.i64_up_down_counter("wakey.terminal.active").build(), diff --git a/wakey-core/tests/telemetry.rs b/wakey-core/tests/telemetry.rs index fc0ec85..97e26e0 100644 --- a/wakey-core/tests/telemetry.rs +++ b/wakey-core/tests/telemetry.rs @@ -409,6 +409,37 @@ async fn exports_all_signals_and_preserves_remote_command_parent() { assert!(names.contains(&name), "missing {name}"); } let latest = metrics.last().unwrap(); + { + let duration = latest + .resource_metrics + .iter() + .flat_map(|r| &r.scope_metrics) + .flat_map(|s| &s.metrics) + .find(|m| m.name == "wakey.operation.duration") + .unwrap(); + assert_eq!(duration.unit, "s"); + let Some(metric::Data::Histogram(histogram)) = &duration.data else { + panic!("operation duration must export a histogram"); + }; + let point = histogram.data_points.first().unwrap(); + assert_eq!( + point.explicit_bounds, + vec![ + 0.0001, 0.0005, 0.001, 0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1.0, 2.5, 5.0, + 10.0, 30.0, 60.0, + ] + ); + // One inventory operation is recorded above. Derive its expected bucket + // from the exported sum, avoiding elapsed-time thresholds or sleeps. + assert_eq!(point.count, 1); + let sample = point.sum.unwrap(); + let bucket = point + .explicit_bounds + .partition_point(|bound| *bound < sample); + let mut expected = vec![0; point.explicit_bounds.len() + 1]; + expected[bucket] = 1; + assert_eq!(point.bucket_counts, expected); + } { for (name, unit, expected) in [ (