feat(app): show GPU metrics in log side panel infrastructure section - #2897
Open
MikeShi42 wants to merge 11 commits into
Open
feat(app): show GPU metrics in log side panel infrastructure section#2897MikeShi42 wants to merge 11 commits into
MikeShi42 wants to merge 11 commits into
Conversation
Add GPU utilization and GPU memory utilization charts to the Infrastructure tab of the log/span side panel, using OTel hardware semantic conventions (hw.gpu.*). - Add useGpuMetricsAvailability hook for cheap metric existence check (queries MetricName values from gauge table, cached 5 min) - Add GpuInfraSection component with per-GPU series via hw.id groupBy - Add getGpuCorrelationWhere to build resource correlation filter (prefers k8s.node.name, falls back to host.name) - Section is fully hidden when no GPU metrics exist for the correlated resource; partial availability renders only available charts - GPU utilization chart filters to hw.gpu.task:general (or unset) to avoid mixing encoder/decoder series HDX-5102 Co-authored-by: Mike Shi <mike@hyperdx.io>
🦋 Changeset detectedLatest commit: ed9d260 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.
|
Contributor
Greptile SummaryThe PR adds availability-gated GPU utilization charts to the infrastructure side panel and directly constructs v2 metric chart configurations.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/app/src/components/DBInfraPanel.tsx | Refactors infrastructure groups, directly builds v2 metric configurations, and gates GPU rendering on current availability. |
| packages/app/src/components/infraCorrelations.ts | Adds the GPU descriptor and chart specifications with per-device and per-task grouping. |
| packages/app/src/hooks/useAvailableMetricNames.ts | Queries an exact bounded set of gauge metric names and marks retained placeholder results as loading. |
| packages/app/src/components/tests/DBInfraPanel.buildChartConfig.test.ts | Covers direct metric configuration, source wiring, grouping, and semantic-convention rename handling. |
| packages/app/src/hooks/tests/useAvailableMetricNames.test.ts | Covers metric-name results, initial loading, retained placeholder data, and empty responses. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Selected log or span] --> B[Resource attributes]
B --> C[Active infrastructure correlations]
C --> D[Pod and Node charts]
C --> E[GPU metric availability query]
E --> F{Supported metric exists?}
F -- Yes --> G[Visible GPU charts]
F -- No --> H[Hide GPU group]
D --> I[Direct v2 metric config]
G --> I
I --> J[DBTimeChart]
Reviews (10): Last reviewed commit: "fix(app): don't let GPU chart availabili..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Use specific type assertions instead of 'as any' to stay within the max-warnings threshold. Fix import sort order in test file. Co-authored-by: Mike Shi <mike@hyperdx.io>
Contributor
E2E Test Results✅ All tests passed • 306 passed • 1 skipped • 938s
Tests ran across 4 shards in parallel. |
Use the stable NOW constant from config instead of new Date() in the GPU infra section's date range calculation, matching the project's date hygiene rules. Co-authored-by: Mike Shi <mike@hyperdx.io>
Structural: - Extend InfraChartSpec with optional groupBy, where, metricType, and fallback fields so GPU registers as a descriptor rather than a separate component - Delete GpuInfraSection.tsx; InfraSubpanelGroup now handles both k8s and GPU charts via the descriptor data - Add requiresMetricAvailability flag to InfraCorrelation; gated groups only render when metric existence is confirmed - Add AvailabilityGatedGroup wrapper that checks availability before rendering Bug fixes: - Fix _exists_ syntax (unsupported) → hw.gpu.task:* per query parser - Push MetricName:hw.gpu.* prefix filter into the availability query so limit doesn't produce false negatives on metric-heavy nodes - Check both gauge and sum tables for availability - Drop host.name fallback (unreachable: tab requires k8s attributes) Acceptance criteria: - Series grouped by concat(hw.id, hw.name, hw.model) for richer labels - hw.gpu.memory.usage / hw.gpu.memory.limit fallback via ratio chart when hw.gpu.memory.utilization isn't emitted - resolveChartAvailability tested for primary, fallback, none, and partial cases Minor: - Use live new Date() (eslint-disable) matching sibling InfraSubpanelGroup - Move GPU_UTILIZATION_NUMBER_FORMAT to ChartUtils for consistency - Remove empty select comment (now annotated in the hook) HDX-5102 Co-authored-by: Mike Shi <mike@hyperdx.io>
Remove the hw.gpu.memory.usage / hw.gpu.memory.limit fallback: convertV1ChartConfigToV2 drops seriesReturnType for metrics, the renderer discards the second series, and Sum uses counter-increase semantics on a non-monotonic UpDownCounter. All three failures are in the renderer and out of scope for this PR. Changes: - Remove InfraChartFallback type and fallback field from InfraChartSpec - Simplify resolveChartAvailability to return boolean (available / not) - Remove sum-table query from useGpuMetricsAvailability (halves cost) - Simplify buildChartConfig (no mode parameter) - Fix 40px empty-div gap: return null from the correlation map entry when both metricsGroup and timeline render nothing, so no empty flex child is emitted into Stack Follow-up: HDX-5102 — support ratio charts over Sum metrics for GPU memory fallback (requires changes to convertV1ChartConfigToV2, renderChartConfig metric select handling, and Sum aggFn projection). HDX-5102 Co-authored-by: Mike Shi <mike@hyperdx.io>
Greptile findings:
- Metric-name cap could hide GPU charts. useGetKeyValues aggregates with
groupUniqArray(limit) even when disableRowLimit is set, so an open-ended
MetricName lookup can drop the name being looked for on a metric-heavy
host. Now the query asks only about the candidate metric names (derived
from the chart specs) and sizes the limit to match, so truncation is
impossible. This also replaces the unanchored MetricName:hw.gpu.* ILIKE
scan with exact equality.
- hasAny was true for any hw.gpu.* metric, so a host emitting only
hw.gpu.io could render GPU controls over an empty grid. Asking only
about chartable metrics removes the failure mode structurally; the
separate hasAny flag is gone.
Drop convertV1ChartConfigToV2 for infra charts and build the v2
BuilderChartConfig directly. The v1 layer was a lossy round trip for
metrics: 'name - Gauge' string-split and re-parsed into an enum, a
table:'metrics' discriminator that only picks a branch, a
seriesReturnType that is silently dropped, a groupBy array joined to a
string via a startsWith('k8s') rewrite, and a valueExpression the
renderer overwrites. getMetricNameSql is now called directly so the k8s
cpu.utilization -> cpu.usage rename still matches both names; a test
pins that.
Also:
- Collapse AvailabilityGatedGroup and InfraSubpanelGroup into one
InfraCorrelationGroup that owns its wrapper, so a group with nothing
to show renders no DOM. The previous fix was ineffective: a React
element is always truthy, so the empty-wrapper check never fired and
the 40px Stack gap remained.
- Gate the availability loading state to gated groups only, so Pod/Node
are not held back by a query they do not run.
- Preserve the pre-existing behavior of dropping a group whose correlate
attribute is present but empty.
- Rename useGpuMetricsAvailability to useAvailableMetricNames; nothing
in it is GPU-specific.
- Add DBInfraPanel.buildChartConfig tests asserting the produced config
rather than an intermediate decision.
HDX-5102
Co-authored-by: Mike Shi <mike@hyperdx.io>
Contributor
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: teeohhem <3245235+teeohhem@users.noreply.github.com>
Contributor
Resolved by merging |
…eral
The per-chart Lucene filter `hw.gpu.task:"general" OR NOT hw.gpu.task:*`
rendered as the identifier `hw`.`gpu`.task rather than a map lookup, because
the field carried no Map column prefix the way the correlation filter gets one
from `resourceAttributesExpression`. Against real `hw.gpu.*` data the GPU
utilization chart failed outright with UNKNOWN_IDENTIFIER.
Rather than prefix the field, drop the filter and add `hw.gpu.task` as a second
groupBy column. A GPU reports utilization per engine (general/encoder/decoder),
so filtering to general hides a node saturated on video encode, while averaging
the engines together understates a busy GPU. A missing task normalizes to
general, which is what a producer emitting a single unlabelled figure means.
This also closes a divergence: useAvailableMetricNames never applied
chart.where, so an encoder-only host passed the availability gate and then
rendered an empty chart. With no per-chart predicate, the gate and the charts
share one WHERE by construction.
InfraChartSpec.where was added earlier in this branch solely for that filter
and now has no callers, so it goes too, along with the aggCondition
composition in buildChartConfig.
Verified against live hw.gpu.* metrics collected from an Apple M3 Pro GPU:
the three engine series resolve as "gpu0 Apple M3 Pro GPU · {general,
encoder, decoder}".
MikeShi42
marked this pull request as ready for review
August 22, 2026 09:16
Contributor
🟡 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
|
Contributor
Deep Review✅ No critical issues found. The core risks flagged on earlier iterations have been verified as addressed in the current diff:
🟡 P2 — recommended
🔵 P3 nitpicks (2)
Reviewers (11): correctness, testing, maintainability, project-standards, kieran-typescript, performance, julik-frontend-races, adversarial, previous-comments, agent-native, learnings-researcher. Testing gaps: No coverage for |
…e gate getActiveInfraCorrelations admitted any non-null detect attribute, so a row carrying `k8s.node.name: ""` surfaced an Infrastructure tab whose groups then all rendered as null -- the renderer's own guard is truthiness, and the two disagreed. That divergence was documented in a comment rather than fixed. Gate on truthiness instead, which is the exact test the renderer uses, so the single-source-of-truth contract between the tab gate and the panel actually holds. Rows whose only Kubernetes attribute is empty now correctly get no tab. This also stops the metric-availability probe firing for an unusable group: the group is no longer active, so InfraCorrelationGroup never mounts. Because a descriptor may detect on one attribute and correlate on another, the correlate side is additionally checked before enabling the probe. Deliberately not done: blanking `where` when the value is empty. useAvailableMetricNames reads an empty correlation filter as "no filter" and would probe every host, reporting metrics as available for a host that has none -- worse than the wasted query.
Comment on lines
+193
to
+207
| const visibleCharts = useMemo(() => { | ||
| if (!isGated) { | ||
| return charts; | ||
| } | ||
| return charts.filter(chart => | ||
| availableMetrics.has(metricNameFor(fieldPrefix, chart)), | ||
| ); | ||
| }, [charts, fieldPrefix, isGated, availableMetrics]); | ||
|
|
||
| const showCharts = | ||
| metricSource != null && | ||
| visibleCharts.length > 0 && | ||
| // Only the gated groups wait on the existence query; ungated groups must | ||
| // not be held back by it. | ||
| (!isGated || !isLoadingAvailability); |
Contributor
There was a problem hiding this comment.
When the side panel switches between rows on nodes with different GPU metrics, keepPreviousData leaves the prior node's availability set active while the new query runs, and this code treats it as current because loading is false. The newly selected node therefore shows unsupported GPU charts or hides supported charts until its availability request completes.
useGetKeyValues sets `placeholderData: keepPreviousData`, so when the side panel moves to a row on a different host the previous host's availability answer stays readable while the new query runs. `isLoading` reads false throughout -- with placeholder data the query is `success`, not `pending` -- so InfraCorrelationGroup treated the stale set as current and rendered the previous host's chart selection: GPU charts on a host with no GPU, or a hidden section on a host that has one, until the new probe landed. Fold `isPlaceholderData` into the `isLoading` the hook reports, so the flag means "the answer for these inputs isn't known yet" rather than "a request is in flight". Every caller gating on availability then gets the right behaviour without repeating the check, and the existing gate in DBInfraPanel is already written against it. The panel is not remounted per row (no key on rowId anywhere from DBSqlRowTableWithSidebar down), so the query observer survives the switch and keepPreviousData does apply -- this is reachable, not theoretical.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds GPU utilization and GPU memory utilization charts to the Infrastructure tab of the log/span side panel, using the OpenTelemetry hardware semantic conventions (
hw.gpu.*).Design
GPU registers as a descriptor in
infraCorrelations.tsalongside Pod and Node — no separate component.InfraChartSpecsupports:groupBy— per-chart SQL expressions for multi-GPU series labelingwhere— per-chart Lucene filter (ANDed with the correlation WHERE)metricType— a realMetricsDataType, defaulting to GaugeInfraCorrelationgainsrequiresMetricAvailability. A singleInfraCorrelationGroupcomponent renders every group (Pod / Node / GPU) and owns its own wrapper, so a group with nothing to show renders no DOM at all rather than an empty flex child.Charts
hw.gpu.utilizationhw.gpu.task:"general" OR NOT hw.gpu.task:*hw.gpu.memory.utilizationCorrelated at node level via
k8s.node.name. Series are labeled byconcat(hw.id, hw.name, hw.model)so multi-GPU nodes show identifiable per-device lines.Metric existence check
useAvailableMetricNamesasks which of a given candidate list exist for the correlated resource. It queries only the candidate names rather than enumerating every distinctMetricNameon the host — the metadata layer aggregates withgroupUniqArray(limit)even whendisableRowLimitis set, so an open-ended lookup can silently drop the name being looked for on a metric-heavy host and hide a chart that does have data. Bounding the universe to the candidates and sizing the limit to match makes truncation impossible, and replaces an unanchoredILIKEscan with exact equality. Cached 5 min.Direct v2 chart configs
Infra charts no longer go through
convertV1ChartConfigToV2. For metrics that layer was a lossy round trip: a'name - Gauge'string split and re-parsed into an enum, atable: 'metrics'discriminator that only selects a branch, aseriesReturnTypethat is accepted and silently dropped, agroupByarray joined into a string via astartsWith('k8s')rewrite, and avalueExpressionthe renderer overwrites.buildChartConfignow emitsBuilderChartConfigWithDateRangedirectly and callsgetMetricNameSqlitself, so the k8scpu.utilization→cpu.usagerename still matches both names — pinned by a test.Graceful degradation
Out of scope (follow-up)
The
hw.gpu.memory.usage / hw.gpu.memory.limitfallback is deferred. It needs ratio-over-Sum support, which is broken in three independent places (seriesReturnTypedropped for metrics inconvertV1ChartConfigToV2; the second select silently discarded inrenderChartConfig; counter-increase semantics applied to non-monotonic UpDownCounters). Tracked on HDX-5102.Screenshots or video
N/A — GPU metrics require an OTel-semconv-compliant GPU collector; the preview demo has no GPU data, so the section stays hidden by design.
How to test on Vercel preview
N/A — no user-visible change without
hw.gpu.*metrics.References
Linear Issue: HDX-5102