Skip to content

Commit c800f42

Browse files
mraszykclaude
andcommitted
feat(migration): always export the oldest-request-age metric
`migration_canister_oldest_request_in_flight_age_seconds` used to be omitted from the metrics output if no request was in flight, which makes the metric hard to use in queries and alerts. Export it unconditionally instead, reporting 0 if there is no pending request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent b074ca2 commit c800f42

3 files changed

Lines changed: 25 additions & 29 deletions

File tree

rs/migration_canister/src/canister_state.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,12 @@ pub mod requests {
133133
}
134134

135135
/// Returns the age in nanos of the request that has been in flight the longest,
136-
/// or `None` if there is no request in flight.
137-
pub fn oldest_request_age_nanos() -> Option<u64> {
136+
/// or 0 if there is no request in flight.
137+
pub fn oldest_request_age_nanos() -> u64 {
138138
let now = now();
139-
ACCEPTED_TIME.with_borrow(|a| a.values().map(|time| now.saturating_sub(time)).max())
139+
ACCEPTED_TIME
140+
.with_borrow(|a| a.values().map(|time| now.saturating_sub(time)).max())
141+
.unwrap_or_default()
140142
}
141143

142144
pub fn remove_request(request: &RequestState) {

rs/migration_canister/src/migration_canister.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,14 +133,12 @@ fn encode_metrics(w: &mut ic_metrics_encoder::MetricsEncoder<Vec<u8>>) -> std::i
133133
"Number of currently ongoing migration requests.",
134134
)?;
135135

136-
// This gauge is not set if there is no request in flight.
137-
if let Some(age_nanos) = oldest_request_age_nanos() {
138-
w.encode_gauge(
139-
"migration_canister_oldest_request_in_flight_age_seconds",
140-
age_nanos as f64 / 1_000_000_000_f64,
141-
"Age in seconds of the migration request that has been in flight the longest.",
142-
)?;
143-
}
136+
// This gauge is 0 if there is no request in flight.
137+
w.encode_gauge(
138+
"migration_canister_oldest_request_in_flight_age_seconds",
139+
oldest_request_age_nanos() as f64 / 1_000_000_000_f64,
140+
"Age in seconds of the migration request that has been in flight the longest.",
141+
)?;
144142

145143
w.encode_gauge(
146144
"migration_canister_num_successes_in_past_24_h",

rs/migration_canister/tests/tests.rs

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -816,22 +816,15 @@ async fn metrics() {
816816
0.0
817817
);
818818

819-
assert_eq!(oldest_request_age(&metrics), None);
819+
assert_eq!(oldest_request_age(&metrics), 0.0);
820820
}
821821

822822
const OLDEST_REQUEST_AGE_METRIC: &str = "migration_canister_oldest_request_in_flight_age_seconds";
823823

824824
/// Reads the age (in seconds) of the oldest request in flight.
825-
/// Returns `None` if the metric is not set, i.e., if no request is in flight.
826-
fn oldest_request_age(metrics: &Scrape) -> Option<f64> {
827-
let is_metric_set = metrics
828-
.samples
829-
.iter()
830-
.any(|sample| sample.metric == OLDEST_REQUEST_AGE_METRIC);
831-
if !is_metric_set {
832-
return None;
833-
}
834-
Some(get_gauge(metrics, OLDEST_REQUEST_AGE_METRIC))
825+
/// The metric is always set and equal to 0 if no request is in flight.
826+
fn oldest_request_age(metrics: &Scrape) -> f64 {
827+
get_gauge(metrics, OLDEST_REQUEST_AGE_METRIC)
835828
}
836829

837830
#[tokio::test]
@@ -865,8 +858,8 @@ async fn oldest_request_in_flight_metric() {
865858
))
866859
.await;
867860

868-
// Without any request in flight, the metric is not set.
869-
assert_eq!(oldest_request_age(&fetch_metrics(&pic).await), None);
861+
// Without any request in flight, the metric is 0.
862+
assert_eq!(oldest_request_age(&fetch_metrics(&pic).await), 0.0);
870863

871864
migrate_canister(&pic, sender, &first).await.unwrap();
872865
pic.advance_time(Duration::from_secs(10)).await;
@@ -878,7 +871,7 @@ async fn oldest_request_in_flight_metric() {
878871
get_gauge(&metrics, "migration_canister_requests_in_flight"),
879872
1.0
880873
);
881-
let age = oldest_request_age(&metrics).unwrap();
874+
let age = oldest_request_age(&metrics);
882875
assert!((10.0..100.0).contains(&age), "unexpected age {age}");
883876

884877
// A second, younger request does not affect the metric:
@@ -889,7 +882,7 @@ async fn oldest_request_in_flight_metric() {
889882
get_gauge(&metrics, "migration_canister_requests_in_flight"),
890883
2.0
891884
);
892-
let age = oldest_request_age(&metrics).unwrap();
885+
let age = oldest_request_age(&metrics);
893886
assert!((10.0..100.0).contains(&age), "unexpected age {age}");
894887

895888
// Drive both migrations to completion. We advance time by a lot so that
@@ -911,7 +904,7 @@ async fn oldest_request_in_flight_metric() {
911904
get_gauge(&metrics, "migration_canister_requests_in_flight"),
912905
0.0
913906
);
914-
assert_eq!(oldest_request_age(&metrics), None);
907+
assert_eq!(oldest_request_age(&metrics), 0.0);
915908
}
916909

917910
#[tokio::test]
@@ -931,14 +924,17 @@ async fn oldest_request_in_flight_metric_reset_on_failure() {
931924
};
932925

933926
migrate_canister(&pic, sender, &args).await.unwrap();
927+
pic.advance_time(Duration::from_secs(10)).await;
928+
pic.tick().await;
934929

935930
// The request has been accepted and thus its age is reported by the metric.
936931
let metrics = fetch_metrics(&pic).await;
937932
assert_eq!(
938933
get_gauge(&metrics, "migration_canister_requests_in_flight"),
939934
1.0
940935
);
941-
assert!(oldest_request_age(&metrics).is_some());
936+
let age = oldest_request_age(&metrics);
937+
assert!(age >= 10.0, "unexpected age {age}");
942938

943939
// Validation succeeded. Now we break migration by interfering.
944940
pic.start_canister(migrated_canister, Some(sender))
@@ -957,7 +953,7 @@ async fn oldest_request_in_flight_metric_reset_on_failure() {
957953
get_gauge(&metrics, "migration_canister_requests_in_flight"),
958954
0.0
959955
);
960-
assert_eq!(oldest_request_age(&metrics), None);
956+
assert_eq!(oldest_request_age(&metrics), 0.0);
961957
}
962958

963959
async fn concurrent_migration(

0 commit comments

Comments
 (0)