[BilalAtique]

Summary

When a scraped series goes away, or one scrape fails, the collector's prometheus receiver sends one more point with FLAG_NO_RECORDED_VALUE set. This is the OTLP form of a Prometheus staleness marker. The ClickHouse exporter stores it with Flags = 1 and zeros: Value = 0 for gauges and sums, zero bucket counts for histograms, and Count = 0, Scale = 0 and no buckets for exponential histograms. The metric queries in renderChartConfig did not read Flags, so they used the marker as a real zero. Alerts use the same queries.

Effects on main, measured in the new integration test:

• Gauge: at the marker minute, min is 0 instead of 10, avg is 5 instead of 10, and count is 2 instead of 1. This is the case from Metric charts read data points flagged "no recorded value" (Prometheus staleness markers) as zeros #3511.
• Sum (cumulative counter): when the series comes back after the marker, the next point is compared with the marker's 0. After one failed scrape on a counter at 120, increase shows 130 for the next minute instead of 10.
• Histogram: the same reset. count is 15 instead of 5, and p50 is 7.5 although all new observations are in (10, 30].
• Exponential histogram: the same reset. Also, the marker's scale 0 makes the quantile query downscale every series to scale 0. p50 is 3.36 instead of a value in (4, 4.36].

Changes:

• New renderMetricWhere in renderChartConfig.ts adds bitAnd(Flags, 1) = 0 to the WHERE of the source CTE. The gauge, sum, histogram and exponential histogram paths use it, so single and multi-series metric charts and metric alerts all get it. The filter applies before the window functions, so the next point is compared with the last real point.
• The filter is added only when the table has a Flags column of a non-Nullable integer type (UInt32 in the OTel schema). The gauge and sum queries already read Flags, but ReqMetricTableColumns does not require it on histogram tables, and no table needs it to be an integer. So a custom table without it, or with another type, gets the same query as before. A failed column lookup also keeps the old query. The lookup reads the DESCRIBE result that the same query already caches, so it adds no ClickHouse query.

Things to be aware of:

• A flagged point never has a value, so results for data without markers do not change. The existing metric integration tests and unit snapshots pass without changes.
• Not changed: PromQL charts (they read the TimeSeries tables), raw SQL charts, and the MCP tools that read only metric names, units and attribute keys. Summary metrics have no chart query.
• A Nullable Flags column is skipped on purpose: bitAnd(NULL, 1) = 0 is NULL, so the filter would drop rows whose Flags is NULL.
• A counter that restarts behind a marker (120 → marker → 5) is now compared with 120, and the Sum query clamps the decrease to 0, as it does for every counter reset (see the comment on its Source CTE). On main, the marker's 0 was the baseline, so this one case read 5. A counter that comes back without a restart (one failed scrape) no longer counts its whole total again. Using the Prometheus reset rule (Value when it is below the previous point) would fix both, but it changes every counter reset, so I left it out of this PR.

Tests

• New packages/common-utils/src/__tests__/metricNoRecordedValue.int.test.ts (ClickHouse): one table per metric type with the OTel columns, and marker rows shaped like the receiver output (checked against receiver/prometheusreceiver/internal/metricfamily.go). 10 tests: gauge min, avg and count at the marker minute, the minute before it (a control), sum increase, a counter restart behind a marker against the same restart without one, histogram count (with the minute before the marker) and p50, and exponential histogram count (with the minute before the marker) and p50.
• renderChartConfig.test.ts: the filter is in the source scan once for each metric type and aggFn when Flags is UInt32, and it is there for UInt8, Int64 and LowCardinality(UInt32). It is not there when a histogram table has no Flags, when Flags is String, Nullable(UInt32) or LowCardinality(Nullable(UInt32)), or when the lookup fails.

With renderChartConfig.ts from main, 9 of the 10 integration tests fail (the control passes), and 9 of the 17 new unit tests fail (the "same query as before" cases pass on both). Without the type check, the 3 type tests fail. renderChartConfig.test.ts and builderToRawSql.test.ts (290 tests, no snapshot changes) and queryChartConfig.int.test.ts (81 tests) pass. ESLint (no new warnings) and Prettier pass. I ran the integration tests on ClickHouse 26.8.14.3 and 26.8.34.13; CI uses 26.8.36.3.

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.

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 two series of one gauge for the last 6 minutes. Series b ends with a marker in the last minute:

   INSERT INTO default.otel_metrics_gauge
     (ServiceName, MetricName, ResourceAttributes, TimeUnix, Value, Flags)
   SELECT
     'repro',
     'repro.cpu',
     map('host', host),
     toStartOfMinute(now()) - toIntervalMinute(5 - m),
     if(host = 'b' AND m = 5, 0, 10),
     if(host = 'b' AND m = 5, 1, 0)
   FROM (SELECT arrayJoin(['a', 'b']) AS host) AS hosts
   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 Min aggregation, over the last 15 minutes with a 1 minute granularity.

3. The last minute drops to 0. With Avg it drops to 5.

On this branch:

1. Do the same steps. The last minute stays at 10 for Min and Avg.

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

How to test on Vercel preview

N/A — non-UI change.

References

• Closes Metric charts read data points flagged "no recorded value" (Prometheus staleness markers) as zeros #3511
• Related: Metric selects ignore valueExpression: gauge and sum charts always aggregate the raw value #3499, Number tile on a gauge changes with the time range: sum adds up every sample in short ranges #3500 and Series limit isn't applied in SQL for metric charts: every series is fetched, then capped in the browser #3502 report other problems in the same metric queries. This PR does not change them.

[changeset-bot]

🦋 Changeset detected

Latest commit: 271830e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@hyperdx/common-utils	Patch
@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

[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. The change is tightly scoped and well-tested. renderMetricWhere appends bitAnd(Flags, 1) = 0 to the source-CTE WHERE only when the metric table exposes a non-Nullable integer Flags column, so staleness-marker points are dropped before any aggregation or delta reads them, and tables without a usable Flags column render exactly as before. Direct verification confirmed the core logic is sound:

• bitAnd(Flags, 1) = 0 isolates bit 0 only, so points carrying unrelated future flag bits are preserved — forward-compatible with OTLP flag additions.
• concatChSql (src/clickhouse/index.ts:325) skips empty operands, so an empty renderWhere result still yields a valid predicate with no dangling AND.
• getColumn resolves through the cached getColumns path (5-min TTL, in-flight dedup), and the gauge/sum branches already warm that cache via getComputedMetricColumns, so no extra ClickHouse round-trip is introduced.
• The Nullable exclusion (!flags.type.includes('Nullable(')) correctly rejects Nullable(UInt32) and LowCardinality(Nullable(UInt32)) while keeping LowCardinality(UInt32), matching the unit-test matrix.
• All four metric paths (gauge, sum, histogram, exponential histogram) route through the helper; summary has no chart query and is correctly untouched.

🔵 P3 nitpicks (2)

• packages/common-utils/src/core/renderChartConfig.ts:2050 — the Nullable guard uses a raw flags.type.includes('Nullable(') substring check that duplicates nullability logic stripTypeWrappers already encodes and depends on the exact DESCRIBE type-string format, so a formatting drift (e.g. interior whitespace) would silently defeat it.
  • Fix: factor the nullability test into a reusable isCHNullable-style predicate alongside isCHIntegerType and call it here instead of a literal substring match.
• packages/common-utils/src/__tests__/metricNoRecordedValue.int.test.ts:1 — the "read as before" skip path (non-integer and Nullable Flags) is asserted only at the SQL-string level in the unit tests; no integration test inserts a Nullable Flags column to behaviorally confirm rows with NULL flags are retained rather than dropped.
  • Fix: add one integration case with a Nullable(UInt32) Flags column and a NULL-flagged row, asserting the row still contributes to the aggregation.

---

Reviewers: correctness, testing, maintainability, project-standards, performance, kieran-typescript, adversarial, previous-comments, agent-native, learnings-researcher.

Testing gaps: Behavioral coverage exists for real UInt32 Flags across all metric types; the Nullable/non-integer skip path is covered only by SQL-string unit assertions (see P3 above). No regression to pre-existing metric tests.

_{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 (271830e):

• P2, untested branches: the failed-lookup fallback was already covered ("reads the table as before when the Flags lookup fails"). Added the positive type cases: the filter is applied for UInt8, Int64 and LowCardinality(UInt32).
• P3, controls before the marker: added. The histogram and exponential histogram count tests now also expect 10 for the minute before the marker.
• P3, a shared isCHNonNullableIntegerType helper: not done here. It would touch clickhouse/index.ts, which this PR does not change.
• P3, the broad catch: intended. The comment says a failed lookup gives the same query as before, which matches how getComputedMetricColumns handles a failed DESCRIBE.
• P3, a named constant for bit 0: left as is. The comment names the bit (FLAG_NO_RECORDED_VALUE), and it is used in one place.
• P3, the regex anchor in the unit test: intended. It checks that the filter is ANDed after the existing WHERE parts, which is what keeps operator precedence right.
• Residual risk, a counter reset at the same time as a marker: covered by the new restart test and listed in the description.

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?