Skip to content

feat: narrow the original search with a trace-id subquery when searching from a correlated event - #3018

Open
karl-power wants to merge 2 commits into
mainfrom
cross-source-search
Open

feat: narrow the original search with a trace-id subquery when searching from a correlated event#3018
karl-power wants to merge 2 commits into
mainfrom
cross-source-search

Conversation

@karl-power

@karl-power karl-power commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

"Search for this value only" on an event from a correlated source (e.g. a log opened from a trace search) previously switched the search to the event's source (#2825), losing the original search's context. It now keeps the searched source, select, and filters, and replaces the search box with a trace-id subquery on the event's table in plain SQL the user can see and edit:

TraceId IN (SELECT TraceId FROM default.otel_logs WHERE Body LIKE '%exception%')
  • Like the same-source action, the search box is replaced ("Search for this value only"); filter facets and select are kept. Works in both directions (log condition on a trace search, span condition on a log search).
  • Attribute chips on cross-source events emit their SQL condition form, since lucene can't be embedded in the subquery's raw SQL.
  • The trace-level attribute chips above the waterfall (feat: Add custom trace-level attributes above trace waterfall #1356) keep their pivot-to-attribute-source behavior, now signalled explicitly via a pivot flag on generateSearchUrl.

Screenshots or video

Screen.Recording.2026-08-28.at.15.32.38.mov

How to test on Vercel preview

Preview routes: /search

Steps:

  1. Select the traces source, run a search, and open a trace row's side panel.
  2. Open the Trace tab and click a correlated log event in the waterfall (marked with the log-line icon).
  3. In the event detail panel, open the Column Values tab, hover a value row, and click the "Search for this value only" action.
  4. Verify the search stays on the traces source with a TraceId IN (SELECT ...) SQL condition in the search box, and results load without a SQL error.

References

@vercel

vercel Bot commented Aug 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview Aug 28, 2026 3:23pm
hyperdx-storybook Ready Ready Preview Aug 28, 2026 3:23pm

Request Review

@changeset-bot

changeset-bot Bot commented Aug 28, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: f7777ca

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch
@hyperdx/otel-collector Patch

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

@github-actions github-actions Bot added the review/tier-2 Low risk — AI review + quick human skim label Aug 28, 2026
@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

🟡 Tier 3 — Standard

Introduces new logic, modifies core functionality, or touches areas with non-trivial risk.

Why this tier:

  • Diff size: 278 production lines changed (Tier 2 max: < 250)

Review process: Full human review — logic, architecture, edge cases.
SLA: First-pass feedback within 1 business day.

Stats
  • Production files changed: 8
  • Production lines changed: 278 (+ 355 in test files, excluded from tier calculation)
  • Branch: cross-source-search
  • Author: karl-power

To override this classification, remove the review/tier-3 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@greptile-apps

greptile-apps Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR preserves the original search source when searching from a correlated event by generating an editable trace-ID SQL subquery.

  • Adds correlated-search SQL generation while retaining the original select and filters.
  • Emits SQL predicates for cross-source attribute actions and preserves explicit source pivots for trace-level attributes.
  • Adds unit and end-to-end coverage for correlated log-to-trace searches.
  • Correctly quotes configured database and table identifiers and escapes resource-attribute paths in response to the previous review findings.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/app/src/utils/correlatedSearch.ts Builds the trace-ID subquery and now safely quotes configured database and table identifiers.
packages/app/src/DBSearchPage.tsx Distinguishes explicit source pivots from correlated-event narrowing while preserving the searched source configuration.
packages/app/src/components/DBRowOverviewPanel.tsx Generates escaped SQL attribute paths for cross-source resource-attribute search actions.
packages/app/src/components/DBHighlightedAttributesList.tsx Selects SQL predicates for correlated attributes while retaining Lucene predicates for same-source and explicit-pivot actions.
packages/app/tests/e2e/features/search/cross-source-search.spec.ts Covers URL generation, source preservation, SQL execution, and overview attribute searches across correlated sources.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Original search source] --> B[Open correlated event]
  B --> C[Search for this value only]
  C --> D[Build event-source SQL predicate]
  D --> E[Build TraceId IN subquery]
  E --> F[Search original source]
Loading

Reviews (2): Last reviewed commit: "fix quoting and escaping" | Re-trigger Greptile

