Skip to content

[Alerting v2] Recovery delay is configurable and persisted even when it has no effect on execution #279941

Description

@baileycash-elastic

Problem

Recovery delay (state_transition.recovering_count / state_transition.recovering_timeframe) only ever does real work in the execution pipeline when a recovered-like transition can actually happen. Today the GUI form and the create/update APIs let a user configure and persist recovery delay in states where it can never apply, with no feedback that it's inert.

When recovery delay is inert

Recovery delay is inert (stored but never applied) exactly when:

recovery_strategy is null/'none'  AND  no_data_strategy !== 'recover'
  • recovery_strategy: 'none' short-circuits condition-based recovery entirely — see CreateRecoveryEventsStep:
if (rule.recovery_strategy == null || rule.recovery_strategy === 'none') {
  return events;
}
  • The one carve-out: no_data_strategy: 'recover' aliases a no_data event to a synthetic recovered event regardless of recovery_strategy (basic_strategy.ts), and CountTimeframeStrategy gates that transition using the same recovering_count / recovering_timeframe fields. So the combination recovery_strategy: 'none' + no_data_strategy: 'recover' is a legitimate, intentional case where recovery delay is not inert — confirmed by new director unit tests (director.test.ts, "no_data_strategy 'recover' + recovery_strategy 'none' with a recovering delay").

Current gaps

  1. UX: RecoveryDelayField (recovery_delay_field.tsx) renders unconditionally in the compose/discover recovery step regardless of recovery_strategy / no_data_strategy, so a user can set a delay that will never be evaluated.
  2. Backend: createRuleDataSchema / updateRuleDataSchema (rule_data_schema.ts) accept and persist state_transition.recovering_count / recovering_timeframe unconditionally — no rejection when the combination above makes it inert.

Decision required

This splits into two independently-decidable parts.

1. UX fix — not in question, proceeding

Hide RecoveryDelayField from the GUI whenever the inert condition above holds, mirroring the existing isNonRepresentableRule gating pattern (is_non_representable.ts). This is being implemented now regardless of the backend decision below.

2. Backend enforcement — open question

  • Option A — Enforce via strict schema validation now. Reject state_transition.recovering_count / recovering_timeframe with 400 on create, and on update against the merged (stored + patched) attributes, whenever the inert condition holds. Consistent with the strict-rejection precedent decided in rna-program/435. If adopted, this fully closes the loophole going forward — an inert recovery delay can never be persisted via the API.
  • Option B — Don't enforce at the API layer now, punt to M3. Keep accepting the combination (e.g. for forward/backward compatibility, or because some external/YAML-authored rules may intentionally carry values the GUI doesn't support). If chosen, isNonRepresentableRule must be extended with a new case: a stored rule with recovery delay configured while inert is non-representable in the GUI and must force YAML-only edit mode, same pattern already used for recovery_strategy outside the form's supported set and no_data_strategy: 'emit'. Full backend work will be done later.

Both options are compatible with the UX fix above; they differ only in what happens for rules created/updated outside the GUI (raw API, YAML mode).

Acceptance criteria

  • RecoveryDelayField hidden in the GUI whenever recovery delay would be inert, regardless of which backend option is chosen
  • Decision recorded: Option A vs Option B
  • If A: create schema and update path (checked against merged attributes) reject inert recovery delay with 400; tests added for both create and update
  • If B: is_non_representable.ts extended to flag stored-but-inert recovery delay; rule forced into YAML mode when editing; tests added
  • Director-level test coverage for no_data_strategy: 'recover' + recovery_strategy: 'none' + a configured delay (already added to director.test.ts)

References

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Feature:AlertingV2Team:ResponseOpsPlatform ResponseOps team (formerly the Cases and Alerting teams) t//Team:actionable-obsFormerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics.Team:rna-project

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions