From d91e6eb0e2fd3cc863a27a49292ab00ed24045e4 Mon Sep 17 00:00:00 2001 From: damocles Date: Fri, 10 Jul 2026 21:17:39 +0200 Subject: [PATCH] fix(hive-metric): blocking OTLP client (no-reactor panic), reject negative counters, allow negative gauges --- Cargo.lock | 3 ++- hive-metric/Cargo.toml | 8 ++++++-- hive-metric/src/main.rs | 31 +++++++++++++++++++++++++++---- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c5872403..e505921b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1591,7 +1591,6 @@ dependencies = [ "opentelemetry", "opentelemetry-otlp", "opentelemetry_sdk", - "tokio", ] [[package]] @@ -3425,7 +3424,9 @@ checksum = "219c5811de6525e5416c7d5d53bb656d3afdbc6c5af816e0802bcfa42dbdc1c3" dependencies = [ "base64", "bytes", + "futures-channel", "futures-core", + "futures-util", "http", "http-body", "http-body-util", diff --git a/hive-metric/Cargo.toml b/hive-metric/Cargo.toml index 794d135f..d71614be 100644 --- a/hive-metric/Cargo.toml +++ b/hive-metric/Cargo.toml @@ -13,10 +13,14 @@ clap.workspace = true # OTEL SDK — only used by this crate, so kept local rather than in workspace.dependencies. opentelemetry = "0.32" opentelemetry_sdk = { version = "0.32", features = ["metrics"] } +# Blocking (not async) reqwest client on purpose: the metrics SDK drives the +# OTLP push from a `PeriodicReader` background thread that has no Tokio runtime, +# so the async client panics there with "no reactor running". The blocking +# client sends on that thread directly — and for a fire-and-forget CLI that +# records one point then `shutdown()`s, a synchronous send is exactly right. opentelemetry-otlp = { version = "0.32", default-features = false, features = [ "metrics", "http-json", - "reqwest-client", + "reqwest-blocking-client", "reqwest-rustls", ] } -tokio = { workspace = true, features = ["rt", "macros"] } diff --git a/hive-metric/src/main.rs b/hive-metric/src/main.rs index 931509a3..48e066eb 100644 --- a/hive-metric/src/main.rs +++ b/hive-metric/src/main.rs @@ -24,13 +24,19 @@ use opentelemetry_otlp::{Protocol, WithExportConfig}; #[derive(Parser)] #[command( name = "hive-metric", - about = "Push a labeled metric to the OTEL collector (requires services.hyperhive.otel.enable = true)" + about = "Push a labeled metric to the OTEL collector (requires services.hyperhive.otel.enable = true)", + // Let a leading-minus value parse as the `value` positional rather than + // being rejected as an unknown flag — negative gauges are legitimate + // (e.g. a delta), and it lets the counter-sign check below produce a clear + // domain error instead of clap's "unexpected argument '-1'". + allow_negative_numbers = true )] struct Cli { /// Metric name (e.g. `tasks_completed`, `latency_ms`). name: String, - /// Numeric value (f64). Floats and integers both accepted. + /// Numeric value (f64). Floats and integers both accepted. Gauges may be + /// negative; counters must be >= 0 (they only increase). value: f64, /// Metric kind: `counter` (cumulative sum, default) or `gauge` (instantaneous value). @@ -52,8 +58,7 @@ enum MetricKind { Gauge, } -#[tokio::main] -async fn main() -> Result<()> { +fn main() -> Result<()> { let cli = Cli::parse(); // Fail early with a clear message when OTEL is not configured. @@ -66,6 +71,18 @@ async fn main() -> Result<()> { let labels = parse_labels(&cli.labels)?; + // Counters are monotonically non-negative — the OTEL SDK silently drops a + // negative `add`, so a typo like `hive-metric tasks_done -1` would vanish + // with exit 0. Reject it loudly instead. + if matches!(cli.metric_type, MetricKind::Counter) && cli.value < 0.0 { + bail!( + "counter '{}' value must be >= 0 (got {}); counters only increase — \ + use `--type gauge` for a value that can go down", + cli.name, + cli.value + ); + } + let exporter = opentelemetry_otlp::MetricExporter::builder() .with_http() .with_protocol(Protocol::HttpJson) @@ -73,6 +90,12 @@ async fn main() -> Result<()> { .context("failed to build OTLP metric exporter")?; // SDK automatically reads OTEL_RESOURCE_ATTRIBUTES for resource labels. + // The metrics SDK has no `with_simple_exporter` (that's a *traces*-only + // API) — a push exporter is always driven by a periodic reader. For this + // fire-and-forget CLI the timer never actually fires: we record one metric + // and immediately `shutdown()`, which force-flushes the pending export + // synchronously before returning. So there's no background-timer flush race + // in practice — the single point is exported exactly once on shutdown. let provider = opentelemetry_sdk::metrics::SdkMeterProvider::builder() .with_periodic_exporter(exporter) .build();