fix: Bound the side panel's row lookup after 'View Trace' to a time window - #2816
fix: Bound the side panel's row lookup after 'View Trace' to a time window#2816pulpdrew wants to merge 5 commits into
Conversation
🦋 Changeset detectedLatest commit: 6e016a1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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.
|
🟡 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
|
Greptile SummaryThis PR bounds the initial cross-source row lookup used by View Trace while preserving an unbounded fallback when the inferred window misses.
Confidence Score: 5/5The PR appears safe to merge. The previously reported composite timestamp failure is fixed: every timestamp component is projected, day-precision columns are skipped, and the highest-precision DateTime value anchors the trace lookup window; no blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/app/src/components/DBRowDataPanel.tsx | Adds timestamp projections, bounded row querying, and a guarded unbounded fallback without leaving the previously reported composite-timestamp failure. |
| packages/app/src/components/DBRowSidePanel.tsx | Resolves the source row’s event-time anchor, carries it through View Trace navigation, and applies the resulting window only to the landed cross-source frame. |
| packages/app/src/utils/rowTimestamps.ts | Adds bracket-aware timestamp projections, metadata-based precision selection, and validated asymmetric lookup-window construction. |
| packages/common-utils/src/core/utils.ts | Adds reusable ClickHouse timestamp-type classification supporting Date, DateTime, DateTime64, timezone arguments, and nullable wrappers. |
| packages/app/src/components/tests/DBRowSidePanel.viewTraceTimeFilter.test.tsx | Covers the full View Trace anchor propagation and verifies that composite Date plus DateTime expressions use the event-time component. |
Sequence Diagram
sequenceDiagram
participant Log as Log row panel
participant Stack as Side-panel stack
participant Trace as Trace row lookup
participant CH as ClickHouse
Log->>Log: Resolve highest-precision timestamp
Log->>Stack: Push trace frame with focusTimestamp
Stack->>Trace: Derive bounded date range
Trace->>CH: Query TraceId + SpanId within window
alt Matching span found
CH-->>Trace: Return span
else Complete bounded result is empty
Trace->>CH: Retry TraceId + SpanId without bounds
CH-->>Trace: Return fallback result
end
Reviews (6): Last reviewed commit: "Merge branch 'main' into drew/timestamp-..." | Re-trigger Greptile
E2E Test Results✅ All tests passed • 275 passed • 1 skipped • 1097s
Tests ran across 4 shards in parallel. |
1a82627 to
183ca71
Compare
|
<!-- deep-review --> Deep Review
🔴 P0/P1 — must fix
The consequence chain was independently traced by two reviewers: the bounded query succeeds with 🟡 P2 — recommended
🔵 P3 nitpicks (10)
Reviewers (12): correctness, adversarial, performance, testing, maintainability, project-standards, kieran-typescript, reliability, api-contract, julik-frontend-races, agent-native, learnings. Testing gaps:
Refuted during review (recorded so they are not re-raised): the fresh- |
Deep Review✅ No critical issues found. The bounded/fallback state machine in 🟡 P2 -- recommended
🔵 P3 nitpicks (5)
Reviewers (8): correctness, adversarial, testing, performance, julik-frontend-races, kieran-typescript, maintainability, project-standards. Testing gaps:
Environment note: |
Summary
When initially opening the trace viewer when navigating from a log row, the current row's span was looked up in the traces table by SpanId and TraceId only. This does not make use of the partition/primary keys on time stamp which are present in the OTEL schemas, making the experience slow for large volume deployments.
This change adds date range bound to the row lookup when a date range can be reasonable inferred from the source panel/page. To account for cases where the target of the lookup may be outside of the date range (eg. looking up a very long span based on a log row which was emitted near the end of the very long span), there is a fallback to the previous no-bounds query.
When navigating from a Log Row Details view to the trace waterfall, the date range is determined by using the first DateTime-type column in the log row's timestamp value expression. This handles the case where the timestamp value expression has a leading
Datetype column, which would then result in the lookup date range being centered around midnight instead of the actual time of the log message. Again, if there are no matching results based on that lookup, then the previous no-bound lookup behavior serves as the fallback.Screenshots or video
Before:
Screen.Recording.2026-08-05.at.8.41.42.AM.mov
After:
Screen.Recording.2026-08-05.at.8.41.04.AM.mov
How to test on Vercel preview
Open the trace side panel and observe the generated queries. Try it when navigating from both traces and from correlated logs. Ensure that the timestamp value expression is used rather than the displayed timestamp value expression as a filter (the latter is often not in the PK).
References