Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 18 additions & 1 deletion apps/ai/src/mcp/lib/alert-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
type AlertValidationError,
} from "@maple/domain/http"
import { QueryEngineAlertReducer } from "@maple/domain"
import { rawAlertSampleCountWarning } from "@maple/domain/raw-sql"
import { McpInvalidInputError } from "../tools/types"
import { doc, type ToolDoc } from "./tool-doc"

Expand Down Expand Up @@ -107,9 +108,25 @@ export const ruleWriteInputErrors = {
export const ruleNotFoundFromError = (error: AlertRuleNotFoundError) =>
Effect.fail(ruleNotFound(error.ruleId))

/** Saved-but-suspicious configuration worth telling the caller about. */
export const ruleConfigWarnings = (rule: {
readonly signalType: string
readonly rawQuerySql: string | null
readonly minimumSampleCount: number
}): ReadonlyArray<string> => {
if (rule.signalType !== "raw_query" || rule.rawQuerySql === null) return []
const warning = rawAlertSampleCountWarning(rule.rawQuerySql, rule.minimumSampleCount)
return warning === null ? [] : [warning]
}

/** The text a create or update returns: the rule as saved. */
export const renderRuleWrite = (title: string, rule: typeof AlertRuleRow.Type): ToolDoc => ({
export const renderRuleWrite = (
title: string,
rule: typeof AlertRuleRow.Type,
warnings: ReadonlyArray<string> = [],
): ToolDoc => ({
title,
...(warnings.length > 0 ? { notices: warnings } : undefined),
blocks: [
doc.fields([
["ID", rule.id],
Expand Down
33 changes: 32 additions & 1 deletion apps/ai/src/mcp/tools/__tests__/alert-tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
UserId,
} from "@maple/domain/http"
import {
CreateAlertRuleOutput,
GetAlertRuleOutput,
ListAlertChecksOutput,
ListAlertDestinationsOutput,
Expand Down Expand Up @@ -114,7 +115,13 @@ const layer = (seen: Seen) =>
listRules: () => Effect.succeed(new AlertRulesListResponse({ rules: [rule] })),
createRule: (_org: unknown, _user: unknown, _roles: unknown, request: AlertRuleUpsertRequest) => {
seen.created = request
return Effect.succeed(rule)
// Echo the fields the write warnings read, so a raw-SQL create reads back as one.
return Effect.succeed({
...rule,
signalType: request.signalType,
rawQuerySql: request.rawQuerySql ?? null,
minimumSampleCount: request.minimumSampleCount ?? 0,
} as never)
},
deleteRule: (_org: unknown, _roles: unknown, id: string) =>
id === RULE_ID
Expand Down Expand Up @@ -399,6 +406,30 @@ describe("alert tools", () => {
)
})

it("create_alert_rule warns when a raw_query minimum sample count would count rows", async () => {
const base = {
name: "Raw",
destination_ids: [],
signal_type: "raw_query",
comparator: "gt",
threshold: 0.05,
minimum_sample_count: 50,
}
const sql = "SELECT count() AS value FROM traces WHERE $__orgFilter AND $__timeFilter(Timestamp)"
const warned = await ok("create_alert_rule", { ...base, raw_query_sql: sql })
const output = Schema.decodeUnknownSync(CreateAlertRuleOutput)(warned.structuredContent)
expect(output.warnings?.[0]).toContain("counts returned rows")
expect(text(warned)).toContain("`samples` column")

const quiet = await ok("create_alert_rule", {
...base,
raw_query_sql: sql.replace("AS value", "AS value, count() AS samples"),
})
expect(
Schema.decodeUnknownSync(CreateAlertRuleOutput)(quiet.structuredContent).warnings,
).toBeUndefined()
})

it("update_alert_rule overlays only the given fields", async () => {
const seen: Seen = {}
await ok(
Expand Down
18 changes: 11 additions & 7 deletions apps/ai/src/mcp/tools/create-alert-rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
ALERT_SEVERITIES,
ALERT_SIGNAL_TYPES,
renderRuleWrite,
ruleConfigWarnings,
ruleWriteInputErrors,
toAlertRuleRow,
} from "../lib/alert-rules"
Expand Down Expand Up @@ -97,10 +98,12 @@ const Parameters = Schema.Struct({
group_by: P.optionalList(
"Evaluate one value per group. Built-in tokens: service.name, span.name, status.code, http.method, severity; attribute keys as attr.<key> (e.g. attr.http.route).",
),
minimum_sample_count: P.optionalNumber("Minimum sample count before evaluating (default: 0)"),
consecutive_breaches: P.optionalNumber("Consecutive breaches before alerting (default: 1)"),
consecutive_healthy: P.optionalNumber("Consecutive healthy evaluations before resolving (default: 1)"),
renotify_interval_minutes: P.optionalNumber("Re-notification interval in minutes (default: 60)"),
minimum_sample_count: P.optionalNumber(
"Skip evaluation below this many samples in the window (default: 0). For raw_query it sums the `samples` column; without one each returned row counts as 1, so it gates on buckets, not events.",
),
consecutive_breaches: P.optionalNumber("Consecutive breaches before alerting (default: 2)"),
consecutive_healthy: P.optionalNumber("Consecutive healthy evaluations before resolving (default: 2)"),
renotify_interval_minutes: P.optionalNumber("Re-notification interval in minutes (default: 30)"),
apdex_threshold_ms: P.optionalNumber(
"Response time counted as satisfactory, in ms. Required for signal_type=apdex.",
),
Expand All @@ -109,7 +112,7 @@ const Parameters = Schema.Struct({
"The query to evaluate, in the query-builder draft shape dashboard custom-query widgets use ({ id, name, dataSource, aggregation, whereClause, groupBy, ... }). Required for signal_type=builder_query.",
),
raw_query_sql: P.optionalText(
"ClickHouse SQL returning a numeric `value` column and optional `group` / `samples` columns. Must reference $__orgFilter and $__timeFilter(col); $__startTime, $__endTime and $__interval_s are also available. Required for signal_type=raw_query.",
"ClickHouse SQL returning a numeric `value` column and optional `group` / `samples` columns (`samples` is the event count behind each row; it feeds minimum_sample_count). A query returning no rows is a no-data check, not a healthy one. Must reference $__orgFilter and $__timeFilter(col); $__startTime, $__endTime and $__interval_s are also available. Required for signal_type=raw_query.",
),
raw_query_reducer: P.optionalOneOf(
ALERT_REDUCERS,
Expand Down Expand Up @@ -295,8 +298,9 @@ export function registerCreateAlertRuleTool(server: McpToolRegistrar) {
}),
)

return { rule: toAlertRuleRow(rule) }
const warnings = ruleConfigWarnings(rule)
return { rule: toAlertRuleRow(rule), ...(warnings.length > 0 ? { warnings } : undefined) }
}),
render: (output) => renderRuleWrite("Alert Rule Created", output.rule),
render: (output) => renderRuleWrite("Alert Rule Created", output.rule, output.warnings),
})
}
4 changes: 3 additions & 1 deletion apps/ai/src/mcp/tools/get-alert-rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Effect, Schema } from "effect"
import { GetAlertRuleOutput } from "@maple/domain/mcp-outputs"
import { CurrentMcpTenant } from "../lib/query-warehouse"
import { AlertRulesService } from "@maple/backend/services/alerts/AlertRulesService"
import { formatCondition, ruleNotFound, toAlertRuleRow } from "../lib/alert-rules"
import { formatCondition, ruleConfigWarnings, ruleNotFound, toAlertRuleRow } from "../lib/alert-rules"
import * as P from "../lib/params"
import { doc, type DocBlock } from "../lib/tool-doc"

Expand Down Expand Up @@ -130,8 +130,10 @@ export function registerGetAlertRuleTool(server: McpToolRegistrar) {
if (rule.notificationBody) blocks.push(doc.text("Body:"), doc.code("", rule.notificationBody))
}

const warnings = ruleConfigWarnings(rule)
return {
title: `Alert Rule: ${rule.name}`,
...(warnings.length > 0 ? { notices: warnings } : undefined),
blocks,
next: [
doc.next(
Expand Down
10 changes: 7 additions & 3 deletions apps/ai/src/mcp/tools/update-alert-rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
ALERT_SEVERITIES,
ALERT_SIGNAL_TYPES,
renderRuleWrite,
ruleConfigWarnings,
ruleNotFound,
ruleNotFoundFromError,
ruleWriteInputErrors,
Expand Down Expand Up @@ -48,7 +49,9 @@ const Parameters = Schema.Struct({
"Dimensions to evaluate the alert per-group (replaces the current grouping; an empty list removes it). " +
"Built-in tokens: service.name, span.name, status.code, http.method, severity. Attribute keys: attr.<key>.",
),
minimum_sample_count: P.optionalNumber("Minimum sample count before evaluating"),
minimum_sample_count: P.optionalNumber(
"Skip evaluation below this many samples in the window. For raw_query it sums the `samples` column; without one each returned row counts as 1.",
),
consecutive_breaches: P.optionalNumber("Consecutive breaches before alerting"),
consecutive_healthy: P.optionalNumber("Consecutive healthy evaluations before resolving"),
renotify_interval_minutes: P.optionalNumber("Re-notification interval in minutes"),
Expand Down Expand Up @@ -171,8 +174,9 @@ export function registerUpdateAlertRuleTool(server: McpToolRegistrar) {
}),
)

return { rule: toAlertRuleRow(rule) }
const warnings = ruleConfigWarnings(rule)
return { rule: toAlertRuleRow(rule), ...(warnings.length > 0 ? { warnings } : undefined) }
}),
render: (output) => renderRuleWrite("Alert Rule Updated", output.rule),
render: (output) => renderRuleWrite("Alert Rule Updated", output.rule, output.warnings),
})
}
20 changes: 17 additions & 3 deletions apps/web/src/components/alerts/signal-and-threshold-section.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
type SetStateAction,
} from "react"
import type { AlertComparator, AlertSeverity, AlertSignalType } from "@maple/domain/http"
import { rawAlertSampleCountWarning } from "@maple/domain/raw-sql"

import { Card } from "@maple/ui/components/ui/card"
import { Input } from "@maple/ui/components/ui/input"
Expand Down Expand Up @@ -360,7 +361,11 @@ export function SignalAndThresholdSection({
<NumericField
id="rule-minimum-samples"
label="Min samples"
hint="Skip below this count."
hint={
form.signalType === "raw_query"
? "Sums the samples column (1 per row without one)."
: "Skip below this count."
}
value={form.minimumSampleCount}
onChange={(value) => onChange((c) => ({ ...c, minimumSampleCount: value }))}
/>
Expand Down Expand Up @@ -657,7 +662,11 @@ function SignalSubConfig({ form, onChange, autocompleteValues }: SignalAndThresh
case "builder_query":
return <AlertQueryPanel form={form} onChange={onChange} autocompleteValues={autocompleteValues} />

case "raw_query":
case "raw_query": {
const sampleWarning = rawAlertSampleCountWarning(
form.rawQuerySql,
Number(form.minimumSampleCount) || 0,
)
return (
<div className="space-y-3">
<RawSqlEditorPanel
Expand All @@ -669,8 +678,12 @@ function SignalSubConfig({ form, onChange, autocompleteValues }: SignalAndThresh
targetLabel="alert rule"
/>
<p className="text-muted-foreground text-xs">
Alert SQL must return a numeric <code>value</code> column.
Return a numeric <code>value</code> column. Optional: <code>group</code> for one
series per value, and <code>samples</code> for the event count behind each row (Min
samples sums it; without it each row counts as 1). A query that returns no rows is a
no-data check.
</p>
{sampleWarning && <p className="text-warning text-xs">{sampleWarning}</p>}
<div className="flex items-end gap-3">
<div className="space-y-1.5">
<Label htmlFor="rule-raw-reducer">Reduce buckets by</Label>
Expand Down Expand Up @@ -700,6 +713,7 @@ function SignalSubConfig({ form, onChange, autocompleteValues }: SignalAndThresh
</div>
</div>
)
}

default:
return null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const TABLE_NOTES: Record<string, ReadonlyArray<string>> = {
"`Duration` is NANOSECONDS — divide by 1e6 for ms.",
"`StatusCode` is Title Case: 'Ok', 'Error', 'Unset'.",
"Does NOT include `SpanAttributes`/`ResourceAttributes` — query `traces` if you need attribute access.",
"Has NO `SpanKind` column: Server, Consumer and root spans are mixed together. Query `traces` to split by kind.",
],
error_events: [
"Per-error-occurrence rows with the OTel `exception` event unwrapped — surfaces `ExceptionType`, `ExceptionMessage`, `Stacktrace`, and a stable `FingerprintHash` for grouping.",
Expand Down
7 changes: 5 additions & 2 deletions packages/domain/src/mcp-outputs/alerts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,12 @@ export const ListAlertDestinationsOutput = Schema.Struct({
enabledOnly: Schema.optionalKey(Schema.Boolean),
})

export const CreateAlertRuleOutput = Schema.Struct({ rule: AlertRuleRow })
/** Configuration that saved but probably does not do what the caller meant. */
const RuleWriteWarnings = Schema.optionalKey(Schema.Array(Schema.String))

export const UpdateAlertRuleOutput = Schema.Struct({ rule: AlertRuleRow })
export const CreateAlertRuleOutput = Schema.Struct({ rule: AlertRuleRow, warnings: RuleWriteWarnings })

export const UpdateAlertRuleOutput = Schema.Struct({ rule: AlertRuleRow, warnings: RuleWriteWarnings })

export const DeleteAlertRuleOutput = Schema.Struct({ id: Schema.String })

Expand Down
35 changes: 34 additions & 1 deletion packages/domain/src/raw-sql.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Schema } from "effect"
import { describe, expect, it } from "vitest"
import { isValidRawSql, RawSqlText, rawSqlIssue } from "./raw-sql"
import { isValidRawSql, rawAlertSampleCountWarning, RawSqlText, rawSqlIssue } from "./raw-sql"

const ok = "SELECT count() FROM logs WHERE $__orgFilter AND $__timeFilter(Timestamp)"

Expand Down Expand Up @@ -165,3 +165,36 @@ describe("RawSqlText", () => {
)
})
})

describe("rawAlertSampleCountWarning", () => {
const noSamples = "SELECT count() AS value FROM traces WHERE $__orgFilter AND $__timeFilter(Timestamp)"
const withSamples =
"SELECT countIf(StatusCode = 'Error') / count() AS value, count() AS samples FROM traces WHERE $__orgFilter AND $__timeFilter(Timestamp)"

it("warns when a minimum above 1 would count rows", () => {
expect(rawAlertSampleCountWarning(noSamples, 50)).toMatch(/counts returned rows/)
})

it("stays quiet when the query selects samples or the minimum is trivial", () => {
expect(rawAlertSampleCountWarning(withSamples, 50)).toBeNull()
expect(rawAlertSampleCountWarning(noSamples, 1)).toBeNull()
expect(rawAlertSampleCountWarning(noSamples, 0)).toBeNull()
})

it("accepts a quoted alias and rejects one the engine would not read", () => {
const quoted = (alias: string) =>
`SELECT count() AS value, count() AS ${alias} FROM traces WHERE $__orgFilter AND $__timeFilter(Timestamp)`
expect(rawAlertSampleCountWarning(quoted("`samples`"), 10)).toBeNull()
expect(rawAlertSampleCountWarning(quoted('"samples"'), 10)).toBeNull()
expect(rawAlertSampleCountWarning(quoted("Samples"), 10)).not.toBeNull()
expect(rawAlertSampleCountWarning(quoted("samples_total"), 10)).not.toBeNull()
expect(rawAlertSampleCountWarning(`${noSamples} AND t.samples > 0`, 10)).not.toBeNull()
})

it("ignores samples mentioned only in a comment or string", () => {
expect(rawAlertSampleCountWarning(`${noSamples} -- samples`, 10)).not.toBeNull()
expect(
rawAlertSampleCountWarning(noSamples.replace("count()", "countIf(x = 'samples')"), 10),
).not.toBeNull()
})
})
20 changes: 20 additions & 0 deletions packages/domain/src/raw-sql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,3 +283,23 @@ export const RawSqlText = Schema.String.check(
title: "Raw SQL",
description: `ClickHouse SELECT with Maple macros. Must reference $__orgFilter. ${SUPPORTED_MACROS_HELP}`,
})

/**
* Whether an alert query aliases a column `AS samples` (the engine reads that
* exact, case-sensitive key). Without one every returned row counts as 1 sample,
* so a minimum sample count gates on buckets, not events.
*/
export const rawAlertSqlSelectsSamples = (sql: string): boolean => {
// Masking keeps offsets (a quoted alias becomes spaces), so read the alias from the original text.
for (const match of maskLiteralsAndComments(sql).matchAll(/\bAS\s/gi)) {
const alias = sql.slice(match.index + match[0].length).trimStart()
if (/^(?:samples|`samples`|"samples")(?![A-Za-z0-9_])/.test(alias)) return true
}
return false
}

/** The warning to show when a raw-SQL rule's minimum sample count is likely counting rows. */
export const rawAlertSampleCountWarning = (sql: string, minimumSampleCount: number): string | null =>
minimumSampleCount > 1 && !rawAlertSqlSelectsSamples(sql)
? `Minimum sample count ${minimumSampleCount} counts returned rows (one per bucket) because the query has no \`samples\` column. Select an event count as \`samples\` (e.g. \`count() AS samples\`) to gate on volume.`
: null
Loading