feat: Auto-detect trace duration and render in adaptive time units (HDX-3909)#2046
feat: Auto-detect trace duration and render in adaptive time units (HDX-3909)#2046
Conversation
…DX-3909) - Add 'duration' output type to NumberFormatSchema for adaptive time unit rendering (e.g., '120.41s', '45ms', '3µs') instead of clock format 'hh:mm:ss' - Add formatDurationMs() utility that adaptively formats milliseconds into the most appropriate unit (µs, ms, s, min, h) - Add getTraceDurationNumberFormat() to detect when chart select expressions reference a trace source's durationExpression - Add useResolvedNumberFormat() hook that auto-defaults to duration format when a chart uses a trace source with duration expressions and no explicit numberFormat is set by the user - Update chart components (DBTimeChart, DBNumberChart, DBListBarChart, DBPieChart, DBTableChart) to use resolved number format - Update NumberFormat UI form to include Duration option with input unit selector (seconds/ms/µs/ns) - Update DBHeatmapChart tick formatter to support the new duration format - Update DBSearchHeatmapChart to use duration format instead of MS_NUMBER_FORMAT - Add comprehensive tests for formatDurationMs and getTraceDurationNumberFormat Co-authored-by: Mike Shi <mike@hyperdx.io>
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
E2E Test Results✅ All tests passed • 131 passed • 3 skipped • 1058s
Tests ran across 4 shards in parallel. |
The ChartDisplaySettingsDrawer now accepts a defaultNumberFormat prop that represents the auto-detected format (e.g., duration for trace sources). When no explicit numberFormat is set, the drawer shows the auto-detected format instead of the generic 'Number' default. - Add defaultNumberFormat prop to ChartDisplaySettingsDrawer - Compute auto-detected format in DBEditTimeChartForm and pass it down - Reset form when defaults change via useEffect - Update test mocks for getTraceDurationNumberFormat Co-authored-by: Mike Shi <mike@hyperdx.io>
…unctions Skip auto-detection for aggregate functions like count, count_distinct, and sum whose output is not in the same unit as the input duration. Only apply duration formatting for functions that preserve the input unit: avg, min, max, quantile, any, last_value, heatmap, and their combinator variants (e.g. avgIf, quantileIfState). Added comprehensive tests for aggFn filtering. Co-authored-by: Mike Shi <mike@hyperdx.io>
Sum of durations still produces a duration value (e.g. total time spent), so it should inherit the duration format. Co-authored-by: Mike Shi <mike@hyperdx.io>
PR ReviewOverall this is a well-scoped, well-tested feature with good coverage of edge cases. A few items worth addressing:
✅ Test coverage is thorough — |
packages/app/src/source.ts
Outdated
| if (expr.includes(durationExpr)) { | ||
| const precision = source.durationPrecision ?? 9; | ||
| return { | ||
| output: 'duration', | ||
| factor: Math.pow(10, -precision), | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
This matches Duration/1e6 and results in the wrong factor being applied.
it('detects duration ms expression reference without parentheses', () => {
const result = getTraceDurationNumberFormat(TRACE_SOURCE, [
{ valueExpression: 'Duration/1e6' },
]);
expect(result).toEqual({
output: 'duration',
factor: 0.001,
});
});
There are also other cases, like when the expression has spaces in it Duration / 1e6, (Duration) / 1e6 that don't work as expected - real parsing would be very nice here.
For this fallback case, maybe the condition should just be exact match to avoid over-matching?
| if (expr.includes(durationExpr)) { | |
| const precision = source.durationPrecision ?? 9; | |
| return { | |
| output: 'duration', | |
| factor: Math.pow(10, -precision), | |
| }; | |
| } | |
| } | |
| if (expr === durationExpr) { | |
| const precision = source.durationPrecision ?? 9; | |
| return { | |
| output: 'duration', | |
| factor: Math.pow(10, -precision), | |
| }; | |
| } | |
| } |
| 'percent', | ||
| 'byte', // legacy, treated as data/bytes_iec | ||
| 'time', | ||
| 'duration', |
There was a problem hiding this comment.
Could we update the OpenAPI Specs as well?
- https://github.com/hyperdxio/hyperdx/blob/main/packages/api/src/routers/external-api/v2/dashboards.ts#L188
- Run yarn docgen in the api directory
github-actions
bot
added
the
review/tier-4
🟡 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
|
github-actions
bot
added
review/tier-3
The raw duration fallback (e.g. 'Duration') now requires an exact match instead of substring includes, preventing 'Duration/1e6' from incorrectly matching with the raw nanosecond factor. Division expressions like 'Duration/1e6' are now matched with a regex that handles whitespace and parentheses variants: (Duration)/1e6, Duration/1e6, Duration / 1e6, (Duration) / 1e6 Wrapped expressions like 'avg(Duration)' or 'round(Duration / 1e6, 2)' correctly do not match any pattern. Co-authored-by: Mike Shi <mike@hyperdx.io>
Remove regex-based division pattern matching. The duration expression can be arbitrary SQL, so substring or regex matching is fragile and error-prone. Now only applies duration formatting when the select valueExpression exactly equals the source's durationExpression. Co-authored-by: Mike Shi <mike@hyperdx.io>
|
@claude can you resolve merge conflicts with main? |
|
Claude encountered an error —— View job Resolving merge conflicts with main
|
DBEditTimeChartForm.tsx was refactored into a directory on main (PR #2026). Applied our changes (getTraceDurationNumberFormat import, autoDetectedNumberFormat computation, defaultNumberFormat prop on ChartDisplaySettingsDrawer) to the new EditTimeChartForm.tsx file. Deleted the old monolithic file. Co-authored-by: Mike Shi <mike@hyperdx.io>
3 participants
Summary
When a chart uses a trace source with a Duration Expression, the chart now automatically defaults to adaptive time unit formatting (e.g.,
120.41s,45ms,3µs) instead of requiring users to manually select a format. Users can still override the format through the existing display settings.Key changes:
durationoutput type inNumberFormatSchema— renders values adaptively asµs,ms,s,min, orhbased on magnitude, instead of the clock-stylehh:mm:ssformatgetTraceDurationNumberFormat()checks if any chart selectvalueExpressionexactly equals the trace source'sdurationExpression. Only applies for unit-preserving aggregate functions (avg,min,max,sum,quantile,any,last_value, etc.) — skipscountandcount_distinctuseResolvedNumberFormat()hook — resolves the effectivenumberFormatfor a chart: returns the user's explicit format if set, otherwise auto-detects duration format for trace sourcesDBHeatmapCharttick formatter to useformatDurationMsfor duration-formatted valuesComponents updated:
DBTimeChart,DBNumberChart,DBListBarChart,DBPieChart,DBTableChart,DBHeatmapChart,DBSearchHeatmapChart,DBEditTimeChartForm,ChartDisplaySettingsDrawerHow to test locally or on Vercel
120.41sor45msinstead of raw numberscountorcount_distinct— verify duration formatting is NOT appliedReferences
Linear Issue: HDX-3909