Comment thread packages/app/src/utils/correlatedSearch.ts Outdated
Comment thread packages/app/src/components/DBRowOverviewPanel.tsx Outdated
@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Deep Review

No critical issues found. This is a self-contained, well-tested frontend change. The two prior P1 review comments (unquoted table identifiers in correlatedSearch.ts; unescaped resource-attribute keys in DBRowOverviewPanel.tsx) are both resolved in the current checkout: table/database names now route through quoteIdentifierIfNeeded, and resource-attribute keys route through mergePath (map branch escapes via escapeSqlSingleQuoted, JSON branch backtick-quotes), each with test coverage.

🟡 P2 — recommended

  • packages/app/src/DBSearchPage.tsx:1694 — The new three-way branch in generateSearchUrl (pivot / correlated-subquery / same-source) is only exercised indirectly: buildCorrelatedSearchWhere is unit-tested and the correlated path has an e2e test, but the branch selection inside generateSearchUrl (especially the pivot branch's URL output, and that select/filters/source are preserved in the correlated branch) has no direct unit assertion.
    • Fix: Add a unit test that drives generateSearchUrl with pivot, correlated, and same-source inputs and asserts the resulting query params.
🔵 P3 nitpicks (2)
  • packages/app/src/utils/correlatedSearch.ts:46buildCorrelatedSearchWhere does not guard an empty/whitespace eventWhere; an empty condition yields the malformed ... WHERE ). Callers currently always pass a non-empty condition, so this is defensive only.
    • Fix: Return early or omit the WHERE clause when eventWhere.trim() is empty.
  • packages/app/src/utils/correlatedSearch.ts:40 — When a source has a databaseName but a falsy tableName, eventTable renders as <db>.undefined; valid configured sources always have a table name, so this is unreachable in practice.
    • Fix: Fall back gracefully (or assert) when tableName is absent rather than interpolating undefined.

Pre-existing (not introduced by this diff): The Column Values "Search for this value only" action in packages/app/src/components/DBRowJsonViewer.tsx builds <field> = '<value>' without escaping the value; a value containing a single quote produces malformed SQL. This diff routes that condition into the new subquery, but the same malformed-SQL risk already existed for the same-source and prior pivot behavior, so it is not a regression. Worth a follow-up to escape via SqlString for parity with EventTag.

Design note (accepted trade-off): The subquery targets the event source's table (e.g. default.otel_logs) but executes on the searched source's connection. If the two sources live on different ClickHouse connections the query fails; the PR documents this as acceptable and the failure surfaces in an editable search box. A unit test covers the cross-connection case.


Reviewers (10): correctness, security, adversarial, testing, maintainability, kieran-typescript, project-standards, previous-comments, agent-native, learnings-researcher.

Coverage note: Findings were synthesized from a direct trace of the full data flow (EventTag → deriveRowSidePanelContextForSourcegenerateSearchUrlbuildCorrelatedSearchWhere, plus the DBRowJsonViewer and DBRowOverviewPanel paths) and the completed previous-comments review, which confirmed both prior P1s resolved.

Testing gaps: No direct unit test of generateSearchUrl branch selection (pivot vs correlated vs same-source URL output).

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 327 passed • 1 skipped • 1289s

Status Count
✅ Passed 327
❌ Failed 0
⚠️ Flaky 0
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

? `${quoteIdentifierIfNeeded(databaseName)}.${quotedTable}`
: quotedTable;

return `${searchedTraceId} IN (SELECT ${eventTraceId} FROM ${eventTable} WHERE ${eventWhere.trim()})`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect this will not scale past relatively small data volumes. Here are the issues I see:

  1. If I'm reading this right, the subselect is not bound by a time range, so it's a full table scan in most cases
  2. I suspect the custom SQL eventWhere will not hit many of our optimizations that try to make use of indexes, like rewrites for using full-text indexes when accessing attributes, it would be good to check that and see if we can do anything to ensure they work
  3. Making (1) worse is the fact that the search page is paginated, so for every page of results, the full-table scan re-runs despite querying the same results. Are there ways we can cache the subselect results?
  4. For conditions that return a lot of traces (eg. 10B traces match the filter), the sub-select returns a very large result set, which is then like a very high-cardinality join, which can use a lot of memory or spill to disk. Should we consider putting a limit on the subselect? This could be a worse user experience though because the results will generally not be complete. This is a difficult problem to solve when the filtering by a low-specificity condition, we've struggled with it in the past.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-3 Standard — full human review required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants