perf(agent-sessions): detect agent traces on a filtered span index, not a raw scan - #692
Open
JeremyFunk wants to merge 5 commits into
Open
perf(agent-sessions): detect agent traces on a filtered span index, not a raw scan#692JeremyFunk wants to merge 5 commits into
JeremyFunk wants to merge 5 commits into
Conversation
…ot a raw scan The Agent Sessions list and facets detect agent traces by mapContains(SpanAttributes, 'maple_ai.vendor.id') over raw traces, and that shape cannot be indexed: GenAI spans are ~0.01% of rows but arrive continuously — roughly one per index granule at production volume — so the mapKeys bloom index prunes nothing and the scan decompresses the fat Map column for every span in the window. Measured against production on 2026-08-29: ~3.6s for one hour, dead at the gateway's 15s kill by a day, while the page offers 30. This adds `ai_trace_index` (migration 0023, local schema v13): a filtered projection in the `error_events` shape holding only the vendor-stamped spans, with the maple_ai.* identity and the failure attributes pre-extracted to plain columns — ~10k narrow rows per day against 70M raw spans. The list's detection subquery and the facets query now read it; the per-trace fan-out still reads trace_detail_spans, and the per-session window/spans reads are untouched (they prune by the session-id VALUE, which the mapValues bloom still serves). The index fills forward from its deploy: windows predating it under-report on these two surfaces until the raw tables' 30-day TTL ages the gap out. The backfill (a chunked BackfillSpec from traces) and the fan-out restructure that collapses the remaining wide-window cost are follow-ups.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| const sessionTraceIds = from(Traces) | ||
| // No vendor-presence predicate: `ai_trace_index_mv` admits only spans with a | ||
| // non-empty vendor id, so membership in the table IS the detection predicate. | ||
| const sessionTraceIds = from(AiTraceIndex) |
There was a problem hiding this comment.
🔴 Older clusters lose agent sessions
When ai_trace_index is absent, aiSessionListQuery fails instead of using raw traces. BYO organizations can lose the list and facets indefinitely.
Prompt for agents
Add a raw-traces fallback for Agent Sessions list and facet reads when ai_trace_index is missing. Migration 0023 is requiredForIngest: false, and apps/api/src/workflows/ClickHouseSchemaApplyWorkflow.run.ts treats performance migrations as best-effort; BYO ClickHouse clusters can therefore lack the table indefinitely. The list and facets handlers in apps/api/src/routes/internal/ai-sessions.http.ts currently execute only the indexed queries. Follow the existing missing-table detection and fallback pattern in apps/api/src/services/warehouse/missing-table.ts and apps/api/src/routes/internal/query-engine.http.ts. Preserve the previous raw traces predicates and identical result semantics for both aiSessionListQuery and aiSessionFacetsQuery.
Was this helpful? React with 👍 or 👎 to provide feedback.
…t is absent Adversarial-review fixes for the ai_trace_index PR — the two serious gaps plus the paper cuts found alongside them: - The repo's documented classic MV failure (a write filter and a read guard that agree with each other and disagree with reality → a 0-row table no test can see) had no guard here: every new test asserted SQL text or schema shape. New ClickHouse e2e suite inserts vendor-stamped spans into `traces` on a real-migration database and asserts the MV materialized them — per column, because a TO-table view inserts by name and a mistyped alias silently fills '' — then runs the compiled list query end to end. - A BYO-ClickHouse org only gains the table when an admin applies migration 0023, and the apply workflow treats requiredForIngest:false migrations as best-effort — so the list/facets reads could 502 indefinitely where they previously worked. They now degrade to an empty page via the existing missing-table pattern (isMissingAiTraceIndex + Effect.catchIf), the same face the fill-forward index shows for pre-deploy windows. - Register the ai_trace_index pair in the materialized-projection-order gate. - Fix a vacuous assertion (the no-filters test still checked for the old SpanAttributes spelling of the vendor filter, which can never appear). - Correct two comments that overstated the code: the index's failure columns are written for the planned limit-first restructure, nothing reads them yet; and aiSessionWindowQuery is a value-pruned raw-traces read, not "the detection scan". - Refresh the stale MV/datasource counts in docs/warehouse-rollups.md.
Round-2 adversarial review of the degrade found the one thing wrong with it: it was silent. An empty 200 from a cluster that lacks ai_trace_index is indistinguishable from "no agent traces" — the exact 0-row blind spot the materialization e2e was added to prevent, reintroduced at the HTTP layer, and reachable in production if one Tinybird CD leg lags the worker deploy. The degrade now logs a warning (with the org) and stamps the handler span with the same `query.rollup.fallback` flag `makeRollupFallback` uses, so the condition is visible without changing the response contract. Locked in with route tests: empty 200 for the missing-index error on both the list and the facets, and — the half that keeps the predicate honest — a 502 preserved for every other WarehouseConfigError (bad DSN and friends). The e2e also grew the two assertions the reviewer showed were reasoned rather than proven: rows decode through the query's own row schema exactly as production does, and OrgId/Timestamp are asserted per column with a foreign-org vendor span that must materialize under its own OrgId and never surface in the org-scoped list.
…parately Round-3 audit: the degrade fixture satisfied BOTH disjuncts of isMissingTable at once, so neither was individually pinned — and the message-regex half is the only one the managed Tinybird fleet exercises (its "Resource … not found" carries no clickhouseType). The list test now fails on the gateway shape, the facets test on the direct-ClickHouse UNKNOWN_TABLE shape, so "simplifying" either half of the predicate breaks a test. Also reattach the group's doc comment (the helper's JSDoc had displaced it) and fold a duplicate import.
Review fallout on `ai_trace_index`, all of it subtraction. Six columns, not ten. `VendorVersion`, `StatusCode`, `ErrorType` and `ResponseStatus` had no reader: detection selects the trace-id set, the facets group on `SessionId` and count `VendorId`/`ServiceName`, and every other fact about an agent span — its status, its failure attributes, its vendor version — is read per-trace off `trace_detail_spans`, which the fan-out already touches. They were carried for a restructure that has not happened, justified by the cost of a second backfill of a table no customer has read yet. Both halves of that argument are hypothetical, so the columns go and the table is exactly its two callers' shape. The missing-index degrade goes with them. It turned an absent table into an empty 200, and `isMissingTable` matches any `UNKNOWN_TABLE` on the ClickHouse path without consulting the table name — so a cluster missing `trace_detail_spans`, which `aiSessionListQuery` also reads, would have rendered "no agent sessions" forever behind a warning naming the wrong table. The test pinned that shape rather than catching it: its message names no table at all, and neither degrade test asserted the warning or the span flag the route's own doc called the difference between loud and silent — the harness disables the logger, so they could not. Deleting the path restores the pre-existing 502, which says the schema is not applied and is the answer an operator can act on. `ai-sessions.http.ts`, `ai-sessions.http.test.ts` and `missing-table.ts` are untouched by this branch again. The remaining edits are comments that stopped being true. The MV no longer claims `hasVendorId` as its read-side twin — this branch deleted it, and membership is the guard now. The e2e header no longer says a mistyped alias fills its column with '': a `TO`-table view maps by name, so an unmatched alias is a hard THERE_IS_NO_COLUMN at CREATE. The catalog note drops a hardcoded deploy date that is wrong for every BYO cluster, and points at `trace_detail_spans` for the attributes this table no longer carries. Regenerated the schema, manifest, insert mappings and the v13 local snapshot; the local identity moves af29f0e6 -> b0cb5bfd.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The Agent Sessions list times out (the gateway kills it at 15s) on 12–24h windows, and the goal is 30-day scans. The cost is entirely in detection: agent traces are found by
mapContains(SpanAttributes, 'maple_ai.vendor.id')over rawtraces, and that predicate cannot be indexed at production shape — GenAI spans are ~0.01% of rows but arrive continuously, roughly one per index granule, so themapKeys(SpanAttributes)bloom prunes nothing and the scan decompresses the fat Map column for every span in the window (~70M/day). Measured in production 2026-08-29: ~3.6s for a one-hour window; a full day dies at the kill. Windows before GenAI stamping existed return instantly — the bloom only wins when the key is absent from whole granules — which is how this stayed cheap until the spans arrived in volume.What
ai_trace_index— a filtered projection (theerror_eventsshape fromdocs/warehouse-rollups.md): only vendor-stamped spans,maple_ai.*identity and failure attributes pre-extracted to plain columns,(OrgId, Timestamp, TraceId)sort key, 30-day TTL. ~10k narrow rows/day at current volume.datasources.ts+materializations.ts: the table andai_trace_index_mv. The write filter is rendered from the sameMAPLE_AI_*domain constants the read side now imports, so the two cannot drift silently.0023(requiredForIngest: false— nothing writes the table directly) and local-store schema v13 with thev12 -> v13edge (purely additive: the v13 bootstrap creates both objects, no data moves).aiSessionListQuery's detection subquery andaiSessionFacetsQueryread the index. The per-trace fan-out still readstrace_detail_spans; the per-session window/spans reads are untouched — they prune by the session-id value, which themapValuesbloom still serves.describe_warehouse_tablesnotes steering ad-hoc SQL onto the index instead of the raw-tracesmapContainsscan.What this does not do (deliberate, follow-ups)
BackfillSpecfromtracesis the follow-up if that month matters.trace_detail_spansfan-out. The follow-up selects the LIMIT 50 sessions from the index first and fans out only their traces with bounds derived from those sessions, which collapses the fan-out window regardless of the scan range.Verification
packages/domain(622),packages/query-engine(1354),packages/query-engine-integrations(314, baseline re-recorded),apps/cli(517) all green;clickhouse:schema:check+tinybird:manifest:checkclean.CLICKHOUSE_E2E=1, server 26.2): fullsrc/services/warehousesweep green — the analyzer gate runs the rewritten queries against a database built by replaying the real migration chain through 0023.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.