fix: support service name expression and quick attribute filters in surrounding context#2558
fix: support service name expression and quick attribute filters in surrounding context#2558MikeShi42 wants to merge 10 commits into
Conversation
…urrounding context - Use serviceNameExpression from the source configuration for the Service filter instead of hardcoded ResourceAttributes['service.name']. This makes the Service filter work with non-OTEL schemas that use custom column names (e.g. ModuleName). - Use resourceAttributesExpression from the source instead of hardcoded 'ResourceAttributes' column name for Host/Pod/Node filters. - Add quick event attribute filters: users can toggle attributes from the current event (resource attributes, event attributes, and top-level columns) to narrow down surrounding context results. - Quick filters are additive (AND'd) with the selected context filter. Co-authored-by: Mike Shi <mike@hyperdx.io>
Co-authored-by: Mike Shi <mike@hyperdx.io>
🦋 Changeset detectedLatest commit: c97c9ca The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR redesigns the Surrounding Context filter UX from a segmented control that only worked with OTEL schemas to a preset + pill system that supports any schema's
Confidence Score: 4/5Safe to merge for OTEL schemas; the one gap — a silent empty WHERE clause for non-OTEL sources with complex Lucene service expressions — affects a narrow edge case and does not corrupt data, only misleads the filter count display. The core of the fix (state reset, preset+pill logic, quote escaping) is correct and well-tested. The only defect is in ContextFilterPills.tsx lines 78–91: when serviceNameExpression is a complex bracket-notation string that cannot be used as a Lucene field path and the row's ResourceAttributes also lacks service.name, the svc filter pill is shown and selectable but generateWhere(false) returns empty string. The Matching on N attributes header counts it, but the actual query receives no service filter. All OTEL schemas use 'ServiceName' which passes the safe-field check, so they are unaffected. packages/app/src/components/ContextFilterPills.tsx — specifically the generateWhere closure inside extractQuickFilters for the svc pill (lines 78–91). Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Row selected] --> B[extractQuickFilters]
B --> C{serviceNameExpression set?}
C -- Yes + value is string --> D[Add 'svc' pill]
C -- No, but resourceAttrs.service.name exists --> E[Add 'ra:service.name' pill]
D --> F[Add promoted RA keys: host, pod, namespace, node]
E --> F
F --> G[Add remaining RA keys]
G --> H[Add event attribute keys]
H --> I[Add top-level column keys]
I --> J[availableFilters rendered as pills]
J --> K{User action}
K -- Clicks preset --> L[setActivePreset + getPresetFilterIds]
K -- Clicks pill --> M[toggleFilter → setActivePreset 'custom']
K -- Clicks Clear all --> N[setSelectedFilterIds empty + setActivePreset 'all']
L --> O[buildContextWhereClause]
M --> O
N --> O
O --> P{selectedFilterIds non-empty?}
P -- Yes --> Q[generateWhere per filter]
P -- No --> R[empty WHERE clause]
Q --> S{Lucene + unsafe expr + no resourceServiceName?}
S -- Yes --> T[generateWhere returns '' — silent no-op]
S -- No --> U[Valid WHERE clause ANDed together]
T --> V[DBSqlRowTable query]
U --> V
R --> V
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Row selected] --> B[extractQuickFilters]
B --> C{serviceNameExpression set?}
C -- Yes + value is string --> D[Add 'svc' pill]
C -- No, but resourceAttrs.service.name exists --> E[Add 'ra:service.name' pill]
D --> F[Add promoted RA keys: host, pod, namespace, node]
E --> F
F --> G[Add remaining RA keys]
G --> H[Add event attribute keys]
H --> I[Add top-level column keys]
I --> J[availableFilters rendered as pills]
J --> K{User action}
K -- Clicks preset --> L[setActivePreset + getPresetFilterIds]
K -- Clicks pill --> M[toggleFilter → setActivePreset 'custom']
K -- Clicks Clear all --> N[setSelectedFilterIds empty + setActivePreset 'all']
L --> O[buildContextWhereClause]
M --> O
N --> O
O --> P{selectedFilterIds non-empty?}
P -- Yes --> Q[generateWhere per filter]
P -- No --> R[empty WHERE clause]
Q --> S{Lucene + unsafe expr + no resourceServiceName?}
S -- Yes --> T[generateWhere returns '' — silent no-op]
S -- No --> U[Valid WHERE clause ANDed together]
T --> V[DBSqlRowTable query]
U --> V
R --> V
Reviews (7): Last reviewed commit: "fix: address surrounding context review ..." | Re-trigger Greptile |
E2E Test Results✅ All tests passed • 224 passed • 3 skipped • 1515s
Tests ran across 4 shards in parallel. |
Address UX feedback: - Remove confusing two-layer system (ContextBy segmented control + separate event filters). All filtering is now done via a single set of always-visible filter pills. - Filter pills are always visible (no toggle/expand needed). - Service, Host, Pod, Node are promoted as the first pills so they're easy to find. - Selected pills show a clear X icon for easy removal. - Property names no longer truncated (wider labels, larger pill maxWidth). - Custom search is available via a search icon toggle. - Multiple pills can be selected simultaneously (AND'd together). Co-authored-by: Mike Shi <mike@hyperdx.io>
Split filter pill logic (extractQuickFilters, FilterPill component, and helper functions) into a separate file to keep ContextSidePanel.tsx under the 300-line limit. Co-authored-by: Mike Shi <mike@hyperdx.io>
- Add useEffect to reset selectedFilterIds when rowId changes, preventing stale filters from carrying over between events. - Remove export from formatColumnEquals (only used internally) to fix Knip unused-export CI check. Co-authored-by: Mike Shi <mike@hyperdx.io>
Implement the hybrid preset + pill design: - MATCH ON segmented control (All/Service/Host/Pod/Node/Custom) acts as preset shortcuts that auto-select groups of related filter pills. - Service preset selects the service pill; Pod selects service + pod + namespace; Host selects service + host; Node selects service + node. - All available attribute pills are always visible below the presets. - Users can manually toggle individual pills on top of or instead of presets for fine-grained control. - 'Matching on N attributes' header with Clear all. - Legend distinguishes matching (solid yellow border) from available (dashed border). - Pills show checkmark when selected, plus when available. - k8s.namespace.name added to promoted resource attributes for Pod preset support. Co-authored-by: Mike Shi <mike@hyperdx.io>
- Rename 'All' preset to 'Anything' for clarity. - MATCH ON segmented control uses fit-content width instead of stretching 100%. - Manual pill toggles flip the preset to 'Custom' to indicate a non-preset selection. - Remove Badge border from the ±time display for visual consistency; show as plain text. - Remove unused Badge import. Co-authored-by: Mike Shi <mike@hyperdx.io>
Co-authored-by: Mike Shi <mike@hyperdx.io>
…, add tests - Remove showCustomSearch state; derive from activePreset === 'custom'. - Move formatColumnEquals to @/utils alongside formatAttributeClause (DRY: was the only local utility not shared). - Add ErrorBoundary around filter pills section to limit blast radius. - Add 19 unit tests for ContextFilterPills (extractQuickFilters, getPresetFilterIds, getAvailablePresets) covering OTEL/non-OTEL schemas, value escaping, promoted attributes, and preset logic. - Add 2 unit tests for formatColumnEquals in utils.test.ts. Co-authored-by: Mike Shi <mike@hyperdx.io>
🟡 Tier 3 — StandardIntroduces new logic, modifies core functionality, or touches areas with non-trivial risk. Why this tier:
Review process: Full human review — logic, architecture, edge cases. Stats
|
Deep ReviewScope: Intent: Make surrounding-context filtering work on non-OTEL schemas and let users toggle event attributes as WHERE-clause filters. The new pill system builds ClickHouse SQL / Lucene predicates from the event's attribute keys and values. 🔴 P0/P1 — must fix
🟡 P2 — recommended
🔵 P3 nitpicks (4)
Reviewers (6): ce-security-reviewer, ce-correctness-reviewer, ce-testing-reviewer, ce-maintainability-reviewer, ce-adversarial-reviewer, ce-kieran-typescript-reviewer. Coverage note: The primary finding was returned by the security reviewer and independently confirmed against the codebase (two existing correct escapers, Testing gaps:
|
- Escape attribute filter values in formatAttributeClause for SQL and Lucene. - Reset custom search form state when the selected row changes. - Use the active custom where language when generating surrounding context queries. - Avoid duplicate service.name pills when serviceNameExpression is available. - Avoid generating Lucene clauses from non-bare serviceNameExpression values. - Extract and test buildContextWhereClause for 0/1/many/custom clause paths. - Add ContextSidePanel regression tests for custom input visibility and row-change reset. Co-authored-by: Mike Shi <mike@hyperdx.io>
| generateWhere: isSql => { | ||
| if (isSql || isSafeLuceneFieldExpression(serviceNameExpr)) { | ||
| return formatColumnEquals(serviceNameExpr, serviceNameValue, isSql); | ||
| } | ||
| if (resourceAttrExpr && resourceServiceName) { | ||
| return formatAttributeClause( | ||
| resourceAttrExpr, | ||
| 'service.name', | ||
| resourceServiceName, | ||
| isSql, | ||
| ); | ||
| } | ||
| return ''; | ||
| }, |
There was a problem hiding this comment.
Service pill silently emits empty WHERE clause in Lucene mode
When serviceNameExpression fails isSafeLuceneFieldExpression (e.g., a bracket-notation expression like ResourceAttributes['service.name']) and resourceAttrs['service.name'] is absent, generateWhere(false) returns ''. buildContextWhereClause silently drops empty clauses, so selecting this pill records it in selectedFilterIds, shows "Matching on 1 attributes" in the header, but generates no actual WHERE condition — the query returns all logs exactly as if "Anything" were selected. Consider either omitting the svc pill when no safe Lucene clause can be produced, or surfacing a tooltip/warning when the selected filter is a no-op.
Summary
Fixes the Surrounding Context feature for non-OTEL schemas (HDX-4665) and redesigns the filter UX with a hybrid preset + pill approach.
Problem: The old design had a segmented control (All/Service/Host/Custom) that only worked with OTel schema. Users with custom schemas saw only "All" and "Custom", losing the ability to quickly filter surrounding context.
Solution: A unified preset + pill system:
serviceNameExpressionfrom the source config, making the service filter work with non-OTEL schemas.Screenshots or video
Default state — "Anything" selected, pills visible:
Default state
"Service" preset auto-selects service pills:
Service preset
Manual pill toggle flips to "Custom":
Custom mode
Demo video:
surrounding_context_polished_ux.mp4
How to test on Vercel preview
Preview routes: /search
Steps:
References
To show artifacts inline, enable in settings.
Linear Issue: HDX-4665