Skip to content

fix(cli): Send native detector datasets and conditions for metric alerts - #1591

Merged
BYK merged 6 commits into
mainfrom
fix/cli-metric-alert-logs-detector-triggers
Oct 22, 2026
Merged

BYK merged 6 commits into
mainfrom
fix/cli-metric-alert-logs-detector-triggers

Conversation

@sentry-junior

@sentry-junior sentry-junior Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1588.

The detectors migration (getsentry/cli#1275, #1338) fixed the list/view mismatch and the legacy /alert-rules/ calls from the issue. But alert metrics create/edit still sends the legacy payload to the detectors API, and the backend rejects it:

  • Datasets: the CLI sends spans / errors / sessions as-is. SnubaQueryValidator only accepts native Dataset values that match queryType (errors isn't a Dataset, and spans isn't valid for queryType: 1). There was also no way to pick logs.
  • Triggers: {alertThreshold, actions} objects are passed as condition-group conditions verbatim. MetricIssueComparisonConditionValidator requires type/comparison/conditionResult, so any --trigger fails.

What changed:

  • CLI datasets now map to native data sources: errors/events → events (type 0), spans → events_analytics_platform + trace_item_span, logs (new) → events_analytics_platform + trace_item_log, sessions/metrics → metrics (crash rate, type 2). Reads map them back, so list/view show the names create accepts. An edit round-trip also keeps a log alert on trace_item_log instead of turning it into a span alert.
  • alertThreshold triggers (optional label, resolveThreshold, thresholdType) become threshold conditions plus the one resolution condition the validator requires. Native detector conditions (with type) pass through untouched. Both shapes are validated with Valibot schemas.
  • Detectors don't take notification actions. So triggers with actions now fail with a clear error instead of being dropped silently. A new --workflow <id> flag on create/edit connects existing alerts through workflowIds.
  • The native values live in a new @sentry/toolkit-core/metric-monitor module, which MCP's metric monitor schemas now use too. @sentry/api types data sources and conditions as unknown[], so there's nothing to import from it. Swapping this module for generated @sentry/api types is tracked in Use typed detector schemas from @sentry/api for metric monitors #1595. On the CLI side, one dataset table in src/types/metric-alert.ts drives --dataset help, validation, aliases, and both mapping directions.

For data sources the CLI doesn't model (for example trace metrics), edit keeps the stored eventTypes and queryType instead of switching the monitor to spans.

via BYK.

--

View Junior Session [Sentry]

`sentry alert metrics create/edit` moved onto the detectors API but still
sent the legacy alert-rule payload: CLI dataset names (`spans`, `errors`)
that the detector `SnubaQueryValidator` rejects, and legacy trigger objects
as condition-group conditions that lack the required `type`. There was also
no way to target logs.

- Map CLI datasets onto native data sources (errors/events -> events,
  spans/logs -> events_analytics_platform with trace_item_span/trace_item_log,
  sessions/metrics -> crash-rate metrics) and add the `logs` dataset.
- Map them back on read so list/view show CLI names and edit keeps log alerts
  on trace_item_log.
- Translate alertThreshold triggers into threshold + resolution conditions;
  pass native detector conditions through.
- Reject trigger actions (the detector API drops them) and add `--workflow`
  to connect existing alerts.

Fixes #1588

Co-Authored-By: Burak Yigit Kaya <burak.kaya@sentry.io>
@vercel

vercel Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
sentry-local Skipped Skipped Oct 22, 2026 12:13pm UTC

Request Review

@github-actions github-actions Bot added the risk: medium PR risk score: medium label Oct 22, 2026
Comment thread packages/cli/test/lib/api/alerts.test.ts
…tors

The edit baseline flattened the detector to a CLI dataset name, so a
rename or status change on an EAP monitor the CLI does not model (e.g.
`trace_item_metric`) rebuilt its data source with span defaults. Carry the
stored `eventTypes`/`queryType` through the baseline and reuse them for
non-CLI datasets.
Comment thread packages/cli/src/commands/alert/metrics/create.ts Outdated
Comment thread packages/cli/src/commands/alert/metrics/edit.ts Outdated
Comment thread packages/cli/src/commands/alert/mutation-utils.ts Outdated
Comment thread packages/cli/src/commands/alert/mutation-utils.ts Outdated
Comment thread packages/cli/src/lib/api/alerts.ts Outdated
Address review on #1591:

- Add `@sentry/toolkit-core/metric-monitor` with the native detector
  datasets, threshold operators, priorities, and per-signal data sources
  (from the Sentry validators, since `@sentry/api` types data sources and
  conditions as `unknown[]`). MCP's metric monitor schemas now use it too.
- Add `src/types/metric-alert.ts` as the single CLI dataset table: the
  `--dataset` help, validation, aliases, write mapping, and read-back name
  are all derived from it.
- Validate `--trigger` with Valibot schemas and pass the typed result to the
  detector body builder, replacing the ad-hoc checks and casts.

@cursor cursor Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread packages/cli/src/types/metric-alert.ts
An empty eventTypes list matched any CLI dataset on the same native
dataset, so EAP monitors without event types read back as spans and an
edit rewrote them onto trace_item_span. Only resolve a CLI name for
empty event types when the native dataset is unambiguous, and keep
missing event types empty on edit instead of defaulting to spans.
Comment thread packages/cli/src/lib/api/alerts.ts
Comment thread packages/cli/src/types/metric-alert.ts
Edits always sent config { detectionType: static }, which replaced the
stored config and turned percent and dynamic detectors into static ones
(and deletes Seer data for dynamic detectors). Carry the stored detector
config through the edit baseline and send it back; creates still default
to static detection.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 77bd980. Configure here.

Comment thread packages/cli/src/lib/api/alerts.ts
Edits resent the stored detector config even when --trigger supplied
alertThreshold triggers, so percent detectors read the thresholds as
percent comparisons and dynamic detectors got static conditions the
backend rejects. Threshold triggers are static thresholds, so send a
static config with them; native conditions and trigger-less edits keep
the stored config.
@BYK
BYK merged commit 5b7ea24 into main Oct 22, 2026
37 checks passed
@BYK
BYK deleted the fix/cli-metric-alert-logs-detector-triggers branch October 22, 2026 12:38

This branch was successfully deployed

1 active and 1 inactive deployments
Preview — ac111f13 Deployed Oct 22, 2026 by vercel[bot]
Actions — ac111f13 Deployed Oct 22, 2026 by sentry-junior[bot] via eval #1354
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: medium PR risk score: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

alert metrics CLI doesn't support detector/workflow log alerts and list/view are inconsistent

1 participant