textparse: fix NaN canonicalization check in OpenMetrics getFloatValue by cuiweixie · Pull Request #18399 · prometheus/prometheus

cuiweixie · 2026-03-30T09:41:49Z

getFloatValue was testing p.exemplarVal instead of the parsed float when normalizing NaN to the canonical representation, so OpenMetrics metric sample values that were NaN were not normalized correctly.

[BUGFIX] Textparse: Normalize NaN in OpenMetrics metric values using the parsed float, not exemplar state.

getFloatValue was testing p.exemplarVal instead of the parsed float when normalizing NaN to the canonical representation, so metric values that were NaN were not normalized correctly. Signed-off-by: Weixie Cui <[email protected]>

bboreham

Certainly looks plausible, but I'm intrigued: how do you get a non-canonical NaN out of strconv.ParseFloat ?

Related: could you write a test for this condition?

cuiweixie · 2026-03-31T13:43:52Z

You're right that strconv.ParseFloat currently returns a canonical NaN.
The issue here is not about how the NaN is produced, but that we should normalize any NaN value we encounter, regardless of its origin.
what's more using p.exemplarVal instead of val is obviously a tiny bug.

krajorama · 2026-03-31T14:49:37Z

You're right that strconv.ParseFloat currently returns a canonical NaN. The issue here is not about how the NaN is produced, but that we should normalize any NaN value we encounter, regardless of its origin. what's more using p.exemplarVal instead of val is obviously a tiny bug.

Seems like this is actually dead code, you cannot get NaN in p.exemplarVal and val cannot be abnormal NaN anyway. At least in the current implementation/architecture we're looking at.

Anyway, I'm ok with the change, but I'd like to suggest making this change to the tests:

diff --git a/model/textparse/interface_test.go b/model/textparse/interface_test.go
index d0b6b293a..6241ea6bb 100644
--- a/model/textparse/interface_test.go
+++ b/model/textparse/interface_test.go
@@ -222,6 +222,7 @@ func requireEntries(t *testing.T, exp, got []parsedEntry) {
                        _, yIsLabels := y.(labels.Labels)
                        return !xIsLabels && !yIsLabels
                }, cmpopts.EquateEmpty()),
+               cmpopts.EquateNaNs(),
                cmp.AllowUnexported(parsedEntry{}),
        })
 }
diff --git a/model/textparse/openmetricsparse_test.go b/model/textparse/openmetricsparse_test.go
index d365d8d95..db612a74c 100644
--- a/model/textparse/openmetricsparse_test.go
+++ b/model/textparse/openmetricsparse_test.go
@@ -16,6 +16,7 @@ package textparse
 import (
        "fmt"
        "io"
+       "math"
        "testing"
 
        "github.com/prometheus/common/model"
@@ -23,6 +24,7 @@ import (
 
        "github.com/prometheus/prometheus/model/exemplar"
        "github.com/prometheus/prometheus/model/labels"
+       "github.com/prometheus/prometheus/model/value"
 )
 
 func int64p(x int64) *int64 { return &x }
@@ -113,7 +115,13 @@ foobar_count 21
 foobar_created 1520430004
 foobar_sum 324789.6
 foobar{quantile="0.95"} 123.8
-foobar{quantile="0.99"} 150.1`
+foobar{quantile="0.99"} 150.1
+# HELP nansum Summary with NaN value
+# TYPE nansum summary
+nansum_count 0
+nansum_sum 0.0
+nansum{quantile="0.95"} nan
+nansum{quantile="0.99"} NaN`
 
        input += "\n# HELP metric foo\x00bar"
        input += "\nnull_byte_metric{a=\"abc\x00\"} 1"
@@ -631,6 +639,42 @@ foobar{quantile="0.99"} 150.1`
                                                labels.FromStrings("__name__", "foobar", "quantile", "0.99"),
                                        ),
                                        st: 1520430004000,
+                               }, {
+                                       m:    "nansum",
+                                       help: "Summary with NaN value",
+                               }, {
+                                       m:   "nansum",
+                                       typ: model.MetricTypeSummary,
+                               }, {
+                                       m: "nansum_count",
+                                       lset: typeAndUnitLabels(
+                                               typeAndUnitEnabled,
+                                               labels.FromStrings("__name__", "nansum_count", "__type__", string(model.MetricTypeSummary)),
+                                               labels.FromStrings("__name__", "nansum_count"),
+                                       ),
+                               }, {
+                                       m: "nansum_sum",
+                                       lset: typeAndUnitLabels(
+                                               typeAndUnitEnabled,
+                                               labels.FromStrings("__name__", "nansum_sum", "__type__", string(model.MetricTypeSummary)),
+                                               labels.FromStrings("__name__", "nansum_sum"),
+                                       ),
+                               }, {
+                                       m: `nansum{quantile="0.95"}`,
+                                       v: math.Float64frombits(value.NormalNaN),
+                                       lset: typeAndUnitLabels(
+                                               typeAndUnitEnabled,
+                                               labels.FromStrings("__name__", "nansum", "__type__", string(model.MetricTypeSummary), "quantile", "0.95"),
+                                               labels.FromStrings("__name__", "nansum", "quantile", "0.95"),
+                                       ),
+                               }, {
+                                       m: `nansum{quantile="0.99"}`,
+                                       v: math.Float64frombits(value.NormalNaN),
+                                       lset: typeAndUnitLabels(
+                                               typeAndUnitEnabled,
+                                               labels.FromStrings("__name__", "nansum", "__type__", string(model.MetricTypeSummary), "quantile", "0.99"),
+                                               labels.FromStrings("__name__", "nansum", "quantile", "0.99"),
+                                       ),
                                }, {
                                        m:    "metric",
                                        help: "foo\x00bar",

I'm using summary as that's a "natural" place for NaN.

bboreham · 2026-04-03T10:19:22Z

If the condition cannot occur and the change is just to make the code internally consistent, this would not have a changelog entry.

Extend TestOpenMetricsParse with nansum summary lines and cmpopts.EquateNaNs in requireEntries so NaN float values compare equal after canonicalization. Signed-off-by: Weixie Cui <[email protected]>

Document the bugfix in CHANGELOG.md (prometheus#18399). Signed-off-by: Weixie Cui <[email protected]>

cuiweixie · 2026-04-03T13:18:49Z

If the condition cannot occur and the change is just to make the code internally consistent, this would not have a changelog entry.

Done

Document the bugfix in CHANGELOG.md (prometheus#18399). Signed-off-by: Weixie Cui <[email protected]>

cuiweixie requested a review from a team as a code owner March 30, 2026 09:41

cuiweixie requested a review from metalmatze March 30, 2026 09:41

bboreham reviewed Mar 31, 2026

View reviewed changes

krajorama self-assigned this Apr 1, 2026

textparse: add tests for OpenMetrics summary NaN quantiles

80d307d

Extend TestOpenMetricsParse with nansum summary lines and cmpopts.EquateNaNs in requireEntries so NaN float values compare equal after canonicalization. Signed-off-by: Weixie Cui <[email protected]>

cuiweixie added a commit to cuiweixie/prometheus that referenced this pull request Apr 3, 2026

textparse: add changelog entry for OpenMetrics NaN sample fix

940d9c7

Document the bugfix in CHANGELOG.md (prometheus#18399). Signed-off-by: Weixie Cui <[email protected]>

cuiweixie force-pushed the textparse/fix-getfloatvalue-nan-check branch from ac74ccb to 940d9c7 Compare April 3, 2026 13:18

textparse: add changelog entry for OpenMetrics NaN sample fix

42d1155

Document the bugfix in CHANGELOG.md (prometheus#18399). Signed-off-by: Weixie Cui <[email protected]>

cuiweixie force-pushed the textparse/fix-getfloatvalue-nan-check branch from 940d9c7 to 42d1155 Compare April 3, 2026 15:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

textparse: fix NaN canonicalization check in OpenMetrics getFloatValue#18399

textparse: fix NaN canonicalization check in OpenMetrics getFloatValue#18399
cuiweixie wants to merge 3 commits intoprometheus:mainfrom
cuiweixie:textparse/fix-getfloatvalue-nan-check

cuiweixie commented Mar 30, 2026

Uh oh!

bboreham left a comment

Uh oh!

cuiweixie commented Mar 31, 2026

Uh oh!

krajorama commented Mar 31, 2026

Uh oh!

bboreham commented Apr 3, 2026

Uh oh!

cuiweixie commented Apr 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

cuiweixie commented Mar 30, 2026

Uh oh!

bboreham left a comment

Choose a reason for hiding this comment

Uh oh!

cuiweixie commented Mar 31, 2026

Uh oh!

krajorama commented Mar 31, 2026

Uh oh!

bboreham commented Apr 3, 2026

Uh oh!

cuiweixie commented Apr 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants