From 35e506aa3239202b5afa44d03cd47ab788d95748 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Thu, 1 Oct 2026 00:58:31 +0200 Subject: [PATCH 1/2] feat(query): OR groups in where-clauses The where-clause parser dropped `(a OR b)` with a warning, so any filter that needed one ran wider than written. The dependency drilldowns were the visible case: an edge named after its messaging or rpc system also holds spans whose destination is literally that name, and the drill could only reach half. Grammar: one level of parentheses holding attribute clauses joined by OR, AND-ed with the rest. AND or nesting inside a group is rejected rather than guessing precedence, and the splitters no longer cut inside parentheses. - parseWhereClause returns groups only to callers that pass `orGroups: true`; everyone else keeps the old warning, so no consumer drops a group silently. - AttributeFilter gains optional `or` alternatives on the same map. buildAttrFilterCondition ORs the members, each under its semconv aliases, and skips the index prefilters for a group. The trace-list MV route and the facet opts leave groups to the raw path. - Dashboards, alerts and MCP query_data (query-builder model) apply groups on traces and logs, and warn on metrics. Members go through the normal clause handler, so a named key like service.name or a span/resource mix is rejected with a warning. formatFiltersAsWhereClause prints groups back. - Traces page: the URL, API input and chips carry groups; a group chip reads "Any of (a OR b)". - Dependency drills use a group for edges named after their system, so they match both halves of the edge. - MCP dashboard docs describe the new grammar. --- apps/ai/src/mcp/lib/dashboard-schema-doc.ts | 6 +- apps/web/src/api/warehouse/traces.ts | 20 +- .../services/dependency-drill.test.ts | 28 +-- .../components/services/dependency-drill.ts | 25 +- .../lib/traces/advanced-filter-sync.test.ts | 37 +++ .../src/lib/traces/advanced-filter-sync.ts | 91 ++++++- .../src/lib/traces/trace-filter-chips.test.ts | 17 ++ apps/web/src/lib/traces/trace-filter-chips.ts | 10 +- apps/web/src/routes/traces/index.tsx | 8 +- packages/domain/src/query-engine.ts | 16 +- packages/domain/src/where-clause.test.ts | 70 ++++++ packages/domain/src/where-clause.ts | 230 +++++++++++------- packages/query-engine/src/ch/ch.test.ts | 17 ++ .../query-engine/src/ch/queries/traces.ts | 1 + .../src/query-builder/model.test.ts | 81 ++++++ .../query-engine/src/query-builder/model.ts | 111 +++++++-- .../query-engine/src/runtime/query-engine.ts | 17 +- packages/query-engine/src/traces-shared.ts | 9 + 18 files changed, 640 insertions(+), 154 deletions(-) diff --git a/apps/ai/src/mcp/lib/dashboard-schema-doc.ts b/apps/ai/src/mcp/lib/dashboard-schema-doc.ts index cf396e49f7..1840ccc2a7 100644 --- a/apps/ai/src/mcp/lib/dashboard-schema-doc.ts +++ b/apps/ai/src/mcp/lib/dashboard-schema-doc.ts @@ -413,7 +413,11 @@ const queriesSection = (): string => { "### `whereClause` is a custom grammar, not SQL", "", "Operators (the only ones): `=`, `!=`, `>`, `<`, `>=`, `<=`, `contains`, `!contains`,", - "`exists`, `!exists`. Clauses join with ` AND `; there is no `OR` and no parentheses.", + "`exists`, `!exists`. Clauses join with ` AND `. On `traces` and `logs`, 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", + "group counts as one filter toward the cap.", "Values use double quotes. **There is no `IS NULL` / `IS NOT NULL`**: write ` exists`", "or ` !exists`. `exists` means present *and* non-empty, because attributes live in", "ClickHouse `Map` columns where a missing key reads back as `''`.", diff --git a/apps/web/src/api/warehouse/traces.ts b/apps/web/src/api/warehouse/traces.ts index d63ec0c342..875efa2ec5 100644 --- a/apps/web/src/api/warehouse/traces.ts +++ b/apps/web/src/api/warehouse/traces.ts @@ -3,6 +3,7 @@ import { QueryEngineExecuteRequest, TracesFacetDimension, type AttributeFilter, + type AttributeFilterLeaf, formatWarehouseDateTime, } from "@maple/query-engine" import { TraceId, SpanId } from "@maple/domain" @@ -40,11 +41,18 @@ const toTraceId = Schema.decodeSync(TraceId) const ContainsMatchMode = Schema.optional(Schema.Literals(["contains"])) -const AttributeFilterInput = Schema.Struct({ +const attributeFilterInputFields = { key: Schema.String, value: Schema.String, matchMode: Schema.optional(Schema.Literals(["contains", "exists", "gt", "gte", "lt", "lte"])), negated: Schema.optional(Schema.Boolean), +} +const AttributeFilterLeafInput = Schema.Struct(attributeFilterInputFields) + +const AttributeFilterInput = Schema.Struct({ + ...attributeFilterInputFields, + /** The other members of an `(a OR b)` where-clause group. */ + or: Schema.optional(Schema.Array(AttributeFilterLeafInput)), }) const ListTracesInputSchema = Schema.Struct({ @@ -189,7 +197,7 @@ function httpAttributeFilter(key: string, values: readonly string[] | undefined) } /** An absent match mode is equality; `exists` is a presence check and carries no value. */ -function toAttributeFilter(entry: typeof AttributeFilterInput.Type): AttributeFilter { +function toAttributeFilterLeaf(entry: typeof AttributeFilterLeafInput.Type): AttributeFilterLeaf { const mode = entry.matchMode ?? "equals" return { key: entry.key, @@ -199,6 +207,14 @@ function toAttributeFilter(entry: typeof AttributeFilterInput.Type): AttributeFi } } +function toAttributeFilter(entry: typeof AttributeFilterInput.Type): AttributeFilter { + const { or, ...first } = entry + return { + ...toAttributeFilterLeaf(first), + ...(or?.length ? { or: or.map(toAttributeFilterLeaf) } : undefined), + } +} + function buildAttributeFilters(input: ListTracesDecoded): AttributeFilter[] { const filters: AttributeFilter[] = [] diff --git a/apps/web/src/components/services/dependency-drill.test.ts b/apps/web/src/components/services/dependency-drill.test.ts index 4467959cda..4d542f859e 100644 --- a/apps/web/src/components/services/dependency-drill.test.ts +++ b/apps/web/src/components/services/dependency-drill.test.ts @@ -12,32 +12,30 @@ describe("dependencyDrillWhereClause", () => { ]) }) - it("drills a messaging edge named by its system to spans without a destination", () => { + // The rollup merges spans whose destination is literally `kafka` into the + // same edge as the fallback spans, so the drill reaches both halves. + it("drills both halves of an edge named after its system", () => { expect(filtersOf("messaging", "kafka", "kafka").filters.attributeFilters).toEqual([ { key: "messaging.system", value: "kafka" }, - { key: "messaging.destination.name", value: "", matchMode: "exists", negated: true }, + { + key: "messaging.destination.name", + value: "kafka", + or: [{ key: "messaging.destination.name", value: "", matchMode: "exists", negated: true }], + }, ]) }) - // The rollup merges `destination = kafka` spans into the same edge as the - // fallback spans; without OR in the where-clause only the fallback half is - // reachable. Pinned so a parser that gains OR support flips this on purpose. - it("drills only the fallback spans when a destination shares its system's name", () => { - expect(filtersOf("messaging", "kafka", "kafka").filters.attributeFilters).toContainEqual({ - key: "messaging.destination.name", - value: "", - matchMode: "exists", - negated: true, - }) - }) - it("drills an rpc service, or the legacy system key when the edge is named by it", () => { expect(filtersOf("rpc", "checkout.Cart", "grpc").filters.attributeFilters).toEqual([ { key: "rpc.service", value: "checkout.Cart" }, ]) expect(filtersOf("rpc", "grpc", "grpc").filters.attributeFilters).toEqual([ { key: "rpc.system", value: "grpc" }, - { key: "rpc.service", value: "", matchMode: "exists", negated: true }, + { + key: "rpc.service", + value: "grpc", + or: [{ key: "rpc.service", value: "", matchMode: "exists", negated: true }], + }, ]) }) diff --git a/apps/web/src/components/services/dependency-drill.ts b/apps/web/src/components/services/dependency-drill.ts index 73f829840a..823db4c83c 100644 --- a/apps/web/src/components/services/dependency-drill.ts +++ b/apps/web/src/components/services/dependency-drill.ts @@ -8,19 +8,15 @@ export type DependencyDrillKind = "service" | "database" | "messaging" | "rpc" | * The traces page filters root spans unless `root_only = false`, and a client * span is almost never a root, so every drill opens the span-level list. There * is no span-kind filter (a `SpanKind = ...` clause becomes a span-attribute - * filter that matches nothing), and the parser drops `(a OR b)` groups, so each - * drill is one aliased key that the query engine matches under every spelling. + * filter that matches nothing). Target keys are aliased, so the query engine + * matches them under every semconv spelling. * * Targets mirror how the edge rollups name them: messaging and rpc fall back to - * the system when the destination or `rpc.service` is absent, so those drills - * also require the absence, otherwise they would match every destination of the - * system. The rpc system uses the legacy key because that is what the rollup - * reads today. - * - * Known gap: when a destination or rpc.service is literally named after its - * system, the rollup merges those spans and the fallback spans into one edge. - * Matching both needs an OR the where-clause parser does not support, so the - * drill shows only the fallback spans. + * the system when the destination or `rpc.service` is absent. An edge named + * after its system therefore holds the fallback spans, plus any spans whose + * destination or service is literally that name, so the drill matches both and + * nothing else of the system. The rpc system uses the legacy key because that + * is what the rollup reads today. */ export function dependencyDrillWhereClause( kind: DependencyDrillKind, @@ -37,11 +33,14 @@ export function dependencyDrillWhereClause( return spans(`db.system.name = ${value}`) case "messaging": return namedBySystem - ? spans(`messaging.system = ${value}`, "messaging.destination.name !exists") + ? spans( + `messaging.system = ${value}`, + `(messaging.destination.name = ${value} OR messaging.destination.name !exists)`, + ) : spans(`messaging.destination.name = ${value}`) case "rpc": return namedBySystem - ? spans(`rpc.system = ${value}`, "rpc.service !exists") + ? spans(`rpc.system = ${value}`, `(rpc.service = ${value} OR rpc.service !exists)`) : spans(`rpc.service = ${value}`) case "http": return spans(`server.address = ${value}`) diff --git a/apps/web/src/lib/traces/advanced-filter-sync.test.ts b/apps/web/src/lib/traces/advanced-filter-sync.test.ts index fec5e8aede..6d9349b2f2 100644 --- a/apps/web/src/lib/traces/advanced-filter-sync.test.ts +++ b/apps/web/src/lib/traces/advanced-filter-sync.test.ts @@ -682,3 +682,40 @@ describe("applyWhereClause removals", () => { expect(result.hasError).toBe(true) }) }) + +describe("OR groups", () => { + it("parses an attribute group onto one entry with or alternatives", () => { + const { filters, warnings } = parseWhereClause( + 'root_only = false AND (messaging.destination.name = "kafka" OR messaging.destination.name !exists)', + ) + expect(warnings).toEqual([]) + expect(filters.rootOnly).toBe(false) + expect(filters.attributeFilters).toEqual([ + { + key: "messaging.destination.name", + value: "kafka", + or: [{ key: "messaging.destination.name", value: "", matchMode: "exists", negated: true }], + }, + ]) + }) + + it("rejects a group that ORs a named field or mixes maps", () => { + for (const whereClause of [ + '(service.name = "api" OR attr.x = "1")', + '(attr.x = "1" OR resource.y = "2")', + ]) { + const { filters, warnings } = parseWhereClause(whereClause) + expect(filters.attributeFilters).toEqual([]) + expect(filters.resourceAttributeFilters).toEqual([]) + expect(filters.service).toBeUndefined() + expect(warnings).toHaveLength(1) + expect(warnings[0]).toContain("OR group ignored") + } + }) + + it("round-trips a group through toWhereClause", () => { + const whereClause = '(attr.a = "1" OR attr.b !exists)' + const { filters } = parseWhereClause(whereClause) + expect(toWhereClause(filters)).toBe(whereClause) + }) +}) diff --git a/apps/web/src/lib/traces/advanced-filter-sync.ts b/apps/web/src/lib/traces/advanced-filter-sync.ts index 0b956e4e0d..543ce918ba 100644 --- a/apps/web/src/lib/traces/advanced-filter-sync.ts +++ b/apps/web/src/lib/traces/advanced-filter-sync.ts @@ -5,6 +5,7 @@ import { parseWhereClause as parseWhereClauses, quoteWhereValue, type Operator, + type ParsedClause, } from "@maple/domain/where-clause" import { Match } from "effect" @@ -14,13 +15,18 @@ import { Match } from "effect" */ export type AttributeMatchMode = "contains" | "exists" | "gt" | "gte" | "lt" | "lte" -interface AttributeFilterEntry { +interface AttributeFilterEntryLeaf { key: string value: string matchMode?: AttributeMatchMode negated?: boolean } +interface AttributeFilterEntry extends AttributeFilterEntryLeaf { + /** The other members of an `(a OR b)` group; the entry matches when any member does. */ + or?: readonly AttributeFilterEntryLeaf[] +} + export interface TracesSearchLike { services?: string[] spanNames?: string[] @@ -118,7 +124,12 @@ export function attributeFilterOperator(entry: Pick formatAttributeClause(prefix, member)).join(" OR ")})` + } const operator = attributeFilterOperator(entry) if (entry.matchMode === "exists") return `${prefix}${entry.key} ${operator}` return `${prefix}${entry.key} ${operator} ${quoteWhereValue(entry.value)}` @@ -136,13 +147,19 @@ export function parseWhereClause(whereClause: string | undefined): { } } - const parsedClauses = parseWhereClauses(whereClause.trim()) + const parsedClauses = parseWhereClauses(whereClause.trim(), { orGroups: true }) const clauses = parsedClauses.clauses const warnings = parsedClauses.warnings.map((warning) => warning.message) let parsed: ParsedWhereClauseFilters = { attributeFilters: [], resourceAttributeFilters: [] } - for (const clause of clauses) { + // Takes `parsed` and `warnings` as parameters (shadowing the outer ones) so an + // OR group member can be applied alone, with its warnings kept apart. + const applyClause = ( + parsed: ParsedWhereClauseFilters, + clause: ParsedClause, + warnings: string[], + ): ParsedWhereClauseFilters => { const key = normalizeKey(clause.key) const isContains = clause.operator === "contains" const isNegated = clause.operator === "!=" @@ -177,12 +194,12 @@ export function parseWhereClause(whereClause: string | undefined): { if (key.startsWith("attr.")) { pushAttribute(parsed.attributeFilters, typedKey.slice(5).trim(), typedKey) - continue + return parsed } if (key.startsWith("resource.")) { pushAttribute(parsed.resourceAttributeFilters, typedKey.slice(9).trim(), typedKey) - continue + return parsed } const unsupported = (supported: string) => { @@ -196,15 +213,15 @@ export function parseWhereClause(whereClause: string | undefined): { const isEqualityOperator = clause.operator === "=" || isNegated if (CONTAINS_FIELD_KEYS.has(key) && !isEqualityOperator && !isContains) { unsupported("=, != and contains") - continue + return parsed } if (EQUALITY_FIELD_KEYS.has(key) && !isEqualityOperator) { unsupported("= and !=") - continue + return parsed } if (SCALAR_KEYS.has(key) && clause.operator !== "=") { unsupported("=") - continue + return parsed } parsed = Match.value(key).pipe( @@ -292,6 +309,62 @@ export function parseWhereClause(whereClause: string | undefined): { return parsed }), ) + return parsed + } + + for (const clause of clauses) parsed = applyClause(parsed, clause, warnings) + + // An `(a OR b)` group: each member goes through `applyClause` on its own, and + // must land as exactly one attribute entry on the same map. Named fields + // (service, span name, http method, ...) are single-valued params and cannot + // be OR-ed. + for (const group of parsedClauses.groups) { + const label = `(${group.map((c) => c.rawKey ?? c.key).join(" OR ")})` + const members: Array<{ + map: "attributeFilters" | "resourceAttributeFilters" + entry: AttributeFilterEntry + }> = [] + for (const clause of group) { + const memberWarnings: string[] = [] + const alone = applyClause( + { attributeFilters: [], resourceAttributeFilters: [] }, + clause, + memberWarnings, + ) + const setsOtherField = Object.entries(alone).some( + ([field, value]) => + field !== "attributeFilters" && + field !== "resourceAttributeFilters" && + value !== undefined, + ) + const map = + alone.attributeFilters.length === 1 && alone.resourceAttributeFilters.length === 0 + ? "attributeFilters" + : alone.resourceAttributeFilters.length === 1 && alone.attributeFilters.length === 0 + ? "resourceAttributeFilters" + : undefined + const entry = map === undefined ? undefined : alone[map][0] + if (memberWarnings.length > 0 || setsOtherField || map === undefined || entry === undefined) { + members.length = 0 + warnings.push(`OR group ignored: only attribute filters can be OR-ed: ${label}`) + break + } + members.push({ map, entry }) + } + const [first, ...rest] = members + if (first === undefined) continue + if (rest.some((m) => m.map !== first.map)) { + warnings.push(`OR group ignored: its members mix span and resource attributes: ${label}`) + continue + } + if (parsed[first.map].length >= 5) { + warnings.push(`Maximum of 5 filters per attribute map; ignoring ${label}`) + continue + } + parsed = { + ...parsed, + [first.map]: [...parsed[first.map], { ...first.entry, or: rest.map((m) => m.entry) }], + } } return { diff --git a/apps/web/src/lib/traces/trace-filter-chips.test.ts b/apps/web/src/lib/traces/trace-filter-chips.test.ts index fffd2e26dd..c6e7a24d5a 100644 --- a/apps/web/src/lib/traces/trace-filter-chips.test.ts +++ b/apps/web/src/lib/traces/trace-filter-chips.test.ts @@ -85,3 +85,20 @@ describe("traceFilterChips", () => { expect(chips.map((c) => c.label)).toEqual(["Environment", "Service", "HTTP Method"]) }) }) + +describe("traceFilterChips OR groups", () => { + it("shows a group as one chip and removes only that group", () => { + const group = { + key: "a", + value: "1", + or: [{ key: "b", value: "", matchMode: "exists" as const, negated: true }], + } + const plain = { key: "a", value: "1" } + const chips = traceFilterChips({ attributeFilters: [group, plain] }) + expect(chips.map((c) => [c.label, c.values])).toEqual([ + ["Any of", ['(a = "1" OR b !exists)']], + ["a", ["1"]], + ]) + expect(chips[0]?.remove({ attributeFilters: [group, plain] }).attributeFilters).toEqual([plain]) + }) +}) diff --git a/apps/web/src/lib/traces/trace-filter-chips.ts b/apps/web/src/lib/traces/trace-filter-chips.ts index 3a9f309eb8..8919720176 100644 --- a/apps/web/src/lib/traces/trace-filter-chips.ts +++ b/apps/web/src/lib/traces/trace-filter-chips.ts @@ -1,5 +1,5 @@ import type { TracesSearchParams } from "@/routes/traces/index" -import { attributeFilterOperator } from "@/lib/traces/advanced-filter-sync" +import { attributeFilterOperator, formatAttributeClause } from "@/lib/traces/advanced-filter-sync" /** * One facet, in both polarities, as the sidebar spells it. `label` has to match the section title @@ -39,8 +39,9 @@ function attributeChips(search: ChipSearch, param: AttributeParam): TraceFilterC const prefix = param === "resourceAttributeFilters" ? "resource." : "" return (search[param] ?? []).map((entry) => { const text = attributeChipText(prefix, entry) + const group = entry.or?.length ? `:or:${JSON.stringify(entry.or)}` : "" return { - id: `${param}:${entry.key}:${entry.value}:${entry.negated ? "not" : "is"}${entry.matchMode ? `:${entry.matchMode}` : ""}`, + id: `${param}:${entry.key}:${entry.value}:${entry.negated ? "not" : "is"}${entry.matchMode ? `:${entry.matchMode}` : ""}${group}`, label: text.label, values: [text.value], negated: entry.negated === true, @@ -51,7 +52,8 @@ function attributeChips(search: ChipSearch, param: AttributeParam): TraceFilterC other.key === entry.key && other.value === entry.value && other.negated === entry.negated && - other.matchMode === entry.matchMode + other.matchMode === entry.matchMode && + JSON.stringify(other.or ?? []) === JSON.stringify(entry.or ?? []) ), ) return { ...s, [param]: rest.length > 0 ? rest : undefined } @@ -64,6 +66,8 @@ type AttributeEntry = NonNullable[number] // The chip reads "