Skip to content

feat(dashboards): release markers on tile charts - #2894

Open
teeohhem wants to merge 1 commit into
mainfrom
tom/release-markers
Open

feat(dashboards): release markers on tile charts#2894
teeohhem wants to merge 1 commit into
mainfrom
tom/release-markers

Conversation

@teeohhem

Copy link
Copy Markdown
Contributor

Summary

Correlating a latency or error spike with a release meant leaving HyperDX. Dashboard tiles can now overlay the moment each version of a service first appeared, derived from the version expression on the tile's source, so no CI integration is required. Markers reuse the annotation overlay built for alert firing/recovery lines, which was written source-agnostic for exactly this. They are off by default and toggled from the dashboard overflow menu, with the state carried in the URL as releaseMarkers so a shared link keeps it.

They are called release markers rather than deployment markers on purpose. What we detect is a new version value appearing in telemetry, which is not the same as a deployment: a deploy that doesn't change the version string produces no marker at all, and a service idle past the lookback draws one when it scales back up. The narrower name keeps the failure mode legible instead of making the feature look broken.

Three rules keep the markers trustworthy rather than noisy. A marker only helps correlation if the reader can attribute it to something visible, so what a tile shows depends on what it charts:

Tile Markers
Filtered to one service That service's releases
Grouped by service Every charted service's releases, each tinted to match its own line
Aggregate line over many services None, since a marker naming a service with no visible line invites false attribution

The query runs against the tile's own source with the tile's own filters, which is what makes scoping possible. The version already running when the window opens is recognised and dropped rather than drawn as a release that never happened. Dense clusters collapse to "N releases", sized from the estimated label width; a cluster spanning several services goes neutral rather than wearing one of their colours and claiming the others' releases as its own.

Markers are available on log and trace sources. Metric sources resolve their table per metric type, so there is no single table to re-aggregate and no way to make a tile's filters meaningful against it; following the source correlation fields to a companion log source is the natural follow-up.

Stack: based on #2893 (the source field this reads). Review that one first. A follow-up PR adds a hover tooltip naming the service behind each marker.

How to test on Vercel preview

Preview routes: /dashboards

Steps:

  1. Open /dashboards and create a new dashboard.
  2. Add a tile, choose the Logs source, and save it.
  3. Open the dashboard overflow menu (data-testid="dashboard-menu-button").
  4. Click "Show release markers" (data-testid="toggle-release-annotations-menu-item").
  5. Verify the URL gains releaseMarkers=true and the menu item now reads "Hide release markers".
  6. Click "Hide release markers" and confirm releaseMarkers is removed from the URL.

Note: whether marker lines render depends on the preview's demo data carrying a version attribute. The steps above assert the toggle and URL state, which hold regardless.

References


Compound Engineering
Claude Code

@changeset-bot

changeset-bot Bot commented Aug 12, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 468ed6e

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 Minor
@hyperdx/api Minor
@hyperdx/otel-collector Minor

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 Aug 12, 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 12, 2026 9:21pm
hyperdx-storybook Ready Ready Preview Aug 12, 2026 9:21pm

Request Review

@greptile-apps

greptile-apps Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Dashboard time-series tiles can now display URL-controlled release markers derived from version values in their log or trace source.

  • Adds source-scoped release aggregation and conversion into chart annotations.
  • Resolves marker colors against chart series and collapses dense labels.
  • Adds dashboard controls, URL persistence, unit coverage, and Playwright coverage.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/app/src/hooks/useReleaseAnnotations.tsx Builds source-scoped release queries and converts first-seen version rows into annotations.
packages/app/src/components/charts/chartAnnotations.tsx Adds annotation merging, series resolution, timestamp validation, and dense-label collapsing.
packages/app/src/HDXMultiSeriesTimeChart.tsx Resolves annotation colors against chart series and supplies plot geometry for label layout.
packages/app/src/DBDashboardPage.tsx Adds the URL-backed release-marker toggle and supplies merged annotations to time charts.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Toggle[releaseMarkers URL toggle] --> Tile[Dashboard tile]
  Tile --> Hook[useReleaseAnnotations]
  Hook --> Query[Source-scoped release query]
  Query --> Convert[Convert rows to annotations]
  Convert --> Merge[Merge with alert annotations]
  Merge --> Resolve[Resolve visible series and colors]
  Resolve --> Chart[Render time-chart markers]
Loading

Reviews (2): Last reviewed commit: "feat(dashboards): release markers on til..." | Re-trigger Greptile

Comment thread packages/app/src/HDXMultiSeriesTimeChart.tsx
Comment on lines +670 to +683
const releaseAnnotations = useReleaseAnnotations(
isFullscreen ? fullscreenDateRange : dateRange,
showReleaseAnnotations,
{
source,
where: isBuilderSavedChartConfig(chart.config)
? chart.config.where
: undefined,
whereLanguage: isBuilderSavedChartConfig(chart.config)
? chart.config.whereLanguage
: undefined,
filters,
},
);

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.

