Skip to content

feat: Show exponential histogram metrics in the metric name drop-down - #2687

Merged
kodiakhq[bot] merged 1 commit into
mainfrom
drew/exp-hist-metric-names
Jul 20, 2026
Merged

feat: Show exponential histogram metrics in the metric name drop-down#2687
kodiakhq[bot] merged 1 commit into
mainfrom
drew/exp-hist-metric-names

Conversation

@pulpdrew

@pulpdrew pulpdrew commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR updates the chart editor to include Exponential Histogram metric names in the metric name selector. This behavior is gated behind a new NEXT_PUBLIC_ENABLE_EXPONENTIAL_HISTOGRAMS environment variable, which is disabled in all environments while support for querying exponential histogram metrics is in progress.

Screenshots or video

Screenshot 2026-07-20 at 3 39 54 PM

How to test on Vercel preview

This must be tested locally, so that you can enable the NEXT_PUBLIC_ENABLE_EXPONENTIAL_HISTOGRAMS flag. You'll also likely need to insert a metric data point into the otel_metrics_exponential_histogram table and ensure your Metrics source has registered that table.

References

  • Linear Issue: Closes HDX-4851
  • Related PRs:

@vercel

vercel Bot commented Jul 20, 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, Comment Jul 20, 2026 7:02pm
hyperdx-storybook Ready Ready Preview, Comment Jul 20, 2026 7:02pm

Request Review

@changeset-bot

changeset-bot Bot commented Jul 20, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5c1f231

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

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

@github-actions github-actions Bot added the review/tier-2 Low risk — AI review + quick human skim label Jul 20, 2026
@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

🔵 Tier 2 — Low Risk

Small, isolated change with no API route or data model modifications.

Why this tier:

  • Standard feature/fix — introduces new logic or modifies core functionality

Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.
SLA: Resolve within 4 business hours.

Stats
  • Production files changed: 2
  • Production lines changed: 60 (+ 54 in test files, excluded from tier calculation)
  • Branch: drew/exp-hist-metric-names
  • Author: pulpdrew

To override this classification, remove the review/tier-2 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds exponential histogram metric names to the chart editor dropdown behind a feature flag. The main changes are:

  • Adds NEXT_PUBLIC_ENABLE_EXPONENTIAL_HISTOGRAMS config support.
  • Fetches exponential histogram metric names only when the flag and table registration are present.
  • Adds exponential histogram options to the metric name selector.
  • Updates unit tests for the expanded metric option list.
  • Adds a patch changeset for the app package.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
packages/app/src/components/MetricNameSelect.tsx Adds feature-flagged exponential histogram metadata fetching and dropdown options.
packages/app/src/config.ts Adds the exponential histogram feature flag using the existing runtime config pattern.
packages/app/src/components/tests/MetricNameSelect.test.ts Updates metric option tests to include the new exponential histogram argument and expected options.
.changeset/bright-histograms-toggle.md Adds a patch changeset for the feature-flagged dropdown update.

Reviews (2): Last reviewed commit: "feat: Show exponential histogram metrics..." | Re-trigger Greptile

Comment thread packages/app/src/components/MetricNameSelect.tsx Outdated
Comment thread packages/app/src/components/MetricNameSelect.tsx
@pulpdrew
pulpdrew force-pushed the drew/exp-hist-metric-names branch from a7eb199 to 5c1f231 Compare July 20, 2026 18:58
@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found. This change adds Exponential Histogram metrics to the metric-name dropdown entirely behind IS_EXPONENTIAL_HISTOGRAMS_ENABLED, which resolves to false in every environment (the env var is unset). Both the react-query enabled guard and the return-value ternary suppress the new path when the flag is off, and both call sites of getMetricOptions were updated for the new positional argument, so there is no reachable production regression in this diff.

