[BilalAtique]

Summary

Two instrumentation scopes can report the same metric with the same resource, scope and point attributes. For example, two instrumentation libraries in one process can both record one HTTP metric. The OTel data model makes these two streams, because the instrumentation scope is part of a stream's identity. The metric queries in renderChartConfig keyed a series on cityHash64(ScopeAttributes, ResourceAttributes, Attributes) only, so they read the two scopes as one series. Alerts use the same queries.

Effects on main, measured in the new integration test. Both scopes report at the same TimeUnix. Scope a has an idle series, and scope b gets 5 new observations a minute:

• Gauge (scope a at 10, scope b at 20): sum is 20 instead of 30, and count is 1 instead of 2. A group-by on ScopeName shows one group, scope.a, with scope b's value of 20.
• Sum (cumulative counter, scope a idle at 1000): increase is 1000 a minute instead of 5. The window compares one scope's total with the other's, so the idle scope's whole total counts as new.
• Histogram: the same problem. count is 1005 a minute instead of 5, and p50 is 5.02 although all new observations are in (10, 30].
• Exponential histogram: the same problem. count is 1005 a minute instead of 5, and p50 is 2.09 although all new observations are in (4, 4.36].

The main values were the same in 3 runs. Rows with the same TimeUnix have no defined order in the window, so other data can give other wrong values.

Changes:

• New renderMetricSeriesKey in renderChartConfig.ts returns the series key cityHash64(ScopeName, ScopeAttributes, ResourceAttributes, Attributes). The gauge and sum Source CTEs use it for AttributesHash. The histogram and exponential histogram queries (count and quantile) get it through a new attrHashExpr input, which replaces the ATTR_HASH_EXPR constant in histogram.ts. Single and multi-series metric charts and metric alerts all get it.
• ScopeName is in the key only when the table has a ScopeName column. HyperDX does not require it, and the histogram queries did not read it, so a custom histogram table without it gets the same query as before. A failed column lookup also keeps the old key. The lookup reads the DESCRIBE result that the same query already caches, so it adds no ClickHouse query.
• A Nullable ScopeName is hashed as ifNull(ScopeName, ''). cityHash64 returns NULL when an argument is NULL, so otherwise every series with a NULL ScopeName would get the same key, whatever its attributes.
• Three test comments that quote the old key now match. Their assertions do not change.

Things to be aware of:

• Results change only for data where two scopes report the same metric with the same attributes. Every other series already had its own key, so it stays one series. The existing metric integration tests and unit snapshots pass without changes (the snapshot mocks have no ScopeName column).
• On a table with ScopeName, the histogram queries now read one more column (ScopeName, a String in the OTel schema). The gauge and sum queries select *, so they read it already.
• ScopeVersion and ScopeSchemaUrl are not in the key. The OTel data model also counts the scope version, but Hash by ScopeName to prevent double counts #1640 is about the name only, so I kept the change to that. I can add the version if you want it.
• Not changed: PromQL charts (they read the TimeSeries tables), raw SQL charts, and the metrics series table (SeriesHash), which no chart query reads.

Tests

• New packages/common-utils/src/__tests__/metricScopeName.int.test.ts (ClickHouse): one table per metric type with the OTel columns, and two scopes that differ only in ScopeName and report at the same TimeUnix, as one SDK reader does. 12 tests: gauge sum and count over both scopes, a gauge group-by on ScopeName, sum increase, a counter in one scope only (a control), histogram count and p50, and exponential histogram count and p50. Then two custom tables: a gauge with a Nullable(String) ScopeName, where two series with a NULL scope and other attributes stay apart (sum and count), and a histogram table without ScopeName.
• renderChartConfig.test.ts (14 tests): the key has ScopeName for each metric type and aggFn when the table has the column. It does not when a histogram table has no ScopeName or the lookup fails. LowCardinality(String) gives the same key, and Nullable(String) and LowCardinality(Nullable(String)) give the ifNull key.

With the code from main, 8 of the 12 integration tests fail (the control and the 3 custom-table tests pass), and 9 of the 14 unit tests fail (the 5 "same query as before" cases pass on both). Without the column lookup, the 3 custom-table tests fail: NULL-scope series merge (sum 20 instead of 30, count 1 instead of 2), and the histogram query fails with Unknown expression or function identifier ScopeName. renderChartConfig.test.ts and builderToRawSql.test.ts (305 tests, no snapshot changes) pass, and so do queryChartConfig.int.test.ts and metricNoRecordedValue.int.test.ts (91 tests, no changes). ESLint (no new warnings) and Prettier pass. I ran the integration tests on ClickHouse 26.8.36.3, the version CI uses.

I did not run the api integration tests (alerts, MCP), tsc for the whole package, or the UI steps below. I ran the SQL below against the OTel gauge schema on ClickHouse 26.8.36.3: the old key gives 1 series for its 12 rows, and the new key gives 2.

How to reproduce

Starting point: yarn dev, which creates the OTel metric tables in the default database of the local ClickHouse.

On main:

1. Insert one gauge from two scopes for the last 6 minutes. Only ScopeName is different:

   INSERT INTO default.otel_metrics_gauge
     (ServiceName, MetricName, ScopeName, ResourceAttributes, TimeUnix, Value)
   SELECT
     'repro',
     'repro.cpu',
     scope,
     map('service.name', 'repro'),
     toStartOfMinute(now()) - toIntervalMinute(5 - m),
     if(scope = 'scope.a', 10, 20)
   FROM (SELECT arrayJoin(['scope.a', 'scope.b']) AS scope) AS scopes
   CROSS JOIN (SELECT number AS m FROM numbers(6)) AS minutes
   

2. Open Chart Explorer. Pick the metrics source and the gauge repro.cpu with the Sum aggregation, over the last 15 minutes with a 1 minute granularity.

3. Each minute shows 10 or 20 (one scope), not 30.

4. Group by ScopeName. Only one line shows.

On this branch:

1. Do the same steps. Each minute shows 30, and the group-by shows scope.a at 10 and scope.b at 20.

The new integration test covers the other metric types: make dev-int-common-utils FILE=metricScopeName.

How to test on Vercel preview

N/A — non-UI change.

References

• Closes Hash by ScopeName to prevent double counts #1640
• Related: fix(common-utils): skip metric points that have no recorded value #3583 changed the same metric queries (it skips points with no recorded value).

[vercel]

@BilalAtique is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

[github-actions]

PR Review

✅ No issues found.

[github-actions]

Deep Review

✅ No critical issues found. This is a focused, well-tested correctness fix: ScopeName is folded into the metric series hash key via a new renderMetricSeriesKey helper (gauge/sum) and a threaded attrHashExpr param (histogram/exp-histogram), so two OTel instrumentation scopes reporting the same metric with identical attributes no longer collapse into one series. The raw-SQL interpolation is provably constant (no injection), the extra column lookup is served from the warm metadata cache, and the integration suite exercises every metric type plus Nullable and missing-column fallbacks.

🟡 P2 — recommended

• packages/common-utils/src/core/renderChartConfig.ts:2081 — A transient metadata.getColumn('ScopeName') failure is caught and treated identically to a genuinely-absent column, so the render silently reverts to the scope-merging key and re-introduces the exact bug this PR fixes for that evaluation; because these queries back metric alerts, a merged-scope result can alter grouping/threshold math for one cycle (degrades only to pre-PR behavior, not a new regression).
  • Fix: Distinguish a thrown lookup error from a successfully-returned-undefined column, and fail or retry the render rather than falling back to the ScopeName-less key when column presence is unknown.
  • _{reliability, adversarial, correctness}

🔵 P3 nitpicks (2)

• packages/common-utils/src/core/renderChartConfig.ts:2064 — The docstring parenthetical "the gauge and sum CTEs read it, the histogram CTEs do not" is now misleading: all four metric paths route through renderMetricSeriesKey, so the histogram and exp-histogram source CTEs do hash ScopeName when the column exists.

  • Fix: Reword to state that every metric type incorporates ScopeName into its series key when the column is present, best-effort on column availability.
  • _{maintainability, reliability}

• packages/common-utils/src/core/histogram.ts:65 — attrHashExpr: string is re-declared by hand across six inline parameter literals rather than a shared type alias; omission is a compile error today, but the shape can drift across signatures.

  • Fix: Extract a shared base type (e.g. have the quantile variants extend HistogramCountInput) so the field and its "must match the gauge/sum key byte-for-byte" invariant are declared once.
  • _{kieran-typescript, maintainability}

Pre-existing (not introduced by this diff)

• packages/common-utils/src/core/histogram.ts:100 — The histogram CTEs interpolate attrHashExpr via chSql's bare-string raw-SQL path (marked for deprecation) while gauge/sum use the explicit { UNSAFE_RAW_SQL } form; behavior is identical and predates this PR. Routing attrHashExpr through { UNSAFE_RAW_SQL } would make the intent consistent.

---

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

Testing gaps:

• No checkAlerts integration test exercises two scopes reporting the same metric with identical attributes; alert per-series threshold semantics under multi-scope data are only covered indirectly through the shared chart-query path.
• No test simulates metadata.getColumn throwing (transient failure) to pin the intended fallback behavior and whether alerts should fail-closed rather than silently re-merge scopes.
• No single-query assertion that every CTE (source + downstream window/partition CTEs) agrees on the same scope-aware key.

_{No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond}

[BilalAtique]

Re the Deep Review above (130256e):

• P2, a failed ScopeName lookup falls back to the old key: intended. A failed lookup can only give the query from main, the same as the Flags lookup in renderMetricWhere and a failed DESCRIBE in getComputedMetricColumns. Throwing would make a chart or an alert fail on a metadata error where it works today. The unit test "reads the table as before when the ScopeName lookup fails" covers this path.
• P3, the renderMetricSeriesKey comment: reworded in 130256e. Every metric type uses the key; the comment now says that the gauge and sum Bucketed CTEs select ScopeName anyway, and the histogram queries did not read it before.
• P3, a shared type for attrHashExpr: not done here. The quantile translators already declare their inputs inline, and a shared base type would refactor those signatures, which this fix does not need.
• Pre-existing, bare-string interpolation in the histogram CTEs: left as is. ATTR_HASH_EXPR used the same form before this PR.
• Testing gaps: alerts render through the same renderChartConfig path, so I did not add a checkAlerts test. The lookup-failure case has the unit test above. Each translator computes attr_hash once in its source CTE and the window CTEs read that column, so they cannot use different keys.

[BilalAtique]

Re the PR Review above: fixed in fe74ddc. The queryChartConfig.int.test.ts comment now gives the key with ScopeName and points at renderMetricSeriesKey. It wrapped over two lines, so my search for the old key missed it.

That is two rounds of bot findings, so I'll stop here. Could a maintainer approve the workflows and look at the scope when you have time?