P2 Non-time tiles issue release queries

When release markers are enabled, every log- or trace-backed tile runs the release aggregation even though only DBTimeChart consumes the annotations, adding discarded ClickHouse queries for table, number, pie, and other non-time-series tiles.

Knowledge Base Used: App Components and Charts

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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

@github-actions

Copy link
Copy Markdown
Contributor

Deep Review

No critical issues found. No P0/P1 defects. The change is well-tested (strong unit coverage on the pure helpers plus a full-stack e2e that asserts markers actually render, are attribution-scoped, and round-trip through the URL) and reuses the existing alert-annotation overlay cleanly. The highest-risk area — parsing ClickHouse timestamp strings for marker placement — was independently verified: the ClickHouse client sets date_time_output_format: 'iso' on every query and release markers reuse the same new Date(rawTs).getTime() parsing the chart's own data points use, so markers stay aligned with the series regardless of format. The findings below are recommended (P2) and nits (P3).

🟡 P2 -- recommended

  • packages/app/src/hooks/useReleaseAnnotations.tsx:314 -- The "No releases found" notification effect (fire condition, isFetching/data == null suppression, once-only hasWarnedRef dedupe, and reset-on-disable) has zero test coverage even though the mock is wired up.
    • Fix: Add tests asserting notifications.show fires once with the empty-state id only when enabled with data present and no annotations, stays silent while fetching or data is null, and re-warns after enabled flips off then on.

Agent-native follow-up

  • The version source (serviceVersionExpression) that drives these markers is user-configurable in the source form but is absent from the MCP mcpSaveSourceSchema and describeSource output, so an agent can neither read nor set it. This field predates this PR and lives outside this diff, so it is a follow-up rather than a finding introduced here.
🔵 P3 nitpicks (7)
  • packages/app/src/hooks/useReleaseAnnotations.tsx:286 -- JSON.parse(scopeKey) returns any, so the ReleaseScope argument to buildReleaseChartConfig is no longer type-checked and could drift silently.
    • Fix: Annotate the parse as JSON.parse(scopeKey) as ReleaseScope to restore the type barrier at the call boundary.
  • packages/app/src/hooks/useReleaseAnnotations.tsx:125 -- The as Filter cast masks that condition: scope.where is string | undefined and stores the un-trimmed value after gating on the trimmed one.
    • Fix: Extract a narrowed const where = scope.where?.trim() local, use it as the condition, and drop the cast.
    • kieran-typescript, project-standards
  • packages/app/src/hooks/useReleaseAnnotations.tsx:133 -- The per-kind source-field mapping (implicitColumnExpression, bodyExpression, serviceNameExpression, …) is re-implemented with ad-hoc 'field' in source narrowing instead of the shared isLogSource/isTraceSource guards used elsewhere, so the two representations can drift.
    • Fix: Reuse the existing isLogSource/isTraceSource type guards to keep a single source of truth for which kinds carry which columns.
  • packages/app/src/DBDashboardPage.tsx:1804 -- The release feature mixes vocabulary across the URL param (releaseMarkers), state/prop (showReleaseAnnotations), and menu label (release markers), unlike the consistently-named alert wiring.
    • Fix: Pick one noun and use it for the query param, state variable, and label to mirror the alert-annotation naming.
  • packages/app/src/hooks/useReleaseAnnotations.tsx:337 -- The new file is 337 lines, over the documented 300-line ceiling in CLAUDE.md / agent_docs/code_style.md.
    • Fix: Extract the pure config/mapping helpers (buildReleaseChartConfig, releaseRowsToAnnotations, canDeriveReleases, resolveVersionExpression, constants) into a sibling module and keep the hook thin.
  • packages/app/src/hooks/useReleaseAnnotations.tsx:297 -- With markers on, a dashboard of N tiles carrying distinct where/filters issues up to N concurrent min(ts) GROUP BY version aggregates; cross-tile dedup only fires for identical scope.
    • Fix: Acceptable given the explicit toggle and 60s cache bucketing, but on very large tables consider a single dashboard-level releases query filtered client-side, or a concurrency cap.
  • packages/app/src/ChartUtils.tsx:1597 -- getSeriesColorForGroup matches a group value against ChartKeyJoiner-split key components, but the collision cases (a group value containing the · joiner, or one equal to a value-column-name prefix) are untested — these are exactly the false-attribution paths the feature exists to prevent.
    • Fix: Add unit cases for a group value containing the joiner and one coinciding with a value-column name to lock the attribution boundary.

Reviewers (8): security, testing, maintainability, kieran-typescript, performance, project-standards, agent-native, learnings-researcher.

