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..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,6 +42,45 @@ describe("summarizeOutcome", () => { expect(slow.note).toContain("may time out") }) + 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, seriesCount, seriesStats: [] }, + flags: [], + }) + const supported = (queries: ReadonlyArray>): InspectionOutcome => ({ + kind: "supported", + data: { + widget: { + id: "w-1", + visualization: "chart", + endpoint: "custom_query_builder_timeseries", + hasFormulaWarning: false, + hasUnsupportedTransform: false, + }, + timeRange, + queries: [...queries], + verdict: "looks_healthy", + flags: [], + notes: [], + }, + }) + + 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", () => { 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..706dcf7381 100644 --- a/apps/ai/src/mcp/lib/inspect-widget.ts +++ b/apps/ai/src/mcp/lib/inspect-widget.ts @@ -842,6 +842,16 @@ 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( + (largest, query) => + query.status === "ok" + ? Math.max(largest, isBreakdown ? query.stats.rowCount : query.stats.seriesCount) + : largest, + 0, + ) return { widgetId: widget.id, ...(widget.display.title !== undefined ? { title: widget.display.title } : undefined), @@ -849,6 +859,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 +1073,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 +1105,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 cd800b9e67..ec9f59347f 100644 --- a/apps/ai/src/mcp/tools/create-dashboard.test.ts +++ b/apps/ai/src/mcp/tools/create-dashboard.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest" -import { normalizeGroupBy } from "./create-dashboard" +import { dataSourceQuerySet } from "@maple/widgets/dashboard" +import { buildSimpleWidgets, normalizeGroupBy } from "./create-dashboard" describe("normalizeGroupBy", () => { it("accepts query_data spellings", () => { @@ -15,3 +16,41 @@ describe("normalizeGroupBy", () => { expect(normalizeGroupBy("resource.host.name", "metrics")).toBe("resource.host.name") }) }) + +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, 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("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"' }, + ]) + 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 5e5d89eb55..a8a5c363c2 100644 --- a/apps/ai/src/mcp/tools/create-dashboard.ts +++ b/apps/ai/src/mcp/tools/create-dashboard.ts @@ -1,11 +1,13 @@ 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 { DashboardTemplateParameterKey, PortableDashboardDocument, + WIDGET_UNIT_TOKENS, defaultWidgetLayout, findNextPosition, } from "@maple/domain/http" @@ -55,7 +57,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 @@ -140,7 +144,13 @@ 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 extraWhere = spec.where?.trim() ?? "" + const where = [ + spec.service_name ? `service.name = ${quoteWhereValue(spec.service_name)}` : "", + extraWhere, + ] + .filter((clause) => clause !== "") + .join(" AND ") let groupBy: string[] if (spec.group_by) { @@ -192,6 +202,9 @@ function simpleSpecToWidget( } if (viz === "list") { + if (extraWhere !== "") { + return `Widget "${spec.title}": where is not supported on list widgets; use service_name, or a chart/table.` + } if (source === "logs") { return { id, @@ -277,7 +290,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[] = [] @@ -354,9 +367,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/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/domain/src/mcp-outputs/dashboards.ts b/packages/domain/src/mcp-outputs/dashboards.ts index 61eea5dc93..e3020e9c4c 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 (groups, for a breakdown) in the widget's largest query, 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..7ff2965620 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,11 +325,24 @@ describe("metricsTimeseriesRateQuery", () => { expect(sql).not.toContain("span_metrics_calls_hourly") }) - it("falls back to raw metrics_sum when attributeValue has no attributeKey", () => { + it("falls back to raw metrics_sum for a negated attribute filter", () => { const q = metricsTimeseriesRateQuery({ metricName: "span.metrics.calls", bucketSeconds: 3600, - attributeValue: "SPAN_KIND_SERVER", + 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 for a label the rollup does not keep", () => { + const q = metricsTimeseriesRateQuery({ + metricName: "span.metrics.calls", + bucketSeconds: 3600, + attributeFilters: [{ key: "http.route", mode: "equals", value: "/api" }], }) const { sql } = compileUnsafe(q, { ...baseParams, metricName: "span.metrics.calls" }) expect(sql).toContain("FROM metrics_sum") @@ -454,13 +465,36 @@ 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", () => { + 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("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", () => { diff --git a/packages/query-engine/src/ch/queries/metrics.ts b/packages/query-engine/src/ch/queries/metrics.ts index 714bdc4f29..f01281bfcb 100644 --- a/packages/query-engine/src/ch/queries/metrics.ts +++ b/packages/query-engine/src/ch/queries/metrics.ts @@ -36,9 +36,11 @@ 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, +): ReadonlyArray { + return (filters ?? []).map((af) => buildAttrFilterCondition(af, "Attributes")) } // Shared options & output types @@ -51,8 +53,8 @@ interface MetricsQueryOpts { groupByAttributeKey?: string /** Group by a ResourceAttributes key instead of a datapoint Attributes key. */ groupByResourceAttributeKey?: string - attributeKey?: string - attributeValue?: string + /** Predicates on the datapoint Attributes map (metric labels). */ + attributeFilters?: readonly AttributeFilter[] resourceAttributeFilters?: readonly AttributeFilter[] groupBy?: readonly string[] seriesLimit?: number @@ -114,12 +116,10 @@ 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, + ...datapointFilterConditions(opts.attributeFilters), ...resourceFilterConditions(opts.resourceAttributeFilters), ]) @@ -151,8 +151,8 @@ export interface MetricsRateTimeseriesOpts { groupByAttributeKey?: string /** Group by a ResourceAttributes key instead of a datapoint Attributes key. */ groupByResourceAttributeKey?: string - attributeKey?: string - attributeValue?: string + /** Predicates on the datapoint Attributes map (metric labels). */ + attributeFilters?: readonly AttributeFilter[] resourceAttributeFilters?: readonly AttributeFilter[] groupBy?: readonly string[] seriesLimit?: number @@ -184,6 +184,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 @@ -195,9 +204,10 @@ 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 || + 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. @@ -246,9 +256,7 @@ 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( "Hour", @@ -421,12 +429,10 @@ 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, + ...datapointFilterConditions(opts.attributeFilters), ...resourceFilterConditions(opts.resourceAttributeFilters), ]) @@ -541,8 +547,8 @@ 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" limit?: number @@ -598,12 +604,10 @@ 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, + ...datapointFilterConditions(opts.attributeFilters), ...resourceFilterConditions(opts.resourceAttributeFilters), ]) .groupBy("name") 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 } | { 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* 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, @@ -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