Skip to content

fix: show OTel exceptions in the side panel and trace waterfall - #3412

Merged
SpencerTorres merged 2 commits into
mainfrom
otel-semconv-exceptions
Oct 20, 2026
Merged

SpencerTorres merged 2 commits into
mainfrom
otel-semconv-exceptions

Conversation

@SpencerTorres

Copy link
Copy Markdown
Contributor

Closes HDX-5732.

Summary

Current OTel records exceptions as log records with exception.* attributes, and span exceptions only load when the trace source sets spanEventsValueExpression, which our default sources didn't. This makes exceptions show up out of the box.

Changes

  • Exception panel: opens for any log or span with exception.type/message/stacktrace attributes, not only exception span events, and now sits above the HTTP panel. Drops the always-empty Breadcrumbs section.
  • Waterfall: logs whose span was never exported (e.g. a crash log) show at the root instead of disappearing. Error logs and exception span events are red.
  • Default sources (docker-compose.yml, entry.local.base.sh, .env.development): set span events/links, add the summary and exponential histogram metric tables, and drop stale rollup and schema-stripped fields, matching the Helm chart. Existing sources aren't migrated (also submitted this change for the Helm chart).

How to test

  1. Open a log with exception.* attributes (e.g. from clickstack-browser): the Exception panel shows.
  2. With Span Events Expression set to Events on the trace source, open a trace with an exception: red marker and red error log in the waterfall.
  3. Open a trace whose error log points at a span that was never sent: the log shows at the root.

Screenshots

Exceptions panel in sidebar:
Exceptions panel in sidebar

Exceptions tooltip in waterfall:
Exceptions tooltip in waterfall

Trace source populates Events and Links by default:
Trace source populates Events and Links by default

@SpencerTorres
SpencerTorres added this pull request to stack #3413 October 20, 2026 02:05
@changeset-bot

changeset-bot Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5c18405

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

@vercel

vercel Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated
hyperdx-oss Ready Ready Preview Oct 20, 2026 7:11pm UTC
hyperdx-storybook Ready Ready Preview Oct 20, 2026 7:11pm UTC

Request Review

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

E2E Test Results

✅ All tests passed • 445 passed • 1 skipped • 1833s

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

Tests ran across 4 shards in parallel.

View full report →

@github-actions github-actions Bot added the review/tier-3 Standard — full human review required label Oct 20, 2026
@github-actions

github-actions Bot commented Oct 20, 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:

  • Cross-layer change: touches frontend (packages/app) + backend (packages/api)

Additional context: touches background tasks or the delivery pipeline lightly (4 lines, under the 30-line bar for Tier 4)

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: 165 (+ 243 in test files, excluded from tier calculation)
  • Branch: otel-semconv-exceptions
  • Author: SpencerTorres

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

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Duplicate code analysis

No new duplicate code introduced by this PR.


jscpd compared this branch against origin/main
across app, api and common-utils. Only new clones are listed; the codebase
currently holds 123 in total (1.62%).
Run yarn dupes locally to reproduce, or yarn dupes:all for the full picture.

