Skip to content

fix(cloudwatch): add write-free receivers and opt-in source-time admission - #6880

Open
cayo-aura wants to merge 2 commits into
keephq:mainfrom
cayo-aura:fix/cloudwatch-source-ordering
Open

cayo-aura wants to merge 2 commits into
keephq:mainfrom
cayo-aura:fix/cloudwatch-source-ordering

Conversation

@cayo-aura

@cayo-aura cayo-aura commented Oct 17, 2026 •

Copy link
Copy Markdown

Problem

CloudWatch scope validation can call PutMetricAlarm on an existing alarm and start a Logs Insights query. Externally managed SNS receivers should not need those side effects. Explicit polling flags are also lost on initial provider provisioning. Separately, late source transitions can replace a newer current state and reach workflows because ingestion time is used for current-alert selection.

Related to #6879. Submitted for maintainer review; production acceptance remains subject to the limitations below.

Changes

  • Remove mutating scope probes. Preserve IAM simulation denials; use bounded read-only probes and explicit unverified diagnostics when simulation is unavailable.
  • Add authentication.webhook_only: true: no AWS clients/scopes, querying or automatic webhook installation. Existing API mode remains the default.
  • Honor top-level pulling_enabled for JSON/YAML initial provisioning and updates, including strict boolean/string parsing.
  • Add experimental, default-off source-time admission via KEEP_SOURCE_EVENT_ORDERING_PROVIDERS=cloudwatch, currently PostgreSQL only. A per-tenant/fingerprint advisory lock protects admission through the existing enqueue boundary without adding tables, another receiver or an external ledger.
  • Preserve ingestion timestamps and archive rejected transitions without moving current state or running ingest-time enrichment/incident/workflow effects. Expose server-generated sourceEventOrdering.accepted and recoveryEligible for notification policy.
  • Protect the admitted fingerprint, persisted event ID and ordering context from extraction, mapping, workflow admission and retry enrichment overlays. Legacy events without an admission context retain their original enrichment behavior. Refuse provider identity conflicts before mutation.
  • Anchor only_on_change and severity_changed comparisons to the persisted triggering event and previous accepted source transition, excluding rejected history and avoiding ingestion-clock ordering. Rejected rows remain inspectable.
  • Add documentation and an isolated PostgreSQL CI job with no AWS credentials or external notification delivery.

Validation

  • 65 targeted tests passed, using real provider service/provisioning paths, SQLite and isolated PostgreSQL. Includes concurrent first arrivals, per-fingerprint lock lifetime/failure release, interleaved retries, equal-time conflicts, worker clock skew, real extraction/mapping rules, SQLite/PostgreSQL history comparisons, actual workflow admission and the scheduler retry boundary.
  • 27 existing workflow and extraction/mapping tests passed; 18 unrelated tests deselected.
  • Regression failures were observed before the fixes for mutating validation, lost polling flags, source ordering/clock skew, identity mutation and provider collisions. The review corrections reproduced five workflow failures before fixing them, plus an additional persisted-event-ID overlay failure before sharing the reserved-field policy across ingestion and dispatch.
  • Ruff, actionlint and diff-check passed. No AWS operations or external alert deliveries were performed.
  • PostgreSQL integration tests require KEEP_TEST_ORDERING_POSTGRES_URL pointing at a disposable database; they create/drop isolated schemas and skip when the explicit test DSN is absent.

Limitations and production acceptance

Do not enable source ordering in production based on this PR. It implements admission and current-state protection, not a complete transactional delivery design.

  1. Workflows already queued may execute after a newer transition. Serializing enqueue is not execution ordering; holding an ingestion lock through arbitrary workflow actions can deadlock reentrant workflows and delay recovery. This needs maintainer agreement and additional regression coverage.
  2. Existing commits between alert persistence, current-state advancement and dispatch need a crash-recovery/atomicity decision. No exactly-once delivery guarantee is claimed.
  3. Unsupported database engines fail closed only when ordering is explicitly enabled. Other providers and default-off ingestion keep their existing behavior.
  4. Migrating an existing generic provider while preserving its ID can leave a historical keep cursor. A reviewed identity/time handoff is required; this implementation refuses that conflict rather than rewriting historical alerts or silently accepting it.

Receiving an initial OK remains valid. recoveryEligible supplies episode context; the workflow decides whether to notify. This contribution is for upstream review and is not selected as a production fork/image.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@cayo-aura
cayo-aura marked this pull request as ready for review October 18, 2026 22:56
@github-actions

Copy link
Copy Markdown
Contributor

No linked issues found. Please add the corresponding issues in the pull request description.
Use GitHub automation to close the issue when a PR is merged

This branch has not been deployed

No deployments
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.

2 participants