Skip to content

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

Merged
teeohhem merged 3 commits into
mainfrom
tom/release-markers
Aug 14, 2026
Merged

feat(dashboards): release markers on tile charts#2894
teeohhem merged 3 commits 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: e30ecc3

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 14, 2026 1:53pm
hyperdx-storybook Ready Ready Preview Aug 14, 2026 1:53pm

Request Review

@greptile-apps

greptile-apps Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Dashboard time-series tiles can now query and display URL-controlled release markers derived from service-version telemetry.

  • Scopes release queries to each tile’s source, series conditions, and dashboard filters.
  • Colors attributable markers by chart series and suppresses ambiguous aggregate markers.
  • Collapses dense labels while preserving individual marker lines.
  • Adds unit and end-to-end coverage for querying, rendering, scoping, and toggling markers.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/app/src/hooks/useReleaseAnnotations.tsx Builds scoped release queries, maps first-seen versions to annotations, and keeps unsupported or disabled queries idle.
packages/app/src/DBDashboardPage.tsx Adds URL-backed release-marker controls and restricts annotation queries to display types rendered by DBTimeChart.
packages/app/src/HDXMultiSeriesTimeChart.tsx Resolves annotation ownership and colors before rendering markers with width-aware label collapsing.
packages/app/src/components/charts/chartAnnotations.tsx Extends the shared annotation model with merging, series attribution, validation, and dense-label layout.
packages/app/tests/e2e/features/release-markers.spec.ts Covers release-marker visibility, tile filter scoping, ambiguous-marker suppression, and URL toggle persistence.

Reviews (7): Last reviewed commit: "fix(dashboards): scope release markers b..." | Re-trigger Greptile

Comment thread packages/app/src/HDXMultiSeriesTimeChart.tsx
Comment thread packages/app/src/DBDashboardPage.tsx
@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
@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

