Skip to content

Commit

Permalink
Fix the threshold precision issue (#1665)
Browse files Browse the repository at this point in the history
Signed-off-by: Jude Wang <[email protected]>
  • Loading branch information
guanw authored and yurishkuro committed Jul 16, 2019
1 parent 6891422 commit e567725
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
13 changes: 11 additions & 2 deletions storage/spanstore/downsampling_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"hash"
"hash/fnv"
"math"
"math/big"
"sync"

"github.com/uber/jaeger-lib/metrics"
Expand Down Expand Up @@ -96,7 +97,6 @@ type Sampler struct {

// NewSampler creates SamplingExecutor
func NewSampler(ratio float64, hashSalt string) Sampler {
threshold := uint64(ratio * float64(math.MaxUint64))
if hashSalt == "" {
hashSalt = defaultHashSalt
}
Expand All @@ -112,12 +112,21 @@ func NewSampler(ratio float64, hashSalt string) Sampler {
},
}
return Sampler{
threshold: threshold,
threshold: calculateThreshold(ratio),
hasherPool: pool,
lengthOfSalt: len(hashSaltBytes),
}
}

func calculateThreshold(ratio float64) uint64 {
// Use big.Float and big.Int to calculate threshold because directly convert
// math.MaxUint64 to float64 will cause digits/bits to be cut off if the converted value
// doesn't fit into bits that are used to store digits for float64 in Golang
boundary := new(big.Float).SetInt(new(big.Int).SetUint64(math.MaxUint64))
res, _ := boundary.Mul(boundary, big.NewFloat(ratio)).Uint64()
return res
}

// ShouldSample decides if a span should be sampled
func (s *Sampler) ShouldSample(span *model.Span) bool {
hasherInstance := s.hasherPool.Get().(*hasher)
Expand Down
6 changes: 6 additions & 0 deletions storage/spanstore/downsampling_writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package spanstore

import (
"errors"
"math"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -80,3 +81,8 @@ func TestDownSamplingWriter_hashBytes(t *testing.T) {
// Same traceID should always be hashed to same uint64 in DownSamplingWriter.
assert.Equal(t, h.hashBytes(), h.hashBytes())
}

func TestDownsamplingWriter_calculateThreshold(t *testing.T) {
var maxUint64 uint64 = math.MaxUint64
assert.Equal(t, maxUint64, calculateThreshold(1.0))
}

0 comments on commit e567725

Please sign in to comment.