diff --git a/.changeset/great-hoops-shave.md b/.changeset/great-hoops-shave.md new file mode 100644 index 0000000000..52b23549a4 --- /dev/null +++ b/.changeset/great-hoops-shave.md @@ -0,0 +1,41 @@ +--- +'@hyperdx/app': patch +--- + +fix: use mapContains for LLM dashboard attribute-presence filters + +The LLM dashboard tested attribute presence with `SpanAttributes['key'] != ''`, +which no skip index can serve — the trace schema's `mapKeys(SpanAttributes)` +index only answers `mapContains`, and `!= ''` normalizes to `notEmpty()`. Every +tile therefore scanned all granules. Map subscripts are also subcolumn +references, so on ClickHouse 26.3+ each one adds a per-part size lookup during +PREWHERE planning. + +These filters now lead with `mapContains`, which the index serves and which +costs no per-part lookups. On a staging trace table the LLM span predicate went +from a 36s planning stall to 7ms, and a two-key filter dropped from 1,306 +granules to 3. + +Gates that pair with a value expression the dashboard groups by keep their +non-empty check, so an attribute set to `''` still cannot appear as a blank row. +There the value term defines the result and the presence term is pruning only, +so it is wrapped in `indexHint` — it reaches skip-index analysis without being +re-evaluated per surviving row. The value term costs no extra per-part lookups, +since it reads the same keys the group-by already reads. Gates that land in a +select-list aggregate are left unhinted, since skip-index analysis does not +reach the select list; the tool-call gate is used in both positions and so is +exposed in both forms. + +The one behavior change is LLM span detection, which is now presence-based: a +span carrying `gen_ai.system` at all is treated as an LLM span whatever the +value. Nothing groups by that predicate. + +One caveat for tables with materialized columns: a `SpanAttributes['key']` +subscript gets rewritten onto a materialized column when an operator has created +one, and `mapContains` is not matched by that rewrite. Such tables were never +affected by the planning cost either, since a rewritten subscript is no longer a +subcolumn reference — so this trades that rewrite for skip-index pruning, which +is the better deal only where those columns do not exist. + +JSON attribute columns are unchanged — their paths are real subcolumns, there is +no key index to prune with, and a presence term would only duplicate reads. diff --git a/.changeset/olive-cameras-repeat.md b/.changeset/olive-cameras-repeat.md new file mode 100644 index 0000000000..6d8385f665 --- /dev/null +++ b/.changeset/olive-cameras-repeat.md @@ -0,0 +1,15 @@ +--- +'@hyperdx/app': minor +--- + +feat: filter the LLM dashboard by end user + +Adds a user filter alongside the existing session filter. It lists the distinct +users seen on LLM spans in the searched range and scopes every tab to the one +selected, including the Errors tab's correlated log events. + +Users are resolved with the same cross-dialect expression the "Top Users" chart +groups by (`user.email`, `enduser.id`, `user.id`, +`ai.telemetry.metadata.userId`), so a value picked from the dropdown always +matches the rows that produced it. The selection lives in the URL, so a filtered +view can be shared. diff --git a/packages/app/src/llm/__tests__/DistinctValueSelect.test.tsx b/packages/app/src/llm/__tests__/DistinctValueSelect.test.tsx new file mode 100644 index 0000000000..8565aaf639 --- /dev/null +++ b/packages/app/src/llm/__tests__/DistinctValueSelect.test.tsx @@ -0,0 +1,95 @@ +import { screen } from '@testing-library/react'; + +import { makeTraceSource } from '@/llm/__fixtures__/sources'; +import { DistinctValueSelect } from '@/llm/dashboard/DistinctValueSelect'; + +let mockRows: Record[] | undefined = []; +let mockLoading = false; +jest.mock('@/hooks/useChartConfig', () => ({ + useQueriedChartConfig: () => ({ + data: mockRows == null ? undefined : { data: mockRows }, + isLoading: mockLoading, + isError: false, + }), +})); + +const TRACE_SOURCE = makeTraceSource(); + +const baseProps = { + source: TRACE_SOURCE, + valueExpression: "SpanAttributes['user.email']", + gateExpression: '1=1', + queryKey: 'llm-user-select', + allLabel: 'All users', + dateRange: [new Date(0), new Date(1000)] as [Date, Date], +}; + +describe('DistinctValueSelect', () => { + beforeEach(() => { + mockRows = []; + mockLoading = false; + }); + + it('shows the applied value when it is present in the fetched options', () => { + mockRows = [{ value: 'alice@x.com' }, { value: 'bob@x.com' }]; + const onChange = jest.fn(); + renderWithMantine( + , + ); + + expect(screen.getByRole('combobox')).toHaveValue('alice@x.com'); + expect(onChange).not.toHaveBeenCalled(); + }); + + // The charts read the value from the URL, so a control that falls back to + // its placeholder here would read "All users" over a filtered dashboard — + // which looks like an unfiltered view returning nothing. + it('keeps the applied value selected when it is absent from the options', () => { + mockRows = [{ value: 'bob@x.com' }]; + const onChange = jest.fn(); + renderWithMantine( + , + ); + + expect(screen.getByRole('combobox')).toHaveValue('alice@x.com'); + // Clearing the parent here would discard a filter the user still has + // applied, so absence must never write back. + expect(onChange).not.toHaveBeenCalled(); + }); + + // Deep-linking into a filtered URL renders once before the distinct-value + // query resolves; the value is not stale then, just not fetched yet. + it('keeps the applied value selected while the options are still loading', () => { + mockRows = undefined; + mockLoading = true; + const onChange = jest.fn(); + renderWithMantine( + , + ); + + expect(screen.getByRole('combobox')).toHaveValue('alice@x.com'); + expect(onChange).not.toHaveBeenCalled(); + }); + + it('falls back to the placeholder when no value is applied', () => { + mockRows = [{ value: 'bob@x.com' }]; + renderWithMantine( + , + ); + + expect(screen.getByRole('combobox')).toHaveValue(''); + expect(screen.getByPlaceholderText('All users')).toBeInTheDocument(); + }); +}); diff --git a/packages/app/src/llm/__tests__/ErrorsTab.test.tsx b/packages/app/src/llm/__tests__/ErrorsTab.test.tsx index 819bc2e79a..211bcd9275 100644 --- a/packages/app/src/llm/__tests__/ErrorsTab.test.tsx +++ b/packages/app/src/llm/__tests__/ErrorsTab.test.tsx @@ -41,6 +41,7 @@ describe('ErrorsTab', () => { logSource={LOG_SOURCE} logExpressions={logExpressions} sessionId="ses_123" + userId="alice@x.com" />, ); @@ -56,6 +57,9 @@ describe('ErrorsTab', () => { expect(traceConditions.some((c: string) => c.includes("'ses_123'"))).toBe( true, ); + expect( + traceConditions.some((c: string) => c.includes("'alice@x.com'")), + ).toBe(true); expect(traceConfig.orderBy[0].ordering).toBe('DESC'); const logConfig = rowTableProps[1].config; @@ -70,6 +74,12 @@ describe('ErrorsTab', () => { c.includes("'ses_123'"), ); expect(sessionCondition).toContain("LogAttributes['session.id']"); + // Same for the user scope: matched against the log source's own column, + // not the trace source's. + const userCondition = logConditions.find((c: string) => + c.includes("'alice@x.com'"), + ); + expect(userCondition).toContain("LogAttributes['user.email']"); }); it('renders only the trace tile without a log source', () => { diff --git a/packages/app/src/llm/__tests__/LatencyTab.test.tsx b/packages/app/src/llm/__tests__/LatencyTab.test.tsx index 60e21fc88e..abb4c2de15 100644 --- a/packages/app/src/llm/__tests__/LatencyTab.test.tsx +++ b/packages/app/src/llm/__tests__/LatencyTab.test.tsx @@ -38,8 +38,13 @@ describe('LatencyTab', () => { jest.clearAllMocks(); }); - it('scopes the heatmap to LLM spans (and session) without the cost alias', () => { - renderWithMantine(); + it('scopes the heatmap to LLM spans (and session/user) without the cost alias', () => { + // This tab passes the scopes into baseLLMChartConfig by hand rather than + // spreading props, and memoizes on them — a scope missing from either + // list leaves the heatmap silently unscoped. + renderWithMantine( + , + ); expect(screen.getByTestId('search-heatmap-chart')).toBeInTheDocument(); const { chartConfig, source } = heatmapChartProps[0]; @@ -47,6 +52,7 @@ describe('LatencyTab', () => { const conditions = chartConfig.filters.map((f: any) => f.condition); expect(conditions[0]).toBe(expressions.isLLMSpan); expect(conditions[1]).toContain("'ses_123'"); + expect(conditions[2]).toContain("'alice@x.com'"); // Delta sampling queries never reference cost; the WITH binding is // skipped to keep them small. expect(chartConfig.with).toBeUndefined(); diff --git a/packages/app/src/llm/__tests__/SessionsTab.test.tsx b/packages/app/src/llm/__tests__/SessionsTab.test.tsx index 659dacab69..08dc3810b8 100644 --- a/packages/app/src/llm/__tests__/SessionsTab.test.tsx +++ b/packages/app/src/llm/__tests__/SessionsTab.test.tsx @@ -46,6 +46,16 @@ describe('SessionsTab', () => { expect(conditions).toContain(expressions.isLLMSpan); expect(conditions).toContain(expressions.hasSessionId); + // aggCondition is a select-list position, so it must use the unhinted + // gate: skip-index analysis never reaches the select list, and the hinted + // form would fold to a constant inside countIf while lengthening every + // Sessions query. + const toolCallsSelect = config.select.find( + (s: any) => s.alias === 'Tool Calls', + ); + expect(toolCallsSelect.aggCondition).toBe(expressions.isToolSpanUnhinted); + expect(toolCallsSelect.aggCondition).not.toContain('indexHint'); + // Token/cost sums use the per-service provided-cost election so apps // emitting several instrumentation dialects in parallel don't double // count (and token-only apps aren't dropped by cost-reporting ones). diff --git a/packages/app/src/llm/__tests__/chartConfig.test.ts b/packages/app/src/llm/__tests__/chartConfig.test.ts index 3ed89460d8..cec84ae563 100644 --- a/packages/app/src/llm/__tests__/chartConfig.test.ts +++ b/packages/app/src/llm/__tests__/chartConfig.test.ts @@ -3,7 +3,7 @@ import { appendWhereClause, baseLLMChartConfig, buildDeltaFilterClause, - buildSessionCondition, + buildScopeCondition, buildTrimmedDeltaSelect, DELTA_ATTRIBUTE_VALUE_MAX_LENGTH, } from '@/llm/dashboard/chartConfig'; @@ -21,15 +21,24 @@ const baseProps = { whereLanguage: 'sql' as const, }; -describe('buildSessionCondition', () => { +describe('buildScopeCondition', () => { it('escapes the session id value', () => { - const condition = buildSessionCondition( + const condition = buildScopeCondition( expressions.sessionId, "ses_1'; DROP TABLE x --", ); expect(condition).toContain(expressions.sessionId); expect(condition).toContain("'ses_1\\'; DROP TABLE x --'"); }); + + it('escapes the user value', () => { + const condition = buildScopeCondition( + expressions.userId, + "alice'; DROP TABLE x --", + ); + expect(condition).toContain(expressions.userId); + expect(condition).toContain("'alice\\'; DROP TABLE x --'"); + }); }); describe('buildDeltaFilterClause', () => { @@ -123,7 +132,7 @@ describe('baseLLMChartConfig session scoping', () => { expect(config.filters).toHaveLength(2); expect(config.filters[1]).toEqual({ type: 'sql', - condition: buildSessionCondition(expressions.sessionId, 'ses_123'), + condition: buildScopeCondition(expressions.sessionId, 'ses_123'), }); }); @@ -136,6 +145,47 @@ describe('baseLLMChartConfig session scoping', () => { expect(config.filters).toHaveLength(3); expect(config.filters[2]).toEqual({ type: 'sql', condition: '1=1' }); }); +}); + +describe('baseLLMChartConfig user scoping', () => { + it('adds no user filter by default', () => { + const config = baseLLMChartConfig(baseProps); + expect(config.filters).toEqual([ + { type: 'sql', condition: expressions.isLLMSpan }, + ]); + }); + + it('appends the user filter to every chart when userId is set', () => { + const config = baseLLMChartConfig({ ...baseProps, userId: 'alice@x.com' }); + expect(config.filters).toHaveLength(2); + expect(config.filters[1]).toEqual({ + type: 'sql', + condition: buildScopeCondition(expressions.userId, 'alice@x.com'), + }); + }); + + // The two scopes are independent, so both must survive together and keep a + // stable order — extra filters are appended relative to them. + it('applies session and user together, session first, extras last', () => { + const config = baseLLMChartConfig({ + ...baseProps, + sessionId: 'ses_123', + userId: 'alice@x.com', + extraFilters: [{ type: 'sql', condition: '1=1' }], + }); + expect(config.filters).toEqual([ + { type: 'sql', condition: expressions.isLLMSpan }, + { + type: 'sql', + condition: buildScopeCondition(expressions.sessionId, 'ses_123'), + }, + { + type: 'sql', + condition: buildScopeCondition(expressions.userId, 'alice@x.com'), + }, + { type: 'sql', condition: '1=1' }, + ]); + }); it('binds the cost expression once as a WITH expression alias when opted in', () => { // The cost expression embeds the model price catalog (~70 KiB of SQL); diff --git a/packages/app/src/llm/__tests__/detect.test.ts b/packages/app/src/llm/__tests__/detect.test.ts index 783c538528..d035de414c 100644 --- a/packages/app/src/llm/__tests__/detect.test.ts +++ b/packages/app/src/llm/__tests__/detect.test.ts @@ -44,19 +44,26 @@ describe('isLLMSpan', () => { }); describe('buildLLMSpanSqlPredicate', () => { - it('builds a map-column predicate', () => { + // mapContains, not `col['k'] != ''`: only the former can be served by a + // mapKeys() skip index, and it is not a subcolumn reference so it stays out + // of the per-part size lookups PREWHERE planning does on CH 26.3+. + it('builds a map-column predicate with mapContains', () => { const predicate = buildLLMSpanSqlPredicate({ attributeField: 'SpanAttributes', isJsonColumn: false, }); expect(predicate).toContain( - "SpanAttributes['gen_ai.operation.name'] != ''", + "mapContains(SpanAttributes, 'gen_ai.operation.name')", ); - expect(predicate).toContain("SpanAttributes['llm.model_name'] != ''"); + expect(predicate).toContain( + "mapContains(SpanAttributes, 'llm.model_name')", + ); + expect(predicate).not.toContain("!= ''"); expect(predicate.startsWith('(')).toBe(true); expect(predicate.endsWith(')')).toBe(true); }); + // JSON paths are real subcolumns; mapContains does not apply to them. it('builds a JSON-column predicate with backtick paths', () => { const predicate = buildLLMSpanSqlPredicate({ attributeField: 'SpanAttributes', @@ -65,5 +72,6 @@ describe('buildLLMSpanSqlPredicate', () => { expect(predicate).toContain( "toString(SpanAttributes.`gen_ai.operation.name`) != ''", ); + expect(predicate).not.toContain('mapContains'); }); }); diff --git a/packages/app/src/llm/__tests__/expressions.test.ts b/packages/app/src/llm/__tests__/expressions.test.ts index 3f04ce58d5..5852854ccc 100644 --- a/packages/app/src/llm/__tests__/expressions.test.ts +++ b/packages/app/src/llm/__tests__/expressions.test.ts @@ -30,7 +30,7 @@ describe('getLLMExpressions', () => { `${expressions.effectiveInputTokens} + ${expressions.outputTokens}`, ); expect(expressions.isLLMSpan).toContain( - "SpanAttributes['gen_ai.operation.name'] != ''", + "mapContains(SpanAttributes, 'gen_ai.operation.name')", ); expect(expressions.isError).toBe("lower(StatusCode) = 'error'"); expect(expressions.durationInMillis).toBe('Duration/1e6'); @@ -51,6 +51,37 @@ describe('getLLMExpressions', () => { expect(expressions.isLLMSpan).toContain( "toString(SpanAttributes.`gen_ai.operation.name`) != ''", ); + // mapContains does not apply to JSON paths. + expect(expressions.isLLMSpan).not.toContain('mapContains'); + expect(expressions.isToolSpan).not.toContain('mapContains'); + + // On JSON there is no key index to prune with, so the gates carry the + // value test alone rather than doubling their subcolumn reads with a + // presence term that says the same thing. + expect(expressions.isToolSpan).not.toContain('indexHint'); + expect(expressions.hasTtft).toBe( + 'greatest(toFloat64OrZero(toString(SpanAttributes.`ttft_ms`)), ' + + 'toFloat64OrZero(toString(SpanAttributes.`ai.response.msToFirstChunk`)), ' + + 'toFloat64OrZero(toString(SpanAttributes.`copilot_chat.time_to_first_token`))) > 0', + ); + expect(expressions.hasSessionId).toBe( + "coalesce(nullif(toString(SpanAttributes.`gen_ai.conversation.id`), ''), " + + "nullif(toString(SpanAttributes.`session.id`), ''), " + + "nullif(toString(SpanAttributes.`ai.telemetry.metadata.sessionId`), ''), '') != ''", + ); + // Unhinted on both branches, so pin the JSON form itself — a + // not.toContain('indexHint') here would hold either way and prove nothing. + expect(expressions.hasReportedTokens).toBe( + "coalesce(nullif(toString(SpanAttributes.`gen_ai.usage.input_tokens`), ''), " + + "nullif(toString(SpanAttributes.`gen_ai.usage.prompt_tokens`), ''), " + + "nullif(toString(SpanAttributes.`gen_ai.usage.output_tokens`), ''), " + + "nullif(toString(SpanAttributes.`gen_ai.usage.completion_tokens`), ''), " + + "nullif(toString(SpanAttributes.`llm.token_count.prompt`), ''), " + + "nullif(toString(SpanAttributes.`llm.token_count.completion`), ''), " + + "nullif(toString(SpanAttributes.`llm.token_count.total`), ''), " + + "nullif(toString(SpanAttributes.`input_tokens`), ''), " + + "nullif(toString(SpanAttributes.`output_tokens`), ''), '') != ''", + ); }); it('derives session id and reported-token gate expressions', () => { @@ -65,17 +96,35 @@ describe('getLLMExpressions', () => { expect(expressions.sessionId).toContain( "nullif(SpanAttributes['ai.telemetry.metadata.sessionId'], '')", ); - expect(expressions.hasSessionId).toContain("!= ''"); + // Presence terms for the index, value term for the meaning: a key set to + // '' must not pass, or SessionsTab groups it as a blank row whose link + // carries no session id. + expect(expressions.hasSessionId).toBe( + "(indexHint((mapContains(SpanAttributes, 'gen_ai.conversation.id') OR " + + "mapContains(SpanAttributes, 'session.id') OR " + + "mapContains(SpanAttributes, 'ai.telemetry.metadata.sessionId'))) AND " + + "coalesce(nullif(SpanAttributes['gen_ai.conversation.id'], ''), " + + "nullif(SpanAttributes['session.id'], ''), " + + "nullif(SpanAttributes['ai.telemetry.metadata.sessionId'], ''), '') != '')", + ); // The gate keys on authoritative usage reporters only — wrapper spans // carrying just ai.usage.* must not satisfy it. expect(expressions.hasReportedTokens).toContain( - "SpanAttributes['gen_ai.usage.input_tokens'] != ''", + "nullif(SpanAttributes['gen_ai.usage.input_tokens'], '')", ); expect(expressions.hasReportedTokens).toContain( - "SpanAttributes['llm.token_count.total'] != ''", + "nullif(SpanAttributes['llm.token_count.total'], '')", ); expect(expressions.hasReportedTokens).not.toContain('ai.usage.inputTokens'); + // This gate decides which rows enter every token and cost aggregate, so + // pin the value test: reverting it to presence-only would let a key set + // to '' inflate the call count. + expect(expressions.hasReportedTokens).toMatch(/^coalesce\(nullif\(/); + expect(expressions.hasReportedTokens).toContain("!= ''"); + // Only ever embedded in select-list aggregates, where a hint cannot prune + // and would double the length of an already-long expression. + expect(expressions.hasReportedTokens).not.toContain('indexHint'); }); it('derives efficiency, attribution, and agent expressions', () => { @@ -126,7 +175,17 @@ describe('getLLMExpressions', () => { expect(expressions.ttftMs).toContain( "SpanAttributes['copilot_chat.time_to_first_token']", ); - expect(expressions.hasTtft).toContain('> 0'); + // Pinned exactly: the terms must be ANDed. With OR, zero-TTFT rows reach + // the p50/p95 the value check exists to keep them out of, and a + // toContain-style assertion would not notice. + expect(expressions.hasTtft).toBe( + "(indexHint((mapContains(SpanAttributes, 'ttft_ms') OR " + + "mapContains(SpanAttributes, 'ai.response.msToFirstChunk') OR " + + "mapContains(SpanAttributes, 'copilot_chat.time_to_first_token'))) AND " + + "greatest(toFloat64OrZero(SpanAttributes['ttft_ms']), " + + "toFloat64OrZero(SpanAttributes['ai.response.msToFirstChunk']), " + + "toFloat64OrZero(SpanAttributes['copilot_chat.time_to_first_token'])) > 0)", + ); // Tool name coalesce includes the flat form. expect(expressions.toolName).toContain("SpanAttributes['tool_name']"); @@ -136,16 +195,70 @@ describe('getLLMExpressions', () => { "SpanAttributes['gen_ai.agent.name']", ); expect(expressions.agentName).toContain("SpanAttributes['agent.name']"); - expect(expressions.hasAgentName).toContain("!= ''"); + expect(expressions.hasAgentName).toBe( + "(indexHint((mapContains(SpanAttributes, 'gen_ai.agent.name') OR " + + "mapContains(SpanAttributes, 'agent.name'))) AND " + + "coalesce(nullif(SpanAttributes['gen_ai.agent.name'], ''), " + + "nullif(SpanAttributes['agent.name'], ''), '') != '')", + ); // Finish reasons normalized out of their JSON-array encoding. expect(expressions.finishReason).toMatch(/^replaceRegexpAll\(/); expect(expressions.finishReason).toContain("SpanAttributes['stop_reason']"); + // Drives the finish-reason breakdown, so it needs the value term to keep + // a blank slice out of the chart. + expect(expressions.hasFinishReason).toBe( + "(indexHint((mapContains(SpanAttributes, 'gen_ai.response.finish_reasons') OR " + + "mapContains(SpanAttributes, 'stop_reason') OR " + + "mapContains(SpanAttributes, 'llm.finish_reason') OR " + + "mapContains(SpanAttributes, 'ai.response.finishReason'))) AND " + + "coalesce(nullif(SpanAttributes['gen_ai.response.finish_reasons'], ''), " + + "nullif(SpanAttributes['stop_reason'], ''), " + + "nullif(SpanAttributes['llm.finish_reason'], ''), " + + "nullif(SpanAttributes['ai.response.finishReason'], ''), '') != '')", + ); + // User attribution coalesce. expect(expressions.userId).toContain("SpanAttributes['user.email']"); expect(expressions.userId).toContain("SpanAttributes['enduser.id']"); - expect(expressions.hasUserId).toContain("!= ''"); + expect(expressions.hasUserId).toContain( + "mapContains(SpanAttributes, 'user.email')", + ); + // The value term keeps a blank user out of the Top Users group-by. + expect(expressions.hasUserId).toContain( + "AND coalesce(nullif(SpanAttributes['user.email'], '')", + ); + + // The span-kind term must stay an equality: widening it to a presence + // check would pull every OpenInference span into the tool charts. + // The hint covers the span-kind key and sits above the OR: a skip index + // cannot prune an OR whose other arm it can't decide, so nesting it inside + // would silently stop pruning the tool charts. + expect(expressions.isToolSpan).toBe( + "(indexHint((mapContains(SpanAttributes, 'openinference.span.kind') OR " + + "mapContains(SpanAttributes, 'gen_ai.tool.name') OR " + + "mapContains(SpanAttributes, 'ai.toolCall.name') OR " + + "mapContains(SpanAttributes, 'tool_name') OR " + + "mapContains(SpanAttributes, 'gen_ai.tool.call.id'))) AND " + + "(SpanAttributes['openinference.span.kind'] = 'TOOL' OR " + + "coalesce(nullif(SpanAttributes['gen_ai.tool.name'], ''), " + + "nullif(SpanAttributes['ai.toolCall.name'], ''), " + + "nullif(SpanAttributes['tool_name'], ''), " + + "nullif(SpanAttributes['gen_ai.tool.call.id'], ''), '') != ''))", + ); + // Every key the tool-name expression can resolve must also satisfy the + // gate, or the span is dropped from the tool charts under a name the + // dashboard could have shown. + for (const key of ['gen_ai.tool.name', 'ai.toolCall.name', 'tool_name']) { + expect(expressions.toolName).toContain(`SpanAttributes['${key}']`); + expect(expressions.isToolSpan).toContain(`SpanAttributes['${key}']`); + } + // SessionsTab uses this one as an aggCondition, where a hint folds to a + // constant. It must stay the bare match, and must still be the same + // predicate the hinted form wraps. + expect(expressions.isToolSpanUnhinted).not.toContain('indexHint'); + expect(expressions.isToolSpan).toContain(expressions.isToolSpanUnhinted); expect(expressions.statusMessage).toBe('StatusMessage'); }); @@ -247,7 +360,7 @@ describe('getLLMLogExpressions', () => { "nullif(LogAttributes['session.id'], '')", ); expect(expressions.isLLMSpan).toContain( - "LogAttributes['gen_ai.provider.name'] != ''", + "mapContains(LogAttributes, 'gen_ai.provider.name')", ); // LLM-related = LLM markers OR any session id, so tool_result / // lifecycle log events that only carry session.id are included. diff --git a/packages/app/src/llm/dashboard/DistinctValueSelect.tsx b/packages/app/src/llm/dashboard/DistinctValueSelect.tsx new file mode 100644 index 0000000000..fe947d79b0 --- /dev/null +++ b/packages/app/src/llm/dashboard/DistinctValueSelect.tsx @@ -0,0 +1,101 @@ +import { useMemo } from 'react'; +import { SourceKind, TTraceSource } from '@hyperdx/common-utils/dist/types'; +import { Select, SelectProps } from '@mantine/core'; + +import { useQueriedChartConfig } from '@/hooks/useChartConfig'; + +/** + * Dropdown over the distinct values of one LLM span expression in the + * searched range — the shape both dashboard-wide scope filters (session, + * user) share. + * + * Plain value/onChange: the dashboard keeps these scopes in the URL rather + * than in the search form, so they can also be set programmatically (e.g. the + * session drawer's "Filter dashboard") without two-way form sync. + */ +export function DistinctValueSelect({ + source, + valueExpression, + gateExpression, + queryKey, + allLabel, + value, + onChange, + dateRange, + ...props +}: { + source: TTraceSource | undefined; + /** Expression whose distinct values populate the dropdown. */ + valueExpression: string | undefined; + /** Scope predicate, ANDed with the LLM-span filter (e.g. hasUserId). */ + gateExpression: string | undefined; + /** Distinguishes this select's cached query from its siblings'. */ + queryKey: string; + /** Label for the "no filter" option, e.g. 'All users'. */ + allLabel: string; + value: string; + onChange: (value: string) => void; + dateRange: [Date, Date]; +} & Omit) { + const enabled = + source?.kind === SourceKind.Trace && !!valueExpression && !!gateExpression; + + const queriedConfig = { + source: source?.id, + timestampValueExpression: source?.timestampValueExpression || '', + from: { + databaseName: source?.from.databaseName || '', + tableName: source?.from.tableName || '', + }, + connection: source?.connection || '', + select: [ + { alias: 'value', valueExpression: `distinct(${valueExpression})` }, + ], + where: gateExpression || '', + whereLanguage: 'sql' as const, + limit: { limit: 10000 }, + dateRange, + }; + + const { data, isLoading, isError } = useQueriedChartConfig(queriedConfig, { + placeholderData: (prev: any) => prev, + queryKey: [queryKey, queriedConfig], + enabled, + }); + + const values = useMemo(() => { + const found: string[] = + data?.data + ?.map((d: any) => d.value) + .filter(Boolean) + .sort() || []; + // Keep the applied value selectable even when it is not in `found`, so the + // control can never read "all" while the charts are still filtered. It is + // missing in three cases, only one of which is staleness: the query has + // not resolved yet (deep link into a filtered URL), the value fell off the + // end of the row limit, or the searched window no longer contains it. + // + // Clearing the parent instead would silently discard a filter in the first + // two, and would mean writing to the URL from a render driven by fetch + // timing — which is what keeping these selects out of the search form is + // meant to avoid. + const applied = value && !found.includes(value) ? [value] : []; + return [{ value: '', label: allLabel }, ...applied, ...found]; + }, [data, allLabel, value]); + + return ( + onChange(v ?? '')} - disabled={isLoading || isError} - comboboxProps={{ withinPortal: false }} - searchable - clearable - placeholder="All sessions" - maxDropdownHeight={280} - nothingFoundMessage={isLoading ? 'Loading more...' : 'No matches found'} + source={source} + valueExpression={expressions?.sessionId} + gateExpression={ + expressions && + `${expressions.isLLMSpan} AND ${expressions.hasSessionId}` + } + queryKey="llm-session-select" + allLabel="All sessions" /> ); } diff --git a/packages/app/src/llm/dashboard/SessionsTab.tsx b/packages/app/src/llm/dashboard/SessionsTab.tsx index 9106e982fa..a0f3f3ee1e 100644 --- a/packages/app/src/llm/dashboard/SessionsTab.tsx +++ b/packages/app/src/llm/dashboard/SessionsTab.tsx @@ -114,7 +114,8 @@ export function SessionsTab(props: LLMChartProps) { alias: 'Tool Calls', aggFn: 'count', valueExpression: '', - aggCondition: expressions.isToolSpan, + // Select-list position: the hinted form cannot prune here. + aggCondition: expressions.isToolSpanUnhinted, aggConditionLanguage: 'sql', numberFormat: INTEGER_NUMBER_FORMAT, }, diff --git a/packages/app/src/llm/dashboard/UserSelect.tsx b/packages/app/src/llm/dashboard/UserSelect.tsx new file mode 100644 index 0000000000..b1e2ccf045 --- /dev/null +++ b/packages/app/src/llm/dashboard/UserSelect.tsx @@ -0,0 +1,40 @@ +import { TTraceSource } from '@hyperdx/common-utils/dist/types'; +import { SelectProps } from '@mantine/core'; + +import { LLMExpressions } from '@/llm/lib/expressions'; + +import { DistinctValueSelect } from './DistinctValueSelect'; + +/** + * End-user filter dropdown: distinct users seen on LLM spans in the searched + * range, resolved via the cross-dialect user expression (user.email, + * enduser.id, user.id, ai.telemetry.metadata.userId). + * + * Values come from the same expression the "Top Users" chart groups by, so a + * selection always matches the rows that produced it. + */ +export function UserSelect({ + source, + expressions, + ...props +}: { + /** Trace source: users are resolved from LLM spans. */ + source: TTraceSource | undefined; + expressions: LLMExpressions | undefined; + dateRange: [Date, Date]; + value: string; + onChange: (userId: string) => void; +} & Omit) { + return ( + + ); +} diff --git a/packages/app/src/llm/dashboard/chartConfig.ts b/packages/app/src/llm/dashboard/chartConfig.ts index 96fe69089d..188eaee50f 100644 --- a/packages/app/src/llm/dashboard/chartConfig.ts +++ b/packages/app/src/llm/dashboard/chartConfig.ts @@ -9,18 +9,21 @@ import { LLM_COST_SQL_ALIAS } from '@/llm/lib/expressions'; import { LLMChartProps } from './types'; -/** `sessionIdExpr = 'value'` condition, SQL-escaped. */ -export function buildSessionCondition( - sessionIdExpr: string, - sessionId: string, -): string { - return SqlString.format('? = ?', [SqlString.raw(sessionIdExpr), sessionId]); +/** + * `scopeExpr = 'value'` condition, SQL-escaped. + * + * Used for the dashboard-wide scope filters (session, user), whose values are + * matched exactly against a cross-dialect coalesced expression — the same + * expression the corresponding dropdown selected them from. + */ +export function buildScopeCondition(scopeExpr: string, value: string): string { + return SqlString.format('? = ?', [SqlString.raw(scopeExpr), value]); } /** * Base chart-config fields shared by every LLM dashboard chart: source * binding, the user's where clause, the LLM-span scope filter, the optional - * session-id scope, and the searched date range. + * session-id and user scopes, and the searched date range. */ export function baseLLMChartConfig({ source, @@ -29,6 +32,7 @@ export function baseLLMChartConfig({ where, whereLanguage, sessionId, + userId, extraFilters = [], withCostAlias = IS_LLM_COST_ENABLED, }: LLMChartProps & { @@ -72,10 +76,15 @@ export function baseLLMChartConfig({ ? [ { type: 'sql' as const, - condition: buildSessionCondition( - expressions.sessionId, - sessionId, - ), + condition: buildScopeCondition(expressions.sessionId, sessionId), + }, + ] + : []), + ...(userId + ? [ + { + type: 'sql' as const, + condition: buildScopeCondition(expressions.userId, userId), }, ] : []), diff --git a/packages/app/src/llm/dashboard/types.ts b/packages/app/src/llm/dashboard/types.ts index d78eeb74f0..82ddd05e00 100644 --- a/packages/app/src/llm/dashboard/types.ts +++ b/packages/app/src/llm/dashboard/types.ts @@ -18,6 +18,12 @@ export interface LLMChartProps { * via the cross-dialect session expression). */ sessionId?: string; + /** + * When set, every chart is additionally scoped to this end user (matched + * via the cross-dialect user expression, which coalesces user.email, + * enduser.id, user.id and ai.telemetry.metadata.userId). + */ + userId?: string; /** Correlated log source for LLM log events, when one is selected. */ logSource?: TLogSource; logExpressions?: LLMLogExpressions; diff --git a/packages/app/src/llm/lib/detect.ts b/packages/app/src/llm/lib/detect.ts index d658ad9627..a429f1d874 100644 --- a/packages/app/src/llm/lib/detect.ts +++ b/packages/app/src/llm/lib/detect.ts @@ -1,3 +1,5 @@ +import SqlString from 'sqlstring'; + import { hasKeyWithPrefix } from './attributeUtils'; import { LLMSpanEvent, SpanAttributeMap } from './types'; @@ -81,10 +83,73 @@ export function isLLMSpan( return (events ?? []).some(event => event.name?.startsWith('gen_ai.')); } +/** + * SQL testing whether an attribute key is present on a row. + * + * For map columns this is `mapContains`, not `col['key'] != ''`, for two + * reasons. It is one of the few shapes a `mapKeys(col)` skip index can serve + * (`MergeTreeIndexConditionText::isSupportedFunction`) — `!= ''` normalizes to + * `notEmpty()`, which no index supports, so it scans every granule. And a map + * subscript is a subcolumn reference, which on ClickHouse 26.3+ costs one + * per-part size lookup during PREWHERE planning; `mapContains` costs none. + * + * JSON columns keep the comparison: their paths are real subcolumns, and + * `mapContains` does not apply. + * + * Trade-off: `fastifySQL` rewrites a `Col['key']` subscript onto a + * materialized column when one exists, and it matches on the subscript AST + * node, so it cannot see this form. On a table where an operator materialized + * one of these keys, the subscript would have read that column instead. The + * subscript is also not a subcolumn reference once rewritten, so such tables + * never had the planning problem — this form trades that rewrite for skip-index + * pruning, which is the better deal only where the columns do not exist (the + * default). + * + * This tests presence only. Callers that pair a gate with a value expression + * they group by need non-emptiness as well, or a key set to '' becomes a blank + * row — see `anyKeyHasValue` in expressions.ts. + * + * queryParser's private `buildMapContains` emits the same call, but it takes a + * rendered `col['key']` subscript and parses it back apart; the field and key + * are already separate here. `attributeField` stays raw because it holds a + * source's `eventAttributesExpression`, which may be a compound expression + * rather than an identifier — the same reason `fieldAccess` interpolates it. + */ +function buildKeyExistsSql({ + attributeField, + key, + isJsonColumn, +}: { + attributeField: string; + key: string; + isJsonColumn: boolean; +}): string { + return isJsonColumn + ? `toString(${attributeField}.\`${key}\`) != ''` + : `mapContains(${attributeField}, ${SqlString.escape(key)})`; +} + +/** SQL matching rows carrying any of the given attribute keys. */ +export function buildAnyKeyExistsSql(args: { + attributeField: string; + keys: readonly string[]; + isJsonColumn: boolean; +}): string { + const { attributeField, keys, isJsonColumn } = args; + const conditions = keys.map(key => + buildKeyExistsSql({ attributeField, key, isJsonColumn }), + ); + return `(${conditions.join(' OR ')})`; +} + /** * Build a SQL predicate matching LLM spans, for use in search filters and * dashboard chart configs. Handles both `Map(String, String)` attribute - * columns (missing key reads as '') and JSON-typed columns. + * columns and JSON-typed columns. + * + * Presence is the right test here: a span carrying `gen_ai.system` at all was + * emitted by LLM instrumentation, whatever the value. Nothing groups by this + * predicate, so an empty value cannot produce a blank row. */ export function buildLLMSpanSqlPredicate({ attributeField, @@ -94,10 +159,9 @@ export function buildLLMSpanSqlPredicate({ attributeField: string; isJsonColumn: boolean; }): string { - const conditions = LLM_MARKER_ATTRIBUTE_KEYS.map(key => - isJsonColumn - ? `toString(${attributeField}.\`${key}\`) != ''` - : `${attributeField}['${key}'] != ''`, - ); - return `(${conditions.join(' OR ')})`; + return buildAnyKeyExistsSql({ + attributeField, + keys: LLM_MARKER_ATTRIBUTE_KEYS, + isJsonColumn, + }); } diff --git a/packages/app/src/llm/lib/expressions.ts b/packages/app/src/llm/lib/expressions.ts index 6b5494753a..72f901a18a 100644 --- a/packages/app/src/llm/lib/expressions.ts +++ b/packages/app/src/llm/lib/expressions.ts @@ -1,7 +1,7 @@ import { TLogSource, TTraceSource } from '@hyperdx/common-utils/dist/types'; import { generateCostSqlExpression } from './cost'; -import { buildLLMSpanSqlPredicate } from './detect'; +import { buildAnyKeyExistsSql, buildLLMSpanSqlPredicate } from './detect'; /** * SQL expression derivation for the LLM dashboard, mirroring @@ -21,7 +21,7 @@ function fieldAccess( /** First non-empty string among the given attribute keys ('' when none). */ function coalesceString( field: string, - keys: string[], + keys: readonly string[], isJsonColumn: boolean, ): string { const args = keys.map( @@ -37,7 +37,7 @@ function coalesceString( */ function greatestNumber( field: string, - keys: string[], + keys: readonly string[], isJsonColumn: boolean, ): string { const args = keys.map( @@ -222,6 +222,71 @@ function getLLMAttributeExpressions({ attributeField: string; isJsonColumn: boolean; }) { + /** Rows carrying any of these keys. Index-friendly — see buildKeyExistsSql. */ + const anyKeyExists = (keys: readonly string[]) => + buildAnyKeyExistsSql({ attributeField, keys, isJsonColumn }); + + /** + * Wrap a presence term that exists only to prune granules. + * + * `indexHint` feeds its argument to skip-index analysis but returns true at + * execution, so the keys are never re-tested per surviving row. This is the + * same thing queryParser.ts does with `mapKeyIndexExpression`. + * + * Only safe when a value term follows that already implies presence. A bare + * presence filter must not be wrapped — `indexHint` alone matches every row. + */ + const prunableKeys = (keys: readonly string[]) => + `indexHint(${anyKeyExists(keys)})`; + + /** + * Pair a value term with a pruning hint over the keys it reads. + * + * `valueTerm` alone defines the result; the hint only lets the index drop + * granules, so it must be implied by `valueTerm` to be sound. + * + * Only worth adding in WHERE position — skip-index analysis does not look at + * the select list, so a gate used solely inside an aggregate should pass its + * value term through unhinted rather than pay the query-length cost. + * + * On JSON columns there is no key index to prune with, so the value term is + * emitted alone. + */ + const withKeyPruning = (keys: readonly string[], valueTerm: string) => + isJsonColumn ? valueTerm : `(${prunableKeys(keys)} AND ${valueTerm})`; + + /** "Any of these keys holds a non-empty value", before any pruning hint. */ + const anyKeyNonEmpty = (keys: readonly string[]) => + `${coalesceString(attributeField, keys, isJsonColumn)} != ''`; + + /** + * Gate for "any of these keys holds a non-empty value". + * + * Presence alone would admit a key explicitly set to '', and every caller + * pairs this gate with a coalesced value expression it groups by, so such a + * row would show up as a blank bar/row whose drill-in link goes nowhere. + * + * The value term costs no extra per-part size lookups during planning: it + * reads the same keys the paired group-by expression already reads. + */ + const anyKeyHasValue = (keys: readonly string[]) => + withKeyPruning(keys, anyKeyNonEmpty(keys)); + + // Derived from TOOL_NAME_KEYS rather than restated: a span whose tool name + // resolves is a tool call, so the two must not drift. gen_ai.tool.call.id is + // the one addition — it marks a tool call without naming it. + // + // Deriving currently changes nothing observable: the flat `tool_name` it + // picks up is not in LLM_MARKER_ATTRIBUTE_KEYS, and every caller ANDs + // isLLMSpan, so a span carrying only that key is dropped before this gate is + // reached. Widening LLM detection to a key that generic is its own decision. + const TOOL_SPAN_KEYS = [...TOOL_NAME_KEYS, 'gen_ai.tool.call.id']; + const isToolSpanMatch = `(${fieldAccess( + attributeField, + 'openinference.span.kind', + isJsonColumn, + )} = 'TOOL' OR ${anyKeyNonEmpty(TOOL_SPAN_KEYS)})`; + const model = coalesceString(attributeField, MODEL_KEYS, isJsonColumn); const inputTokens = greatestNumber( attributeField, @@ -284,7 +349,7 @@ function getLLMAttributeExpressions({ ttftMs: greatestNumber(attributeField, TTFT_MS_KEYS, isJsonColumn), toolName: coalesceString(attributeField, TOOL_NAME_KEYS, isJsonColumn), agentName: coalesceString(attributeField, AGENT_NAME_KEYS, isJsonColumn), - hasAgentName: `${coalesceString(attributeField, AGENT_NAME_KEYS, isJsonColumn)} != ''`, + hasAgentName: anyKeyHasValue(AGENT_NAME_KEYS), // Emitters disagree on encoding ('stop' vs '["stop"]'); strip the JSON // array wrapper so the group-by buckets align. // The char class is written backslash-free (leading ] in an RE2 class @@ -311,22 +376,47 @@ function getLLMAttributeExpressions({ * an app's own authoritative per-call reporters — see llmGatedSumExpr. */ hasProvidedCost: `(${providedCost} > 0)`, - /** See REPORTED_TOKEN_KEYS: gates sums so wrapper spans don't double count. */ - hasReportedTokens: `(${REPORTED_TOKEN_KEYS.map(key => - isJsonColumn - ? `toString(${attributeField}.\`${key}\`) != ''` - : `${attributeField}['${key}'] != ''`, - ).join(' OR ')})`, - hasSessionId: `${coalesceString(attributeField, SESSION_ID_KEYS, isJsonColumn)} != ''`, - hasTtft: `${greatestNumber(attributeField, TTFT_MS_KEYS, isJsonColumn)} > 0`, - hasUserId: `${coalesceString(attributeField, USER_ID_KEYS, isJsonColumn)} != ''`, - hasFinishReason: `${coalesceString(attributeField, FINISH_REASON_KEYS, isJsonColumn)} != ''`, - isToolSpan: `(${[ - `${fieldAccess(attributeField, 'openinference.span.kind', isJsonColumn)} = 'TOOL'`, - `${fieldAccess(attributeField, 'gen_ai.tool.name', isJsonColumn)} != ''`, - `${fieldAccess(attributeField, 'gen_ai.tool.call.id', isJsonColumn)} != ''`, - `${fieldAccess(attributeField, 'ai.toolCall.name', isJsonColumn)} != ''`, - ].join(' OR ')})`, + /** + * See REPORTED_TOKEN_KEYS: gates sums so wrapper spans don't double count. + * Unhinted — perServiceElectionExpr only ever embeds this inside select-list + * aggregates, where a hint cannot prune and would just lengthen a query this + * file already keeps under max_query_size (see LLM_COST_SQL_ALIAS). + */ + hasReportedTokens: anyKeyNonEmpty(REPORTED_TOKEN_KEYS), + hasSessionId: anyKeyHasValue(SESSION_ID_KEYS), + // Numeric rather than non-empty: it feeds a latency percentile, and a + // zero would drag p50/p95 down. Same shape as anyKeyHasValue otherwise — + // `> 0` implies the key is present, so presence is pruning only. + hasTtft: withKeyPruning( + TTFT_MS_KEYS, + `${greatestNumber(attributeField, TTFT_MS_KEYS, isJsonColumn)} > 0`, + ), + hasUserId: anyKeyHasValue(USER_ID_KEYS), + hasFinishReason: anyKeyHasValue(FINISH_REASON_KEYS), + /** + * Tool-call rows, for WHERE position. + * + * The `= 'TOOL'` term stays a value comparison — it discriminates one + * OpenInference span kind from the others, and the query builder already + * rewrites equality onto the attribute-items index. The remaining terms + * gate a group-by on toolName, so they keep their value check too. + * + * The hint sits above the OR, not inside it: a skip index can only drop a + * granule when every arm of an OR is decidable by that same index, and the + * span-kind arm is not. Hoisting is sound because both arms imply one of + * the hinted keys. + */ + isToolSpan: withKeyPruning( + ['openinference.span.kind', ...TOOL_SPAN_KEYS], + isToolSpanMatch, + ), + /** + * isToolSpan for select-list aggregate positions (`aggCondition`), which + * renderChartConfig emits inside countIf/sumIf. Skip-index analysis never + * reaches the select list, so the hint would fold to a constant while + * lengthening every query — see withKeyPruning. + */ + isToolSpanUnhinted: isToolSpanMatch, }; } diff --git a/packages/common-utils/src/__tests__/rewriteSqlFilterWithKvItems.test.ts b/packages/common-utils/src/__tests__/rewriteSqlFilterWithKvItems.test.ts index fb81d16a91..ae7624f26a 100644 --- a/packages/common-utils/src/__tests__/rewriteSqlFilterWithKvItems.test.ts +++ b/packages/common-utils/src/__tests__/rewriteSqlFilterWithKvItems.test.ts @@ -425,4 +425,30 @@ describe('rewriteSqlFilterWithKvItems', () => { expect(result).not.toContain('hasAny('); }); }); + + describe('indexHint', () => { + // The LLM dashboard gates pair a value term with indexHint(a OR b), which + // is the first caller to put a bare boolean OR in an argument position. + // This function bails to the input verbatim on a parse error, so a grammar + // that rejected that shape would silently stop rewriting the rest of the + // condition rather than fail — hence asserting the sibling rewrite here. + it('rewrites a sibling equality and leaves the hint intact', () => { + const result = rewriteSqlFilterWithKvItems( + "(indexHint((mapContains(LogAttributes, 'openinference.span.kind') OR " + + "mapContains(LogAttributes, 'gen_ai.tool.name'))) AND " + + "(LogAttributes['openinference.span.kind'] = 'TOOL' OR " + + "coalesce(nullif(LogAttributes['gen_ai.tool.name'], ''), '') != ''))", + defaultLookup, + ); + expect(result).toContain( + "has(`LogAttributeItems`, concat('openinference.span.kind', '=', 'TOOL'))", + ); + // The hint's own mapContains calls must survive untouched — rewriting + // them to has() would point the hint at the wrong index. + expect(result).toContain( + "indexHint((mapContains(LogAttributes, 'openinference.span.kind') OR " + + "mapContains(LogAttributes, 'gen_ai.tool.name')))", + ); + }); + }); });