diff --git a/apps/cli-docs/src/fragments/commands/alert.md b/apps/cli-docs/src/fragments/commands/alert.md index ee32b001f..9b6d80c1d 100644 --- a/apps/cli-docs/src/fragments/commands/alert.md +++ b/apps/cli-docs/src/fragments/commands/alert.md @@ -49,16 +49,33 @@ sentry alert issues delete my-org/my-project/12345 --dry-run ### Create a metric alert rule ```bash -# Create an organization metric alert rule +# Alert when P95 span duration goes above 500ms sentry alert metrics create my-org \ --name "P95 Latency" \ + --project my-app \ --query "environment:prod" \ --aggregate "p95(span.duration)" \ --dataset spans \ --time-window 5 \ - --trigger '{"alertThreshold":500,"actions":[{"id":"sentry.mail.actions.NotifyEmailAction","targetType":"Team","targetIdentifier":1}]}' + --trigger '{"alertThreshold":500}' + +# Alert on any matching log line and notify through an existing alert +sentry alert metrics create my-org \ + --name "AI chat failures" \ + --project my-app \ + --query "action:ai.chat result:error" \ + --aggregate "count()" \ + --dataset logs \ + --time-window 5 \ + --trigger '{"alertThreshold":0,"resolveThreshold":0}' \ + --workflow 12345 ``` +Triggers take `alertThreshold`, an optional `label` (`critical` or `warning`), +`resolveThreshold`, and `thresholdType` (`0` above, `1` below). Notification +actions are not part of triggers; pass `--workflow` with the ID of an existing +alert to notify. + ### List metric alert rules ```bash diff --git a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/alert.md b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/alert.md index 75108dc2d..b7b150e57 100644 --- a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/alert.md +++ b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/alert.md @@ -155,25 +155,38 @@ Create a metric alert rule - `--name - Rule name` - `--query - Metric query filter string` - `--aggregate - Aggregate expression (for example count(), p95(span.duration))` -- `--dataset - Dataset: errors (error-events), sessions, events, spans, metrics (transaction(s) routes to spans)` +- `--dataset - Dataset: errors, events, spans, logs, metrics, sessions (error-events is errors; transaction(s) routes to spans)` - `--time-window - Evaluation window in minutes` - `-t, --trigger ... - Trigger object JSON (repeatable, or pass one JSON array)` - `-p, --project ... - Project slug filter (repeatable or comma-separated)` - `--environment - Environment filter` - `--owner - Owner value accepted by Sentry API` +- `--workflow ... - Alert (workflow) id to notify (repeatable or comma-separated)` - `-n, --dry-run - Show what would happen without making changes` **Examples:** ```bash -# Create an organization metric alert rule +# Alert when P95 span duration goes above 500ms sentry alert metrics create my-org \ --name "P95 Latency" \ + --project my-app \ --query "environment:prod" \ --aggregate "p95(span.duration)" \ --dataset spans \ --time-window 5 \ - --trigger '{"alertThreshold":500,"actions":[{"id":"sentry.mail.actions.NotifyEmailAction","targetType":"Team","targetIdentifier":1}]}' + --trigger '{"alertThreshold":500}' + +# Alert on any matching log line and notify through an existing alert +sentry alert metrics create my-org \ + --name "AI chat failures" \ + --project my-app \ + --query "action:ai.chat result:error" \ + --aggregate "count()" \ + --dataset logs \ + --time-window 5 \ + --trigger '{"alertThreshold":0,"resolveThreshold":0}' \ + --workflow 12345 ``` ### `sentry alert metrics delete ` @@ -201,12 +214,13 @@ Edit a metric alert rule - `--status - active or disabled` - `--query - Metric query filter` - `--aggregate - Aggregate expression` -- `--dataset - Dataset: errors (error-events), sessions, events, spans, metrics (transaction(s) routes to spans)` +- `--dataset - Dataset: errors, events, spans, logs, metrics, sessions (error-events is errors; transaction(s) routes to spans)` - `--time-window - Evaluation window in minutes` - `-t, --trigger ... - Trigger object JSON (repeatable, or pass one JSON array)` - `-p, --project ... - Project slug filter (repeatable or comma-separated)` - `--environment - Environment value (pass empty string to clear)` - `--owner - Owner value (pass empty string to clear)` +- `--workflow ... - Replace connected alert (workflow) ids (repeatable or comma-separated; empty string disconnects all)` **Examples:** diff --git a/packages/cli/src/commands/alert/metrics/create.ts b/packages/cli/src/commands/alert/metrics/create.ts index 79390420a..a29922906 100644 --- a/packages/cli/src/commands/alert/metrics/create.ts +++ b/packages/cli/src/commands/alert/metrics/create.ts @@ -12,13 +12,17 @@ import { warning } from "../../../lib/formatters/colors.js"; import { CommandOutput } from "../../../lib/formatters/output.js"; import { DRY_RUN_ALIASES, DRY_RUN_FLAG } from "../../../lib/mutate-command.js"; import { resolveOrgOnlyFromArg } from "../../../lib/resolve-target.js"; +import { + METRIC_ALERT_DATASET_BRIEF, + parseMetricTriggers, +} from "../../../types/metric-alert.js"; import { normalizeProjectList, parseJsonObjectList, + parseWorkflowIds, resolveMetricDataset, validateMetricDataset, validateMetricTimeWindow, - validateMetricTriggers, } from "../mutation-utils.js"; import { metricAlertStatusLabel } from "./status.js"; @@ -32,6 +36,7 @@ type CreateFlags = { readonly project?: string[]; readonly environment?: string; readonly owner?: string; + readonly workflow?: string[]; readonly "dry-run": boolean; readonly json: boolean; readonly fields?: string[]; @@ -69,20 +74,26 @@ export const createCommand = buildCommand({ docs: { brief: "Create a metric alert rule", fullDescription: - "Create an organization-scoped metric alert rule.\n\n" + + "Create a metric alert rule (a metric monitor in the detectors API).\n\n" + "Required fields:\n" + " --name, --query, --aggregate, --dataset, --time-window, --trigger (>=1), --project\n\n" + "Optional fields:\n" + - " --environment, --owner\n\n" + + " --environment, --owner, --workflow\n\n" + + "Triggers:\n" + + ' {"alertThreshold":N, "label":"critical"|"warning", "resolveThreshold":N, "thresholdType":0|1}\n' + + " thresholdType 0 alerts above the threshold (default), 1 below. Without resolveThreshold\n" + + " the rule resolves at the critical threshold. Native detector conditions\n" + + ' ({"type":"gt","comparison":N,"conditionResult":75}) are passed through as-is.\n' + + " Notifications are not part of triggers: pass --workflow with an existing alert id.\n\n" + "Examples:\n" + " sentry alert metrics create my-org/ --name 'P95 latency' \\\n" + " --query 'environment:prod' --aggregate 'p95(span.duration)' \\\n" + - " --dataset spans --time-window 5 \\\n" + - ' --trigger \'{"alertThreshold":500,"actions":[{"id":"sentry.mail.actions.NotifyEmailAction","targetType":"Team","targetIdentifier":1}]}\'\n\n' + - " sentry alert metrics create my-org/ --name 'Error volume' \\\n" + - " --query 'event.type:error' --aggregate 'count()' --dataset errors \\\n" + - ' --time-window 15 --trigger \'[{"alertThreshold":100,"actions":[{"id":"sentry.mail.actions.NotifyEmailAction","targetType":"Team","targetIdentifier":1}]}]\' \\\n' + - " --project my-app --dry-run", + " --dataset spans --time-window 5 --project my-app \\\n" + + " --trigger '{\"alertThreshold\":500}'\n\n" + + " sentry alert metrics create my-org/ --name 'AI chat failures' \\\n" + + " --query 'action:ai.chat result:error' --aggregate 'count()' --dataset logs \\\n" + + ' --time-window 5 --trigger \'{"alertThreshold":0,"resolveThreshold":0}\' \\\n' + + " --project my-app --workflow 123 --dry-run", }, output: { human: formatCreated, @@ -118,8 +129,7 @@ export const createCommand = buildCommand({ dataset: { kind: "parsed", parse: String, - brief: - "Dataset: errors (error-events), sessions, events, spans, metrics (transaction(s) routes to spans)", + brief: METRIC_ALERT_DATASET_BRIEF, }, "time-window": { kind: "parsed", @@ -152,6 +162,13 @@ export const createCommand = buildCommand({ optional: true, brief: "Owner value accepted by Sentry API", }, + workflow: { + kind: "parsed", + parse: String, + variadic: true, + optional: true, + brief: "Alert (workflow) id to notify (repeatable or comma-separated)", + }, "dry-run": DRY_RUN_FLAG, }, aliases: { ...DRY_RUN_ALIASES, t: "trigger", p: "project" }, @@ -179,8 +196,9 @@ export const createCommand = buildCommand({ this.stderr.write(`${warning(`Tip: ${datasetNotice}`)}\n`); } - const triggers = parseJsonObjectList(flags.trigger, "trigger"); - validateMetricTriggers(triggers); + const triggers = parseMetricTriggers( + parseJsonObjectList(flags.trigger, "trigger"), + ); const projects = normalizeProjectList(flags.project); if (!projects || projects.length === 0) { throw new ValidationError( @@ -208,6 +226,10 @@ export const createCommand = buildCommand({ if (flags.owner !== undefined) { body.owner = flags.owner; } + const workflowIds = parseWorkflowIds(flags.workflow); + if (workflowIds) { + body.workflowIds = workflowIds; + } if (flags["dry-run"]) { yield new CommandOutput({ diff --git a/packages/cli/src/commands/alert/metrics/edit.ts b/packages/cli/src/commands/alert/metrics/edit.ts index 6bba1666a..e9c480af7 100644 --- a/packages/cli/src/commands/alert/metrics/edit.ts +++ b/packages/cli/src/commands/alert/metrics/edit.ts @@ -14,16 +14,20 @@ import { ValidationError } from "../../../lib/errors.js"; import { warning } from "../../../lib/formatters/colors.js"; import { CommandOutput } from "../../../lib/formatters/output.js"; import { resolveOrgOnlyFromArg } from "../../../lib/resolve-target.js"; +import { + METRIC_ALERT_DATASET_BRIEF, + parseMetricTriggers, +} from "../../../types/metric-alert.js"; import { normalizeMetricDataset, normalizeProjectList, parseJsonObjectList, parseStatusFlag, + parseWorkflowIds, resolveMetricDataset, statusToMetricValue, validateMetricDataset, validateMetricTimeWindow, - validateMetricTriggers, } from "../mutation-utils.js"; import { parseMetricRuleArg, resolveMetricAlertRule } from "./rule-resolve.js"; import { metricAlertStatusLabel } from "./status.js"; @@ -42,6 +46,7 @@ type EditFlags = { readonly project?: string[]; readonly environment?: string; readonly owner?: string; + readonly workflow?: string[]; readonly json: boolean; readonly fields?: string[]; }; @@ -62,7 +67,8 @@ function hasMetricMutations(flags: EditFlags): boolean { flags.trigger !== undefined || flags.project !== undefined || flags.environment !== undefined || - flags.owner !== undefined + flags.owner !== undefined || + flags.workflow !== undefined ); } @@ -118,7 +124,9 @@ function applyMetricOptionalFields( flags: EditFlags, ): Record { if (flags.trigger !== undefined) { - body.triggers = parseJsonObjectList(flags.trigger, "trigger"); + body.triggers = parseMetricTriggers( + parseJsonObjectList(flags.trigger, "trigger"), + ); } if (flags.project !== undefined) { body.projects = normalizeProjectList(flags.project) ?? []; @@ -130,6 +138,9 @@ function applyMetricOptionalFields( if (flags.owner !== undefined) { body.owner = flags.owner.trim() === "" ? null : flags.owner; } + if (flags.workflow !== undefined) { + body.workflowIds = parseWorkflowIds(flags.workflow) ?? []; + } return body; } @@ -137,11 +148,6 @@ function validateMetricBody( body: Record, flags: EditFlags, ): void { - if (flags.trigger !== undefined) { - validateMetricTriggers( - body.triggers as Record[] | undefined, - ); - } if (flags.dataset !== undefined) { validateMetricDataset(normalizeMetricDataset(String(body.dataset))); } @@ -221,8 +227,7 @@ export const editCommand = buildCommand({ kind: "parsed", parse: String, optional: true, - brief: - "Dataset: errors (error-events), sessions, events, spans, metrics (transaction(s) routes to spans)", + brief: METRIC_ALERT_DATASET_BRIEF, }, "time-window": { kind: "parsed", @@ -256,6 +261,14 @@ export const editCommand = buildCommand({ optional: true, brief: "Owner value (pass empty string to clear)", }, + workflow: { + kind: "parsed", + parse: String, + variadic: true, + optional: true, + brief: + "Replace connected alert (workflow) ids (repeatable or comma-separated; empty string disconnects all)", + }, }, aliases: { t: "trigger", diff --git a/packages/cli/src/commands/alert/mutation-utils.ts b/packages/cli/src/commands/alert/mutation-utils.ts index 059bd59b8..97275e4c9 100644 --- a/packages/cli/src/commands/alert/mutation-utils.ts +++ b/packages/cli/src/commands/alert/mutation-utils.ts @@ -3,47 +3,13 @@ */ import { ValidationError } from "../../lib/errors.js"; +import { + METRIC_ALERT_DATASET_ALIASES, + METRIC_ALERT_DATASET_NAMES, + metricAlertSource, +} from "../../types/metric-alert.js"; const ISSUE_MATCH_MODES = new Set(["all", "any"]); -const METRIC_DATASET_VALUES = new Set([ - "errors", - "sessions", - "events", - "spans", - "metrics", -]); - -/** - * Maps user-provided dataset aliases to canonical metric alert dataset values. - * - * Only aliases that resolve to a value already accepted by the metric alert API - * are listed here: the singular forms (`error` → `errors`, `span` → `spans`) - * and the dashboard terminology for the error dataset (`error-events` → - * `errors`). This lets users copying from dashboard docs avoid a validation - * error without changing which dataset is sent. - * - * Names that denote a *distinct* dataset in the metric alert path — e.g. - * `tracemetrics`, `metricsenhanced`, `eap`, `events_analytics_platform` — are - * deliberately NOT aliased here. Rewriting them onto `metrics`/`spans` would - * silently send the wrong dataset in the create/edit payload (the CLI's - * canonical `metrics` is session/crash-rate, not trace metrics), with no - * validation error to catch it. - * - * The deprecated `transaction`/`transactions` datasets are handled separately - * by {@link resolveMetricDataset} — they route to `spans` with an added - * `is_transaction:true` filter rather than a plain rename, so they are not - * listed here. - */ -const METRIC_DATASET_ALIASES: Record = { - // Singular forms - error: "errors", - session: "sessions", - metric: "metrics", - span: "spans", - // Dashboard terminology for the error dataset - "error-events": "errors", -}; - /** Deprecated dataset names that now route to `spans` with `is_transaction:true`. */ const TRANSACTION_DATASET_NAMES = new Set(["transaction", "transactions"]); @@ -52,6 +18,8 @@ const IS_TRANSACTION_FILTER = "is_transaction:true"; /** Splits a query string on runs of whitespace. */ const QUERY_TOKEN_SEPARATOR = /\s+/; +/** Workflow (alert) ids are numeric. */ +const WORKFLOW_ID_PATTERN = /^\d+$/; const METRIC_TIME_WINDOWS = new Set([ 1, 5, 10, 15, 30, 60, 120, 240, 360, 720, 1440, ]); @@ -221,7 +189,7 @@ export function normalizeProjectList( */ export function normalizeMetricDataset(dataset: string): string { const lower = dataset.trim().toLowerCase(); - return METRIC_DATASET_ALIASES[lower] ?? lower; + return METRIC_ALERT_DATASET_ALIASES[lower] ?? lower; } /** @@ -277,12 +245,11 @@ export function resolveMetricDataset( /** Validate that `dataset` is one of the allowed Sentry metric alert dataset values. */ export function validateMetricDataset(dataset: string): void { - const normalized = normalizeMetricDataset(dataset); - if (METRIC_DATASET_VALUES.has(normalized)) { + if (metricAlertSource(normalizeMetricDataset(dataset))) { return; } throw new ValidationError( - `dataset must be one of: ${[...METRIC_DATASET_VALUES].join(", ")}.`, + `dataset must be one of: ${METRIC_ALERT_DATASET_NAMES.join(", ")}.`, "dataset", ); } @@ -298,33 +265,23 @@ export function validateMetricTimeWindow(timeWindow: number): void { ); } -/** Validate that each trigger has an `alertThreshold` and a non-empty `actions` array. */ -export function validateMetricTriggers( - triggers: readonly Record[] | undefined, -): void { - if (!triggers || triggers.length === 0) { - throw new ValidationError( - "Pass at least one --trigger JSON object.", - "trigger", - ); +/** Parse `--workflow` values (repeatable or comma-separated) into numeric workflow ids. */ +export function parseWorkflowIds( + values: readonly string[] | undefined, +): number[] | undefined { + const ids = normalizeProjectList(values); + if (!ids) { + return; } - - for (const trigger of triggers) { - const threshold = trigger.alertThreshold; - if (typeof threshold !== "number" && typeof threshold !== "string") { + return ids.map((id) => { + if (!WORKFLOW_ID_PATTERN.test(id)) { throw new ValidationError( - "Each trigger must include alertThreshold.", - "trigger", + `Workflow id must be numeric (got ${JSON.stringify(id)}).`, + "workflow", ); } - const actions = trigger.actions; - if (!Array.isArray(actions) || actions.length === 0) { - throw new ValidationError( - "Each trigger must include a non-empty actions array.", - "trigger", - ); - } - } + return Number(id); + }); } /** Map a human-readable status string to the numeric value the metric alert API expects. */ diff --git a/packages/cli/src/lib/api/alerts.ts b/packages/cli/src/lib/api/alerts.ts index 084c2edc9..74b0fdc14 100644 --- a/packages/cli/src/lib/api/alerts.ts +++ b/packages/cli/src/lib/api/alerts.ts @@ -16,6 +16,17 @@ import { updateOrganizationDetector, updateOrganizationWorkflow, } from "@sentry/api"; +import { + METRIC_MONITOR_PRIORITY, + METRIC_MONITOR_SOURCES, + type MetricMonitorSource, +} from "@sentry/toolkit-core/metric-monitor"; +import { + isDetectorConditionList, + type MetricTriggers, + metricAlertDatasetName, + metricAlertSource, +} from "../../types/metric-alert.js"; import { ApiError, ValidationError } from "../errors.js"; import { resolveOrgRegion } from "../region.js"; import { @@ -89,6 +100,8 @@ type MetricDetector = { owner?: { type: string; name?: string; id?: string } | string | null; dateCreated?: string; dataSources?: Record[] | null; + /** Detection settings (`detectionType`, `comparisonDelta`, …), kept on edit. */ + config?: Record | null; }; /** @@ -205,7 +218,10 @@ function mapDetectorToMetricAlertRule( status: detector.enabled === false ? 1 : 0, query: pickDetectorString(nested.query), aggregate: pickDetectorString(nested.aggregate), - dataset: pickDetectorString(nested.dataset), + dataset: metricAlertDatasetName( + pickDetectorString(nested.dataset), + Array.isArray(nested.eventTypes) ? nested.eventTypes : [], + ), timeWindow: timeWindowSeconds > 0 ? Math.round(timeWindowSeconds / 60) : 0, environment, owner: pickDetectorOwner(detector.owner), @@ -428,6 +444,14 @@ export async function getMetricAlertRule( orgSlug: string, ruleId: string, ): Promise { + return mapDetectorToMetricAlertRule(await getMetricDetector(orgSlug, ruleId)); +} + +/** Fetch a metric-issue detector, rejecting other detector kinds with a 404. */ +async function getMetricDetector( + orgSlug: string, + ruleId: string, +): Promise { const config = await getOrgSdkConfig(orgSlug); const result = await getOrganizationDetector({ ...config, @@ -453,7 +477,7 @@ export async function getMetricAlertRule( `/organizations/${orgSlug}/detectors/${encodeURIComponent(ruleId)}/`, ); } - return mapDetectorToMetricAlertRule(data); + return data; } // Issue alert write operations @@ -546,28 +570,70 @@ export async function createIssueAlertRule( // command layer unchanged while the transport moves off the retiring // `/alert-rules/` endpoint (getsentry/cli#1274, #1277). +/** Native data source fields sent on a detector write. */ +type DetectorSourceFields = Omit & { + readonly dataset: string; +}; + /** - * Datasets whose events flow through the error/transaction pipeline - * (`queryType: 0`) rather than the trace-item pipeline (`queryType: 1`). + * Resolve the native data source fields for a flat metric alert body. + * + * CLI dataset names come from the shared dataset table. Any other (native) + * dataset comes from the edit baseline built by `getMetricAlertRuleDocument`, + * which carries the stored `eventTypes`/`queryType`. Missing event types stay + * empty rather than defaulting to spans, so an edit never re-targets an + * ambiguous EAP monitor onto `trace_item_span`. */ -const ERROR_LIKE_DATASETS = new Set(["errors", "events"]); - -/** Detector `queryType` for a metric alert dataset. */ -function datasetQueryType(dataset: string): 0 | 1 { - return ERROR_LIKE_DATASETS.has(dataset.trim().toLowerCase()) ? 0 : 1; +function detectorSourceFields( + body: Record, +): DetectorSourceFields { + const dataset = String(body.dataset ?? ""); + const named = metricAlertSource(dataset); + if (named) { + return named; + } + const stored = body as Partial; + return { + queryType: stored.queryType ?? METRIC_MONITOR_SOURCES.spans.queryType, + eventTypes: stored.eventTypes ?? [], + dataset, + }; } /** - * Default `eventTypes` for a metric alert dataset. + * Translate parsed triggers into detector condition-group conditions. * - * Error-like datasets track `error`/`default` events; everything else measures - * spans. The backend accepts an explicit `eventTypes` on the data source and - * derives the internal query type from `queryType`. + * Threshold triggers become one threshold condition each plus the single + * resolution condition the metric detector validator requires. Resolution uses + * the first `resolveThreshold` given, else the critical (or first) alert + * threshold. Native conditions pass through untouched. */ -function datasetEventTypes(dataset: string): string[] { - return ERROR_LIKE_DATASETS.has(dataset.trim().toLowerCase()) - ? ["error", "default"] - : ["trace_item_span"]; +function buildDetectorConditions( + triggers: MetricTriggers, +): readonly Record[] { + if (isDetectorConditionList(triggers)) { + return triggers; + } + + const below = triggers.some((trigger) => trigger.thresholdType === 1); + const conditions: Record[] = triggers.map((trigger) => ({ + type: below ? "lt" : "gt", + comparison: trigger.alertThreshold, + conditionResult: METRIC_MONITOR_PRIORITY[trigger.label ?? "critical"], + })); + + const critical = + triggers.find((trigger) => (trigger.label ?? "critical") === "critical") ?? + triggers[0]; + const resolveThreshold = + triggers.find((trigger) => trigger.resolveThreshold != null) + ?.resolveThreshold ?? critical?.alertThreshold; + conditions.push({ + type: below ? "gte" : "lte", + comparison: resolveThreshold, + conditionResult: METRIC_MONITOR_PRIORITY.resolved, + }); + return conditions; } /** @@ -580,15 +646,15 @@ function datasetEventTypes(dataset: string): string[] { function buildDetectorDataSource( body: Record, ): Record { - const dataset = typeof body.dataset === "string" ? body.dataset : ""; + const native = detectorSourceFields(body); const timeWindowMinutes = typeof body.timeWindow === "number" ? body.timeWindow : 0; const source: Record = { aggregate: typeof body.aggregate === "string" ? body.aggregate : "", - dataset, + dataset: native.dataset, query: typeof body.query === "string" ? body.query : "", - queryType: datasetQueryType(dataset), - eventTypes: datasetEventTypes(dataset), + queryType: native.queryType, + eventTypes: [...native.eventTypes], timeWindow: timeWindowMinutes * 60, }; if (typeof body.environment === "string" && body.environment !== "") { @@ -597,32 +663,48 @@ function buildDetectorDataSource( return source; } +function isDetectorConfig(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + /** * Translate a flat metric alert body into the nested detector write payload. * * The flat body is what the create/edit commands assemble (and the edit merge - * baseline, which reuses the `MetricAlertRule` shape). Trigger objects are - * passed through verbatim as the condition group's conditions — their - * `alertThreshold`/`actions` shape is already validated command-side. `status` - * (0 active / 1 disabled) maps onto the detector's `enabled` boolean. + * baseline, which reuses the `MetricAlertRule` shape). Triggers are translated + * into detector conditions by {@link buildDetectorConditions}; `workflowIds` + * connects existing alerts. `status` (0 active / 1 disabled) maps onto the + * detector's `enabled` boolean. */ function buildDetectorBody( body: Record, ): Record { + // create/edit store triggers already parsed by parseMetricTriggers. + const triggers = body.triggers as MetricTriggers | undefined; + // Threshold triggers are static thresholds. Otherwise edits carry the stored + // config so percent and dynamic detectors keep their detection mode, and + // creates default to static. + const keepStoredConfig = + isDetectorConfig(body.detectorConfig) && + !(triggers && !isDetectorConditionList(triggers)); const detectorBody: Record = { name: typeof body.name === "string" ? body.name : "", type: "metric_issue", dataSources: [buildDetectorDataSource(body)], - config: { detectionType: "static" }, + config: keepStoredConfig + ? body.detectorConfig + : { detectionType: "static" }, }; - const triggers = Array.isArray(body.triggers) ? body.triggers : undefined; if (triggers) { detectorBody.conditionGroup = { logicType: "any", - conditions: triggers, + conditions: buildDetectorConditions(triggers), }; } + if (Array.isArray(body.workflowIds)) { + detectorBody.workflowIds = body.workflowIds; + } if (body.status !== undefined) { detectorBody.enabled = Number(body.status) !== 1; @@ -691,13 +773,30 @@ export async function deleteMetricAlertRule( * `MetricAlertRule` shape, then hands the merged body to `putMetricAlertRule`, * which translates it into the nested detector document. Returning the flat * shape (rather than the raw detector) keeps the command-side merge coherent. + * The stored `eventTypes`/`queryType` ride along so an edit of a data source + * the CLI does not model (e.g. `trace_item_metric`) keeps its signal, and the + * stored detector `config` rides along as `detectorConfig` so an edit keeps a + * percent or dynamic detector's detection mode. */ export async function getMetricAlertRuleDocument( orgSlug: string, ruleId: string, ): Promise> { - const rule = await getMetricAlertRule(orgSlug, ruleId); - return { ...rule }; + const detector = await getMetricDetector(orgSlug, ruleId); + const source = resolveThresholdSource(detector.dataSources?.[0] ?? {}); + const document: Record = { + ...mapDetectorToMetricAlertRule(detector), + }; + if (Array.isArray(source.eventTypes)) { + document.eventTypes = source.eventTypes; + } + if (typeof source.queryType === "number") { + document.queryType = source.queryType; + } + if (isDetectorConfig(detector.config)) { + document.detectorConfig = detector.config; + } + return document; } /** diff --git a/packages/cli/src/types/metric-alert.ts b/packages/cli/src/types/metric-alert.ts new file mode 100644 index 000000000..4c1d9a4a2 --- /dev/null +++ b/packages/cli/src/types/metric-alert.ts @@ -0,0 +1,215 @@ +/** + * Metric alert types and schemas + * + * The CLI `--dataset` vocabulary for `sentry alert metrics`, mapped onto the + * shared metric monitor data sources, plus Valibot schemas for `--trigger`. + * Every dataset list, help string, and lookup is derived from + * {@link METRIC_ALERT_DATASETS} so they cannot diverge. + */ + +import { + METRIC_MONITOR_PRIORITY, + METRIC_MONITOR_SOURCES, + METRIC_MONITOR_THRESHOLD_TYPES, + type MetricMonitorSource, +} from "@sentry/toolkit-core/metric-monitor"; +import { + array, + finite, + getDotPath, + type InferOutput, + looseObject, + maxLength, + nullable, + number, + object, + optional, + picklist, + pipe, + record, + safeParse, + string, + transform, + union, + unknown, +} from "valibot"; + +import { ValidationError } from "../lib/errors.js"; + +// --------------------------------------------------------------------------- +// Datasets +// --------------------------------------------------------------------------- + +/** + * CLI dataset names and the detector data source each selects. + * + * Key order drives help text and, for shared sources, which name a stored + * monitor reads back as (`errors` over `events`, `metrics` over `sessions`). + */ +export const METRIC_ALERT_DATASETS = { + errors: METRIC_MONITOR_SOURCES.errors, + events: METRIC_MONITOR_SOURCES.errors, + spans: METRIC_MONITOR_SOURCES.spans, + logs: METRIC_MONITOR_SOURCES.logs, + metrics: METRIC_MONITOR_SOURCES.crashRate, + sessions: METRIC_MONITOR_SOURCES.crashRate, +} as const satisfies Record; + +export type MetricAlertDataset = keyof typeof METRIC_ALERT_DATASETS; + +export const METRIC_ALERT_DATASET_NAMES = Object.keys( + METRIC_ALERT_DATASETS, +) as MetricAlertDataset[]; + +/** + * Aliases for {@link METRIC_ALERT_DATASETS} names. + * + * Only singular forms and the dashboard name for errors (`error-events`) are + * aliased. Names of *distinct* datasets — `tracemetrics`, `eap`, + * `events_analytics_platform` — are deliberately not, because rewriting them + * onto `metrics`/`spans` would silently send the wrong data source (the CLI's + * `metrics` is crash rate, not trace metrics). `transaction(s)` is routed to + * `spans` with an `is_transaction:true` filter by `resolveMetricDataset`. + */ +export const METRIC_ALERT_DATASET_ALIASES: Record = + { + error: "errors", + "error-events": "errors", + span: "spans", + log: "logs", + metric: "metrics", + session: "sessions", + }; + +/** `--dataset` flag help, derived from the dataset table. */ +export const METRIC_ALERT_DATASET_BRIEF = `Dataset: ${METRIC_ALERT_DATASET_NAMES.join(", ")} (error-events is errors; transaction(s) routes to spans)`; + +/** Data source for a CLI dataset name, or `undefined` for other names. */ +export function metricAlertSource( + dataset: string, +): MetricMonitorSource | undefined { + const name = dataset.trim().toLowerCase(); + return Object.hasOwn(METRIC_ALERT_DATASETS, name) + ? METRIC_ALERT_DATASETS[name as MetricAlertDataset] + : undefined; +} + +/** + * CLI dataset name for a stored data source, or the native dataset when no + * CLI name selects it (e.g. EAP `trace_item_metric`). + * + * Missing or empty event types only resolve to a CLI name when every CLI name + * on that native dataset selects the same source; EAP is shared by spans, logs, + * and trace metrics, so it stays native rather than being guessed as `spans`. + */ +export function metricAlertDatasetName( + dataset: string, + eventTypes: readonly string[], +): string { + const candidates = METRIC_ALERT_DATASET_NAMES.filter( + (name) => METRIC_ALERT_DATASETS[name].dataset === dataset, + ); + if (eventTypes.length === 0) { + const sources = new Set( + candidates.map((name) => METRIC_ALERT_DATASETS[name]), + ); + return sources.size === 1 && candidates[0] ? candidates[0] : dataset; + } + const match = candidates.find((name) => { + const source: MetricMonitorSource = METRIC_ALERT_DATASETS[name]; + return eventTypes.every((type) => source.eventTypes.includes(type)); + }); + return match ?? dataset; +} + +// --------------------------------------------------------------------------- +// Triggers +// --------------------------------------------------------------------------- + +/** A threshold given as a number or numeric string. */ +const ThresholdSchema = pipe( + union([number(), string()]), + transform(Number), + number(), + finite(), +); + +const ACTIONS_MESSAGE = + "Metric alerts no longer carry notification actions on triggers. Remove 'actions' and connect an existing alert with --workflow ."; + +/** + * Legacy-style trigger: alert above (`thresholdType` 0, default) or below (1) + * `alertThreshold`. Notification actions are rejected because detectors do + * not accept them and would drop them silently. + */ +const ThresholdTriggerSchema = object({ + alertThreshold: ThresholdSchema, + label: optional(picklist(["critical", "warning"])), + resolveThreshold: optional(nullable(ThresholdSchema)), + thresholdType: optional(picklist([0, 1])), + actions: optional(pipe(array(unknown()), maxLength(0, ACTIONS_MESSAGE))), +}); + +/** Native detector condition, passed to the API as-is. */ +const DetectorConditionSchema = looseObject({ + type: picklist([...METRIC_MONITOR_THRESHOLD_TYPES, "anomaly_detection"]), + comparison: union([number(), record(string(), unknown())]), + conditionResult: picklist(Object.values(METRIC_MONITOR_PRIORITY)), +}); + +const ThresholdTriggersSchema = array(ThresholdTriggerSchema); +const DetectorConditionsSchema = array(DetectorConditionSchema); + +export type ThresholdTrigger = InferOutput; +export type DetectorCondition = InferOutput; + +/** Parsed `--trigger` values: all thresholds or all native conditions. */ +export type MetricTriggers = ThresholdTrigger[] | DetectorCondition[]; + +/** Whether parsed triggers are native detector conditions. */ +export function isDetectorConditionList( + triggers: MetricTriggers, +): triggers is DetectorCondition[] { + return triggers.some((trigger) => "type" in trigger); +} + +/** + * Parse and validate `--trigger` JSON objects. + * + * Triggers must be all threshold triggers or all native detector conditions + * (shape chosen by the presence of `type`). + * + * @throws {ValidationError} with the offending path and allowed values + */ +export function parseMetricTriggers( + raw: readonly Record[] | undefined, +): MetricTriggers { + if (!raw || raw.length === 0) { + throw new ValidationError( + "Pass at least one --trigger JSON object.", + "trigger", + ); + } + const nativeCount = raw.filter((trigger) => "type" in trigger).length; + if (nativeCount > 0 && nativeCount < raw.length) { + throw new ValidationError( + "Do not mix detector conditions (with 'type') and alertThreshold triggers.", + "trigger", + ); + } + + const result = + nativeCount > 0 + ? safeParse(DetectorConditionsSchema, raw) + : safeParse(ThresholdTriggersSchema, raw); + if (result.success) { + return result.output; + } + const issues = result.issues.map( + (issue) => `${getDotPath(issue) ?? "trigger"}: ${issue.message}`, + ); + throw new ValidationError( + `Invalid --trigger:\n${issues.join("\n")}`, + "trigger", + ); +} diff --git a/packages/cli/test/commands/alert/metrics/create.test.ts b/packages/cli/test/commands/alert/metrics/create.test.ts index a0692b72e..5d2f11484 100644 --- a/packages/cli/test/commands/alert/metrics/create.test.ts +++ b/packages/cli/test/commands/alert/metrics/create.test.ts @@ -74,7 +74,7 @@ describe("alert metrics create", () => { aggregate: "count()", dataset: "unknown", "time-window": 5, - trigger: ['{"alertThreshold":100,"actions":[{"id":"notify"}]}'], + trigger: ['{"alertThreshold":100}'], "dry-run": true, json: true, }, @@ -113,7 +113,7 @@ describe("alert metrics create", () => { aggregate: overrides.aggregate, dataset: "errors", "time-window": 5, - trigger: ['{"alertThreshold":100,"actions":[{"id":"notify"}]}'], + trigger: ['{"alertThreshold":100}'], "dry-run": true, json: true, }, @@ -139,7 +139,7 @@ describe("alert metrics create", () => { aggregate: "count()", dataset: "errors", "time-window": 5, - trigger: ['{"alertThreshold":100,"actions":[{"id":"notify"}]}'], + trigger: ['{"alertThreshold":100}'], "dry-run": true, json: true, }, @@ -164,7 +164,7 @@ describe("alert metrics create", () => { aggregate: "count()", dataset: "errors", "time-window": 5, - trigger: ['{"alertThreshold":100,"actions":[{"id":"notify"}]}'], + trigger: ['{"alertThreshold":100}'], project: ["backend"], "dry-run": true, json: true, @@ -206,7 +206,7 @@ describe("alert metrics create", () => { aggregate: "count()", dataset: "errors", "time-window": 5, - trigger: ['{"alertThreshold":100,"actions":[{"id":"notify"}]}'], + trigger: ['{"alertThreshold":100}'], project: ["backend"], "dry-run": true, json: true, @@ -236,7 +236,7 @@ describe("alert metrics create", () => { aggregate: "count()", dataset: "errors", "time-window": 5, - trigger: ['{"alertThreshold":100,"actions":[{"id":"notify"}]}'], + trigger: ['{"alertThreshold":100}'], project: ["backend"], "dry-run": true, json: true, @@ -264,7 +264,7 @@ describe("alert metrics create", () => { aggregate: "count()", dataset: "errors", "time-window": 5, - trigger: ['{"alertThreshold":100,"actions":[{"id":"notify"}]}'], + trigger: ['{"alertThreshold":100}'], project: ["frontend, backend", "api"], environment: "prod", owner: "team:ops", @@ -287,7 +287,7 @@ describe("alert metrics create", () => { aggregate: "count()", dataset: "errors", timeWindow: 5, - triggers: [{ alertThreshold: 100, actions: [{ id: "notify" }] }], + triggers: [{ alertThreshold: 100 }], projects: ["frontend", "backend", "api"], environment: "prod", owner: "team:ops", @@ -317,7 +317,7 @@ describe("alert metrics create", () => { aggregate: "count()", dataset: "errors", "time-window": 5, - trigger: ['{"alertThreshold":100,"actions":[{"id":"notify"}]}'], + trigger: ['{"alertThreshold":100}'], project: ["backend"], "dry-run": false, json: true, @@ -331,7 +331,7 @@ describe("alert metrics create", () => { aggregate: "count()", dataset: "errors", timeWindow: 5, - triggers: [{ alertThreshold: 100, actions: [{ id: "notify" }] }], + triggers: [{ alertThreshold: 100 }], projects: ["backend"], }); }); @@ -357,7 +357,7 @@ describe("alert metrics create", () => { aggregate: "p95(span.duration)", dataset: "transactions", "time-window": 5, - trigger: ['{"alertThreshold":100,"actions":[{"id":"notify"}]}'], + trigger: ['{"alertThreshold":100}'], project: ["backend"], "dry-run": false, json: true, @@ -371,7 +371,7 @@ describe("alert metrics create", () => { aggregate: "p95(span.duration)", dataset: "spans", timeWindow: 5, - triggers: [{ alertThreshold: 100, actions: [{ id: "notify" }] }], + triggers: [{ alertThreshold: 100 }], projects: ["backend"], }); expect(context.stderr.write).toHaveBeenCalledWith( diff --git a/packages/cli/test/commands/alert/metrics/edit.test.ts b/packages/cli/test/commands/alert/metrics/edit.test.ts index c3134f012..3aed4abac 100644 --- a/packages/cli/test/commands/alert/metrics/edit.test.ts +++ b/packages/cli/test/commands/alert/metrics/edit.test.ts @@ -90,7 +90,7 @@ describe("alert metrics edit", () => { aggregate: "count()", dataset: "errors", timeWindow: 5, - triggers: [{ alertThreshold: 100, actions: [{ id: "notify" }] }], + triggers: [{ alertThreshold: 100 }], }); putSpy.mockResolvedValue({ id: "9", @@ -100,7 +100,7 @@ describe("alert metrics edit", () => { aggregate: "count()", dataset: "spans", timeWindow: 15, - triggers: [{ alertThreshold: 200, actions: [{ id: "notify" }] }], + triggers: [{ alertThreshold: 200 }], }); const func = (await editCommand.loader()) as unknown as ( this: unknown, @@ -116,7 +116,7 @@ describe("alert metrics edit", () => { aggregate: "count()", dataset: "spans", "time-window": 15, - trigger: ['{"alertThreshold":200,"actions":[{"id":"notify"}]}'], + trigger: ['{"alertThreshold":200}'], json: true, }, "test-org/9", @@ -130,7 +130,7 @@ describe("alert metrics edit", () => { aggregate: "count()", dataset: "spans", timeWindow: 15, - triggers: [{ alertThreshold: 200, actions: [{ id: "notify" }] }], + triggers: [{ alertThreshold: 200 }], }); }); @@ -199,7 +199,7 @@ describe("alert metrics edit", () => { aggregate: "count()", dataset: "errors", timeWindow: 5, - triggers: [{ alertThreshold: 100, actions: [{ id: "notify" }] }], + triggers: [{ alertThreshold: 100 }], }); putSpy.mockResolvedValue({ id: "9", @@ -209,7 +209,7 @@ describe("alert metrics edit", () => { aggregate: "count()", dataset: "errors", timeWindow: 5, - triggers: [{ alertThreshold: 100, actions: [{ id: "notify" }] }], + triggers: [{ alertThreshold: 100 }], }); const func = (await editCommand.loader()) as unknown as ( this: unknown, @@ -245,7 +245,7 @@ describe("alert metrics edit", () => { aggregate: "count()", dataset: "errors", timeWindow: 5, - triggers: [{ alertThreshold: 100, actions: [{ id: "notify" }] }], + triggers: [{ alertThreshold: 100 }], }); putSpy.mockResolvedValue({ id: "9", @@ -255,7 +255,7 @@ describe("alert metrics edit", () => { aggregate: "count()", dataset: "spans", timeWindow: 5, - triggers: [{ alertThreshold: 100, actions: [{ id: "notify" }] }], + triggers: [{ alertThreshold: 100 }], }); const func = (await editCommand.loader()) as unknown as ( this: unknown, diff --git a/packages/cli/test/commands/alert/mutation-utils.test.ts b/packages/cli/test/commands/alert/mutation-utils.test.ts index 9c1ab5f1b..7089b3060 100644 --- a/packages/cli/test/commands/alert/mutation-utils.test.ts +++ b/packages/cli/test/commands/alert/mutation-utils.test.ts @@ -12,7 +12,7 @@ import { validateIssueRuleArrays, validateMetricDataset, validateMetricTimeWindow, - validateMetricTriggers, + parseWorkflowIds, } from "../../../src/commands/alert/mutation-utils.js"; import { ValidationError } from "../../../src/lib/errors.js"; @@ -291,7 +291,7 @@ describe("resolveMetricDataset", () => { }); describe("validateMetricDataset", () => { - const valid = ["errors", "sessions", "events", "spans", "metrics"]; + const valid = ["errors", "sessions", "events", "spans", "logs", "metrics"]; for (const dataset of valid) { test(`passes for "${dataset}"`, () => { @@ -299,7 +299,7 @@ describe("validateMetricDataset", () => { }); } - test.each(["error", "error-events", "METRIC", "span"])( + test.each(["error", "error-events", "METRIC", "span", "log"])( 'passes for alias "%s"', (dataset) => { expect(() => validateMetricDataset(dataset)).not.toThrow(); @@ -345,58 +345,17 @@ describe("validateMetricTimeWindow", () => { }); }); -describe("validateMetricTriggers", () => { - test("throws for undefined triggers", () => { - try { - validateMetricTriggers(undefined); - expect.unreachable("should have thrown"); - } catch (error) { - expect(error).toBeInstanceOf(ValidationError); - } +describe("parseWorkflowIds", () => { + test("returns undefined when absent", () => { + expect(parseWorkflowIds(undefined)).toBeUndefined(); }); - test("throws for empty triggers", () => { - try { - validateMetricTriggers([]); - expect.unreachable("should have thrown"); - } catch (error) { - expect(error).toBeInstanceOf(ValidationError); - } - }); - - test("throws for missing alertThreshold", () => { - try { - validateMetricTriggers([{ actions: [{ id: "a" }] }]); - expect.unreachable("should have thrown"); - } catch (error) { - expect(error).toBeInstanceOf(ValidationError); - } + test("splits repeatable and comma-separated ids", () => { + expect(parseWorkflowIds(["12,34", "56"])).toEqual([12, 34, 56]); }); - test("throws for missing actions array", () => { - try { - validateMetricTriggers([{ alertThreshold: 100 }]); - expect.unreachable("should have thrown"); - } catch (error) { - expect(error).toBeInstanceOf(ValidationError); - } - }); - - test("throws for empty actions array", () => { - try { - validateMetricTriggers([{ alertThreshold: 100, actions: [] }]); - expect.unreachable("should have thrown"); - } catch (error) { - expect(error).toBeInstanceOf(ValidationError); - } - }); - - test("passes for valid trigger", () => { - expect(() => - validateMetricTriggers([ - { alertThreshold: 100, actions: [{ type: "email" }] }, - ]), - ).not.toThrow(); + test("rejects non-numeric ids", () => { + expect(() => parseWorkflowIds(["abc"])).toThrow(ValidationError); }); }); diff --git a/packages/cli/test/lib/api/alerts.test.ts b/packages/cli/test/lib/api/alerts.test.ts index 11397a5a9..2b226d90b 100644 --- a/packages/cli/test/lib/api/alerts.test.ts +++ b/packages/cli/test/lib/api/alerts.test.ts @@ -239,6 +239,46 @@ describe("getMetricAlertRule", () => { expect(rule.timeWindow).toBe(10); }); + test.each([ + ["events", ["error", "default"], "errors"], + ["events_analytics_platform", ["trace_item_span"], "spans"], + ["events_analytics_platform", ["trace_item_log"], "logs"], + [ + "events_analytics_platform", + ["trace_item_metric"], + "events_analytics_platform", + ], + ["events_analytics_platform", [], "events_analytics_platform"], + ["metrics", [], "metrics"], + ])( + "maps native dataset %s with eventTypes %j to CLI dataset %s", + async (dataset, eventTypes, expected) => { + globalThis.fetch = mockFetch(async () => + Response.json( + metricDetector({ + dataSources: [ + { + type: "snuba_query_subscription", + queryObj: { + snubaQuery: { + aggregate: "count()", + dataset, + eventTypes, + query: "", + timeWindow: 300, + }, + }, + }, + ], + }), + ), + ); + + const rule = await getMetricAlertRule("test-org", "9"); + expect(rule.dataset).toBe(expected); + }, + ); + test("prefers projectSlug and falls back to a projects array for projects", async () => { globalThis.fetch = mockFetch(async () => Response.json( @@ -394,7 +434,7 @@ describe("createMetricAlertRule", () => { dataSources: [ { aggregate: "p95(span.duration)", - dataset: "spans", + dataset: "events_analytics_platform", query: "environment:prod", queryType: 1, eventTypes: ["trace_item_span"], @@ -406,7 +446,11 @@ describe("createMetricAlertRule", () => { config: { detectionType: "static" }, conditionGroup: { logicType: "any", - conditions: [{ alertThreshold: 500, actions: [{ id: "notify" }] }], + // The legacy trigger becomes a threshold plus a resolution condition. + conditions: [ + { type: "gt", comparison: 500, conditionResult: 75 }, + { type: "lte", comparison: 500, conditionResult: 0 }, + ], }, }); return Response.json( @@ -422,7 +466,7 @@ describe("createMetricAlertRule", () => { dataset: "spans", timeWindow: 5, environment: "prod", - triggers: [{ alertThreshold: 500, actions: [{ id: "notify" }] }], + triggers: [{ alertThreshold: 500 }], projects: ["backend"], }); expect(created.id).toBe("77"); @@ -444,16 +488,146 @@ describe("createMetricAlertRule", () => { aggregate: "count()", dataset: "errors", timeWindow: 15, - triggers: [{ alertThreshold: 100, actions: [{ id: "notify" }] }], + triggers: [{ alertThreshold: 100 }], projects: ["backend"], }); const source = (sentBody.dataSources as Record[])[0]; + expect(source?.dataset).toBe("events"); expect(source?.queryType).toBe(0); expect(source?.eventTypes).toEqual(["error", "default"]); expect(source?.timeWindow).toBe(900); }); + test("maps logs to the EAP dataset with trace_item_log and connects workflows", async () => { + let sentBody: Record = {}; + globalThis.fetch = mockFetch(async (input, init) => { + sentBody = (await new Request(input!, init).json()) as Record< + string, + unknown + >; + return Response.json(metricDetector({ id: "1" }), { status: 201 }); + }); + + await createMetricAlertRule("test-org", { + name: "AI chat failures", + query: "action:ai.chat result:error", + aggregate: "count()", + dataset: "logs", + timeWindow: 5, + triggers: [{ alertThreshold: 0, resolveThreshold: 0 }], + projects: ["backend"], + workflowIds: [42], + }); + + expect(sentBody.dataSources).toEqual([ + { + aggregate: "count()", + dataset: "events_analytics_platform", + query: "action:ai.chat result:error", + queryType: 1, + eventTypes: ["trace_item_log"], + timeWindow: 300, + }, + ]); + expect(sentBody.conditionGroup).toEqual({ + logicType: "any", + conditions: [ + { type: "gt", comparison: 0, conditionResult: 75 }, + { type: "lte", comparison: 0, conditionResult: 0 }, + ], + }); + expect(sentBody.workflowIds).toEqual([42]); + }); + + test("maps sessions to the crash-rate metrics dataset", async () => { + let sentBody: Record = {}; + globalThis.fetch = mockFetch(async (input, init) => { + sentBody = (await new Request(input!, init).json()) as Record< + string, + unknown + >; + return Response.json(metricDetector({ id: "1" }), { status: 201 }); + }); + + await createMetricAlertRule("test-org", { + name: "Crash rate", + query: "", + aggregate: + "percentage(sessions_crashed, sessions) AS _crash_rate_alert_aggregate", + dataset: "sessions", + timeWindow: 60, + triggers: [{ alertThreshold: 1 }], + projects: ["backend"], + }); + + const source = (sentBody.dataSources as Record[])[0]; + expect(source?.dataset).toBe("metrics"); + expect(source?.queryType).toBe(2); + expect(source?.eventTypes).toEqual([]); + }); + + test("translates warning and below-threshold triggers", async () => { + let sentBody: Record = {}; + globalThis.fetch = mockFetch(async (input, init) => { + sentBody = (await new Request(input!, init).json()) as Record< + string, + unknown + >; + return Response.json(metricDetector({ id: "1" }), { status: 201 }); + }); + + await createMetricAlertRule("test-org", { + name: "Low throughput", + query: "", + aggregate: "count()", + dataset: "spans", + timeWindow: 5, + triggers: [ + { alertThreshold: 10, label: "critical", thresholdType: 1 }, + { alertThreshold: 50, label: "warning", thresholdType: 1 }, + ], + projects: ["backend"], + }); + + expect( + (sentBody.conditionGroup as Record).conditions, + ).toEqual([ + { type: "lt", comparison: 10, conditionResult: 75 }, + { type: "lt", comparison: 50, conditionResult: 50 }, + { type: "gte", comparison: 10, conditionResult: 0 }, + ]); + }); + + test("passes native detector conditions through unchanged", async () => { + let sentBody: Record = {}; + globalThis.fetch = mockFetch(async (input, init) => { + sentBody = (await new Request(input!, init).json()) as Record< + string, + unknown + >; + return Response.json(metricDetector({ id: "1" }), { status: 201 }); + }); + + const conditions = [ + { type: "gte", comparison: 5, conditionResult: 75 }, + { type: "lt", comparison: 5, conditionResult: 0 }, + ]; + await createMetricAlertRule("test-org", { + name: "Native", + query: "", + aggregate: "count()", + dataset: "errors", + timeWindow: 5, + triggers: conditions, + projects: ["backend"], + }); + + expect( + (sentBody.conditionGroup as Record).conditions, + ).toEqual(conditions); + }); + test("throws ValidationError when no project is provided", async () => { await expect( createMetricAlertRule("test-org", { @@ -462,7 +636,7 @@ describe("createMetricAlertRule", () => { aggregate: "count()", dataset: "errors", timeWindow: 5, - triggers: [{ alertThreshold: 1, actions: [{ id: "notify" }] }], + triggers: [{ alertThreshold: 1 }], }), ).rejects.toMatchObject({ name: "ValidationError" }); }); @@ -487,6 +661,171 @@ describe("getMetricAlertRuleDocument", () => { }); }); +describe("metric alert edit round-trip", () => { + test("keeps the stored eventTypes of an unmodelled EAP monitor", async () => { + const storedSource = { + type: "snuba_query_subscription", + queryObj: { + snubaQuery: { + aggregate: "sum(value,my.metric,counter,-)", + dataset: "events_analytics_platform", + eventTypes: ["trace_item_metric"], + queryType: 1, + query: "", + timeWindow: 300, + }, + }, + }; + let putSource: Record | undefined; + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input!, init); + if (req.method === "PUT") { + const body = (await req.json()) as Record; + putSource = (body.dataSources as Record[])[0]; + } + return Response.json(metricDetector({ dataSources: [storedSource] })); + }); + + const doc = await getMetricAlertRuleDocument("test-org", "9"); + await putMetricAlertRule("test-org", "9", { ...doc, name: "Renamed" }); + + expect(putSource).toMatchObject({ + dataset: "events_analytics_platform", + eventTypes: ["trace_item_metric"], + queryType: 1, + }); + }); +}); + +describe("metric alert edit round-trip without event types", () => { + test.each([ + ["empty", { eventTypes: [] }], + ["missing", {}], + ])( + "does not rewrite an EAP monitor with %s eventTypes onto spans", + async (_label, eventTypes) => { + const storedSource = { + type: "snuba_query_subscription", + queryObj: { + snubaQuery: { + aggregate: "count()", + dataset: "events_analytics_platform", + ...eventTypes, + queryType: 1, + query: "", + timeWindow: 300, + }, + }, + }; + let putSource: Record | undefined; + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input!, init); + if (req.method === "PUT") { + const body = (await req.json()) as Record; + putSource = (body.dataSources as Record[])[0]; + } + return Response.json(metricDetector({ dataSources: [storedSource] })); + }); + + const doc = await getMetricAlertRuleDocument("test-org", "9"); + expect(doc.dataset).toBe("events_analytics_platform"); + await putMetricAlertRule("test-org", "9", { ...doc, name: "Renamed" }); + + expect(putSource).toMatchObject({ + dataset: "events_analytics_platform", + eventTypes: [], + queryType: 1, + }); + }, + ); +}); + +describe("metric alert edit round-trip of detector config", () => { + test("keeps a percent detector's config on a rename", async () => { + const storedConfig = { detectionType: "percent", comparisonDelta: 3600 }; + let putConfig: unknown; + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input!, init); + if (req.method === "PUT") { + const body = (await req.json()) as Record; + putConfig = body.config; + } + return Response.json(metricDetector({ config: storedConfig })); + }); + + const doc = await getMetricAlertRuleDocument("test-org", "9"); + await putMetricAlertRule("test-org", "9", { ...doc, name: "Renamed" }); + + expect(putConfig).toEqual(storedConfig); + }); + + test.each([ + ["percent", { detectionType: "percent", comparisonDelta: 3600 }], + ["dynamic", { detectionType: "dynamic" }], + ])( + "switches a %s detector to static for threshold triggers", + async (_label, storedConfig) => { + let putConfig: unknown; + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input!, init); + if (req.method === "PUT") { + const body = (await req.json()) as Record; + putConfig = body.config; + } + return Response.json(metricDetector({ config: storedConfig })); + }); + + const doc = await getMetricAlertRuleDocument("test-org", "9"); + await putMetricAlertRule("test-org", "9", { + ...doc, + triggers: [{ alertThreshold: 100 }], + }); + + expect(putConfig).toEqual({ detectionType: "static" }); + }, + ); + + test("keeps the stored config for native detector conditions", async () => { + const storedConfig = { detectionType: "percent", comparisonDelta: 3600 }; + let putConfig: unknown; + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input!, init); + if (req.method === "PUT") { + const body = (await req.json()) as Record; + putConfig = body.config; + } + return Response.json(metricDetector({ config: storedConfig })); + }); + + const doc = await getMetricAlertRuleDocument("test-org", "9"); + await putMetricAlertRule("test-org", "9", { + ...doc, + triggers: [ + { type: "gt", comparison: 110, conditionResult: 75 }, + { type: "lte", comparison: 105, conditionResult: 0 }, + ], + }); + + expect(putConfig).toEqual(storedConfig); + }); + + test("defaults to static detection without a stored config", async () => { + let putConfig: unknown; + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input!, init); + if (req.method === "PUT") { + const body = (await req.json()) as Record; + putConfig = body.config; + } + return Response.json(metricDetector({})); + }); + + await putMetricAlertRule("test-org", "9", { name: "Renamed" }); + + expect(putConfig).toEqual({ detectionType: "static" }); + }); +}); + describe("putMetricAlertRule", () => { test("PUTs a nested detector body to the /detectors/{id}/ endpoint", async () => { globalThis.fetch = mockFetch(async (input, init) => { @@ -510,7 +849,7 @@ describe("putMetricAlertRule", () => { dataset: "spans", timeWindow: 5, status: 1, - triggers: [{ alertThreshold: 500, actions: [{ id: "notify" }] }], + triggers: [{ alertThreshold: 500 }], }); expect(updated.id).toBe("9"); }); diff --git a/packages/cli/test/types/metric-alert.test.ts b/packages/cli/test/types/metric-alert.test.ts new file mode 100644 index 000000000..f918ed4d3 --- /dev/null +++ b/packages/cli/test/types/metric-alert.test.ts @@ -0,0 +1,147 @@ +import { describe, expect, test } from "vitest"; +import { ValidationError } from "../../src/lib/errors.js"; +import { + METRIC_ALERT_DATASET_BRIEF, + METRIC_ALERT_DATASET_NAMES, + metricAlertDatasetName, + metricAlertSource, + parseMetricTriggers, +} from "../../src/types/metric-alert.js"; + +describe("metric alert datasets", () => { + test("help text lists every dataset name", () => { + for (const name of METRIC_ALERT_DATASET_NAMES) { + expect(METRIC_ALERT_DATASET_BRIEF).toContain(name); + } + }); + + test("every dataset name reads back as a dataset name", () => { + for (const name of METRIC_ALERT_DATASET_NAMES) { + const source = metricAlertSource(name); + expect(source).toBeDefined(); + const readBack = metricAlertDatasetName( + source!.dataset, + source!.eventTypes, + ); + expect(metricAlertSource(readBack)).toEqual(source); + } + }); + + test("keeps EAP native when event types are missing or empty", () => { + expect(metricAlertDatasetName("events_analytics_platform", [])).toBe( + "events_analytics_platform", + ); + }); + + test("resolves unambiguous datasets without event types", () => { + expect(metricAlertDatasetName("events", [])).toBe("errors"); + expect(metricAlertDatasetName("metrics", [])).toBe("metrics"); + }); + + test("keeps unmodelled EAP event types native", () => { + expect( + metricAlertDatasetName("events_analytics_platform", [ + "trace_item_metric", + ]), + ).toBe("events_analytics_platform"); + }); + + test("ignores prototype keys", () => { + expect(metricAlertSource("constructor")).toBeUndefined(); + }); +}); + +describe("parseMetricTriggers", () => { + test("throws for undefined triggers", () => { + try { + parseMetricTriggers(undefined); + expect.unreachable("should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(ValidationError); + } + }); + + test("throws for empty triggers", () => { + try { + parseMetricTriggers([]); + expect.unreachable("should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(ValidationError); + } + }); + + test("throws for missing alertThreshold", () => { + try { + parseMetricTriggers([{ actions: [{ id: "a" }] }]); + expect.unreachable("should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(ValidationError); + } + }); + + test("passes for a threshold trigger without actions", () => { + expect(() => + parseMetricTriggers([ + { alertThreshold: 100, label: "warning", resolveThreshold: 50 }, + ]), + ).not.toThrow(); + }); + + test("rejects trigger actions, pointing at --workflow", () => { + expect(() => + parseMetricTriggers([ + { alertThreshold: 100, actions: [{ type: "email" }] }, + ]), + ).toThrow(/--workflow/); + }); + + test("rejects an unknown label", () => { + expect(() => + parseMetricTriggers([{ alertThreshold: 100, label: "info" }]), + ).toThrow(ValidationError); + }); + + test("passes native detector conditions", () => { + expect(() => + parseMetricTriggers([ + { type: "gt", comparison: 0, conditionResult: 75 }, + { type: "lte", comparison: 0, conditionResult: 0 }, + ]), + ).not.toThrow(); + }); + + test("rejects native conditions missing comparison", () => { + expect(() => + parseMetricTriggers([{ type: "gt", conditionResult: 75 }]), + ).toThrow(ValidationError); + }); + + test("rejects mixing native conditions and threshold triggers", () => { + expect(() => + parseMetricTriggers([ + { alertThreshold: 100 }, + { type: "lte", comparison: 0, conditionResult: 0 }, + ]), + ).toThrow(ValidationError); + }); +}); + +describe("parseMetricTriggers output", () => { + test("coerces numeric-string thresholds", () => { + expect( + parseMetricTriggers([{ alertThreshold: "10", resolveThreshold: "5" }]), + ).toEqual([{ alertThreshold: 10, resolveThreshold: 5 }]); + }); + + test("rejects non-numeric thresholds", () => { + expect(() => parseMetricTriggers([{ alertThreshold: "high" }])).toThrow( + ValidationError, + ); + }); + + test("rejects unknown condition results", () => { + expect(() => + parseMetricTriggers([{ type: "gt", comparison: 1, conditionResult: 99 }]), + ).toThrow(ValidationError); + }); +}); diff --git a/packages/mcp-core/src/tools/support/metric-monitor-config.ts b/packages/mcp-core/src/tools/support/metric-monitor-config.ts index 2ed94cc8f..0534fb865 100644 --- a/packages/mcp-core/src/tools/support/metric-monitor-config.ts +++ b/packages/mcp-core/src/tools/support/metric-monitor-config.ts @@ -1,3 +1,8 @@ +import { + METRIC_MONITOR_DATASETS, + METRIC_MONITOR_PRIORITY, + METRIC_MONITOR_THRESHOLD_TYPES, +} from "@sentry/toolkit-core/metric-monitor"; import { z } from "zod"; import type { Detector, MetricMonitorUpdate } from "../../api-client/types"; import { UserInputError } from "../../errors"; @@ -8,10 +13,15 @@ import { } from "./detector-details"; const metricConditionSchema = conditionGroupSchema.shape.conditions.element; +const { critical, warning, resolved } = METRIC_MONITOR_PRIORITY; const thresholdCondition = metricConditionSchema.extend({ - type: z.enum(["gt", "lt", "gte", "lte"]), + type: z.enum(METRIC_MONITOR_THRESHOLD_TYPES), comparison: z.number(), - conditionResult: z.union([z.literal(75), z.literal(50), z.literal(0)]), + conditionResult: z.union([ + z.literal(critical), + z.literal(warning), + z.literal(resolved), + ]), }); const anomalyCondition = metricConditionSchema.extend({ type: z.literal("anomaly_detection"), @@ -20,17 +30,11 @@ const anomalyCondition = metricConditionSchema.extend({ seasonality: z.literal("auto"), thresholdType: z.union([z.literal(0), z.literal(1), z.literal(2)]), }), - conditionResult: z.union([z.literal(75), z.literal(50)]), + conditionResult: z.union([z.literal(critical), z.literal(warning)]), }); const metricMonitorQuerySchema = metricQueryDetailsSchema.extend({ - dataset: z.enum([ - "events", - "transactions", - "generic_metrics", - "metrics", - "events_analytics_platform", - ]), + dataset: z.enum(METRIC_MONITOR_DATASETS), aggregate: z.string().min(1), timeWindowSeconds: z.number().int().positive(), }); diff --git a/packages/toolkit-core/package.json b/packages/toolkit-core/package.json index 3f4ff17c7..0c0a64fe1 100644 --- a/packages/toolkit-core/package.json +++ b/packages/toolkit-core/package.json @@ -27,6 +27,10 @@ "types": "./src/events-sort.ts", "default": "./src/events-sort.ts" }, + "./metric-monitor": { + "types": "./src/metric-monitor.ts", + "default": "./src/metric-monitor.ts" + }, "./oauth-device": { "types": "./src/oauth-device.ts", "default": "./src/oauth-device.ts" diff --git a/packages/toolkit-core/src/metric-monitor.ts b/packages/toolkit-core/src/metric-monitor.ts new file mode 100644 index 000000000..d3a0880c9 --- /dev/null +++ b/packages/toolkit-core/src/metric-monitor.ts @@ -0,0 +1,65 @@ +/** + * Metric monitor (metric-issue detector) constants shared by the CLI and MCP. + * + * `@sentry/api` types detector `data_sources` and `condition_group.conditions` + * as `Array`, so these values come from the Sentry source instead: + * - Datasets: `sentry.snuba.snuba_query_validator.QUERY_TYPE_VALID_DATASETS` + * - Event types: `QUERY_TYPE_VALID_EVENT_TYPES` in the same module + * - Conditions and priorities: + * `sentry.incidents.metric_issue_detector.MetricIssueComparisonConditionValidator` + */ + +/** Native `Dataset` values the detector validator accepts. */ +export const METRIC_MONITOR_DATASETS = [ + "events", + "transactions", + "generic_metrics", + "metrics", + "events_analytics_platform", +] as const; + +export type MetricMonitorDataset = (typeof METRIC_MONITOR_DATASETS)[number]; + +/** Static/percent threshold operators (`anomaly_detection` is separate). */ +export const METRIC_MONITOR_THRESHOLD_TYPES = [ + "gt", + "lt", + "gte", + "lte", +] as const; + +export type MetricMonitorThresholdType = + (typeof METRIC_MONITOR_THRESHOLD_TYPES)[number]; + +/** `DetectorPriorityLevel` values a metric condition can resolve to. */ +export const METRIC_MONITOR_PRIORITY = { + critical: 75, + warning: 50, + resolved: 0, +} as const; + +/** `SnubaQuery.Type`: 0 error, 1 performance (incl. EAP), 2 crash rate. */ +export type MetricMonitorQueryType = 0 | 1 | 2; + +/** The dataset, query type, and event types that select one signal. */ +export type MetricMonitorSource = { + readonly dataset: MetricMonitorDataset; + readonly queryType: MetricMonitorQueryType; + readonly eventTypes: readonly string[]; +}; + +/** Data sources for the signals clients expose by name. */ +export const METRIC_MONITOR_SOURCES = { + errors: { dataset: "events", queryType: 0, eventTypes: ["error", "default"] }, + spans: { + dataset: "events_analytics_platform", + queryType: 1, + eventTypes: ["trace_item_span"], + }, + logs: { + dataset: "events_analytics_platform", + queryType: 1, + eventTypes: ["trace_item_log"], + }, + crashRate: { dataset: "metrics", queryType: 2, eventTypes: [] }, +} as const satisfies Record;