From 76f6b7c3b7907c1764b16fbf5889d15b6bb4df38 Mon Sep 17 00:00:00 2001 From: atlas Date: Wed, 19 Aug 2026 00:16:34 +0200 Subject: [PATCH] fix(otel): let the SDK resolve hive-c0re's OTLP endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hive-c0re's container-resource exporter has POSTed to a 404 for as long as it has existed, silently: it passed the collector's base address to `with_endpoint`, which the SDK takes verbatim, so every export went to `/` instead of `/v1/metrics`. Nothing reported it — OTLP export failures go to an error handler no binary here installs — so the daemon logged "exporter enabled" and delivered nothing. VictoriaMetrics has never held a sample under `service.name=hyperhive-c0re`. Fix the way the rest of the repo already resolves an endpoint: an endpoint option names a BASE, and the layer that knows the signal appends to it. `hive-metric` — same SDK, same collector — never calls `with_endpoint`, and `docs/observability.md` documents the append as system behaviour; the one place a full path is spelled out is the VictoriaMetrics exporter, because its far end is not a standard OTLP path. So drop the call. The builder is now byte-identical to hive-metric's, and hive-c0re's unit carries the standard `OTEL_EXPORTER_OTLP_ENDPOINT` for the SDK to read. The address is bound once in nix and consumed twice, so what a hive hands its agents and what it exports to itself cannot drift. The enable signal moves to that same standard variable: "configured" and "where it actually goes" become one string rather than two that agree by convention. `HYPERHIVE_OTEL_*` keeps its own job, the agent-config transport meta.rs reads — a name the SDK has never known, which is the bug. The test changes shape with the fix. The old one asserted a URL this module built; the new one pins that the exporter is gated on the variable the SDK itself reads, because the fix is now an absence and an absence is what a later "the endpoint is right there, just pass it" edit puts back. Refs #3402 --- hive-c0re/src/stats/otel_metrics.rs | 80 +++++++++++++++++++--- nix/host-modules/hive-c0re/environment.nix | 20 +++++- 2 files changed, 90 insertions(+), 10 deletions(-) diff --git a/hive-c0re/src/stats/otel_metrics.rs b/hive-c0re/src/stats/otel_metrics.rs index 0883f04e..0adf3f6a 100644 --- a/hive-c0re/src/stats/otel_metrics.rs +++ b/hive-c0re/src/stats/otel_metrics.rs @@ -1,8 +1,9 @@ //! Per-agent container-resource OTEL export. hive-c0re already samples each //! agent container's cgroup load for the dashboard //! ([`super::container_stats`]); this rides those same gauges out to the -//! hive's own collector — the same first hop the agents use, arriving as -//! `HYPERHIVE_OTEL_ENDPOINT` (see `nix/host-modules/hive-c0re`). No toggle of +//! hive's own collector — the same first hop the agents use, named by the +//! standard `OTEL_EXPORTER_OTLP_ENDPOINT` (see `nix/host-modules/hive-c0re`, +//! which sets it from the same binding it hands agents). No toggle of //! its own, and no credential: a hive's collector takes unauthenticated OTLP //! on the bridge, and the only hop that presents anything is the swarm tier's, //! which is the one that leaves the swarm. @@ -45,8 +46,8 @@ static SNAPSHOT: OnceLock>>> = OnceLock::new(); static PROVIDER: OnceLock = OnceLock::new(); /// Spawn the container-resource OTEL exporter if OTEL is configured -/// (`HYPERHIVE_OTEL_ENDPOINT` non-empty — the same enable signal -/// `meta::otel_config` uses). No-op otherwise. Call once at startup. +/// (`OTEL_EXPORTER_OTLP_ENDPOINT` non-empty). No-op otherwise. Call once at +/// startup. pub fn spawn_exporter(hyperhive_flake: &str) { let Some(endpoint) = endpoint() else { tracing::debug!("otel container-metrics: no endpoint configured, exporter disabled"); @@ -69,7 +70,7 @@ pub fn spawn_exporter(hyperhive_flake: &str) { } }); - match build_provider(&endpoint, interval, snapshot, hyperhive_flake) { + match build_provider(interval, snapshot, hyperhive_flake) { Ok(provider) => { let _ = PROVIDER.set(provider); tracing::info!(%endpoint, ?interval, "otel container-metrics: exporter enabled"); @@ -81,7 +82,6 @@ pub fn spawn_exporter(hyperhive_flake: &str) { } fn build_provider( - endpoint: &str, interval: Duration, snapshot: Arc>>, hyperhive_flake: &str, @@ -90,12 +90,24 @@ fn build_provider( // hive-metric). The Claude SDK path honours `HYPERHIVE_OTEL_PROTOCOL` for // its own export; this exporter is always http/json. // + // The address is deliberately NOT passed here. The SDK reads + // `OTEL_EXPORTER_OTLP_ENDPOINT` itself and appends the signal path + // (`/v1/metrics`); `with_endpoint` is taken **verbatim**, so handing it a + // collector's base address POSTs to `/` and 404s on every export — with + // nothing logged, because OTLP export failures go to an error handler no + // binary here installs. That is not hypothetical: it was this module's + // behaviour for its whole existence, and no sample ever reached the store. + // An endpoint names a BASE throughout hyperhive and the + // layer that knows the signal appends to it (the one exception is the + // VictoriaMetrics exporter, whose far end is not a standard OTLP path). + // Construction is identical to `hive-metric`'s on purpose: two producers, + // one collector, one way to resolve the address. + // // No auth headers: the destination is this hive's own collector, which // takes unauthenticated OTLP on the bridge. Adding one here would put the // upstream credential on a hop that never uses it. let exporter = MetricExporter::builder() .with_http() - .with_endpoint(endpoint) .with_protocol(Protocol::HttpJson) .build() .context("build OTLP metric exporter")?; @@ -268,9 +280,12 @@ fn host_arch() -> &'static str { } } -/// `HYPERHIVE_OTEL_ENDPOINT`, non-empty. The enable signal. +/// `OTEL_EXPORTER_OTLP_ENDPOINT`, non-empty. The enable signal — and the very +/// variable the SDK reads to build the exporter's URL, so "configured" and +/// "where it goes" cannot disagree. Read here only to decide whether to start +/// at all; the value is never handed to the builder (see `build_provider`). fn endpoint() -> Option { - std::env::var("HYPERHIVE_OTEL_ENDPOINT") + std::env::var("OTEL_EXPORTER_OTLP_ENDPOINT") .ok() .map(|s| s.trim().to_owned()) .filter(|s| !s.is_empty()) @@ -351,4 +366,51 @@ mod tests { #[cfg(target_arch = "aarch64")] assert_eq!(host_arch(), "arm64"); } + + /// The exporter's destination must come from the standard OTLP variable, + /// because that is the only spelling the SDK appends the signal path to. + /// + /// Asserted rather than left to review because the fix here is an + /// *absence* — no `with_endpoint` call — and an absence is exactly what + /// a later "the endpoint is right there, just pass it" edit restores. If + /// this exporter is ever gated on a variable the SDK does not itself read, + /// it resumes posting to a base URL that 404s in silence. + #[test] + fn endpoint_is_the_standard_otlp_var() { + // SAFETY: single-threaded mutation of a process env var no other test + // in this module asserts on; both names are cleared before returning. + unsafe { + std::env::remove_var("OTEL_EXPORTER_OTLP_ENDPOINT"); + std::env::set_var("HYPERHIVE_OTEL_ENDPOINT", "http://hyperhive.invalid:4318"); + } + assert_eq!( + endpoint(), + None, + "the hive-wide agent-config variable must NOT enable this exporter — \ + the SDK does not read it, so its address would never reach the builder" + ); + + unsafe { + std::env::set_var("OTEL_EXPORTER_OTLP_ENDPOINT", " http://10.42.0.1:4318 "); + } + assert_eq!( + endpoint(), + Some("http://10.42.0.1:4318".to_owned()), + "the standard variable enables the exporter, trimmed" + ); + + unsafe { + std::env::set_var("OTEL_EXPORTER_OTLP_ENDPOINT", " "); + } + assert_eq!( + endpoint(), + None, + "whitespace-only is not a configured endpoint" + ); + + unsafe { + std::env::remove_var("OTEL_EXPORTER_OTLP_ENDPOINT"); + std::env::remove_var("HYPERHIVE_OTEL_ENDPOINT"); + } + } } diff --git a/nix/host-modules/hive-c0re/environment.nix b/nix/host-modules/hive-c0re/environment.nix index 64975b30..92b612e8 100644 --- a/nix/host-modules/hive-c0re/environment.nix +++ b/nix/host-modules/hive-c0re/environment.nix @@ -81,6 +81,11 @@ in # don't render no-op env lines. let otel = config.services.hyperhive.otel; + # The first hop, bound once and consumed twice below: what agents are + # handed, and where hive-c0re's own exporter sends. One binding so the + # address a hive tells its agents about and the one it uses itself + # cannot drift apart. + firstHop = "http://${config.services.hyperhive.network.bridgeIp}:${toString otel.collector.port}"; in { # `otel.endpoint` means "where telemetry ultimately goes" and keeps @@ -88,7 +93,20 @@ in # always this hive's own collector. Deriving it rather than # redefining `endpoint` is what lets every existing deployment keep # its configured value untouched. - HYPERHIVE_OTEL_ENDPOINT = "http://${config.services.hyperhive.network.bridgeIp}:${toString otel.collector.port}"; + HYPERHIVE_OTEL_ENDPOINT = firstHop; + # hive-c0re's OWN container-resource exporter (stats/otel_metrics.rs) + # reads the STANDARD OTLP variable — the same one hive-metric and every + # agent read — and lets the SDK resolve the URL, which appends the + # signal path (`/v1/metrics`). Handing the SDK an address + # programmatically instead takes it verbatim: it POSTs to the + # collector's root, gets a 404 on every export, and says nothing, + # because OTLP export failures go to an error handler no binary here + # installs — the daemon logged "exporter enabled" and never delivered a + # sample, for as long as it had that exporter. The variable above cannot + # replace this one: + # it is the agent-config transport meta.rs reads, and the SDK does not + # know that name. + OTEL_EXPORTER_OTLP_ENDPOINT = firstHop; # The first hop is the collector's OTLP/HTTP receiver, which speaks # protobuf regardless of what the upstream wants — `otel.protocol` # describes the *upstream* link, and the collector's own exporter is