github-actions Bot commented Aug 12, 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: 806 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: 806 (+ 1318 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

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Deep Review

No critical issues found. No P0/P1: the diff introduces no data loss, crash on the happy path, auth/authz change, or injection. Security review confirmed the raw-SQL interpolation of serviceVersionExpression/serviceNameExpression is the established, app-wide chart-config pattern (source-level operator config, not request-time user input) and is not widened here. The two P2s below are correctness/UX defects gated behind specific configurations; the rest are nits.

🟡 P2 -- recommended

  • packages/app/src/hooks/useReleaseAnnotations.tsx:237 -- orderBy: 'firstSeen ASC' with limit 500 over the lookback-widened window keeps the oldest rows, which releaseRowsToAnnotations then discards as pre-window incumbents, so a broad tile with >500 distinct version·service pairs never fetches its recent releases and can even show a false "No releases found" toast when the whole budget is spent on incumbents.
    • Fix: Order the release query by firstSeen DESC, or push the firstSeen >= windowStart cut into the query, so the row cap is spent on in-window releases rather than discarded incumbents.
    • correctness, adversarial
  • packages/app/src/ChartUtils.tsx:216 -- getSeriesColorForGroup matches a service group against every component of a composite series key via split(ChartKeyJoiner).includes(group), so on a chart grouped by more than one dimension (e.g. [service, region]) a release tints to and is kept for an arbitrary unrelated line — the exact misattribution the feature is meant to prevent.
    • Fix: Match the group value only against the key segment corresponding to the service dimension, or fall back to the neutral color when more than one line matches the same group value.
    • correctness, adversarial
  • packages/app/src/hooks/useReleaseAnnotations.tsx:393 -- The new empty-state notification effect (four branches) and the group = row.service mapping that drives per-service tinting are new user-visible behavior with no unit assertions, and the non-timeseries tileCanDrawAnnotations query-gating is covered only by the toggle-level e2e.
    • Fix: Add unit tests asserting notifications.show fires exactly once on the empty path (and not while fetching or disabled), that group is set to the service (and undefined when absent), and that a non-timeseries tile passes enabled: false.
    • testing
🔵 P3 nitpicks (7)
  • packages/app/src/components/charts/chartAnnotations.tsx:277 -- The MAX_ANNOTATION_MARKERS slice runs across all kinds after per-kind collapse, so a flapping alert producing >1000 markers can starve every release marker out of the render even though its query was paid for.
    • Fix: Cap per kind, or interleave kinds before slicing so no single kind exhausts the budget.
  • packages/app/src/hooks/useReleaseAnnotations.tsx:405 -- The "No releases found" toast also fires when rows were returned but all were incumbents/capped, and a failed query is swallowed silently (no onError), so both misleading-empty and error states read as "feature broken".
    • Fix: Distinguish zero-rows from all-incumbents before warning, and surface the query-error state distinctly.
  • packages/app/src/hooks/useReleaseAnnotations.tsx:345 -- The scope is built via JSON.stringify then JSON.parse back into buildReleaseChartConfig, yielding an untyped any and re-stringifying every render.
    • Fix: Memoize a typed ReleaseScope object keyed on the string and pass the object directly.
    • kieran-typescript, maintainability, performance
  • packages/app/src/hooks/useReleaseAnnotations.tsx:197 -- The as Filter cast hides that condition: scope.where is string | undefined, since optional-chain truthiness does not narrow it.
    • Fix: Bind const whereCondition = scope.where?.trim() and build the Filter from that so condition is a genuine string with no cast.
  • packages/app/src/hooks/useReleaseAnnotations.tsx:352 -- Scope/config construction runs per-tile on every render even when release markers are off (the default), and getSeriesColorForGroup is O(annotations × lineData) with a split allocation per pair.
    • Fix: Gate scope/config construction on enabled, and precompute a group → color map once from lineData.
    • performance
  • packages/app/src/components/charts/chartAnnotations.tsx:47 -- LABEL_CHAR_WIDTH_PX (documented "at fontSize 10") must stay in sync with the hardcoded fontSize={10} ~240 lines away, with no cross-reference; changing one silently breaks label collision estimation.
    • Fix: Derive both from a single ANNOTATION_LABEL_FONT_SIZE_PX constant.
  • packages/app/src/hooks/useReleaseAnnotations.tsx:1 -- The new hook file is 415 lines, over the repo's documented 300-line max (chartAnnotations.tsx is marginally over at 302).
    • Fix: Split the pure query/mapping helpers (buildReleaseChartConfig, aggConditionScopeFilter, releaseRowsToAnnotations) into a sibling module.
    • project-standards

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

Testing gaps:

  • The ORDER BY firstSeen ASC + LIMIT 500 + incumbent-drop interaction (>500 version·service rows) is untested — the core row-budget behavior.
  • The empty-state notification effect (4 branches) has zero assertions despite the mock being wired.
  • group = row.service derivation and the per-service tint chain are unverified end-to-end.
  • Multi-dimension getSeriesColorForGroup collision / first-match ordering is unpinned.
  • No test asserts identical-scope tiles collapse to a single network request (the dedup claim).

@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 294 passed • 1 skipped • 1139s

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

Tests ran across 4 shards in parallel.

View full report →

Comment thread packages/app/src/DBDashboardPage.tsx Outdated
Comment on lines +683 to +688
where: isBuilderSavedChartConfig(chart.config)
? chart.config.where
: undefined,
whereLanguage: isBuilderSavedChartConfig(chart.config)
? chart.config.whereLanguage
: undefined,

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.

Timeseries charts don't have where or whereLanguage, they use per-series aggCondition instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oof good catch. Addressed in a follow-up commit + tests. Adding E2E as well

// Drawable width, so markers too close together share one label. Zero on
// the first paint (before ResponsiveContainer measures), which the
// renderer treats as "label everything".
plotWidth: Math.max(0, containerWidth - Y_AXIS_WIDTH),

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.

This is nice

* keeps `min()` over a `DateTime64` out of the aggregate-function machinery.
*/
export function buildReleaseChartConfig(
source: TSource,

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.

nit: if you type this TTraceSource | TLogSource, that cleans this up below:

implicitColumnExpression:
      'implicitColumnExpression' in source
        ? source.implicitColumnExpression
        : undefined,
    useTextIndexForImplicitColumn:
      'useTextIndexForImplicitColumn' in source
        ? source.useTextIndexForImplicitColumn
        : undefined,

@pulpdrew pulpdrew 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.

LGTM

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.
Only DBTimeChart consumes the `annotations` prop, so a tile rendering as a
table, number, pie, bar, heatmap or search discarded whatever the annotation
hooks fetched. With release markers toggled on, that meant a
`min(timestamp) ... GROUP BY version, service` aggregation against ClickHouse
for every log- or trace-backed tile on the dashboard, non-time-series ones
included — and on a dashboard of only such tiles, a spurious "No releases
found" notification about markers that were never going to be drawn.

Gate both annotation hooks on the tile's display type via the existing shared
`isTimeSeriesDisplayType` predicate. The alert-history fetch had the same
latent waste (cheaper, but a Number tile with an alert still queried history
nothing rendered); gating both off one boolean keeps the two call sites
consistent.
A builder time chart doesn't use the statement-level `where` — the editor
clears it and each series carries its own `aggCondition` instead. Reading only
`where` meant the release query was effectively unscoped for every time chart,
which is the only display type that draws markers: exactly the case the
scoping exists for. Confirmed against the shipped dashboard templates, where
every time-series tile has `where: ''` and its filter in `aggCondition`.

The visible symptom isn't just over-fetching. A tile filtered to one service
would pull releases for every service in the source, so the markers span
several groups, none of which matches a series on an ungrouped chart —
`resolveAnnotationSeries` then drops all of them. The user filters to one
service and sees no markers at all.

Fold the series conditions into one predicate the same way renderChartConfig
does: OR them, and only when every series is filtered (leave one unfiltered and
the tile scans every row anyway). Identical conditions collapse rather than
OR with themselves, and series in different condition languages can't be
combined into one predicate, so those tiles stay unscoped rather than scoped
wrongly.

Only the folded condition keys the config memo, so a cosmetic series edit
(color, number format) doesn't refire the query.

The existing E2E spec missed this because it scopes via a dashboard-global
filter, which travels through `filters` and was already applied.
@teeohhem
teeohhem force-pushed the tom/release-markers branch from 319c3c3 to e30ecc3 Compare August 14, 2026 13:49
@teeohhem
teeohhem merged commit 08b8783 into main Aug 14, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge review/tier-3 Standard — full human review required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants