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

Precision Issue with Downsampling Writer Threshold #1664

Closed
guanw opened this issue Jul 16, 2019 · 2 comments
Closed

Precision Issue with Downsampling Writer Threshold #1664

guanw opened this issue Jul 16, 2019 · 2 comments

Comments

@guanw
Copy link
Contributor

guanw commented Jul 16, 2019

Requirement - what kind of business use case are you trying to solve?

The current downsampling writer has a precision issue with threshold calculation.
Specifically, at L99 in storage/spanstore/downsampling_writer.go, three calculations are performed:

  1. max uint64 is converted to float64 boundary
  2. float64 boundary multiplied with ratio to calculate threshold
  3. calculated threshold is converted back to uint64 as threshold to compare with trace id.

The issue is that:
At step 1 Golang's float precision is 52 bits so if we have a long uint64 it's going to round up for bits longer than 52.(e.g ratio = 1.0)
At step 3 if round up float64 exceeds the maximum number uint64 can represent converted threshold will be wrong.

Problem - what in Jaeger blocks you from solving the requirement?

None

Proposal - what do you suggest to solve the problem or improve the existing situation?

Use big int and float for calculation

Any open questions to address

  • We should also verify if similar issues exist in client libraries.
@guanw
Copy link
Contributor Author

guanw commented Jul 16, 2019

Add the following example to better illustrate the issue with precision
image

@guanw
Copy link
Contributor Author

guanw commented Aug 8, 2019

Similar issues exist in jaeger-client-go sampler.go. See screenshot for details:
image

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

No branches or pull requests

2 participants