From 8101290933289a827cfb70a81fbdf94b0f73d2c9 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sun, 23 Aug 2026 06:56:00 +0200 Subject: [PATCH] CB-632: rename the exported metrics bridged_* -> fleet_* Part of #145 (CB-632). Doing this NOW, ahead of the rest of the path renames, for one reason: the vms lead is about to wire a monitoring dashboard to these series. Renaming a metric after a dashboard points at it breaks continuity and silently leaves a dead panel. Renaming it before costs nothing, so it goes first rather than at the cutover. Nine series renamed, all declared in FleetMetrics. Two real defects found while doing it: - FleetApp had "bridged_auth_failures_total" written as a LITERAL instead of using FleetMetrics.AUTH_FAILURES -- a second hand-written copy of a name, which is how these drift. It now uses the constant, so there is one source for that name again. - Nothing guarded the prefix. One test does assert a wire name (FleetAppAuthTest checks the real /metrics body for fleet_sessions), which is good, but it covers one series out of nine. The literal above was covered by nothing at all. So this adds MetricNamesTest, which reads the constants reflectively rather than listing them -- a test that lists the nine names is itself a second hand-written copy, and would pass while a tenth went unchecked. It asserts its own denominator too: "no name starts with bridged_" is true of an empty set, so a sweep that found nothing would pass loudly. Asserting the count of 9 makes a broken sweep fail instead. Also renamed three herdr contract-test workspace labels, __bridged_* -> __fleet_*. Those are throwaway workspaces created by ensureWorkspace, not metrics, but they are the same word. Verified: mvn clean install green, 52 classes, 881 tests, 0 failures. Test count is up by 3 -- the new prefix, denominator and uniqueness checks. No bridged_ string remains anywhere outside wiki/. --- .../dev/ltms/fleet/metrics/FleetMetrics.java | 22 ++--- .../java/dev/ltms/fleet/rest/FleetApp.java | 3 +- .../fleet/herdr/AgentControlContractTest.java | 2 +- .../fleet/herdr/PaneLocatorContractTest.java | 2 +- .../herdr/WorkspacePlacementContractTest.java | 2 +- .../ltms/fleet/metrics/MetricNamesTest.java | 87 +++++++++++++++++++ .../dev/ltms/fleet/metrics/MetricsTest.java | 40 ++++----- .../dev/ltms/fleet/rest/FleetAppAuthTest.java | 2 +- docs/CB-402-OpenCode-Adapter.md | 4 +- docs/CB-5xx-Hardening.md | 18 ++-- docs/M4-Fleet-Health.md | 16 ++-- 11 files changed, 143 insertions(+), 55 deletions(-) create mode 100644 bridged/src/test/java/dev/ltms/fleet/metrics/MetricNamesTest.java diff --git a/bridged/src/main/java/dev/ltms/fleet/metrics/FleetMetrics.java b/bridged/src/main/java/dev/ltms/fleet/metrics/FleetMetrics.java index f6ad8f4..3961755 100644 --- a/bridged/src/main/java/dev/ltms/fleet/metrics/FleetMetrics.java +++ b/bridged/src/main/java/dev/ltms/fleet/metrics/FleetMetrics.java @@ -13,31 +13,31 @@ import java.util.Map; * *

