observability: Alertmanager's only receiver was named "null" - #1112
Open
mdheller wants to merge 3 commits into
Open
observability: Alertmanager's only receiver was named "null"#1112mdheller wants to merge 3 commits into
mdheller wants to merge 3 commits into
Conversation
Every alert in this estate was routed to a receiver that discards, and nothing reported a problem — from inside Alertmanager, "delivered to null" and "delivered" are the same code path. alertmanager_notifications_ total rose, dashboards stayed green, and the entire alerting corpus was decorative. At the time this was found Alertmanager was holding 20 firing alerts nobody had ever seen, including four KubeJobFailed and a KubeletDown. Adds a ~250-line stdlib-Python webhook sink that turns each notification into an EvidenceReceipt (contracts/EvidenceReceipt.v0.1.json) on stdout, collected by fluentbit-gke into Google Cloud Logging. No credential and no external endpoint, so it could be wired without provisioning anything; human paging is a separate, credentialed decision documented in the README. Delivery is proved continuously rather than assumed. The chart's Watchdog alert fires forever by design, so routing it here with a 1h repeat gives the sink a permanent heartbeat: alert_sink_notifications_total must always rise, and AlertDeliveryDead fires when it stops. Silence becomes the alarm instead of the default. Two mechanisms ship together. The Helm values set the estate-wide default route; an AlertmanagerConfig CR covers namespace=observability without waiting on a chart sync, which is where every guard in this wave fires. Proof it fires — observed, not asserted: Wave1DeliveryProof active receivers=[observability/alert-sink/alert-sink] 05:29:44 evr-alert-delivered-ee132fdb... accepted firing 05:34:44 evr-alert-delivered-ae71c3a8... succeeded resolved Both receipts validate against the contract. A malformed payload was also exercised: HTTP 400, errors_total 0 -> 1, and a status="failed" receipt rather than silent absorption. Note recorded in the README: Loki is deployed, Running, holds a 10Gi PVC, and has zero ingested bytes. Nothing has ever written to it. The logging story deliberately targets Cloud Logging instead.
This was referenced Jul 30, 2026
…eceiver Every alert in this PrometheusRule emitted with no namespace label, so the AlertmanagerConfig sub-route (operator-scoped OnNamespace) would not match them: the alert reporting 'delivery is dead' would itself be delivered to null. The fix shipping with that defect inside it. Caught by measuring rather than assuming. Live check after deploying the sub-route: 19 firing alerts still routed to null, 0 to the sink, because none carry namespace=observability. Watchdog in particular carries no namespace label at all, so the heartbeat this rule depends on only starts once the Helm DEFAULT route lands. Documented rather than papered over.
Member
Author
Correction after live verification — the fix contained the defect it fixesPushed as After deploying the sink and the Nineteen alerts still going nowhere. Two distinct causes: 1. The operator scopes an 2. Worse — the alerts in this PR's own PrometheusRule had no So Both now carry an explicit What is and is not proven
The narrow lesson: an |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds an in-cluster Alertmanager webhook “alert-sink” receiver that emits structured EvidenceReceipt JSON lines to stdout (for Cloud Logging), plus Prometheus meta-monitoring to ensure alert delivery can’t silently fail.
Changes:
- Introduces a Python stdlib webhook sink service with
/alerts,/metrics,/healthz, and/recent. - Deploys Kubernetes resources (Deployment/Service/ServiceMonitor/PrometheusRule) and an Argo CD Application for alert-delivery.
- Updates Alertmanager Helm values to route the default receiver (and Watchdog heartbeat) to the sink.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| infra/k8s/alert-delivery/base/sink.py | Implements the webhook sink + receipts + metrics endpoints. |
| infra/k8s/alert-delivery/base/deployment.yaml | Deploys the sink as a pinned python image with probes and hardened security context. |
| infra/k8s/alert-delivery/base/service.yaml | Exposes the sink internally via ClusterIP service. |
| infra/k8s/alert-delivery/base/servicemonitor.yaml | Enables Prometheus scraping of the sink’s /metrics. |
| infra/k8s/alert-delivery/base/prometheusrule-delivery.yaml | Adds meta-alerting for delivery dead/sink down/rejections. |
| infra/k8s/alert-delivery/base/kustomization.yaml | Wires base resources + configmap generation for sink.py. |
| infra/k8s/alert-delivery/base/alertmanagerconfig.yaml | Adds namespace-scoped AlertmanagerConfig route/receiver for observability namespace. |
| infra/k8s/alert-delivery/README.md | Documents the delivery fix, guarantees, and operational runbook. |
| deploy/argocd/observability-services.yaml | Sets Alertmanager default route + Watchdog heartbeat to the sink. |
| deploy/argocd/alert-delivery.yaml | Adds Argo CD Application to deploy the alert-sink base. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+5
to
+9
| resources: | ||
| - deployment.yaml | ||
| - service.yaml | ||
| - servicemonitor.yaml | ||
| - prometheusrule-delivery.yaml |
Comment on lines
+87
to
+90
| def emit(obj: dict) -> None: | ||
| """One JSON object per line to stdout -> fluentbit-gke -> Cloud Logging.""" | ||
| sys.stdout.write(canonical(obj) + "\n") | ||
| sys.stdout.flush() |
Comment on lines
+59
to
+60
| summary: "alert-sink refused {{ $value }} malformed deliveries in 15m" | ||
| description: >- |
Comment on lines
+93
to
+99
| def receipt(action: str, status: str, subject_ref: str, body: dict, **extra) -> dict: | ||
| """An EvidenceReceipt.v0.1. Fields and enum values come from the contract.""" | ||
| h = digest(body) | ||
| r = { | ||
| "version": VERSION, | ||
| "receipt_id": "evr-%s-%s" % (action, h[:32]), | ||
| "created_at": utc_now(), |
Comment on lines
+172
to
+173
| class Handler(BaseHTTPRequestHandler): | ||
| protocol_version = "HTTP/1.1" |
Comment on lines
+186
to
+192
| def _send(self, code: int, obj: dict) -> None: | ||
| body = canonical(obj).encode("utf-8") | ||
| self.send_response(code) | ||
| self.send_header("Content-Type", "application/json") | ||
| self.send_header("Content-Length", str(len(body))) | ||
| self.end_headers() | ||
| self.wfile.write(body) |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (4)
infra/k8s/alert-delivery/base/sink.py:213
do_POST()accepts POSTs on any path, but this service is intended to be a webhook receiver (e.g./alerts). Allowing POST to arbitrary paths makes it easier for in-cluster callers to spam receipts/metrics and pollute Cloud Logging. Consider enforcingself.path == \"/alerts\"(or a small allowlist) and returning 404 for other POST paths.
def do_POST(self): # noqa: N802 - stdlib hook
length = int(self.headers.get("Content-Length") or 0)
infra/k8s/alert-delivery/base/sink.py:132
- Any
statusother than exactly\"firing\"is classified as\"succeeded\", including missing/unknown values. That can produce misleading receipts and severity/status metrics while still returning 200. Since Alertmanager webhook statuses are expected to befiringorresolved, validate explicitly and raise on unexpected values so the request is rejected (400) anderrors_totalreflects the bad payload.
status = str(a.get("status", "unknown"))
# firing -> the condition is live and was delivered here.
# resolved -> Alertmanager observed it clear.
ev_status = "accepted" if status == "firing" else "succeeded"
infra/k8s/alert-delivery/base/deployment.yaml:32
- With
readOnlyRootFilesystem: true, CPython may attempt to create__pycache__/.pycand emit noisy permission errors (and extra exception overhead) at startup/runtime. To keep logs clean and behavior consistent, setPYTHONDONTWRITEBYTECODE=1(or runpython3 -B) in this container.
image: python:3.12-slim@sha256:57cd7c3a7a273101a6485ba99423ee568157882804b1124b4dd04266317710de
command: ["python3", "/opt/sink/sink.py"]
infra/k8s/alert-delivery/base/prometheusrule-delivery.yaml:88
- The comment says this rule ‘catches it in 5m’, but the expression/window is
[3h]withfor: 15m, so it won’t alert in ~5 minutes. Update the comment to match the actual detection time, or adjust the range/forto match the stated 5m behavior.
# Guards the regression that started all of this: if Alertmanager is
# reconfigured back to a null-only receiver set, notifications stop and
# AlertDeliveryDead catches it within 3h. This catches it in 5m by
# watching Alertmanager's own delivery counter for the sink integration.
- alert: AlertmanagerNotNotifying
expr: |
increase(alertmanager_notifications_total{integration="webhook"}[3h]) == 0
or absent(alertmanager_notifications_total{integration="webhook"})
for: 15m
Comment on lines
+5
to
+9
| resources: | ||
| - deployment.yaml | ||
| - service.yaml | ||
| - servicemonitor.yaml | ||
| - prometheusrule-delivery.yaml |
Comment on lines
+87
to
+90
| def emit(obj: dict) -> None: | ||
| """One JSON object per line to stdout -> fluentbit-gke -> Cloud Logging.""" | ||
| sys.stdout.write(canonical(obj) + "\n") | ||
| sys.stdout.flush() |
…() atomic Two orphaned-control fixes on the null-receiver PR: 1. base/alertmanagerconfig.yaml was authored but never listed in the base kustomization resources:, so the Argo app that syncs infra/k8s/alert-delivery/base never applied it. The namespace-scoped receiver route shipped as a file nobody read. Wired in; kubectl kustomize now renders the AlertmanagerConfig CR. 2. emit() did an unguarded sys.stdout.write()+flush() from a ThreadingHTTPServer, so concurrent alerts could interleave two receipts into one unparseable JSONL line — silent corruption of the exact receipt stream this sink exists to guarantee. A dedicated _emit_lock (separate from the counter _lock; no reentrancy) makes the write+flush atomic. Adds a deterministic concurrency test (write-splitting stdout) that is red on the unguarded version and green with the lock.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (4)
infra/k8s/alert-delivery/base/sink.py:224
- This holds
_lockwhile performing response I/O in_send(); a slow client can block other request threads from updating counters/ring or serving/metrics. Copylist(_recent)under the lock, release the lock, then call_send()outside the critical section (similar to how_metrics()snapshots state).
if self.path.startswith("/recent"):
with _lock:
return self._send(200, {"recent": list(_recent)})
infra/k8s/alert-delivery/base/sink.py:143
- Any Alertmanager status other than exactly
\"firing\"is currently mapped to EvidenceReceipt status\"succeeded\", including\"unknown\"(or any unexpected value). That can produce misleading receipts for malformed/unexpected alert objects. Consider explicitly mapping onlyfiring -> acceptedandresolved -> succeeded, and treating any other status as a failure/denied condition (or emitting a distinct metrics label) so unexpected states remain loud.
status = str(a.get("status", "unknown"))
infra/k8s/alert-delivery/base/sink.py:147
- Any Alertmanager status other than exactly
\"firing\"is currently mapped to EvidenceReceipt status\"succeeded\", including\"unknown\"(or any unexpected value). That can produce misleading receipts for malformed/unexpected alert objects. Consider explicitly mapping onlyfiring -> acceptedandresolved -> succeeded, and treating any other status as a failure/denied condition (or emitting a distinct metrics label) so unexpected states remain loud.
ev_status = "accepted" if status == "firing" else "succeeded"
infra/k8s/alert-delivery/base/sink.py:294
- Prometheus text exposition requires escaping label values (e.g., backslash, double-quote, and newline). Since
CLUSTERcomes from the environment, an unexpected value containing\\or\"could break the/metricsendpoint. Add a small helper to escape label values per the Prometheus text format before interpolating them into metric lines.
'alert_sink_build_info{version="%s",cluster="%s"} 1' % (VERSION, CLUSTER),
|
|
||
|
|
||
| class Handler(BaseHTTPRequestHandler): | ||
| protocol_version = "HTTP/1.1" |
Comment on lines
+227
to
+242
| def do_POST(self): # noqa: N802 - stdlib hook | ||
| length = int(self.headers.get("Content-Length") or 0) | ||
| if length > MAX_BODY: | ||
| with _lock: | ||
| _counters["errors_total"] += 1 | ||
| emit( | ||
| receipt( | ||
| "alert-delivered", | ||
| "denied", | ||
| "alert/-/<oversize>", | ||
| {"content_length": length, "limit": MAX_BODY}, | ||
| ) | ||
| ) | ||
| return self._send(413, {"error": "body too large", "limit": MAX_BODY}) | ||
|
|
||
| raw = self.rfile.read(length) if length else b"" |
Member
Author
|
Merge-gate disposition (Copilot 3-channel + adversarial pass) FIXED in
Acknowledged, non-blocking (fair, low-severity): Verdict: MERGE-READY after the two fixes above. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
Alertmanager had exactly one receiver:
Every alert in this estate — SLO breaches,
KubeJobFailed, the PVC guard's data-loss warnings — was routed to a receiver that discards. Nothing reported a problem, because from inside Alertmanager "delivered to null" and "delivered" are the same code path.alertmanager_notifications_totalrose. Dashboards were green.At the moment this was found, Alertmanager was holding 20 firing alerts that no human had ever seen, including four
KubeJobFailedand aKubeletDown.This is the first link: every other control in this wave is decorative until it exists.
What this adds
A ~250-line stdlib-Python webhook sink. Each Alertmanager notification becomes an EvidenceReceipt (
contracts/EvidenceReceipt.v0.1.json) on stdout →fluentbit-gke→ Google Cloud Logging.Deliberately a destination needing no credential and no external endpoint, so it could be wired without provisioning anything. Human paging is a separate decision — options and costs are in the README, and nothing here pages a person.
Why it cannot silently stop working
kube-prometheus-stackships aWatchdogalert that fires permanently by design. Routing it here withrepeat_interval: 1hgives the sink a heartbeat forever, soalert_sink_notifications_totalmust always rise. When it stops,AlertDeliveryDeadfires.That inverts the failure mode: silence stops meaning "quiet night" and starts meaning "alarm".
Refusals are equally loud — a malformed payload increments
alert_sink_errors_total, emits astatus: "failed"receipt, and returns 400. Never absorbed.Bounds
MAX_BODY_BYTESdeniedreceiptRECENT_RINGmax_alertsProof it fires — observed, not asserted
Receipts emitted, firing then resolved:
Counters 1 → 2. Both validate against the contract (required fields,
statusin enum,versionconst).Malformed payload also exercised: HTTP 400,
errors_total0 → 1,status: "failed"receipt.The nine real findings from the companion rule-liveness guard were then delivered through this same path end-to-end.
Loki
Recorded in the README because it is the same defect in another costume: Loki is deployed,
Running, holds a 10Gi PVC, and has zero ingested bytes and zero label values. Nothing has ever written to it. The logging story here targets Cloud Logging instead. Not addressed in this PR.Two mechanisms, on purpose
AlertmanagerConfigCR coversnamespace=observabilitywithout waiting on a chart sync — which is where every guard in this wave fires.(2) is not a substitute for (1); alerts in
socioprophet,scm,servingneed the default route.Coordination
New paths (
infra/k8s/alert-delivery/**,deploy/argocd/alert-delivery.yaml) plus the Alertmanager block ofdeploy/argocd/observability-services.yaml. No overlap with #1091, #1105,images.yml,infra/k8s/zot/**,apps/health-twin/**,tools/,gitops-promote.yml, orvalidate-target-diagnostics.yml.kubectl kustomize,kubectl apply --dry-run=server, and the live end-to-end run above.