Compare commits

..
5 changed files with 12 additions and 143 deletions

View file

@ -36,8 +36,6 @@ mod socket_server;
mod stats;
mod stores;
mod swarm_status;
#[cfg(test)]
mod test_env;
mod webhook_secret;
mod workers;

View file

@ -2222,12 +2222,8 @@ mod tests {
// no endpoint signal, no hyperhive.otel lines are emitted (agents
// keep the the harness modules disabled default).
//
// Serialised against every other env-mutating test in the crate.
// `stats::otel_metrics::tests` perturbs HYPERHIVE_OTEL_ENDPOINT too,
// and lands in this same test binary — "no other test asserts on
// these" was true of this module and false of the process.
let _env = crate::test_env::lock();
// SAFETY: serialised by the guard above; restored before returning.
// SAFETY: single-threaded mutation of process env vars no other
// test asserts on; restored before returning.
let render = || {
render_flake(
"github:example/hyperhive",

View file

@ -1,9 +1,8 @@
//! 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, 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
//! 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
//! 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.
@ -46,8 +45,8 @@ static SNAPSHOT: OnceLock<Arc<Mutex<Vec<ContainerResource>>>> = OnceLock::new();
static PROVIDER: OnceLock<SdkMeterProvider> = OnceLock::new();
/// Spawn the container-resource OTEL exporter if OTEL is configured
/// (`OTEL_EXPORTER_OTLP_ENDPOINT` non-empty). No-op otherwise. Call once at
/// startup.
/// (`HYPERHIVE_OTEL_ENDPOINT` non-empty — the same enable signal
/// `meta::otel_config` uses). 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");
@ -70,7 +69,7 @@ pub fn spawn_exporter(hyperhive_flake: &str) {
}
});
match build_provider(interval, snapshot, hyperhive_flake) {
match build_provider(&endpoint, interval, snapshot, hyperhive_flake) {
Ok(provider) => {
let _ = PROVIDER.set(provider);
tracing::info!(%endpoint, ?interval, "otel container-metrics: exporter enabled");
@ -82,6 +81,7 @@ pub fn spawn_exporter(hyperhive_flake: &str) {
}
fn build_provider(
endpoint: &str,
interval: Duration,
snapshot: Arc<Mutex<Vec<ContainerResource>>>,
hyperhive_flake: &str,
@ -90,24 +90,12 @@ 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")?;
@ -280,12 +268,9 @@ fn host_arch() -> &'static str {
}
}
/// `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`).
/// `HYPERHIVE_OTEL_ENDPOINT`, non-empty. The enable signal.
fn endpoint() -> Option<String> {
std::env::var("OTEL_EXPORTER_OTLP_ENDPOINT")
std::env::var("HYPERHIVE_OTEL_ENDPOINT")
.ok()
.map(|s| s.trim().to_owned())
.filter(|s| !s.is_empty())
@ -366,57 +351,4 @@ 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() {
// Serialised against every other env-mutating test in the crate. Both
// variables below are also read by `meta::tests` in this same test
// binary, so "no other test in this module" would be the wrong
// boundary — the module is not the unit that shares the environment,
// the process is.
let _env = crate::test_env::lock();
// SAFETY: serialised by the guard above; both names are restored (to
// absent) before it drops at the end of this test.
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");
}
}
}

View file

@ -1,39 +0,0 @@
//! Test-only: the crate's single lock for tests that mutate process
//! environment variables.
//!
//! Environment variables are one process-global, and every `#[test]` in this
//! crate lands in the *same* test binary, run on parallel threads. A test that
//! sets a variable, asserts, then restores it is only safe against a *concurrent*
//! test if both take the same lock — so any test here that touches the
//! environment must route through [`lock`] rather than rolling its own
//! save/restore.
//!
//! ⚠️ **The lock has to be crate-wide, not per-module.** `hive-bash-mcp`'s
//! sibling helper records why: two separate per-module mutexes serialise
//! nothing against each other, and that produced a CI-only flake. The pair that
//! motivated this one is `meta::tests` (which renders a flake from
//! `HYPERHIVE_OTEL_*`) and `stats::otel_metrics::tests` (which asserts which
//! variable enables the exporter) — different modules, same variable, same
//! binary.
//!
//! ⚠️ **A green local run is weak evidence for this class.** An agent container
//! has the hyperhive variables ambient-set, so a losing race still finds a
//! plausible value; the nix sandbox strips them, so there the race can delete a
//! variable out from under another thread. Reproduce the sandbox shape with
//! `env -u HYPERHIVE_OTEL_ENDPOINT cargo test -p hive-c0re`.
use std::sync::{Mutex, MutexGuard, PoisonError};
static ENV_LOCK: Mutex<()> = Mutex::new(());
/// Serialise this test against every other environment-mutating test in the
/// crate. Hold the returned guard for as long as the variables are perturbed —
/// bind it (`let _env = lock();`), never discard it with `let _ = lock();`,
/// which drops the guard immediately and serialises nothing.
///
/// Recovers from poisoning: a test that panicked mid-mutation has already
/// failed and reported, and refusing to run every later test on top of that
/// turns one failure into a cascade that hides which test actually broke.
pub fn lock() -> MutexGuard<'static, ()> {
ENV_LOCK.lock().unwrap_or_else(PoisonError::into_inner)
}

View file

@ -81,11 +81,6 @@ 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
@ -93,20 +88,7 @@ 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 = 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;
HYPERHIVE_OTEL_ENDPOINT = "http://${config.services.hyperhive.network.bridgeIp}:${toString otel.collector.port}";
# 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