From 2c6ae153834f53a26ceaf80ae257af0848655201 Mon Sep 17 00:00:00 2001 From: atlas Date: Tue, 13 Oct 2026 15:01:28 +0200 Subject: [PATCH] swarm-review-classifier: test that Otel::record exports the run's cost Nothing exercised the real record path: the acp tests use a mock sink and the cost tests only covered amount(). Split the counter construction and the add out of Otel::record so a test can run them against an in-memory exporter, and assert the counter name, the amount and the four labels. Dev-dependency: opentelemetry_sdk's `testing` feature (same crate; adds tokio to its lockfile entry). --- Cargo.lock | 1 + swarm-review-classifier/Cargo.toml | 2 + swarm-review-classifier/src/cost.rs | 93 +++++++++++++++++++++++++---- 3 files changed, 83 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 48718522..08d2a4c6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3341,6 +3341,7 @@ dependencies = [ "portable-atomic", "rand 0.9.5", "thiserror 2.0.18", + "tokio", ] [[package]] diff --git a/swarm-review-classifier/Cargo.toml b/swarm-review-classifier/Cargo.toml index 47a4f412..9c0c8a05 100644 --- a/swarm-review-classifier/Cargo.toml +++ b/swarm-review-classifier/Cargo.toml @@ -33,6 +33,8 @@ tracing.workspace = true [dev-dependencies] tempfile = "3" +# `InMemoryMetricExporter` for the cost counter test. +opentelemetry_sdk = { workspace = true, features = ["testing"] } [lints] workspace = true diff --git a/swarm-review-classifier/src/cost.rs b/swarm-review-classifier/src/cost.rs index 9c3a75b6..78c0222c 100644 --- a/swarm-review-classifier/src/cost.rs +++ b/swarm-review-classifier/src/cost.rs @@ -66,18 +66,31 @@ impl CostSink for Otel { let Some((_, counter)) = COUNTER.get_or_init(build) else { return; }; - counter.add( - cost.amount, - &[ - KeyValue::new("repo", cost.repo.clone()), - KeyValue::new("pr", i64::try_from(cost.pr).unwrap_or(i64::MAX)), - KeyValue::new("model", cost.model.clone()), - KeyValue::new("currency", CURRENCY), - ], - ); + add(counter, cost); } } +/// The counter on `provider`. +fn cost_counter(provider: &SdkMeterProvider) -> Counter { + provider + .meter("hyperhive.review_classifier") + .f64_counter("hyperhive.review_classifier.cost.usage") + .build() +} + +/// Adds `cost` to `counter` with the run's labels. +fn add(counter: &Counter, cost: &Cost) { + counter.add( + cost.amount, + &[ + KeyValue::new("repo", cost.repo.clone()), + KeyValue::new("pr", i64::try_from(cost.pr).unwrap_or(i64::MAX)), + KeyValue::new("model", cost.model.clone()), + KeyValue::new("currency", CURRENCY), + ], + ); +} + /// The provider, kept alive with the counter since it exports only while it /// lives, or `None` when no collector is configured or the exporter failed. fn build() -> Option<(SdkMeterProvider, Counter)> { @@ -101,16 +114,18 @@ fn build() -> Option<(SdkMeterProvider, Counter)> { .with_interval(INTERVAL) .build(); let provider = SdkMeterProvider::builder().with_reader(reader).build(); - let counter = provider - .meter("hyperhive.review_classifier") - .f64_counter("hyperhive.review_classifier.cost.usage") - .build(); + let counter = cost_counter(&provider); Some((provider, counter)) } #[cfg(test)] mod tests { use super::*; + use opentelemetry::Value; + use opentelemetry_sdk::metrics::InMemoryMetricExporter; + use opentelemetry_sdk::metrics::data::{ + AggregatedMetrics, MetricData, ResourceMetrics, ScopeMetrics, + }; fn usage(input: u64, output: u64, cache_read: u64, cache_write: u64) -> TokenUsage { let mut usage = TokenUsage::default(); @@ -147,4 +162,56 @@ mod tests { let cost = amount(&prices, &usage(0, 0, 500_000, 500_000)); assert!((cost - 2.0).abs() < 1e-12, "{cost}"); } + + #[test] + fn a_record_adds_the_amount_under_the_counter_name_and_the_run_labels() { + let exporter = InMemoryMetricExporter::default(); + let reader = PeriodicReader::builder(exporter.clone()).build(); + let provider = SdkMeterProvider::builder().with_reader(reader).build(); + let counter = cost_counter(&provider); + let prices = Prices { + input: 3.0, + output: 15.0, + cache_read: None, + cache_write: None, + }; + let cost = Cost { + repo: "hyperhive/hyperhive".to_owned(), + pr: 5214, + model: "some-model".to_owned(), + amount: amount(&prices, &usage(1_000_000, 100_000, 0, 0)), + }; + add(&counter, &cost); + provider.force_flush().expect("flush"); + + let metrics = exporter.get_finished_metrics().expect("metrics"); + let metric = metrics + .iter() + .flat_map(ResourceMetrics::scope_metrics) + .flat_map(ScopeMetrics::metrics) + .find(|m| m.name() == "hyperhive.review_classifier.cost.usage") + .expect("the cost counter was exported"); + let AggregatedMetrics::F64(MetricData::Sum(sum)) = metric.data() else { + panic!("not an f64 sum: {:?}", metric.data()); + }; + let points: Vec<_> = sum.data_points().collect(); + assert_eq!(points.len(), 1); + let point = points[0]; + assert!((point.value() - 4.5).abs() < 1e-12, "{}", point.value()); + let labels: Vec<(String, Value)> = point + .attributes() + .map(|kv| (kv.key.to_string(), kv.value.clone())) + .collect(); + let label = |key: &str| { + labels + .iter() + .find(|(k, _)| k == key) + .map(|(_, v)| v.clone()) + }; + assert_eq!(label("repo"), Some(Value::from("hyperhive/hyperhive"))); + assert_eq!(label("pr"), Some(Value::from(5214_i64))); + assert_eq!(label("model"), Some(Value::from("some-model"))); + assert_eq!(label("currency"), Some(Value::from("USD"))); + assert_eq!(labels.len(), 4); + } }