Repository navigation
fix(alerts): read metric alert rules from org-scoped /detectors/ - #1275
Merged
Merged
Conversation
Migrate the metric-alert read path (list/view/get) off the legacy
/organizations/{org}/alert-rules/ endpoint, which is being retired on
2026-08-17, to the new org-scoped /organizations/{org}/detectors/
endpoint filtered to type:metric_issue.
Detectors return a nested shape, so mapDetectorToMetricAlertRule
flattens each detector into the existing MetricAlertRule shape the
list/view/resolve commands already consume (threshold fields live in
dataSources[0], active/disabled state is the top-level enabled flag).
Mirrors the proven issue-alert read migration (#1215/#1216). Metric
alert mutations (create/edit/delete) still use the legacy endpoint
pending the follow-up mutation migration.
Refs #1274, #1182
Contributor
|
Contributor
Codecov Results 📊✅ Patch coverage is 90.48%. Project has 5479 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 81.68% 81.70% +0.02%
==========================================
Files 426 426 —
Lines 29909 29945 +36
Branches 19433 19487 +54
==========================================
+ Hits 24431 24466 +35
- Misses 5478 5479 +1
- Partials 2035 2040 +5Generated by Codecov Action |
Contributor
Author
|
Marked ready for review — CI is green (build, lint & typecheck, unit tests, and the npm/binary smoke jobs all passed). Self-review found nothing to change. Couldn't auto-assign a reviewer here (issue reporter is a bot), so please pull in whoever owns the alerts API path. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Metric detectors expose the threshold window in seconds via dataSources/snubaQuery, but MetricAlertRule and every CLI formatter treat timeWindow as minutes (list/view append 'm'). Convert seconds->minutes so windows no longer render ~60x too large. Detectors are project-scoped and expose projectSlug rather than the legacy projects slug array, so metric alert view was showing '(all)'; map projects from projectSlug with a projects[] fallback. Also harden pickDetectorNumber to coerce non-numeric/NaN values to 0 so a malformed window renders as 0m instead of NaNm. Addresses Cursor Bugbot and Sentry review findings on #1275.
The detectors API can nest the SnubaQuery threshold fields (aggregate/dataset/query/timeWindow) at dataSources[0].queryObj.snubaQuery, not just under a flat queryObj or snubaQuery. The old queryObj ?? snubaQuery ?? source chain picked the queryObj wrapper and read empty fields off it, flattening aggregate/query/dataset to empty and timeWindow to 0 on list/view. Add resolveThresholdSource to walk queryObj.snubaQuery -> queryObj -> snubaQuery -> dataSource and return the first container that actually exposes an aggregate/query. Adds a queryObj.snubaQuery mapping test. Addresses Cursor Bugbot finding on #1275.
The /detectors/{id}/ endpoint returns any detector kind (uptime, cron,
error), unlike the type:metric_issue-filtered list. getMetricAlertRule
mapped the payload blind, so a numeric id resolving to a non-metric
detector rendered as a metric alert with empty threshold fields instead
of 404ing like the old /alert-rules/{id}/ path.
Type the detector 'type' field and throw a 404 ApiError when it is
present and not metric_issue; an absent type still maps (defensive for
deployments that omit it). Adds a rejection test.
Addresses Cursor Bugbot finding on #1275.
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 472b6b8. Configure here.
pickDetectorOwner flattened object owners to name/id, dropping the actor type. Legacy /alert-rules/ returned owner as an actor string (user:123 / team:456) and the CLI formatters/JSON still treat it that way, so list/ view emitted a different, less-usable owner value. Reconstruct type:id from the detector owner object when both are present; fall back to bare id/name otherwise. Adds a mapping test. Addresses Cursor Bugbot finding on #1275.
4 tasks
BYK
reviewed
Jul 22, 2026
BYK
reviewed
Jul 22, 2026
BYK
reviewed
Jul 22, 2026
Address review feedback on #1275: - listMetricAlertsPaginated / getMetricAlertRule now call the generated @sentry/api listOrganizationDetectors / getOrganizationDetector helpers via getOrgSdkConfig, instead of hand-rolled /detectors/ URL strings + apiRequestToRegion. Matches the repositories.ts / monitors.ts pattern and the AGENTS.md guidance to prefer @sentry/api. Errors now flow through the shared unwrap* helpers (endpoint is no longer set, consistent with every other SDK-backed module); tests updated accordingly. - Inline the one-line seconds->minutes timeWindow conversion instead of a standalone helper. - Name the detector fixture dateCreated constant (DETECTOR_DATE_CREATED) rather than an inline magic timestamp.
BYK
approved these changes
Jul 24, 2026
5 tasks
BYK
pushed a commit
that referenced
this pull request
Aug 2, 2026
Migrates metric-alert **create/edit/delete** off the legacy org-scoped
`/organizations/{org}/alert-rules/` endpoint (retiring **2026-08-17**)
onto the detectors write API, completing the read-path migration from
#1275. The CLI commands keep building the flat legacy body; a new
`buildDetectorBody` translator in `alerts.ts` converts it into the
nested detector document (`dataSources` / `conditionGroup` / `config`),
so the command layer is largely unchanged.
- `createMetricAlertRule` → `createOrganizationProjectDetector`
(project-scoped `POST /projects/{project}/detectors/`). Detector create
is project-scoped, so `--project` is now **required** for `create`; the
target is taken from the flat body's `projects`.
- `putMetricAlertRule` → `updateOrganizationDetector` (`PUT
/detectors/{id}/`).
- `getMetricAlertRuleDocument` (edit baseline) → detector `GET`,
returned in the flat `MetricAlertRule` shape so the command-side merge
stays coherent.
- `deleteMetricAlertRule` → `deleteOrganizationDetector` (`DELETE
/detectors/{id}/`).
Dataset → `queryType`/`eventTypes` mapping: error-like datasets
(`errors`/`events`) → `queryType: 0` + `["error","default"]`; everything
else → `queryType: 1` + `["trace_item_span"]`. `timeWindow` is converted
minutes→seconds (inverse of the read mapping), and `status` (0/1) maps
to the detector `enabled` flag.
### Known limitations (best-effort draft)
- `--project` on **edit** is now a no-op: the detector update endpoint
is not project-scoped and has no `projects` field.
- Uses static (threshold) detection only (`config.detectionType:
"static"`); change/dynamic detection types aren't surfaced by the flat
CLI input. Per #1274/#1142 full write parity is still gated on backend
feature gaps.
## Testing
- `pnpm exec vitest run test/lib/api/alerts.test.ts test/commands/alert`
— 170 passed. Added detector-endpoint assertions for create (nested
body, project-scoped path, error-like dataset mapping, missing-project
validation), put, delete, and the flat edit baseline.
- `pnpm exec tsc --noEmit` — clean. `biome check` — clean.
Closes #1277
<!--
## Plan
Design: keep the command layer building the flat legacy body; confine
the flat→nested translation to alerts.ts (mirrors the read-path
precedent). Keeps existing command tests' flat-body contract intact.
SDK surface (@sentry/api 0.253.0): deleteOrganizationDetector,
updateOrganizationDetector, getOrganizationDetector (already used),
createOrganizationProjectDetector (project-scoped POST). No org-scoped
createOrganizationDetector, so create is project-scoped.
alerts.ts: buildDetectorBody(flat) -> { name, type: metric_issue,
dataSources:[buildDetectorDataSource], config:{detectionType:static},
conditionGroup?, enabled?, owner? }. datasetQueryType/datasetEventTypes
keyed on ERROR_LIKE_DATASETS={errors,events}. resolveDetectorProject
picks first non-empty projects[] entry, else ValidationError.
getMetricAlertRuleDocument returns getMetricAlertRule(...) flat shape;
put/create/delete go through the SDK detector fns; remove
fetchMetricAlertRuleJson. detector_id string passed via the same
double-cast pattern used by issue-alert writes.
create.ts: require --project (detectors create is project-scoped).
Tests: rewrite deleteMetricAlertRule to detectors DELETE; add
create/put/getDocument tests; add --project to the 4 create-path command
tests + expected projects in the flat body + a missing-project rejection
test.
-->
Co-authored-by: jared-outpost[bot] <jared-outpost[bot]@users.noreply.github.com>
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
1 participant
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.
Migrates the metric-alert read path (
list/view/ rule resolution) off the legacy/organizations/{org}/alert-rules/endpoint — which is being retired on 2026-08-17 and 410s during brownouts — to the new org-scoped/organizations/{org}/detectors/endpoint filtered totype:metric_issue. AmapDetectorToMetricAlertRulehelper flattens the nested detector payload into the existingMetricAlertRuleshape, so command formatters and resolvers work unchanged.This mirrors the proven issue-alert read migration (#1215/#1216). Metric-alert mutations (
create/edit/delete) still use the legacy endpoint pending the follow-up mutation migration tracked in #1277 — same transitional state as issue alerts today.Testing
pnpm run typecheck— cleanpnpm run lint— cleannpx vitest run test/lib/api/alerts.test.ts test/commands/alert— 150 passing (updated metric-list mocks to/detectors/; added list/get + nested-snubaQuerymapping tests)Refs #1274, getsentry/toolkit#1482