🟡 P2 -- recommended

  • packages/app/src/components/MetricNameSelect.tsx:120 -- The flag-gating suppression in useMetricNames (the query enabled guard and the IS_EXPONENTIAL_HISTOGRAMS_ENABLED ? ... : undefined ternary) is the core new behavior and has zero test coverage; every added test exercises only the pure getMetricOptions function, which has no knowledge of the flag.
    • Fix: Add a hook/component test that mocks @/config and @/hooks/useMetadata to assert the exponential-histogram query is called with enabled: false and yields undefined metrics when the flag is off, and flows through when on.
    • testing, project-standards, correctness
  • packages/app/src/components/MetricNameSelect.tsx:159 -- Once the flag graduates, a user can select an exponential-histogram metric, but the shared renderer translateMetricChartConfig has no ExponentialHistogram branch and would throw, and the MCP tool schemas exclude it from QUERYABLE_METRIC_KINDS, so charting the selection fails and agents cannot reach parity.
    • Fix: Track renderer and MCP QUERYABLE_METRIC_KINDS support for exponential histograms as a prerequisite before NEXT_PUBLIC_ENABLE_EXPONENTIAL_HISTOGRAMS is enabled anywhere.
    • ce-agent-native-reviewer
🔵 P3 nitpicks (5)
  • packages/app/src/components/MetricNameSelect.tsx:136 -- getMetricOptions now takes four consecutive string[] | undefined positional arguments plus metricName/metricType, so a future metric type or reordered call could silently transpose arguments the compiler cannot distinguish.
    • Fix: Convert the signature to a single options object destructuring the named metric lists.
    • maintainability, kieran-typescript
  • packages/app/src/components/MetricNameSelect.tsx:172 -- The saved-metric fallback label uses capitalizeFirstLetter(metricType), producing (Exponential histogram), which does not match the query-option label (Exponential Histogram) for the same metric.
    • Fix: Derive both labels from a shared metric-type-to-label map so casing stays consistent.
  • packages/app/src/components/MetricNameSelect.tsx:127 -- The gaugeMetrics?.[0].value pattern (copied verbatim into the new exponentialHistogramMetrics?.[0].value line) optional-chains the array but not the [0] index, so an empty Facet[] would throw; the current aggregate query always returns one row, so this is latent, not reachable today.
    • Fix: Use ?.[0]?.value on all four lines to guard the index access.
    • kieran-typescript, correctness
  • packages/app/src/components/MetricNameSelect.tsx:120 -- The flag is checked twice (the query enabled guard and the return ternary); the two checks are independent and can drift.
    • Fix: Compute one isExponentialHistogramAvailable boolean and reuse it, or rely solely on enabled: false keeping the value undefined.
  • packages/app/src/components/__tests__/MetricNameSelect.test.ts:62 -- Gauge/histogram/sum option values are asserted against hardcoded literals (gauge, histogram, sum) while the new exponential-histogram case references MetricsDataType.ExponentialHistogram, so an accidental enum-value change would surface inconsistently.
    • Fix: Reference the MetricsDataType enum members in all option-value assertions.

Reviewers (7): correctness, testing, maintainability, project-standards, kieran-typescript, ce-agent-native-reviewer, ce-learnings-researcher.

Testing gaps:

  • No test toggles the flag through useMetricNames; the suppression-when-off behavior (the point of the change) is unverified.
  • No test covers the onChange round-trip for the space-containing exponential histogram value (split on SEPARATOR + toLowerCase() + setMetricType).
  • No test exercises the ?.[0].value access against an empty Facet[] result.

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 241 passed • 1 skipped • 1017s

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

Tests ran across 4 shards in parallel.

View full report →

@jordan-simonovski jordan-simonovski 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.

neat!

@kodiakhq
kodiakhq Bot merged commit de2c8a0 into main Jul 20, 2026
28 checks passed
@kodiakhq
kodiakhq Bot deleted the drew/exp-hist-metric-names branch July 20, 2026 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge review/tier-2 Low risk — AI review + quick human skim

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants