Release the replication lag alert when the master link drops

[jamby77]

Summary

Fixes #538. replication.lag was only dispatched while master_link_status was up. If the link dropped while lag was above the threshold, the fired alert never saw a recovery sample and stayed latched until the 24h state TTL, so a later lag episode on the same replica would not alert.

The alert state is now cleared whenever no meaningful lag sample can arrive: the master link is not up, or the node has been promoted to primary. A new episode after the link returns fires a fresh alert.

The lag alert key was also global (replication_lag), so one replica's lag latched the alert for every connection. With clearing added that would also let a healthy primary wipe another replica's live alert, so the key is now per connection (replication_lag:<connectionId>), matching newer threshold alerts such as scaling_readiness_low.

Changes

• WebhookDispatcherService.clearThresholdAlert(alertKey) drops a latched alert state
• WebhookEventsProService.clearReplicationLag(connectionId) and a per-connection lag alert key; clearReplicationLag added to IWebhookEventsProService
• PrometheusService clears the lag alert when a replica's link is not up and when the node reports role:master

Checklist

• Unit / integration tests added
• Docs added / updated
• Roborev review passed — run roborev review --branch or /roborev-review-branch in Claude Code (internal)
• Competitive analysis done / discussed (internal)
• Blog post about it discussed (internal)

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes replication lag alerting and hysteresis keys in the metrics poll path; incorrect clearing could miss or duplicate alerts, but scope is limited to PRO replication.lag webhooks with new tests.

Overview
Fixes stuck replication.lag webhook alerts when lag stops being observable (master link down or replica promoted) and cross-connection alert coupling from a single global hysteresis key.

WebhookDispatcherService adds clearThresholdAlert so latched threshold state can be dropped without a sub-threshold “recovery” sample—needed because lag samples are only sent while master_link_status is up.

WebhookEventsProService keys lag alerts per connection (replication_lag:<connectionId> instead of replication_lag) and exposes clearReplicationLag on the shared Pro interface. PrometheusService calls clear when the node reports role: master or when a replica’s master link is not up (no dispatch path).

Unit tests cover dispatcher clear/re-fire behavior, Pro dispatch/clear keys, and Prometheus metric update wiring.

^{Reviewed by Cursor Bugbot for commit d16ceb1. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

• Bug Fixes
  • Replication-lag alerts now clear when a replica’s master link goes down, lag data is unavailable, or the replica is promoted to master.
  • Cleared alerts can trigger again if the issue returns, without requiring a recovery sample first.
  • Replication-lag alerts are tracked separately for identified connections.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: ebd497f5-b571-417e-8196-82bf3e282e06

📥 Commits

Reviewing files that changed from the base of the PR and between 7839962 and d16ceb1.

📒 Files selected for processing: 7

• apps/api/src/prometheus/prometheus.replication-lag.spec.ts
• apps/api/src/prometheus/prometheus.service.ts
• apps/api/src/webhooks/__tests__/webhook-dispatcher.service.spec.ts
• apps/api/src/webhooks/webhook-dispatcher.service.ts
• packages/shared/src/webhooks/types.ts
• proprietary/webhook-pro/__tests__/replication-lag-dispatch.spec.ts
• proprietary/webhook-pro/webhook-events-pro.service.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Replication-lag alerts now use connection-specific keys when a connection ID is available. The service clears latched alert state when a replica is no longer eligible for dispatch or is promoted to master.

Changes: Replication-lag alert lifecycle

Layer / File(s)	Summary
Keyed alert dispatch and clearing packages/shared/src/webhooks/types.ts, apps/api/src/webhooks/webhook-dispatcher.service.ts, apps/api/src/webhooks/__tests__/webhook-dispatcher.service.spec.ts, proprietary/webhook-pro/...	The webhook interface adds clearReplicationLag. The Pro service uses matching per-connection keys for dispatch and clearing, and the dispatcher removes the cached threshold-alert state. Tests cover dispatch and clearing.
Prometheus replication-lag lifecycle apps/api/src/prometheus/prometheus.service.ts, apps/api/src/prometheus/prometheus.replication-lag.spec.ts	Prometheus clears replication-lag state when a connection is a master or when replica dispatch conditions are not met. Tests cover dispatch while the master link is up and clearing when it is down or the replica becomes a master.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant PrometheusService
  participant WebhookEventsProService
  participant WebhookDispatcherService
  PrometheusService->>WebhookEventsProService: Dispatch replication lag with connection ID
  WebhookEventsProService->>WebhookDispatcherService: Dispatch threshold alert with keyed alert
  PrometheusService->>WebhookEventsProService: Clear replication lag when dispatch conditions do not apply
  WebhookEventsProService->>WebhookDispatcherService: Clear threshold alert with matching key

Loading

Suggested reviewers: kivanow

Merge Risk: ⚪ Minimal · up to d16ce

Replication-lag alerts can be cleared when the link drops or the node is promoted, allowing a later lag episode to alert again. No merge-blocking issue was identified.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 7 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: clearing the replication-lag alert when the master link drops.
Description check		The description includes the required Summary, Changes, and Checklist sections. It explains the root cause, implementation, per-connection alert keys, and added tests. Unchecked documentation and inte…
Linked Issues check		The PR meets the coding requirement in #538. PrometheusService clears replication-lag state when the master link is not up and when the node reports the master role. WebhookEventsProService us…
Out of Scope Changes check		The changes stay within #538. The dispatcher clear method, the Pro service clear method, the shared interface update, the per-connection replication-lag key, Prometheus integration, and related tests …

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

apps/api/src/prometheus/prometheus.replication-lag.spec.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

apps/api/src/prometheus/prometheus.service.ts

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).

apps/api/src/webhooks/__tests__/webhook-dispatcher.service.spec.ts

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).

• 1 others

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[KIvanow]

Good fix for #538 — the per-connection key and the clear primitive are the right shape, and the root-cause writeup made this easy to review. A few gaps at the edges of the fix, plus one regression worth closing before merge.

1. The else-clear also fires when the link is up but the lag field is merely unparsable (prometheus.service.ts:1888)

The dispatch condition is masterLinkStatus === 'up' && lastIoSecondsAgo !== null, and the new else clears on any non-dispatch poll. So a single poll where INFO omits or garbles master_last_io_seconds_ago (truncated INFO, proxy/fork variants) while the link is still up deletes a live latch, and the next normal poll re-fires a duplicate webhook for an unchanged incident — pre-PR the latch survived this. Suggest scoping the clear to masterLinkStatus !== 'up' specifically (a null field alone shouldn't clear), or dispatching a below-threshold recovery sample and letting the existing hysteresis decide.

2. The role !== 'master' && !isReplica branch sweeps the gauges but keeps the latch (prometheus.service.ts:1826)

That branch calls clearRoleSpecificReplicationSeries — acknowledging the node stopped being a replica — but doesn't call clearReplicationLag, asymmetric with the role === 'master' branch just below, which does. By the PR's own "no meaningful lag sample can arrive" rule, this path qualifies too.

3. Connection removal/retirement never clears the latch (prometheus.service.ts, cleanupConnectionMetrics)

onConnectionRemoved → cleanupConnectionMetrics prunes all other per-connection state but not replication_lag:<id>. Concrete same-process trigger: auto-registered cluster children get retired on topology changes (which fires onConnectionRemoved) and later reactivated under the same id — a latch from before retirement then suppresses the first real breach after reactivation for up to the 24h TTL. One-line fix: call clearReplicationLag(connectionId) in the cleanup hook.

4. The same global-key bug is still live in four sibling alerts (fast-follow, not this PR)

slowlog_threshold, latency_spike, connection_spike, and enterprise compliance_alert all pass per-connection data into a globally-shared latch key — the exact cross-connection coupling this PR fixes for lag. compliance_alert is the worst case: its caller only dispatches on breach (usedPercent > 80), so the shared latch never sees a recovery sample and one connection's breach permanently suppresses every other connection's compliance alerts. Since dispatchThresholdAlert already receives connectionId (and dispatchThresholdAlertPerWebhook already scopes centrally with ${prefix}:${connectionId}:${webhook.id}), the clean fix is one line in the dispatcher — which would fix all four siblings and let replicationLagKey() be deleted. Happy to file it as an issue if you'd rather keep this PR scoped.

5. The optional-connectionId fallback is dead code that invites the bug back (webhook-events-pro.service.ts:13)

Every caller passes an id, and the siblings (scaling_readiness_low, metric_forecast_limit) require it in their keys. Keeping connectionId optional in replicationLagKey/clearReplicationLag/the interface allows an asymmetric dispatch-without-id vs clear-with-id pair — a silent re-latch with no signal, the exact class this PR fixes. Making it required is compile-safe (no existing caller or mock breaks).

6. No test drives the motivating sequence (prometheus.replication-lag.spec.ts)

Each test does a single poll against a fresh service; fire → clear → re-fire is only proven inside the dispatcher spec with a hand-built 'lag_test' key, so nothing guards the cross-layer key agreement the fix depends on. One multi-poll sequence test (lag fires → link down clears → link up re-fires) through the service would pin it. Minor: the spec's 9-positional-arg construction with as never is the pattern prometheus.key-sizes.spec.ts's buildService() helper avoids — worth reusing.

Also noting non-blocking: declaring implements IWebhookEventsProService on WebhookEventsProService would make the new interface method compiler-enforced against the implementation.