From 5949a1a8d8e7d02d8ea5cfbb28ab2e15df4fd52d Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Mon, 12 Oct 2020 15:28:38 +0800 Subject: [PATCH] Update sample rate precision handling (#834) * 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 --- sampler.go | 24 +++++++++++++++++++----- scripts/before_install.sh | 1 + span_test.go | 8 ++++---- tracecontext.go | 8 ++++---- transaction.go | 2 +- transaction_test.go | 7 +++++-- 6 files changed, 34 insertions(+), 16 deletions(-) diff --git a/sampler.go b/sampler.go index 011941fc9..88c254ad1 100644 --- a/sampler.go +++ b/sampler.go @@ -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 } @@ -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)) @@ -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 +} diff --git a/scripts/before_install.sh b/scripts/before_install.sh index 76e2b35e2..78539221a 100755 --- a/scripts/before_install.sh +++ b/scripts/before_install.sh @@ -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 diff --git a/span_test.go b/span_test.go index 805e0d3e6..73602f929 100644 --- a/span_test.go +++ b/span_test.go @@ -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. @@ -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) } diff --git a/tracecontext.go b/tracecontext.go index dc6ab86af..5ea5d7354 100644 --- a/tracecontext.go +++ b/tracecontext.go @@ -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) } diff --git a/transaction.go b/transaction.go index 2facab40d..9d10a6886 100644 --- a/transaction.go +++ b/transaction.go @@ -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), diff --git a/transaction_test.go b/transaction_test.go index fa0813ca1..d625516e5 100644 --- a/transaction_test.go +++ b/transaction_test.go @@ -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 @@ -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)