Skip to content

feat(app): add faceted filters to the sessions page - #2992

Open
vinzee wants to merge 1 commit into
hyperdxio:mainfrom
vinzee:add-session-filters
Open

feat(app): add faceted filters to the sessions page#2992
vinzee wants to merge 1 commit into
hyperdxio:mainfrom
vinzee:add-session-filters

Conversation

@vinzee

@vinzee vinzee commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Bring the search page's faceted filter sidebar to /sessions and wire the selected filters into the session aggregation query.

  • Add a SessionFilters wrapper around DBSearchPageFilters that hides the analysis-mode tabs, denoise, and root-spans-only toggles that don't apply to sessions.
  • Persist sidebar filters in the URL and thread them through useSessions so they constrain the trace aggregation alongside the free-text query.
  • Scope both the facet sidebar and the aggregation to RUM session spans:
  • Add unit tests covering the new sessions behavior.

Screenshots or video

The Sessions Tab now has a sidebar with filters:
image

How to test on Vercel preview

Preview routes: /sessions

Steps (/sessions):

  1. Open /sessions.
  2. In the "Data Source" dropdown (combobox labeled "Data Source"), confirm a session source (labeled "Sessions") is selected; if it is not, open the dropdown and choose the "Sessions" source.
  3. Wait for the results area (data-testid "session-card-list") to render at least one session card (each card has a data-testid beginning with "session-card-").
  4. Confirm the filters sidebar is visible to the left of the results with a "Filters" heading.
  5. Confirm the sidebar is populated: at least one facet group is listed (for example a group labeled "ServiceName") and the text "No filters available" is NOT shown.
  6. Click the "ServiceName" facet group header to expand it, then click the first value's checkbox inside its panel.
  7. Confirm an active filter pill appears above the results list and the page URL gains a "filters=" query parameter.
  8. Click "Hide filters", confirm the sidebar collapses, then click "Show filters" and confirm the sidebar reappears.
  9. Verify the results area (data-testid "session-card-list") re-rendered after the filter was applied and no red error notification was shown.

References

  • Linear Issue: n/a
  • Related PRs: n/a

@changeset-bot

changeset-bot Bot commented Aug 25, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 2b5bdd1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel

vercel Bot commented Aug 25, 2026

Copy link
Copy Markdown

@vinzee is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps

greptile-apps Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR adds URL-persisted faceted filtering to the sessions page and applies those filters to correlated RUM trace aggregation.

  • Adds a sessions-specific wrapper around the shared filter sidebar.
  • Builds facet queries scoped to RUM session spans.
  • Threads structured filters through session aggregation and adds focused unit coverage.

Confidence Score: 3/5

The PR is not yet safe to merge because service scoping can hide valid sessions and source changes can execute stale filters against a different trace schema.

The service-discovery optimization still turns a partial session-source service set into a hard trace filter, including when independently retained trace data has no corresponding recording row, while source changes continue forwarding filters created for the prior trace source.

Files Needing Attention: packages/app/src/sessions.ts and packages/app/src/SessionsPage.tsx

Important Files Changed

Filename Overview
packages/app/src/SessionsPage.tsx Adds filter URL state, trace metadata resolution, the facet sidebar, active-filter pills, and filter forwarding into session queries.
packages/app/src/sessions.ts Extends session aggregation with structured filters and a session-source-derived service-scope optimization.
packages/app/src/components/DBSearchPageFilters.tsx Makes analysis controls optional and supports forcing exact facet discovery for the sessions view.
packages/app/src/components/SessionFilters.tsx Provides a thin sessions-specific configuration of the shared facet sidebar.
packages/app/src/tests/sessions.test.ts Covers service-scope SQL construction, escaping, and URL filter parsing.
packages/app/src/components/tests/SessionFilters.test.tsx Verifies the sessions wrapper forces exact facets and hides inapplicable analysis controls.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    U[Sessions page controls] --> S[Selected session source]
    S --> T[Correlated trace source]
    U --> F[URL-persisted facet filters]
    F --> Q[Session trace aggregation]
    T --> Q
    S --> D[Service-scope discovery]
    D --> Q
    Q --> L[Session card list]
Loading

Reviews (3): Last reviewed commit: "feat(app): add faceted filters to the se..." | Re-trigger Greptile

Comment thread packages/app/src/sessions.ts Outdated
Comment thread packages/app/src/SessionsPage.tsx
@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Deep Review

Faceted-filter sidebar added to /sessions, URL-persisted filters threaded into the session trace aggregation, plus a ServiceName IN (...) service-scope optimization derived from a 30-day session-source scan. Findings below were verified directly against the diff and the surrounding code; the two prior P1 review comments on the service-scope behavior were re-checked against the current code and remain applicable.

