Skip to content

Conversation

@the-mikedavis
Copy link
Contributor

The formatting function should take the state as the first argument and new data as the second. See format/1 which passes in the trailing linefeed as the second argument. This doesn't make much difference when calling format/1 because the swapped arguments only caused the metrics to be concatenated in reverse. But this fix is important for using format_into/3 correctly, for example with cowboy_req:stream_body/3.

@codecov
Copy link

codecov bot commented Nov 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/formats/prometheus_text_format.erl 94.52% <100.00%> (ø)
test/eunit/format/prometheus_text_format_tests.erl 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The formatting function should take the state as the first argument and
new data as the second. See `format/1` which passes in the trailing
linefeed as the second argument. This doesn't make much difference when
calling `format/1` because the swapped arguments only caused the metrics
to be concatenated in reverse. But this fix is important for using
`format_into/3` correctly, for example with `cowboy_req:stream_body/3`.
Copy link
Member

@NelsonVides NelsonVides left a comment

Choose a reason for hiding this comment

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

Great to have a regression test for it too!

@NelsonVides NelsonVides merged commit fbce631 into prometheus-erl:master Nov 22, 2025
5 checks passed
@the-mikedavis the-mikedavis deleted the md/fix-format-into branch November 22, 2025 20:29
@NelsonVides
Copy link
Member

https://hex.pm/packages/prometheus/6.1.1 😁

@the-mikedavis
Copy link
Contributor Author

the-mikedavis commented Nov 22, 2025

Thanks for the super quick turnaround on this! I'm re-running my tests in RabbitMQ with 6.1.1. I can confirm that it works now 😅. And so far the comparison is still looking very good - I think the per-object metrics RabbitMQ end up using quite a few floats so the fast-path we added in #196 probably cuts down on temporary allocations by quite a bit.

Edit: yep with these changes and cowboy_req:stream_body/3 this is looking great! rabbitmq/rabbitmq-server#14885 (comment)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants