From 5a302eb6cd05f2d8b2adc82f4a6f495750a7e64f Mon Sep 17 00:00:00 2001 From: Makisuo Date: Sun, 11 Oct 2026 01:18:49 +0200 Subject: [PATCH 1/4] feat(query-engine): full attribute filters on metrics, create_dashboard where/unit/series count Metrics widgets could only filter a datapoint label by equality, and the executor read just the first attribute filter, ignoring its mode and negation. So a per-state metric like system.cpu.utilization could not be scoped with attr.state != "idle". - Metrics CH queries take the full attributeFilters list (equals, !=, contains, exists, in, numeric), built by the shared buildAttrFilterCondition. A lone span.kind equality still uses the hourly span-metrics rollup. - The query builder lowers every attr.* operator for metrics (cap 5) and accepts OR groups, which are the IN form. - create_dashboard simple specs gain `where`, enumerate `unit` from the widget unit catalog, and accept resource. group_by for metrics. - Validation entries report seriesCount so a group_by split is visible. --- apps/ai/src/mcp/lib/dashboard-schema-doc.ts | 7 +++- .../mcp/lib/inspect-widget-summary.test.ts | 28 +++++++++++++ apps/ai/src/mcp/lib/inspect-widget.ts | 11 ++++- .../ai/src/mcp/tools/create-dashboard.test.ts | 35 +++++++++++++++- apps/ai/src/mcp/tools/create-dashboard.ts | 37 +++++++++++----- packages/domain/src/mcp-outputs/dashboards.ts | 2 + .../src/ch/queries/metrics.test.ts | 38 +++++++++++++++++ .../query-engine/src/ch/queries/metrics.ts | 28 +++++++++++++ .../src/query-builder/model.test.ts | 41 ++++++++++++++++-- .../query-engine/src/query-builder/model.ts | 42 +++++++++---------- .../query-engine/src/runtime/query-engine.ts | 8 +--- 11 files changed, 232 insertions(+), 45 deletions(-) diff --git a/apps/ai/src/mcp/lib/dashboard-schema-doc.ts b/apps/ai/src/mcp/lib/dashboard-schema-doc.ts index 5bd3f87786..8a7f30fcd6 100644 --- a/apps/ai/src/mcp/lib/dashboard-schema-doc.ts +++ b/apps/ai/src/mcp/lib/dashboard-schema-doc.ts @@ -410,7 +410,7 @@ const queriesSection = (): string => { "### `whereClause` is a custom grammar, not SQL", "", "Operators (the only ones): `=`, `!=`, `>`, `<`, `>=`, `<=`, `contains`, `!contains`,", - "`exists`, `!exists`. Clauses join with ` AND `. On `traces` and `logs`, attribute", + "`exists`, `!exists`. Clauses join with ` AND `. On `traces`, `logs` and `metrics`, attribute", 'clauses can be OR-ed inside one level of parentheses: `(attr.a = "1" OR attr.b !exists)`.', "Every member must be an attribute on the same map (all span or all resource); named keys", "like `service.name` cannot be OR-ed, and there is no AND or nesting inside a group. A", @@ -424,6 +424,11 @@ const queriesSection = (): string => { 'a span attribute, so `db.system = "clickhouse"` works directly. Cap: 5 `attr.*` plus 5', "`resource.*` filters per query.", "", + "On `metrics`, `attr.` filters the datapoint labels with any operator. A metric reported", + "per `state` (`system.cpu.utilization`) averages to roughly 1/states unless scoped: `avg` with", + '`attr.state = "idle"` is the idle fraction (busy is 1 minus it), `attr.state != "idle"` drops', + 'idle, and an OR group is the `IN` form: `(attr.state = "user" OR attr.state = "system")`.', + "", "### Formulas and hidden series", "", "`formulas: [{ id, name, expression, legend }]` references queries by `name` (`A / B`), and", diff --git a/apps/ai/src/mcp/lib/inspect-widget-summary.test.ts b/apps/ai/src/mcp/lib/inspect-widget-summary.test.ts index 5011134108..7785f0fd5a 100644 --- a/apps/ai/src/mcp/lib/inspect-widget-summary.test.ts +++ b/apps/ai/src/mcp/lib/inspect-widget-summary.test.ts @@ -37,6 +37,34 @@ describe("summarizeOutcome", () => { expect(slow.note).toContain("may time out") }) + it("reports how many series a query-builder widget returned", () => { + const query = (queryId: string, status: "ok" | "error", seriesCount: number) => ({ + queryId, + queryName: queryId, + status, + stats: { rowCount: 10, seriesCount, seriesStats: [] }, + flags: [], + }) + const entry = summarizeOutcome(widget, { + kind: "supported", + data: { + widget: { + id: "w-1", + visualization: "chart", + endpoint: "custom_query_builder_timeseries", + hasFormulaWarning: false, + hasUnsupportedTransform: false, + }, + timeRange, + queries: [query("A", "ok", 4), query("B", "ok", 2), query("C", "error", 9)], + verdict: "looks_healthy", + flags: [], + notes: [], + }, + }) + expect(entry.seriesCount).toBe(6) + }) + it("scores a funnel widget from its query instead of skipping it", () => { const funnel = (first: number): InspectionOutcome => ({ kind: "unsupported", diff --git a/apps/ai/src/mcp/lib/inspect-widget.ts b/apps/ai/src/mcp/lib/inspect-widget.ts index cf8e23a561..e50409a2b8 100644 --- a/apps/ai/src/mcp/lib/inspect-widget.ts +++ b/apps/ai/src/mcp/lib/inspect-widget.ts @@ -842,6 +842,10 @@ export function summarizeOutcome(widget: DashboardWidget, outcome: InspectionOut // change, so it rides along on the mutation-tool summary rather than only // appearing in a full `inspect_chart_data` response. const actionableNote = outcome.data.notes.find((note) => note.startsWith('unit "percent')) + const seriesCount = outcome.data.queries.reduce( + (total, query) => total + (query.status === "ok" ? query.stats.seriesCount : 0), + 0, + ) return { widgetId: widget.id, ...(widget.display.title !== undefined ? { title: widget.display.title } : undefined), @@ -849,6 +853,7 @@ export function summarizeOutcome(widget: DashboardWidget, outcome: InspectionOut verdict: outcome.data.verdict satisfies WidgetInspectionVerdict, flags: [...outcome.data.flags], ...(actionableNote !== undefined ? { note: actionableNote } : undefined), + seriesCount, } } if (outcome.kind === "unsupported" && outcome.funnel !== undefined) { @@ -1062,8 +1067,9 @@ export function validationDoc( const single = options.single && summary.inspected.length === 1 ? summary.inspected[0] : undefined if (single !== undefined) { const flagPart = single.flags.length > 0 ? ` (${single.flags.join(", ")})` : "" + const seriesPart = single.seriesCount !== undefined ? `, ${single.seriesCount} series` : "" const lines = [ - `${verdictIcon(single.verdict)} "${single.title ?? single.widgetId}": ${single.verdict}${flagPart}`, + `${verdictIcon(single.verdict)} "${single.title ?? single.widgetId}": ${single.verdict}${seriesPart}${flagPart}`, ] if (single.note) lines.push(single.note) if (single.verdict === "suspicious" || single.verdict === "broken") { @@ -1093,8 +1099,9 @@ export function validationDoc( doc.list( summary.inspected.map((entry) => { const flagPart = entry.flags.length > 0 ? ` (${entry.flags.join(", ")})` : "" + const seriesPart = entry.seriesCount !== undefined ? `, ${entry.seriesCount} series` : "" const notePart = entry.note ? `: ${entry.note}` : "" - return `${verdictIcon(entry.verdict)} ${entry.widgetId} "${entry.title ?? ""}": ${entry.verdict}${flagPart}${notePart}` + return `${verdictIcon(entry.verdict)} ${entry.widgetId} "${entry.title ?? ""}": ${entry.verdict}${seriesPart}${flagPart}${notePart}` }), ), ) diff --git a/apps/ai/src/mcp/tools/create-dashboard.test.ts b/apps/ai/src/mcp/tools/create-dashboard.test.ts index 3a1418495c..8307dad94b 100644 --- a/apps/ai/src/mcp/tools/create-dashboard.test.ts +++ b/apps/ai/src/mcp/tools/create-dashboard.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest" -import { normalizeGroupBy } from "./create-dashboard" +import { buildSimpleWidgets, normalizeGroupBy } from "./create-dashboard" describe("normalizeGroupBy", () => { it("accepts query_data spellings", () => { @@ -14,3 +14,36 @@ describe("normalizeGroupBy", () => { expect(normalizeGroupBy("attr.signal", "metrics")).toBe("attr.signal") }) }) + +describe("buildSimpleWidgets", () => { + const cpu = { + title: "Idle CPU by host", + source: "metrics" as const, + metric_name: "system.cpu.utilization", + metric_type: "gauge" as const, + group_by: "resource.host.name", + where: 'attr.state = "idle"', + unit: "percent", + } + + it("accepts a resource. group_by for metrics", () => { + expect(typeof buildSimpleWidgets([cpu])).not.toBe("string") + expect(buildSimpleWidgets([{ ...cpu, source: "logs" }])).toContain( + 'invalid group_by "resource.host.name"', + ) + }) + + it("AND-s a where clause with service_name into the query draft", () => { + const widgets = buildSimpleWidgets([{ ...cpu, group_by: "attr.cpu", service_name: "node" }]) + expect(typeof widgets).not.toBe("string") + expect(JSON.stringify(widgets)).toContain( + JSON.stringify('service.name = "node" AND attr.state = "idle"'), + ) + }) + + it("rejects a where clause on a list widget instead of dropping it", () => { + const { group_by: _groupBy, ...rest } = cpu + const result = buildSimpleWidgets([{ ...rest, source: "logs", visualization: "list" }]) + expect(result).toContain("where is not supported on list widgets") + }) +}) diff --git a/apps/ai/src/mcp/tools/create-dashboard.ts b/apps/ai/src/mcp/tools/create-dashboard.ts index 30de56c728..73532e38a4 100644 --- a/apps/ai/src/mcp/tools/create-dashboard.ts +++ b/apps/ai/src/mcp/tools/create-dashboard.ts @@ -6,6 +6,7 @@ import { DashboardPersistenceService } from "@maple/backend/services/dashboards/ import { DashboardTemplateParameterKey, PortableDashboardDocument, + WIDGET_UNIT_TOKENS, defaultWidgetLayout, findNextPosition, } from "@maple/domain/http" @@ -55,7 +56,9 @@ const SimpleWidgetSpec = Schema.Struct({ metric_type: Schema.optionalKey(Schema.Literals(QUERY_BUILDER_METRIC_TYPES)), service_name: Schema.optionalKey(Schema.String), group_by: Schema.optionalKey(Schema.String), - unit: Schema.optionalKey(Schema.String), + /** A whereClause in the query-builder grammar, AND-ed with `service_name`. */ + where: Schema.optionalKey(Schema.String), + unit: Schema.optionalKey(Schema.Literals(WIDGET_UNIT_TOKENS)), }) type SimpleWidgetSpec = typeof SimpleWidgetSpec.Type @@ -86,6 +89,14 @@ const VALID_GROUP_BY: Record = { /** Sources whose query builder also groups by an attribute key, `attr.`. */ const ATTR_GROUP_BY_SOURCES: ReadonlySet = new Set(["metrics", "product_events"]) +/** Sources that also group by a resource attribute, `resource.` (host.name, k8s.pod.name). */ +const RESOURCE_GROUP_BY_SOURCES: ReadonlySet = new Set(["metrics"]) + +const prefixedGroupBys = (source: string): string[] => [ + ...(ATTR_GROUP_BY_SOURCES.has(source) ? ["attr."] : []), + ...(RESOURCE_GROUP_BY_SOURCES.has(source) ? ["resource."] : []), +] + /** query_data's group_by spellings, accepted here too so one vocabulary works in both tools. */ const QUERY_DATA_GROUP_BY_ALIASES: ReadonlyMap = new Map([ ["service", "service.name"], @@ -110,8 +121,10 @@ function validateGroupBy(rawGroupBy: string, source: string, widgetTitle: string if (validOptions.includes(rawGroupBy)) return null if (allowsAttr && rawGroupBy.startsWith("attr.") && rawGroupBy.length > 5) return null + if (RESOURCE_GROUP_BY_SOURCES.has(source) && rawGroupBy.startsWith("resource.") && rawGroupBy.length > 9) + return null - const optsList = [...validOptions, ...(allowsAttr ? ["attr."] : [])] + const optsList = [...validOptions, ...prefixedGroupBys(source)] return `Widget "${widgetTitle}": invalid group_by "${rawGroupBy}" for source=${source}. Valid: ${optsList.join(", ")}. ${allowsAttr ? "Example: attr.signal" : ""}` } @@ -131,7 +144,9 @@ function simpleSpecToWidget( const metricType = spec.metric_type const metric = spec.metric ?? (source === "metrics" ? "avg" : "count") - const where = spec.service_name ? `service.name = "${spec.service_name}"` : "" + const where = [spec.service_name ? `service.name = "${spec.service_name}"` : "", spec.where?.trim() ?? ""] + .filter((clause) => clause !== "") + .join(" AND ") let groupBy: string[] if (spec.group_by) { @@ -183,6 +198,9 @@ function simpleSpecToWidget( } if (viz === "list") { + if (spec.where?.trim()) { + return `Widget "${spec.title}": where is not supported on list widgets; use service_name, or a chart/table.` + } if (source === "logs") { return { id, @@ -268,7 +286,7 @@ function computeAutoLayout( return placed.map((widget) => widget.layout) } -function buildSimpleWidgets(specs: ReadonlyArray): WidgetDef[] | string { +export function buildSimpleWidgets(specs: ReadonlyArray): WidgetDef[] | string { const layouts = computeAutoLayout(specs) const widgets: WidgetDef[] = [] const errors: string[] = [] @@ -305,10 +323,7 @@ export function registerCreateDashboardTool(server: McpToolRegistrar) { // (`listTemplateMetadata`); only the separator is this tool's. const templateList = DASHBOARD_TEMPLATES.map((t) => ` ${t.id}: ${t.description}`).join("\n") const groupByDoc = (source: keyof typeof VALID_GROUP_BY) => - [ - ...(VALID_GROUP_BY[source] ?? []), - ...(ATTR_GROUP_BY_SOURCES.has(source) ? ["attr."] : []), - ].join("|") + [...(VALID_GROUP_BY[source] ?? []), ...prefixedGroupBys(source)].join("|") server.define({ name: TOOL, @@ -342,9 +357,11 @@ export function registerCreateDashboardTool(server: McpToolRegistrar) { ), widgets: P.optionalJson( Schema.Array(SimpleWidgetSpec), - `Simplified widget specs, one per widget: { title, visualization?: ${SIMPLE_SPEC_VISUALIZATIONS.join("|")}, source: ${QUERY_BUILDER_DATA_SOURCES.join("|")}, metric?, metric_name?, metric_type?, service_name?, group_by?, unit? }. ` + + `Simplified widget specs, one per widget: { title, visualization?: ${SIMPLE_SPEC_VISUALIZATIONS.join("|")}, source: ${QUERY_BUILDER_DATA_SOURCES.join("|")}, metric?, metric_name?, metric_type?, service_name?, group_by?, where?, unit?: ${WIDGET_UNIT_TOKENS.join("|")} }. ` + `Not the query_data vocabulary: group_by is ${groupByDoc("traces")} for traces, ${groupByDoc("logs")} for logs, ${groupByDoc("metrics")} for metrics, ${groupByDoc("product_events")} for product_events. ` + - "table needs a group_by; list shows recent traces or logs. Charts default to group_by service.name.", + "table needs a group_by; list shows recent traces or logs. Charts default to group_by service.name. " + + 'where is a whereClause (describe_dashboard_schema "queries"), e.g. attr.state = "idle" to read one state of a per-state metric like system.cpu.utilization (unscoped it averages to about 1/states). ' + + "unit percent expects a 0-1 fraction, percent_100 a 0-100 value.", ), dashboard_json: optionalJsonText( PortableDashboardDocument, diff --git a/packages/domain/src/mcp-outputs/dashboards.ts b/packages/domain/src/mcp-outputs/dashboards.ts index 61eea5dc93..41def5cf3e 100644 --- a/packages/domain/src/mcp-outputs/dashboards.ts +++ b/packages/domain/src/mcp-outputs/dashboards.ts @@ -57,6 +57,8 @@ export const WidgetInspectionEntry = Schema.Struct({ verdict: WidgetInspectionVerdict, flags: Schema.Array(InspectChartFlag), note: Schema.optionalKey(Schema.String), + /** Series (or breakdown groups) the widget's queries returned, so a `group_by` split is visible. */ + seriesCount: Schema.optionalKey(Schema.Number), }) /** The automatic check the mutation tools run on the widgets they wrote. */ diff --git a/packages/query-engine/src/ch/queries/metrics.test.ts b/packages/query-engine/src/ch/queries/metrics.test.ts index c9926e3f74..d49c9f05bc 100644 --- a/packages/query-engine/src/ch/queries/metrics.test.ts +++ b/packages/query-engine/src/ch/queries/metrics.test.ts @@ -327,6 +327,30 @@ describe("metricsTimeseriesRateQuery", () => { expect(sql).not.toContain("span_metrics_calls_hourly") }) + it("serves a lone span.kind attribute filter from the hourly rollup", () => { + const q = metricsTimeseriesRateQuery({ + metricName: "span.metrics.calls", + bucketSeconds: 3600, + attributeFilters: [{ key: "span.kind", mode: "equals", value: "SPAN_KIND_SERVER" }], + }) + const { sql } = compileUnsafe(q, { ...baseParams, metricName: "span.metrics.calls" }) + expect(sql).toContain("span_metrics_calls_hourly") + expect(sql).toContain("SpanKind = 'SPAN_KIND_SERVER'") + }) + + it("falls back to raw metrics_sum for a negated attribute filter", () => { + const q = metricsTimeseriesRateQuery({ + metricName: "span.metrics.calls", + bucketSeconds: 3600, + attributeFilters: [ + { key: "span.kind", mode: "equals", value: "SPAN_KIND_SERVER", negated: true }, + ], + }) + const { sql } = compileUnsafe(q, { ...baseParams, metricName: "span.metrics.calls" }) + expect(sql).toContain("FROM metrics_sum") + expect(sql).toContain("NOT (Attributes['span.kind'] = 'SPAN_KIND_SERVER')") + }) + it("falls back to raw metrics_sum when attributeValue has no attributeKey", () => { const q = metricsTimeseriesRateQuery({ metricName: "span.metrics.calls", @@ -463,6 +487,20 @@ describe("metricsBreakdownQuery", () => { expect(sql).toContain("metrics_sum.Attributes['state'] = 'idle'") }) + it("applies negated and IN attribute filters on the datapoint labels", () => { + const q = metricsBreakdownQuery({ + metricType: "gauge", + groupByResourceAttributeKey: "host.name", + attributeFilters: [ + { key: "state", mode: "equals", value: "idle", negated: true }, + { key: "cpu", mode: "in", values: ["cpu0", "cpu1"] }, + ], + }) + const { sql } = compileUnsafe(q, baseParams) + expect(sql).toContain("NOT (Attributes['state'] = 'idle')") + expect(sql).toContain("Attributes['cpu'] IN ('cpu0', 'cpu1')") + }) + it("ranks groups by the requested aggregate before the limit", () => { const { sql } = compileUnsafe(metricsBreakdownQuery({ metricType: "sum", rankBy: "max" }), baseParams) expect(sql).toContain("ORDER BY maxValue DESC") diff --git a/packages/query-engine/src/ch/queries/metrics.ts b/packages/query-engine/src/ch/queries/metrics.ts index 714bdc4f29..ca8ef5ce53 100644 --- a/packages/query-engine/src/ch/queries/metrics.ts +++ b/packages/query-engine/src/ch/queries/metrics.ts @@ -41,6 +41,13 @@ function datapointAttrCondition(column: CH.Expr, value: string | undefin return value === undefined ? column.neq("") : column.eq(value) } +/** WHERE conditions for `attributeFilters`: any mode or negation on the datapoint Attributes map. */ +function datapointFilterConditions( + filters: readonly AttributeFilter[] | undefined, +): ReadonlyArray { + return (filters ?? []).map((af) => buildAttrFilterCondition(af, "Attributes")) +} + // Shared options & output types interface MetricsQueryOpts { @@ -53,6 +60,8 @@ interface MetricsQueryOpts { groupByResourceAttributeKey?: string attributeKey?: string attributeValue?: string + /** Datapoint Attributes predicates, AND-ed with `attributeKey`/`attributeValue`. */ + attributeFilters?: readonly AttributeFilter[] resourceAttributeFilters?: readonly AttributeFilter[] groupBy?: readonly string[] seriesLimit?: number @@ -120,6 +129,7 @@ export function metricsTimeseriesQuery(opts: MetricsTimeseriesOpts) { opts.environments?.length ? CH.inList(deploymentEnvExpr($.ResourceAttributes), opts.environments) : undefined, + ...datapointFilterConditions(opts.attributeFilters), ...resourceFilterConditions(opts.resourceAttributeFilters), ]) @@ -153,6 +163,8 @@ export interface MetricsRateTimeseriesOpts { groupByResourceAttributeKey?: string attributeKey?: string attributeValue?: string + /** Datapoint Attributes predicates, AND-ed with `attributeKey`/`attributeValue`. */ + attributeFilters?: readonly AttributeFilter[] resourceAttributeFilters?: readonly AttributeFilter[] groupBy?: readonly string[] seriesLimit?: number @@ -184,6 +196,15 @@ const metricsRateTimeseriesColumns = { // from both sides, so the rollup held 0 rows and every read took the raw path. const SPAN_METRICS_CALLS_NAMES = new Set(["span.metrics.calls", "calls", "traces.span.metrics.calls"]) +// The value of a lone, plain `span.kind = X` filter, else undefined. +function spanKindEqualsValue(filters: readonly AttributeFilter[] | undefined): string | undefined { + const [only, ...rest] = filters ?? [] + if (only === undefined || rest.length > 0) return undefined + return only.key === "span.kind" && only.mode === "equals" && !only.negated && !only.or?.length + ? only.value + : undefined +} + function canUseSpanMetricsCallsHourly(opts: MetricsRateTimeseriesOpts): boolean { return ( // The hourly MV is keyed by a single MetricName; an `IN (...)` candidate set @@ -198,6 +219,9 @@ function canUseSpanMetricsCallsHourly(opts: MetricsRateTimeseriesOpts): boolean (opts.attributeValue === undefined || opts.attributeKey !== undefined) && (opts.attributeKey === undefined || opts.attributeKey === "span.kind") && (opts.groupByAttributeKey === undefined || opts.groupByAttributeKey === "span.kind") && + // The hourly MV keeps only SpanKind of the datapoint labels. + ((opts.attributeFilters?.length ?? 0) === 0 || + spanKindEqualsValue(opts.attributeFilters) !== undefined) && // The hourly MV folds ResourceAttributes into a fingerprint — it cannot // serve resource-attribute filters or group-bys, and `deployment.environment` // lives in that same map, so an environment filter also drops to the raw path. @@ -249,6 +273,7 @@ function metricsTimeseriesRateFromSpanMetricsCallsHourly( CH.when(opts.attributeKey === "span.kind" ? opts.attributeValue : undefined, (v: string) => $.SpanKind.eq(v), ), + CH.when(spanKindEqualsValue(opts.attributeFilters), (v: string) => $.SpanKind.eq(v)), ]) .groupBy( "Hour", @@ -427,6 +452,7 @@ export function metricsTimeseriesRateQuery( opts.environments?.length ? CH.inList(deploymentEnvExpr($.ResourceAttributes), opts.environments) : undefined, + ...datapointFilterConditions(opts.attributeFilters), ...resourceFilterConditions(opts.resourceAttributeFilters), ]) @@ -543,6 +569,7 @@ export interface MetricsBreakdownOpts { environments?: readonly string[] attributeKey?: string attributeValue?: string + attributeFilters?: readonly AttributeFilter[] /** Aggregate that ranks groups before `limit` applies. Default: count. */ rankBy?: "avg" | "sum" | "min" | "max" | "count" limit?: number @@ -604,6 +631,7 @@ export function metricsBreakdownQuery(opts: MetricsBreakdownOpts) { opts.environments?.length ? CH.inList(deploymentEnvExpr($.ResourceAttributes), opts.environments) : undefined, + ...datapointFilterConditions(opts.attributeFilters), ...resourceFilterConditions(opts.resourceAttributeFilters), ]) .groupBy("name") diff --git a/packages/query-engine/src/query-builder/model.test.ts b/packages/query-engine/src/query-builder/model.test.ts index 0d706fd146..019e598fa1 100644 --- a/packages/query-engine/src/query-builder/model.test.ts +++ b/packages/query-engine/src/query-builder/model.test.ts @@ -347,6 +347,38 @@ describe("metrics resource.* support", () => { expect(filters?.environments).toEqual(["production", "staging"]) }) + it("lowers a negated attr.* filter so per-state CPU can exclude idle", () => { + const { warnings, filters } = metricsFiltersOf({ whereClause: 'attr.state != "idle"' }) + expect(warnings).toEqual([]) + expect(filters?.attributeFilters).toEqual([ + { key: "state", mode: "equals", negated: true, value: "idle" }, + ]) + }) + + it("keeps several attr.* filters and lowers OR groups onto one filter", () => { + const { warnings, filters } = metricsFiltersOf({ + whereClause: 'attr.cpu exists AND (attr.state = "user" OR attr.state = "system")', + }) + expect(warnings).toEqual([]) + expect(filters?.attributeFilters).toEqual([ + { key: "cpu", mode: "exists" }, + { + key: "state", + mode: "equals", + value: "user", + or: [{ key: "state", mode: "equals", value: "system" }], + }, + ]) + }) + + it("drops a metrics OR group that mixes in a named dimension", () => { + const { warnings, filters } = metricsFiltersOf({ + whereClause: '(attr.state = "user" OR service.name = "api")', + }) + expect(filters).not.toHaveProperty("attributeFilters") + expect(warnings.some((w) => w.includes("Metrics OR group ignored"))).toBe(true) + }) + it("warns when the 5 resource-filter cap is exceeded", () => { const clause = ["a", "b", "c", "d", "e", "f"].map((k) => `resource.${k} = "1"`).join(" AND ") const { warnings, filters } = metricsFiltersOf({ whereClause: clause }) @@ -719,7 +751,7 @@ describe("buildTimeseriesQuerySpec where-clause OR groups", () => { } }) - it("applies groups to logs and reports them as unsupported elsewhere", () => { + it("applies groups to logs and metrics and reports them as unsupported elsewhere", () => { const logs = buildTimeseriesQuerySpec( tracesDraft({ dataSource: "logs", whereClause: '(attr.a = "1" OR attr.b = "2")' }), ) @@ -736,9 +768,10 @@ describe("buildTimeseriesQuerySpec where-clause OR groups", () => { whereClause: '(attr.a = "1" OR attr.b = "2")', }), ) - expect(metrics.warnings).toContain( - 'Unsupported clause syntax ignored: (attr.a = "1" OR attr.b = "2")', - ) + expect(metrics.warnings).toEqual([]) + expect(metrics.query).toMatchObject({ + filters: { attributeFilters: [{ key: "a", value: "1", or: [{ key: "b", value: "2" }] }] }, + }) const productEvents = buildTimeseriesQuerySpec( tracesDraft({ diff --git a/packages/query-engine/src/query-builder/model.ts b/packages/query-engine/src/query-builder/model.ts index f840ae85c1..f0d0d51a22 100644 --- a/packages/query-engine/src/query-builder/model.ts +++ b/packages/query-engine/src/query-builder/model.ts @@ -413,12 +413,17 @@ function applyOrGroup( ): A { const label = `(${group.map((c) => `${typedKey(c)} ${c.operator} ${c.value}`.trim()).join(" OR ")})` const members: Array<{ map: keyof AttributeFilterAccumulator; filter: AccumulatedAttributeFilter }> = [] + const emptyFields = new Map(Object.entries(empty)) for (const clause of group) { const memberWarnings: string[] = [] const next = apply(empty, clause, memberWarnings) + // Fields the empty accumulator already carries (the metrics metric name) are not set by the member. const setsOtherField = Object.entries(next).some( ([field, value]) => - field !== "attributeFilters" && field !== "resourceAttributeFilters" && value !== undefined, + field !== "attributeFilters" && + field !== "resourceAttributeFilters" && + value !== undefined && + value !== emptyFields.get(field), ) const map = next.attributeFilters.length === 1 && next.resourceAttributeFilters.length === 0 @@ -976,19 +981,12 @@ function applyMetricsClause( ): MetricsFilterAccumulator { const key = normalizeKey(clause.key) - // Datapoint labels live in the Attributes map. The metrics CH path carries a - // single equality predicate on it (attributeKey/attributeValue), so exactly - // one `attr. = value` clause is honored; anything else warns instead of - // being silently dropped. + // Datapoint labels live in the Attributes map. Every operator applies, so + // `attr.state != idle` can drop one state of a per-state metric. if (key.startsWith("attr.")) { const attributeKey = typedKey(clause, 5) - const { mode, negated } = operatorToAttrFilter(clause.operator) - if (mode !== "equals" || negated) { - warnings.push(`Metrics attr.* filters support only equality; ignoring attr.${attributeKey}`) - return filters - } - if (filters.attributeFilters.length >= 1) { - warnings.push(`Metrics queries support a single attr.* filter; ignoring attr.${attributeKey}`) + if (filters.attributeFilters.length >= 5) { + warnings.push(`Maximum of 5 attr.* filters supported; ignoring attr.${attributeKey}`) return filters } return { @@ -1454,7 +1452,8 @@ export function buildTimeseriesQuerySpec(query: QueryBuilderQueryDraftPayload): } = parseWhereClause(query.whereClause ?? "", { // Only the sources that lower groups ask for them. Any other source gets the // parser's "unsupported clause" warning, so a group is never dropped silently. - orGroups: query.dataSource === "traces" || query.dataSource === "logs", + orGroups: + query.dataSource === "traces" || query.dataSource === "logs" || query.dataSource === "metrics", }) for (const w of parseWarnings) warnings.push(w.message) @@ -1668,14 +1667,15 @@ export function buildTimeseriesQuerySpec(query: QueryBuilderQueryDraftPayload): } } - const metricsFilters = clauses.reduce( - (acc, clause) => applyMetricsClause(acc, clause, warnings), - { - metricName: query.metricName, - metricType: query.metricType ?? "gauge", - attributeFilters: [], - resourceAttributeFilters: [], - }, + const emptyMetrics: MetricsFilterAccumulator = { + metricName: query.metricName, + metricType: query.metricType ?? "gauge", + attributeFilters: [], + resourceAttributeFilters: [], + } + const metricsFilters = groups.reduce( + (acc, group) => applyOrGroup(acc, group, applyMetricsClause, emptyMetrics, "Metrics", warnings), + clauses.reduce((acc, clause) => applyMetricsClause(acc, clause, warnings), emptyMetrics), ) const metricsGroupByKeys: MetricsGroupByKey[] = [] diff --git a/packages/query-engine/src/runtime/query-engine.ts b/packages/query-engine/src/runtime/query-engine.ts index 29e8516475..705102442c 100644 --- a/packages/query-engine/src/runtime/query-engine.ts +++ b/packages/query-engine/src/runtime/query-engine.ts @@ -972,14 +972,12 @@ const executeMetricsTimeseriesRows = Effect.fnUntraced(function* Date: Sun, 11 Oct 2026 01:34:22 +0200 Subject: [PATCH 2/4] fix: count breakdown groups in seriesCount, drop legacy metrics label options - seriesCount counted a breakdown as 1 series (its groups are rows) and summed chart queries, double-counting formula operands. It now takes the largest query, using rowCount for breakdowns. - Remove attributeKey/attributeValue from the metrics query opts; only tests still used them. attributeFilters is the single way to filter datapoint labels, which also simplifies the span-metrics rollup check. - create_dashboard computes the extra where clause once; its tests read the query draft instead of matching serialized JSON. --- .../mcp/lib/inspect-widget-summary.test.ts | 32 +++++++++++---- apps/ai/src/mcp/lib/inspect-widget.ts | 8 +++- .../ai/src/mcp/tools/create-dashboard.test.ts | 14 +++---- apps/ai/src/mcp/tools/create-dashboard.ts | 5 ++- packages/domain/src/mcp-outputs/dashboards.ts | 2 +- .../src/ch/queries/metrics.test.ts | 40 ++++++------------- .../query-engine/src/ch/queries/metrics.ts | 30 ++------------ packages/query-engine/src/traces-shared.ts | 2 +- 8 files changed, 59 insertions(+), 74 deletions(-) diff --git a/apps/ai/src/mcp/lib/inspect-widget-summary.test.ts b/apps/ai/src/mcp/lib/inspect-widget-summary.test.ts index 7785f0fd5a..4a1c3c15d6 100644 --- a/apps/ai/src/mcp/lib/inspect-widget-summary.test.ts +++ b/apps/ai/src/mcp/lib/inspect-widget-summary.test.ts @@ -1,4 +1,9 @@ import { describe, expect, it } from "vitest" +import { + makeQueryBuilderBreakdownDataSource, + makeQueryBuilderTimeseriesDataSource, + makeQueryDraft, +} from "@maple/backend/dashboard-templates/helpers" import { summarizeOutcome, SLOW_RAW_SQL_MS, type InspectionOutcome } from "./inspect-widget" const widget = { @@ -37,15 +42,16 @@ describe("summarizeOutcome", () => { expect(slow.note).toContain("may time out") }) - it("reports how many series a query-builder widget returned", () => { - const query = (queryId: string, status: "ok" | "error", seriesCount: number) => ({ - queryId, - queryName: queryId, + describe("seriesCount", () => { + const draft = makeQueryDraft({ id: "q-a", name: "A", dataSource: "metrics", aggregation: "avg" }) + const query = (status: "ok" | "error", rowCount: number, seriesCount: number) => ({ + queryId: "q", + queryName: "q", status, - stats: { rowCount: 10, seriesCount, seriesStats: [] }, + stats: { rowCount, seriesCount, seriesStats: [] }, flags: [], }) - const entry = summarizeOutcome(widget, { + const supported = (queries: ReadonlyArray>): InspectionOutcome => ({ kind: "supported", data: { widget: { @@ -56,13 +62,23 @@ describe("summarizeOutcome", () => { hasUnsupportedTransform: false, }, timeRange, - queries: [query("A", "ok", 4), query("B", "ok", 2), query("C", "error", 9)], + queries: [...queries], verdict: "looks_healthy", flags: [], notes: [], }, }) - expect(entry.seriesCount).toBe(6) + + it("takes the largest chart query, so a formula over its operands is not counted twice", () => { + const chart = { ...widget, dataSource: makeQueryBuilderTimeseriesDataSource([draft]) } + const outcome = supported([query("ok", 60, 4), query("ok", 60, 4), query("error", 0, 9)]) + expect(summarizeOutcome(chart, outcome).seriesCount).toBe(4) + }) + + it("counts a breakdown's groups, which are its rows", () => { + const table = { ...widget, dataSource: makeQueryBuilderBreakdownDataSource([draft]) } + expect(summarizeOutcome(table, supported([query("ok", 12, 1)])).seriesCount).toBe(12) + }) }) it("scores a funnel widget from its query instead of skipping it", () => { diff --git a/apps/ai/src/mcp/lib/inspect-widget.ts b/apps/ai/src/mcp/lib/inspect-widget.ts index e50409a2b8..706dcf7381 100644 --- a/apps/ai/src/mcp/lib/inspect-widget.ts +++ b/apps/ai/src/mcp/lib/inspect-widget.ts @@ -842,8 +842,14 @@ export function summarizeOutcome(widget: DashboardWidget, outcome: InspectionOut // change, so it rides along on the mutation-tool summary rather than only // appearing in a full `inspect_chart_data` response. const actionableNote = outcome.data.notes.find((note) => note.startsWith('unit "percent')) + // A breakdown's groups are its rows. The largest query, not the sum, so hidden + // operands and the formula built from them are not counted twice. + const isBreakdown = dataSourceQuerySet(widget.dataSource)?.resultShape === "breakdown" const seriesCount = outcome.data.queries.reduce( - (total, query) => total + (query.status === "ok" ? query.stats.seriesCount : 0), + (largest, query) => + query.status === "ok" + ? Math.max(largest, isBreakdown ? query.stats.rowCount : query.stats.seriesCount) + : largest, 0, ) return { diff --git a/apps/ai/src/mcp/tools/create-dashboard.test.ts b/apps/ai/src/mcp/tools/create-dashboard.test.ts index 8307dad94b..7e461751f2 100644 --- a/apps/ai/src/mcp/tools/create-dashboard.test.ts +++ b/apps/ai/src/mcp/tools/create-dashboard.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "vitest" +import { dataSourceQuerySet } from "@maple/widgets/dashboard" import { buildSimpleWidgets, normalizeGroupBy } from "./create-dashboard" describe("normalizeGroupBy", () => { @@ -34,16 +35,15 @@ describe("buildSimpleWidgets", () => { }) it("AND-s a where clause with service_name into the query draft", () => { - const widgets = buildSimpleWidgets([{ ...cpu, group_by: "attr.cpu", service_name: "node" }]) - expect(typeof widgets).not.toBe("string") - expect(JSON.stringify(widgets)).toContain( - JSON.stringify('service.name = "node" AND attr.state = "idle"'), - ) + const widgets = buildSimpleWidgets([{ ...cpu, service_name: "node" }]) + const draft = typeof widgets === "string" ? undefined : dataSourceQuerySet(widgets[0]?.dataSource) + expect(draft?.queries[0]?.whereClause).toBe('service.name = "node" AND attr.state = "idle"') }) it("rejects a where clause on a list widget instead of dropping it", () => { - const { group_by: _groupBy, ...rest } = cpu - const result = buildSimpleWidgets([{ ...rest, source: "logs", visualization: "list" }]) + const result = buildSimpleWidgets([ + { title: "Recent logs", source: "logs", visualization: "list", where: 'attr.a = "1"' }, + ]) expect(result).toContain("where is not supported on list widgets") }) }) diff --git a/apps/ai/src/mcp/tools/create-dashboard.ts b/apps/ai/src/mcp/tools/create-dashboard.ts index 73532e38a4..ea8e29b294 100644 --- a/apps/ai/src/mcp/tools/create-dashboard.ts +++ b/apps/ai/src/mcp/tools/create-dashboard.ts @@ -144,7 +144,8 @@ function simpleSpecToWidget( const metricType = spec.metric_type const metric = spec.metric ?? (source === "metrics" ? "avg" : "count") - const where = [spec.service_name ? `service.name = "${spec.service_name}"` : "", spec.where?.trim() ?? ""] + const extraWhere = spec.where?.trim() ?? "" + const where = [spec.service_name ? `service.name = "${spec.service_name}"` : "", extraWhere] .filter((clause) => clause !== "") .join(" AND ") @@ -198,7 +199,7 @@ function simpleSpecToWidget( } if (viz === "list") { - if (spec.where?.trim()) { + if (extraWhere !== "") { return `Widget "${spec.title}": where is not supported on list widgets; use service_name, or a chart/table.` } if (source === "logs") { diff --git a/packages/domain/src/mcp-outputs/dashboards.ts b/packages/domain/src/mcp-outputs/dashboards.ts index 41def5cf3e..e3020e9c4c 100644 --- a/packages/domain/src/mcp-outputs/dashboards.ts +++ b/packages/domain/src/mcp-outputs/dashboards.ts @@ -57,7 +57,7 @@ export const WidgetInspectionEntry = Schema.Struct({ verdict: WidgetInspectionVerdict, flags: Schema.Array(InspectChartFlag), note: Schema.optionalKey(Schema.String), - /** Series (or breakdown groups) the widget's queries returned, so a `group_by` split is visible. */ + /** Series (groups, for a breakdown) in the widget's largest query, so a `group_by` split is visible. */ seriesCount: Schema.optionalKey(Schema.Number), }) diff --git a/packages/query-engine/src/ch/queries/metrics.test.ts b/packages/query-engine/src/ch/queries/metrics.test.ts index d49c9f05bc..b418d0ecf3 100644 --- a/packages/query-engine/src/ch/queries/metrics.test.ts +++ b/packages/query-engine/src/ch/queries/metrics.test.ts @@ -102,11 +102,10 @@ describe("metricsTimeseriesQuery", () => { expect(sql).toContain("ServiceName = 'api'") }) - it("applies attribute key/value filter", () => { + it("applies an attribute filter", () => { const q = metricsTimeseriesQuery({ metricType: "sum", - attributeKey: "region", - attributeValue: "us-east-1", + attributeFilters: [{ key: "region", mode: "equals", value: "us-east-1" }], }) const { sql } = compileUnsafe(q, baseParams) expect(sql).toContain("Attributes['region'] = 'us-east-1'") @@ -213,10 +212,10 @@ describe("metricsTimeseriesRateQuery", () => { expect(sql).toContain("- INTERVAL 600 SECOND") }) - it("treats an attribute key with no value as 'has the label'", () => { - const q = metricsTimeseriesRateQuery({ attributeKey: "worker" }) + it("reads an exists filter as 'has a non-empty label'", () => { + const q = metricsTimeseriesRateQuery({ attributeFilters: [{ key: "worker", mode: "exists" }] }) const { sql } = compileUnsafe(q, baseParams) - expect(sql).toContain("metrics_sum.Attributes['worker'] != ''") + expect(sql).toContain("Attributes['worker'] != ''") }) it("applies serviceName filter in CTE", () => { @@ -225,10 +224,9 @@ describe("metricsTimeseriesRateQuery", () => { expect(sql).toContain("ServiceName = 'api'") }) - it("applies attributeKey filter", () => { + it("applies an attribute filter in the CTE", () => { const q = metricsTimeseriesRateQuery({ - attributeKey: "region", - attributeValue: "us-east-1", + attributeFilters: [{ key: "region", mode: "equals", value: "us-east-1" }], }) const { sql } = compileUnsafe(q, baseParams) expect(sql).toContain("Attributes['region'] = 'us-east-1'") @@ -296,11 +294,11 @@ describe("metricsTimeseriesRateQuery", () => { const q = metricsTimeseriesRateQuery({ metricName: "span.metrics.calls", bucketSeconds: 3600, - attributeKey: "span.kind", - attributeValue: "SPAN_KIND_SERVER", + attributeFilters: [{ key: "span.kind", mode: "equals", value: "SPAN_KIND_SERVER" }], groupByAttributeKey: "span.kind", }) const { sql } = compileUnsafe(q, { ...baseParams, metricName: "span.metrics.calls" }) + expect(sql).toContain("span_metrics_calls_hourly") expect(sql).toContain("SpanKind = 'SPAN_KIND_SERVER'") expect(sql).toContain("SpanKind AS attributeValue") expect(sql).toContain("GROUP BY bucket, serviceName, attributeValue") @@ -327,17 +325,6 @@ describe("metricsTimeseriesRateQuery", () => { expect(sql).not.toContain("span_metrics_calls_hourly") }) - it("serves a lone span.kind attribute filter from the hourly rollup", () => { - const q = metricsTimeseriesRateQuery({ - metricName: "span.metrics.calls", - bucketSeconds: 3600, - attributeFilters: [{ key: "span.kind", mode: "equals", value: "SPAN_KIND_SERVER" }], - }) - const { sql } = compileUnsafe(q, { ...baseParams, metricName: "span.metrics.calls" }) - expect(sql).toContain("span_metrics_calls_hourly") - expect(sql).toContain("SpanKind = 'SPAN_KIND_SERVER'") - }) - it("falls back to raw metrics_sum for a negated attribute filter", () => { const q = metricsTimeseriesRateQuery({ metricName: "span.metrics.calls", @@ -351,11 +338,11 @@ describe("metricsTimeseriesRateQuery", () => { expect(sql).toContain("NOT (Attributes['span.kind'] = 'SPAN_KIND_SERVER')") }) - it("falls back to raw metrics_sum when attributeValue has no attributeKey", () => { + it("falls back to raw metrics_sum for a label the rollup does not keep", () => { const q = metricsTimeseriesRateQuery({ metricName: "span.metrics.calls", bucketSeconds: 3600, - attributeValue: "SPAN_KIND_SERVER", + attributeFilters: [{ key: "http.route", mode: "equals", value: "/api" }], }) const { sql } = compileUnsafe(q, { ...baseParams, metricName: "span.metrics.calls" }) expect(sql).toContain("FROM metrics_sum") @@ -478,13 +465,12 @@ describe("metricsBreakdownQuery", () => { metricType: "sum", serviceName: "api", environments: ["production"], - attributeKey: "state", - attributeValue: "idle", + attributeFilters: [{ key: "state", mode: "equals", value: "idle" }], }) const { sql } = compileUnsafe(q, baseParams) expect(sql).toContain("metrics_sum.ServiceName = 'api'") expect(sql).toContain("'production'") - expect(sql).toContain("metrics_sum.Attributes['state'] = 'idle'") + expect(sql).toContain("Attributes['state'] = 'idle'") }) it("applies negated and IN attribute filters on the datapoint labels", () => { diff --git a/packages/query-engine/src/ch/queries/metrics.ts b/packages/query-engine/src/ch/queries/metrics.ts index ca8ef5ce53..f01281bfcb 100644 --- a/packages/query-engine/src/ch/queries/metrics.ts +++ b/packages/query-engine/src/ch/queries/metrics.ts @@ -36,11 +36,6 @@ function resourceFilterConditions( return (filters ?? []).map((rf) => buildAttrFilterCondition(rf, "ResourceAttributes")) } -// A key with no value means "has the label": `.eq("")` would match its absence. -function datapointAttrCondition(column: CH.Expr, value: string | undefined): CH.Condition { - return value === undefined ? column.neq("") : column.eq(value) -} - /** WHERE conditions for `attributeFilters`: any mode or negation on the datapoint Attributes map. */ function datapointFilterConditions( filters: readonly AttributeFilter[] | undefined, @@ -58,9 +53,7 @@ interface MetricsQueryOpts { groupByAttributeKey?: string /** Group by a ResourceAttributes key instead of a datapoint Attributes key. */ groupByResourceAttributeKey?: string - attributeKey?: string - attributeValue?: string - /** Datapoint Attributes predicates, AND-ed with `attributeKey`/`attributeValue`. */ + /** Predicates on the datapoint Attributes map (metric labels). */ attributeFilters?: readonly AttributeFilter[] resourceAttributeFilters?: readonly AttributeFilter[] groupBy?: readonly string[] @@ -123,9 +116,6 @@ export function metricsTimeseriesQuery(opts: MetricsTimeseriesOpts) { $.TimeUnix.gte(param.dateTime("startTime")), $.TimeUnix.lte(param.dateTime("endTime")), CH.when(opts.serviceName, (v: string) => $.ServiceName.eq(v)), - CH.when(opts.attributeKey, (k: string) => - datapointAttrCondition($.Attributes.get(k), opts.attributeValue), - ), opts.environments?.length ? CH.inList(deploymentEnvExpr($.ResourceAttributes), opts.environments) : undefined, @@ -161,9 +151,7 @@ export interface MetricsRateTimeseriesOpts { groupByAttributeKey?: string /** Group by a ResourceAttributes key instead of a datapoint Attributes key. */ groupByResourceAttributeKey?: string - attributeKey?: string - attributeValue?: string - /** Datapoint Attributes predicates, AND-ed with `attributeKey`/`attributeValue`. */ + /** Predicates on the datapoint Attributes map (metric labels). */ attributeFilters?: readonly AttributeFilter[] resourceAttributeFilters?: readonly AttributeFilter[] groupBy?: readonly string[] @@ -216,8 +204,6 @@ function canUseSpanMetricsCallsHourly(opts: MetricsRateTimeseriesOpts): boolean opts.bucketSeconds !== undefined && opts.bucketSeconds >= 3600 && opts.bucketSeconds % 3600 === 0 && - (opts.attributeValue === undefined || opts.attributeKey !== undefined) && - (opts.attributeKey === undefined || opts.attributeKey === "span.kind") && (opts.groupByAttributeKey === undefined || opts.groupByAttributeKey === "span.kind") && // The hourly MV keeps only SpanKind of the datapoint labels. ((opts.attributeFilters?.length ?? 0) === 0 || @@ -270,9 +256,6 @@ function metricsTimeseriesRateFromSpanMetricsCallsHourly( $.Hour.gte(previousBucket), $.Hour.lte(endBucket), CH.when(opts.serviceName, (v: string) => $.ServiceName.eq(v)), - CH.when(opts.attributeKey === "span.kind" ? opts.attributeValue : undefined, (v: string) => - $.SpanKind.eq(v), - ), CH.when(spanKindEqualsValue(opts.attributeFilters), (v: string) => $.SpanKind.eq(v)), ]) .groupBy( @@ -446,9 +429,6 @@ export function metricsTimeseriesRateQuery( ), $.TimeUnix.lte(param.dateTime("endTime")), CH.when(opts.serviceName, (v: string) => $.ServiceName.eq(v)), - CH.when(opts.attributeKey, (k: string) => - datapointAttrCondition($.Attributes.get(k), opts.attributeValue), - ), opts.environments?.length ? CH.inList(deploymentEnvExpr($.ResourceAttributes), opts.environments) : undefined, @@ -567,8 +547,7 @@ export interface MetricsBreakdownOpts { resourceAttributeFilters?: readonly AttributeFilter[] serviceName?: string environments?: readonly string[] - attributeKey?: string - attributeValue?: string + /** Predicates on the datapoint Attributes map (metric labels). */ attributeFilters?: readonly AttributeFilter[] /** Aggregate that ranks groups before `limit` applies. Default: count. */ rankBy?: "avg" | "sum" | "min" | "max" | "count" @@ -625,9 +604,6 @@ export function metricsBreakdownQuery(opts: MetricsBreakdownOpts) { CH.when(groupKey, (k: string) => $.Attributes.get(k).neq("")), CH.when(resourceGroupKey, (k: string) => $.ResourceAttributes.get(k).neq("")), CH.when(opts.serviceName, (v: string) => $.ServiceName.eq(v)), - CH.when(opts.attributeKey, (k: string) => - datapointAttrCondition($.Attributes.get(k), opts.attributeValue), - ), opts.environments?.length ? CH.inList(deploymentEnvExpr($.ResourceAttributes), opts.environments) : undefined, diff --git a/packages/query-engine/src/traces-shared.ts b/packages/query-engine/src/traces-shared.ts index 7378689151..0fb9fd0d45 100644 --- a/packages/query-engine/src/traces-shared.ts +++ b/packages/query-engine/src/traces-shared.ts @@ -131,7 +131,7 @@ export function buildAttrFilterCondition( .map((member) => buildAttrFilterCondition(member, mapName, "none")) .reduce((acc, cond) => acc.or(cond)) } - // `product_events.Attributes` carries no skip index, so the exact predicate stands alone. + // `Attributes` (product events, metric datapoints) carries no skip index, so the exact predicate stands alone. const indexMode: AttributeIndexMode = mapName === "Attributes" ? "none" : requestedIndexMode const mapExpr = CH.dynamicColumn>(mapName) // Attributes renamed across OTel semconv versions match either spelling, From 766dedb86ccda703af157abad59bc0673c60b150 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Mon, 12 Oct 2026 02:28:25 +0200 Subject: [PATCH 3/4] chore: clear knip findings from main Drop the unused TIME_PRESETS, bucketSecondsFor and OverviewGrain exports, and register the tool playground vitest config as an apps/ai knip entry. --- apps/web/src/components/infra/constants.ts | 30 +------------------ knip.json | 3 +- .../query-engine/src/ch/queries/services.ts | 7 ++--- 3 files changed, 6 insertions(+), 34 deletions(-) diff --git a/apps/web/src/components/infra/constants.ts b/apps/web/src/components/infra/constants.ts index 9d7f0ab2b1..de82297a92 100644 --- a/apps/web/src/components/infra/constants.ts +++ b/apps/web/src/components/infra/constants.ts @@ -1,32 +1,5 @@ import { toEpochMs } from "@maple/ui/lib/time-format" -// Shared controls for the infra detail routes. Per-resource metric tabs/strips -// stay in their own routes (genuinely page-specific); only the time-range -// vocabulary is common. - -export const TIME_PRESETS = [ - { value: "15m", label: "Last 15 minutes" }, - { value: "1h", label: "Last hour" }, - { value: "6h", label: "Last 6 hours" }, - { value: "12h", label: "Last 12 hours" }, - { value: "24h", label: "Last 24 hours" }, - { value: "7d", label: "Last 7 days" }, -] as const - -const BUCKET_SECONDS: Record = { - "15m": 15, - "1h": 60, - "6h": 300, - "12h": 600, - "24h": 900, - "7d": 3600, -} satisfies Record - -/** Chart bucket width for a preset; falls back to 60s for anything unrecognized. */ -export function bucketSecondsFor(preset: string): number { - return BUCKET_SECONDS[preset] ?? 60 -} - /** * Bucket widths a kubelet-sampled series can honestly be drawn at. The floor is * the scrape interval — anything finer would draw gaps, not resolution. @@ -48,8 +21,7 @@ const BUCKET_LADDER: ReadonlyArray = [ /** * Chart bucket for an arbitrary window: about a hundred points, snapped up to - * the ladder. The preset table above can't serve a page whose window lives in - * the URL, where a custom range has no preset to look up. + * the ladder, so a custom range in the URL needs no preset to look up. */ export function bucketSecondsForRange(startTime: string, endTime: string): number { const windowSeconds = Math.max((toEpochMs(endTime) - toEpochMs(startTime)) / 1000, 60) diff --git a/knip.json b/knip.json index ea78464aad..14fcc37ff8 100644 --- a/knip.json +++ b/knip.json @@ -43,8 +43,9 @@ }, // Single-module Workers: the root alchemy.run.ts imports src/worker.ts, and // alchemy bundles it, so it is the app's one entry. + // The tool playground config is run by path from scripts/tool-playground.sh. "apps/ai": { - "entry": ["src/worker.ts"] + "entry": ["src/worker.ts", "vitest.playground.config.ts", "src/evals/playground/*.playground.ts"] }, "apps/alerting": { "entry": ["src/worker.ts"] diff --git a/packages/query-engine/src/ch/queries/services.ts b/packages/query-engine/src/ch/queries/services.ts index 9205cf368a..63402d9066 100644 --- a/packages/query-engine/src/ch/queries/services.ts +++ b/packages/query-engine/src/ch/queries/services.ts @@ -63,7 +63,7 @@ export interface ServiceWindowFilters { } /** - * The bucket grain a service-history stream is reconstructed at. + * The bucket grain (`grain`) a service-history stream is reconstructed at. * * `"hour"` is the two-tier splice: raw partial boundary hours + the hourly * rollup's interior. Correct for any query that aggregates the whole window, and @@ -75,10 +75,9 @@ export interface ServiceWindowFilters { * `bucketSeconds` is not a multiple of 3600 — an hour-floored row landing in a * sub-hour bucket puts a whole hour of traffic on the bucket containing `:00` * and leaves the rest of the hour reading zero. + * + * The hour grain always splices the hourly tier; only the minute grain can drop it. */ -export type OverviewGrain = "hour" | "minute" - -/** The hour grain always splices the hourly tier; only the minute grain can drop it. */ export type ServiceWindowTiers = | { readonly grain?: "hour"; readonly includeHourly?: true } | { From b4ebf3c56d7eabb2808cf3bcd57554c6adfd4629 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Mon, 12 Oct 2026 02:36:54 +0200 Subject: [PATCH 4/4] fix: quote service_name in widget where clauses, skip missing labels in numeric filters A service_name holding a double quote could open a second where clause; it now goes through quoteWhereValue. Numeric attribute filters also require the key to be present, since a missing key read as 0 and matched lt/lte filters. --- .../ai/src/mcp/tools/create-dashboard.test.ts | 6 ++++++ apps/ai/src/mcp/tools/create-dashboard.ts | 6 +++++- .../src/ch/queries/metrics.test.ts | 10 ++++++++++ packages/query-engine/src/traces-shared.ts | 20 ++++++++----------- 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/apps/ai/src/mcp/tools/create-dashboard.test.ts b/apps/ai/src/mcp/tools/create-dashboard.test.ts index 4644108981..ec9f59347f 100644 --- a/apps/ai/src/mcp/tools/create-dashboard.test.ts +++ b/apps/ai/src/mcp/tools/create-dashboard.test.ts @@ -41,6 +41,12 @@ describe("buildSimpleWidgets", () => { expect(draft?.queries[0]?.whereClause).toBe('service.name = "node" AND attr.state = "idle"') }) + it("quotes service_name so a double quote cannot open a second clause", () => { + const widgets = buildSimpleWidgets([{ ...cpu, where: undefined, service_name: 'x" AND attr.a = "1' }]) + const draft = typeof widgets === "string" ? undefined : dataSourceQuerySet(widgets[0]?.dataSource) + expect(draft?.queries[0]?.whereClause).toBe(`service.name = 'x" AND attr.a = "1'`) + }) + it("rejects a where clause on a list widget instead of dropping it", () => { const result = buildSimpleWidgets([ { title: "Recent logs", source: "logs", visualization: "list", where: 'attr.a = "1"' }, diff --git a/apps/ai/src/mcp/tools/create-dashboard.ts b/apps/ai/src/mcp/tools/create-dashboard.ts index cc3cee486c..a8a5c363c2 100644 --- a/apps/ai/src/mcp/tools/create-dashboard.ts +++ b/apps/ai/src/mcp/tools/create-dashboard.ts @@ -1,6 +1,7 @@ import { McpInvalidInputError, McpQueryError, type McpToolRegistrar } from "./types" import { Effect, Schema } from "effect" import { CreateDashboardOutput } from "@maple/domain/mcp-outputs" +import { quoteWhereValue } from "@maple/domain/where-clause" import { CurrentMcpTenant } from "../lib/query-warehouse" import { DashboardPersistenceService } from "@maple/backend/services/dashboards/DashboardPersistenceService" import { @@ -144,7 +145,10 @@ function simpleSpecToWidget( const metric = spec.metric ?? (source === "metrics" ? "avg" : "count") const extraWhere = spec.where?.trim() ?? "" - const where = [spec.service_name ? `service.name = "${spec.service_name}"` : "", extraWhere] + const where = [ + spec.service_name ? `service.name = ${quoteWhereValue(spec.service_name)}` : "", + extraWhere, + ] .filter((clause) => clause !== "") .join(" AND ") diff --git a/packages/query-engine/src/ch/queries/metrics.test.ts b/packages/query-engine/src/ch/queries/metrics.test.ts index b418d0ecf3..7ff2965620 100644 --- a/packages/query-engine/src/ch/queries/metrics.test.ts +++ b/packages/query-engine/src/ch/queries/metrics.test.ts @@ -487,6 +487,16 @@ describe("metricsBreakdownQuery", () => { expect(sql).toContain("Attributes['cpu'] IN ('cpu0', 'cpu1')") }) + it("excludes datapoints missing the label from numeric filters", () => { + const q = metricsBreakdownQuery({ + metricType: "gauge", + attributeFilters: [{ key: "cpu", mode: "lt", value: "4" }], + }) + const { sql } = compileUnsafe(q, baseParams) + expect(sql).toContain("Attributes['cpu'] != ''") + expect(sql).toContain("toFloat64OrZero(Attributes['cpu']) < 4") + }) + it("ranks groups by the requested aggregate before the limit", () => { const { sql } = compileUnsafe(metricsBreakdownQuery({ metricType: "sum", rankBy: "max" }), baseParams) expect(sql).toContain("ORDER BY maxValue DESC") diff --git a/packages/query-engine/src/traces-shared.ts b/packages/query-engine/src/traces-shared.ts index 0fb9fd0d45..91477e3656 100644 --- a/packages/query-engine/src/traces-shared.ts +++ b/packages/query-engine/src/traces-shared.ts @@ -186,18 +186,14 @@ export function buildAttrFilterCondition( if (values.length === 0) return CH.rawCond("0") return CH.inList(colExpr, values) } - if (af.mode === "gt") { - return CH.toFloat64OrZero(colExpr).gt(Number(value)) - } - if (af.mode === "gte") { - return CH.toFloat64OrZero(colExpr).gte(Number(value)) - } - if (af.mode === "lt") { - return CH.toFloat64OrZero(colExpr).lt(Number(value)) - } - if (af.mode === "lte") { - return CH.toFloat64OrZero(colExpr).lte(Number(value)) - } + // A missing key reads as "" and would coerce to 0, so `attr.x < 5` must not + // match rows that lack `x`. + const numeric = CH.toFloat64OrZero(colExpr) + const present = colExpr.neq("") + if (af.mode === "gt") return present.and(numeric.gt(Number(value))) + if (af.mode === "gte") return present.and(numeric.gte(Number(value))) + if (af.mode === "lt") return present.and(numeric.lt(Number(value))) + if (af.mode === "lte") return present.and(numeric.lte(Number(value))) // equals (default) const exact = colExpr.eq(value) // Empty values intentionally share ClickHouse Map's "missing key returns