The set is deliberately small: each series maps to a failure mode this project has actually * hit, not to whatever was easy to count. The two worth watching in practice are - * {@code bridged_sends_total{outcome="completion_fallback"}} — a rising share means turn detection + * {@code fleet_sends_total{outcome="completion_fallback"}} — a rising share means turn detection * is degrading, the CB-115/116/118 failure family — and - * {@code bridged_push_nudges_total{outcome="exhausted"}}, which means the primary stopped draining + * {@code fleet_push_nudges_total{outcome="exhausted"}}, which means the primary stopped draining * its inbox and CB-307's active push gave up. */ public final class FleetMetrics { /** Counter: delegated sends by terminal outcome. */ - public static final String SENDS = "bridged_sends_total"; + public static final String SENDS = "fleet_sends_total"; /** Counter: worker replies by the path that carried them (rendezvous vs stranded-to-inbox). */ - public static final String REPLIES = "bridged_replies_total"; + public static final String REPLIES = "fleet_replies_total"; /** Counter: push-loop nudges to the primary, by outcome. */ - public static final String PUSH_NUDGES = "bridged_push_nudges_total"; + public static final String PUSH_NUDGES = "fleet_push_nudges_total"; /** Counter: idle-lead heartbeat nudges to the lead, by outcome (CB-551). */ - public static final String HEARTBEAT_NUDGES = "bridged_lead_heartbeat_nudges_total"; + public static final String HEARTBEAT_NUDGES = "fleet_lead_heartbeat_nudges_total"; /** Counter: spawn attempts by peer kind and outcome. */ - public static final String SPAWNS = "bridged_spawns_total"; + public static final String SPAWNS = "fleet_spawns_total"; /** Counter: herdr socket calls by method and outcome. */ - public static final String HERDR_CALLS = "bridged_herdr_calls_total"; + public static final String HERDR_CALLS = "fleet_herdr_calls_total"; /** Counter: rejected requests by reason (CB-501). */ - public static final String AUTH_FAILURES = "bridged_auth_failures_total"; + public static final String AUTH_FAILURES = "fleet_auth_failures_total"; /** Gauge: session census by lifecycle state. */ - public static final String SESSIONS = "bridged_sessions"; + public static final String SESSIONS = "fleet_sessions"; /** Gauge: undrained replies held per target. */ - public static final String INBOX_DEPTH = "bridged_inbox_depth"; + public static final String INBOX_DEPTH = "fleet_inbox_depth"; private FleetMetrics() { } diff --git a/bridged/src/main/java/dev/ltms/fleet/rest/FleetApp.java b/bridged/src/main/java/dev/ltms/fleet/rest/FleetApp.java index ba78fc8..04f1b49 100644 --- a/bridged/src/main/java/dev/ltms/fleet/rest/FleetApp.java +++ b/bridged/src/main/java/dev/ltms/fleet/rest/FleetApp.java @@ -7,6 +7,7 @@ import dev.ltms.fleet.auth.Authz; import dev.ltms.fleet.auth.CallerResolver; import dev.ltms.fleet.auth.Principal; import dev.ltms.fleet.guard.GuardException; +import dev.ltms.fleet.metrics.FleetMetrics; import dev.ltms.fleet.metrics.Metrics; import dev.ltms.fleet.herdr.Agent; import dev.ltms.fleet.herdr.HerdrClient; @@ -166,7 +167,7 @@ public final class FleetApp { private void countAuthFailure(String reason) { if (metrics != null) { - metrics.inc("bridged_auth_failures_total", "reason", reason); + metrics.inc(FleetMetrics.AUTH_FAILURES, "reason", reason); } } diff --git a/bridged/src/test/java/dev/ltms/fleet/herdr/AgentControlContractTest.java b/bridged/src/test/java/dev/ltms/fleet/herdr/AgentControlContractTest.java index 5ced79c..957fd30 100644 --- a/bridged/src/test/java/dev/ltms/fleet/herdr/AgentControlContractTest.java +++ b/bridged/src/test/java/dev/ltms/fleet/herdr/AgentControlContractTest.java @@ -31,7 +31,7 @@ class AgentControlContractTest { assumeTrue(!noSocket(), "no herdr socket — skipping"); try (UnixSocketHerdrClient herdr = UnixSocketHerdrClient.connect()) { WorkspaceControl spaces = new WorkspaceControl(herdr); - Workspace space = spaces.ensureWorkspace("__bridged_env_contract__"); + Workspace space = spaces.ensureWorkspace("__fleet_env_contract__"); Tab.Created tab = spaces.createTab(space.workspaceId(), null, Map.of("ANTHROPIC_BASE_URL", "http://gx00.gw:8000")); try { diff --git a/bridged/src/test/java/dev/ltms/fleet/herdr/PaneLocatorContractTest.java b/bridged/src/test/java/dev/ltms/fleet/herdr/PaneLocatorContractTest.java index 1ffcd1a..0a8ea4d 100644 --- a/bridged/src/test/java/dev/ltms/fleet/herdr/PaneLocatorContractTest.java +++ b/bridged/src/test/java/dev/ltms/fleet/herdr/PaneLocatorContractTest.java @@ -25,7 +25,7 @@ class PaneLocatorContractTest { assumeTrue(Files.exists(UnixSocketHerdrClient.defaultSocketPath()), "no herdr socket — skipping"); try (UnixSocketHerdrClient herdr = UnixSocketHerdrClient.connect()) { WorkspaceControl spaces = new WorkspaceControl(herdr); - Workspace space = spaces.ensureWorkspace("__bridged_pid_contract__"); + Workspace space = spaces.ensureWorkspace("__fleet_pid_contract__"); Tab.Created tab = spaces.createTab(space.workspaceId(), null, Map.of()); try { JsonNode info = herdr.call("pane.process_info", Map.of("pane_id", tab.rootPaneId())) diff --git a/bridged/src/test/java/dev/ltms/fleet/herdr/WorkspacePlacementContractTest.java b/bridged/src/test/java/dev/ltms/fleet/herdr/WorkspacePlacementContractTest.java index 40d46e9..82006d5 100644 --- a/bridged/src/test/java/dev/ltms/fleet/herdr/WorkspacePlacementContractTest.java +++ b/bridged/src/test/java/dev/ltms/fleet/herdr/WorkspacePlacementContractTest.java @@ -22,7 +22,7 @@ import static org.junit.jupiter.api.Assumptions.assumeTrue; @Tag("contract") class WorkspacePlacementContractTest { - private static final String LABEL = "__bridged_contract__"; + private static final String LABEL = "__fleet_contract__"; private boolean noSocket() { return !Files.exists(UnixSocketHerdrClient.defaultSocketPath()); diff --git a/bridged/src/test/java/dev/ltms/fleet/metrics/MetricNamesTest.java b/bridged/src/test/java/dev/ltms/fleet/metrics/MetricNamesTest.java new file mode 100644 index 0000000..28d4e5a --- /dev/null +++ b/bridged/src/test/java/dev/ltms/fleet/metrics/MetricNamesTest.java @@ -0,0 +1,87 @@ +package dev.ltms.fleet.metrics; + +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * CB-632: every exported series is named {@code fleet_*}, and nothing still says {@code bridged_}. + * + *

Why this test exists. The rename from {@code bridged_} to {@code fleet_} had nothing watching + * it. {@link FleetMetrics} holds the names as constants, but one caller had written + * {@code "bridged_auth_failures_total"} as a literal instead of using {@link + * FleetMetrics#AUTH_FAILURES} — a second hand-written copy of a name, which is how these drift. A + * rename that updated the constants and missed that literal would have shipped a daemon exporting + * eight series under one prefix and one under another, and no test would have failed. + * + *

The check is reflective on purpose. A test that lists the nine names is itself a second + * hand-written copy, so it would pass while a tenth constant added later went unchecked. Reading + * the fields means a new series is covered the moment it is declared. + * + *

It also asserts its own denominator. "No name starts with bridged_" is true of an empty set, + * so a reflective sweep that silently found nothing would pass loudly. Asserting the count means a + * broken sweep fails instead of reporting success. + */ +class MetricNamesTest { + + /** The number of series {@link FleetMetrics} declares. Update deliberately when adding one. */ + private static final int EXPECTED_SERIES = 9; + + private static List nameConstants() { + List out = new ArrayList<>(); + for (Field f : FleetMetrics.class.getDeclaredFields()) { + int m = f.getModifiers(); + if (Modifier.isPublic(m) && Modifier.isStatic(m) && Modifier.isFinal(m) + && f.getType() == String.class) { + out.add(f); + } + } + return out; + } + + private static String valueOf(Field f) { + try { + return (String) f.get(null); + } catch (IllegalAccessException e) { + throw new AssertionError("cannot read " + f.getName(), e); + } + } + + @Test + void theSweepActuallyFoundTheSeriesItClaimsToCheck() { + assertEquals(EXPECTED_SERIES, nameConstants().size(), + "this test reads FleetMetrics' name constants reflectively; if the count moved, " + + "either a series was added (update EXPECTED_SERIES) or the sweep broke " + + "and every assertion below is now vacuously true"); + } + + @Test + void everyDeclaredSeriesUsesTheFleetPrefix() { + for (Field f : nameConstants()) { + String name = valueOf(f); + assertTrue(name.startsWith("fleet_"), + "FleetMetrics." + f.getName() + " is \"" + name + "\" — every exported series " + + "must be named fleet_* since CB-632"); + assertFalse(name.contains("bridged"), + "FleetMetrics." + f.getName() + " still says bridged: \"" + name + "\""); + } + } + + /** + * The names must also be unique. Two constants sharing a value would collide on the wire, and + * a copy-paste when adding a series is exactly how that happens. + */ + @Test + void noTwoSeriesShareAName() { + List names = nameConstants().stream().map(MetricNamesTest::valueOf).toList(); + assertEquals(names.size(), names.stream().distinct().count(), + "two FleetMetrics constants hold the same series name: " + names); + } +} diff --git a/bridged/src/test/java/dev/ltms/fleet/metrics/MetricsTest.java b/bridged/src/test/java/dev/ltms/fleet/metrics/MetricsTest.java index c2f062b..e9a4fc3 100644 --- a/bridged/src/test/java/dev/ltms/fleet/metrics/MetricsTest.java +++ b/bridged/src/test/java/dev/ltms/fleet/metrics/MetricsTest.java @@ -13,29 +13,29 @@ class MetricsTest { @Test void countersAccumulatePerLabelSet() { Metrics m = new Metrics(); - m.inc("bridged_sends_total", "outcome", "replied"); - m.inc("bridged_sends_total", "outcome", "replied"); - m.inc("bridged_sends_total", "outcome", "timeout"); + m.inc("fleet_sends_total", "outcome", "replied"); + m.inc("fleet_sends_total", "outcome", "replied"); + m.inc("fleet_sends_total", "outcome", "timeout"); - assertEquals(2, m.count("bridged_sends_total", "outcome", "replied")); - assertEquals(1, m.count("bridged_sends_total", "outcome", "timeout")); - assertEquals(0, m.count("bridged_sends_total", "outcome", "failed"), + assertEquals(2, m.count("fleet_sends_total", "outcome", "replied")); + assertEquals(1, m.count("fleet_sends_total", "outcome", "timeout")); + assertEquals(0, m.count("fleet_sends_total", "outcome", "failed"), "an untouched series reads as zero, not an error"); } @Test void rendersHelpAndTypeOncePerFamily() { Metrics m = new Metrics(); - m.describe("bridged_sends_total", "counter", "Delegated sends by outcome."); - m.inc("bridged_sends_total", "outcome", "replied"); - m.inc("bridged_sends_total", "outcome", "timeout"); + m.describe("fleet_sends_total", "counter", "Delegated sends by outcome."); + m.inc("fleet_sends_total", "outcome", "replied"); + m.inc("fleet_sends_total", "outcome", "timeout"); String out = m.render(); - assertEquals(1, countOccurrences(out, "# HELP bridged_sends_total"), + assertEquals(1, countOccurrences(out, "# HELP fleet_sends_total"), "HELP is per family, not per series"); - assertEquals(1, countOccurrences(out, "# TYPE bridged_sends_total counter")); - assertTrue(out.contains("bridged_sends_total{outcome=\"replied\"} 1")); - assertTrue(out.contains("bridged_sends_total{outcome=\"timeout\"} 1")); + assertEquals(1, countOccurrences(out, "# TYPE fleet_sends_total counter")); + assertTrue(out.contains("fleet_sends_total{outcome=\"replied\"} 1")); + assertTrue(out.contains("fleet_sends_total{outcome=\"timeout\"} 1")); } @Test @@ -53,11 +53,11 @@ class MetricsTest { void gaugesAreEvaluatedAtScrapeTimeNotRegistrationTime() { Metrics m = new Metrics(); int[] live = {1}; - m.gauge("bridged_sessions", () -> live[0], "state", "ready"); + m.gauge("fleet_sessions", () -> live[0], "state", "ready"); - assertTrue(m.render().contains("bridged_sessions{state=\"ready\"} 1")); + assertTrue(m.render().contains("fleet_sessions{state=\"ready\"} 1")); live[0] = 5; - assertTrue(m.render().contains("bridged_sessions{state=\"ready\"} 5"), + assertTrue(m.render().contains("fleet_sessions{state=\"ready\"} 5"), "the gauge must read current state on every scrape"); } @@ -78,15 +78,15 @@ class MetricsTest { void collectorsDiscoverTheirLabelSetPerScrape() { Metrics m = new Metrics(); Map depths = new LinkedHashMap<>(); - m.collector("bridged_inbox_depth", "target", () -> depths); + m.collector("fleet_inbox_depth", "target", () -> depths); - assertFalse(m.render().contains("bridged_inbox_depth"), "no targets yet ⇒ no series"); + assertFalse(m.render().contains("fleet_inbox_depth"), "no targets yet ⇒ no series"); depths.put("term_a", 2); depths.put("term_b", 0); String out = m.render(); - assertTrue(out.contains("bridged_inbox_depth{target=\"term_a\"} 2")); - assertTrue(out.contains("bridged_inbox_depth{target=\"term_b\"} 0")); + assertTrue(out.contains("fleet_inbox_depth{target=\"term_a\"} 2")); + assertTrue(out.contains("fleet_inbox_depth{target=\"term_b\"} 0")); } @Test diff --git a/bridged/src/test/java/dev/ltms/fleet/rest/FleetAppAuthTest.java b/bridged/src/test/java/dev/ltms/fleet/rest/FleetAppAuthTest.java index 33c9479..881db6f 100644 --- a/bridged/src/test/java/dev/ltms/fleet/rest/FleetAppAuthTest.java +++ b/bridged/src/test/java/dev/ltms/fleet/rest/FleetAppAuthTest.java @@ -184,7 +184,7 @@ class FleetAppAuthTest { HttpResponse ok = send(port, "GET", "/metrics", null, "Bearer s3cret"); assertEquals(200, ok.statusCode()); assertTrue(ok.headers().firstValue("Content-Type").orElse("").startsWith("text/plain")); - assertTrue(ok.body().contains("bridged_sessions{state=\"ready\"}"), + assertTrue(ok.body().contains("fleet_sessions{state=\"ready\"}"), "the session census gauge is exported even when empty"); } diff --git a/docs/CB-402-OpenCode-Adapter.md b/docs/CB-402-OpenCode-Adapter.md index c1de564..5fe59f0 100644 --- a/docs/CB-402-OpenCode-Adapter.md +++ b/docs/CB-402-OpenCode-Adapter.md @@ -301,5 +301,5 @@ identity (loopback peer PID → herdr pane) classified an **opencode** process a opencode-specific handling — confirming the identity model is peer-kind-agnostic, which is exactly what CB-308 needs when it stretches the roster across hosts. -CB-502 counters for the same run: `bridged_sends_total{outcome="replied"} 1`, -`bridged_replies_total{path="rendezvous"} 1`, `bridged_inbox_depth{...} 0`. +CB-502 counters for the same run: `fleet_sends_total{outcome="replied"} 1`, +`fleet_replies_total{path="rendezvous"} 1`, `fleet_inbox_depth{...} 0`. diff --git a/docs/CB-5xx-Hardening.md b/docs/CB-5xx-Hardening.md index ada5f87..cda5a75 100644 --- a/docs/CB-5xx-Hardening.md +++ b/docs/CB-5xx-Hardening.md @@ -188,15 +188,15 @@ Deliberately small; every one maps to a failure mode we have actually hit. | Metric | Type | Why it exists | |---|---|---| -| `bridged_sends_total{outcome}` | counter | outcome ∈ replied\|completion_fallback\|timeout\|failed — the completion-fallback rate is the health signal for turn detection (CB-115/116/118) | -| `bridged_send_duration_seconds` | histogram | delegated turn latency | -| `bridged_replies_total{path}` | counter | path ∈ rendezvous\|inbox — how often a reply strands (CB-307's whole reason to exist) | -| `bridged_inbox_depth{target}` | gauge | undrained replies; steady-state should be 0 | -| `bridged_push_nudges_total{outcome}` | counter | outcome ∈ delivered\|exhausted — a rising `exhausted` means the primary is not draining | -| `bridged_spawns_total{kind,outcome}` | counter | outcome ∈ ready\|timeout\|guard_rejected; per peer kind (CB-402) | -| `bridged_sessions{state}` | gauge | SPAWNING/READY/BUSY/DONE census | -| `bridged_herdr_calls_total{method,outcome}` | counter | socket health — the dependency everything rests on | -| `bridged_auth_failures_total{reason}` | counter | only meaningful once CB-501 lands; catches misconfigured workers | +| `fleet_sends_total{outcome}` | counter | outcome ∈ replied\|completion_fallback\|timeout\|failed — the completion-fallback rate is the health signal for turn detection (CB-115/116/118) | +| `fleet_send_duration_seconds` | histogram | delegated turn latency | +| `fleet_replies_total{path}` | counter | path ∈ rendezvous\|inbox — how often a reply strands (CB-307's whole reason to exist) | +| `fleet_inbox_depth{target}` | gauge | undrained replies; steady-state should be 0 | +| `fleet_push_nudges_total{outcome}` | counter | outcome ∈ delivered\|exhausted — a rising `exhausted` means the primary is not draining | +| `fleet_spawns_total{kind,outcome}` | counter | outcome ∈ ready\|timeout\|guard_rejected; per peer kind (CB-402) | +| `fleet_sessions{state}` | gauge | SPAWNING/READY/BUSY/DONE census | +| `fleet_herdr_calls_total{method,outcome}` | counter | socket health — the dependency everything rests on | +| `fleet_auth_failures_total{reason}` | counter | only meaningful once CB-501 lands; catches misconfigured workers | --- diff --git a/docs/M4-Fleet-Health.md b/docs/M4-Fleet-Health.md index dd9da54..ff1343b 100644 --- a/docs/M4-Fleet-Health.md +++ b/docs/M4-Fleet-Health.md @@ -687,14 +687,14 @@ The sink response body is ignored. A webhook cannot direct recovery. n8n remains M4 adds bounded-label series: ```text -bridged_health_incidents{scope,state,severity} -bridged_health_incidents_total{event} -bridged_health_notifications_total{event,outcome} -bridged_health_notification_queue_depth -bridged_health_notification_last_success_seconds -bridged_health_notification_capability{mode,status} -bridged_lead_health{lead,state} -bridged_lead_assigned_incidents{lead} +fleet_health_incidents{scope,state,severity} +fleet_health_incidents_total{event} +fleet_health_notifications_total{event,outcome} +fleet_health_notification_queue_depth +fleet_health_notification_last_success_seconds +fleet_health_notification_capability{mode,status} +fleet_lead_health{lead,state} +fleet_lead_assigned_incidents{lead} ``` Metric labels never include terminal ids, incident ids, URLs, or error text.