🔴 P0/P1 — must fix

  • packages/app/src/sessions.ts:234 — The derived ServiceName IN (...) scope is applied as a hard filter, so a non-empty-but-incomplete service list silently drops valid sessions whenever a service has qualifying RUM trace spans in range but no session-source row in the 30-day window (trace retention > session retention, a service that stopped emitting session rows, or session/trace ServiceName divergence); the try/catch only covers query failure and the empty-list path only covers zero results, leaving the incomplete-list case unguarded.
    • Fix: Use the derived service list only as a non-exclusive index hint, or skip the IN predicate entirely when the list cannot be proven complete, so the aggregation never removes a service the unscoped scan would have returned.

🟡 P2 — recommended

  • packages/app/src/SessionsPage.tsx:345 — On a source change the auto-submit spreads the form's existing filters into setAppliedConfig for the newly selected source, so facet filters built against the previous trace schema are sent to a source that may lack those columns, producing a failed or mis-scoped query.
    • Fix: Reset the form and URL filters to [] when the selected source changes before submitting.
  • packages/app/src/sessions.ts:192 — The riskiest new behavior — the service-scope scan, its try/catch unscoped fallback, the empty-list fallback, and the hasSearchQuery HAVING/final-query branching — has no test coverage; only the pure buildServiceScopeFilters helper and the URL parser are exercised.
    • Fix: Add useSessions tests covering the fallback-to-unscoped path and the hasSearchQuery branch selection.
🔵 P3 nitpicks (3)
  • packages/app/src/components/DBSearchPageFilters.tsx:550 — Previously required props (analysisMode, setAnalysisMode, showDelta, denoiseResults, setDenoiseResults) were made optional with silent voidFunc/false/'results' defaults, removing the compile-time guarantee that the search-page caller supplies them.
    • Fix: Keep the props required and expose the sessions-only relaxations through a separate optional prop type consumed only by SessionFilters.
  • packages/app/src/SessionsPage.tsx:406 — The RUM-session-scope SQL predicate is constructed here (notEmpty(...['rum.sessionId']) AND indexHint(...)) and again in sessions.ts, so the two scoping definitions can drift.
    • Fix: Extract the RUM-session-scope predicate into one shared helper used by both call sites.
  • .changeset/ — No changeset accompanies these @hyperdx/app changes, which will skip a version bump if one is expected for this package.
    • Fix: Add a changeset if this user-facing change should trigger a release, or confirm the omission is intentional.

Reviewers (10): correctness, security, adversarial, performance, kieran-typescript, julik-frontend-races, testing, maintainability, previous-comments, project-standards.

Testing gaps:

  • No coverage of the service-scope fallback (query failure → unscoped scan; empty list → unscoped scan).
  • No coverage of stale-filter behavior when the session source changes.
  • No coverage of the hasSearchQuery HAVING / final-query branch selection in useSessions.

Verified as sound (no finding): escapeSqlString correctly neutralizes quote/backslash injection in buildServiceScopeFilters (doubles backslashes then single quotes, matching ClickHouse literal escaping), and the ?filters= parser rejects malformed shapes and defaults to [] rather than white-screening. The raw-condition capability of type:'sql' filters is pre-existing search-page behavior, not newly introduced here.

@vinzee
vinzee force-pushed the add-session-filters branch from fa84b72 to d81257b Compare August 25, 2026 01:22
Comment thread packages/app/src/sessions.ts
Bring the search page's faceted filter sidebar to /sessions and wire the
selected filters into the session aggregation query.

- Add a SessionFilters wrapper around DBSearchPageFilters that hides the
  analysis-mode tabs, denoise, and root-spans-only toggles that don't
  apply to sessions.
- Persist sidebar filters in the URL and thread them through useSessions
  so they constrain the trace aggregation alongside the free-text query.
  Validate the ?filters= param against FilterSchema with a [] default so a
  stale/hand-edited param resolves to the default instead of white-screening.
- Speed up the aggregation: resolve the RUM service name(s) from the
  session source and constrain the otel_traces scan by ServiceName,
  turning a ~100k-mark full scan into a small primary-key range. Enumerate
  those services over a window wider than the selected range so sessions
  that started just before the range aren't dropped, and on any failure or
  when no services resolve, fall back to the unscoped (still correct) scan.
- Harden the ServiceName scope: escape ingested names with escapeSqlString
  (backslash then quote) in a dedicated buildServiceScopeFilters helper and
  build it inside the try/catch, so a crafted/backslash name can't inject
  into or break the aggregation.
- Fix the empty filter sidebar: scope facet queries to RUM session spans
  via a rum.sessionId indexHint and force exact facet mode, so the
  'show all values' path can't strip the scope and sample the whole trace
  table (which timed out and left the sidebar empty).
- Portal the source-select dropdown so it renders above the filter
  sidebar instead of being clipped by its stacking context.
- Add unit tests: SessionFilters prop forwarding, buildServiceScopeFilters
  escaping/fallback, and the ?filters= param validator.

Co-authored-by: Cursor <cursoragent@cursor.com>
@vinzee
vinzee force-pushed the add-session-filters branch from d81257b to 2b5bdd1 Compare August 25, 2026 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant