Skip to content

fix: include counter names in negative value errors - #2315

Merged
zeitlinger merged 5 commits into
prometheus:mainfrom
HarshDevelops:fix/1090-counter-negative-name
Sep 15, 2026
Merged

fix: include counter names in negative value errors#2315
zeitlinger merged 5 commits into
prometheus:mainfrom
HarshDevelops:fix/1090-counter-negative-name

Conversation

@HarshDevelops

@HarshDevelops HarshDevelops commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Fixes #1090

What was wrong

When a counter scrape produced a negative value, the exception was only the bare number and the phrase counters cannot have a negative value. In large applications that is hard to map back to a meter.

What changed

CounterDataPointSnapshot can carry an optional metric name used only in the validation message. Counter and CounterWithCallback pass the name when collecting. Labels are appended when present. Callers such as Micrometer can set the name through the builder.

Testing

  • CounterSnapshotTest.testNegativeValueIncludesMetricNameInMessage
  • CounterSnapshotTest (9 tests, 0 failures) on JDK 25
  • Signed-off-by (DCO)
  • Honest note: local package of prometheus-metrics-core succeeded; full multi-module CI is expected on GitHub

@HarshDevelops
Include counter metric name in negative value errors
014f248 
When a CounterSnapshot data point has a negative value, the exception
message can now include the metric name (and labels) so large scrapes
are easier to diagnose. Counter and CounterWithCallback pass the name
at collect time; callers such as Micrometer can set it via the builder.

Fixes prometheus#1090

Signed-off-by: Harsh Srivastava <harsh10822@gmail.com>
zeitlinger
zeitlinger requested changes Jul 20, 2026
View reviewed changes

@zeitlinger zeitlinger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. Please make these changes before approval:

  • Do not add new public constructors to CounterDataPointSnapshot. This is stable API surface; keep the existing constructors and use the builder for the optional metric name instead. Update the internal collection paths to configure the builder rather than adding public constructor overloads.
  • Add coverage through the actual metric collection paths, especially CounterWithCallback, to verify that a negative callback value includes the metric name while existing constructor behavior remains unchanged.

@zeitlinger

zeitlinger commented Sep 15, 2026

Copy link
Copy Markdown
Member

Addressed the requested changes in signed commit 1defbbfe8:

  • Removed the two added public CounterDataPointSnapshot constructor overloads; internal collection paths now use the builder for the optional metric name.
  • Added collection-path coverage in CounterWithCallbackTest proving negative callback values include the metric name.
  • Existing constructor behavior remains covered by CounterSnapshotTest.

Validation: model/core targeted suite passed (CounterSnapshotTest 9, CounterWithCallbackTest 3, CounterTest 30) when the exposition-format module was included in the reactor. mise run lint:fix and mise run build -- -DskipITs=true passed. The full test task had an unrelated Spring smoke failure caused by a protobuf gencode/runtime mismatch (gencode 4.35.1 vs runtime 4.34.2); the same smoke test passes on origin/main.

@zeitlinger zeitlinger changed the title Include counter metric name in negative value errors fix: include counter names in negative value errors Sep 15, 2026
@zeitlinger

zeitlinger commented Sep 15, 2026

Copy link
Copy Markdown
Member

Also synchronized the branch with current main in signed merge commit 3bec32437. This updates the stale generated protobuf sources/BOM together (the PR had generated code 4.35.1 while current main is 4.36.1).

The Spring smoke test now passes in the merged branch: 3 tests passed. The earlier failure was due to the stale branch BOM selecting protobuf runtime 4.35.1 against current generated code 4.36.1, not the counter changes.

@zeitlinger
fix: include counter names in negative value errors
fbcf1d6 
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger
zeitlinger force-pushed the fix/1090-counter-negative-name branch from 3bec324 to fbcf1d6 Compare September 15, 2026 12:51
@zeitlinger

zeitlinger commented Sep 15, 2026

Copy link
Copy Markdown
Member

The branch is now based directly on current main (rather than a merge commit) in signed commit fbcf1d63; this also satisfies DCO.

Final targeted reactor tests passed: CounterSnapshotTest (9), CounterWithCallbackTest (3), and CounterTest (30). The Spring ApplicationIT smoke test passed all 3 tests after rebuilding the current BOM/generated protobuf pair. The prior protobuf mismatch was confirmed to be stale branch/BOM state.

@zeitlinger
fix: include counter names in negative value errors
3b1b704 
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger
chore: preserve original PR contribution ancestry
c6bf406 
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger
zeitlinger force-pushed the fix/1090-counter-negative-name branch from 99cf975 to c6bf406 Compare September 15, 2026 12:55
@zeitlinger
chore: refresh API diff for counter error context
1733f4a 
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger
zeitlinger force-pushed the fix/1090-counter-negative-name branch from 314171d to 1733f4a Compare September 15, 2026 13:13
@github-actions

github-actions Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

⚠️ API changes detected — maintainer review required

This PR modifies the published API diff for the following module(s):

  • prometheus-metrics-model

Please review the changes in docs/apidiffs/current_vs_latest/ carefully before approving.

@zeitlinger

zeitlinger commented Sep 15, 2026

Copy link
Copy Markdown
Member

thanks for the contribution

@zeitlinger
zeitlinger merged commit ea8f935 into prometheus:main Sep 15, 2026
23 checks passed
@github-actions github-actions Bot mentioned this pull request Sep 15, 2026
Merged
zeitlinger pushed a commit that referenced this pull request Sep 16, 2026
@github-actions
chore(main): release 1.9.0 (#2237)
71e821c 
🤖 I have created a release *beep* *boop*
---


##
[1.9.0](v1.8.0...v1.9.0)
(2026-09-16)


### Features

* support metric name filtering in OpenTelemetry exporter
([#2344](#2344))
([9b0ede8](9b0ede8))


### Bug Fixes

* avoid protobuf debug reflection in native images
([#2251](#2251))
([7f899e7](7f899e7))
* bound HTTPServer request resources
([#2333](#2333))
([33ec556](33ec556))
* bound observation buffering during collection
([#2336](#2336))
([43788f5](43788f5))
* bound scrape query parameters
([#2334](#2334))
([27e1912](27e1912))
* **ci:** skip benchmark report for skipped runs
([#2422](#2422))
([40eddb0](40eddb0))
* clarify benchmark regression report verdicts
([#2394](#2394))
([e5fa067](e5fa067))
* **deps:** update dependency com.google.guava:guava to v33.7.0-jre
([#2387](#2387))
([bf0db49](bf0db49))
* **deps:** update dependency io.dropwizard.metrics:metrics-core to
v4.2.40 ([#2432](#2432))
([dd88326](dd88326))
* **deps:** update dependency io.dropwizard.metrics5:metrics-core to
v5.0.8 ([#2433](#2433))
([42f3c8a](42f3c8a))
* **deps:** update dependency
io.opentelemetry.instrumentation:opentelemetry-instrumentation-bom-alpha
to v2.29.0-alpha
([#2235](#2235))
([cf9f702](cf9f702))
* **deps:** update dependency
io.opentelemetry.instrumentation:opentelemetry-instrumentation-bom-alpha
to v2.30.0-alpha
([#2328](#2328))
([1ca2716](1ca2716))
* **deps:** update dependency
io.opentelemetry.instrumentation:opentelemetry-instrumentation-bom-alpha
to v2.30.0-alpha
([#2330](#2330))
([07623c1](07623c1))
* **deps:** update dependency
io.opentelemetry.instrumentation:opentelemetry-instrumentation-bom-alpha
to v2.31.0-alpha
([#2401](#2401))
([6c26619](6c26619))
* **deps:** update dependency
io.opentelemetry.instrumentation:opentelemetry-instrumentation-bom-alpha
to v2.31.0-alpha
([#2402](#2402))
([ac0d68a](ac0d68a))
* **deps:** update dependency
io.opentelemetry.instrumentation:opentelemetry-instrumentation-bom-alpha
to v2.31.1-alpha
([#2409](#2409))
([5eea652](5eea652))
* **deps:** update dependency
io.opentelemetry.instrumentation:opentelemetry-instrumentation-bom-alpha
to v2.31.1-alpha
([#2410](#2410))
([0bcef89](0bcef89))
* **deps:** update dependency org.apache.tomcat.embed:tomcat-embed-core
to v11.0.23
([#2241](#2241))
([a017f80](a017f80))
* **deps:** update dependency org.apache.tomcat.embed:tomcat-embed-core
to v11.0.24
([#2294](#2294))
([63967bd](63967bd))
* **deps:** update dependency org.apache.tomcat.embed:tomcat-embed-core
to v11.0.25
([#2389](#2389))
([92f8344](92f8344))
* **deps:** update dependency org.apache.tomcat.embed:tomcat-embed-core
to v11.0.26
([#2477](#2477))
([05146c0](05146c0))
* **deps:** update dependency
org.springframework.boot:spring-boot-starter-parent to v4.1.1
([#2399](#2399))
([a0b0880](a0b0880))
* **deps:** update jetty monorepo to v12.1.11
([#2279](#2279))
([4dc54da](4dc54da))
* **deps:** update jetty monorepo to v12.1.12
([#2371](#2371))
([08967e0](08967e0))
* **deps:** update jetty monorepo to v12.1.13
([#2459](#2459))
([b217f05](b217f05))
* **deps:** update junit-framework monorepo to v6.1.2
([#2300](#2300))
([5966d1d](5966d1d))
* **deps:** update junit-framework monorepo to v6.1.3
([#2374](#2374))
([d1ade52](d1ade52))
* **deps:** update otel.instrumentation.version
([#2236](#2236))
([158230d](158230d))
* **deps:** update protobuf
([#2400](#2400))
([e2db1ed](e2db1ed))
* **deps:** update protobuf
([#2438](#2438))
([8ad6fa8](8ad6fa8))
* **deps:** update protobuf to v4.35.1
([#2221](#2221))
([cf17073](cf17073))
* disable micrometer compat build cache
([#2457](#2457))
([6a40eda](6a40eda))
* drop +Inf bound from OpenTelemetry classic histogram boundaries
([#2458](#2458))
([a3bce9a](a3bce9a))
* **exposition:** export internal package for OSGi resolution
([#2415](#2415))
([28b503d](28b503d))
* **httpserver:** make scrape error responses secure and configurable
([f6d9df5](f6d9df5))
* include counter names in negative value errors
([#2315](#2315))
([ea8f935](ea8f935))
* include license files in release source jars
([#2250](#2250))
([08cf925](08cf925)),
closes [#2216](#2216)
* keep late observations out of subsequent collection buffers
([#2471](#2471))
([d78b149](d78b149))
* keep PR title check required after rebases
([#2414](#2414))
([e3d4c3b](e3d4c3b))
* prevent buffer stripe index overflow
([#2331](#2331))
([b6cd000](b6cd000))
* redact invalid configuration values
([#2335](#2335))
([7e7e533](7e7e533))
* show uncertainty in benchmark comparisons
([#2476](#2476))
([398d087](398d087))
* stabilize OpenTelemetry exporter builder API
([#2257](#2257))
([09e6e2d](09e6e2d))
* Summary quantiles collapsing for targeted quantiles with 2*epsilon
&gt;= 1-quantile
([#2396](#2396))
([9c7479f](9c7479f))
* update component-prefixed action tags
([#2419](#2419))
([4acf481](4acf481))


### Performance Improvements

* skip snapshot rebuild in mergeDuplicates when names are unique
([#2441](#2441))
([8722230](8722230))


### Documentation

* add API design guideline to contributing docs
([#2350](#2350))
([23ae29a](23ae29a))
* document scrape query limits in request API
([#2391](#2391))
([ba8f5eb](ba8f5eb))
* document semantic PR title guidance
([#2318](#2318))
([5e813a0](5e813a0))

---
> [!IMPORTANT]
> Close and reopen this PR to trigger CI checks.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zeitlinger zeitlinger zeitlinger approved these changes
@fstab fstab Awaiting requested review from fstab fstab is a code owner
@dhoard dhoard Awaiting requested review from dhoard dhoard is a code owner
@jaydeluca jaydeluca Awaiting requested review from jaydeluca jaydeluca is a code owner

Assignees

No one assigned

Labels

api-change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add counter name to exception message when negative value is detected

2 participants

@HarshDevelops @zeitlinger