Skip to content

fix(api): bound alert history lookback by check interval#2515

Open
satyasai1916 wants to merge 3 commits into
hyperdxio:mainfrom
satyasai1916:fix/2434-alert-history-lookback
Open

fix(api): bound alert history lookback by check interval#2515
satyasai1916 wants to merge 3 commits into
hyperdxio:mainfrom
satyasai1916:fix/2434-alert-history-lookback

Conversation

@satyasai1916

Copy link
Copy Markdown
Contributor

Summary

  • Replace the fixed 7-day lookback in getPreviousAlertHistories with an interval-based narrow window (5x check interval, 1h floor).
  • Fall back to the existing 7-day window only when the narrow query returns no rows, preserving behavior after checker gaps.
  • Pass each alert's interval from the default provider so lookback scales per alert.

Fixes #2434

Test plan

  • Unit tests: computeAlertHistoryLookbackMs values
  • Integration tests: existing getPreviousAlertHistories suite + fallback case
  • CI: lint, unit, integration

Replace the fixed 7-day getPreviousAlertHistories scan with an
interval-based narrow window (with floor and 7-day fallback) to
reduce MongoDB index scans on the alert checker hot path.

Fixes hyperdxio#2434

Co-authored-by: Cursor <cursoragent@cursor.com>
@vercel

vercel Bot commented Jun 24, 2026

Copy link
Copy Markdown

@satyasai1916 is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

@changeset-bot

changeset-bot Bot commented Jun 24, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: eef46f4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/api Patch
@hyperdx/app Patch
@hyperdx/otel-collector Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@greptile-apps

greptile-apps Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Replaces the hard-coded 7-day lookback in getPreviousAlertHistories with a dynamic narrow window (5× check interval, floored at 1 h, capped at 7 d), then falls back to the 7-day window only when the narrow query returns nothing and AlertHistory.exists() confirms older history actually exists — cutting MongoDB index scans on the hot path.

  • index.ts: Extracts the aggregation pipeline into aggregatePreviousAlertHistoriesForAlert, adds computeAlertHistoryLookbackMs, introduces AlertHistoryLookup type, and wires the two-phase narrow→fallback logic into getPreviousAlertHistories.
  • default.ts: Passes each alert's interval through to the query helper, a one-line plumbing change.
  • checkAlerts.test.ts: Adds unit tests for computeAlertHistoryLookbackMs and two integration tests covering the no-history (narrow only) and gap-recovery (fallback) branches; updates all existing call-sites to use the new AlertHistoryLookup shape.

Confidence Score: 5/5

Safe to merge; the narrow-window optimisation is additive and the fallback preserves existing behaviour for gap-recovery scenarios.

The two-phase narrow→fallback logic is correct for all reachable cases: alerts with recent history get one aggregation, alerts with older-than-narrow history fall back correctly via the exists() guard, and alerts with no history at all skip the wide query entirely. The computeAlertHistoryLookbackMs formula is verified by unit tests for every distinct range (floor, mid, cap).

No files require special attention.

Important Files Changed

Filename Overview
packages/api/src/tasks/checkAlerts/index.ts Introduces computeAlertHistoryLookbackMs, aggregatePreviousAlertHistoriesForAlert, and the two-phase narrow→exists→fallback logic; all edge cases (no history, gap recovery, equal-to-MAX guard) handle correctly.
packages/api/src/tasks/checkAlerts/tests/checkAlerts.test.ts Adds computeAlertHistoryLookbackMs unit tests and two integration tests (no-history / fallback). Existing call-sites updated to pass AlertHistoryLookup.
packages/api/src/tasks/checkAlerts/providers/default.ts Passes each alert's interval into getPreviousAlertHistories — straightforward plumbing, no logic change.
.changeset/fix-alert-history-lookback.md Patch-level changeset entry for @hyperdx/api; description matches the implementation.

Reviews (3): Last reviewed commit: "fix(api): prettier formatting in checkAl..." | Re-trigger Greptile

Comment thread packages/api/src/tasks/checkAlerts/index.ts
Use a cheap exists check before the 7-day fallback so new alerts
without any history only run one aggregation per tick.

Co-authored-by: Cursor <cursoragent@cursor.com>
fleon
fleon previously approved these changes Jun 29, 2026
@fleon

fleon commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

@satyasai1916 One of the lint jobs is failing, could you please take a look

/home/runner/work/hyperdx/hyperdx/packages/api/src/tasks/checkAlerts/__tests__/checkAlerts.test.ts
    8974:24  error  Replace `⏎······ids:·string[],⏎······interval:·AlertInterval·=·'5m',` with `ids:·string[],·interval:·AlertInterval·=·'5m')·=>`  prettier/prettier
    8977:5   error  Replace `)·=>` with `·`                                                                                                         prettier/prettier

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

checkAlerts: getPreviousAlertHistories scans a fixed 7-day window per alert per tick (only needs the latest record per group)

2 participants