Skip to content

feat(gos): split Discipline B — retire alerting, decouple sample_p from countdown - #385

Merged
zzylol merged 1 commit into
feat/gos-unified-monitoringfrom
split/pr-retire-discipline-b-alerting
Jul 17, 2026
Merged

zzylol merged 1 commit into
feat/gos-unified-monitoringfrom
split/pr-retire-discipline-b-alerting

Conversation

@zzylol

@zzylol zzylol commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Coordinator-side counterpart to ASAPCollector PR #532 (split/pr-retire-discipline-b-alerting).

Discipline B's global-threshold alerting (Monitor::rebroadcast's CMY slack-countdown → Action::Alert → the control-plane violation sink) retires per ASAPCollector/docs/design-gos-unified-edge-telemetry.md §11: "Alerting and any other query-time decision is made entirely at the backend against [the reconstructed sketch state]; the edge no longer makes alerting decisions itself." A collector/edge must never be the thing that fires an alert.

The whole-sketch ε-floor sampling law (p_i = 1/(1+ε²·rate_i)) was always a pure per-edge function with no cross-edge coordination or global-estimate dependency, so decoupling it from the retired countdown is a genuine simplification, not a workaround:

  • Monitor drops known_value/departed_mass/alerted/tau/slack/gap entirely — just tracks each edge's last-reported rate.
  • on_report answers ONLY the reporting edge with a freshly computed grant (no more re-broadcast to every registered edge on every report — that fan-out existed only because slack sizing depended on the full edge set).
  • The retired "<2 edges ⇒ p=1" special case (needed only for the alert-fire safety proof under sampling noise) is gone: a lone high-rate edge is now sampled exactly like it would be in a larger edge set.

Removes alert.rs (AlertSink/global_threshold_violation) and the now-dead ViolationKind::GlobalThresholdCrossed. MonitorConfig.tau / Grant.LocalSlack stay on the wire (vestigial) for compatibility with the Go edge side, which still sends them.

Companion PR: ASAPCollector #532 — the edge side. Engine.Observe now reports its rate on its own periodic cadence (ReportEveryN observations), decoupled from any value/slack threshold.

Verified with a real cross-language run (Go e2edriver ↔ Rust monitor_coordinator_harness over live gRPC): the coordinator answers the edge's periodic rate report with a genuine computed sample_p grant.

Test plan

  • data_plane::monitor::coordinator unit tests rewritten for the simplified per-edge model (registration grants nothing, report grants only the reporting edge, single-edge sampling, epoch reset, etc.)
  • data_plane/tests/monitor_grpc.rs rewritten: dropped the two alert-firing tests, kept/added real gRPC round-trip tests for single-edge sampling, differentiated sample_p across edges, and report-scoped-to-reporter.
  • Repurposed monitor_coordinator_harness to poll for a real coordinated-sampling grant instead of waiting for a retired alert; updated deploy/mvp-multinode/scripts/monitor_e2e.sh accordingly.
  • Real cross-language e2e verified manually against the matching ASAPCollector worktree: MONITOR_GRANT edge=e2e-edge agg_id=1 sample_p=0.9975....
  • cargo build/cargo test -p data_plane clean. cargo test -p control_plane has one pre-existing unrelated failure (Duration not in scope in optimizer/cost/mod.rs) present on the base branch before this change — not touched here.

🤖 Generated with Claude Code

…om countdown

Discipline B's global-threshold alerting (Monitor::rebroadcast's CMY
slack-countdown -> Action::Alert -> the control-plane violation sink) retires
per ASAPCollector/docs/design-gos-unified-edge-telemetry.md §11: "Alerting and
any other query-time decision is made entirely at the backend against [the
reconstructed sketch state]; the edge no longer makes alerting decisions
itself." A collector/edge must never be the thing that fires an alert.

The whole-sketch ε-floor sampling law (p_i = 1/(1+ε²·rate_i)) was ALWAYS a
pure per-edge function with no cross-edge coordination or global-estimate
dependency, so decoupling it from the retired countdown is a genuine
simplification, not a workaround:
- Monitor drops known_value/departed_mass/alerted/tau/slack/gap entirely —
  just tracks each edge's last-reported rate.
- on_report answers ONLY the reporting edge with a freshly computed grant
  (no more re-broadcast to every registered edge on every report — that
  fan-out existed only because slack sizing depended on the full edge set).
- The retired "<2 edges => p=1" special case (needed only for the alert-fire
  safety proof under sampling noise) is gone: a lone high-rate edge is now
  sampled exactly like it would be in a larger edge set.

Removes alert.rs (AlertSink/global_threshold_violation) and the now-dead
ViolationKind::GlobalThresholdCrossed. MonitorConfig.tau / Grant.LocalSlack
stay on the wire (vestigial) for compatibility with the Go edge side, which
still sends them.

Companion PR: ASAPCollector split/pr-retire-discipline-b-alerting — the edge
side. Engine.Observe now reports its rate on its own periodic cadence
(ReportEveryN observations), decoupled from any value/slack threshold.

Verified with a real cross-language run (Go e2edriver <-> Rust
monitor_coordinator_harness over live gRPC): the coordinator answers the
edge's periodic rate report with a genuine computed sample_p grant.

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@zzylol
zzylol merged commit 39f2978 into feat/gos-unified-monitoring Jul 17, 2026
@zzylol
zzylol deleted the split/pr-retire-discipline-b-alerting branch July 17, 2026 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant