From b244375e15c3a2441ddc6d69f437350039a8fe5f Mon Sep 17 00:00:00 2001 From: "jared-outpost[bot]" Date: Sun, 2 Aug 2026 13:03:06 +0000 Subject: [PATCH] fix(alerts): migrate metric alert mutations to detectors API Repoint create/edit/delete of metric alert rules off the legacy org-scoped /alert-rules/ endpoint (retiring 2026-08-17) onto the detectors write API, following the read-path migration in #1275. The CLI commands still build the flat legacy body; the new buildDetectorBody translator in alerts.ts converts it into the nested detector document (dataSources / conditionGroup / config) so the command layer is unchanged: - createMetricAlertRule -> createOrganizationProjectDetector (project-scoped POST; project taken from the flat body's projects, now required for create) - 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 Fixes #1277 --- .../cli/src/commands/alert/metrics/create.ts | 10 +- packages/cli/src/lib/api/alerts.ts | 237 +++++++++++++++--- .../commands/alert/metrics/create.test.ts | 31 +++ packages/cli/test/lib/api/alerts.test.ts | 147 ++++++++++- 4 files changed, 381 insertions(+), 44 deletions(-) diff --git a/packages/cli/src/commands/alert/metrics/create.ts b/packages/cli/src/commands/alert/metrics/create.ts index 82b23f59e6..6199a8dc9d 100644 --- a/packages/cli/src/commands/alert/metrics/create.ts +++ b/packages/cli/src/commands/alert/metrics/create.ts @@ -79,9 +79,9 @@ export const createCommand = buildCommand({ fullDescription: "Create an organization-scoped metric alert rule.\n\n" + "Required fields:\n" + - " --name, --query, --aggregate, --dataset, --time-window, --trigger (>=1)\n\n" + + " --name, --query, --aggregate, --dataset, --time-window, --trigger (>=1), --project\n\n" + "Optional fields:\n" + - " --project (repeatable), --environment, --owner\n\n" + + " --environment, --owner\n\n" + "Examples:\n" + " sentry alert metrics create my-org --name 'P95 latency' \\\n" + " --query 'environment:prod' --aggregate 'p95(transaction.duration)' \\\n" + @@ -184,6 +184,12 @@ export const createCommand = buildCommand({ const triggers = parseJsonObjectList(flags.trigger, "trigger"); validateMetricTriggers(triggers); const projects = normalizeProjectList(flags.project); + if (!projects || projects.length === 0) { + throw new ValidationError( + "A project is required to create a metric alert rule (pass --project).", + "project" + ); + } const orgSlug = await resolveMetricCreateOrg(arg, cwd); diff --git a/packages/cli/src/lib/api/alerts.ts b/packages/cli/src/lib/api/alerts.ts index 754ccc65f7..c1259e7063 100644 --- a/packages/cli/src/lib/api/alerts.ts +++ b/packages/cli/src/lib/api/alerts.ts @@ -7,13 +7,16 @@ */ import { + createOrganizationProjectDetector, createOrganizationWorkflow, + deleteOrganizationDetector, getOrganizationDetector, getOrganizationWorkflow, listOrganizationDetectors, + updateOrganizationDetector, updateOrganizationWorkflow, } from "@sentry/api"; -import { ApiError } from "../errors.js"; +import { ApiError, ValidationError } from "../errors.js"; import { resolveOrgRegion } from "../region.js"; import { apiRequestToRegion, @@ -362,8 +365,8 @@ export async function getIssueAlertRule( // 2026-08-17 (getsentry/cli#1274, #1182). Detectors return a nested shape, so // `mapDetectorToMetricAlertRule` flattens each detector into the existing // `MetricAlertRule` shape the commands already consume. Mutations -// (create/update/delete) still use the legacy endpoint pending a follow-up -// migration to the detectors/monitors + workflows write APIs. +// (create/update/delete) also target the detectors write API — the flat CLI +// input is translated into the nested detector body by `buildDetectorBody`. /** Search query that limits the detectors endpoint to metric alert rules. */ const METRIC_DETECTOR_QUERY = "type:metric_issue"; @@ -406,19 +409,6 @@ export async function listMetricAlertsPaginated( return { data: data.map(mapDetectorToMetricAlertRule), nextCursor }; } -/** Single GET for org metric alert rule (typed or full JSON for PUT). */ -async function fetchMetricAlertRuleJson( - orgSlug: string, - ruleId: string -): Promise> { - const regionUrl = await resolveOrgRegion(orgSlug); - const { data } = await apiRequestToRegion>( - regionUrl, - `/organizations/${orgSlug}/alert-rules/${encodeURIComponent(ruleId)}/` - ); - return data; -} - /** * Get a single metric alert rule by ID. * @@ -547,55 +537,226 @@ export async function createIssueAlertRule( } // Metric alert (org) write operations +// +// The CLI create/edit commands build a *flat* legacy alert-rule body (name, +// query, aggregate, dataset, timeWindow, triggers, projects, environment, +// owner). The detectors write API takes a *nested* shape (dataSources / +// conditionGroup / config), so `buildDetectorBody` translates the flat body +// into the detector document before the request is sent. This keeps the +// command layer unchanged while the transport moves off the retiring +// `/alert-rules/` endpoint (getsentry/cli#1274, #1277). + +/** + * Datasets whose events flow through the error/transaction pipeline + * (`queryType: 0`) rather than the trace-item pipeline (`queryType: 1`). + */ +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; +} + +/** + * Default `eventTypes` for a metric alert dataset. + * + * 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`. + */ +function datasetEventTypes(dataset: string): string[] { + return ERROR_LIKE_DATASETS.has(dataset.trim().toLowerCase()) + ? ["error", "default"] + : ["trace_item_span"]; +} + +/** + * Build the single data source describing the threshold query for a detector. + * + * `timeWindow` is carried by the flat body in *minutes* (every CLI formatter + * appends "m"), but the detector API expects *seconds*, matching the inverse + * conversion in `mapDetectorToMetricAlertRule`. + */ +function buildDetectorDataSource( + body: Record +): Record { + const dataset = typeof body.dataset === "string" ? body.dataset : ""; + const timeWindowMinutes = + typeof body.timeWindow === "number" ? body.timeWindow : 0; + const source: Record = { + aggregate: typeof body.aggregate === "string" ? body.aggregate : "", + dataset, + query: typeof body.query === "string" ? body.query : "", + queryType: datasetQueryType(dataset), + eventTypes: datasetEventTypes(dataset), + timeWindow: timeWindowMinutes * 60, + }; + if (typeof body.environment === "string" && body.environment !== "") { + source.environment = body.environment; + } + return source; +} + +/** + * 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. + */ +function buildDetectorBody( + body: Record +): Record { + const detectorBody: Record = { + name: typeof body.name === "string" ? body.name : "", + type: "metric_issue", + dataSources: [buildDetectorDataSource(body)], + config: { detectionType: "static" }, + }; + + const triggers = Array.isArray(body.triggers) ? body.triggers : undefined; + if (triggers) { + detectorBody.conditionGroup = { + logicType: "any", + conditions: triggers, + }; + } + + if (body.status !== undefined) { + detectorBody.enabled = Number(body.status) !== 1; + } + if (body.owner !== undefined) { + detectorBody.owner = body.owner as string | null; + } + return detectorBody; +} + +/** + * Resolve the target project slug for a metric detector create. + * + * Detector create is project-scoped (`/projects/{project}/detectors/`), unlike + * the legacy org-scoped `/alert-rules/` create. The flat body carries the + * project(s) in `projects`; the first entry is used. A metric alert without a + * project cannot be created against the detectors API. + * + * @throws {ValidationError} when no project slug is present + */ +function resolveDetectorProject(body: Record): string { + const projects = Array.isArray(body.projects) ? body.projects : []; + const project = projects.find( + (value): value is string => typeof value === "string" && value.trim() !== "" + ); + if (!project) { + throw new ValidationError( + "A project is required to create a metric alert rule (pass --project).", + "project" + ); + } + return project; +} /** - * Delete a metric (organization) alert rule. May return 202 with no body. + * Delete a metric alert rule via the org-scoped `/detectors/{id}/` endpoint. + * + * `ruleId` is the detector id surfaced by the migrated read path. Succeeds with + * 204 No Content and no response body. */ export async function deleteMetricAlertRule( orgSlug: string, ruleId: string ): Promise { - const regionUrl = await resolveOrgRegion(orgSlug); - await apiRequestToRegionNoContent( - regionUrl, - `/organizations/${orgSlug}/alert-rules/${encodeURIComponent(ruleId)}/`, - { method: "DELETE" } - ); + const config = await getOrgSdkConfig(orgSlug); + const result = await deleteOrganizationDetector({ + ...config, + // Detector IDs are opaque strings the API accepts verbatim; the SDK types + // detector_id as number, so pass the string through the string-keyed path. + path: { + organization_id_or_slug: orgSlug, + detector_id: ruleId, + } as unknown as { + organization_id_or_slug: string; + detector_id: number; + }, + }); + unwrapResult(result, `Failed to delete metric alert rule '${ruleId}'`); } -export function getMetricAlertRuleDocument( +/** + * Fetch the edit baseline for a metric alert rule. + * + * The edit command merges flag overrides onto this baseline in the flat + * `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. + */ +export async function getMetricAlertRuleDocument( orgSlug: string, ruleId: string ): Promise> { - return fetchMetricAlertRuleJson(orgSlug, ruleId); + const rule = await getMetricAlertRule(orgSlug, ruleId); + return { ...rule }; } +/** + * Update a metric alert rule via the org-scoped `/detectors/{id}/` endpoint. + * + * `body` is the flat merged edit body; it is translated into the nested + * detector update payload before the PUT. + */ export async function putMetricAlertRule( orgSlug: string, ruleId: string, body: Record ): Promise> { - const regionUrl = await resolveOrgRegion(orgSlug); - const { data } = await apiRequestToRegion>( - regionUrl, - `/organizations/${orgSlug}/alert-rules/${encodeURIComponent(ruleId)}/`, - { method: "PUT", body } + const config = await getOrgSdkConfig(orgSlug); + const result = await updateOrganizationDetector({ + ...config, + path: { + organization_id_or_slug: orgSlug, + detector_id: ruleId, + } as unknown as { + organization_id_or_slug: string; + detector_id: number; + }, + // The SDK types the detector body with snake_case keys and opaque + // dataSources/conditions; the CLI supplies the camelCase document the + // endpoint's CamelSnakeSerializer expects, so cast into the typed arg. + body: buildDetectorBody(body) as unknown as Parameters< + typeof updateOrganizationDetector + >[0]["body"], + }); + return unwrapResult>( + result, + `Failed to update metric alert rule '${ruleId}'` ); - return data; } /** - * Create a metric (organization) alert rule. + * Create a metric alert rule via the project-scoped `/detectors/` endpoint. + * + * `body` is the flat create body; the target project is taken from + * `body.projects` and the rest is translated into the nested detector payload. */ export async function createMetricAlertRule( orgSlug: string, body: Record ): Promise> { - const regionUrl = await resolveOrgRegion(orgSlug); - const { data } = await apiRequestToRegion>( - regionUrl, - `/organizations/${orgSlug}/alert-rules/`, - { method: "POST", body } + const projectSlug = resolveDetectorProject(body); + const config = await getOrgSdkConfig(orgSlug); + const result = await createOrganizationProjectDetector({ + ...config, + path: { + organization_id_or_slug: orgSlug, + project_id_or_slug: projectSlug, + }, + body: buildDetectorBody(body) as unknown as Parameters< + typeof createOrganizationProjectDetector + >[0]["body"], + }); + return unwrapResult>( + result, + "Failed to create metric alert rule" ); - return data; } diff --git a/packages/cli/test/commands/alert/metrics/create.test.ts b/packages/cli/test/commands/alert/metrics/create.test.ts index 961a8651a5..400c13ffe3 100644 --- a/packages/cli/test/commands/alert/metrics/create.test.ts +++ b/packages/cli/test/commands/alert/metrics/create.test.ts @@ -114,6 +114,32 @@ describe("alert metrics create", () => { ).rejects.toBeInstanceOf(ValidationError); }); + test("rejects create without a project", async () => { + const context = createContext(); + const func = (await createCommand.loader()) as unknown as ( + this: unknown, + flags: CreateFlags, + arg: string + ) => Promise; + + await expect( + func.call( + context, + { + name: "Metric Rule", + query: "event.type:error", + aggregate: "count()", + dataset: "errors", + "time-window": 5, + trigger: ['{"alertThreshold":100,"actions":[{"id":"notify"}]}'], + "dry-run": true, + json: true, + }, + "test-org" + ) + ).rejects.toBeInstanceOf(ValidationError); + }); + test("dry run does not call create API", async () => { const context = createContext(); resolveOrgSpy.mockResolvedValue({ org: "test-org" }); @@ -132,6 +158,7 @@ describe("alert metrics create", () => { dataset: "errors", "time-window": 5, trigger: ['{"alertThreshold":100,"actions":[{"id":"notify"}]}'], + project: ["backend"], "dry-run": true, json: true, }, @@ -159,6 +186,7 @@ describe("alert metrics create", () => { dataset: "errors", "time-window": 5, trigger: ['{"alertThreshold":100,"actions":[{"id":"notify"}]}'], + project: ["backend"], "dry-run": true, json: true, }, @@ -190,6 +218,7 @@ describe("alert metrics create", () => { dataset: "errors", "time-window": 5, trigger: ['{"alertThreshold":100,"actions":[{"id":"notify"}]}'], + project: ["backend"], "dry-run": true, json: true, }, @@ -275,6 +304,7 @@ describe("alert metrics create", () => { dataset: "errors", "time-window": 5, trigger: ['{"alertThreshold":100,"actions":[{"id":"notify"}]}'], + project: ["backend"], "dry-run": false, json: true, }, @@ -288,6 +318,7 @@ describe("alert metrics create", () => { dataset: "errors", timeWindow: 5, triggers: [{ alertThreshold: 100, actions: [{ id: "notify" }] }], + projects: ["backend"], }); }); }); diff --git a/packages/cli/test/lib/api/alerts.test.ts b/packages/cli/test/lib/api/alerts.test.ts index a01fd23b00..40a1e7646a 100644 --- a/packages/cli/test/lib/api/alerts.test.ts +++ b/packages/cli/test/lib/api/alerts.test.ts @@ -1,13 +1,16 @@ import { afterEach, beforeEach, describe, expect, test } from "vitest"; import { createIssueAlertRule, + createMetricAlertRule, deleteIssueAlertRule, deleteMetricAlertRule, getIssueAlertRule, getIssueAlertWorkflowDocument, getMetricAlertRule, + getMetricAlertRuleDocument, listIssueAlertsPaginated, listMetricAlertsPaginated, + putMetricAlertRule, resolveErrorDetectorId, updateIssueAlertRule, } from "../../../src/lib/api/alerts.js"; @@ -360,15 +363,15 @@ describe("resolveErrorDetectorId", () => { }); describe("deleteMetricAlertRule", () => { - test("treats empty-body 202 as success", async () => { + test("deletes via the org-scoped /detectors/{id}/ endpoint", async () => { globalThis.fetch = mockFetch(async (input, init) => { const req = new Request(input!, init); expect(req.method).toBe("DELETE"); - expect(req.url).toBe( - `${DEFAULT_SENTRY_URL}/api/0/organizations/test-org/alert-rules/9/` + expect(new URL(req.url).pathname).toBe( + "/api/0/organizations/test-org/detectors/9/" ); expect(req.headers.get("Authorization")).toBe("Bearer test-token"); - return new Response(null, { status: 202 }); + return new Response(null, { status: 204 }); }); await expect( @@ -376,3 +379,139 @@ describe("deleteMetricAlertRule", () => { ).resolves.toBeUndefined(); }); }); + +describe("createMetricAlertRule", () => { + test("POSTs a nested detector body to the project-scoped /detectors/ endpoint", async () => { + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input!, init); + expect(req.method).toBe("POST"); + expect(new URL(req.url).pathname).toBe( + "/api/0/organizations/test-org/projects/backend/detectors/" + ); + expect(await req.json()).toEqual({ + name: "P95 latency", + type: "metric_issue", + dataSources: [ + { + aggregate: "p95(transaction.duration)", + dataset: "transactions", + query: "environment:prod", + queryType: 1, + eventTypes: ["trace_item_span"], + // 5m from the flat body is sent to the detector API as 300s. + timeWindow: 300, + environment: "prod", + }, + ], + config: { detectionType: "static" }, + conditionGroup: { + logicType: "any", + conditions: [{ alertThreshold: 500, actions: [{ id: "notify" }] }], + }, + }); + return Response.json( + { ...metricDetector({ id: "77", name: "P95 latency" }) }, + { status: 201 } + ); + }); + + const created = await createMetricAlertRule("test-org", { + name: "P95 latency", + query: "environment:prod", + aggregate: "p95(transaction.duration)", + dataset: "transactions", + timeWindow: 5, + environment: "prod", + triggers: [{ alertThreshold: 500, actions: [{ id: "notify" }] }], + projects: ["backend"], + }); + expect(created.id).toBe("77"); + }); + + test("maps error-like datasets to queryType 0 and error eventTypes", 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: "Error volume", + query: "event.type:error", + aggregate: "count()", + dataset: "errors", + timeWindow: 15, + triggers: [{ alertThreshold: 100, actions: [{ id: "notify" }] }], + projects: ["backend"], + }); + + const source = (sentBody.dataSources as Record[])[0]; + expect(source?.queryType).toBe(0); + expect(source?.eventTypes).toEqual(["error", "default"]); + expect(source?.timeWindow).toBe(900); + }); + + test("throws ValidationError when no project is provided", async () => { + await expect( + createMetricAlertRule("test-org", { + name: "No project", + query: "", + aggregate: "count()", + dataset: "errors", + timeWindow: 5, + triggers: [{ alertThreshold: 1, actions: [{ id: "notify" }] }], + }) + ).rejects.toMatchObject({ name: "ValidationError" }); + }); +}); + +describe("getMetricAlertRuleDocument", () => { + test("returns the flat rule baseline from the detector GET", async () => { + globalThis.fetch = mockFetch(async (input, init) => { + const url = new URL(new Request(input!, init).url); + expect(url.pathname).toBe("/api/0/organizations/test-org/detectors/9/"); + return Response.json(metricDetector({ id: "9", name: "Baseline" })); + }); + + const doc = await getMetricAlertRuleDocument("test-org", "9"); + expect(doc).toMatchObject({ + id: "9", + name: "Baseline", + aggregate: "p95(transaction.duration)", + dataset: "transactions", + timeWindow: 5, + }); + }); +}); + +describe("putMetricAlertRule", () => { + test("PUTs a nested detector body to the /detectors/{id}/ endpoint", async () => { + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input!, init); + expect(req.method).toBe("PUT"); + expect(new URL(req.url).pathname).toBe( + "/api/0/organizations/test-org/detectors/9/" + ); + const body = (await req.json()) as Record; + expect(body.name).toBe("Renamed"); + expect(body.type).toBe("metric_issue"); + // status 1 (disabled) maps onto the detector's enabled=false flag. + expect(body.enabled).toBe(false); + return Response.json(metricDetector({ id: "9", name: "Renamed" })); + }); + + const updated = await putMetricAlertRule("test-org", "9", { + name: "Renamed", + query: "environment:prod", + aggregate: "p95(transaction.duration)", + dataset: "transactions", + timeWindow: 5, + status: 1, + triggers: [{ alertThreshold: 500, actions: [{ id: "notify" }] }], + }); + expect(updated.id).toBe("9"); + }); +});