diff --git a/apps/ai/src/mcp/lib/alert-rules.ts b/apps/ai/src/mcp/lib/alert-rules.ts index 8c798c3339..c249864c01 100644 --- a/apps/ai/src/mcp/lib/alert-rules.ts +++ b/apps/ai/src/mcp/lib/alert-rules.ts @@ -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" @@ -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 => { + 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 = [], +): ToolDoc => ({ title, + ...(warnings.length > 0 ? { notices: warnings } : undefined), blocks: [ doc.fields([ ["ID", rule.id], diff --git a/apps/ai/src/mcp/tools/__tests__/alert-tools.test.ts b/apps/ai/src/mcp/tools/__tests__/alert-tools.test.ts index 56e0f61ef7..38acf8bdc0 100644 --- a/apps/ai/src/mcp/tools/__tests__/alert-tools.test.ts +++ b/apps/ai/src/mcp/tools/__tests__/alert-tools.test.ts @@ -16,6 +16,7 @@ import { UserId, } from "@maple/domain/http" import { + CreateAlertRuleOutput, GetAlertRuleOutput, ListAlertChecksOutput, ListAlertDestinationsOutput, @@ -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 @@ -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( diff --git a/apps/ai/src/mcp/tools/create-alert-rule.ts b/apps/ai/src/mcp/tools/create-alert-rule.ts index 94fe1f5b80..bad06f276c 100644 --- a/apps/ai/src/mcp/tools/create-alert-rule.ts +++ b/apps/ai/src/mcp/tools/create-alert-rule.ts @@ -16,6 +16,7 @@ import { ALERT_SEVERITIES, ALERT_SIGNAL_TYPES, renderRuleWrite, + ruleConfigWarnings, ruleWriteInputErrors, toAlertRuleRow, } from "../lib/alert-rules" @@ -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. (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.", ), @@ -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, @@ -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), }) } diff --git a/apps/ai/src/mcp/tools/get-alert-rule.ts b/apps/ai/src/mcp/tools/get-alert-rule.ts index 2b41727b71..963f84d470 100644 --- a/apps/ai/src/mcp/tools/get-alert-rule.ts +++ b/apps/ai/src/mcp/tools/get-alert-rule.ts @@ -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" @@ -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( diff --git a/apps/ai/src/mcp/tools/update-alert-rule.ts b/apps/ai/src/mcp/tools/update-alert-rule.ts index 3be531dc13..29c3777bcd 100644 --- a/apps/ai/src/mcp/tools/update-alert-rule.ts +++ b/apps/ai/src/mcp/tools/update-alert-rule.ts @@ -16,6 +16,7 @@ import { ALERT_SEVERITIES, ALERT_SIGNAL_TYPES, renderRuleWrite, + ruleConfigWarnings, ruleNotFound, ruleNotFoundFromError, ruleWriteInputErrors, @@ -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..", ), - 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"), @@ -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), }) } diff --git a/apps/web/src/components/alerts/signal-and-threshold-section.tsx b/apps/web/src/components/alerts/signal-and-threshold-section.tsx index aa0081b8e8..1a18eeb8a5 100644 --- a/apps/web/src/components/alerts/signal-and-threshold-section.tsx +++ b/apps/web/src/components/alerts/signal-and-threshold-section.tsx @@ -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" @@ -360,7 +361,11 @@ export function SignalAndThresholdSection({ onChange((c) => ({ ...c, minimumSampleCount: value }))} /> @@ -657,7 +662,11 @@ function SignalSubConfig({ form, onChange, autocompleteValues }: SignalAndThresh case "builder_query": return - case "raw_query": + case "raw_query": { + const sampleWarning = rawAlertSampleCountWarning( + form.rawQuerySql, + Number(form.minimumSampleCount) || 0, + ) return (

- Alert SQL must return a numeric value column. + Return a numeric value column. Optional: group for one + series per value, and samples 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.

+ {sampleWarning &&

{sampleWarning}

}
@@ -700,6 +713,7 @@ function SignalSubConfig({ form, onChange, autocompleteValues }: SignalAndThresh
) + } default: return null diff --git a/packages/backend/src/services/warehouse/warehouse-catalog.ts b/packages/backend/src/services/warehouse/warehouse-catalog.ts index 334f66039e..c0ad4f5e85 100644 --- a/packages/backend/src/services/warehouse/warehouse-catalog.ts +++ b/packages/backend/src/services/warehouse/warehouse-catalog.ts @@ -50,6 +50,7 @@ const TABLE_NOTES: Record> = { "`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.", diff --git a/packages/domain/src/mcp-outputs/alerts.ts b/packages/domain/src/mcp-outputs/alerts.ts index 9bc1400081..1edde5c8dd 100644 --- a/packages/domain/src/mcp-outputs/alerts.ts +++ b/packages/domain/src/mcp-outputs/alerts.ts @@ -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 }) diff --git a/packages/domain/src/raw-sql.test.ts b/packages/domain/src/raw-sql.test.ts index 01c63e6474..5c601bdfdf 100644 --- a/packages/domain/src/raw-sql.test.ts +++ b/packages/domain/src/raw-sql.test.ts @@ -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)" @@ -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() + }) +}) diff --git a/packages/domain/src/raw-sql.ts b/packages/domain/src/raw-sql.ts index 5a52422805..3a741d70b5 100644 --- a/packages/domain/src/raw-sql.ts +++ b/packages/domain/src/raw-sql.ts @@ -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