libbeat/common: make Round results identical across architectures#51967
libbeat/common: make Round results identical across architectures#51967AndersonQ wants to merge 1 commit into
Conversation
Round scales a value and then rounds it. The rounding step relied on floating-point behaviour that is not guaranteed to be identical across CPU architectures, so for values sitting exactly on a rounding midpoint (for example 0.50005) amd64 and arm64 could disagree: the same input produced a different rounded number depending on which architecture the beat ran on. This surfaced as the libbeat arm64 unit tests failing after the Go 1.26 upgrade. Rounding now uses the standard library's math.Round on the scaled value, a well-defined operation that every architecture computes the same way. Results are now stable and portable regardless of CPU or compiler, and the behaviour the tests pin down is unchanged. Assisted-By: Claude Code
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
🤖 GitHub commentsJust comment with:
|
There was a problem hiding this comment.
Pull request overview
This PR updates libbeat/common.Round to make rounding deterministic across CPU architectures by using math.Round on the scaled value, avoiding architecture-dependent edge behavior at midpoint boundaries (e.g. 0.50005 at 4 decimal places).
Changes:
- Replace custom midpoint rounding logic (
Modf+Ceil/Floor) withmath.Roundon the scaled value. - Update the
Roundfunction comment to explicitly describe half-away-from-zero behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
This change mirrors https://github.com/elastic/elastic-agent-system-metrics/pull/295/changes#diff-5ecedbe075db3830efdb44b74505a8748573ed54c5679946f377d3a7db1adf70 👍
I'd suggest to add tests for negative values, as it is changing with this change.
|
Tick the box to add this pull request to the merge queue (same as
|
Proposed commit message
Checklist
[ ] I have made corresponding changes to the documentation[ ] I have made corresponding change to the default configuration files[ ] I have added tests that prove my fix is effective or that my feature works. Where relevant, I have used the- the existing tests are now passingstresstest.shscript to run them under stress conditions and race detector to verify their stability.[ ] I have added an entry in./changelog/fragmentsusing the changelog tool.Disruptive User Impact
None
How to test this PR locally
run on ARM:
Related issues