|
|
@@ -705,3 +705,154 @@ Pushed: 3 commits on master mirroring M0–M5's pattern:
|
|
|
green runs, 11/11 checks each, 600-alert dupe storm
|
|
|
confirms free-for-dupes, 0 rate-limit hits)
|
|
|
3. M6(3/3): this PROMPT bump + README + SPEC §23.
|
|
|
+
|
|
|
+**2026-06-14 — M6.5 shipped (Router-level dedupe collapse)**
|
|
|
+
|
|
|
+The natural follow-on to M6, falling out of the M6 "What's
|
|
|
+NOT" list. M6 gave us "the recipient sees ×N" (100 messages,
|
|
|
+the last one tagged). M6.5 gives us "the recipient sees 1
|
|
|
+message" (collapsed by the router, not 100).
|
|
|
+
|
|
|
+New code (~700 LoC Go + 1 new test binary + 1 new env var):
|
|
|
+
|
|
|
+- `internal/dedupe/collapser.go` (NEW, ~200 LoC): the
|
|
|
+ Collapser. Public API: `NewCollapser(flushMs, onFlush)`,
|
|
|
+ `Observe(sourceID, dedupeKey, alert) Decision`,
|
|
|
+ `Run(ctx)`, `FlushAll()`. The Decision is one of:
|
|
|
+ * `Passthrough` — empty dedupe_key; deliver immediately
|
|
|
+ * `CollapseNew` — first arrival of (source, key);
|
|
|
+ caller does the expensive work (resolve recipients)
|
|
|
+ and caches it
|
|
|
+ * `CollapseDupe` — subsequent arrival within the
|
|
|
+ window; stored alert's dedupe_count already updated
|
|
|
+ Per-source isolation enforced by keying on
|
|
|
+ `(source_id, dedupe_key)`, not on `dedupe_key` alone.
|
|
|
+
|
|
|
+- `internal/dedupe/collapser_test.go` (NEW, ~250 LoC, 7
|
|
|
+ tests, all green): passthrough-on-empty-key,
|
|
|
+ single-collapse-100-alerts, per-source-isolation,
|
|
|
+ max-wait-re-flush, empty-key-doesn't-block-real-key,
|
|
|
+ 8-goroutine-concurrent-same-key-storm, FlushAll-drain.
|
|
|
+
|
|
|
+- `cmd/routerd/collapse.go` (NEW, ~250 LoC): wires the
|
|
|
+ Collapser into routerd. The router holds a tiny
|
|
|
+ `fanoutState` (source+key → cached resolved targets) so
|
|
|
+ duplicate alerts skip the DB-bound `ResolveTargets`
|
|
|
+ call. The flush callback reads the cached targets and
|
|
|
+ publishes one delivery per (target, channel) with the
|
|
|
+ final dedupe_count.
|
|
|
+
|
|
|
+- `cmd/routerd/main.go` (refactored): `handleOne` routes
|
|
|
+ through `observeAndFanout`. The Collapser's Run loop
|
|
|
+ starts in a goroutine in `consume()`. `FlushAll()` is
|
|
|
+ called on graceful shutdown so the last few pending
|
|
|
+ collapses still get delivered. Config now loaded via
|
|
|
+ `config.LoadRouterd()` (was `LoadCommon`).
|
|
|
+
|
|
|
+- `internal/config/config.go`: new `Routerd` struct with
|
|
|
+ `DedupeFlushMs` field. Env var
|
|
|
+ `BA_ROUTERD_DEDUPE_FLUSH_MS`, default 2000ms.
|
|
|
+
|
|
|
+- `testfakes/tailcount/main.go` (NEW, ~70 LoC): a tiny Go
|
|
|
+ helper that subscribes to the M5 tail WS and counts
|
|
|
+ events, filtering on a substring. Used only by the
|
|
|
+ smoke; no production code touches it.
|
|
|
+
|
|
|
+- `loadgen/cmd/ws/main.go` (small change): `makeAlert`
|
|
|
+ injects the `--dedupe-key` into the title as
|
|
|
+ `LG M5 burst <key> #N` when forced. The compact tail
|
|
|
+ event includes title but NOT dedupe_key (by design),
|
|
|
+ so the smoke needs to filter on something the operator
|
|
|
+ can see.
|
|
|
+
|
|
|
+- `docker-compose.yml` + `.env.example`:
|
|
|
+ `BA_ROUTERD_DEDUPE_FLUSH_MS=2000` documented.
|
|
|
+
|
|
|
+Two real bugs caught + fixed in M6.5 verification:
|
|
|
+
|
|
|
+1. **Tail filter was on `dedupe_key` but the compact tail
|
|
|
+ event doesn't include it.** The tail event JSON is just
|
|
|
+ `{alert_id, company_id, source_id, severity, title,
|
|
|
+ received_at, transport, dedupe_count}` — no
|
|
|
+ `dedupe_key` field by design (M5 spec keeps the event
|
|
|
+ small). Fixed by making the loadgen put the key in the
|
|
|
+ title, and the smoke filters on the title prefix
|
|
|
+ instead. The dedupe_key field is preserved in the
|
|
|
+ payload (for the deliverers) but not in the tail
|
|
|
+ event.
|
|
|
+
|
|
|
+2. **Globex had no recipients in the dev DB.** The dev
|
|
|
+ schema only seeds acme; the m6 smoke never sent
|
|
|
+ globex alerts. Step 5's per-source-isolation test
|
|
|
+ surfaced this ("no recipients resolved" for grafana).
|
|
|
+ The smoke now auto-seeds the globex company, source,
|
|
|
+ individual, and subscription at the top (idempotent
|
|
|
+ INSERT ... ON CONFLICT DO NOTHING). This is a smoke
|
|
|
+ helper, not a migration — the real seed lives in the
|
|
|
+ M2 migration files. The smoke needs globex to exist
|
|
|
+ for Step 5; the production code doesn't care.
|
|
|
+
|
|
|
+3. **Step 3 (tail + collapse) needed the burst to fit
|
|
|
+ inside one flush window.** First version sent 25
|
|
|
+ alerts at 10/s = 2.5s, which crossed the 2s flush
|
|
|
+ boundary and produced 2 messages for one burst. Bumped
|
|
|
+ the rate to 50/s = 0.5s, so the whole burst is inside
|
|
|
+ a single window. The test still proves the headline
|
|
|
+ behavior (1 message + tail sees the storm), but the
|
|
|
+ timing has to be tuned to the configured flush window.
|
|
|
+
|
|
|
+Design choice recap (per the M6.5 scope conversation):
|
|
|
+
|
|
|
+- **Router-side, max-wait debounce.** Ingest publishes
|
|
|
+ unconditionally (so alerts hit durable storage + tail).
|
|
|
+ The collapse happens *between* NATS and the deliverers,
|
|
|
+ i.e. in the router.
|
|
|
+- **2s default flush window.** Long enough to coalesce
|
|
|
+ most bursts; short enough that critical alerts feel
|
|
|
+ instant. Continuous stream re-flushes every window.
|
|
|
+- **Per-source isolation.** Same key from two sources is
|
|
|
+ two separate collapses. We considered global collapse
|
|
|
+ but it has bad blast-radius properties (one noisy source
|
|
|
+ could shadow another). Per-source-only ships.
|
|
|
+- **Cached resolved targets.** The first arrival does
|
|
|
+ the DB-bound `ResolveTargets` call. Subsequent dupes
|
|
|
+ update the dedupe_count on the stored alert and
|
|
|
+ return. Net: 100-alert burst produces 1 DB call, not
|
|
|
+ 100. (This is the biggest perf win — without it,
|
|
|
+ the 100-alert burst would have done 100 DB queries,
|
|
|
+ which is the original M2–M5 cost we were trying to
|
|
|
+ avoid.)
|
|
|
+- **Trust the ingestd's count.** The router doesn't
|
|
|
+ INCR itself; it takes the max of the stored and
|
|
|
+ incoming values. Ingestd is the canonical counter;
|
|
|
+ this avoids a second Redis round-trip per arrival.
|
|
|
+
|
|
|
+What's NOT in M6.5 (deferred):
|
|
|
+
|
|
|
+- **Active collapse metrics** — no Prometheus metric for
|
|
|
+ "how many collapses happened" or "how many alerts
|
|
|
+ were collapsed". The router logs them at info level
|
|
|
+ with `dedupe_count`, but we don't have a
|
|
|
+ `ba_routerd_collapse_total{source}` counter yet. M7
|
|
|
+ dashboard work.
|
|
|
+- **Per-source override of `BA_ROUTERD_DEDUPE_FLUSH_MS`** —
|
|
|
+ a noisy source might want a longer window. The
|
|
|
+ migration added `sources.dedupe_flush_ms` but the M6.5
|
|
|
+ path uses the env default for all sources.
|
|
|
+- **Collapse-across-sources** — currently per-source.
|
|
|
+ Could be a config flag but the blast-radius worries
|
|
|
+ are real.
|
|
|
+- **Tombstone on collapse** — the collapsed delivery
|
|
|
+ currently looks like a normal alert. A future M7 could
|
|
|
+ add a `collapsed_count: N` field to the payload so
|
|
|
+ deliverers can render "5 messages collapsed" in
|
|
|
+ addition to the `(×N)` suffix.
|
|
|
+
|
|
|
+Pushed: 3 commits on master mirroring M0–M6's pattern:
|
|
|
+1. M6.5(1/3): code (Collapser + collapse.go wiring +
|
|
|
+ config + compose + 7 new collapser tests)
|
|
|
+2. M6.5(2/3): verified (M6.5_VERIFICATION.md +
|
|
|
+ scripts/m6.5_smoke.sh + M6.5_SMOKE_LOG.md, 3
|
|
|
+ consecutive green runs, 9/9 checks each, 100-alert
|
|
|
+ burst → 1 message confirmed)
|
|
|
+3. M6.5(3/3): this PROMPT bump + README + SPEC §23.
|