Testing gaps:

  • Empty-releases notification effect (fire/suppress/dedupe/reset) is entirely uncovered.
  • releaseRowsToAnnotations is only tested with ISO-8601 Z timestamps; a non-ISO/epoch firstSeen fixture would pin the parse behavior (low risk — the driver is configured for iso output, so Z strings are representative).
  • The documented idle-service residual artifact (first post-idle row reading as a release at the left edge) has no characterization test.
  • No test asserts cross-tile query dedup for identical source+scope.
  • Correctness and adversarial reviewers did not return within the review window; their scope (logic/edge-case/failure-mode analysis) was partially covered by the testing, maintainability, and typescript reviewers plus independent verification of the timestamp-parsing path, but was not fully re-run.

Base automatically changed from tom/source-version-expression to main August 12, 2026 21:17
Correlating a latency or error spike with a release meant leaving the
product. Dashboard tiles can now overlay the moment each version of a
service first appeared, derived from the version expression on the
tile's source, so no CI integration is needed.

Markers reuse the annotation overlay built for alert firing/recovery
lines, which was written source-agnostic for exactly this. They are off
by default, toggled from the dashboard overflow menu, with the state in
the URL as `releaseMarkers` so a shared link keeps it.

Called release markers rather than deployment markers on purpose. What
is detected is a new version value appearing in telemetry, which is not
the same as a deployment: a deploy that doesn't change the version
string produces no marker at all, and a service idle past the lookback
draws one when it scales back up. The narrower name keeps the failure
mode legible.

Three rules keep them trustworthy rather than noisy. A marker only
helps if the reader can attribute it, so what a tile shows depends on
what it charts:

- Filtered to one service: that service's releases.
- Grouped by service: every charted service's releases, each tinted to
  match its own line.
- An aggregate line over many services: none, since a marker naming a
  service with no visible line invites false attribution.

The query runs against the tile's own source with the tile's own
filters, which is what makes scoping possible. The version already
running when the window opens is recognised and dropped rather than
drawn as a release that never happened. Dense clusters collapse to
"N releases", sized from the estimated label width; a cluster spanning
several services goes neutral rather than wearing one of their colours
and claiming the others' releases as its own.
@teeohhem
teeohhem force-pushed the tom/release-markers branch from 4e044ef to 468ed6e Compare August 12, 2026 21:17
@github-actions github-actions Bot added the review/tier-3 Standard — full human review required label Aug 12, 2026
@github-actions

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: 713 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: 5
  • Production lines changed: 713 (+ 1131 in test files, excluded from tier calculation)
  • Branch: tom/release-markers
  • Author: teeohhem

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

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found. The feature is off by default, gated behind an explicit toggle, and the ClickHouse query it builds reuses the codebase's established trusted-source-config interpolation model (verified against renderChartConfig.ts and existing serviceNameExpression call sites), so it introduces no new injection surface. The items below are recommendations and nits.

🟡 P2 — recommended

  • packages/app/src/hooks/useReleaseAnnotations.tsx — The empty-state toast uses one shared notification id and fires from any tile whose source cannot resolve a version, so a dashboard mixing sources can surface a global "No releases found" warning even while other tiles are rendering release markers.

    • Fix: Suppress the empty-state notification when at least one tile on the dashboard resolved release markers, or replace the id-shared global toast with per-tile inline state.
  • packages/app/src/components/charts/chartAnnotations.tsx:277MAX_ANNOTATION_MARKERS is now sliced after positionAnnotations and collapseLabels run over the full uncapped input, so the O(n·k) collapse executes before the safety cap that was written to guard against pathological marker counts freezing the tab.

    • Fix: Apply the MAX_ANNOTATION_MARKERS slice to the time-sorted input before positioning and collapsing.
🔵 P3 nitpicks (3)
  • packages/app/src/ChartUtils.tsxgetSeriesColorForGroup matches a group by splitting the series key on ChartKeyJoiner (' · '), so a service/group value that itself contains that separator substring fails to match and loses its series tint.
    • Fix: Match the group against the structured group values rather than re-splitting the joined key string.
  • packages/app/src/HDXMultiSeriesTimeChart.tsx:1447 — The annotationElements memo depends on containerWidth, recomputing every annotation element on each resize tick.
    • Fix: Debounce containerWidth for the annotation memo, or accept the churn and document it.
  • packages/app/src/hooks/useReleaseAnnotations.tsx — The versionExpression override is documented in-code as a testing seam yet lives on the public hook options object.
    • Fix: Move the override behind a test-only path or clearly mark it non-public so callers don't rely on it.

Reviewers (11 dispatched): correctness, security, adversarial, testing, maintainability, kieran-typescript, julik-frontend-races, performance, project-standards, agent-native, learnings-researcher.

Testing gaps: New logic carries strong unit coverage (chartAnnotations.test.tsx, ChartUtils.test.ts, useReleaseAnnotations.test.tsx) plus an e2e toggle/URL spec; no dedicated assertion exists for the mixed-source empty-notification behavior flagged above.

@github-actions

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 282 passed • 1 skipped • 1132s

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

Tests ran across 4 shards in parallel.

View full report →

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.

1 participant