@@ -1261,7 +1262,9 @@ export function DBTraceWaterfallChartContainer({

const barBackgroundColor =

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.

🔵 minor — Log color rule is now kept in two places: the label swatch and barColor

This diff adds the same isError ? getChartColorError() : getChartColorSuccess() log rule to both barColor (L128) and the inline barBackgroundColor ternary that colors the label swatch. Any future change to one will leave the swatch and the bar out of sync. For log rows, compute the swatch with barColor({ isHighlighted: false, isError, type }), or pull the log branch into a small shared helper.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

PR Review

If you are a coding agent acting for the author, read this first.
Nothing below blocks merge automatically; a maintainer decides. What they will expect:

  • 🔴 critical or 🟠 major in code this PR adds or changes: fix before asking for review.
  • 🔵 minor in code this PR changes: your call. Fix if small, otherwise reply.
  • Anything about surrounding code, or asking you to widen the change (hoist a helper,
    dedupe with another file, fix other call sites): never fix here, whatever the
    severity. Reply with one sentence; if it is critical, say so plainly so a human sees it.

One commit per review round. After two rounds, stop and ask a maintainer to review
scope rather than addressing more automated findings. Full rule: AGENTS.md.

3 finding(s): 🔴 0 critical · 🟠 0 major · 🔵 3 minor

2 posted as inline comment(s) on the changed lines. 1 listed below.

Findings outside the changed lines

1 minor
  • 🔵 packages/app/src/components/ExceptionSubpanel.tsx:504 — Breadcrumbs code is now unreachable in the app, and its new test proves nothing → DBRowOverviewPanel was the only app caller that passed breadcrumbs. With that gone, the breadcrumb path is only reachable from Storybook, including the reworded message join here and the whole chronologicalBreadcrumbs / Breadcrumbs section. The new ExceptionSubpanel.test.tsx checks that 'Breadcrumbs' is absent without passing breadcrumbs, so it would also pass on the old code. Either remove the breadcrumbs prop and its code, or keep it and test the message join by passing breadcrumbs={[]}.

Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides.

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found.

No P0/P1 defects. The change is well-scoped and the risky areas (waterfall root-node logic, exception-attribute precedence, memo dependency arrays, the new config defaults) were traced and hold up. The prior Greptile P1 — fresh installs lose trace views because bare Events/Links fail against the seeded schema — was independently evaluated by three reviewers and is a false positive: ClickHouse exposes a flattened Nested structure (Events.Timestamp/Events.Name/Events.Attributes, Links.*) as a selectable Array(Tuple(...)) column, and packages/app/src/source.ts has emitted spanEventsValueExpression: 'Events' against this identical schema since before this PR, so the new defaults only hardcode what the app already derives.

🟡 P2 — recommended

  • docker-compose.yml:100 — The default Sessions source was stripped of bodyExpression, eventAttributesExpression, serviceNameExpression, severityTextExpression, displayedTimestampValueExpression, traceId/spanIdExpression and its logSourceId/metricSourceId back-links across all three config blobs, a default-behavior change outside the stated exception scope whose impact on session view/correlation is unverified.
    • Fix: Confirm session search, replay, and cross-source navigation still work out of the box with the trimmed Sessions source, or restore the dropped expressions.
    • project-standards, ce-adversarial-reviewer
  • packages/app/src/components/__tests__/DBTraceWaterfallChart.test.tsx:345 — The PR's headline waterfall behavior (a crash log whose parent span was never exported surfacing at the root) is untested; both root-promotion cases use logs with no ParentSpanId, exercising only the !nodeParentSpanId branch and never the dangling-parent branch !validSpanIDs.has(nodeParentSpanId).
    • Fix: Add a case where the log's ParentSpanId points to a span id absent from the trace and assert the log node becomes a root rather than nesting.
    • testing, correctness, ce-adversarial-reviewer
🔵 P3 nitpicks (6)
  • packages/app/src/components/DBRowOverviewPanel.tsx:150 — The exceptionAttributes memo encodes an undocumented two-tier precedence and its fallback returns the entire flattenedEventAttributes bag, so the name reads as exception-scoped when it is not.
    • Fix: Rename to something like exceptionAttributeSource and add a comment stating the span-event-over-flattened precedence, or narrow the fallback to the exception.* keys.
    • maintainability, kieran-typescript
  • docker/hyperdx/entry.local.base.sh:39 — The DEFAULT_SOURCES JSON blob is duplicated across four locations (docker-compose.yml, this script's base and beta variants, and .env.development) with no single source of truth, which forced this PR to hand-edit every copy in lockstep (pre-existing).
    • Fix: Extract the default sources into one canonical JSON file or generator that all deploy paths read.
  • .changeset/otel-semconv-exceptions.md:5 — The changeset documents the span-events/links and metric-table additions but omits the Sessions-source trimming and the removal of inter-source relationship IDs it also introduces.
    • Fix: Add a sentence covering the Sessions-source and cross-link changes so the changelog reflects the actual default-source behavior.
  • packages/app/src/components/DBRowOverviewPanel.tsx:158 — A log carrying only exception.parsed_stacktrace (no type/message/stacktrace) would not open the Exception panel, since EXCEPTION_ATTRIBUTE_KEYS omits exception.parsed_stacktrace while exceptionValues reads from it.
    • Fix: Add exception.parsed_stacktrace to EXCEPTION_ATTRIBUTE_KEYS if that attribute shape can appear on a flattened log.
  • packages/app/src/components/DBTraceWaterfallChart.tsx:1267 — The new error-log red-coloring branch for barBackgroundColor (the row-label dot) is not asserted by any test; only barColor's error branch is covered.
    • Fix: Assert the rendered error-log indicator color, or consolidate the duplicated coloring logic with barColor.
  • packages/app/src/components/ExceptionSubpanel.tsx:509 — The breadcrumb message built via [type, value].filter(Boolean).join(': ') is not asserted; only the inline display join is tested, leaving the no-dangling-colon behavior unlocked there.
    • Fix: Add an assertion on the breadcrumb message when only type or only value is present.

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

Testing gaps: crash-log dangling-parent root promotion (the stated intent) is unverified; no test asserts the tooltip renders absolute epochMs vs the offset timestamp; no test confirms a log whose parent span is present stays nested rather than double-promoted; the Exception panel opening from flattened log attributes (vs span-event attributes) and their precedence is uncovered.

No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond

@greptile-apps

greptile-apps Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The changes affect exception and trace views plus local default source setup.

Fresh installations can still lose trace details and the waterfall, so the previous finding needs a fix before merging.

Fix All in Claude CodeFindings

  1. P1 Fresh installations lose trace views ▶

Summary

Shows exceptions from row attributes and keeps logs visible when their spans were not exported.

  • The side panel shows exceptions from logs and spans.
  • Trace waterfalls keep logs whose spans are missing and mark errors in red.
  • Default trace sources load span events and links automatically.

The previous-review merge-base checks did not match, so this review used the full PR diff.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Row attributes] --> C{Exception event present?}
  B[Span exception event] --> C
  C -->|Yes| D[Show event exception]
  C -->|No| E[Show row exception attributes]
  F[Correlated log] --> G{Matching span present?}
  G -->|Yes| H[Show beneath span]
  G -->|No| I[Show at root]
Loading

Reviews (5) · Last reviewed commit: "fix: exception timestamp in waterfall to..." · Reviewed by Greptile

Comment thread docker-compose.yml
'[{"from":{"databaseName":"default","tableName":"otel_logs"},"kind":"log","timestampValueExpression":"Timestamp","name":"Logs","displayedTimestampValueExpression":"Timestamp","implicitColumnExpression":"Body","serviceNameExpression":"ServiceName","bodyExpression":"Body","eventAttributesExpression":"LogAttributes","resourceAttributesExpression":"ResourceAttributes","defaultTableSelectExpression":"Timestamp,ServiceName,SeverityText,Body","severityTextExpression":"SeverityText","traceIdExpression":"TraceId","spanIdExpression":"SpanId","metadataMaterializedViews":{"kvRollupTable":"otel_logs_kv_rollup_15m","granularity":"15
minute"},"connection":"Local
ClickHouse","traceSourceId":"Traces","sessionSourceId":"Sessions","metricSourceId":"Metrics"},{"from":{"databaseName":"default","tableName":"otel_traces"},"kind":"trace","timestampValueExpression":"Timestamp","name":"Traces","displayedTimestampValueExpression":"Timestamp","implicitColumnExpression":"SpanName","serviceNameExpression":"ServiceName","eventAttributesExpression":"SpanAttributes","resourceAttributesExpression":"ResourceAttributes","defaultTableSelectExpression":"Timestamp,ServiceName,StatusCode,round(Duration/1e6),SpanName","traceIdExpression":"TraceId","spanIdExpression":"SpanId","durationExpression":"Duration","durationPrecision":9,"parentSpanIdExpression":"ParentSpanId","spanNameExpression":"SpanName","spanKindExpression":"SpanKind","statusCodeExpression":"StatusCode","statusMessageExpression":"StatusMessage","metadataMaterializedViews":{"keyRollupTable":"otel_traces_key_rollup_15m","kvRollupTable":"otel_traces_kv_rollup_15m","granularity":"15
ClickHouse","traceSourceId":"Traces","metricSourceId":"Metrics"},{"from":{"databaseName":"default","tableName":"otel_traces"},"kind":"trace","timestampValueExpression":"Timestamp","name":"Traces","displayedTimestampValueExpression":"Timestamp","implicitColumnExpression":"SpanName","serviceNameExpression":"ServiceName","eventAttributesExpression":"SpanAttributes","resourceAttributesExpression":"ResourceAttributes","defaultTableSelectExpression":"Timestamp,ServiceName,StatusCode,round(Duration/1e6),SpanName","traceIdExpression":"TraceId","spanIdExpression":"SpanId","durationExpression":"Duration","durationPrecision":9,"parentSpanIdExpression":"ParentSpanId","spanNameExpression":"SpanName","spanKindExpression":"SpanKind","statusCodeExpression":"StatusCode","statusMessageExpression":"StatusMessage","spanEventsValueExpression":"Events","spanLinksValueExpression":"Links","metadataMaterializedViews":{"kvRollupTable":"otel_traces_kv_rollup_15m","granularity":"15

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.

P1 Fresh installations lose trace views

The seeded otel_traces table declares Events.Timestamp, Events.Name, and Events.Attributes, but no Events column. The same applies to Links. These new defaults make useRowData select bare Events and Links, and the waterfall select bare Events. Those queries fail against the seeded schema, so newly provisioned sources cannot load trace details or the waterfall.

Build the expected arrays from the dotted columns, or add matching columns before enabling these defaults. The same defaults appear in docker/hyperdx/entry.local.base.sh and packages/api/.env.development.

Fix in Claude Code Fix in Conductor Fix in Cursor Fix in Codex

const formatTime = useFormatTime();
// OTel records exceptions on spans as events named `exception`.
const isException = marker.name === 'exception';
const markerColor = isException

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.

🔵 minor — Exception marker red uses a UI token, not the chart error color the waterfall uses

The waterfall paints error bars and error-log swatches with getChartColorError() (--color-chart-error, set in packages/app/src/utils.ts:765). This marker uses var(--color-bg-danger), which is a different red in both brands (for example mantine red-4 or #ef4444). So an error span and its exception marker come out in two different reds. Use getChartColorError() here instead, as the chart-color contract in the repository conventions asks.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

@@ -1261,7 +1266,9 @@ export function DBTraceWaterfallChartContainer({

const barBackgroundColor =

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.

🔵 minor — Label swatch color repeats barColor logic, and both had to be edited by hand

This PR changes the log/error color rule in two places: barColor() (line 128) and this inline barBackgroundColor ternary. The two already disagree for error spans: the bar is red but the swatch stays the service color. Build the swatch from barColor({ isHighlighted: false, isError, type, serviceColor }), or one shared helper, so the color rule has a single source.

Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond

@jordan-simonovski jordan-simonovski left a comment

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.

Looks good! Very exciting

@SpencerTorres
SpencerTorres merged commit e7b38c2 into main Oct 20, 2026
29 checks passed
@SpencerTorres
SpencerTorres deleted the otel-semconv-exceptions branch October 20, 2026 19:19

This branch was successfully deployed

2 active deployments
Preview – hyperdx-oss — 5c184054 Deployed Oct 20, 2026 by vercel[bot]
Preview – hyperdx-storybook — 5c184054 Deployed Oct 20, 2026 by vercel[bot]
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.

3 participants