Skip to content

Commit

Permalink
Update sample rate precision handling (#834)
Browse files Browse the repository at this point in the history
* Update sample rate precision handling

We already had sampling rate rounding,
but it was using the older proposal of
3 decimal places, and did not clamp the
lower bound.

Updated to use the same logic in both the
ratio sampler, and the implementation-
independent interpretation of the sample
rate.

Also, use the "round" function instead
of math.Round directly, to support older
versions of Go.

* scripts: pin ugorji/go
  • Loading branch information
axw authored Oct 12, 2020
1 parent 372d026 commit 5949a1a
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 16 deletions.
24 changes: 19 additions & 5 deletions sampler.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,15 @@ type SampleResult struct {
Sampled bool

// SampleRate holds the sample rate in effect at the
// time of the sampling decision.
// time of the sampling decision. This is used for
// propagating the value downstream, and for inclusion
// in events sent to APM Server.
//
// The sample rate will be rounded to 4 decimal places
// half away from zero, except if it is in the interval
// (0, 0.0001], in which case it is set to 0.0001. The
// Sampler implementation should also internally apply
// this logic to ensure consistency.
SampleRate float64
}

Expand All @@ -82,10 +90,7 @@ func NewRatioSampler(r float64) Sampler {
if r < 0 || r > 1.0 {
panic(errors.Errorf("ratio %v out of range [0,1.0]", r))
}
if r > 0 && r < 0.0001 {
r = 0.0001
}
r = math.Round(r*10000) / 10000
r = roundSampleRate(r)
var x big.Float
x.SetUint64(math.MaxUint64)
x.Mul(&x, big.NewFloat(r))
Expand Down Expand Up @@ -114,3 +119,12 @@ func (s ratioSampler) SampleExtended(args SampleParams) SampleResult {
}
return result
}

// roundSampleRate rounds r to 4 decimal places half away from zero,
// with the exception of values > 0 and < 0.0001, which are set to 0.0001.
func roundSampleRate(r float64) float64 {
if r > 0 && r < 0.0001 {
r = 0.0001
}
return round(r*10000) / 10000
}
1 change: 1 addition & 0 deletions scripts/before_install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ if (! go run scripts/mingoversion.go 1.11 &>/dev/null); then
pin github.com/elastic/go-sysinfo v1.3.0
pin google.golang.org/grpc v1.30.0 https://github.com/grpc/grpc-go
pin github.com/jinzhu/gorm v1.9.16
pin github.com/ugorji/go v1.1.10
fi

if (! go run scripts/mingoversion.go 1.10 &>/dev/null); then
Expand Down
8 changes: 4 additions & 4 deletions span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func TestTracerStartSpanIDSpecified(t *testing.T) {
func TestSpanSampleRate(t *testing.T) {
tracer := apmtest.NewRecordingTracer()
defer tracer.Close()
tracer.SetSampler(apm.NewRatioSampler(0.5555))
tracer.SetSampler(apm.NewRatioSampler(0.55555))

tx := tracer.StartTransactionOptions("name", "type", apm.TransactionOptions{
// Use a known transaction ID for deterministic sampling.
Expand All @@ -164,7 +164,7 @@ func TestSpanSampleRate(t *testing.T) {
tracer.Flush(nil)

payloads := tracer.Payloads()
assert.Equal(t, 0.556, *payloads.Transactions[0].SampleRate)
assert.Equal(t, 0.556, *payloads.Spans[0].SampleRate)
assert.Equal(t, 0.556, *payloads.Spans[1].SampleRate)
assert.Equal(t, 0.5556, *payloads.Transactions[0].SampleRate)
assert.Equal(t, 0.5556, *payloads.Spans[0].SampleRate)
assert.Equal(t, 0.5556, *payloads.Spans[1].SampleRate)
}
8 changes: 4 additions & 4 deletions tracecontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,8 +330,8 @@ func (e *TraceStateEntry) validateValue() error {
}

func formatElasticTracestateValue(sampleRate float64) string {
// 0 -> "s:0"
// 1 -> "s:1"
// 0.5555 -> "s:0.555" (any rounding should be applied prior)
return fmt.Sprintf("s:%.3g", sampleRate)
// 0 -> "s:0"
// 1 -> "s:1"
// 0.55555 -> "s:0.5555" (any rounding should be applied prior)
return fmt.Sprintf("s:%.4g", sampleRate)
}
2 changes: 1 addition & 1 deletion transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func (t *Tracer) StartTransactionOptions(name, transactionType string, opts Tran
// we will scale the sampled transactions.
result.SampleRate = 0
}
sampleRate := round(1000*result.SampleRate) / 1000
sampleRate := roundSampleRate(result.SampleRate)
tx.traceContext.State = NewTraceState(TraceStateEntry{
Key: elasticTracestateVendorKey,
Value: formatElasticTracestateValue(sampleRate),
Expand Down
7 changes: 5 additions & 2 deletions transaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,10 @@ func TestTransactionSampleRate(t *testing.T) {
tests := []test{
{0, 0, "es=s:0"},
{1, 1, "es=s:1"},
{0.5555, 0.556, "es=s:0.556"},
{0.00001, 0.0001, "es=s:0.0001"},
{0.55554, 0.5555, "es=s:0.5555"},
{0.55555, 0.5556, "es=s:0.5556"},
{0.55556, 0.5556, "es=s:0.5556"},
}
for _, test := range tests {
test := test // copy for closure
Expand All @@ -195,7 +198,7 @@ func TestTransactionSampleRate(t *testing.T) {
tracer.SetSampler(apm.NewRatioSampler(test.actualSampleRate))
tx := tracer.StartTransactionOptions("name", "type", apm.TransactionOptions{
// Use a known transaction ID for deterministic sampling.
TransactionID: apm.SpanID{1, 2, 3, 4, 5, 6, 7, 8},
TransactionID: apm.SpanID{0, 1, 2, 3, 4, 5, 6, 7},
})
tx.End()
tracer.Flush(nil)
Expand Down

0 comments on commit 5949a1a

Please sign in to comment.