feat(dashboards): release markers on tile charts - #2894
Conversation
🦋 Changeset detectedLatest commit: e30ecc3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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.
|
Greptile SummaryDashboard time-series tiles can now query and display URL-controlled release markers derived from service-version telemetry.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| 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
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 🟡 P2 -- recommended
Agent-native follow-up
🔵 P3 nitpicks (7)
Reviewers (8): security, testing, maintainability, kieran-typescript, performance, project-standards, agent-native, learnings-researcher. Testing gaps:
|
4e044ef to
468ed6e
Compare
🟡 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
|
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 🟡 P2 -- recommended
🔵 P3 nitpicks (7)
Reviewers (11): correctness, adversarial, security, kieran-typescript, julik-frontend-races, performance, testing, maintainability, project-standards, agent-native, learnings-researcher. Testing gaps:
|
E2E Test Results✅ All tests passed • 294 passed • 1 skipped • 1139s
Tests ran across 4 shards in parallel. |
| where: isBuilderSavedChartConfig(chart.config) | ||
| ? chart.config.where | ||
| : undefined, | ||
| whereLanguage: isBuilderSavedChartConfig(chart.config) | ||
| ? chart.config.whereLanguage | ||
| : undefined, |
There was a problem hiding this comment.
Timeseries charts don't have where or whereLanguage, they use per-series aggCondition instead.
There was a problem hiding this comment.
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), |
| * keeps `min()` over a `DateTime64` out of the aggregate-function machinery. | ||
| */ | ||
| export function buildReleaseChartConfig( | ||
| source: TSource, |
There was a problem hiding this comment.
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,
41caac4 to
430cc33
Compare
430cc33 to
319c3c3
Compare
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.
319c3c3 to
e30ecc3
Compare
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
releaseMarkersso 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:
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:
data-testid="dashboard-menu-button").data-testid="toggle-release-annotations-menu-item").releaseMarkers=trueand the menu item now reads "Hide release markers".releaseMarkersis 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