Skip to content

printer: (fix) format float to avoid overflow#16083

Merged
bwplotka merged 1 commit intoprometheus:mainfrom
giulianopz:main
Mar 3, 2025
Merged

printer: (fix) format float to avoid overflow#16083
bwplotka merged 1 commit intoprometheus:mainfrom
giulianopz:main

Conversation

@giulianopz
Copy link
Contributor

@giulianopz giulianopz commented Feb 27, 2025

No description provided.

@giulianopz
fix: format float to avoid overflow
365d019 
Signed-off-by: giulianopz <panzironi.giuliano@gmail.com>
bwplotka
bwplotka approved these changes Mar 3, 2025
View reviewed changes
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Thanks!

@bwplotka bwplotka merged commit 2ef8706 into prometheus:main Mar 3, 2025
27 checks passed
@charleskorn charleskorn mentioned this pull request Mar 21, 2025
Merged
charleskorn added a commit to grafana/mimir that referenced this pull request Mar 21, 2025
@charleskorn
Adjust test to match new pretty printing format in prometheus/prometh…
0e6a8a7 
…eus#16083
charleskorn added a commit to grafana/mimir that referenced this pull request Mar 21, 2025
@charleskorn
Adjust tests to match new pretty printing format in prometheus/promet…
90819e3 
…heus#16083
zenador pushed a commit to grafana/mimir that referenced this pull request Mar 21, 2025
@charleskorn
Upgrade mimir-prometheus (#10956)
3ef028b 
* Upgrade mimir-prometheus

* Adjust tests to match new pretty printing format in prometheus/prometheus#16083

* Fix breaking change from prometheus/prometheus#16156

* Upgrade to github.com/oklog/ulid/v2 (prometheus/prometheus#16168)

* Bring in changes from prometheus/prometheus#16199

* Remove OOO native histograms flag (prometheus/prometheus#16207)

* Add changelog entries

* Ignore deprecation warning for `model.NameValidationScheme`

* Remove outdated native histogram OOO integration test
@olivierlemasle
Copy link
Contributor

olivierlemasle commented Nov 3, 2025

@giulianopz @bwplotka Hi, what caused this change? What was the risk of overflow this change suppresses?

I try to understand because I use promtool to format my PromQL queries, and the format changed with this PR:

  • With version 3.2.0:

    $ ./prometheus-3.2.0.linux-amd64/promtool --experimental promql format "1000000000"
1e+09

  • With version 3.3.0-rc.0:

    $ ./prometheus-3.3.0-rc.0.linux-amd64/promtool --experimental promql format "1000000000"
1000000000

NB: even now, the format function can modify a value:

$ ./prometheus-3.3.0-rc.0.linux-amd64/promtool --experimental promql format "1000000000000.00001"
1000000000000

It looks like I'm not the only one to expect the PromQL format to be consistent: Mimir also updated their tests after this PR: e.g. grafana/mimir@90819e3

@giulianopz
Copy link
Contributor Author

giulianopz commented Nov 3, 2025

Hi @olivierlemasle, sorry for my bad English (and thinking): what I meant to say is that relying on fmt.Sprint without formatting directives was causing numbers of modest length, like the one in the test case I added, to be represented with scientific notation. This seemed to me a readability issue, compared to decimal form.

I understand that this is questionable. I apologise if I caused any unnecessary changes to you. @bwplotka, feel free to revert this commit if not needed/wanted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bwplotka bwplotka bwplotka approved these changes

@roidelapluie roidelapluie Awaiting requested review from roidelapluie roidelapluie is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@giulianopz @olivierlemasle @bwplotka