Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integer-only Counters #1464

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

notrobpike
Copy link

Avoid rounding error for large-valued Counters.

@ArthurSens @bwplotka @kakkoyun

Not Rob Pike added 2 commits March 10, 2024 12:38
Signed-off-by: Not Rob Pike <[email protected]>
indeed the ival and fval can get updated separately, but such update
is always one or the other, not both.
@notrobpike
Copy link
Author

There are 2 lint failures. One of them exists in main already and is not due to any change in this PR. The other is introduced because the now field of Opts isn't used by any other type alias, so when I introduced CounterOpt as its own type, that field got left behind. I'm not so sure about removing it, although clearly it's correct to do so. Maybe you prefer a different fix.

@@ -23,6 +23,9 @@ import (
"google.golang.org/protobuf/types/known/timestamppb"
)

// 64-bit float mantissa: https://en.wikipedia.org/wiki/Double-precision_floating-point_format
var float64Mantissa uint64 = 9007199254740992
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this really be a const instead of a var? IMHO it would also be clearer and more legible if it were defined as 1 << 53 rather than a long decimal number.

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