Skip to content

fix(alerts): migrate metric alert mutations to detectors API #1338

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BYK merged 1 commit into from
Aug 2, 2026
Merged

fix(alerts): migrate metric alert mutations to detectors API #1338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions packages/cli/src/commands/alert/metrics/create.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" +
Expand Down Expand Up @@ -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);

@sentry sentry Bot Aug 2, 2026

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.

Bug: The create and edit metric alert commands always show the status as "active" in their output, regardless of the rule's actual state.
Severity: LOW

Suggested Fix

Modify the create and edit commands to correctly handle the API response. Instead of directly using the raw response, map it to a MetricAlertRule object (which has the status field) before passing it to the output rendering logic, or adjust the rendering logic to read the enabled field.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/cli/src/commands/alert/metrics/create.ts#L195

Potential issue: The `create` and `edit` commands for metric alerts display an incorrect
status in their output. These commands receive a raw API response containing an
`enabled: boolean` field. However, the output rendering logic attempts to read a
`status` property from this response object. Because `created.status` (or
`updated.status`) is `undefined`, the `metricAlertStatusLabel` function incorrectly
defaults to returning "active", even if the rule was created or updated as disabled.

Also affects:

  • packages/cli/src/commands/alert/metrics/edit.ts:281

Did we get this right? 👍 / 👎 to inform future reviews.

Expand Down
237 changes: 199 additions & 38 deletions packages/cli/src/lib/api/alerts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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<Record<string, unknown>> {
const regionUrl = await resolveOrgRegion(orgSlug);
const { data } = await apiRequestToRegion<Record<string, unknown>>(
regionUrl,
`/organizations/${orgSlug}/alert-rules/${encodeURIComponent(ruleId)}/`
);
return data;
}

/**
* Get a single metric alert rule by ID.
*
Expand Down Expand Up @@ -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<string, unknown>
): Record<string, unknown> {
const dataset = typeof body.dataset === "string" ? body.dataset : "";
const timeWindowMinutes =
typeof body.timeWindow === "number" ? body.timeWindow : 0;
const source: Record<string, unknown> = {
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<string, unknown>
): Record<string, unknown> {
const detectorBody: Record<string, unknown> = {
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,
};
}
Comment on lines +619 to +625

@sentry sentry Bot Aug 2, 2026

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.

Bug: Editing a metric alert without the --trigger flag deletes all existing triggers because the CLI fails to fetch and preserve them during the update.
Severity: HIGH

Suggested Fix

Update the MetricAlertRule type and the mapDetectorToMetricAlertRule function to include the triggers field. This will ensure that when an alert is fetched for an edit, its existing triggers are included in the baseline data and preserved if not explicitly overridden.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/cli/src/lib/api/alerts.ts#L619-L625

Potential issue: When editing a metric alert via the CLI without explicitly
re-specifying the `--trigger` flag, all existing trigger conditions are deleted. This
occurs because the function `getMetricAlertRuleDocument`, which fetches the alert's
current state, fails to include the `triggers` field in the object it returns. The
`buildDetectorBody` function then constructs an update payload without a
`conditionGroup`, as `body.triggers` is `undefined`. The backend API interprets this as
a full replacement and removes the alert's triggers.

Did we get this right? 👍 / 👎 to inform future reviews.


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, unknown>): 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<void> {
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<Record<string, unknown>> {
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<string, unknown>
): Promise<Record<string, unknown>> {
const regionUrl = await resolveOrgRegion(orgSlug);
const { data } = await apiRequestToRegion<Record<string, unknown>>(
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<Record<string, unknown>>(
result,
`Failed to update metric alert rule '${ruleId}'`

@cursor cursor Bot Aug 2, 2026

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.

Mutation response status mishandled

Medium Severity

putMetricAlertRule and createMetricAlertRule return the raw detector payload (enabled) instead of the flat rule shape with status. Edit then runs metricAlertStatusLabel on updated.status, so a successful disable still reports active and JSON output exposes the nested detector document.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b244375. Configure here.

@jared-outpost jared-outpost Bot Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed — this is a real bug: createMetricAlertRule/putMetricAlertRule return the raw detector payload (enabled), but the create/edit commands render metricAlertStatusLabel(created.status), so a disabled rule prints as active and JSON exposes the nested detector doc. The PR merged before this could be addressed on-branch, so it needs a follow-up on main (map the mutation response through mapDetectorToMetricAlertRule before rendering). The paired sentry[bot] review also flagged the related edit-without---trigger trigger-loss issue — both should go in the same follow-up.

);
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<string, unknown>
): Promise<Record<string, unknown>> {
const regionUrl = await resolveOrgRegion(orgSlug);
const { data } = await apiRequestToRegion<Record<string, unknown>>(
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<Record<string, unknown>>(
result,
"Failed to create metric alert rule"
);
return data;
}
31 changes: 31 additions & 0 deletions packages/cli/test/commands/alert/metrics/create.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>;

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" });
Expand All @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand All @@ -288,6 +318,7 @@ describe("alert metrics create", () => {
dataset: "errors",
timeWindow: 5,
triggers: [{ alertThreshold: 100, actions: [{ id: "notify" }] }],
projects: ["backend"],
});
});
});
Loading
Loading