From bf06b79526994874ce980b0207eca4a394325414 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Wed, 27 Oct 2021 13:33:39 -0700 Subject: [PATCH 01/34] Second prototype (based on OTel-Go draft PR #2177 from main repo) --- samplers/probability/consistent/go.mod | 10 + samplers/probability/consistent/go.sum | 20 + samplers/probability/consistent/sampling.go | 366 ++++++++++++++++++ .../probability/consistent/sampling_test.go | 150 +++++++ 4 files changed, 546 insertions(+) create mode 100644 samplers/probability/consistent/go.mod create mode 100644 samplers/probability/consistent/go.sum create mode 100644 samplers/probability/consistent/sampling.go create mode 100644 samplers/probability/consistent/sampling_test.go diff --git a/samplers/probability/consistent/go.mod b/samplers/probability/consistent/go.mod new file mode 100644 index 00000000000..21fe56aeb1e --- /dev/null +++ b/samplers/probability/consistent/go.mod @@ -0,0 +1,10 @@ +module go.opentelemetry.io/contrib/samplers/probability/consistent + +go 1.15 + +require ( + github.com/stretchr/testify v1.7.0 + go.opentelemetry.io/otel v1.0.1 + go.opentelemetry.io/otel/sdk v1.0.1 + go.opentelemetry.io/otel/trace v1.0.1 +) diff --git a/samplers/probability/consistent/go.sum b/samplers/probability/consistent/go.sum new file mode 100644 index 00000000000..ba30ee2b1fb --- /dev/null +++ b/samplers/probability/consistent/go.sum @@ -0,0 +1,20 @@ +github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +go.opentelemetry.io/otel v1.0.1 h1:4XKyXmfqJLOQ7feyV5DB6gsBFZ0ltB8vLtp6pj4JIcc= +go.opentelemetry.io/otel v1.0.1/go.mod h1:OPEOD4jIT2SlZPMmwT6FqZz2C0ZNdQqiWcoK6M0SNFU= +go.opentelemetry.io/otel/sdk v1.0.1 h1:wXxFEWGo7XfXupPwVJvTBOaPBC9FEg0wB8hMNrKk+cA= +go.opentelemetry.io/otel/sdk v1.0.1/go.mod h1:HrdXne+BiwsOHYYkBE5ysIcv2bvdZstxzmCQhxTcZkI= +go.opentelemetry.io/otel/trace v1.0.1 h1:StTeIH6Q3G4r0Fiw34LTokUFESZgIDUr0qIJ7mKmAfw= +go.opentelemetry.io/otel/trace v1.0.1/go.mod h1:5g4i4fKLaX2BQpSBsxw8YYcgKpMMSW3x7ZTuYBr3sUk= +golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7 h1:iGu644GcxtEcrInvDsQRCwJjtCIOlT2V7IRt6ah2Whw= +golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/samplers/probability/consistent/sampling.go b/samplers/probability/consistent/sampling.go new file mode 100644 index 00000000000..4f5c71a0ed1 --- /dev/null +++ b/samplers/probability/consistent/sampling.go @@ -0,0 +1,366 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package consistent // import "go.opentelemetry.io/contrib/samplers/probability/consistent" + +import ( + "fmt" + "math" + "math/bits" + "math/rand" + "strconv" + "strings" + "sync" + + "go.opentelemetry.io/otel" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" +) + +const ( + traceStateKey = "ot" + pValueSubkey = "p" + rValueSubkey = "r" + pZeroValue = 63 + valueNumberBase = 10 + valueBitWidth = 6 +) + +var ( + errTraceStateSyntax = fmt.Errorf("otel tracestate: %w", strconv.ErrSyntax) +) + +type ( + ConsistentProbabilityBasedOption interface { + apply(*consistentProbabilityBasedConfig) + } + + consistentProbabilityBasedConfig struct { + source rand.Source + } + + consistentProbabilityBasedRandomSource struct { + rand.Source + } + + consistentProbabilityBased struct { + // "LAC" is an abbreviation for the logarithm of + // adjusted count. Greater values have greater + // representivity, therefore lesser sampling + // probability. + + // lowLAC is the lower-probability log-adjusted count + lowLAC uint8 + // highLAC is the higher-probability log-adjusted + // count. except for the zero probability special + // case, highLAC == lowLAC - 1. + highLAC uint8 + // lowProb is the probability that lowLAC should be used, + // in the interval (0, 1]. For exact powers of two and the + // special case of 0 probability, lowProb == 1. + lowProb float64 + + // lock protects rnd + lock sync.Mutex + rnd *rand.Rand + } + + otelTraceState struct { + rvalue uint8 // valid in the interval [0, 62] + pvalue uint8 // valid in the interval [0, 63] + unknown []string + } +) + +// WithRandomSource sets the source of the random number used in this +// prototype of OTEP 170, which assumes the randomness is not derived +// from the TraceID. +func WithRandomSource(source rand.Source) ConsistentProbabilityBasedOption { + return consistentProbabilityBasedRandomSource{source} +} + +func (s consistentProbabilityBasedRandomSource) apply(cfg *consistentProbabilityBasedConfig) { + cfg.source = s.Source +} + +// These are IEEE 754 double-width floating point constants used with +// math.Float64bits. +const ( + offsetExponentMask = 0x7ff0000000000000 + offsetExponentBias = 1023 + significandBits = 52 +) + +// expFromFloat64 returns floor(log2(x)). +func expFromFloat64(x float64) int { + return int((math.Float64bits(x)&offsetExponentMask)>>significandBits) - offsetExponentBias +} + +// expToFloat64 returns 2^x. +func expToFloat64(x int) float64 { + return math.Float64frombits(uint64(offsetExponentBias+x) << significandBits) +} + +// splitProb returns the two values of log-adjusted-count nearest to p +// Example: +// +// splitProb(0.375) => (2, 1, 0.5) +// +// indicates to sample with probability (2^-2) 50% of the time +// and (2^-1) 50% of the time. +func splitProb(p float64) (uint8, uint8, float64) { + if p < 2e-62 { + return pZeroValue, pZeroValue, 1 + } + // Take the exponent and drop the significand to locate the + // smaller of two powers of two. + exp := expFromFloat64(p) + + // Low is the smaller of two log-adjusted counts, the negative + // of the exponent computed above. + low := -exp + // High is the greater of two log-adjusted counts (i.e., one + // less than low, a smaller adjusted count means a larger + // probability). + high := low - 1 + + // Return these to probability values and use linear + // interpolation to compute the required probability of + // choosing the low-probability Sampler. + lowP := expToFloat64(-low) + highP := expToFloat64(-high) + lowProb := (highP - p) / (highP - lowP) + + return uint8(low), uint8(high), lowProb +} + +// ConsistentProbabilityBased samples a given fraction of traces. Based on the +// OpenTelemetry specification, this Sampler supports only power-of-two +// fractions. When the input fraction is not a power of two, it will +// be rounded down. +// - Fractions >= 1 will always sample. +// - Fractions < 2^-62 are treated as zero. +// +// This Sampler sets the `sampler.adjusted_count` attribute. +// +// To respect the parent trace's `SampledFlag`, this sampler should be +// used as the root delegate of a `Parent` sampler. +func ConsistentProbabilityBased(fraction float64, opts ...ConsistentProbabilityBasedOption) sdktrace.Sampler { + cfg := consistentProbabilityBasedConfig{} + for _, opt := range opts { + opt.apply(&cfg) + } + + if fraction < 0 { + fraction = 0 + } else if fraction > 1 { + fraction = 1 + } + + lowLAC, highLAC, lowProb := splitProb(fraction) + + return &consistentProbabilityBased{ + lowLAC: lowLAC, + highLAC: highLAC, + lowProb: lowProb, + rnd: rand.New(cfg.source), + } +} + +func (cs *consistentProbabilityBased) newR() uint8 { + cs.lock.Lock() + defer cs.lock.Unlock() + return uint8(bits.LeadingZeros64(uint64(cs.rnd.Int63())) - 1) +} + +func (cs *consistentProbabilityBased) lowChoice() bool { + cs.lock.Lock() + defer cs.lock.Unlock() + return cs.rnd.Float64() < cs.lowProb +} + +func newTraceState() otelTraceState { + return otelTraceState{ + rvalue: 64, // out-of-range => !hasRValue() + pvalue: 64, // out-of-range => !hasPValue() + } +} + +// ShouldSample implements Sampler. +func (cs *consistentProbabilityBased) ShouldSample(p sdktrace.SamplingParameters) sdktrace.SamplingResult { + psc := trace.SpanContextFromContext(p.ParentContext) + var otts otelTraceState + var state trace.TraceState + + if !psc.IsValid() { + // A new root is happening. Compute the r-value. + otts = newTraceState() + otts.rvalue = cs.newR() + } else { + // A valid parent context. + // It does not matter if psc.IsSampled(). + state = psc.TraceState() + + var err error + otts, err = parseOTelTraceState(state.Get(traceStateKey)) + + if err != nil { + otel.Handle(err) + } + + // @@@ if !hasRandom() { } + } + + var decision sdktrace.SamplingDecision + var lac uint8 + + if cs.lowProb == 1 || cs.lowChoice() { + lac = cs.lowLAC + } else { + lac = cs.highLAC + } + + if lac <= otts.rvalue { + decision = sdktrace.RecordAndSample + } else { + decision = sdktrace.Drop + } + + otts.pvalue = lac + + state, err := state.Insert(traceStateKey, otts.serialize()) + if err != nil { + otel.Handle(err) + } + + return sdktrace.SamplingResult{ + Decision: decision, + Tracestate: state, + } +} + +// Description implements Sampler. +func (cs *consistentProbabilityBased) Description() string { + var prob float64 + if cs.lowLAC != pZeroValue { + prob = cs.lowProb * expToFloat64(-int(cs.lowLAC)) + prob += (1 - cs.lowProb) * expToFloat64(-int(cs.highLAC)) + } + return fmt.Sprintf("ConsistentProbabilityBased{%g}", prob) +} + +func (otts otelTraceState) serialize() string { + var sb strings.Builder + if otts.hasPValue() { + _, _ = sb.WriteString(fmt.Sprintf("p:%02d;", otts.pvalue)) + } + if otts.hasRValue() { + _, _ = sb.WriteString(fmt.Sprintf("r:%02d;", otts.rvalue)) + } + for _, unk := range otts.unknown { + _, _ = sb.WriteString(unk) + _, _ = sb.WriteString(";") + } + res := sb.String() + if len(res) != 0 { + res = res[:len(res)-1] + } + return res +} + +func parseError(key string, err error) error { + return fmt.Errorf("otel tracestate: %s-value %w", key, err) +} + +func parseOTelTraceState(ts string) (otelTraceState, error) { + // TODO: Limits to apply from the spec? + otts := newTraceState() + for len(ts) > 0 { + eqPos := 0 + for ; eqPos < len(ts); eqPos++ { + if ts[eqPos] >= 'a' && ts[eqPos] <= 'z' { + continue + } + break + } + if eqPos == 0 || eqPos == len(ts) || ts[eqPos] != ':' { + return newTraceState(), errTraceStateSyntax + } + + key := ts[0:eqPos] + tail := ts[eqPos+1:] + + sepPos := 0 + + if key == pValueSubkey || key == rValueSubkey { + + for ; sepPos < len(tail); sepPos++ { + if tail[sepPos] >= '0' && tail[sepPos] <= '9' { + continue + } + break + } + value, err := strconv.ParseUint( + tail[0:sepPos], + valueNumberBase, + valueBitWidth, + ) + if err != nil { + return newTraceState(), parseError(key, err) + } + if key == pValueSubkey { + if value > pZeroValue { + return newTraceState(), parseError(key, strconv.ErrRange) + } + otts.pvalue = uint8(value) + } else if key == rValueSubkey { + if value > (pZeroValue - 1) { + return newTraceState(), parseError(key, strconv.ErrRange) + } + otts.rvalue = uint8(value) + } + + } else { + // Note: Spec valid character set for forward compatibility. + // Should this be the base64 characters? + // @@@@@@ @@@ lcalnum + for ; sepPos < len(tail); sepPos++ { + if tail[sepPos] >= '0' && tail[sepPos] <= '9' { + continue + } + } + otts.unknown = append(otts.unknown, ts[0:sepPos]) + } + + if sepPos == 0 || (sepPos < len(tail) && tail[sepPos] != ';') { + return newTraceState(), errTraceStateSyntax + } + + if sepPos == len(tail) { + break + } + + ts = tail[sepPos+1:] + } + + return otts, nil +} + +func (otts otelTraceState) hasRValue() bool { + return otts.rvalue < pZeroValue +} + +func (otts otelTraceState) hasPValue() bool { + return otts.pvalue <= pZeroValue +} diff --git a/samplers/probability/consistent/sampling_test.go b/samplers/probability/consistent/sampling_test.go new file mode 100644 index 00000000000..1fd80912563 --- /dev/null +++ b/samplers/probability/consistent/sampling_test.go @@ -0,0 +1,150 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package consistent + +import ( + "errors" + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSplitProb(t *testing.T) { + require.Equal(t, -1, expFromFloat64(0.6)) + require.Equal(t, -2, expFromFloat64(0.4)) + require.Equal(t, 0.5, expToFloat64(-1)) + require.Equal(t, 0.25, expToFloat64(-2)) + + for _, tc := range []struct { + in float64 + low uint8 + lowProb float64 + }{ + // Probability 0.75 corresponds with choosing S=1 (the + // "low" probability) 50% of the time and S=0 (the + // "high" probability) 50% of the time. + {0.75, 1, 0.5}, + {0.6, 1, 0.8}, + {0.9, 1, 0.2}, + + // Powers of 2 exactly + {1, 0, 1}, + {0.5, 1, 1}, + {0.25, 2, 1}, + + // Smaller numbers + {0.05, 5, 0.4}, + {0.1, 4, 0.4}, // 0.1 == 0.4 * 1/16 + 0.6 * 1/8 + {0.003, 9, 0.464}, + + // Special cases: + {0, 63, 1}, + } { + low, high, lowProb := splitProb(tc.in) + require.Equal(t, tc.low, low, "got %v want %v", low, tc.low) + if lowProb != 1 { + require.Equal(t, tc.low-1, high, "got %v want %v", high, tc.low-1) + } + require.InEpsilon(t, tc.lowProb, lowProb, 1e-6, "got %v want %v", lowProb, tc.lowProb) + } +} + +func TestDescription(t *testing.T) { + for _, tc := range []struct { + prob float64 + expect string + }{ + {0.75, "ConsistentProbabilityBased{0.75}"}, + {0.05, "ConsistentProbabilityBased{0.05}"}, + {0.003, "ConsistentProbabilityBased{0.003}"}, + {0.99999999, "ConsistentProbabilityBased{0.99999999}"}, + {0.00000001, "ConsistentProbabilityBased{1e-08}"}, + {1, "ConsistentProbabilityBased{1}"}, + {0, "ConsistentProbabilityBased{0}"}, + } { + s := ConsistentProbabilityBased(tc.prob) + require.Equal(t, tc.expect, s.Description()) + } +} + +func TestNewTraceState(t *testing.T) { + otts := newTraceState() + require.False(t, otts.hasPValue()) + require.False(t, otts.hasRValue()) + require.Equal(t, "", otts.serialize()) +} + +func TestParseTraceState(t *testing.T) { + type testCase struct { + in string + pval, rval uint8 + expectErr error + } + const notset = 255 + for _, test := range []testCase{ + {"r:1;p:2", 2, 1, nil}, + {"r:1;p:2;", 2, 1, nil}, + {"p:2;r:1;", 2, 1, nil}, + {"p:2;r:1", 2, 1, nil}, + {"r:1;", notset, 1, nil}, + {"r:1", notset, 1, nil}, + {"r:1=p:2", notset, notset, strconv.ErrSyntax}, + {"r:1;p:2=s:3", notset, notset, strconv.ErrSyntax}, + {":1;p:2=s:3", notset, notset, strconv.ErrSyntax}, + {":;p:2=s:3", notset, notset, strconv.ErrSyntax}, + {":;:", notset, notset, strconv.ErrSyntax}, + {":", notset, notset, strconv.ErrSyntax}, + {"", notset, notset, nil}, + {"r:", notset, notset, strconv.ErrSyntax}, + {"r:;p=1", notset, notset, strconv.ErrSyntax}, + {"r:1", notset, 1, nil}, + {"r:10", notset, 10, nil}, + {"r:33", notset, 33, nil}, + {"r:61", notset, 61, nil}, + {"r:62", notset, 62, nil}, // max r-value + {"r:63", notset, notset, strconv.ErrRange}, // out-of-range + {"r:100", notset, notset, strconv.ErrRange}, // out-of-range + {"r:100001", notset, notset, strconv.ErrRange}, // out-of-range + {"p:1", 1, notset, nil}, + {"p:62", 62, notset, nil}, + {"p:63", 63, notset, nil}, + {"p:64", notset, notset, strconv.ErrRange}, + {"p:100", notset, notset, strconv.ErrRange}, + {"r:1a", notset, notset, strconv.ErrSyntax}, // not-hexadecimal + {"p:-1", notset, notset, strconv.ErrSyntax}, // non-negative + } { + t.Run(strings.NewReplacer(":", "_", ";", "_").Replace(test.in), func(t *testing.T) { + otts, err := parseOTelTraceState(test.in) + + if test.expectErr != nil { + require.True(t, errors.Is(err, test.expectErr), "not expecting %v", err) + } + if test.pval != notset { + require.True(t, otts.hasPValue()) + require.Equal(t, test.pval, otts.pvalue) + } else { + require.False(t, otts.hasPValue(), "should have no p-value") + } + if test.rval != notset { + require.True(t, otts.hasRValue()) + require.Equal(t, test.rval, otts.rvalue) + } else { + require.False(t, otts.hasRValue(), "should have no r-value") + } + }) + } +} From b17b13c71c11f9383d62616550588ec488a84b51 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Wed, 27 Oct 2021 14:29:54 -0700 Subject: [PATCH 02/34] comments --- samplers/probability/consistent/sampling.go | 25 ++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/samplers/probability/consistent/sampling.go b/samplers/probability/consistent/sampling.go index 4f5c71a0ed1..c8707c3f2c7 100644 --- a/samplers/probability/consistent/sampling.go +++ b/samplers/probability/consistent/sampling.go @@ -211,15 +211,20 @@ func (cs *consistentProbabilityBased) ShouldSample(p sdktrace.SamplingParameters // A valid parent context. // It does not matter if psc.IsSampled(). state = psc.TraceState() + otts.clearPValue() var err error otts, err = parseOTelTraceState(state.Get(traceStateKey)) if err != nil { + // Note: we've reset trace state. otel.Handle(err) } - // @@@ if !hasRandom() { } + if !otts.hasRValue() { + // Specification says to set r-value if missing. + otts.rvalue = cs.newR() + } } var decision sdktrace.SamplingDecision @@ -242,6 +247,7 @@ func (cs *consistentProbabilityBased) ShouldSample(p sdktrace.SamplingParameters state, err := state.Insert(traceStateKey, otts.serialize()) if err != nil { otel.Handle(err) + // TODO: Spec how to handle this. } return sdktrace.SamplingResult{ @@ -273,6 +279,7 @@ func (otts otelTraceState) serialize() string { _, _ = sb.WriteString(";") } res := sb.String() + // Disregard a trailing `;` if len(res) != 0 { res = res[:len(res)-1] } @@ -285,6 +292,8 @@ func parseError(key string, err error) error { func parseOTelTraceState(ts string) (otelTraceState, error) { // TODO: Limits to apply from the spec? + // TODO: Key syntax + // TODO: Value syntax otts := newTraceState() for len(ts) > 0 { eqPos := 0 @@ -304,7 +313,9 @@ func parseOTelTraceState(ts string) (otelTraceState, error) { sepPos := 0 if key == pValueSubkey || key == rValueSubkey { - + // See TODOs above. Have one syntax check, + // then let ParseUint return ErrSyntax if need + // be. for ; sepPos < len(tail); sepPos++ { if tail[sepPos] >= '0' && tail[sepPos] <= '9' { continue @@ -334,9 +345,9 @@ func parseOTelTraceState(ts string) (otelTraceState, error) { } else { // Note: Spec valid character set for forward compatibility. // Should this be the base64 characters? - // @@@@@@ @@@ lcalnum for ; sepPos < len(tail); sepPos++ { if tail[sepPos] >= '0' && tail[sepPos] <= '9' { + // See TODOs above. continue } } @@ -364,3 +375,11 @@ func (otts otelTraceState) hasRValue() bool { func (otts otelTraceState) hasPValue() bool { return otts.pvalue <= pZeroValue } + +func (otts otelTraceState) clearRValue() { + otts.rvalue = pZeroValue +} + +func (otts otelTraceState) clearPValue() { + otts.pvalue = pZeroValue + 1 +} From 852c24207b398879bc4e68915ebec34967808b37 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Wed, 27 Oct 2021 16:33:41 -0700 Subject: [PATCH 03/34] add a test for sampled vs r/p-value consistency --- samplers/probability/consistent/base2.go | 69 ++++ samplers/probability/consistent/base2_test.go | 61 +++ samplers/probability/consistent/parent.go | 66 +++ samplers/probability/consistent/sampler.go | 188 +++++++++ .../probability/consistent/sampler_test.go | 39 ++ samplers/probability/consistent/sampling.go | 385 ------------------ samplers/probability/consistent/tracestate.go | 198 +++++++++ .../{sampling_test.go => tracestate_test.go} | 123 +++--- 8 files changed, 684 insertions(+), 445 deletions(-) create mode 100644 samplers/probability/consistent/base2.go create mode 100644 samplers/probability/consistent/base2_test.go create mode 100644 samplers/probability/consistent/parent.go create mode 100644 samplers/probability/consistent/sampler.go create mode 100644 samplers/probability/consistent/sampler_test.go delete mode 100644 samplers/probability/consistent/sampling.go create mode 100644 samplers/probability/consistent/tracestate.go rename samplers/probability/consistent/{sampling_test.go => tracestate_test.go} (60%) diff --git a/samplers/probability/consistent/base2.go b/samplers/probability/consistent/base2.go new file mode 100644 index 00000000000..62f559c35da --- /dev/null +++ b/samplers/probability/consistent/base2.go @@ -0,0 +1,69 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package consistent // import "go.opentelemetry.io/contrib/samplers/probability/consistent" + +import "math" + +// These are IEEE 754 double-width floating point constants used with +// math.Float64bits. +const ( + offsetExponentMask = 0x7ff0000000000000 + offsetExponentBias = 1023 + significandBits = 52 +) + +// expFromFloat64 returns floor(log2(x)). +func expFromFloat64(x float64) int { + return int((math.Float64bits(x)&offsetExponentMask)>>significandBits) - offsetExponentBias +} + +// expToFloat64 returns 2^x. +func expToFloat64(x int) float64 { + return math.Float64frombits(uint64(offsetExponentBias+x) << significandBits) +} + +// splitProb returns the two values of log-adjusted-count nearest to p +// Example: +// +// splitProb(0.375) => (2, 1, 0.5) +// +// indicates to sample with probability (2^-2) 50% of the time +// and (2^-1) 50% of the time. +func splitProb(p float64) (uint8, uint8, float64) { + if p < 2e-62 { + // Note: spec. + return pZeroValue, pZeroValue, 1 + } + // Take the exponent and drop the significand to locate the + // smaller of two powers of two. + exp := expFromFloat64(p) + + // Low is the smaller of two log-adjusted counts, the negative + // of the exponent computed above. + low := -exp + // High is the greater of two log-adjusted counts (i.e., one + // less than low, a smaller adjusted count means a larger + // probability). + high := low - 1 + + // Return these to probability values and use linear + // interpolation to compute the required probability of + // choosing the low-probability Sampler. + lowP := expToFloat64(-low) + highP := expToFloat64(-high) + lowProb := (highP - p) / (highP - lowP) + + return uint8(low), uint8(high), lowProb +} diff --git a/samplers/probability/consistent/base2_test.go b/samplers/probability/consistent/base2_test.go new file mode 100644 index 00000000000..9dad5c46f22 --- /dev/null +++ b/samplers/probability/consistent/base2_test.go @@ -0,0 +1,61 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package consistent + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSplitProb(t *testing.T) { + require.Equal(t, -1, expFromFloat64(0.6)) + require.Equal(t, -2, expFromFloat64(0.4)) + require.Equal(t, 0.5, expToFloat64(-1)) + require.Equal(t, 0.25, expToFloat64(-2)) + + for _, tc := range []struct { + in float64 + low uint8 + lowProb float64 + }{ + // Probability 0.75 corresponds with choosing S=1 (the + // "low" probability) 50% of the time and S=0 (the + // "high" probability) 50% of the time. + {0.75, 1, 0.5}, + {0.6, 1, 0.8}, + {0.9, 1, 0.2}, + + // Powers of 2 exactly + {1, 0, 1}, + {0.5, 1, 1}, + {0.25, 2, 1}, + + // Smaller numbers + {0.05, 5, 0.4}, + {0.1, 4, 0.4}, // 0.1 == 0.4 * 1/16 + 0.6 * 1/8 + {0.003, 9, 0.464}, + + // Special cases: + {0, 63, 1}, + } { + low, high, lowProb := splitProb(tc.in) + require.Equal(t, tc.low, low, "got %v want %v", low, tc.low) + if lowProb != 1 { + require.Equal(t, tc.low-1, high, "got %v want %v", high, tc.low-1) + } + require.InEpsilon(t, tc.lowProb, lowProb, 1e-6, "got %v want %v", lowProb, tc.lowProb) + } +} diff --git a/samplers/probability/consistent/parent.go b/samplers/probability/consistent/parent.go new file mode 100644 index 00000000000..d853423413f --- /dev/null +++ b/samplers/probability/consistent/parent.go @@ -0,0 +1,66 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package consistent // import "go.opentelemetry.io/contrib/samplers/probability/consistent" + +import ( + "strings" + + "go.opentelemetry.io/otel" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" +) + +type ( + parentProbabilitySampler struct { + delegate sdktrace.Sampler + } +) + +func ConsistentParentProbabilityBased(root sdktrace.Sampler, samplers ...sdktrace.ParentBasedSamplerOption) sdktrace.Sampler { + return &parentProbabilitySampler{ + delegate: sdktrace.ParentBased(root, samplers...), + } +} + +func (p *parentProbabilitySampler) ShouldSample(params sdktrace.SamplingParameters) sdktrace.SamplingResult { + psc := trace.SpanContextFromContext(params.ParentContext) + + if !psc.IsValid() { + return p.delegate.ShouldSample(params) + } + + state := psc.TraceState() + + otts, err := parseOTelTraceState(state.Get(traceStateKey), psc.IsSampled()) + + if err != nil { + otel.Handle(err) + state.Insert(traceStateKey, otts.serialize()) + + // Fix the broken tracestate before calling the delegate. + params.ParentContext = + trace.ContextWithSpanContext(params.ParentContext, psc.WithTraceState(state)) + } + + return p.delegate.ShouldSample(params) +} + +func (p *parentProbabilitySampler) Description() string { + s := p.delegate.Description() + if strings.HasPrefix(s, "ParentBased") { + s = s[len("ParentBased"):] + } + return "ConsistentParentProbabilityBased" + s +} diff --git a/samplers/probability/consistent/sampler.go b/samplers/probability/consistent/sampler.go new file mode 100644 index 00000000000..6f67605ea8f --- /dev/null +++ b/samplers/probability/consistent/sampler.go @@ -0,0 +1,188 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package consistent // import "go.opentelemetry.io/contrib/samplers/probability/consistent" + +import ( + "fmt" + "math/bits" + "math/rand" + "sync" + + "go.opentelemetry.io/otel" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" +) + +type ( + ConsistentProbabilityBasedOption interface { + apply(*consistentProbabilityBasedConfig) + } + + consistentProbabilityBasedConfig struct { + source rand.Source + } + + consistentProbabilityBasedRandomSource struct { + rand.Source + } + + consistentProbabilityBased struct { + // "LAC" is an abbreviation for the logarithm of + // adjusted count. Greater values have greater + // representivity, therefore lesser sampling + // probability. + + // lowLAC is the lower-probability log-adjusted count + lowLAC uint8 + // highLAC is the higher-probability log-adjusted + // count. except for the zero probability special + // case, highLAC == lowLAC - 1. + highLAC uint8 + // lowProb is the probability that lowLAC should be used, + // in the interval (0, 1]. For exact powers of two and the + // special case of 0 probability, lowProb == 1. + lowProb float64 + + // lock protects rnd + lock sync.Mutex + rnd *rand.Rand + } +) + +// WithRandomSource sets the source of the random number used in this +// prototype of OTEP 170, which assumes the randomness is not derived +// from the TraceID. +func WithRandomSource(source rand.Source) ConsistentProbabilityBasedOption { + return consistentProbabilityBasedRandomSource{source} +} + +func (s consistentProbabilityBasedRandomSource) apply(cfg *consistentProbabilityBasedConfig) { + cfg.source = s.Source +} + +// ConsistentProbabilityBased samples a given fraction of traces. Based on the +// OpenTelemetry specification, this Sampler supports only power-of-two +// fractions. When the input fraction is not a power of two, it will +// be rounded down. +// - Fractions >= 1 will always sample. +// - Fractions < 2^-62 are treated as zero. +// +// This Sampler sets the `sampler.adjusted_count` attribute. +// +// To respect the parent trace's `SampledFlag`, this sampler should be +// used as the root delegate of a `Parent` sampler. +func ConsistentProbabilityBased(fraction float64, opts ...ConsistentProbabilityBasedOption) sdktrace.Sampler { + cfg := consistentProbabilityBasedConfig{} + for _, opt := range opts { + opt.apply(&cfg) + } + + if fraction < 0 { + fraction = 0 + } else if fraction > 1 { + fraction = 1 + } + + lowLAC, highLAC, lowProb := splitProb(fraction) + + return &consistentProbabilityBased{ + lowLAC: lowLAC, + highLAC: highLAC, + lowProb: lowProb, + rnd: rand.New(cfg.source), + } +} + +func (cs *consistentProbabilityBased) newR() uint8 { + cs.lock.Lock() + defer cs.lock.Unlock() + return uint8(bits.LeadingZeros64(uint64(cs.rnd.Int63())) - 1) +} + +func (cs *consistentProbabilityBased) lowChoice() bool { + cs.lock.Lock() + defer cs.lock.Unlock() + return cs.rnd.Float64() < cs.lowProb +} + +// ShouldSample implements Sampler. +func (cs *consistentProbabilityBased) ShouldSample(p sdktrace.SamplingParameters) sdktrace.SamplingResult { + psc := trace.SpanContextFromContext(p.ParentContext) + var otts otelTraceState + var state trace.TraceState + + if !psc.IsValid() { + // A new root is happening. Compute the r-value. + otts = newTraceState() + otts.rvalue = cs.newR() + } else { + // A valid parent context. + state = psc.TraceState() + + var err error + otts, err = parseOTelTraceState(state.Get(traceStateKey), psc.IsSampled()) + + if err != nil { + // Note: a state.Insert(traceStateKey) + // follows, nothing else needs to be done here. + otel.Handle(err) + } + + if !otts.hasRValue() { + // Specification says to set r-value if missing. + otts.rvalue = cs.newR() + } + } + + var decision sdktrace.SamplingDecision + var lac uint8 + + if cs.lowProb == 1 || cs.lowChoice() { + lac = cs.lowLAC + } else { + lac = cs.highLAC + } + + if lac <= otts.rvalue { + decision = sdktrace.RecordAndSample + } else { + decision = sdktrace.Drop + } + + otts.pvalue = lac + + state, err := state.Insert(traceStateKey, otts.serialize()) + if err != nil { + otel.Handle(err) + // Note: see the note in + // "go.opentelemetry.io/otel/trace".TraceState.Insert() + // this is not a condition we're supposed to handle. + } + + return sdktrace.SamplingResult{ + Decision: decision, + Tracestate: state, + } +} + +// Description implements Sampler. +func (cs *consistentProbabilityBased) Description() string { + var prob float64 + if cs.lowLAC != pZeroValue { + prob = cs.lowProb * expToFloat64(-int(cs.lowLAC)) + prob += (1 - cs.lowProb) * expToFloat64(-int(cs.highLAC)) + } + return fmt.Sprintf("ConsistentProbabilityBased{%g}", prob) +} diff --git a/samplers/probability/consistent/sampler_test.go b/samplers/probability/consistent/sampler_test.go new file mode 100644 index 00000000000..af6f2ae97a7 --- /dev/null +++ b/samplers/probability/consistent/sampler_test.go @@ -0,0 +1,39 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package consistent + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestDescription(t *testing.T) { + for _, tc := range []struct { + prob float64 + expect string + }{ + {0.75, "ConsistentProbabilityBased{0.75}"}, + {0.05, "ConsistentProbabilityBased{0.05}"}, + {0.003, "ConsistentProbabilityBased{0.003}"}, + {0.99999999, "ConsistentProbabilityBased{0.99999999}"}, + {0.00000001, "ConsistentProbabilityBased{1e-08}"}, + {1, "ConsistentProbabilityBased{1}"}, + {0, "ConsistentProbabilityBased{0}"}, + } { + s := ConsistentProbabilityBased(tc.prob) + require.Equal(t, tc.expect, s.Description()) + } +} diff --git a/samplers/probability/consistent/sampling.go b/samplers/probability/consistent/sampling.go deleted file mode 100644 index c8707c3f2c7..00000000000 --- a/samplers/probability/consistent/sampling.go +++ /dev/null @@ -1,385 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package consistent // import "go.opentelemetry.io/contrib/samplers/probability/consistent" - -import ( - "fmt" - "math" - "math/bits" - "math/rand" - "strconv" - "strings" - "sync" - - "go.opentelemetry.io/otel" - sdktrace "go.opentelemetry.io/otel/sdk/trace" - "go.opentelemetry.io/otel/trace" -) - -const ( - traceStateKey = "ot" - pValueSubkey = "p" - rValueSubkey = "r" - pZeroValue = 63 - valueNumberBase = 10 - valueBitWidth = 6 -) - -var ( - errTraceStateSyntax = fmt.Errorf("otel tracestate: %w", strconv.ErrSyntax) -) - -type ( - ConsistentProbabilityBasedOption interface { - apply(*consistentProbabilityBasedConfig) - } - - consistentProbabilityBasedConfig struct { - source rand.Source - } - - consistentProbabilityBasedRandomSource struct { - rand.Source - } - - consistentProbabilityBased struct { - // "LAC" is an abbreviation for the logarithm of - // adjusted count. Greater values have greater - // representivity, therefore lesser sampling - // probability. - - // lowLAC is the lower-probability log-adjusted count - lowLAC uint8 - // highLAC is the higher-probability log-adjusted - // count. except for the zero probability special - // case, highLAC == lowLAC - 1. - highLAC uint8 - // lowProb is the probability that lowLAC should be used, - // in the interval (0, 1]. For exact powers of two and the - // special case of 0 probability, lowProb == 1. - lowProb float64 - - // lock protects rnd - lock sync.Mutex - rnd *rand.Rand - } - - otelTraceState struct { - rvalue uint8 // valid in the interval [0, 62] - pvalue uint8 // valid in the interval [0, 63] - unknown []string - } -) - -// WithRandomSource sets the source of the random number used in this -// prototype of OTEP 170, which assumes the randomness is not derived -// from the TraceID. -func WithRandomSource(source rand.Source) ConsistentProbabilityBasedOption { - return consistentProbabilityBasedRandomSource{source} -} - -func (s consistentProbabilityBasedRandomSource) apply(cfg *consistentProbabilityBasedConfig) { - cfg.source = s.Source -} - -// These are IEEE 754 double-width floating point constants used with -// math.Float64bits. -const ( - offsetExponentMask = 0x7ff0000000000000 - offsetExponentBias = 1023 - significandBits = 52 -) - -// expFromFloat64 returns floor(log2(x)). -func expFromFloat64(x float64) int { - return int((math.Float64bits(x)&offsetExponentMask)>>significandBits) - offsetExponentBias -} - -// expToFloat64 returns 2^x. -func expToFloat64(x int) float64 { - return math.Float64frombits(uint64(offsetExponentBias+x) << significandBits) -} - -// splitProb returns the two values of log-adjusted-count nearest to p -// Example: -// -// splitProb(0.375) => (2, 1, 0.5) -// -// indicates to sample with probability (2^-2) 50% of the time -// and (2^-1) 50% of the time. -func splitProb(p float64) (uint8, uint8, float64) { - if p < 2e-62 { - return pZeroValue, pZeroValue, 1 - } - // Take the exponent and drop the significand to locate the - // smaller of two powers of two. - exp := expFromFloat64(p) - - // Low is the smaller of two log-adjusted counts, the negative - // of the exponent computed above. - low := -exp - // High is the greater of two log-adjusted counts (i.e., one - // less than low, a smaller adjusted count means a larger - // probability). - high := low - 1 - - // Return these to probability values and use linear - // interpolation to compute the required probability of - // choosing the low-probability Sampler. - lowP := expToFloat64(-low) - highP := expToFloat64(-high) - lowProb := (highP - p) / (highP - lowP) - - return uint8(low), uint8(high), lowProb -} - -// ConsistentProbabilityBased samples a given fraction of traces. Based on the -// OpenTelemetry specification, this Sampler supports only power-of-two -// fractions. When the input fraction is not a power of two, it will -// be rounded down. -// - Fractions >= 1 will always sample. -// - Fractions < 2^-62 are treated as zero. -// -// This Sampler sets the `sampler.adjusted_count` attribute. -// -// To respect the parent trace's `SampledFlag`, this sampler should be -// used as the root delegate of a `Parent` sampler. -func ConsistentProbabilityBased(fraction float64, opts ...ConsistentProbabilityBasedOption) sdktrace.Sampler { - cfg := consistentProbabilityBasedConfig{} - for _, opt := range opts { - opt.apply(&cfg) - } - - if fraction < 0 { - fraction = 0 - } else if fraction > 1 { - fraction = 1 - } - - lowLAC, highLAC, lowProb := splitProb(fraction) - - return &consistentProbabilityBased{ - lowLAC: lowLAC, - highLAC: highLAC, - lowProb: lowProb, - rnd: rand.New(cfg.source), - } -} - -func (cs *consistentProbabilityBased) newR() uint8 { - cs.lock.Lock() - defer cs.lock.Unlock() - return uint8(bits.LeadingZeros64(uint64(cs.rnd.Int63())) - 1) -} - -func (cs *consistentProbabilityBased) lowChoice() bool { - cs.lock.Lock() - defer cs.lock.Unlock() - return cs.rnd.Float64() < cs.lowProb -} - -func newTraceState() otelTraceState { - return otelTraceState{ - rvalue: 64, // out-of-range => !hasRValue() - pvalue: 64, // out-of-range => !hasPValue() - } -} - -// ShouldSample implements Sampler. -func (cs *consistentProbabilityBased) ShouldSample(p sdktrace.SamplingParameters) sdktrace.SamplingResult { - psc := trace.SpanContextFromContext(p.ParentContext) - var otts otelTraceState - var state trace.TraceState - - if !psc.IsValid() { - // A new root is happening. Compute the r-value. - otts = newTraceState() - otts.rvalue = cs.newR() - } else { - // A valid parent context. - // It does not matter if psc.IsSampled(). - state = psc.TraceState() - otts.clearPValue() - - var err error - otts, err = parseOTelTraceState(state.Get(traceStateKey)) - - if err != nil { - // Note: we've reset trace state. - otel.Handle(err) - } - - if !otts.hasRValue() { - // Specification says to set r-value if missing. - otts.rvalue = cs.newR() - } - } - - var decision sdktrace.SamplingDecision - var lac uint8 - - if cs.lowProb == 1 || cs.lowChoice() { - lac = cs.lowLAC - } else { - lac = cs.highLAC - } - - if lac <= otts.rvalue { - decision = sdktrace.RecordAndSample - } else { - decision = sdktrace.Drop - } - - otts.pvalue = lac - - state, err := state.Insert(traceStateKey, otts.serialize()) - if err != nil { - otel.Handle(err) - // TODO: Spec how to handle this. - } - - return sdktrace.SamplingResult{ - Decision: decision, - Tracestate: state, - } -} - -// Description implements Sampler. -func (cs *consistentProbabilityBased) Description() string { - var prob float64 - if cs.lowLAC != pZeroValue { - prob = cs.lowProb * expToFloat64(-int(cs.lowLAC)) - prob += (1 - cs.lowProb) * expToFloat64(-int(cs.highLAC)) - } - return fmt.Sprintf("ConsistentProbabilityBased{%g}", prob) -} - -func (otts otelTraceState) serialize() string { - var sb strings.Builder - if otts.hasPValue() { - _, _ = sb.WriteString(fmt.Sprintf("p:%02d;", otts.pvalue)) - } - if otts.hasRValue() { - _, _ = sb.WriteString(fmt.Sprintf("r:%02d;", otts.rvalue)) - } - for _, unk := range otts.unknown { - _, _ = sb.WriteString(unk) - _, _ = sb.WriteString(";") - } - res := sb.String() - // Disregard a trailing `;` - if len(res) != 0 { - res = res[:len(res)-1] - } - return res -} - -func parseError(key string, err error) error { - return fmt.Errorf("otel tracestate: %s-value %w", key, err) -} - -func parseOTelTraceState(ts string) (otelTraceState, error) { - // TODO: Limits to apply from the spec? - // TODO: Key syntax - // TODO: Value syntax - otts := newTraceState() - for len(ts) > 0 { - eqPos := 0 - for ; eqPos < len(ts); eqPos++ { - if ts[eqPos] >= 'a' && ts[eqPos] <= 'z' { - continue - } - break - } - if eqPos == 0 || eqPos == len(ts) || ts[eqPos] != ':' { - return newTraceState(), errTraceStateSyntax - } - - key := ts[0:eqPos] - tail := ts[eqPos+1:] - - sepPos := 0 - - if key == pValueSubkey || key == rValueSubkey { - // See TODOs above. Have one syntax check, - // then let ParseUint return ErrSyntax if need - // be. - for ; sepPos < len(tail); sepPos++ { - if tail[sepPos] >= '0' && tail[sepPos] <= '9' { - continue - } - break - } - value, err := strconv.ParseUint( - tail[0:sepPos], - valueNumberBase, - valueBitWidth, - ) - if err != nil { - return newTraceState(), parseError(key, err) - } - if key == pValueSubkey { - if value > pZeroValue { - return newTraceState(), parseError(key, strconv.ErrRange) - } - otts.pvalue = uint8(value) - } else if key == rValueSubkey { - if value > (pZeroValue - 1) { - return newTraceState(), parseError(key, strconv.ErrRange) - } - otts.rvalue = uint8(value) - } - - } else { - // Note: Spec valid character set for forward compatibility. - // Should this be the base64 characters? - for ; sepPos < len(tail); sepPos++ { - if tail[sepPos] >= '0' && tail[sepPos] <= '9' { - // See TODOs above. - continue - } - } - otts.unknown = append(otts.unknown, ts[0:sepPos]) - } - - if sepPos == 0 || (sepPos < len(tail) && tail[sepPos] != ';') { - return newTraceState(), errTraceStateSyntax - } - - if sepPos == len(tail) { - break - } - - ts = tail[sepPos+1:] - } - - return otts, nil -} - -func (otts otelTraceState) hasRValue() bool { - return otts.rvalue < pZeroValue -} - -func (otts otelTraceState) hasPValue() bool { - return otts.pvalue <= pZeroValue -} - -func (otts otelTraceState) clearRValue() { - otts.rvalue = pZeroValue -} - -func (otts otelTraceState) clearPValue() { - otts.pvalue = pZeroValue + 1 -} diff --git a/samplers/probability/consistent/tracestate.go b/samplers/probability/consistent/tracestate.go new file mode 100644 index 00000000000..70f135ab33d --- /dev/null +++ b/samplers/probability/consistent/tracestate.go @@ -0,0 +1,198 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package consistent // import "go.opentelemetry.io/contrib/samplers/probability/consistent" + +import ( + "fmt" + "strconv" + "strings" +) + +const ( + traceStateKey = "ot" + pValueSubkey = "p" + rValueSubkey = "r" + pZeroValue = 63 + traceStateSizeLimit = 256 +) + +var ( + errTraceStateSyntax = fmt.Errorf("otel tracestate: %w", strconv.ErrSyntax) + errTraceStateInconsistent = fmt.Errorf("r-value and p-value are inconsistent") +) + +type otelTraceState struct { + rvalue uint8 // valid in the interval [0, 62] + pvalue uint8 // valid in the interval [0, 63] + unknown []string +} + +func newTraceState() otelTraceState { + return otelTraceState{ + rvalue: pZeroValue + 1, // out-of-range => !hasRValue() + pvalue: pZeroValue + 1, // out-of-range => !hasPValue() + } +} + +func (otts otelTraceState) serialize() string { + var sb strings.Builder + semi := func() { + if sb.Len() != 0 { + _, _ = sb.WriteString(";") + } + } + + if otts.hasPValue() { + _, _ = sb.WriteString(fmt.Sprintf("p:%02d", otts.pvalue)) + } + if otts.hasRValue() { + semi() + _, _ = sb.WriteString(fmt.Sprintf("r:%02d", otts.rvalue)) + } + for _, unk := range otts.unknown { + ex := 0 + if sb.Len() != 0 { + ex = 1 + } + if sb.Len()+ex+len(unk) > traceStateSizeLimit { + break + } + semi() + _, _ = sb.WriteString(unk) + } + return sb.String() +} + +func isLCAlphaNum(r byte) bool { + if isLCAlpha(r) { + return true + } + return r >= '0' && r <= '9' +} + +func isLCAlpha(r byte) bool { + return r >= 'a' && r <= 'z' +} + +func parseOTelTraceState(ts string, isSampled bool) (otelTraceState, error) { + var pval, rval string + var unknown []string + + if len(ts) > traceStateSizeLimit { + return newTraceState(), errTraceStateSyntax + } + + for len(ts) > 0 { + eqPos := 0 + for ; eqPos < len(ts); eqPos++ { + if eqPos == 0 && isLCAlpha(ts[eqPos]) { + continue + } else if isLCAlphaNum(ts[eqPos]) { + continue + } + break + } + if eqPos == 0 || eqPos == len(ts) || ts[eqPos] != ':' { + return newTraceState(), errTraceStateSyntax + } + + key := ts[0:eqPos] + tail := ts[eqPos+1:] + + sepPos := 0 + + for ; sepPos < len(tail); sepPos++ { + if isLCAlphaNum(tail[sepPos]) { + continue + } + break + } + + if key == pValueSubkey { + // Note: does the spec say how to handle duplicates? + pval = tail[0:sepPos] + } else if key == rValueSubkey { + rval = tail[0:sepPos] + } else { + unknown = append(unknown, ts[0:sepPos]) + } + + if sepPos == 0 || (sepPos < len(tail) && tail[sepPos] != ';') { + return newTraceState(), errTraceStateSyntax + } + + if sepPos == len(tail) { + break + } + + ts = tail[sepPos+1:] + } + + otts := newTraceState() + otts.unknown = unknown + + // Note: set R before P, so that P won't propagate if R has an error. + if value, err := parseNumber(rValueSubkey, rval, pZeroValue-1); err != nil { + return otts, err + } else { + otts.rvalue = value + } + + if value, err := parseNumber(pValueSubkey, pval, pZeroValue); err != nil { + return otts, err + } else { + otts.pvalue = value + } + + // Invariant checking: unset P when the values are inconsistent. + if otts.hasPValue() && otts.hasRValue() { + implied := otts.pvalue <= otts.rvalue || otts.pvalue == pZeroValue + + if implied != isSampled { + otts.pvalue = pZeroValue + 1 + // Note: the error ensures the parent-based + // sampler repairs the broken tracestate entry. + return otts, parseError(pValueSubkey, errTraceStateInconsistent) + } + } + + return otts, nil +} + +func parseNumber(key string, input string, maximum uint8) (uint8, error) { + if input == "" { + return maximum + 1, nil + } + value, err := strconv.ParseUint(input, 10, 64) + if err != nil { + return maximum + 1, parseError(key, err) + } + if value > uint64(maximum) { + return maximum + 1, parseError(key, strconv.ErrRange) + } + return uint8(value), nil +} + +func parseError(key string, err error) error { + return fmt.Errorf("otel tracestate: %s-value %w", key, err) +} + +func (otts otelTraceState) hasRValue() bool { + return otts.rvalue < pZeroValue +} + +func (otts otelTraceState) hasPValue() bool { + return otts.pvalue <= pZeroValue +} diff --git a/samplers/probability/consistent/sampling_test.go b/samplers/probability/consistent/tracestate_test.go similarity index 60% rename from samplers/probability/consistent/sampling_test.go rename to samplers/probability/consistent/tracestate_test.go index 1fd80912563..a2087988348 100644 --- a/samplers/probability/consistent/sampling_test.go +++ b/samplers/probability/consistent/tracestate_test.go @@ -23,64 +23,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestSplitProb(t *testing.T) { - require.Equal(t, -1, expFromFloat64(0.6)) - require.Equal(t, -2, expFromFloat64(0.4)) - require.Equal(t, 0.5, expToFloat64(-1)) - require.Equal(t, 0.25, expToFloat64(-2)) - - for _, tc := range []struct { - in float64 - low uint8 - lowProb float64 - }{ - // Probability 0.75 corresponds with choosing S=1 (the - // "low" probability) 50% of the time and S=0 (the - // "high" probability) 50% of the time. - {0.75, 1, 0.5}, - {0.6, 1, 0.8}, - {0.9, 1, 0.2}, - - // Powers of 2 exactly - {1, 0, 1}, - {0.5, 1, 1}, - {0.25, 2, 1}, - - // Smaller numbers - {0.05, 5, 0.4}, - {0.1, 4, 0.4}, // 0.1 == 0.4 * 1/16 + 0.6 * 1/8 - {0.003, 9, 0.464}, - - // Special cases: - {0, 63, 1}, - } { - low, high, lowProb := splitProb(tc.in) - require.Equal(t, tc.low, low, "got %v want %v", low, tc.low) - if lowProb != 1 { - require.Equal(t, tc.low-1, high, "got %v want %v", high, tc.low-1) - } - require.InEpsilon(t, tc.lowProb, lowProb, 1e-6, "got %v want %v", lowProb, tc.lowProb) - } -} - -func TestDescription(t *testing.T) { - for _, tc := range []struct { - prob float64 - expect string - }{ - {0.75, "ConsistentProbabilityBased{0.75}"}, - {0.05, "ConsistentProbabilityBased{0.05}"}, - {0.003, "ConsistentProbabilityBased{0.003}"}, - {0.99999999, "ConsistentProbabilityBased{0.99999999}"}, - {0.00000001, "ConsistentProbabilityBased{1e-08}"}, - {1, "ConsistentProbabilityBased{1}"}, - {0, "ConsistentProbabilityBased{0}"}, - } { - s := ConsistentProbabilityBased(tc.prob) - require.Equal(t, tc.expect, s.Description()) - } -} - func TestNewTraceState(t *testing.T) { otts := newTraceState() require.False(t, otts.hasPValue()) @@ -88,7 +30,7 @@ func TestNewTraceState(t *testing.T) { require.Equal(t, "", otts.serialize()) } -func TestParseTraceState(t *testing.T) { +func TestParseTraceStateUnsampled(t *testing.T) { type testCase struct { in string pval, rval uint8 @@ -96,6 +38,7 @@ func TestParseTraceState(t *testing.T) { } const notset = 255 for _, test := range []testCase{ + // All are unsampled tests, i.e., `sampled` is not set in traceparent. {"r:1;p:2", 2, 1, nil}, {"r:1;p:2;", 2, 1, nil}, {"p:2;r:1;", 2, 1, nil}, @@ -128,10 +71,70 @@ func TestParseTraceState(t *testing.T) { {"p:-1", notset, notset, strconv.ErrSyntax}, // non-negative } { t.Run(strings.NewReplacer(":", "_", ";", "_").Replace(test.in), func(t *testing.T) { - otts, err := parseOTelTraceState(test.in) + // Note: passing isSampled=false as stated above. + otts, err := parseOTelTraceState(test.in, false) + + if test.expectErr != nil { + require.True(t, errors.Is(err, test.expectErr), "not expecting %v", err) + } + if test.pval != notset { + require.True(t, otts.hasPValue()) + require.Equal(t, test.pval, otts.pvalue) + } else { + require.False(t, otts.hasPValue(), "should have no p-value") + } + if test.rval != notset { + require.True(t, otts.hasRValue()) + require.Equal(t, test.rval, otts.rvalue) + } else { + require.False(t, otts.hasRValue(), "should have no r-value") + } + }) + } +} + +func TestParseTraceStateSampled(t *testing.T) { + type testCase struct { + in string + rval, pval uint8 + expectErr error + } + const notset = 255 + for _, test := range []testCase{ + // All are sampled tests, i.e., `sampled` is set in traceparent. + {"r:2;p:2", 2, 2, nil}, + {"r:2;p:1", 2, 1, nil}, + {"r:2;p:0", 2, 0, nil}, + + {"r:1;p:1", 1, 1, nil}, + {"r:1;p:0", 1, 0, nil}, + + {"r:0;p:0", 0, 0, nil}, + + {"r:62;p:0", 62, 0, nil}, + {"r:62;p:62", 62, 62, nil}, + + // The important special case: + {"r:0;p:63", 0, 63, nil}, + {"r:2;p:63", 2, 63, nil}, + {"r:62;p:63", 62, 63, nil}, + + // Inconsistencies cause unset p-value. + {"r:2;p:3", 2, notset, errTraceStateInconsistent}, + {"r:2;p:4", 2, notset, errTraceStateInconsistent}, + {"r:2;p:62", 2, notset, errTraceStateInconsistent}, + {"r:0;p:1", 0, notset, errTraceStateInconsistent}, + {"r:1;p:2", 1, notset, errTraceStateInconsistent}, + {"r:61;p:62", 61, notset, errTraceStateInconsistent}, + } { + t.Run(strings.NewReplacer(":", "_", ";", "_").Replace(test.in), func(t *testing.T) { + // Note: passing isSampled=true as stated above. + otts, err := parseOTelTraceState(test.in, true) if test.expectErr != nil { require.True(t, errors.Is(err, test.expectErr), "not expecting %v", err) + } else { + require.NoError(t, err) } if test.pval != notset { require.True(t, otts.hasPValue()) From 7fc989b041002f73d00a96794940f6b3883a1c4a Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Thu, 28 Oct 2021 07:50:59 -0700 Subject: [PATCH 04/34] adding tests --- samplers/probability/consistent/go.mod | 6 +- samplers/probability/consistent/go.sum | 15 ++- samplers/probability/consistent/sampler.go | 4 +- .../probability/consistent/sampler_test.go | 20 ++- .../consistent/test/adjustedcount_test.go | 49 +++++++ samplers/probability/consistent/test/go.mod | 11 ++ samplers/probability/consistent/test/go.sum | 22 +++ .../probability/consistent/test/kolmogorov.go | 127 ++++++++++++++++++ .../consistent/test/kolmogorov_test.go | 28 ++++ 9 files changed, 269 insertions(+), 13 deletions(-) create mode 100644 samplers/probability/consistent/test/adjustedcount_test.go create mode 100644 samplers/probability/consistent/test/go.mod create mode 100644 samplers/probability/consistent/test/go.sum create mode 100644 samplers/probability/consistent/test/kolmogorov.go create mode 100644 samplers/probability/consistent/test/kolmogorov_test.go diff --git a/samplers/probability/consistent/go.mod b/samplers/probability/consistent/go.mod index 21fe56aeb1e..1023c2c9da1 100644 --- a/samplers/probability/consistent/go.mod +++ b/samplers/probability/consistent/go.mod @@ -4,7 +4,7 @@ go 1.15 require ( github.com/stretchr/testify v1.7.0 - go.opentelemetry.io/otel v1.0.1 - go.opentelemetry.io/otel/sdk v1.0.1 - go.opentelemetry.io/otel/trace v1.0.1 + go.opentelemetry.io/otel v1.1.0 + go.opentelemetry.io/otel/sdk v1.1.0 + go.opentelemetry.io/otel/trace v1.1.0 ) diff --git a/samplers/probability/consistent/go.sum b/samplers/probability/consistent/go.sum index ba30ee2b1fb..c1b1589324b 100644 --- a/samplers/probability/consistent/go.sum +++ b/samplers/probability/consistent/go.sum @@ -1,20 +1,23 @@ github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ= github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -go.opentelemetry.io/otel v1.0.1 h1:4XKyXmfqJLOQ7feyV5DB6gsBFZ0ltB8vLtp6pj4JIcc= -go.opentelemetry.io/otel v1.0.1/go.mod h1:OPEOD4jIT2SlZPMmwT6FqZz2C0ZNdQqiWcoK6M0SNFU= -go.opentelemetry.io/otel/sdk v1.0.1 h1:wXxFEWGo7XfXupPwVJvTBOaPBC9FEg0wB8hMNrKk+cA= -go.opentelemetry.io/otel/sdk v1.0.1/go.mod h1:HrdXne+BiwsOHYYkBE5ysIcv2bvdZstxzmCQhxTcZkI= -go.opentelemetry.io/otel/trace v1.0.1 h1:StTeIH6Q3G4r0Fiw34LTokUFESZgIDUr0qIJ7mKmAfw= -go.opentelemetry.io/otel/trace v1.0.1/go.mod h1:5g4i4fKLaX2BQpSBsxw8YYcgKpMMSW3x7ZTuYBr3sUk= +go.opentelemetry.io/otel v1.1.0 h1:8p0uMLcyyIx0KHNTgO8o3CW8A1aA+dJZJW6PvnMz0Wc= +go.opentelemetry.io/otel v1.1.0/go.mod h1:7cww0OW51jQ8IaZChIEdqLwgh+44+7uiTdWsAL0wQpA= +go.opentelemetry.io/otel/sdk v1.1.0 h1:j/1PngUJIDOddkCILQYTevrTIbWd494djgGkSsMit+U= +go.opentelemetry.io/otel/sdk v1.1.0/go.mod h1:3aQvM6uLm6C4wJpHtT8Od3vNzeZ34Pqc6bps8MywWzo= +go.opentelemetry.io/otel/trace v1.1.0 h1:N25T9qCL0+7IpOT8RrRy0WYlL7y6U0WiUJzXcVdXY/o= +go.opentelemetry.io/otel/trace v1.1.0/go.mod h1:i47XtdcBQiktu5IsrPqOHe8w+sBmnLwwHt8wiUsWGTI= golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7 h1:iGu644GcxtEcrInvDsQRCwJjtCIOlT2V7IRt6ah2Whw= golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/samplers/probability/consistent/sampler.go b/samplers/probability/consistent/sampler.go index 6f67605ea8f..8e145af3674 100644 --- a/samplers/probability/consistent/sampler.go +++ b/samplers/probability/consistent/sampler.go @@ -84,7 +84,9 @@ func (s consistentProbabilityBasedRandomSource) apply(cfg *consistentProbability // To respect the parent trace's `SampledFlag`, this sampler should be // used as the root delegate of a `Parent` sampler. func ConsistentProbabilityBased(fraction float64, opts ...ConsistentProbabilityBasedOption) sdktrace.Sampler { - cfg := consistentProbabilityBasedConfig{} + cfg := consistentProbabilityBasedConfig{ + source: rand.NewSource(rand.Int63()), + } for _, opt := range opts { opt.apply(&cfg) } diff --git a/samplers/probability/consistent/sampler_test.go b/samplers/probability/consistent/sampler_test.go index af6f2ae97a7..7c93c3d6025 100644 --- a/samplers/probability/consistent/sampler_test.go +++ b/samplers/probability/consistent/sampler_test.go @@ -21,19 +21,33 @@ import ( ) func TestDescription(t *testing.T) { + const minProb = 0x1p-62 // 2.168404344971009e-19 + for _, tc := range []struct { prob float64 expect string }{ + {1, "ConsistentProbabilityBased{1}"}, + {0, "ConsistentProbabilityBased{0}"}, {0.75, "ConsistentProbabilityBased{0.75}"}, {0.05, "ConsistentProbabilityBased{0.05}"}, {0.003, "ConsistentProbabilityBased{0.003}"}, {0.99999999, "ConsistentProbabilityBased{0.99999999}"}, {0.00000001, "ConsistentProbabilityBased{1e-08}"}, - {1, "ConsistentProbabilityBased{1}"}, - {0, "ConsistentProbabilityBased{0}"}, + {minProb, "ConsistentProbabilityBased{2.168404344971009e-19}"}, + {minProb * 1.5, "ConsistentProbabilityBased{3.2526065174565133e-19}"}, + {3e-19, "ConsistentProbabilityBased{3e-19}"}, + + // out-of-range > 1 + {1.01, "ConsistentProbabilityBased{1}"}, + {101.1, "ConsistentProbabilityBased{1}"}, + + // out-of-range < 2^-62 + {-1, "ConsistentProbabilityBased{0}"}, + {-0.001, "ConsistentProbabilityBased{0}"}, + {minProb * 0.999, "ConsistentProbabilityBased{0}"}, } { s := ConsistentProbabilityBased(tc.prob) - require.Equal(t, tc.expect, s.Description()) + require.Equal(t, tc.expect, s.Description(), "%#v", tc.prob) } } diff --git a/samplers/probability/consistent/test/adjustedcount_test.go b/samplers/probability/consistent/test/adjustedcount_test.go new file mode 100644 index 00000000000..3f8fb06ad84 --- /dev/null +++ b/samplers/probability/consistent/test/adjustedcount_test.go @@ -0,0 +1,49 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package test + +import ( + "math/rand" + "testing" + + "go.opentelemetry.io/contrib/samplers/probability/consistent" + sdktrace "go.opentelemetry.io/otel/sdk/trace" +) + +type testSpanRecorder struct { +} + +func newSource() rand.Source { + return rand.NewSource(77777677777) +} + +func TestAdjustedCount(t *testing.T) { + +} + +func testX(prob float64, source rand.Source) { + sampler := consistent.ConsistentProbabilityBased( + prob, + consistent.WithRandomSource(source), + ) + recorder := &testSpanRecorder{} + provider := sdktrace.NewTracerProvider( + sdktrace.WithSyncer(recorder), + sdktrace.WithSampler(sampler), + ) + _ = provider + _ = recorder + +} diff --git a/samplers/probability/consistent/test/go.mod b/samplers/probability/consistent/test/go.mod new file mode 100644 index 00000000000..b80f2305ef4 --- /dev/null +++ b/samplers/probability/consistent/test/go.mod @@ -0,0 +1,11 @@ +module go.opentelemetry.io/contrib/samplers/probability/consistent/test + +go 1.16 + +replace go.opentelemetry.io/contrib/samplers/probability/consistent => ../ + +require ( + github.com/stretchr/testify v1.7.0 + go.opentelemetry.io/contrib/samplers/probability/consistent v0.0.0-00010101000000-000000000000 + go.opentelemetry.io/otel/sdk v1.1.0 +) diff --git a/samplers/probability/consistent/test/go.sum b/samplers/probability/consistent/test/go.sum new file mode 100644 index 00000000000..3c407bd9737 --- /dev/null +++ b/samplers/probability/consistent/test/go.sum @@ -0,0 +1,22 @@ +github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ= +github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +go.opentelemetry.io/otel v1.1.0 h1:8p0uMLcyyIx0KHNTgO8o3CW8A1aA+dJZJW6PvnMz0Wc= +go.opentelemetry.io/otel v1.1.0/go.mod h1:7cww0OW51jQ8IaZChIEdqLwgh+44+7uiTdWsAL0wQpA= +go.opentelemetry.io/otel/sdk v1.1.0 h1:j/1PngUJIDOddkCILQYTevrTIbWd494djgGkSsMit+U= +go.opentelemetry.io/otel/sdk v1.1.0/go.mod h1:3aQvM6uLm6C4wJpHtT8Od3vNzeZ34Pqc6bps8MywWzo= +go.opentelemetry.io/otel/trace v1.1.0 h1:N25T9qCL0+7IpOT8RrRy0WYlL7y6U0WiUJzXcVdXY/o= +go.opentelemetry.io/otel/trace v1.1.0/go.mod h1:i47XtdcBQiktu5IsrPqOHe8w+sBmnLwwHt8wiUsWGTI= +golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7 h1:iGu644GcxtEcrInvDsQRCwJjtCIOlT2V7IRt6ah2Whw= +golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/samplers/probability/consistent/test/kolmogorov.go b/samplers/probability/consistent/test/kolmogorov.go new file mode 100644 index 00000000000..2ae7c62d861 --- /dev/null +++ b/samplers/probability/consistent/test/kolmogorov.go @@ -0,0 +1,127 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package test + +import ( + "math" +) + +/* + Copied from the C program to compute Kolmogorov's distribution + + K(n,d) = Prob(D_n < d) + + where + + D_n = max(x_1-0/n,x_2-1/n...,x_n-(n-1)/n,1/n-x_1,2/n-x_2,...,n/n-x_n) + + with x_17 digit accuracy in the right tail. + s := d * d * float64(n) + if s > 7.24 || (s > 3.76 && n > 99) { + return 1 - 2*math.Exp(-(2.000071+.331/math.Sqrt(float64(n))+1.409/float64(n))*s) + } + + k := int((float64(n) * d) + 1) + m := 2*k - 1 + h := float64(k) - float64(n)*d + H := make([]float64, m*m) + Q := make([]float64, m*m) + for i := 0; i < m; i++ { + for j := 0; j < m; j++ { + if i-j+1 < 0 { + H[i*m+j] = 0 + } else { + H[i*m+j] = 1 + } + } + } + for i := 0; i < m; i++ { + H[i*m] -= math.Pow(h, float64(i+1)) + H[(m-1)*m+i] -= math.Pow(h, float64(m-i)) + } + if 2*h-1 > 0 { + H[(m-1)*m] += math.Pow(2*h-1, float64(m)) + } + for i := 0; i < m; i++ { + for j := 0; j < m; j++ { + if i-j+1 > 0 { + for g := 1; g <= i-j+1; g++ { + H[i*m+j] /= float64(g) + } + } + } + } + eQ := 0 + mPower(H, 0, Q, &eQ, m, n) + s = Q[(k-1)*m+k-1] + for i := 1; i <= n; i++ { + s = s * float64(i) / float64(n) + if s < 1e-140 { + s *= 1e140 + eQ -= 140 + } + } + s *= math.Pow(10, float64(eQ)) + return s +} + +func mMultiply(A, B, C []float64, m int) { + for i := 0; i < m; i++ { + for j := 0; j < m; j++ { + s := 0.0 + for k := 0; k < m; k++ { + s += A[i*m+k] * B[k*m+j] + } + C[i*m+j] = s + } + } +} + +func mPower(A []float64, eA int, V []float64, eV *int, m, n int) { + if n == 1 { + for i := 0; i < m*m; i++ { + V[i] = A[i] + } + *eV = eA + return + } + mPower(A, eA, V, eV, m, n/2) + B := make([]float64, m*m) + mMultiply(V, V, B, m) + eB := 2 * (*eV) + if n%2 == 0 { + for i := 0; i < m*m; i++ { + V[i] = B[i] + *eV = eB + } + } else { + mMultiply(A, B, V, m) + *eV = eA + eB + } + if V[(m/2)*m+(m/2)] > 1e140 { + for i := 0; i < m*m; i++ { + V[i] = V[i] * 1e-140 + } + *eV += 140 + } +} diff --git a/samplers/probability/consistent/test/kolmogorov_test.go b/samplers/probability/consistent/test/kolmogorov_test.go new file mode 100644 index 00000000000..d5a91c92c74 --- /dev/null +++ b/samplers/probability/consistent/test/kolmogorov_test.go @@ -0,0 +1,28 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package test + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestKolmogorov(t *testing.T) { + require.InEpsilon(t, 0.9999999996465492, kolmogorov(10, 1), 1e-15) + require.InEpsilon(t, 0.2512809600000001, kolmogorov(10, 0.2), 1e-15) + require.InEpsilon(t, 0.0467840289364274, kolmogorov(100, 0.05), 1e-15) + require.InEpsilon(t, 0.6345548933440742, kolmogorov(1000, 0.028933906713247914), 1e-15) +} From d6295d5d646cc775f181587ba8c26dc1fa2d004c Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Fri, 29 Oct 2021 00:15:43 -0700 Subject: [PATCH 05/34] test various D values --- samplers/probability/consistent/go.mod | 1 + samplers/probability/consistent/go.sum | 64 ++++++++++ .../consistent/test/adjustedcount_test.go | 119 ++++++++++++++++-- samplers/probability/consistent/test/go.sum | 22 ---- .../consistent/test/{go.mod => go_mod} | 2 + 5 files changed, 177 insertions(+), 31 deletions(-) delete mode 100644 samplers/probability/consistent/test/go.sum rename samplers/probability/consistent/test/{go.mod => go_mod} (82%) diff --git a/samplers/probability/consistent/go.mod b/samplers/probability/consistent/go.mod index 1023c2c9da1..ca49ae32841 100644 --- a/samplers/probability/consistent/go.mod +++ b/samplers/probability/consistent/go.mod @@ -7,4 +7,5 @@ require ( go.opentelemetry.io/otel v1.1.0 go.opentelemetry.io/otel/sdk v1.1.0 go.opentelemetry.io/otel/trace v1.1.0 + gonum.org/v1/gonum v0.9.3 ) diff --git a/samplers/probability/consistent/go.sum b/samplers/probability/consistent/go.sum index c1b1589324b..6d4fa56cbb7 100644 --- a/samplers/probability/consistent/go.sum +++ b/samplers/probability/consistent/go.sum @@ -1,10 +1,32 @@ +dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= +gioui.org v0.0.0-20210308172011-57750fc8a0a6/go.mod h1:RSH6KIUZ0p2xy5zHDxgAM4zumjgTw83q2ge/PI+yyw8= +github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= +github.com/ajstarks/svgo v0.0.0-20180226025133-644b8db467af/go.mod h1:K08gAheRH3/J6wwsYMMT4xOr94bZjxIelGM0+d/wbFw= +github.com/boombuler/barcode v1.0.0/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8= github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/fogleman/gg v1.2.1-0.20190220221249-0403632d5b90/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k= +github.com/fogleman/gg v1.3.0/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k= +github.com/go-fonts/dejavu v0.1.0/go.mod h1:4Wt4I4OU2Nq9asgDCteaAaWZOV24E+0/Pwo0gppep4g= +github.com/go-fonts/latin-modern v0.2.0/go.mod h1:rQVLdDMK+mK1xscDwsqM5J8U2jrRa3T0ecnM9pNujks= +github.com/go-fonts/liberation v0.1.1/go.mod h1:K6qoJYypsmfVjWg8KOVDQhLc8UDgIK2HYqyqAO9z7GY= +github.com/go-fonts/stix v0.1.0/go.mod h1:w/c1f0ldAUlJmLBvlbkvVXLAD+tAMqobIIQpmnUIzUY= +github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= +github.com/go-latex/latex v0.0.0-20210118124228-b3d85cf34e07/go.mod h1:CO1AlKB2CSIqUrmQPqA0gdRIlnLEY0gK5JGjh37zN5U= +github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0/go.mod h1:E/TSTwGwJL78qG/PmXZO1EjYhfJinVAhrmmHX6Z8B9k= github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ= github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/jung-kurt/gofpdf v1.0.0/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes= +github.com/jung-kurt/gofpdf v1.0.3-0.20190309125859-24315acbbda5/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes= +github.com/phpdave11/gofpdf v1.4.2/go.mod h1:zpO6xFn9yxo3YLyMvW8HcKWVdbNqgIfOOp2dXMnm1mY= +github.com/phpdave11/gofpdi v1.0.12/go.mod h1:vBmVV0Do6hSBHC8uKUQ71JGW+ZGQq74llk/7bXwjDoI= +github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/ruudk/golang-pdf417 v0.0.0-20181029194003-1af4ab5afa58/go.mod h1:6lfFZQK844Gfx8o5WFuvpxWRwnSoipWe/p622j1v06w= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= go.opentelemetry.io/otel v1.1.0 h1:8p0uMLcyyIx0KHNTgO8o3CW8A1aA+dJZJW6PvnMz0Wc= @@ -13,11 +35,53 @@ go.opentelemetry.io/otel/sdk v1.1.0 h1:j/1PngUJIDOddkCILQYTevrTIbWd494djgGkSsMit go.opentelemetry.io/otel/sdk v1.1.0/go.mod h1:3aQvM6uLm6C4wJpHtT8Od3vNzeZ34Pqc6bps8MywWzo= go.opentelemetry.io/otel/trace v1.1.0 h1:N25T9qCL0+7IpOT8RrRy0WYlL7y6U0WiUJzXcVdXY/o= go.opentelemetry.io/otel/trace v1.1.0/go.mod h1:i47XtdcBQiktu5IsrPqOHe8w+sBmnLwwHt8wiUsWGTI= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= +golang.org/x/exp v0.0.0-20180807140117-3d87b88a115f/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= +golang.org/x/exp v0.0.0-20190125153040-c74c464bbbf2/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= +golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= +golang.org/x/exp v0.0.0-20191002040644-a1355ae1e2c3 h1:n9HxLrNxWWtEb1cA950nuEEj3QnKbtsCJ6KjcgisNUs= +golang.org/x/exp v0.0.0-20191002040644-a1355ae1e2c3/go.mod h1:NOZ3BPKG0ec/BKJQgnvsSFpcKLM5xXVWnvZS97DWHgE= +golang.org/x/image v0.0.0-20180708004352-c73c2afc3b81/go.mod h1:ux5Hcp/YLpHSI86hEcLt0YII63i6oz57MZXIpbrjZUs= +golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= +golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= +golang.org/x/image v0.0.0-20190910094157-69e4b8554b2a/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= +golang.org/x/image v0.0.0-20200119044424-58c23975cae1/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= +golang.org/x/image v0.0.0-20200430140353-33d19683fad8/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= +golang.org/x/image v0.0.0-20200618115811-c13761719519/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= +golang.org/x/image v0.0.0-20201208152932-35266b937fa6/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= +golang.org/x/image v0.0.0-20210216034530-4410531fe030/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= +golang.org/x/mobile v0.0.0-20190719004257-d2bd2a29d028/go.mod h1:E/iHnbuqvinMTCcRqshq8CkpyQDoeVncDDYHnLhea+o= +golang.org/x/mod v0.1.0/go.mod h1:0QHyrYULN0/3qlju5TqG8bIK38QM8yzMo5ekMj3DlcY= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210304124612-50617c2ba197/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7 h1:iGu644GcxtEcrInvDsQRCwJjtCIOlT2V7IRt6ah2Whw= golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/tools v0.0.0-20180525024113-a5b4c53f6e8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20190206041539-40960b6deb8e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20190927191325-030b2cf1153e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +gonum.org/v1/gonum v0.0.0-20180816165407-929014505bf4/go.mod h1:Y+Yx5eoAFn32cQvJDxZx5Dpnq+c3wtXuadVZAcxbbBo= +gonum.org/v1/gonum v0.8.2/go.mod h1:oe/vMfY3deqTw+1EZJhuvEW2iwGF1bW9wwu7XCu0+v0= +gonum.org/v1/gonum v0.9.3 h1:DnoIG+QAMaF5NvxnGe/oKsgKcAc6PcUyl8q0VetfQ8s= +gonum.org/v1/gonum v0.9.3/go.mod h1:TZumC3NeyVQskjXqmyWt4S3bINhy7B4eYwW69EbyX+0= +gonum.org/v1/netlib v0.0.0-20190313105609-8cb42192e0e0 h1:OE9mWmgKkjJyEmDAAtGMPjXu+YNeGvK9VTSHY6+Qihc= +gonum.org/v1/netlib v0.0.0-20190313105609-8cb42192e0e0/go.mod h1:wa6Ws7BG/ESfp6dHfk7C6KdzKA7wR7u/rKwOGE66zvw= +gonum.org/v1/plot v0.0.0-20190515093506-e2840ee46a6b/go.mod h1:Wt8AAjI+ypCyYX3nZBvf6cAIx93T+c/OS2HFAYskSZc= +gonum.org/v1/plot v0.9.0/go.mod h1:3Pcqqmp6RHvJI72kgb8fThyUnav364FOsdDo2aGW5lY= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +rsc.io/pdf v0.1.1/go.mod h1:n8OzWcQ6Sp37PL01nO98y4iUCRdTGarVfzxY20ICaU4= diff --git a/samplers/probability/consistent/test/adjustedcount_test.go b/samplers/probability/consistent/test/adjustedcount_test.go index 3f8fb06ad84..224bdcb63ac 100644 --- a/samplers/probability/consistent/test/adjustedcount_test.go +++ b/samplers/probability/consistent/test/adjustedcount_test.go @@ -15,14 +15,19 @@ package test import ( + "context" + "fmt" "math/rand" + "sort" "testing" "go.opentelemetry.io/contrib/samplers/probability/consistent" sdktrace "go.opentelemetry.io/otel/sdk/trace" + "gonum.org/v1/gonum/stat/distuv" ) type testSpanRecorder struct { + spans []sdktrace.ReadOnlySpan } func newSource() rand.Source { @@ -30,20 +35,116 @@ func newSource() rand.Source { } func TestAdjustedCount(t *testing.T) { - + // testPowerOfTwo(t, 500, 100, 1000, 0.5, newSource()) + // testPowerOfTwo(t, 500, 100, 2000, 0.25, newSource()) + testPowerOfTwo(t, 5000, 100, 4000, 0.125, newSource()) } -func testX(prob float64, source rand.Source) { +func testPowerOfTwo(t *testing.T, repeats, trials, tosses int, prob float64, source rand.Source) { + + ctx := context.Background() + sampler := consistent.ConsistentProbabilityBased( prob, consistent.WithRandomSource(source), ) - recorder := &testSpanRecorder{} - provider := sdktrace.NewTracerProvider( - sdktrace.WithSyncer(recorder), - sdktrace.WithSampler(sampler), - ) - _ = provider - _ = recorder + modelDist := distuv.Binomial{ + N: float64(tosses), + P: prob, + } + + var dvalues []float64 + + for repeat := 0; repeat < repeats; repeat++ { + var results []int + + for trial := 0; trial < trials; trial++ { + recorder := &testSpanRecorder{} + provider := sdktrace.NewTracerProvider( + sdktrace.WithSyncer(recorder), + sdktrace.WithSampler(sampler), + ) + + tracer := provider.Tracer("test") + + recording := 0 + for i := 0; i < tosses; i++ { + _, span := tracer.Start(ctx, "span") + if span.IsRecording() { + recording++ + } + span.End() + } + results = append(results, recording) + } + + sort.Ints(results) + + // Like Knuth 3.3.1 algorithm B, one-sample, but without the + // sqrt(trials) term, thus using the exact Kolmogorov D + // distribution instead of K+ and K- like the text. + d := 0.0 + + for i := 0; i < trials; i++ { + for i < trials-1 && results[i+1] == results[i] { + i++ // Scanning past duplicates + } + + x := float64(results[i]) + low := float64(i+1) / float64(trials) + high := float64(i) / float64(trials) + + if dPlus := low - modelDist.CDF(x); dPlus > d { + d = dPlus + } + if dMinus := modelDist.CDF(x) - high; dMinus > d { + d = dMinus + } + } + + //fmt.Printf("K single D %f%%\n", 100*kolmogorov(trials, d)) + + dvalues = append(dvalues, d) + + //if len(dvalues)%step == 0 { + show(dvalues, trials) + //} + } +} + +func show(dvalues []float64, trials int) { + repeats := len(dvalues) + + sort.Float64s(dvalues) + + d := 0.0 + + for i := 0; i < repeats; i++ { + for i < repeats-1 && dvalues[i+1] == dvalues[i] { + i++ // Scanning past duplicates + } + + x := float64(dvalues[i]) + low := float64(i+1) / float64(repeats) + high := float64(i) / float64(repeats) + + if dPlus := low - kolmogorov(trials, x); dPlus > d { + d = dPlus + } + if dMinus := kolmogorov(trials, x) - high; dMinus > d { + d = dMinus + } + } + + fmt.Printf("K multi D %f%%\n", 100*kolmogorov(repeats, d)) +} + +func (tsr *testSpanRecorder) ExportSpans(ctx context.Context, spans []sdktrace.ReadOnlySpan) error { + tsr.spans = append(tsr.spans, spans...) + return nil +} + +func (tsr *testSpanRecorder) Shutdown(ctx context.Context) error { + return nil } diff --git a/samplers/probability/consistent/test/go.sum b/samplers/probability/consistent/test/go.sum deleted file mode 100644 index 3c407bd9737..00000000000 --- a/samplers/probability/consistent/test/go.sum +++ /dev/null @@ -1,22 +0,0 @@ -github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= -github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ= -github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= -github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= -github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -go.opentelemetry.io/otel v1.1.0 h1:8p0uMLcyyIx0KHNTgO8o3CW8A1aA+dJZJW6PvnMz0Wc= -go.opentelemetry.io/otel v1.1.0/go.mod h1:7cww0OW51jQ8IaZChIEdqLwgh+44+7uiTdWsAL0wQpA= -go.opentelemetry.io/otel/sdk v1.1.0 h1:j/1PngUJIDOddkCILQYTevrTIbWd494djgGkSsMit+U= -go.opentelemetry.io/otel/sdk v1.1.0/go.mod h1:3aQvM6uLm6C4wJpHtT8Od3vNzeZ34Pqc6bps8MywWzo= -go.opentelemetry.io/otel/trace v1.1.0 h1:N25T9qCL0+7IpOT8RrRy0WYlL7y6U0WiUJzXcVdXY/o= -go.opentelemetry.io/otel/trace v1.1.0/go.mod h1:i47XtdcBQiktu5IsrPqOHe8w+sBmnLwwHt8wiUsWGTI= -golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7 h1:iGu644GcxtEcrInvDsQRCwJjtCIOlT2V7IRt6ah2Whw= -golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= -gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/samplers/probability/consistent/test/go.mod b/samplers/probability/consistent/test/go_mod similarity index 82% rename from samplers/probability/consistent/test/go.mod rename to samplers/probability/consistent/test/go_mod index b80f2305ef4..4364da23f1f 100644 --- a/samplers/probability/consistent/test/go.mod +++ b/samplers/probability/consistent/test/go_mod @@ -7,5 +7,7 @@ replace go.opentelemetry.io/contrib/samplers/probability/consistent => ../ require ( github.com/stretchr/testify v1.7.0 go.opentelemetry.io/contrib/samplers/probability/consistent v0.0.0-00010101000000-000000000000 + go.opentelemetry.io/otel v1.1.0 go.opentelemetry.io/otel/sdk v1.1.0 + go.opentelemetry.io/otel/trace v1.1.0 ) From 63f36fb9a96de00dc0ad941a2e440d759e9c91ad Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Tue, 2 Nov 2021 02:07:07 -0700 Subject: [PATCH 06/34] test spec --- samplers/probability/consistent/go.mod | 1 - samplers/probability/consistent/go.sum | 64 ----- .../consistent/test/adjustedcount_test.go | 259 ++++++++++++------ samplers/probability/consistent/test/go_mod | 13 - .../probability/consistent/test/kolmogorov.go | 127 --------- .../consistent/test/kolmogorov_test.go | 28 -- 6 files changed, 175 insertions(+), 317 deletions(-) delete mode 100644 samplers/probability/consistent/test/go_mod delete mode 100644 samplers/probability/consistent/test/kolmogorov.go delete mode 100644 samplers/probability/consistent/test/kolmogorov_test.go diff --git a/samplers/probability/consistent/go.mod b/samplers/probability/consistent/go.mod index ca49ae32841..1023c2c9da1 100644 --- a/samplers/probability/consistent/go.mod +++ b/samplers/probability/consistent/go.mod @@ -7,5 +7,4 @@ require ( go.opentelemetry.io/otel v1.1.0 go.opentelemetry.io/otel/sdk v1.1.0 go.opentelemetry.io/otel/trace v1.1.0 - gonum.org/v1/gonum v0.9.3 ) diff --git a/samplers/probability/consistent/go.sum b/samplers/probability/consistent/go.sum index 6d4fa56cbb7..c1b1589324b 100644 --- a/samplers/probability/consistent/go.sum +++ b/samplers/probability/consistent/go.sum @@ -1,32 +1,10 @@ -dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= -gioui.org v0.0.0-20210308172011-57750fc8a0a6/go.mod h1:RSH6KIUZ0p2xy5zHDxgAM4zumjgTw83q2ge/PI+yyw8= -github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= -github.com/ajstarks/svgo v0.0.0-20180226025133-644b8db467af/go.mod h1:K08gAheRH3/J6wwsYMMT4xOr94bZjxIelGM0+d/wbFw= -github.com/boombuler/barcode v1.0.0/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8= github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/fogleman/gg v1.2.1-0.20190220221249-0403632d5b90/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k= -github.com/fogleman/gg v1.3.0/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k= -github.com/go-fonts/dejavu v0.1.0/go.mod h1:4Wt4I4OU2Nq9asgDCteaAaWZOV24E+0/Pwo0gppep4g= -github.com/go-fonts/latin-modern v0.2.0/go.mod h1:rQVLdDMK+mK1xscDwsqM5J8U2jrRa3T0ecnM9pNujks= -github.com/go-fonts/liberation v0.1.1/go.mod h1:K6qoJYypsmfVjWg8KOVDQhLc8UDgIK2HYqyqAO9z7GY= -github.com/go-fonts/stix v0.1.0/go.mod h1:w/c1f0ldAUlJmLBvlbkvVXLAD+tAMqobIIQpmnUIzUY= -github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= -github.com/go-latex/latex v0.0.0-20210118124228-b3d85cf34e07/go.mod h1:CO1AlKB2CSIqUrmQPqA0gdRIlnLEY0gK5JGjh37zN5U= -github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0/go.mod h1:E/TSTwGwJL78qG/PmXZO1EjYhfJinVAhrmmHX6Z8B9k= github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ= github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/jung-kurt/gofpdf v1.0.0/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes= -github.com/jung-kurt/gofpdf v1.0.3-0.20190309125859-24315acbbda5/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes= -github.com/phpdave11/gofpdf v1.4.2/go.mod h1:zpO6xFn9yxo3YLyMvW8HcKWVdbNqgIfOOp2dXMnm1mY= -github.com/phpdave11/gofpdi v1.0.12/go.mod h1:vBmVV0Do6hSBHC8uKUQ71JGW+ZGQq74llk/7bXwjDoI= -github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= -github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/ruudk/golang-pdf417 v0.0.0-20181029194003-1af4ab5afa58/go.mod h1:6lfFZQK844Gfx8o5WFuvpxWRwnSoipWe/p622j1v06w= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= go.opentelemetry.io/otel v1.1.0 h1:8p0uMLcyyIx0KHNTgO8o3CW8A1aA+dJZJW6PvnMz0Wc= @@ -35,53 +13,11 @@ go.opentelemetry.io/otel/sdk v1.1.0 h1:j/1PngUJIDOddkCILQYTevrTIbWd494djgGkSsMit go.opentelemetry.io/otel/sdk v1.1.0/go.mod h1:3aQvM6uLm6C4wJpHtT8Od3vNzeZ34Pqc6bps8MywWzo= go.opentelemetry.io/otel/trace v1.1.0 h1:N25T9qCL0+7IpOT8RrRy0WYlL7y6U0WiUJzXcVdXY/o= go.opentelemetry.io/otel/trace v1.1.0/go.mod h1:i47XtdcBQiktu5IsrPqOHe8w+sBmnLwwHt8wiUsWGTI= -golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= -golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= -golang.org/x/exp v0.0.0-20180807140117-3d87b88a115f/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= -golang.org/x/exp v0.0.0-20190125153040-c74c464bbbf2/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= -golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= -golang.org/x/exp v0.0.0-20191002040644-a1355ae1e2c3 h1:n9HxLrNxWWtEb1cA950nuEEj3QnKbtsCJ6KjcgisNUs= -golang.org/x/exp v0.0.0-20191002040644-a1355ae1e2c3/go.mod h1:NOZ3BPKG0ec/BKJQgnvsSFpcKLM5xXVWnvZS97DWHgE= -golang.org/x/image v0.0.0-20180708004352-c73c2afc3b81/go.mod h1:ux5Hcp/YLpHSI86hEcLt0YII63i6oz57MZXIpbrjZUs= -golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= -golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= -golang.org/x/image v0.0.0-20190910094157-69e4b8554b2a/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= -golang.org/x/image v0.0.0-20200119044424-58c23975cae1/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= -golang.org/x/image v0.0.0-20200430140353-33d19683fad8/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= -golang.org/x/image v0.0.0-20200618115811-c13761719519/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= -golang.org/x/image v0.0.0-20201208152932-35266b937fa6/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= -golang.org/x/image v0.0.0-20210216034530-4410531fe030/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= -golang.org/x/mobile v0.0.0-20190719004257-d2bd2a29d028/go.mod h1:E/iHnbuqvinMTCcRqshq8CkpyQDoeVncDDYHnLhea+o= -golang.org/x/mod v0.1.0/go.mod h1:0QHyrYULN0/3qlju5TqG8bIK38QM8yzMo5ekMj3DlcY= -golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= -golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210304124612-50617c2ba197/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7 h1:iGu644GcxtEcrInvDsQRCwJjtCIOlT2V7IRt6ah2Whw= golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/tools v0.0.0-20180525024113-a5b4c53f6e8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= -golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= -golang.org/x/tools v0.0.0-20190206041539-40960b6deb8e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= -golang.org/x/tools v0.0.0-20190927191325-030b2cf1153e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= -golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -gonum.org/v1/gonum v0.0.0-20180816165407-929014505bf4/go.mod h1:Y+Yx5eoAFn32cQvJDxZx5Dpnq+c3wtXuadVZAcxbbBo= -gonum.org/v1/gonum v0.8.2/go.mod h1:oe/vMfY3deqTw+1EZJhuvEW2iwGF1bW9wwu7XCu0+v0= -gonum.org/v1/gonum v0.9.3 h1:DnoIG+QAMaF5NvxnGe/oKsgKcAc6PcUyl8q0VetfQ8s= -gonum.org/v1/gonum v0.9.3/go.mod h1:TZumC3NeyVQskjXqmyWt4S3bINhy7B4eYwW69EbyX+0= -gonum.org/v1/netlib v0.0.0-20190313105609-8cb42192e0e0 h1:OE9mWmgKkjJyEmDAAtGMPjXu+YNeGvK9VTSHY6+Qihc= -gonum.org/v1/netlib v0.0.0-20190313105609-8cb42192e0e0/go.mod h1:wa6Ws7BG/ESfp6dHfk7C6KdzKA7wR7u/rKwOGE66zvw= -gonum.org/v1/plot v0.0.0-20190515093506-e2840ee46a6b/go.mod h1:Wt8AAjI+ypCyYX3nZBvf6cAIx93T+c/OS2HFAYskSZc= -gonum.org/v1/plot v0.9.0/go.mod h1:3Pcqqmp6RHvJI72kgb8fThyUnav364FOsdDo2aGW5lY= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -rsc.io/pdf v0.1.1/go.mod h1:n8OzWcQ6Sp37PL01nO98y4iUCRdTGarVfzxY20ICaU4= diff --git a/samplers/probability/consistent/test/adjustedcount_test.go b/samplers/probability/consistent/test/adjustedcount_test.go index 224bdcb63ac..c142f1e57ef 100644 --- a/samplers/probability/consistent/test/adjustedcount_test.go +++ b/samplers/probability/consistent/test/adjustedcount_test.go @@ -17,31 +17,67 @@ package test import ( "context" "fmt" + "math" "math/rand" - "sort" + "strconv" + "strings" "testing" + "github.com/stretchr/testify/require" "go.opentelemetry.io/contrib/samplers/probability/consistent" sdktrace "go.opentelemetry.io/otel/sdk/trace" - "gonum.org/v1/gonum/stat/distuv" ) +var ( + populationSize = int64(1e6) + trials = 20 + significance = 1 / float64(trials) + + // import "gonum.org/v1/gonum/stat/distuv" + // with significance = 0.05 + // chiSquaredDF1 = distuv.ChiSquared{K: 1}.Quantile(significance) + // chiSquaredDF2 = distuv.ChiSquared{K: 2}.Quantile(significance) + chiSquaredDF1 = 0.003932140000019522 + chiSquaredDF2 = 0.1025865887751011 + + chiSquaredByDF = [3]float64{ + 0, + chiSquaredDF1, + chiSquaredDF2, + } +) + +func init() { + fmt.Println("chi-squared", chiSquaredByDF) +} + +func parsePR(s string) (p, r string) { + for _, kvf := range strings.Split(s, ";") { + kv := strings.SplitN(kvf, ":", 2) + switch kv[0] { + case "p": + p = kv[1] + case "r": + r = kv[1] + } + } + return +} + type testSpanRecorder struct { spans []sdktrace.ReadOnlySpan } -func newSource() rand.Source { - return rand.NewSource(77777677777) +func (tsr *testSpanRecorder) ExportSpans(ctx context.Context, spans []sdktrace.ReadOnlySpan) error { + tsr.spans = append(tsr.spans, spans...) + return nil } -func TestAdjustedCount(t *testing.T) { - // testPowerOfTwo(t, 500, 100, 1000, 0.5, newSource()) - // testPowerOfTwo(t, 500, 100, 2000, 0.25, newSource()) - testPowerOfTwo(t, 5000, 100, 4000, 0.125, newSource()) +func (tsr *testSpanRecorder) Shutdown(ctx context.Context) error { + return nil } -func testPowerOfTwo(t *testing.T, repeats, trials, tosses int, prob float64, source rand.Source) { - +func sampleTrials(t *testing.T, prob float64, degrees, upperP, spans int64, source rand.Source) float64 { ctx := context.Background() sampler := consistent.ConsistentProbabilityBased( @@ -49,102 +85,157 @@ func testPowerOfTwo(t *testing.T, repeats, trials, tosses int, prob float64, sou consistent.WithRandomSource(source), ) - modelDist := distuv.Binomial{ - N: float64(tosses), - P: prob, - } - - var dvalues []float64 - - for repeat := 0; repeat < repeats; repeat++ { - var results []int - - for trial := 0; trial < trials; trial++ { - recorder := &testSpanRecorder{} - provider := sdktrace.NewTracerProvider( - sdktrace.WithSyncer(recorder), - sdktrace.WithSampler(sampler), - ) + recorder := &testSpanRecorder{} + provider := sdktrace.NewTracerProvider( + sdktrace.WithSyncer(recorder), + sdktrace.WithSampler(sampler), + ) - tracer := provider.Tracer("test") + tracer := provider.Tracer("test") - recording := 0 - for i := 0; i < tosses; i++ { - _, span := tracer.Start(ctx, "span") - if span.IsRecording() { - recording++ - } - span.End() - } - results = append(results, recording) - } + for i := int64(0); i < spans; i++ { + _, span := tracer.Start(ctx, "span") + span.End() + } - sort.Ints(results) + var minP, maxP int64 - // Like Knuth 3.3.1 algorithm B, one-sample, but without the - // sqrt(trials) term, thus using the exact Kolmogorov D - // distribution instead of K+ and K- like the text. - d := 0.0 + counts := map[int64]int64{} - for i := 0; i < trials; i++ { - for i < trials-1 && results[i+1] == results[i] { - i++ // Scanning past duplicates - } + for idx, r := range recorder.spans { + ts := r.SpanContext().TraceState() + p, _ := parsePR(ts.Get("ot")) - x := float64(results[i]) - low := float64(i+1) / float64(trials) - high := float64(i) / float64(trials) + pi, err := strconv.ParseUint(p, 10, 64) + require.NoError(t, err) - if dPlus := low - modelDist.CDF(x); dPlus > d { - d = dPlus + if idx == 0 { + maxP = int64(pi) + minP = maxP + } else { + if int64(pi) < minP { + minP = int64(pi) } - if dMinus := modelDist.CDF(x) - high; dMinus > d { - d = dMinus + if int64(pi) > maxP { + maxP = int64(pi) } } + counts[int64(pi)]++ + } - //fmt.Printf("K single D %f%%\n", 100*kolmogorov(trials, d)) + require.Less(t, maxP, minP+degrees, "%v %v %v", minP, maxP, degrees) + require.Less(t, maxP, int64(63)) + require.LessOrEqual(t, len(counts), 2) - dvalues = append(dvalues, d) + var upperProb, lowerProb, lowerChoice float64 - //if len(dvalues)%step == 0 { - show(dvalues, trials) - //} + if degrees == 2 { + if len(counts) != 0 { + require.Equal(t, minP+1, maxP) + require.Equal(t, upperP, maxP) + } + upperProb = 1 / float64(int64(1)< d { - d = dPlus - } - if dMinus := kolmogorov(trials, x) - high; dMinus > d { - d = dMinus - } + chi2 := 0.0 + chi2 += math.Pow(float64(unsampled)-expectUnsampled, 2) / expectUnsampled + chi2 += math.Pow(float64(lowerCount)-expectLowerCount, 2) / expectLowerCount + if degrees == 2 { + chi2 += math.Pow(float64(upperCount)-expectUpperCount, 2) / expectUpperCount } - fmt.Printf("K multi D %f%%\n", 100*kolmogorov(repeats, d)) + return chi2 } -func (tsr *testSpanRecorder) ExportSpans(ctx context.Context, spans []sdktrace.ReadOnlySpan) error { - tsr.spans = append(tsr.spans, spans...) - return nil +type probSeed struct { + prob float64 + upperP int64 + degrees int64 + seed int64 } -func (tsr *testSpanRecorder) Shutdown(ctx context.Context) error { - return nil +func TestPowerOfTwoSampling(t *testing.T) { + // Note that each of the seeds used in this test comes from the + // source below. The seed's position in the sequence is listed + // in the comments below. + newSource := func() rand.Source { + return rand.NewSource(77777677777) + } + for _, ps := range []probSeed{ + // Non-powers of two + {0.6, 1, 2, 0x7e436d5ff98928b8}, // 14th + {0.33, 2, 2, 0x3b7603f0b2596e2f}, // 4th + {0.1, 4, 2, 0x31fb79834649508c}, // 1st + {0.05, 5, 2, 0x31fb79834649508c}, // 1st + {0.01, 7, 2, 0x3b7603f0b2596e2f}, // 4th + {0.005, 8, 2, 0x222159400ee1080}, // 2nd + {0.001, 10, 2, 0x62e520750be95257}, // 6th + {0.0005, 11, 2, 0x222159400ee1080}, // 2nd + {0.0001, 14, 2, 0x53c7cf004663c656}, // 3rd + + // Powers of two + {0x1p-1, 1, 1, 0x31fb79834649508c}, // 1st + {0x1p-4, 4, 1, 0x53c7cf004663c656}, // 3rd + {0x1p-7, 7, 1, 0x3b7603f0b2596e2f}, // 4th + {0x1p-10, 10, 1, 0x31fb79834649508c}, // 1st + {0x1p-13, 13, 1, 0x222159400ee1080}, // 2nd + } { + t.Run(fmt.Sprint(ps.prob), func(t *testing.T) { + rnd := rand.New(newSource()) + + for { + seed := ps.seed + if seed == 0 { + seed = rnd.Int63() + } + src := rand.NewSource(seed) + less := 0 + + for j := 0; j < trials; j++ { + x := sampleTrials(t, ps.prob, ps.degrees, ps.upperP, populationSize, src) + + if x < chiSquaredByDF[ps.degrees] { + less++ + } + } + + if less != 1 && ps.seed == 0 { + t.Logf("%d probabilistic failures, trying a new seed for %g was 0x%x", less, ps.prob, seed) + continue + } else if less != 1 { + t.Errorf("incorrect number of probabilistic failures, should be 1 was %d", less) + } else if ps.seed == 0 { + t.Logf("update the test for %g to use seed 0x%x", ps.prob, seed) + t.Fail() + return + } else { + // pass + break + } + } + }) + } } diff --git a/samplers/probability/consistent/test/go_mod b/samplers/probability/consistent/test/go_mod deleted file mode 100644 index 4364da23f1f..00000000000 --- a/samplers/probability/consistent/test/go_mod +++ /dev/null @@ -1,13 +0,0 @@ -module go.opentelemetry.io/contrib/samplers/probability/consistent/test - -go 1.16 - -replace go.opentelemetry.io/contrib/samplers/probability/consistent => ../ - -require ( - github.com/stretchr/testify v1.7.0 - go.opentelemetry.io/contrib/samplers/probability/consistent v0.0.0-00010101000000-000000000000 - go.opentelemetry.io/otel v1.1.0 - go.opentelemetry.io/otel/sdk v1.1.0 - go.opentelemetry.io/otel/trace v1.1.0 -) diff --git a/samplers/probability/consistent/test/kolmogorov.go b/samplers/probability/consistent/test/kolmogorov.go deleted file mode 100644 index 2ae7c62d861..00000000000 --- a/samplers/probability/consistent/test/kolmogorov.go +++ /dev/null @@ -1,127 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package test - -import ( - "math" -) - -/* - Copied from the C program to compute Kolmogorov's distribution - - K(n,d) = Prob(D_n < d) - - where - - D_n = max(x_1-0/n,x_2-1/n...,x_n-(n-1)/n,1/n-x_1,2/n-x_2,...,n/n-x_n) - - with x_17 digit accuracy in the right tail. - s := d * d * float64(n) - if s > 7.24 || (s > 3.76 && n > 99) { - return 1 - 2*math.Exp(-(2.000071+.331/math.Sqrt(float64(n))+1.409/float64(n))*s) - } - - k := int((float64(n) * d) + 1) - m := 2*k - 1 - h := float64(k) - float64(n)*d - H := make([]float64, m*m) - Q := make([]float64, m*m) - for i := 0; i < m; i++ { - for j := 0; j < m; j++ { - if i-j+1 < 0 { - H[i*m+j] = 0 - } else { - H[i*m+j] = 1 - } - } - } - for i := 0; i < m; i++ { - H[i*m] -= math.Pow(h, float64(i+1)) - H[(m-1)*m+i] -= math.Pow(h, float64(m-i)) - } - if 2*h-1 > 0 { - H[(m-1)*m] += math.Pow(2*h-1, float64(m)) - } - for i := 0; i < m; i++ { - for j := 0; j < m; j++ { - if i-j+1 > 0 { - for g := 1; g <= i-j+1; g++ { - H[i*m+j] /= float64(g) - } - } - } - } - eQ := 0 - mPower(H, 0, Q, &eQ, m, n) - s = Q[(k-1)*m+k-1] - for i := 1; i <= n; i++ { - s = s * float64(i) / float64(n) - if s < 1e-140 { - s *= 1e140 - eQ -= 140 - } - } - s *= math.Pow(10, float64(eQ)) - return s -} - -func mMultiply(A, B, C []float64, m int) { - for i := 0; i < m; i++ { - for j := 0; j < m; j++ { - s := 0.0 - for k := 0; k < m; k++ { - s += A[i*m+k] * B[k*m+j] - } - C[i*m+j] = s - } - } -} - -func mPower(A []float64, eA int, V []float64, eV *int, m, n int) { - if n == 1 { - for i := 0; i < m*m; i++ { - V[i] = A[i] - } - *eV = eA - return - } - mPower(A, eA, V, eV, m, n/2) - B := make([]float64, m*m) - mMultiply(V, V, B, m) - eB := 2 * (*eV) - if n%2 == 0 { - for i := 0; i < m*m; i++ { - V[i] = B[i] - *eV = eB - } - } else { - mMultiply(A, B, V, m) - *eV = eA + eB - } - if V[(m/2)*m+(m/2)] > 1e140 { - for i := 0; i < m*m; i++ { - V[i] = V[i] * 1e-140 - } - *eV += 140 - } -} diff --git a/samplers/probability/consistent/test/kolmogorov_test.go b/samplers/probability/consistent/test/kolmogorov_test.go deleted file mode 100644 index d5a91c92c74..00000000000 --- a/samplers/probability/consistent/test/kolmogorov_test.go +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package test - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestKolmogorov(t *testing.T) { - require.InEpsilon(t, 0.9999999996465492, kolmogorov(10, 1), 1e-15) - require.InEpsilon(t, 0.2512809600000001, kolmogorov(10, 0.2), 1e-15) - require.InEpsilon(t, 0.0467840289364274, kolmogorov(100, 0.05), 1e-15) - require.InEpsilon(t, 0.6345548933440742, kolmogorov(1000, 0.028933906713247914), 1e-15) -} From f6e50b5910da184a7fd2b1f61746d822dff7b088 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Tue, 2 Nov 2021 12:37:29 -0700 Subject: [PATCH 07/34] have the test print the specification table --- .../consistent/test/adjustedcount_test.go | 254 ++++++++++++------ 1 file changed, 171 insertions(+), 83 deletions(-) diff --git a/samplers/probability/consistent/test/adjustedcount_test.go b/samplers/probability/consistent/test/adjustedcount_test.go index c142f1e57ef..022a48d0aab 100644 --- a/samplers/probability/consistent/test/adjustedcount_test.go +++ b/samplers/probability/consistent/test/adjustedcount_test.go @@ -29,14 +29,17 @@ import ( ) var ( - populationSize = int64(1e6) + populationSize = 1e6 trials = 20 significance = 1 / float64(trials) + // These may be computed using Gonum, e.g., // import "gonum.org/v1/gonum/stat/distuv" // with significance = 0.05 // chiSquaredDF1 = distuv.ChiSquared{K: 1}.Quantile(significance) // chiSquaredDF2 = distuv.ChiSquared{K: 2}.Quantile(significance) + // + // These have been specified using significance = 0.05: chiSquaredDF1 = 0.003932140000019522 chiSquaredDF2 = 0.1025865887751011 @@ -47,9 +50,15 @@ var ( } ) -func init() { - fmt.Println("chi-squared", chiSquaredByDF) -} +type ( + testDegrees int + pValue int +) + +const ( + oneDegree testDegrees = 1 + twoDegrees testDegrees = 2 +) func parsePR(s string) (p, r string) { for _, kvf := range strings.Split(s, ";") { @@ -77,7 +86,7 @@ func (tsr *testSpanRecorder) Shutdown(ctx context.Context) error { return nil } -func sampleTrials(t *testing.T, prob float64, degrees, upperP, spans int64, source rand.Source) float64 { +func sampleTrials(t *testing.T, prob float64, degrees testDegrees, upperP pValue, source rand.Source) (float64, []float64) { ctx := context.Background() sampler := consistent.ConsistentProbabilityBased( @@ -93,14 +102,14 @@ func sampleTrials(t *testing.T, prob float64, degrees, upperP, spans int64, sour tracer := provider.Tracer("test") - for i := int64(0); i < spans; i++ { + for i := 0; i < int(populationSize); i++ { _, span := tracer.Start(ctx, "span") span.End() } - var minP, maxP int64 + var minP, maxP pValue - counts := map[int64]int64{} + counts := map[pValue]int64{} for idx, r := range recorder.spans { ts := r.SpanContext().TraceState() @@ -110,56 +119,64 @@ func sampleTrials(t *testing.T, prob float64, degrees, upperP, spans int64, sour require.NoError(t, err) if idx == 0 { - maxP = int64(pi) + maxP = pValue(pi) minP = maxP } else { - if int64(pi) < minP { - minP = int64(pi) + if pValue(pi) < minP { + minP = pValue(pi) } - if int64(pi) > maxP { - maxP = int64(pi) + if pValue(pi) > maxP { + maxP = pValue(pi) } } - counts[int64(pi)]++ + counts[pValue(pi)]++ } - require.Less(t, maxP, minP+degrees, "%v %v %v", minP, maxP, degrees) - require.Less(t, maxP, int64(63)) + require.Less(t, maxP, minP+pValue(degrees), "%v %v %v", minP, maxP, degrees) + require.Less(t, maxP, pValue(63)) require.LessOrEqual(t, len(counts), 2) - var upperProb, lowerProb, lowerChoice float64 + var ceilingProb, floorProb, floorChoice float64 + + // Note: we have to test len(counts) == 0 because this outcome + // is actually possible, just very unlikely. If this happens + // during development, a new initial seed must be used for + // this test. + // + // The test specification ensures the test ensures there are + // at least 20 expected items per category in these tests. + require.NotEqual(t, 0, len(counts)) if degrees == 2 { - if len(counts) != 0 { - require.Equal(t, minP+1, maxP) - require.Equal(t, upperP, maxP) - } - upperProb = 1 / float64(int64(1)<= 0 { + seed = seedBank[seedIndex] + } else { + seedIndex = trySeedIndex + seed = seedBank[trySeedIndex] + trySeedIndex++ } - src := rand.NewSource(seed) - less := 0 - for j := 0; j < trials; j++ { - x := sampleTrials(t, ps.prob, ps.degrees, ps.upperP, populationSize, src) + countFailures := func(src rand.Source) int { + failed := 0 + + for j := 0; j < trials; j++ { + var x float64 + x, expected = sampleTrials(t, test.prob, test.degrees, test.upperP, src) - if x < chiSquaredByDF[ps.degrees] { - less++ + if x < chiSquaredByDF[test.degrees] { + failed++ + } } + return failed } - if less != 1 && ps.seed == 0 { - t.Logf("%d probabilistic failures, trying a new seed for %g was 0x%x", less, ps.prob, seed) + failed := countFailures(rand.NewSource(seed)) + + if failed != 1 && test.seedIndex < 0 { + t.Logf("%d probabilistic failures, trying a new seed for %g was 0x%x", failed, test.prob, seed) continue - } else if less != 1 { - t.Errorf("incorrect number of probabilistic failures, should be 1 was %d", less) - } else if ps.seed == 0 { - t.Logf("update the test for %g to use seed 0x%x", ps.prob, seed) + } else if failed != 1 { + t.Errorf("wrong number of probabilistic failures for %g, should be 1 was %d for seed 0x%x", test.prob, failed, seed) + } else if test.seedIndex < 0 { + t.Logf("update the test for %g to use seed index %d", test.prob, seedIndex) t.Fail() return } else { - // pass + // Note: this can be uncommented to verify that the preceding seed failed the test, + // for example: + // if seedIndex != 0 && countFailures(rand.NewSource(seedBank[seedIndex-1])) == 1 { + // t.Logf("update the test for %g to use seed index < %d", test.prob, seedIndex) + // t.Fail() + // } break } } }) + testSummary = append(testSummary, testResult{ + test: test, + expected: expected, + }) + } + + for idx, res := range testSummary { + var probability, pvalues, expectLower, expectUpper, expectUnsampled string + if res.test.degrees == twoDegrees { + probability = fmt.Sprintf("%.6f", res.test.prob) + pvalues = fmt.Sprint(res.test.upperP-1, ", ", res.test.upperP) + expectUnsampled = fmt.Sprintf("%.10g", res.expected[0]) + expectLower = fmt.Sprintf("%.10g", res.expected[1]) + expectUpper = fmt.Sprintf("%.10g", res.expected[2]) + } else { + probability = fmt.Sprintf("%x (%.6f)", res.test.prob, res.test.prob) + pvalues = fmt.Sprint(res.test.upperP) + expectUnsampled = fmt.Sprintf("%.10g", res.expected[0]) + expectLower = fmt.Sprintf("%.10g", res.expected[1]) + expectUpper = "n/a" + } + t.Logf("| %d | %s | %s | %s | %s | %s |\n", idx, probability, pvalues, expectLower, expectUpper, expectUnsampled) } } From 6bf816fbc4a720a6b7670af53710debb04e7b66b Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Tue, 2 Nov 2021 13:43:37 -0700 Subject: [PATCH 08/34] output the table used in the spec --- samplers/probability/consistent/test/adjustedcount_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samplers/probability/consistent/test/adjustedcount_test.go b/samplers/probability/consistent/test/adjustedcount_test.go index 022a48d0aab..a8229f4b5a4 100644 --- a/samplers/probability/consistent/test/adjustedcount_test.go +++ b/samplers/probability/consistent/test/adjustedcount_test.go @@ -324,6 +324,6 @@ func TestPowerOfTwoSampling(t *testing.T) { expectLower = fmt.Sprintf("%.10g", res.expected[1]) expectUpper = "n/a" } - t.Logf("| %d | %s | %s | %s | %s | %s |\n", idx, probability, pvalues, expectLower, expectUpper, expectUnsampled) + t.Logf("| %d | %s | %s | %s | %s | %s |\n", idx+1, probability, pvalues, expectLower, expectUpper, expectUnsampled) } } From 418aac5f960200b7f9cb754d533f89947a31e15f Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Tue, 2 Nov 2021 13:46:05 -0700 Subject: [PATCH 09/34] remove a package --- .../probability/consistent/{test => }/adjustedcount_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename samplers/probability/consistent/{test => }/adjustedcount_test.go (99%) diff --git a/samplers/probability/consistent/test/adjustedcount_test.go b/samplers/probability/consistent/adjustedcount_test.go similarity index 99% rename from samplers/probability/consistent/test/adjustedcount_test.go rename to samplers/probability/consistent/adjustedcount_test.go index a8229f4b5a4..daf83ac4dfd 100644 --- a/samplers/probability/consistent/test/adjustedcount_test.go +++ b/samplers/probability/consistent/adjustedcount_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package test +package consistent_test import ( "context" From 8ac4b54f92de3405bbf6d1a897dcad06b438e807 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Tue, 2 Nov 2021 14:09:59 -0700 Subject: [PATCH 10/34] comment --- samplers/probability/consistent/adjustedcount_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/samplers/probability/consistent/adjustedcount_test.go b/samplers/probability/consistent/adjustedcount_test.go index daf83ac4dfd..76bc4d0ae5b 100644 --- a/samplers/probability/consistent/adjustedcount_test.go +++ b/samplers/probability/consistent/adjustedcount_test.go @@ -218,6 +218,8 @@ func TestPowerOfTwoSampling(t *testing.T) { degrees testDegrees // seedIndex is the index into seedBank of the test seed. + // If this is -1 the code below will search for the smallest + // seed index that passes the test. seedIndex int } testResult struct { From b3a2e9da1df51a8fb711d1e378f5f6a577eb196c Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Wed, 3 Nov 2021 12:51:36 -0700 Subject: [PATCH 11/34] add tracestate handling tests --- .../consistent/adjustedcount_test.go | 36 +-- samplers/probability/consistent/tracestate.go | 46 +++- .../probability/consistent/tracestate_test.go | 221 ++++++++++++++---- 3 files changed, 232 insertions(+), 71 deletions(-) diff --git a/samplers/probability/consistent/adjustedcount_test.go b/samplers/probability/consistent/adjustedcount_test.go index 76bc4d0ae5b..4c47561ea85 100644 --- a/samplers/probability/consistent/adjustedcount_test.go +++ b/samplers/probability/consistent/adjustedcount_test.go @@ -232,26 +232,26 @@ func TestPowerOfTwoSampling(t *testing.T) { for _, test := range []testCase{ // Non-powers of two {0.90000, 1, twoDegrees, 5}, - {0.60000, 1, twoDegrees, 14}, - {0.33000, 2, twoDegrees, 3}, - {0.13000, 3, twoDegrees, 2}, - {0.10000, 4, twoDegrees, 0}, - {0.05000, 5, twoDegrees, 0}, - {0.01700, 6, twoDegrees, 2}, - {0.01000, 7, twoDegrees, 3}, - {0.00500, 8, twoDegrees, 1}, - {0.00290, 9, twoDegrees, 1}, - {0.00100, 10, twoDegrees, 5}, - {0.00050, 11, twoDegrees, 1}, - {0.00026, 12, twoDegrees, 3}, - {0.00023, 13, twoDegrees, 0}, - {0.00010, 14, twoDegrees, 2}, + // {0.60000, 1, twoDegrees, 14}, + // {0.33000, 2, twoDegrees, 3}, + // {0.13000, 3, twoDegrees, 2}, + // {0.10000, 4, twoDegrees, 0}, + // {0.05000, 5, twoDegrees, 0}, + // {0.01700, 6, twoDegrees, 2}, + // {0.01000, 7, twoDegrees, 3}, + // {0.00500, 8, twoDegrees, 1}, + // {0.00290, 9, twoDegrees, 1}, + // {0.00100, 10, twoDegrees, 5}, + // {0.00050, 11, twoDegrees, 1}, + // {0.00026, 12, twoDegrees, 3}, + // {0.00023, 13, twoDegrees, 0}, + // {0.00010, 14, twoDegrees, 2}, // Powers of two - {0x1p-1, 1, oneDegree, 0}, - {0x1p-4, 4, oneDegree, 2}, - {0x1p-7, 7, oneDegree, 3}, - {0x1p-10, 10, oneDegree, 0}, + // {0x1p-1, 1, oneDegree, 0}, + // {0x1p-4, 4, oneDegree, 2}, + // {0x1p-7, 7, oneDegree, 3}, + // {0x1p-10, 10, oneDegree, 0}, {0x1p-13, 13, oneDegree, 1}, } { var expected []float64 diff --git a/samplers/probability/consistent/tracestate.go b/samplers/probability/consistent/tracestate.go index 70f135ab33d..a8761d5c80b 100644 --- a/samplers/probability/consistent/tracestate.go +++ b/samplers/probability/consistent/tracestate.go @@ -25,12 +25,14 @@ const ( pValueSubkey = "p" rValueSubkey = "r" pZeroValue = 63 + invalidValue = pZeroValue + 1 // invalid for p or r traceStateSizeLimit = 256 ) var ( errTraceStateSyntax = fmt.Errorf("otel tracestate: %w", strconv.ErrSyntax) errTraceStateInconsistent = fmt.Errorf("r-value and p-value are inconsistent") + errTraceStateSizeLimit = fmt.Errorf("otel tracestate: size limit exceeded") ) type otelTraceState struct { @@ -41,8 +43,8 @@ type otelTraceState struct { func newTraceState() otelTraceState { return otelTraceState{ - rvalue: pZeroValue + 1, // out-of-range => !hasRValue() - pvalue: pZeroValue + 1, // out-of-range => !hasPValue() + rvalue: invalidValue, // out-of-range => !hasRValue() + pvalue: invalidValue, // out-of-range => !hasPValue() } } @@ -55,11 +57,11 @@ func (otts otelTraceState) serialize() string { } if otts.hasPValue() { - _, _ = sb.WriteString(fmt.Sprintf("p:%02d", otts.pvalue)) + _, _ = sb.WriteString(fmt.Sprintf("p:%d", otts.pvalue)) } if otts.hasRValue() { semi() - _, _ = sb.WriteString(fmt.Sprintf("r:%02d", otts.rvalue)) + _, _ = sb.WriteString(fmt.Sprintf("r:%d", otts.rvalue)) } for _, unk := range otts.unknown { ex := 0 @@ -67,6 +69,7 @@ func (otts otelTraceState) serialize() string { ex = 1 } if sb.Len()+ex+len(unk) > traceStateSizeLimit { + // Note: should this generate an explicit error? break } semi() @@ -75,6 +78,16 @@ func (otts otelTraceState) serialize() string { return sb.String() } +func isValueByte(r byte) bool { + if isLCAlphaNum(r) { + return true + } + if isUCAlpha(r) { + return true + } + return r == '.' || r == '_' || r == '-' +} + func isLCAlphaNum(r byte) bool { if isLCAlpha(r) { return true @@ -86,6 +99,10 @@ func isLCAlpha(r byte) bool { return r >= 'a' && r <= 'z' } +func isUCAlpha(r byte) bool { + return r >= 'A' && r <= 'Z' +} + func parseOTelTraceState(ts string, isSampled bool) (otelTraceState, error) { var pval, rval string var unknown []string @@ -97,8 +114,10 @@ func parseOTelTraceState(ts string, isSampled bool) (otelTraceState, error) { for len(ts) > 0 { eqPos := 0 for ; eqPos < len(ts); eqPos++ { - if eqPos == 0 && isLCAlpha(ts[eqPos]) { - continue + if eqPos == 0 { + if isLCAlpha(ts[eqPos]) { + continue + } } else if isLCAlphaNum(ts[eqPos]) { continue } @@ -114,7 +133,7 @@ func parseOTelTraceState(ts string, isSampled bool) (otelTraceState, error) { sepPos := 0 for ; sepPos < len(tail); sepPos++ { - if isLCAlphaNum(tail[sepPos]) { + if isValueByte(tail[sepPos]) { continue } break @@ -126,10 +145,10 @@ func parseOTelTraceState(ts string, isSampled bool) (otelTraceState, error) { } else if key == rValueSubkey { rval = tail[0:sepPos] } else { - unknown = append(unknown, ts[0:sepPos]) + unknown = append(unknown, ts[0:sepPos+eqPos+1]) } - if sepPos == 0 || (sepPos < len(tail) && tail[sepPos] != ';') { + if sepPos < len(tail) && tail[sepPos] != ';' { return newTraceState(), errTraceStateSyntax } @@ -138,6 +157,11 @@ func parseOTelTraceState(ts string, isSampled bool) (otelTraceState, error) { } ts = tail[sepPos+1:] + + // test for a trailing ; + if ts == "" { + return newTraceState(), errTraceStateSyntax + } } otts := newTraceState() @@ -160,10 +184,10 @@ func parseOTelTraceState(ts string, isSampled bool) (otelTraceState, error) { if otts.hasPValue() && otts.hasRValue() { implied := otts.pvalue <= otts.rvalue || otts.pvalue == pZeroValue - if implied != isSampled { - otts.pvalue = pZeroValue + 1 + if !isSampled || !implied { // Note: the error ensures the parent-based // sampler repairs the broken tracestate entry. + otts.pvalue = invalidValue return otts, parseError(pValueSubkey, errTraceStateInconsistent) } } diff --git a/samplers/probability/consistent/tracestate_test.go b/samplers/probability/consistent/tracestate_test.go index a2087988348..671b13e6a15 100644 --- a/samplers/probability/consistent/tracestate_test.go +++ b/samplers/probability/consistent/tracestate_test.go @@ -23,6 +23,14 @@ import ( "github.com/stretchr/testify/require" ) +func testName(in string) string { + x := strings.NewReplacer(":", "_", ";", "_").Replace(in) + if len(x) > 32 { + return "" + } + return x +} + func TestNewTraceState(t *testing.T) { otts := newTraceState() require.False(t, otts.hasPValue()) @@ -30,65 +38,86 @@ func TestNewTraceState(t *testing.T) { require.Equal(t, "", otts.serialize()) } +func TestTraceStatePRValueSerialize(t *testing.T) { + otts := newTraceState() + otts.pvalue = 3 + otts.rvalue = 4 + otts.unknown = []string{"a:b", "c:d"} + require.True(t, otts.hasPValue()) + require.True(t, otts.hasRValue()) + require.Equal(t, "p:3;r:4;a:b;c:d", otts.serialize()) +} + +func TestTraceStateSerializeOverflow(t *testing.T) { + long := "x:" + strings.Repeat(".", 254) + otts := newTraceState() + otts.unknown = []string{long} + // this drops the extra key, sorry! + require.Equal(t, long, otts.serialize()) + otts.pvalue = 1 + require.Equal(t, "p:1", otts.serialize()) +} + func TestParseTraceStateUnsampled(t *testing.T) { type testCase struct { - in string - pval, rval uint8 - expectErr error + in string + rval uint8 + expectErr error } const notset = 255 for _, test := range []testCase{ // All are unsampled tests, i.e., `sampled` is not set in traceparent. - {"r:1;p:2", 2, 1, nil}, - {"r:1;p:2;", 2, 1, nil}, - {"p:2;r:1;", 2, 1, nil}, - {"p:2;r:1", 2, 1, nil}, - {"r:1;", notset, 1, nil}, - {"r:1", notset, 1, nil}, - {"r:1=p:2", notset, notset, strconv.ErrSyntax}, - {"r:1;p:2=s:3", notset, notset, strconv.ErrSyntax}, - {":1;p:2=s:3", notset, notset, strconv.ErrSyntax}, - {":;p:2=s:3", notset, notset, strconv.ErrSyntax}, - {":;:", notset, notset, strconv.ErrSyntax}, - {":", notset, notset, strconv.ErrSyntax}, - {"", notset, notset, nil}, - {"r:", notset, notset, strconv.ErrSyntax}, - {"r:;p=1", notset, notset, strconv.ErrSyntax}, - {"r:1", notset, 1, nil}, - {"r:10", notset, 10, nil}, - {"r:33", notset, 33, nil}, - {"r:61", notset, 61, nil}, - {"r:62", notset, 62, nil}, // max r-value - {"r:63", notset, notset, strconv.ErrRange}, // out-of-range - {"r:100", notset, notset, strconv.ErrRange}, // out-of-range - {"r:100001", notset, notset, strconv.ErrRange}, // out-of-range - {"p:1", 1, notset, nil}, - {"p:62", 62, notset, nil}, - {"p:63", 63, notset, nil}, - {"p:64", notset, notset, strconv.ErrRange}, - {"p:100", notset, notset, strconv.ErrRange}, - {"r:1a", notset, notset, strconv.ErrSyntax}, // not-hexadecimal - {"p:-1", notset, notset, strconv.ErrSyntax}, // non-negative + {"r:2", 2, nil}, + {"r:1;", notset, strconv.ErrSyntax}, + {"r:1", 1, nil}, + {"r:1=p:2", notset, strconv.ErrSyntax}, + {"r:1;p:2=s:3", notset, strconv.ErrSyntax}, + {":1;p:2=s:3", notset, strconv.ErrSyntax}, + {":;p:2=s:3", notset, strconv.ErrSyntax}, + {":;:", notset, strconv.ErrSyntax}, + {":", notset, strconv.ErrSyntax}, + {"", notset, nil}, + {"r:;p=1", notset, strconv.ErrSyntax}, + {"r:1", 1, nil}, + {"r:10", 10, nil}, + {"r:33", 33, nil}, + {"r:61", 61, nil}, + {"r:62", 62, nil}, // max r-value + {"r:63", notset, strconv.ErrRange}, // out-of-range + {"r:100", notset, strconv.ErrRange}, // out-of-range + {"r:100001", notset, strconv.ErrRange}, // out-of-range + {"p:64", notset, strconv.ErrRange}, + {"p:100", notset, strconv.ErrRange}, + {"r:1a", notset, strconv.ErrSyntax}, // not-hexadecimal + {"p:-1", notset, strconv.ErrSyntax}, // non-negative + + // Inconsistent trace state: any p-value when unsampled + {"p:4;r:2", 2, errTraceStateInconsistent}, + {"p:1;r:2", 2, errTraceStateInconsistent}, } { - t.Run(strings.NewReplacer(":", "_", ";", "_").Replace(test.in), func(t *testing.T) { + t.Run(testName(test.in), func(t *testing.T) { // Note: passing isSampled=false as stated above. otts, err := parseOTelTraceState(test.in, false) + require.False(t, otts.hasPValue(), "should have no p-value") + if test.expectErr != nil { require.True(t, errors.Is(err, test.expectErr), "not expecting %v", err) } - if test.pval != notset { - require.True(t, otts.hasPValue()) - require.Equal(t, test.pval, otts.pvalue) - } else { - require.False(t, otts.hasPValue(), "should have no p-value") - } if test.rval != notset { require.True(t, otts.hasRValue()) require.Equal(t, test.rval, otts.rvalue) } else { require.False(t, otts.hasRValue(), "should have no r-value") } + require.EqualValues(t, []string(nil), otts.unknown) + + if test.expectErr == nil { + // Require serialize to round-trip + otts2, err := parseOTelTraceState(otts.serialize(), false) + require.NoError(t, err) + require.Equal(t, otts, otts2) + } }) } } @@ -119,15 +148,33 @@ func TestParseTraceStateSampled(t *testing.T) { {"r:2;p:63", 2, 63, nil}, {"r:62;p:63", 62, 63, nil}, - // Inconsistencies cause unset p-value. + // Inconsistent p causes unset p-value. {"r:2;p:3", 2, notset, errTraceStateInconsistent}, {"r:2;p:4", 2, notset, errTraceStateInconsistent}, {"r:2;p:62", 2, notset, errTraceStateInconsistent}, {"r:0;p:1", 0, notset, errTraceStateInconsistent}, {"r:1;p:2", 1, notset, errTraceStateInconsistent}, {"r:61;p:62", 61, notset, errTraceStateInconsistent}, + + // Inconsistent r causes unset p-value and r-value. + {"r:63;p:2", notset, notset, strconv.ErrRange}, + {"r:120;p:2", notset, notset, strconv.ErrRange}, + {"r:ab;p:2", notset, notset, strconv.ErrSyntax}, + + // Syntax is tested before range errors + {"r:ab;p:77", notset, notset, strconv.ErrSyntax}, + + // p without r (when sampled) + {"p:1", notset, 1, nil}, + {"p:62", notset, 62, nil}, + {"p:63", notset, 63, nil}, + + // r without p (when sampled) + {"r:2", 2, notset, nil}, + {"r:62", 62, notset, nil}, + {"r:0", 0, notset, nil}, } { - t.Run(strings.NewReplacer(":", "_", ";", "_").Replace(test.in), func(t *testing.T) { + t.Run(testName(test.in), func(t *testing.T) { // Note: passing isSampled=true as stated above. otts, err := parseOTelTraceState(test.in, true) @@ -148,6 +195,96 @@ func TestParseTraceStateSampled(t *testing.T) { } else { require.False(t, otts.hasRValue(), "should have no r-value") } + require.EqualValues(t, []string(nil), otts.unknown) + + if test.expectErr == nil { + // Require serialize to round-trip + otts2, err := parseOTelTraceState(otts.serialize(), true) + require.NoError(t, err) + require.Equal(t, otts, otts2) + } + }) + } +} + +func TestParseTraceStateExtra(t *testing.T) { + type testCase struct { + in string + rval, pval uint8 + sampled bool + extra []string + expectErr error + } + const notset = 255 + for _, test := range []testCase{ + // one field + {"e100:1", notset, notset, false, []string{"e100:1"}, nil}, + + // two fields + {"e1:1;e2:2", notset, notset, false, []string{"e1:1", "e2:2"}, nil}, + {"e1:1;e2:2", notset, notset, false, []string{"e1:1", "e2:2"}, nil}, + + // one extra key, three ways + {"r:2;p:2;extra:stuff", 2, 2, true, []string{"extra:stuff"}, nil}, + {"extra:stuff;r:2;p:2", 2, 2, true, []string{"extra:stuff"}, nil}, + {"p:2;extra:stuff;r:2", 2, 2, true, []string{"extra:stuff"}, nil}, + + // extra with inconsistent p with and without sampling + {"r:3;extra:stuff;p:4", 3, notset, true, []string{"extra:stuff"}, errTraceStateInconsistent}, + {"extra:stuff;r:3;p:2", 3, notset, false, []string{"extra:stuff"}, errTraceStateInconsistent}, + + // two extra fields + {"e100:100;r:2;p:1;e101:101", 2, 1, true, []string{"e100:100", "e101:101"}, nil}, + {"r:2;p:1;e100:100;e101:101", 2, 1, true, []string{"e100:100", "e101:101"}, nil}, + {"e100:100;e101:101;r:2;p:1", 2, 1, true, []string{"e100:100", "e101:101"}, nil}, + + // parse error prevents capturing unrecognized keys + {"1:1;u:V", notset, notset, true, nil, strconv.ErrSyntax}, + {"X:1;u:V", notset, notset, true, nil, strconv.ErrSyntax}, + {"x:1;u:V", notset, notset, true, []string{"x:1", "u:V"}, nil}, + + // no trailing ; + {"x:1;", notset, notset, true, nil, strconv.ErrSyntax}, + + // empty key + {"x:", notset, notset, true, []string{"x:"}, nil}, + + // charset test + {"x:0X1FFF;y:.-_-.;z:", notset, notset, true, []string{"x:0X1FFF", "y:.-_-.", "z:"}, nil}, + {"x1y2z3:1-2-3;y1:y_1;xy:-;r:50", 50, notset, true, []string{"x1y2z3:1-2-3", "y1:y_1", "xy:-"}, nil}, + + // size exceeded + {"x:" + strings.Repeat("_", 255), notset, notset, false, nil, strconv.ErrSyntax}, + {"x:" + strings.Repeat("_", 254), notset, notset, false, []string{"x:" + strings.Repeat("_", 254)}, nil}, + } { + t.Run(testName(test.in), func(t *testing.T) { + // Note: These tests are independent of sampling state, + // so both are tested. + otts, err := parseOTelTraceState(test.in, test.sampled) + + if test.expectErr != nil { + require.True(t, errors.Is(err, test.expectErr), "not expecting %v", err) + } else { + require.NoError(t, err) + } + if test.pval != notset { + require.True(t, otts.hasPValue()) + require.Equal(t, test.pval, otts.pvalue) + } else { + require.False(t, otts.hasPValue(), "should have no p-value") + } + if test.rval != notset { + require.True(t, otts.hasRValue()) + require.Equal(t, test.rval, otts.rvalue) + } else { + require.False(t, otts.hasRValue(), "should have no r-value") + } + require.EqualValues(t, test.extra, otts.unknown) + + // on success w/o r-value or p-value, serialize() should not modify + if !otts.hasRValue() && !otts.hasPValue() && test.expectErr == nil { + require.Equal(t, test.in, otts.serialize()) + } }) } } From 6e56ff4952a560405acc43508de95996b37d46df Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Wed, 3 Nov 2021 12:53:16 -0700 Subject: [PATCH 12/34] remove a file --- .../consistent/adjustedcount_test.go | 331 ------------------ .../probability/consistent/sampler_test.go | 309 ++++++++++++++++ 2 files changed, 309 insertions(+), 331 deletions(-) delete mode 100644 samplers/probability/consistent/adjustedcount_test.go diff --git a/samplers/probability/consistent/adjustedcount_test.go b/samplers/probability/consistent/adjustedcount_test.go deleted file mode 100644 index 4c47561ea85..00000000000 --- a/samplers/probability/consistent/adjustedcount_test.go +++ /dev/null @@ -1,331 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package consistent_test - -import ( - "context" - "fmt" - "math" - "math/rand" - "strconv" - "strings" - "testing" - - "github.com/stretchr/testify/require" - "go.opentelemetry.io/contrib/samplers/probability/consistent" - sdktrace "go.opentelemetry.io/otel/sdk/trace" -) - -var ( - populationSize = 1e6 - trials = 20 - significance = 1 / float64(trials) - - // These may be computed using Gonum, e.g., - // import "gonum.org/v1/gonum/stat/distuv" - // with significance = 0.05 - // chiSquaredDF1 = distuv.ChiSquared{K: 1}.Quantile(significance) - // chiSquaredDF2 = distuv.ChiSquared{K: 2}.Quantile(significance) - // - // These have been specified using significance = 0.05: - chiSquaredDF1 = 0.003932140000019522 - chiSquaredDF2 = 0.1025865887751011 - - chiSquaredByDF = [3]float64{ - 0, - chiSquaredDF1, - chiSquaredDF2, - } -) - -type ( - testDegrees int - pValue int -) - -const ( - oneDegree testDegrees = 1 - twoDegrees testDegrees = 2 -) - -func parsePR(s string) (p, r string) { - for _, kvf := range strings.Split(s, ";") { - kv := strings.SplitN(kvf, ":", 2) - switch kv[0] { - case "p": - p = kv[1] - case "r": - r = kv[1] - } - } - return -} - -type testSpanRecorder struct { - spans []sdktrace.ReadOnlySpan -} - -func (tsr *testSpanRecorder) ExportSpans(ctx context.Context, spans []sdktrace.ReadOnlySpan) error { - tsr.spans = append(tsr.spans, spans...) - return nil -} - -func (tsr *testSpanRecorder) Shutdown(ctx context.Context) error { - return nil -} - -func sampleTrials(t *testing.T, prob float64, degrees testDegrees, upperP pValue, source rand.Source) (float64, []float64) { - ctx := context.Background() - - sampler := consistent.ConsistentProbabilityBased( - prob, - consistent.WithRandomSource(source), - ) - - recorder := &testSpanRecorder{} - provider := sdktrace.NewTracerProvider( - sdktrace.WithSyncer(recorder), - sdktrace.WithSampler(sampler), - ) - - tracer := provider.Tracer("test") - - for i := 0; i < int(populationSize); i++ { - _, span := tracer.Start(ctx, "span") - span.End() - } - - var minP, maxP pValue - - counts := map[pValue]int64{} - - for idx, r := range recorder.spans { - ts := r.SpanContext().TraceState() - p, _ := parsePR(ts.Get("ot")) - - pi, err := strconv.ParseUint(p, 10, 64) - require.NoError(t, err) - - if idx == 0 { - maxP = pValue(pi) - minP = maxP - } else { - if pValue(pi) < minP { - minP = pValue(pi) - } - if pValue(pi) > maxP { - maxP = pValue(pi) - } - } - counts[pValue(pi)]++ - } - - require.Less(t, maxP, minP+pValue(degrees), "%v %v %v", minP, maxP, degrees) - require.Less(t, maxP, pValue(63)) - require.LessOrEqual(t, len(counts), 2) - - var ceilingProb, floorProb, floorChoice float64 - - // Note: we have to test len(counts) == 0 because this outcome - // is actually possible, just very unlikely. If this happens - // during development, a new initial seed must be used for - // this test. - // - // The test specification ensures the test ensures there are - // at least 20 expected items per category in these tests. - require.NotEqual(t, 0, len(counts)) - - if degrees == 2 { - require.Equal(t, minP+1, maxP) - require.Equal(t, upperP, maxP) - ceilingProb = 1 / float64(int64(1)<= 0 { - seed = seedBank[seedIndex] - } else { - seedIndex = trySeedIndex - seed = seedBank[trySeedIndex] - trySeedIndex++ - } - - countFailures := func(src rand.Source) int { - failed := 0 - - for j := 0; j < trials; j++ { - var x float64 - x, expected = sampleTrials(t, test.prob, test.degrees, test.upperP, src) - - if x < chiSquaredByDF[test.degrees] { - failed++ - } - } - return failed - } - - failed := countFailures(rand.NewSource(seed)) - - if failed != 1 && test.seedIndex < 0 { - t.Logf("%d probabilistic failures, trying a new seed for %g was 0x%x", failed, test.prob, seed) - continue - } else if failed != 1 { - t.Errorf("wrong number of probabilistic failures for %g, should be 1 was %d for seed 0x%x", test.prob, failed, seed) - } else if test.seedIndex < 0 { - t.Logf("update the test for %g to use seed index %d", test.prob, seedIndex) - t.Fail() - return - } else { - // Note: this can be uncommented to verify that the preceding seed failed the test, - // for example: - // if seedIndex != 0 && countFailures(rand.NewSource(seedBank[seedIndex-1])) == 1 { - // t.Logf("update the test for %g to use seed index < %d", test.prob, seedIndex) - // t.Fail() - // } - break - } - } - }) - testSummary = append(testSummary, testResult{ - test: test, - expected: expected, - }) - } - - for idx, res := range testSummary { - var probability, pvalues, expectLower, expectUpper, expectUnsampled string - if res.test.degrees == twoDegrees { - probability = fmt.Sprintf("%.6f", res.test.prob) - pvalues = fmt.Sprint(res.test.upperP-1, ", ", res.test.upperP) - expectUnsampled = fmt.Sprintf("%.10g", res.expected[0]) - expectLower = fmt.Sprintf("%.10g", res.expected[1]) - expectUpper = fmt.Sprintf("%.10g", res.expected[2]) - } else { - probability = fmt.Sprintf("%x (%.6f)", res.test.prob, res.test.prob) - pvalues = fmt.Sprint(res.test.upperP) - expectUnsampled = fmt.Sprintf("%.10g", res.expected[0]) - expectLower = fmt.Sprintf("%.10g", res.expected[1]) - expectUpper = "n/a" - } - t.Logf("| %d | %s | %s | %s | %s | %s |\n", idx+1, probability, pvalues, expectLower, expectUpper, expectUnsampled) - } -} diff --git a/samplers/probability/consistent/sampler_test.go b/samplers/probability/consistent/sampler_test.go index 7c93c3d6025..ff47592e481 100644 --- a/samplers/probability/consistent/sampler_test.go +++ b/samplers/probability/consistent/sampler_test.go @@ -15,9 +15,16 @@ package consistent import ( + "context" + "fmt" + "math" + "math/rand" + "strconv" + "strings" "testing" "github.com/stretchr/testify/require" + sdktrace "go.opentelemetry.io/otel/sdk/trace" ) func TestDescription(t *testing.T) { @@ -51,3 +58,305 @@ func TestDescription(t *testing.T) { require.Equal(t, tc.expect, s.Description(), "%#v", tc.prob) } } + +var ( + populationSize = 1e6 + trials = 20 + significance = 1 / float64(trials) + + // These may be computed using Gonum, e.g., + // import "gonum.org/v1/gonum/stat/distuv" + // with significance = 0.05 + // chiSquaredDF1 = distuv.ChiSquared{K: 1}.Quantile(significance) + // chiSquaredDF2 = distuv.ChiSquared{K: 2}.Quantile(significance) + // + // These have been specified using significance = 0.05: + chiSquaredDF1 = 0.003932140000019522 + chiSquaredDF2 = 0.1025865887751011 + + chiSquaredByDF = [3]float64{ + 0, + chiSquaredDF1, + chiSquaredDF2, + } +) + +type ( + testDegrees int + pValue int +) + +const ( + oneDegree testDegrees = 1 + twoDegrees testDegrees = 2 +) + +func parsePR(s string) (p, r string) { + for _, kvf := range strings.Split(s, ";") { + kv := strings.SplitN(kvf, ":", 2) + switch kv[0] { + case "p": + p = kv[1] + case "r": + r = kv[1] + } + } + return +} + +type testSpanRecorder struct { + spans []sdktrace.ReadOnlySpan +} + +func (tsr *testSpanRecorder) ExportSpans(ctx context.Context, spans []sdktrace.ReadOnlySpan) error { + tsr.spans = append(tsr.spans, spans...) + return nil +} + +func (tsr *testSpanRecorder) Shutdown(ctx context.Context) error { + return nil +} + +func sampleTrials(t *testing.T, prob float64, degrees testDegrees, upperP pValue, source rand.Source) (float64, []float64) { + ctx := context.Background() + + sampler := ConsistentProbabilityBased( + prob, + WithRandomSource(source), + ) + + recorder := &testSpanRecorder{} + provider := sdktrace.NewTracerProvider( + sdktrace.WithSyncer(recorder), + sdktrace.WithSampler(sampler), + ) + + tracer := provider.Tracer("test") + + for i := 0; i < int(populationSize); i++ { + _, span := tracer.Start(ctx, "span") + span.End() + } + + var minP, maxP pValue + + counts := map[pValue]int64{} + + for idx, r := range recorder.spans { + ts := r.SpanContext().TraceState() + p, _ := parsePR(ts.Get("ot")) + + pi, err := strconv.ParseUint(p, 10, 64) + require.NoError(t, err) + + if idx == 0 { + maxP = pValue(pi) + minP = maxP + } else { + if pValue(pi) < minP { + minP = pValue(pi) + } + if pValue(pi) > maxP { + maxP = pValue(pi) + } + } + counts[pValue(pi)]++ + } + + require.Less(t, maxP, minP+pValue(degrees), "%v %v %v", minP, maxP, degrees) + require.Less(t, maxP, pValue(63)) + require.LessOrEqual(t, len(counts), 2) + + var ceilingProb, floorProb, floorChoice float64 + + // Note: we have to test len(counts) == 0 because this outcome + // is actually possible, just very unlikely. If this happens + // during development, a new initial seed must be used for + // this test. + // + // The test specification ensures the test ensures there are + // at least 20 expected items per category in these tests. + require.NotEqual(t, 0, len(counts)) + + if degrees == 2 { + require.Equal(t, minP+1, maxP) + require.Equal(t, upperP, maxP) + ceilingProb = 1 / float64(int64(1)<= 0 { + seed = seedBank[seedIndex] + } else { + seedIndex = trySeedIndex + seed = seedBank[trySeedIndex] + trySeedIndex++ + } + + countFailures := func(src rand.Source) int { + failed := 0 + + for j := 0; j < trials; j++ { + var x float64 + x, expected = sampleTrials(t, test.prob, test.degrees, test.upperP, src) + + if x < chiSquaredByDF[test.degrees] { + failed++ + } + } + return failed + } + + failed := countFailures(rand.NewSource(seed)) + + if failed != 1 && test.seedIndex < 0 { + t.Logf("%d probabilistic failures, trying a new seed for %g was 0x%x", failed, test.prob, seed) + continue + } else if failed != 1 { + t.Errorf("wrong number of probabilistic failures for %g, should be 1 was %d for seed 0x%x", test.prob, failed, seed) + } else if test.seedIndex < 0 { + t.Logf("update the test for %g to use seed index %d", test.prob, seedIndex) + t.Fail() + return + } else { + // Note: this can be uncommented to verify that the preceding seed failed the test, + // for example: + // if seedIndex != 0 && countFailures(rand.NewSource(seedBank[seedIndex-1])) == 1 { + // t.Logf("update the test for %g to use seed index < %d", test.prob, seedIndex) + // t.Fail() + // } + break + } + } + }) + testSummary = append(testSummary, testResult{ + test: test, + expected: expected, + }) + } + + for idx, res := range testSummary { + var probability, pvalues, expectLower, expectUpper, expectUnsampled string + if res.test.degrees == twoDegrees { + probability = fmt.Sprintf("%.6f", res.test.prob) + pvalues = fmt.Sprint(res.test.upperP-1, ", ", res.test.upperP) + expectUnsampled = fmt.Sprintf("%.10g", res.expected[0]) + expectLower = fmt.Sprintf("%.10g", res.expected[1]) + expectUpper = fmt.Sprintf("%.10g", res.expected[2]) + } else { + probability = fmt.Sprintf("%x (%.6f)", res.test.prob, res.test.prob) + pvalues = fmt.Sprint(res.test.upperP) + expectUnsampled = fmt.Sprintf("%.10g", res.expected[0]) + expectLower = fmt.Sprintf("%.10g", res.expected[1]) + expectUpper = "n/a" + } + t.Logf("| %d | %s | %s | %s | %s | %s |\n", idx+1, probability, pvalues, expectLower, expectUpper, expectUnsampled) + } +} From c5c42faca7e4181f2e83b1da58d6592acbb61177 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Wed, 3 Nov 2021 14:17:58 -0700 Subject: [PATCH 13/34] add parent test --- samplers/probability/consistent/parent.go | 14 +- .../probability/consistent/parent_test.go | 174 ++++++++++++++++++ .../probability/consistent/sampler_test.go | 4 +- 3 files changed, 185 insertions(+), 7 deletions(-) create mode 100644 samplers/probability/consistent/parent_test.go diff --git a/samplers/probability/consistent/parent.go b/samplers/probability/consistent/parent.go index d853423413f..9035eb77b83 100644 --- a/samplers/probability/consistent/parent.go +++ b/samplers/probability/consistent/parent.go @@ -37,17 +37,21 @@ func ConsistentParentProbabilityBased(root sdktrace.Sampler, samplers ...sdktrac func (p *parentProbabilitySampler) ShouldSample(params sdktrace.SamplingParameters) sdktrace.SamplingResult { psc := trace.SpanContextFromContext(params.ParentContext) - if !psc.IsValid() { - return p.delegate.ShouldSample(params) - } - + // Note: We do not check psc.IsValid(), i.e., we repair the tracestate + // with or without a parent TraceId and SpanId. state := psc.TraceState() otts, err := parseOTelTraceState(state.Get(traceStateKey), psc.IsSampled()) if err != nil { otel.Handle(err) - state.Insert(traceStateKey, otts.serialize()) + // Note: do not handle a second error in this path. + value := otts.serialize() + if len(value) > 0 { + state, _ = state.Insert(traceStateKey, value) + } else { + state = state.Delete(traceStateKey) + } // Fix the broken tracestate before calling the delegate. params.ParentContext = diff --git a/samplers/probability/consistent/parent_test.go b/samplers/probability/consistent/parent_test.go new file mode 100644 index 00000000000..d65eef0ae8f --- /dev/null +++ b/samplers/probability/consistent/parent_test.go @@ -0,0 +1,174 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package consistent + +import ( + "context" + "strings" + "testing" + + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/attribute" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" +) + +func TestParentSamplerDescription(t *testing.T) { + opts := []sdktrace.ParentBasedSamplerOption{ + sdktrace.WithRemoteParentNotSampled(sdktrace.AlwaysSample()), + } + root := ConsistentProbabilityBased(1) + compare := sdktrace.ParentBased(root, opts...) + parent := ConsistentParentProbabilityBased(root, opts...) + require.Equal(t, + strings.Replace( + compare.Description(), + "ParentBased", + "ConsistentParentProbabilityBased", + 1, + ), + parent.Description(), + ) +} + +func TestParentSamplerRootContext(t *testing.T) { + +} + +func TestParentSamplerValidContext(t *testing.T) { + parent := ConsistentParentProbabilityBased(sdktrace.NeverSample()) + type testCase struct { + in string + sampled bool + } + for _, valid := range []testCase{ + // sampled tests + {"r:10", true}, + {"r:10;a:b", true}, + {"r:10;p:1", true}, + {"r:10;p:10", true}, + {"r:10;p:10;a:b", true}, + {"r:10;p:63", true}, + {"r:10;p:63;a:b", true}, + {"p:0", true}, + {"p:10;a:b", true}, + {"p:63", true}, + {"p:63;a:b", true}, + + // unsampled tests + {"r:10", false}, + {"r:10;a:b", false}, + } { + t.Run(testName(valid.in), func(t *testing.T) { + traceID, _ := trace.TraceIDFromHex("4bf92f3577b34da6a3ce929d0e0e4736") + spanID, _ := trace.SpanIDFromHex("00f067aa0ba902b7") + traceState, err := trace.TraceState{}.Insert(traceStateKey, valid.in) + require.NoError(t, err) + + sccfg := trace.SpanContextConfig{ + TraceID: traceID, + SpanID: spanID, + TraceState: traceState, + } + + if valid.sampled { + sccfg.TraceFlags = trace.FlagsSampled + } + + parentCtx := trace.ContextWithSpanContext( + context.Background(), + trace.NewSpanContext(sccfg), + ) + + result := parent.ShouldSample( + sdktrace.SamplingParameters{ + ParentContext: parentCtx, + TraceID: traceID, + Name: "test", + Kind: trace.SpanKindServer, + }, + ) + + if valid.sampled { + require.Equal(t, sdktrace.RecordAndSample, result.Decision) + } else { + require.Equal(t, sdktrace.Drop, result.Decision) + } + require.Equal(t, []attribute.KeyValue(nil), result.Attributes) + require.Equal(t, valid.in, result.Tracestate.Get(traceStateKey)) + }) + } +} + +func TestParentSamplerInvalidContext(t *testing.T) { + parent := ConsistentParentProbabilityBased(sdktrace.NeverSample()) + type testCase struct { + in string + sampled bool + expect string + } + for _, valid := range []testCase{ + // sampled + {"r:100", true, ""}, + {"r:100;p:1", true, ""}, + {"r:100;p:1;a:b", true, "a:b"}, + {"r:10;p:100", true, "r:10"}, + {"r:10;p:100;a:b", true, "r:10;a:b"}, + + // unsampled + {"r:63;p:1", false, ""}, + {"r:10;p:1", false, "r:10"}, + {"r:10;p:1;a:b", false, "r:10;a:b"}, + } { + t.Run(testName(valid.in), func(t *testing.T) { + traceID, _ := trace.TraceIDFromHex("4bf92f3577b34da6a3ce929d0e0e4736") + spanID, _ := trace.SpanIDFromHex("00f067aa0ba902b7") + traceState, err := trace.TraceState{}.Insert(traceStateKey, valid.in) + require.NoError(t, err) + + sccfg := trace.SpanContextConfig{ + TraceID: traceID, + SpanID: spanID, + TraceState: traceState, + } + + if valid.sampled { + sccfg.TraceFlags = trace.FlagsSampled + } + + parentCtx := trace.ContextWithSpanContext( + context.Background(), + trace.NewSpanContext(sccfg), + ) + + result := parent.ShouldSample( + sdktrace.SamplingParameters{ + ParentContext: parentCtx, + TraceID: traceID, + Name: "test", + Kind: trace.SpanKindServer, + }, + ) + + if valid.sampled { + require.Equal(t, sdktrace.RecordAndSample, result.Decision) + } else { + require.Equal(t, sdktrace.Drop, result.Decision) + } + require.Equal(t, []attribute.KeyValue(nil), result.Attributes) + require.Equal(t, valid.expect, result.Tracestate.Get(traceStateKey)) + }) + } +} diff --git a/samplers/probability/consistent/sampler_test.go b/samplers/probability/consistent/sampler_test.go index ff47592e481..8fe1974e77f 100644 --- a/samplers/probability/consistent/sampler_test.go +++ b/samplers/probability/consistent/sampler_test.go @@ -27,7 +27,7 @@ import ( sdktrace "go.opentelemetry.io/otel/sdk/trace" ) -func TestDescription(t *testing.T) { +func TestSamplerDescription(t *testing.T) { const minProb = 0x1p-62 // 2.168404344971009e-19 for _, tc := range []struct { @@ -218,7 +218,7 @@ func sampleTrials(t *testing.T, prob float64, degrees testDegrees, upperP pValue return chi2, expected } -func TestPowerOfTwoSampling(t *testing.T) { +func TestStatisticalTest(t *testing.T) { // This test is seeded with a "palindromic wing prime". seedBankRng := rand.New(rand.NewSource(77777677777)) seedBank := make([]int64, 15) // N.B. Max=14 below. From 9b6a591b885c7ff17448ba2098da6f17fdb41a0d Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Wed, 3 Nov 2021 15:52:59 -0700 Subject: [PATCH 14/34] 100% coverage --- samplers/probability/consistent/parent.go | 4 +- samplers/probability/consistent/sampler.go | 50 ++- .../probability/consistent/sampler_test.go | 287 ++++++++++++++---- samplers/probability/consistent/tracestate.go | 4 + 4 files changed, 251 insertions(+), 94 deletions(-) diff --git a/samplers/probability/consistent/parent.go b/samplers/probability/consistent/parent.go index 9035eb77b83..4673a38ffba 100644 --- a/samplers/probability/consistent/parent.go +++ b/samplers/probability/consistent/parent.go @@ -45,9 +45,11 @@ func (p *parentProbabilitySampler) ShouldSample(params sdktrace.SamplingParamete if err != nil { otel.Handle(err) - // Note: do not handle a second error in this path. value := otts.serialize() if len(value) > 0 { + // Note: see the note in + // "go.opentelemetry.io/otel/trace".TraceState.Insert(). The + // error below is not a condition we're supposed to handle. state, _ = state.Insert(traceStateKey, value) } else { state = state.Delete(traceStateKey) diff --git a/samplers/probability/consistent/sampler.go b/samplers/probability/consistent/sampler.go index 8e145af3674..30897d77914 100644 --- a/samplers/probability/consistent/sampler.go +++ b/samplers/probability/consistent/sampler.go @@ -122,30 +122,21 @@ func (cs *consistentProbabilityBased) lowChoice() bool { // ShouldSample implements Sampler. func (cs *consistentProbabilityBased) ShouldSample(p sdktrace.SamplingParameters) sdktrace.SamplingResult { psc := trace.SpanContextFromContext(p.ParentContext) - var otts otelTraceState - var state trace.TraceState - if !psc.IsValid() { - // A new root is happening. Compute the r-value. - otts = newTraceState() + // Note: this ignores whether psc.IsValid() because this + // allows other otel trace state keys to pass through even + // for root decisions. + state := psc.TraceState() + + otts, err := parseOTelTraceState(state.Get(traceStateKey), psc.IsSampled()) + if err != nil { + // Note: a state.Insert(traceStateKey) + // follows, nothing else needs to be done here. + otel.Handle(err) + } + + if !otts.hasRValue() { otts.rvalue = cs.newR() - } else { - // A valid parent context. - state = psc.TraceState() - - var err error - otts, err = parseOTelTraceState(state.Get(traceStateKey), psc.IsSampled()) - - if err != nil { - // Note: a state.Insert(traceStateKey) - // follows, nothing else needs to be done here. - otel.Handle(err) - } - - if !otts.hasRValue() { - // Specification says to set r-value if missing. - otts.rvalue = cs.newR() - } } var decision sdktrace.SamplingDecision @@ -159,19 +150,16 @@ func (cs *consistentProbabilityBased) ShouldSample(p sdktrace.SamplingParameters if lac <= otts.rvalue { decision = sdktrace.RecordAndSample + otts.pvalue = lac } else { decision = sdktrace.Drop + otts.pvalue = invalidValue } - otts.pvalue = lac - - state, err := state.Insert(traceStateKey, otts.serialize()) - if err != nil { - otel.Handle(err) - // Note: see the note in - // "go.opentelemetry.io/otel/trace".TraceState.Insert() - // this is not a condition we're supposed to handle. - } + // Note: see the note in + // "go.opentelemetry.io/otel/trace".TraceState.Insert(). The + // error below is not a condition we're supposed to handle. + state, _ = state.Insert(traceStateKey, otts.serialize()) return sdktrace.SamplingResult{ Decision: decision, diff --git a/samplers/probability/consistent/sampler_test.go b/samplers/probability/consistent/sampler_test.go index 8fe1974e77f..09e25d869cb 100644 --- a/samplers/probability/consistent/sampler_test.go +++ b/samplers/probability/consistent/sampler_test.go @@ -21,43 +21,35 @@ import ( "math/rand" "strconv" "strings" + "sync" "testing" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/trace" ) -func TestSamplerDescription(t *testing.T) { - const minProb = 0x1p-62 // 2.168404344971009e-19 - - for _, tc := range []struct { - prob float64 - expect string - }{ - {1, "ConsistentProbabilityBased{1}"}, - {0, "ConsistentProbabilityBased{0}"}, - {0.75, "ConsistentProbabilityBased{0.75}"}, - {0.05, "ConsistentProbabilityBased{0.05}"}, - {0.003, "ConsistentProbabilityBased{0.003}"}, - {0.99999999, "ConsistentProbabilityBased{0.99999999}"}, - {0.00000001, "ConsistentProbabilityBased{1e-08}"}, - {minProb, "ConsistentProbabilityBased{2.168404344971009e-19}"}, - {minProb * 1.5, "ConsistentProbabilityBased{3.2526065174565133e-19}"}, - {3e-19, "ConsistentProbabilityBased{3e-19}"}, +type ( + testDegrees int + pValue int - // out-of-range > 1 - {1.01, "ConsistentProbabilityBased{1}"}, - {101.1, "ConsistentProbabilityBased{1}"}, + testSpanRecorder struct { + lock sync.Mutex + spans []sdktrace.ReadOnlySpan + } - // out-of-range < 2^-62 - {-1, "ConsistentProbabilityBased{0}"}, - {-0.001, "ConsistentProbabilityBased{0}"}, - {minProb * 0.999, "ConsistentProbabilityBased{0}"}, - } { - s := ConsistentProbabilityBased(tc.prob) - require.Equal(t, tc.expect, s.Description(), "%#v", tc.prob) + testErrorHandler struct { + lock sync.Mutex + errors []error } -} +) + +const ( + oneDegree testDegrees = 1 + twoDegrees testDegrees = 2 +) var ( populationSize = 1e6 @@ -81,16 +73,6 @@ var ( } ) -type ( - testDegrees int - pValue int -) - -const ( - oneDegree testDegrees = 1 - twoDegrees testDegrees = 2 -) - func parsePR(s string) (p, r string) { for _, kvf := range strings.Split(s, ";") { kv := strings.SplitN(kvf, ":", 2) @@ -104,11 +86,9 @@ func parsePR(s string) (p, r string) { return } -type testSpanRecorder struct { - spans []sdktrace.ReadOnlySpan -} - func (tsr *testSpanRecorder) ExportSpans(ctx context.Context, spans []sdktrace.ReadOnlySpan) error { + tsr.lock.Lock() + defer tsr.lock.Unlock() tsr.spans = append(tsr.spans, spans...) return nil } @@ -117,6 +97,187 @@ func (tsr *testSpanRecorder) Shutdown(ctx context.Context) error { return nil } +func (eh *testErrorHandler) Handle(err error) { + eh.lock.Lock() + defer eh.lock.Unlock() + eh.errors = append(eh.errors, err) +} + +func (eh *testErrorHandler) Errors() []error { + eh.lock.Lock() + defer eh.lock.Unlock() + return eh.errors +} + +func TestSamplerDescription(t *testing.T) { + const minProb = 0x1p-62 // 2.168404344971009e-19 + + for _, tc := range []struct { + prob float64 + expect string + }{ + {1, "ConsistentProbabilityBased{1}"}, + {0, "ConsistentProbabilityBased{0}"}, + {0.75, "ConsistentProbabilityBased{0.75}"}, + {0.05, "ConsistentProbabilityBased{0.05}"}, + {0.003, "ConsistentProbabilityBased{0.003}"}, + {0.99999999, "ConsistentProbabilityBased{0.99999999}"}, + {0.00000001, "ConsistentProbabilityBased{1e-08}"}, + {minProb, "ConsistentProbabilityBased{2.168404344971009e-19}"}, + {minProb * 1.5, "ConsistentProbabilityBased{3.2526065174565133e-19}"}, + {3e-19, "ConsistentProbabilityBased{3e-19}"}, + + // out-of-range > 1 + {1.01, "ConsistentProbabilityBased{1}"}, + {101.1, "ConsistentProbabilityBased{1}"}, + + // out-of-range < 2^-62 + {-1, "ConsistentProbabilityBased{0}"}, + {-0.001, "ConsistentProbabilityBased{0}"}, + {minProb * 0.999, "ConsistentProbabilityBased{0}"}, + } { + s := ConsistentProbabilityBased(tc.prob) + require.Equal(t, tc.expect, s.Description(), "%#v", tc.prob) + } +} + +func getUnknowns(otts otelTraceState) string { + otts.pvalue = invalidValue + otts.rvalue = invalidValue + return otts.serialize() +} + +func TestSamplerBehavior(t *testing.T) { + type testGroup struct { + probability float64 + minP uint8 + maxP uint8 + } + type testCase struct { + isRoot bool + parentSampled bool + ctxTracestate string + hasErrors bool + } + + for _, group := range []testGroup{ + {1.0, 0, 0}, + {0.75, 0, 1}, + {0.5, 1, 1}, + {0, 63, 63}, + } { + t.Run(fmt.Sprint(group.probability), func(t *testing.T) { + for _, test := range []testCase{ + // roots do not care if the context is + // sampled, however preserve other + // otel tracestate keys + {true, false, "", false}, + {true, false, "a:b", false}, + + // non-roots insert r + {false, true, "", false}, + {false, true, "a:b", false}, + {false, false, "", false}, + {false, false, "a:b", false}, + + // error cases: r-p inconsistency + {false, true, "r:10;p:20", true}, + {false, true, "r:10;p:20;a:b", true}, + {false, false, "r:10;p:5", true}, + {false, false, "r:10;p:5;a:b", true}, + + // error cases: out-of-range + {false, false, "r:100", true}, + {false, false, "r:100;a:b", true}, + {false, true, "r:100;p:100", true}, + {false, true, "r:100;p:100;a:b", true}, + {false, true, "r:10;p:100", true}, + {false, true, "r:10;p:100;a:b", true}, + } { + t.Run(testName(test.ctxTracestate), func(t *testing.T) { + handler := &testErrorHandler{} + otel.SetErrorHandler(handler) + + src := rand.NewSource(99999199999) + sampler := ConsistentProbabilityBased(group.probability, WithRandomSource(src)) + + traceID, _ := trace.TraceIDFromHex("4bf92f3577b34da6a3ce929d0e0e4736") + spanID, _ := trace.SpanIDFromHex("00f067aa0ba902b7") + + traceState := trace.TraceState{} + if test.ctxTracestate != "" { + var err error + traceState, err = traceState.Insert(traceStateKey, test.ctxTracestate) + require.NoError(t, err) + } + + sccfg := trace.SpanContextConfig{ + TraceState: traceState, + } + + if !test.isRoot { + sccfg.TraceID = traceID + sccfg.SpanID = spanID + } + + if test.parentSampled { + sccfg.TraceFlags = trace.FlagsSampled + } + + parentCtx := trace.ContextWithSpanContext( + context.Background(), + trace.NewSpanContext(sccfg), + ) + + // Note: the error below is sometimes expected + testState, _ := parseOTelTraceState(test.ctxTracestate, test.parentSampled) + hasRValue := testState.hasRValue() + + const repeats = 10 + for i := 0; i < repeats; i++ { + result := sampler.ShouldSample( + sdktrace.SamplingParameters{ + ParentContext: parentCtx, + TraceID: traceID, + Name: "test", + Kind: trace.SpanKindServer, + }, + ) + sampled := result.Decision == sdktrace.RecordAndSample + + // The result is deterministically random. Parse the tracestate + // to see that it is consistent. + otts, err := parseOTelTraceState(result.Tracestate.Get(traceStateKey), sampled) + require.NoError(t, err) + require.True(t, otts.hasRValue()) + require.Equal(t, []attribute.KeyValue(nil), result.Attributes) + + if otts.hasPValue() { + require.LessOrEqual(t, group.minP, otts.pvalue) + require.LessOrEqual(t, otts.pvalue, group.maxP) + require.Equal(t, sdktrace.RecordAndSample, result.Decision) + } else { + require.Equal(t, sdktrace.Drop, result.Decision) + } + + require.Equal(t, getUnknowns(testState), getUnknowns(otts)) + + if hasRValue { + require.Equal(t, testState.rvalue, otts.rvalue) + } + + if test.hasErrors { + require.Less(t, 0, len(handler.Errors())) + } else { + require.Equal(t, 0, len(handler.Errors())) + } + } + }) + } + }) + } +} + func sampleTrials(t *testing.T, prob float64, degrees testDegrees, upperP pValue, source rand.Source) (float64, []float64) { ctx := context.Background() @@ -218,8 +379,10 @@ func sampleTrials(t *testing.T, prob float64, degrees testDegrees, upperP pValue return chi2, expected } -func TestStatisticalTest(t *testing.T) { - // This test is seeded with a "palindromic wing prime". +func TestSamplerStatistics(t *testing.T) { + if testing.Short() { + t.Skip("statistical test does not run in short mode") + } seedBankRng := rand.New(rand.NewSource(77777677777)) seedBank := make([]int64, 15) // N.B. Max=14 below. for i := range seedBank { @@ -263,26 +426,26 @@ func TestStatisticalTest(t *testing.T) { for _, test := range []testCase{ // Non-powers of two {0.90000, 1, twoDegrees, 5}, - // {0.60000, 1, twoDegrees, 14}, - // {0.33000, 2, twoDegrees, 3}, - // {0.13000, 3, twoDegrees, 2}, - // {0.10000, 4, twoDegrees, 0}, - // {0.05000, 5, twoDegrees, 0}, - // {0.01700, 6, twoDegrees, 2}, - // {0.01000, 7, twoDegrees, 3}, - // {0.00500, 8, twoDegrees, 1}, - // {0.00290, 9, twoDegrees, 1}, - // {0.00100, 10, twoDegrees, 5}, - // {0.00050, 11, twoDegrees, 1}, - // {0.00026, 12, twoDegrees, 3}, - // {0.00023, 13, twoDegrees, 0}, - // {0.00010, 14, twoDegrees, 2}, + {0.60000, 1, twoDegrees, 14}, + {0.33000, 2, twoDegrees, 3}, + {0.13000, 3, twoDegrees, 2}, + {0.10000, 4, twoDegrees, 0}, + {0.05000, 5, twoDegrees, 0}, + {0.01700, 6, twoDegrees, 2}, + {0.01000, 7, twoDegrees, 3}, + {0.00500, 8, twoDegrees, 1}, + {0.00290, 9, twoDegrees, 1}, + {0.00100, 10, twoDegrees, 5}, + {0.00050, 11, twoDegrees, 1}, + {0.00026, 12, twoDegrees, 3}, + {0.00023, 13, twoDegrees, 0}, + {0.00010, 14, twoDegrees, 2}, // Powers of two - // {0x1p-1, 1, oneDegree, 0}, - // {0x1p-4, 4, oneDegree, 2}, - // {0x1p-7, 7, oneDegree, 3}, - // {0x1p-10, 10, oneDegree, 0}, + {0x1p-1, 1, oneDegree, 0}, + {0x1p-4, 4, oneDegree, 2}, + {0x1p-7, 7, oneDegree, 3}, + {0x1p-10, 10, oneDegree, 0}, {0x1p-13, 13, oneDegree, 1}, } { var expected []float64 diff --git a/samplers/probability/consistent/tracestate.go b/samplers/probability/consistent/tracestate.go index a8761d5c80b..38fb3b8976c 100644 --- a/samplers/probability/consistent/tracestate.go +++ b/samplers/probability/consistent/tracestate.go @@ -107,6 +107,10 @@ func parseOTelTraceState(ts string, isSampled bool) (otelTraceState, error) { var pval, rval string var unknown []string + if len(ts) == 0 { + return newTraceState(), nil + } + if len(ts) > traceStateSizeLimit { return newTraceState(), errTraceStateSyntax } From 1c3145f817c81f97a4e9392d2ce16b2495973709 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Wed, 3 Nov 2021 16:10:08 -0700 Subject: [PATCH 15/34] lint --- .github/dependabot.yml | 10 +++++ samplers/probability/consistent/parent.go | 8 +--- .../probability/consistent/parent_test.go | 11 ++--- samplers/probability/consistent/sampler.go | 10 ++--- .../probability/consistent/sampler_test.go | 40 +++++++++---------- samplers/probability/consistent/tracestate.go | 13 +++--- 6 files changed, 49 insertions(+), 43 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 121cd2726ab..35064b8ed5b 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -723,3 +723,13 @@ updates: schedule: interval: "weekly" day: "sunday" + - + package-ecosystem: "gomod" + directory: "/samplers/probability/consistent" + labels: + - dependencies + - go + - "Skip Changelog" + schedule: + interval: "weekly" + day: "sunday" diff --git a/samplers/probability/consistent/parent.go b/samplers/probability/consistent/parent.go index 4673a38ffba..bb5a60b2e00 100644 --- a/samplers/probability/consistent/parent.go +++ b/samplers/probability/consistent/parent.go @@ -28,7 +28,7 @@ type ( } ) -func ConsistentParentProbabilityBased(root sdktrace.Sampler, samplers ...sdktrace.ParentBasedSamplerOption) sdktrace.Sampler { +func ParentProbabilityBased(root sdktrace.Sampler, samplers ...sdktrace.ParentBasedSamplerOption) sdktrace.Sampler { return &parentProbabilitySampler{ delegate: sdktrace.ParentBased(root, samplers...), } @@ -64,9 +64,5 @@ func (p *parentProbabilitySampler) ShouldSample(params sdktrace.SamplingParamete } func (p *parentProbabilitySampler) Description() string { - s := p.delegate.Description() - if strings.HasPrefix(s, "ParentBased") { - s = s[len("ParentBased"):] - } - return "ConsistentParentProbabilityBased" + s + return "ParentProbabilityBased" + strings.TrimPrefix(p.delegate.Description(), "ParentBased") } diff --git a/samplers/probability/consistent/parent_test.go b/samplers/probability/consistent/parent_test.go index d65eef0ae8f..65f0f844c79 100644 --- a/samplers/probability/consistent/parent_test.go +++ b/samplers/probability/consistent/parent_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/attribute" sdktrace "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/trace" @@ -29,14 +30,14 @@ func TestParentSamplerDescription(t *testing.T) { opts := []sdktrace.ParentBasedSamplerOption{ sdktrace.WithRemoteParentNotSampled(sdktrace.AlwaysSample()), } - root := ConsistentProbabilityBased(1) + root := ProbabilityBased(1) compare := sdktrace.ParentBased(root, opts...) - parent := ConsistentParentProbabilityBased(root, opts...) + parent := ParentProbabilityBased(root, opts...) require.Equal(t, strings.Replace( compare.Description(), "ParentBased", - "ConsistentParentProbabilityBased", + "ParentProbabilityBased", 1, ), parent.Description(), @@ -48,7 +49,7 @@ func TestParentSamplerRootContext(t *testing.T) { } func TestParentSamplerValidContext(t *testing.T) { - parent := ConsistentParentProbabilityBased(sdktrace.NeverSample()) + parent := ParentProbabilityBased(sdktrace.NeverSample()) type testCase struct { in string sampled bool @@ -113,7 +114,7 @@ func TestParentSamplerValidContext(t *testing.T) { } func TestParentSamplerInvalidContext(t *testing.T) { - parent := ConsistentParentProbabilityBased(sdktrace.NeverSample()) + parent := ParentProbabilityBased(sdktrace.NeverSample()) type testCase struct { in string sampled bool diff --git a/samplers/probability/consistent/sampler.go b/samplers/probability/consistent/sampler.go index 30897d77914..5b5f5ae579d 100644 --- a/samplers/probability/consistent/sampler.go +++ b/samplers/probability/consistent/sampler.go @@ -26,7 +26,7 @@ import ( ) type ( - ConsistentProbabilityBasedOption interface { + ProbabilityBasedOption interface { apply(*consistentProbabilityBasedConfig) } @@ -64,7 +64,7 @@ type ( // WithRandomSource sets the source of the random number used in this // prototype of OTEP 170, which assumes the randomness is not derived // from the TraceID. -func WithRandomSource(source rand.Source) ConsistentProbabilityBasedOption { +func WithRandomSource(source rand.Source) ProbabilityBasedOption { return consistentProbabilityBasedRandomSource{source} } @@ -72,7 +72,7 @@ func (s consistentProbabilityBasedRandomSource) apply(cfg *consistentProbability cfg.source = s.Source } -// ConsistentProbabilityBased samples a given fraction of traces. Based on the +// ProbabilityBased samples a given fraction of traces. Based on the // OpenTelemetry specification, this Sampler supports only power-of-two // fractions. When the input fraction is not a power of two, it will // be rounded down. @@ -83,7 +83,7 @@ func (s consistentProbabilityBasedRandomSource) apply(cfg *consistentProbability // // To respect the parent trace's `SampledFlag`, this sampler should be // used as the root delegate of a `Parent` sampler. -func ConsistentProbabilityBased(fraction float64, opts ...ConsistentProbabilityBasedOption) sdktrace.Sampler { +func ProbabilityBased(fraction float64, opts ...ProbabilityBasedOption) sdktrace.Sampler { cfg := consistentProbabilityBasedConfig{ source: rand.NewSource(rand.Int63()), } @@ -174,5 +174,5 @@ func (cs *consistentProbabilityBased) Description() string { prob = cs.lowProb * expToFloat64(-int(cs.lowLAC)) prob += (1 - cs.lowProb) * expToFloat64(-int(cs.highLAC)) } - return fmt.Sprintf("ConsistentProbabilityBased{%g}", prob) + return fmt.Sprintf("ProbabilityBased{%g}", prob) } diff --git a/samplers/probability/consistent/sampler_test.go b/samplers/probability/consistent/sampler_test.go index 09e25d869cb..b94d3ca8e22 100644 --- a/samplers/probability/consistent/sampler_test.go +++ b/samplers/probability/consistent/sampler_test.go @@ -25,6 +25,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" sdktrace "go.opentelemetry.io/otel/sdk/trace" @@ -54,11 +55,10 @@ const ( var ( populationSize = 1e6 trials = 20 - significance = 1 / float64(trials) // These may be computed using Gonum, e.g., // import "gonum.org/v1/gonum/stat/distuv" - // with significance = 0.05 + // with significance = 1 / float64(trials) = 0.05 // chiSquaredDF1 = distuv.ChiSquared{K: 1}.Quantile(significance) // chiSquaredDF2 = distuv.ChiSquared{K: 2}.Quantile(significance) // @@ -116,27 +116,27 @@ func TestSamplerDescription(t *testing.T) { prob float64 expect string }{ - {1, "ConsistentProbabilityBased{1}"}, - {0, "ConsistentProbabilityBased{0}"}, - {0.75, "ConsistentProbabilityBased{0.75}"}, - {0.05, "ConsistentProbabilityBased{0.05}"}, - {0.003, "ConsistentProbabilityBased{0.003}"}, - {0.99999999, "ConsistentProbabilityBased{0.99999999}"}, - {0.00000001, "ConsistentProbabilityBased{1e-08}"}, - {minProb, "ConsistentProbabilityBased{2.168404344971009e-19}"}, - {minProb * 1.5, "ConsistentProbabilityBased{3.2526065174565133e-19}"}, - {3e-19, "ConsistentProbabilityBased{3e-19}"}, + {1, "ProbabilityBased{1}"}, + {0, "ProbabilityBased{0}"}, + {0.75, "ProbabilityBased{0.75}"}, + {0.05, "ProbabilityBased{0.05}"}, + {0.003, "ProbabilityBased{0.003}"}, + {0.99999999, "ProbabilityBased{0.99999999}"}, + {0.00000001, "ProbabilityBased{1e-08}"}, + {minProb, "ProbabilityBased{2.168404344971009e-19}"}, + {minProb * 1.5, "ProbabilityBased{3.2526065174565133e-19}"}, + {3e-19, "ProbabilityBased{3e-19}"}, // out-of-range > 1 - {1.01, "ConsistentProbabilityBased{1}"}, - {101.1, "ConsistentProbabilityBased{1}"}, + {1.01, "ProbabilityBased{1}"}, + {101.1, "ProbabilityBased{1}"}, // out-of-range < 2^-62 - {-1, "ConsistentProbabilityBased{0}"}, - {-0.001, "ConsistentProbabilityBased{0}"}, - {minProb * 0.999, "ConsistentProbabilityBased{0}"}, + {-1, "ProbabilityBased{0}"}, + {-0.001, "ProbabilityBased{0}"}, + {minProb * 0.999, "ProbabilityBased{0}"}, } { - s := ConsistentProbabilityBased(tc.prob) + s := ProbabilityBased(tc.prob) require.Equal(t, tc.expect, s.Description(), "%#v", tc.prob) } } @@ -199,7 +199,7 @@ func TestSamplerBehavior(t *testing.T) { otel.SetErrorHandler(handler) src := rand.NewSource(99999199999) - sampler := ConsistentProbabilityBased(group.probability, WithRandomSource(src)) + sampler := ProbabilityBased(group.probability, WithRandomSource(src)) traceID, _ := trace.TraceIDFromHex("4bf92f3577b34da6a3ce929d0e0e4736") spanID, _ := trace.SpanIDFromHex("00f067aa0ba902b7") @@ -281,7 +281,7 @@ func TestSamplerBehavior(t *testing.T) { func sampleTrials(t *testing.T, prob float64, degrees testDegrees, upperP pValue, source rand.Source) (float64, []float64) { ctx := context.Background() - sampler := ConsistentProbabilityBased( + sampler := ProbabilityBased( prob, WithRandomSource(source), ) diff --git a/samplers/probability/consistent/tracestate.go b/samplers/probability/consistent/tracestate.go index 38fb3b8976c..4aedb6be13d 100644 --- a/samplers/probability/consistent/tracestate.go +++ b/samplers/probability/consistent/tracestate.go @@ -32,7 +32,6 @@ const ( var ( errTraceStateSyntax = fmt.Errorf("otel tracestate: %w", strconv.ErrSyntax) errTraceStateInconsistent = fmt.Errorf("r-value and p-value are inconsistent") - errTraceStateSizeLimit = fmt.Errorf("otel tracestate: size limit exceeded") ) type otelTraceState struct { @@ -172,17 +171,17 @@ func parseOTelTraceState(ts string, isSampled bool) (otelTraceState, error) { otts.unknown = unknown // Note: set R before P, so that P won't propagate if R has an error. - if value, err := parseNumber(rValueSubkey, rval, pZeroValue-1); err != nil { + value, err := parseNumber(rValueSubkey, rval, pZeroValue-1) + if err != nil { return otts, err - } else { - otts.rvalue = value } + otts.rvalue = value - if value, err := parseNumber(pValueSubkey, pval, pZeroValue); err != nil { + value, err = parseNumber(pValueSubkey, pval, pZeroValue) + if err != nil { return otts, err - } else { - otts.pvalue = value } + otts.pvalue = value // Invariant checking: unset P when the values are inconsistent. if otts.hasPValue() && otts.hasRValue() { From 72c418efbb92e88b9c42033b608b8e20b96c2bbf Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Wed, 17 Nov 2021 09:07:42 -0800 Subject: [PATCH 16/34] propose short tests --- Makefile | 10 +-- .../probability/consistent/sampler_test.go | 68 +++++++++++-------- 2 files changed, 43 insertions(+), 35 deletions(-) diff --git a/Makefile b/Makefile index 5d6197381c5..7a0f89ccdba 100644 --- a/Makefile +++ b/Makefile @@ -11,7 +11,7 @@ REGISTRY_BASE_URL = https://raw.githubusercontent.com/open-telemetry/opentelemet CONTRIB_REPO_URL = https://github.com/open-telemetry/opentelemetry-go-contrib/tree/main GO = go -GOTEST_MIN = go test -v -timeout 30s +GOTEST_MIN = go test -v -timeout 45s GOTEST = $(GOTEST_MIN) -race GOTEST_WITH_COVERAGE = $(GOTEST) -coverprofile=coverage.out -covermode=atomic @@ -125,17 +125,17 @@ build: .PHONY: test test: set -e; for dir in $(ALL_GO_MOD_DIRS); do \ - echo "go test ./... + race in $${dir}"; \ + echo "go test -short ./... in $${dir}"; \ (cd "$${dir}" && \ - $(GOTEST) ./...); \ + $(GOTEST_MIN) -short ./...); \ done .PHONY: test-short -test-short: +test-long: set -e; for dir in $(ALL_GO_MOD_DIRS); do \ echo "go test ./... + race in $${dir}"; \ (cd "$${dir}" && \ - $(GOTEST_MIN) -short ./...); \ + $(GOTEST) ./...); \ done .PHONY: lint diff --git a/samplers/probability/consistent/sampler_test.go b/samplers/probability/consistent/sampler_test.go index b94d3ca8e22..96e1059dcb7 100644 --- a/samplers/probability/consistent/sampler_test.go +++ b/samplers/probability/consistent/sampler_test.go @@ -23,6 +23,7 @@ import ( "strings" "sync" "testing" + "time" "github.com/stretchr/testify/require" @@ -380,9 +381,7 @@ func sampleTrials(t *testing.T, prob float64, degrees testDegrees, upperP pValue } func TestSamplerStatistics(t *testing.T) { - if testing.Short() { - t.Skip("statistical test does not run in short mode") - } + seedBankRng := rand.New(rand.NewSource(77777677777)) seedBank := make([]int64, 15) // N.B. Max=14 below. for i := range seedBank { @@ -421,33 +420,42 @@ func TestSamplerStatistics(t *testing.T) { expected []float64 } ) - var testSummary []testResult - - for _, test := range []testCase{ - // Non-powers of two - {0.90000, 1, twoDegrees, 5}, - {0.60000, 1, twoDegrees, 14}, - {0.33000, 2, twoDegrees, 3}, - {0.13000, 3, twoDegrees, 2}, - {0.10000, 4, twoDegrees, 0}, - {0.05000, 5, twoDegrees, 0}, - {0.01700, 6, twoDegrees, 2}, - {0.01000, 7, twoDegrees, 3}, - {0.00500, 8, twoDegrees, 1}, - {0.00290, 9, twoDegrees, 1}, - {0.00100, 10, twoDegrees, 5}, - {0.00050, 11, twoDegrees, 1}, - {0.00026, 12, twoDegrees, 3}, - {0.00023, 13, twoDegrees, 0}, - {0.00010, 14, twoDegrees, 2}, - - // Powers of two - {0x1p-1, 1, oneDegree, 0}, - {0x1p-4, 4, oneDegree, 2}, - {0x1p-7, 7, oneDegree, 3}, - {0x1p-10, 10, oneDegree, 0}, - {0x1p-13, 13, oneDegree, 1}, - } { + var ( + testSummary []testResult + + allTests = []testCase{ + // Non-powers of two + {0.90000, 1, twoDegrees, 5}, + {0.60000, 1, twoDegrees, 14}, + {0.33000, 2, twoDegrees, 3}, + {0.13000, 3, twoDegrees, 2}, + {0.10000, 4, twoDegrees, 0}, + {0.05000, 5, twoDegrees, 0}, + {0.01700, 6, twoDegrees, 2}, + {0.01000, 7, twoDegrees, 3}, + {0.00500, 8, twoDegrees, 1}, + {0.00290, 9, twoDegrees, 1}, + {0.00100, 10, twoDegrees, 5}, + {0.00050, 11, twoDegrees, 1}, + {0.00026, 12, twoDegrees, 3}, + {0.00023, 13, twoDegrees, 0}, + {0.00010, 14, twoDegrees, 2}, + + // Powers of two + {0x1p-1, 1, oneDegree, 0}, + {0x1p-4, 4, oneDegree, 2}, + {0x1p-7, 7, oneDegree, 3}, + {0x1p-10, 10, oneDegree, 0}, + {0x1p-13, 13, oneDegree, 1}, + } + ) + + if testing.Short() { + one := rand.New(rand.NewSource(time.Now().UnixNano())).Intn(len(allTests)) + allTests = allTests[one : one+1] + } + + for _, test := range allTests { var expected []float64 t.Run(fmt.Sprint(test.prob), func(t *testing.T) { trySeedIndex := 0 From 4715db2513e7e296b9afffca9315a2455d36dfe0 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Wed, 17 Nov 2021 09:11:31 -0800 Subject: [PATCH 17/34] clarify test settings --- Makefile | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 7a0f89ccdba..d1139a86c66 100644 --- a/Makefile +++ b/Makefile @@ -11,9 +11,9 @@ REGISTRY_BASE_URL = https://raw.githubusercontent.com/open-telemetry/opentelemet CONTRIB_REPO_URL = https://github.com/open-telemetry/opentelemetry-go-contrib/tree/main GO = go -GOTEST_MIN = go test -v -timeout 45s -GOTEST = $(GOTEST_MIN) -race -GOTEST_WITH_COVERAGE = $(GOTEST) -coverprofile=coverage.out -covermode=atomic +GOTEST_TIMED = go test -timeout 45s +GOTEST_RACE = $(GOTEST_TIMED) -race +GOTEST_WITH_COVERAGE = $(GOTEST_RACE) -coverprofile=coverage.out -covermode=atomic .DEFAULT_GOAL := precommit @@ -127,15 +127,15 @@ test: set -e; for dir in $(ALL_GO_MOD_DIRS); do \ echo "go test -short ./... in $${dir}"; \ (cd "$${dir}" && \ - $(GOTEST_MIN) -short ./...); \ + $(GOTEST_TIMED) -short ./...); \ done -.PHONY: test-short +.PHONY: test-long test-long: set -e; for dir in $(ALL_GO_MOD_DIRS); do \ echo "go test ./... + race in $${dir}"; \ (cd "$${dir}" && \ - $(GOTEST) ./...); \ + $(GOTEST_RACE) ./...); \ done .PHONY: lint From 090c7f272e476a0120e8c0f2c2760af6416f8bcc Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Wed, 17 Nov 2021 09:27:05 -0800 Subject: [PATCH 18/34] raise test timeout --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index d1139a86c66..835f1991f64 100644 --- a/Makefile +++ b/Makefile @@ -11,7 +11,7 @@ REGISTRY_BASE_URL = https://raw.githubusercontent.com/open-telemetry/opentelemet CONTRIB_REPO_URL = https://github.com/open-telemetry/opentelemetry-go-contrib/tree/main GO = go -GOTEST_TIMED = go test -timeout 45s +GOTEST_TIMED = go test -timeout 60s GOTEST_RACE = $(GOTEST_TIMED) -race GOTEST_WITH_COVERAGE = $(GOTEST_RACE) -coverprofile=coverage.out -covermode=atomic From fb0fe84415d57dab2257fd929bfd59bbd605d8d0 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Wed, 17 Nov 2021 10:08:13 -0800 Subject: [PATCH 19/34] raise more --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 835f1991f64..4aba534c7f1 100644 --- a/Makefile +++ b/Makefile @@ -11,7 +11,7 @@ REGISTRY_BASE_URL = https://raw.githubusercontent.com/open-telemetry/opentelemet CONTRIB_REPO_URL = https://github.com/open-telemetry/opentelemetry-go-contrib/tree/main GO = go -GOTEST_TIMED = go test -timeout 60s +GOTEST_TIMED = go test -timeout 90s GOTEST_RACE = $(GOTEST_TIMED) -race GOTEST_WITH_COVERAGE = $(GOTEST_RACE) -coverprofile=coverage.out -covermode=atomic From d951723b488408265c5b41dbeea9b2076ea2afb9 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Wed, 17 Nov 2021 10:26:31 -0800 Subject: [PATCH 20/34] disable statistical test w/ race detector --- Makefile | 16 +- .../probability/consistent/sampler_test.go | 153 ------------------ 2 files changed, 8 insertions(+), 161 deletions(-) diff --git a/Makefile b/Makefile index 4aba534c7f1..5d6197381c5 100644 --- a/Makefile +++ b/Makefile @@ -11,9 +11,9 @@ REGISTRY_BASE_URL = https://raw.githubusercontent.com/open-telemetry/opentelemet CONTRIB_REPO_URL = https://github.com/open-telemetry/opentelemetry-go-contrib/tree/main GO = go -GOTEST_TIMED = go test -timeout 90s -GOTEST_RACE = $(GOTEST_TIMED) -race -GOTEST_WITH_COVERAGE = $(GOTEST_RACE) -coverprofile=coverage.out -covermode=atomic +GOTEST_MIN = go test -v -timeout 30s +GOTEST = $(GOTEST_MIN) -race +GOTEST_WITH_COVERAGE = $(GOTEST) -coverprofile=coverage.out -covermode=atomic .DEFAULT_GOAL := precommit @@ -125,17 +125,17 @@ build: .PHONY: test test: set -e; for dir in $(ALL_GO_MOD_DIRS); do \ - echo "go test -short ./... in $${dir}"; \ + echo "go test ./... + race in $${dir}"; \ (cd "$${dir}" && \ - $(GOTEST_TIMED) -short ./...); \ + $(GOTEST) ./...); \ done -.PHONY: test-long -test-long: +.PHONY: test-short +test-short: set -e; for dir in $(ALL_GO_MOD_DIRS); do \ echo "go test ./... + race in $${dir}"; \ (cd "$${dir}" && \ - $(GOTEST_RACE) ./...); \ + $(GOTEST_MIN) -short ./...); \ done .PHONY: lint diff --git a/samplers/probability/consistent/sampler_test.go b/samplers/probability/consistent/sampler_test.go index 96e1059dcb7..b0f915f8548 100644 --- a/samplers/probability/consistent/sampler_test.go +++ b/samplers/probability/consistent/sampler_test.go @@ -23,7 +23,6 @@ import ( "strings" "sync" "testing" - "time" "github.com/stretchr/testify/require" @@ -379,155 +378,3 @@ func sampleTrials(t *testing.T, prob float64, degrees testDegrees, upperP pValue return chi2, expected } - -func TestSamplerStatistics(t *testing.T) { - - seedBankRng := rand.New(rand.NewSource(77777677777)) - seedBank := make([]int64, 15) // N.B. Max=14 below. - for i := range seedBank { - seedBank[i] = seedBankRng.Int63() - } - type ( - testCase struct { - // prob is the sampling probability under test. - prob float64 - - // upperP reflects the larger of the one or two - // distinct adjusted counts represented in the test. - // - // For power-of-two tests, there is one distinct p-value, - // and each span counts as 2**upperP representative spans. - // - // For non-power-of-two tests, there are two distinct - // p-values expected, the test is specified using the - // larger of these values corresponding with the - // smaller sampling probability. The sampling - // probability under test rounded down to the nearest - // power of two is expected to equal 2**(-upperP). - upperP pValue - - // degrees is 1 for power-of-two tests and 2 for - // non-power-of-two tests. - degrees testDegrees - - // seedIndex is the index into seedBank of the test seed. - // If this is -1 the code below will search for the smallest - // seed index that passes the test. - seedIndex int - } - testResult struct { - test testCase - expected []float64 - } - ) - var ( - testSummary []testResult - - allTests = []testCase{ - // Non-powers of two - {0.90000, 1, twoDegrees, 5}, - {0.60000, 1, twoDegrees, 14}, - {0.33000, 2, twoDegrees, 3}, - {0.13000, 3, twoDegrees, 2}, - {0.10000, 4, twoDegrees, 0}, - {0.05000, 5, twoDegrees, 0}, - {0.01700, 6, twoDegrees, 2}, - {0.01000, 7, twoDegrees, 3}, - {0.00500, 8, twoDegrees, 1}, - {0.00290, 9, twoDegrees, 1}, - {0.00100, 10, twoDegrees, 5}, - {0.00050, 11, twoDegrees, 1}, - {0.00026, 12, twoDegrees, 3}, - {0.00023, 13, twoDegrees, 0}, - {0.00010, 14, twoDegrees, 2}, - - // Powers of two - {0x1p-1, 1, oneDegree, 0}, - {0x1p-4, 4, oneDegree, 2}, - {0x1p-7, 7, oneDegree, 3}, - {0x1p-10, 10, oneDegree, 0}, - {0x1p-13, 13, oneDegree, 1}, - } - ) - - if testing.Short() { - one := rand.New(rand.NewSource(time.Now().UnixNano())).Intn(len(allTests)) - allTests = allTests[one : one+1] - } - - for _, test := range allTests { - var expected []float64 - t.Run(fmt.Sprint(test.prob), func(t *testing.T) { - trySeedIndex := 0 - - for { - var seed int64 - seedIndex := test.seedIndex - if seedIndex >= 0 { - seed = seedBank[seedIndex] - } else { - seedIndex = trySeedIndex - seed = seedBank[trySeedIndex] - trySeedIndex++ - } - - countFailures := func(src rand.Source) int { - failed := 0 - - for j := 0; j < trials; j++ { - var x float64 - x, expected = sampleTrials(t, test.prob, test.degrees, test.upperP, src) - - if x < chiSquaredByDF[test.degrees] { - failed++ - } - } - return failed - } - - failed := countFailures(rand.NewSource(seed)) - - if failed != 1 && test.seedIndex < 0 { - t.Logf("%d probabilistic failures, trying a new seed for %g was 0x%x", failed, test.prob, seed) - continue - } else if failed != 1 { - t.Errorf("wrong number of probabilistic failures for %g, should be 1 was %d for seed 0x%x", test.prob, failed, seed) - } else if test.seedIndex < 0 { - t.Logf("update the test for %g to use seed index %d", test.prob, seedIndex) - t.Fail() - return - } else { - // Note: this can be uncommented to verify that the preceding seed failed the test, - // for example: - // if seedIndex != 0 && countFailures(rand.NewSource(seedBank[seedIndex-1])) == 1 { - // t.Logf("update the test for %g to use seed index < %d", test.prob, seedIndex) - // t.Fail() - // } - break - } - } - }) - testSummary = append(testSummary, testResult{ - test: test, - expected: expected, - }) - } - - for idx, res := range testSummary { - var probability, pvalues, expectLower, expectUpper, expectUnsampled string - if res.test.degrees == twoDegrees { - probability = fmt.Sprintf("%.6f", res.test.prob) - pvalues = fmt.Sprint(res.test.upperP-1, ", ", res.test.upperP) - expectUnsampled = fmt.Sprintf("%.10g", res.expected[0]) - expectLower = fmt.Sprintf("%.10g", res.expected[1]) - expectUpper = fmt.Sprintf("%.10g", res.expected[2]) - } else { - probability = fmt.Sprintf("%x (%.6f)", res.test.prob, res.test.prob) - pvalues = fmt.Sprint(res.test.upperP) - expectUnsampled = fmt.Sprintf("%.10g", res.expected[0]) - expectLower = fmt.Sprintf("%.10g", res.expected[1]) - expectUpper = "n/a" - } - t.Logf("| %d | %s | %s | %s | %s | %s |\n", idx+1, probability, pvalues, expectLower, expectUpper, expectUnsampled) - } -} From 6dbfe35628bd847dc45504e689b2381f561117fa Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Wed, 17 Nov 2021 10:28:03 -0800 Subject: [PATCH 21/34] missing file --- .../consistent/statistical_test.go | 176 ++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 samplers/probability/consistent/statistical_test.go diff --git a/samplers/probability/consistent/statistical_test.go b/samplers/probability/consistent/statistical_test.go new file mode 100644 index 00000000000..6f380b72eeb --- /dev/null +++ b/samplers/probability/consistent/statistical_test.go @@ -0,0 +1,176 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build !race + +package consistent + +import ( + "fmt" + "math/rand" + "testing" + "time" +) + +func TestSamplerStatistics(t *testing.T) { + + seedBankRng := rand.New(rand.NewSource(77777677777)) + seedBank := make([]int64, 15) // N.B. Max=14 below. + for i := range seedBank { + seedBank[i] = seedBankRng.Int63() + } + type ( + testCase struct { + // prob is the sampling probability under test. + prob float64 + + // upperP reflects the larger of the one or two + // distinct adjusted counts represented in the test. + // + // For power-of-two tests, there is one distinct p-value, + // and each span counts as 2**upperP representative spans. + // + // For non-power-of-two tests, there are two distinct + // p-values expected, the test is specified using the + // larger of these values corresponding with the + // smaller sampling probability. The sampling + // probability under test rounded down to the nearest + // power of two is expected to equal 2**(-upperP). + upperP pValue + + // degrees is 1 for power-of-two tests and 2 for + // non-power-of-two tests. + degrees testDegrees + + // seedIndex is the index into seedBank of the test seed. + // If this is -1 the code below will search for the smallest + // seed index that passes the test. + seedIndex int + } + testResult struct { + test testCase + expected []float64 + } + ) + var ( + testSummary []testResult + + allTests = []testCase{ + // Non-powers of two + {0.90000, 1, twoDegrees, 5}, + {0.60000, 1, twoDegrees, 14}, + {0.33000, 2, twoDegrees, 3}, + {0.13000, 3, twoDegrees, 2}, + {0.10000, 4, twoDegrees, 0}, + {0.05000, 5, twoDegrees, 0}, + {0.01700, 6, twoDegrees, 2}, + {0.01000, 7, twoDegrees, 3}, + {0.00500, 8, twoDegrees, 1}, + {0.00290, 9, twoDegrees, 1}, + {0.00100, 10, twoDegrees, 5}, + {0.00050, 11, twoDegrees, 1}, + {0.00026, 12, twoDegrees, 3}, + {0.00023, 13, twoDegrees, 0}, + {0.00010, 14, twoDegrees, 2}, + + // Powers of two + {0x1p-1, 1, oneDegree, 0}, + {0x1p-4, 4, oneDegree, 2}, + {0x1p-7, 7, oneDegree, 3}, + {0x1p-10, 10, oneDegree, 0}, + {0x1p-13, 13, oneDegree, 1}, + } + ) + + if testing.Short() { + one := rand.New(rand.NewSource(time.Now().UnixNano())).Intn(len(allTests)) + allTests = allTests[one : one+1] + } + + for _, test := range allTests { + var expected []float64 + t.Run(fmt.Sprint(test.prob), func(t *testing.T) { + trySeedIndex := 0 + + for { + var seed int64 + seedIndex := test.seedIndex + if seedIndex >= 0 { + seed = seedBank[seedIndex] + } else { + seedIndex = trySeedIndex + seed = seedBank[trySeedIndex] + trySeedIndex++ + } + + countFailures := func(src rand.Source) int { + failed := 0 + + for j := 0; j < trials; j++ { + var x float64 + x, expected = sampleTrials(t, test.prob, test.degrees, test.upperP, src) + + if x < chiSquaredByDF[test.degrees] { + failed++ + } + } + return failed + } + + failed := countFailures(rand.NewSource(seed)) + + if failed != 1 && test.seedIndex < 0 { + t.Logf("%d probabilistic failures, trying a new seed for %g was 0x%x", failed, test.prob, seed) + continue + } else if failed != 1 { + t.Errorf("wrong number of probabilistic failures for %g, should be 1 was %d for seed 0x%x", test.prob, failed, seed) + } else if test.seedIndex < 0 { + t.Logf("update the test for %g to use seed index %d", test.prob, seedIndex) + t.Fail() + return + } else { + // Note: this can be uncommented to verify that the preceding seed failed the test, + // for example: + // if seedIndex != 0 && countFailures(rand.NewSource(seedBank[seedIndex-1])) == 1 { + // t.Logf("update the test for %g to use seed index < %d", test.prob, seedIndex) + // t.Fail() + // } + break + } + } + }) + testSummary = append(testSummary, testResult{ + test: test, + expected: expected, + }) + } + + for idx, res := range testSummary { + var probability, pvalues, expectLower, expectUpper, expectUnsampled string + if res.test.degrees == twoDegrees { + probability = fmt.Sprintf("%.6f", res.test.prob) + pvalues = fmt.Sprint(res.test.upperP-1, ", ", res.test.upperP) + expectUnsampled = fmt.Sprintf("%.10g", res.expected[0]) + expectLower = fmt.Sprintf("%.10g", res.expected[1]) + expectUpper = fmt.Sprintf("%.10g", res.expected[2]) + } else { + probability = fmt.Sprintf("%x (%.6f)", res.test.prob, res.test.prob) + pvalues = fmt.Sprint(res.test.upperP) + expectUnsampled = fmt.Sprintf("%.10g", res.expected[0]) + expectLower = fmt.Sprintf("%.10g", res.expected[1]) + expectUpper = "n/a" + } + t.Logf("| %d | %s | %s | %s | %s | %s |\n", idx+1, probability, pvalues, expectLower, expectUpper, expectUnsampled) + } +} From 04dae4b0b9c7f044a0ff45a74695d9e4c000b9f3 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Thu, 18 Nov 2021 16:12:11 -0800 Subject: [PATCH 22/34] test fix --- samplers/probability/consistent/go.mod | 10 ++++++++-- samplers/probability/consistent/go.sum | 6 ++++-- samplers/probability/consistent/statistical_test.go | 10 +++++----- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/samplers/probability/consistent/go.mod b/samplers/probability/consistent/go.mod index 1023c2c9da1..859bd56ae58 100644 --- a/samplers/probability/consistent/go.mod +++ b/samplers/probability/consistent/go.mod @@ -4,7 +4,13 @@ go 1.15 require ( github.com/stretchr/testify v1.7.0 - go.opentelemetry.io/otel v1.1.0 + go.opentelemetry.io/otel v1.2.0 go.opentelemetry.io/otel/sdk v1.1.0 - go.opentelemetry.io/otel/trace v1.1.0 + go.opentelemetry.io/otel/trace v1.2.0 ) + +replace go.opentelemetry.io/otel => ../../../../go + +replace go.opentelemetry.io/otel/trace => ../../../../go/trace + +replace go.opentelemetry.io/otel/sdk => ../../../../go/sdk diff --git a/samplers/probability/consistent/go.sum b/samplers/probability/consistent/go.sum index c1b1589324b..7f5a81b7735 100644 --- a/samplers/probability/consistent/go.sum +++ b/samplers/probability/consistent/go.sum @@ -7,12 +7,14 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -go.opentelemetry.io/otel v1.1.0 h1:8p0uMLcyyIx0KHNTgO8o3CW8A1aA+dJZJW6PvnMz0Wc= go.opentelemetry.io/otel v1.1.0/go.mod h1:7cww0OW51jQ8IaZChIEdqLwgh+44+7uiTdWsAL0wQpA= +go.opentelemetry.io/otel v1.2.0 h1:YOQDvxO1FayUcT9MIhJhgMyNO1WqoduiyvQHzGN0kUQ= +go.opentelemetry.io/otel v1.2.0/go.mod h1:aT17Fk0Z1Nor9e0uisf98LrntPGMnk4frBO9+dkf69I= go.opentelemetry.io/otel/sdk v1.1.0 h1:j/1PngUJIDOddkCILQYTevrTIbWd494djgGkSsMit+U= go.opentelemetry.io/otel/sdk v1.1.0/go.mod h1:3aQvM6uLm6C4wJpHtT8Od3vNzeZ34Pqc6bps8MywWzo= -go.opentelemetry.io/otel/trace v1.1.0 h1:N25T9qCL0+7IpOT8RrRy0WYlL7y6U0WiUJzXcVdXY/o= go.opentelemetry.io/otel/trace v1.1.0/go.mod h1:i47XtdcBQiktu5IsrPqOHe8w+sBmnLwwHt8wiUsWGTI= +go.opentelemetry.io/otel/trace v1.2.0 h1:Ys3iqbqZhcf28hHzrm5WAquMkDHNZTUkw7KHbuNjej0= +go.opentelemetry.io/otel/trace v1.2.0/go.mod h1:N5FLswTubnxKxOJHM7XZC074qpeEdLy3CgAVsdMucK0= golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7 h1:iGu644GcxtEcrInvDsQRCwJjtCIOlT2V7IRt6ah2Whw= golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= diff --git a/samplers/probability/consistent/statistical_test.go b/samplers/probability/consistent/statistical_test.go index 6f380b72eeb..71fb7e81b4c 100644 --- a/samplers/probability/consistent/statistical_test.go +++ b/samplers/probability/consistent/statistical_test.go @@ -99,8 +99,8 @@ func TestSamplerStatistics(t *testing.T) { } for _, test := range allTests { - var expected []float64 t.Run(fmt.Sprint(test.prob), func(t *testing.T) { + var expected []float64 trySeedIndex := 0 for { @@ -149,10 +149,10 @@ func TestSamplerStatistics(t *testing.T) { break } } - }) - testSummary = append(testSummary, testResult{ - test: test, - expected: expected, + testSummary = append(testSummary, testResult{ + test: test, + expected: expected, + }) }) } From 23489ae3470ea363f861d220b3ff49663c642f4b Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Fri, 7 Jan 2022 15:24:32 -0800 Subject: [PATCH 23/34] mod tidy --- samplers/probability/consistent/go.mod | 6 ------ 1 file changed, 6 deletions(-) diff --git a/samplers/probability/consistent/go.mod b/samplers/probability/consistent/go.mod index 859bd56ae58..fbbb8ef1d15 100644 --- a/samplers/probability/consistent/go.mod +++ b/samplers/probability/consistent/go.mod @@ -8,9 +8,3 @@ require ( go.opentelemetry.io/otel/sdk v1.1.0 go.opentelemetry.io/otel/trace v1.2.0 ) - -replace go.opentelemetry.io/otel => ../../../../go - -replace go.opentelemetry.io/otel/trace => ../../../../go/trace - -replace go.opentelemetry.io/otel/sdk => ../../../../go/sdk From 7cfe76683c4a6bebca85a47e503d3ea4aa05a619 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Fri, 4 Feb 2022 14:51:36 -0800 Subject: [PATCH 24/34] shorten the test runtime --- .../probability/consistent/sampler_test.go | 129 ------------- .../consistent/statistical_test.go | 177 ++++++++++++++++-- 2 files changed, 158 insertions(+), 148 deletions(-) diff --git a/samplers/probability/consistent/sampler_test.go b/samplers/probability/consistent/sampler_test.go index b0f915f8548..073bbd96eae 100644 --- a/samplers/probability/consistent/sampler_test.go +++ b/samplers/probability/consistent/sampler_test.go @@ -17,9 +17,7 @@ package consistent import ( "context" "fmt" - "math" "math/rand" - "strconv" "strings" "sync" "testing" @@ -47,32 +45,6 @@ type ( } ) -const ( - oneDegree testDegrees = 1 - twoDegrees testDegrees = 2 -) - -var ( - populationSize = 1e6 - trials = 20 - - // These may be computed using Gonum, e.g., - // import "gonum.org/v1/gonum/stat/distuv" - // with significance = 1 / float64(trials) = 0.05 - // chiSquaredDF1 = distuv.ChiSquared{K: 1}.Quantile(significance) - // chiSquaredDF2 = distuv.ChiSquared{K: 2}.Quantile(significance) - // - // These have been specified using significance = 0.05: - chiSquaredDF1 = 0.003932140000019522 - chiSquaredDF2 = 0.1025865887751011 - - chiSquaredByDF = [3]float64{ - 0, - chiSquaredDF1, - chiSquaredDF2, - } -) - func parsePR(s string) (p, r string) { for _, kvf := range strings.Split(s, ";") { kv := strings.SplitN(kvf, ":", 2) @@ -277,104 +249,3 @@ func TestSamplerBehavior(t *testing.T) { }) } } - -func sampleTrials(t *testing.T, prob float64, degrees testDegrees, upperP pValue, source rand.Source) (float64, []float64) { - ctx := context.Background() - - sampler := ProbabilityBased( - prob, - WithRandomSource(source), - ) - - recorder := &testSpanRecorder{} - provider := sdktrace.NewTracerProvider( - sdktrace.WithSyncer(recorder), - sdktrace.WithSampler(sampler), - ) - - tracer := provider.Tracer("test") - - for i := 0; i < int(populationSize); i++ { - _, span := tracer.Start(ctx, "span") - span.End() - } - - var minP, maxP pValue - - counts := map[pValue]int64{} - - for idx, r := range recorder.spans { - ts := r.SpanContext().TraceState() - p, _ := parsePR(ts.Get("ot")) - - pi, err := strconv.ParseUint(p, 10, 64) - require.NoError(t, err) - - if idx == 0 { - maxP = pValue(pi) - minP = maxP - } else { - if pValue(pi) < minP { - minP = pValue(pi) - } - if pValue(pi) > maxP { - maxP = pValue(pi) - } - } - counts[pValue(pi)]++ - } - - require.Less(t, maxP, minP+pValue(degrees), "%v %v %v", minP, maxP, degrees) - require.Less(t, maxP, pValue(63)) - require.LessOrEqual(t, len(counts), 2) - - var ceilingProb, floorProb, floorChoice float64 - - // Note: we have to test len(counts) == 0 because this outcome - // is actually possible, just very unlikely. If this happens - // during development, a new initial seed must be used for - // this test. - // - // The test specification ensures the test ensures there are - // at least 20 expected items per category in these tests. - require.NotEqual(t, 0, len(counts)) - - if degrees == 2 { - require.Equal(t, minP+1, maxP) - require.Equal(t, upperP, maxP) - ceilingProb = 1 / float64(int64(1)< maxP { + maxP = pValue(pi) + } + } + counts[pValue(pi)]++ + } + + require.Less(t, maxP, minP+pValue(degrees), "%v %v %v", minP, maxP, degrees) + require.Less(t, maxP, pValue(63)) + require.LessOrEqual(t, len(counts), 2) + + var ceilingProb, floorProb, floorChoice float64 + + // Note: we have to test len(counts) == 0 because this outcome + // is actually possible, just very unlikely. If this happens + // during development, a new initial seed must be used for + // this test. + // + // The test specification ensures the test ensures there are + // at least 20 expected items per category in these tests. + require.NotEqual(t, 0, len(counts)) + + if degrees == 2 { + // Note: because the test is probabilistic, we can't be + // sure that both the min and max P values happen. We + // can only assert that one of these is true. + require.GreaterOrEqual(t, maxP, upperP - 1) + require.GreaterOrEqual(t, minP, upperP - 1) + require.LessOrEqual(t, maxP, upperP) + require.LessOrEqual(t, minP, upperP) + require.LessOrEqual(t, maxP-minP, 1) + + ceilingProb = 1 / float64(int64(1)< Date: Fri, 4 Feb 2022 15:19:59 -0800 Subject: [PATCH 25/34] even shorter --- .../beego/otelbeego/example_middleware_test.go | 2 +- .../probability/consistent/statistical_test.go | 16 +++++++--------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/instrumentation/github.com/astaxie/beego/otelbeego/example_middleware_test.go b/instrumentation/github.com/astaxie/beego/otelbeego/example_middleware_test.go index 9501bd0f535..ad67f74e34c 100644 --- a/instrumentation/github.com/astaxie/beego/otelbeego/example_middleware_test.go +++ b/instrumentation/github.com/astaxie/beego/otelbeego/example_middleware_test.go @@ -24,7 +24,7 @@ type ExampleController struct { func (c *ExampleController) Get() { // name of the template in the views directory - c.TplName = "index.tpl" + c.Controller.TplName = "index.tpl" // explicit call to Render if err := Render(&c.Controller); err != nil { diff --git a/samplers/probability/consistent/statistical_test.go b/samplers/probability/consistent/statistical_test.go index dc22a563349..7aa4965fd2e 100644 --- a/samplers/probability/consistent/statistical_test.go +++ b/samplers/probability/consistent/statistical_test.go @@ -113,13 +113,11 @@ func TestSamplerStatistics(t *testing.T) { {0.00290, 9, twoDegrees, 4}, {0.00100, 10, twoDegrees, 6}, {0.00050, 11, twoDegrees, 0}, - {0.00026, 12, twoDegrees, 3}, // Powers of two {0x1p-1, 1, oneDegree, 0}, {0x1p-4, 4, oneDegree, 0}, {0x1p-7, 7, oneDegree, 1}, - {0x1p-10, 10, oneDegree, 1}, } ) @@ -172,11 +170,10 @@ func TestSamplerStatistics(t *testing.T) { } else { // Note: this can be uncommented to verify that the preceding seed failed the test, // for example: - // @@@ - if seedIndex != 0 && countFailures(rand.NewSource(seedBank[seedIndex-1])) == 1 { - t.Logf("update the test for %g to use seed index < %d", test.prob, seedIndex) - t.Fail() - } + // if seedIndex != 0 && countFailures(rand.NewSource(seedBank[seedIndex-1])) == 1 { + // t.Logf("update the test for %g to use seed index < %d", test.prob, seedIndex) + // t.Fail() + // } break } } @@ -277,8 +274,8 @@ func sampleTrials(t *testing.T, prob float64, degrees testDegrees, upperP pValue require.LessOrEqual(t, minP, upperP) require.LessOrEqual(t, maxP-minP, 1) - ceilingProb = 1 / float64(int64(1)< Date: Fri, 4 Feb 2022 15:33:10 -0800 Subject: [PATCH 26/34] shorten the test; lint --- .../probability/consistent/statistical_test.go | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/samplers/probability/consistent/statistical_test.go b/samplers/probability/consistent/statistical_test.go index 7aa4965fd2e..bcdbfe9edb0 100644 --- a/samplers/probability/consistent/statistical_test.go +++ b/samplers/probability/consistent/statistical_test.go @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +//go:build !race // +build !race package consistent @@ -23,7 +24,6 @@ import ( "math/rand" "strconv" "testing" - "time" "github.com/stretchr/testify/require" @@ -36,7 +36,7 @@ const ( ) var ( - trials = 20 + trials = 20 populationSize = 1e5 // These may be computed using Gonum, e.g., @@ -121,11 +121,6 @@ func TestSamplerStatistics(t *testing.T) { } ) - if testing.Short() { - one := rand.New(rand.NewSource(time.Now().UnixNano())).Intn(len(allTests)) - allTests = allTests[one : one+1] - } - for _, test := range allTests { t.Run(fmt.Sprint(test.prob), func(t *testing.T) { var expected []float64 @@ -169,7 +164,7 @@ func TestSamplerStatistics(t *testing.T) { return } else { // Note: this can be uncommented to verify that the preceding seed failed the test, - // for example: + // however this just doubles runtime and adds little evidence. For example: // if seedIndex != 0 && countFailures(rand.NewSource(seedBank[seedIndex-1])) == 1 { // t.Logf("update the test for %g to use seed index < %d", test.prob, seedIndex) // t.Fail() @@ -184,6 +179,8 @@ func TestSamplerStatistics(t *testing.T) { }) } + // Note: This produces a table that should match what is in + // the specification if it's the same test. for idx, res := range testSummary { var probability, pvalues, expectLower, expectUpper, expectUnsampled string if res.test.degrees == twoDegrees { @@ -268,8 +265,8 @@ func sampleTrials(t *testing.T, prob float64, degrees testDegrees, upperP pValue // Note: because the test is probabilistic, we can't be // sure that both the min and max P values happen. We // can only assert that one of these is true. - require.GreaterOrEqual(t, maxP, upperP - 1) - require.GreaterOrEqual(t, minP, upperP - 1) + require.GreaterOrEqual(t, maxP, upperP-1) + require.GreaterOrEqual(t, minP, upperP-1) require.LessOrEqual(t, maxP, upperP) require.LessOrEqual(t, minP, upperP) require.LessOrEqual(t, maxP-minP, 1) From e8257a1a0676a9f390ffb3f1ccfee28e81d5e8c0 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Mon, 14 Feb 2022 12:45:48 -0800 Subject: [PATCH 27/34] run 3 / 15 non-deterministically --- samplers/probability/consistent/statistical_test.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/samplers/probability/consistent/statistical_test.go b/samplers/probability/consistent/statistical_test.go index bcdbfe9edb0..e91ddb7075b 100644 --- a/samplers/probability/consistent/statistical_test.go +++ b/samplers/probability/consistent/statistical_test.go @@ -24,6 +24,7 @@ import ( "math/rand" "strconv" "testing" + "time" "github.com/stretchr/testify/require" @@ -59,7 +60,7 @@ var ( func TestSamplerStatistics(t *testing.T) { seedBankRng := rand.New(rand.NewSource(77777677777)) - seedBank := make([]int64, 15) // N.B. Max=14 below. + seedBank := make([]int64, 7) // N.B. Max=6 below. for i := range seedBank { seedBank[i] = seedBankRng.Int63() } @@ -121,6 +122,13 @@ func TestSamplerStatistics(t *testing.T) { } ) + // Limit the test runtime by choosing 3 of the above + // non-deterministically + rand.New(rand.NewSource(time.Now().UnixNano())).Shuffle(len(allTests), func(i, j int) { + allTests[i], allTests[j] = allTests[j], allTests[i] + }) + allTests = allTests[0:3] + for _, test := range allTests { t.Run(fmt.Sprint(test.prob), func(t *testing.T) { var expected []float64 From 554e3e48a79df8d61bfe37588cf4da7c5bacaf94 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Tue, 22 Feb 2022 10:43:07 -0800 Subject: [PATCH 28/34] Update samplers/probability/consistent/sampler.go --- samplers/probability/consistent/sampler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samplers/probability/consistent/sampler.go b/samplers/probability/consistent/sampler.go index 5b5f5ae579d..745669cf920 100644 --- a/samplers/probability/consistent/sampler.go +++ b/samplers/probability/consistent/sampler.go @@ -79,7 +79,7 @@ func (s consistentProbabilityBasedRandomSource) apply(cfg *consistentProbability // - Fractions >= 1 will always sample. // - Fractions < 2^-62 are treated as zero. // -// This Sampler sets the `sampler.adjusted_count` attribute. +// This Sampler sets the OpenTelemetry tracestate p-value and/or r-value. // // To respect the parent trace's `SampledFlag`, this sampler should be // used as the root delegate of a `Parent` sampler. From 58d6ddc96ff34bb6240a5d9d98d04f7136adbf9a Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Fri, 1 Apr 2022 10:46:04 -0700 Subject: [PATCH 29/34] Apply suggestions from code review Co-authored-by: Tyler Yahn --- samplers/probability/consistent/parent.go | 3 +-- samplers/probability/consistent/sampler.go | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/samplers/probability/consistent/parent.go b/samplers/probability/consistent/parent.go index bb5a60b2e00..f89642eed84 100644 --- a/samplers/probability/consistent/parent.go +++ b/samplers/probability/consistent/parent.go @@ -56,8 +56,7 @@ func (p *parentProbabilitySampler) ShouldSample(params sdktrace.SamplingParamete } // Fix the broken tracestate before calling the delegate. - params.ParentContext = - trace.ContextWithSpanContext(params.ParentContext, psc.WithTraceState(state)) + params.ParentContext = trace.ContextWithSpanContext(params.ParentContext, psc.WithTraceState(state)) } return p.delegate.ShouldSample(params) diff --git a/samplers/probability/consistent/sampler.go b/samplers/probability/consistent/sampler.go index 745669cf920..abe7d596082 100644 --- a/samplers/probability/consistent/sampler.go +++ b/samplers/probability/consistent/sampler.go @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +// Package consistent provides a consistent probability based sampler. package consistent // import "go.opentelemetry.io/contrib/samplers/probability/consistent" import ( From 77e3fbe497ac3462eb7fad88127b2da38ed6ddd8 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Fri, 1 Apr 2022 11:38:13 -0700 Subject: [PATCH 30/34] test one more thing --- samplers/probability/consistent/go.mod | 10 ++--- samplers/probability/consistent/go.sum | 28 ++++++------- samplers/probability/consistent/parent.go | 3 +- .../probability/consistent/parent_test.go | 39 ++++++++++++------- samplers/probability/consistent/sampler.go | 1 + 5 files changed, 47 insertions(+), 34 deletions(-) diff --git a/samplers/probability/consistent/go.mod b/samplers/probability/consistent/go.mod index fbbb8ef1d15..3f21dde3d7e 100644 --- a/samplers/probability/consistent/go.mod +++ b/samplers/probability/consistent/go.mod @@ -1,10 +1,10 @@ module go.opentelemetry.io/contrib/samplers/probability/consistent -go 1.15 +go 1.16 require ( - github.com/stretchr/testify v1.7.0 - go.opentelemetry.io/otel v1.2.0 - go.opentelemetry.io/otel/sdk v1.1.0 - go.opentelemetry.io/otel/trace v1.2.0 + github.com/stretchr/testify v1.7.1 + go.opentelemetry.io/otel v1.6.1 + go.opentelemetry.io/otel/sdk v1.6.1 + go.opentelemetry.io/otel/trace v1.6.1 ) diff --git a/samplers/probability/consistent/go.sum b/samplers/probability/consistent/go.sum index 7f5a81b7735..c2909f67f14 100644 --- a/samplers/probability/consistent/go.sum +++ b/samplers/probability/consistent/go.sum @@ -1,23 +1,25 @@ github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ= -github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= +github.com/go-logr/logr v1.2.3 h1:2DntVwHkVopvECVRSlL5PSo9eG+cAkDCuckLubN+rq0= +github.com/go-logr/logr v1.2.3/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= +github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= +github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= +github.com/google/go-cmp v0.5.7 h1:81/ik6ipDQS2aGcBfIN5dHDB36BwrStyeAQquSYCV4o= +github.com/google/go-cmp v0.5.7/go.mod h1:n+brtR0CgQNWTVd5ZUFpTBC8YFBDLK/h/bpaJ8/DtOE= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= -github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -go.opentelemetry.io/otel v1.1.0/go.mod h1:7cww0OW51jQ8IaZChIEdqLwgh+44+7uiTdWsAL0wQpA= -go.opentelemetry.io/otel v1.2.0 h1:YOQDvxO1FayUcT9MIhJhgMyNO1WqoduiyvQHzGN0kUQ= -go.opentelemetry.io/otel v1.2.0/go.mod h1:aT17Fk0Z1Nor9e0uisf98LrntPGMnk4frBO9+dkf69I= -go.opentelemetry.io/otel/sdk v1.1.0 h1:j/1PngUJIDOddkCILQYTevrTIbWd494djgGkSsMit+U= -go.opentelemetry.io/otel/sdk v1.1.0/go.mod h1:3aQvM6uLm6C4wJpHtT8Od3vNzeZ34Pqc6bps8MywWzo= -go.opentelemetry.io/otel/trace v1.1.0/go.mod h1:i47XtdcBQiktu5IsrPqOHe8w+sBmnLwwHt8wiUsWGTI= -go.opentelemetry.io/otel/trace v1.2.0 h1:Ys3iqbqZhcf28hHzrm5WAquMkDHNZTUkw7KHbuNjej0= -go.opentelemetry.io/otel/trace v1.2.0/go.mod h1:N5FLswTubnxKxOJHM7XZC074qpeEdLy3CgAVsdMucK0= +github.com/stretchr/testify v1.7.1 h1:5TQK59W5E3v0r2duFAb7P95B6hEeOyEnHRa8MjYSMTY= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +go.opentelemetry.io/otel v1.6.1 h1:6r1YrcTenBvYa1x491d0GGpTVBsNECmrc/K6b+zDeis= +go.opentelemetry.io/otel v1.6.1/go.mod h1:blzUabWHkX6LJewxvadmzafgh/wnvBSDBdOuwkAtrWQ= +go.opentelemetry.io/otel/sdk v1.6.1 h1:ZmcNyMhcuAYIb/Nr6QhBPTMopMTbov/47wHt1gibkoY= +go.opentelemetry.io/otel/sdk v1.6.1/go.mod h1:IVYrddmFZ+eJqu2k38qD3WezFR2pymCzm8tdxyh3R4E= +go.opentelemetry.io/otel/trace v1.6.1 h1:f8c93l5tboBYZna1nWk0W9DYyMzJXDWdZcJZ0Kb400U= +go.opentelemetry.io/otel/trace v1.6.1/go.mod h1:RkFRM1m0puWIq10oxImnGEduNBzxiN7TXluRBtE+5j0= golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7 h1:iGu644GcxtEcrInvDsQRCwJjtCIOlT2V7IRt6ah2Whw= golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/samplers/probability/consistent/parent.go b/samplers/probability/consistent/parent.go index bb5a60b2e00..37e874e822a 100644 --- a/samplers/probability/consistent/parent.go +++ b/samplers/probability/consistent/parent.go @@ -56,8 +56,7 @@ func (p *parentProbabilitySampler) ShouldSample(params sdktrace.SamplingParamete } // Fix the broken tracestate before calling the delegate. - params.ParentContext = - trace.ContextWithSpanContext(params.ParentContext, psc.WithTraceState(state)) + params.ParentContext = trace.ContextWithSpanContext(params.ParentContext, psc.WithTraceState(state)) } return p.delegate.ShouldSample(params) diff --git a/samplers/probability/consistent/parent_test.go b/samplers/probability/consistent/parent_test.go index 65f0f844c79..800a4912ade 100644 --- a/samplers/probability/consistent/parent_test.go +++ b/samplers/probability/consistent/parent_test.go @@ -44,10 +44,6 @@ func TestParentSamplerDescription(t *testing.T) { ) } -func TestParentSamplerRootContext(t *testing.T) { - -} - func TestParentSamplerValidContext(t *testing.T) { parent := ParentProbabilityBased(sdktrace.NeverSample()) type testCase struct { @@ -120,7 +116,7 @@ func TestParentSamplerInvalidContext(t *testing.T) { sampled bool expect string } - for _, valid := range []testCase{ + for _, invalid := range []testCase{ // sampled {"r:100", true, ""}, {"r:100;p:1", true, ""}, @@ -133,19 +129,25 @@ func TestParentSamplerInvalidContext(t *testing.T) { {"r:10;p:1", false, "r:10"}, {"r:10;p:1;a:b", false, "r:10;a:b"}, } { - t.Run(testName(valid.in), func(t *testing.T) { + testInvalid := func(t *testing.T, isChildContext bool) { + traceID, _ := trace.TraceIDFromHex("4bf92f3577b34da6a3ce929d0e0e4736") - spanID, _ := trace.SpanIDFromHex("00f067aa0ba902b7") - traceState, err := trace.TraceState{}.Insert(traceStateKey, valid.in) + traceState, err := trace.TraceState{}.Insert(traceStateKey, invalid.in) require.NoError(t, err) sccfg := trace.SpanContextConfig{ - TraceID: traceID, - SpanID: spanID, TraceState: traceState, } + if isChildContext { + spanID, _ := trace.SpanIDFromHex("00f067aa0ba902b7") - if valid.sampled { + sccfg.TraceID = traceID + sccfg.SpanID = spanID + } else { + // Note: this is testing a fabricated situation where + // the context has a tracestate and no TraceID. + } + if invalid.sampled { sccfg.TraceFlags = trace.FlagsSampled } @@ -157,19 +159,28 @@ func TestParentSamplerInvalidContext(t *testing.T) { result := parent.ShouldSample( sdktrace.SamplingParameters{ ParentContext: parentCtx, - TraceID: traceID, + TraceID: sccfg.TraceID, Name: "test", Kind: trace.SpanKindServer, }, ) - if valid.sampled { + if isChildContext && invalid.sampled { require.Equal(t, sdktrace.RecordAndSample, result.Decision) } else { + // if we're not a child context, ShouldSample + // falls through to the delegate, which is NeverSample. require.Equal(t, sdktrace.Drop, result.Decision) } require.Equal(t, []attribute.KeyValue(nil), result.Attributes) - require.Equal(t, valid.expect, result.Tracestate.Get(traceStateKey)) + require.Equal(t, invalid.expect, result.Tracestate.Get(traceStateKey)) + } + + t.Run(testName(invalid.in)+"_with_parent", func(t *testing.T) { + testInvalid(t, true) + }) + t.Run(testName(invalid.in)+"_no_parent", func(t *testing.T) { + testInvalid(t, false) }) } } diff --git a/samplers/probability/consistent/sampler.go b/samplers/probability/consistent/sampler.go index 745669cf920..abe7d596082 100644 --- a/samplers/probability/consistent/sampler.go +++ b/samplers/probability/consistent/sampler.go @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +// Package consistent provides a consistent probability based sampler. package consistent // import "go.opentelemetry.io/contrib/samplers/probability/consistent" import ( From 50516b04893dbabb502906f6d43e4b50873330f3 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Fri, 1 Apr 2022 12:28:45 -0700 Subject: [PATCH 31/34] lint --- samplers/probability/consistent/parent_test.go | 7 ++++--- samplers/probability/consistent/sampler_test.go | 16 ---------------- .../probability/consistent/statistical_test.go | 7 ++++--- 3 files changed, 8 insertions(+), 22 deletions(-) diff --git a/samplers/probability/consistent/parent_test.go b/samplers/probability/consistent/parent_test.go index 800a4912ade..056179d7d50 100644 --- a/samplers/probability/consistent/parent_test.go +++ b/samplers/probability/consistent/parent_test.go @@ -143,9 +143,10 @@ func TestParentSamplerInvalidContext(t *testing.T) { sccfg.TraceID = traceID sccfg.SpanID = spanID - } else { - // Note: this is testing a fabricated situation where - // the context has a tracestate and no TraceID. + + // Note: the other branch is testing a fabricated + // situation where the context has a tracestate and + // no TraceID. } if invalid.sampled { sccfg.TraceFlags = trace.FlagsSampled diff --git a/samplers/probability/consistent/sampler_test.go b/samplers/probability/consistent/sampler_test.go index 073bbd96eae..a7d74a77191 100644 --- a/samplers/probability/consistent/sampler_test.go +++ b/samplers/probability/consistent/sampler_test.go @@ -34,11 +34,6 @@ type ( testDegrees int pValue int - testSpanRecorder struct { - lock sync.Mutex - spans []sdktrace.ReadOnlySpan - } - testErrorHandler struct { lock sync.Mutex errors []error @@ -58,17 +53,6 @@ func parsePR(s string) (p, r string) { return } -func (tsr *testSpanRecorder) ExportSpans(ctx context.Context, spans []sdktrace.ReadOnlySpan) error { - tsr.lock.Lock() - defer tsr.lock.Unlock() - tsr.spans = append(tsr.spans, spans...) - return nil -} - -func (tsr *testSpanRecorder) Shutdown(ctx context.Context) error { - return nil -} - func (eh *testErrorHandler) Handle(err error) { eh.lock.Lock() defer eh.lock.Unlock() diff --git a/samplers/probability/consistent/statistical_test.go b/samplers/probability/consistent/statistical_test.go index e91ddb7075b..8da51ca366b 100644 --- a/samplers/probability/consistent/statistical_test.go +++ b/samplers/probability/consistent/statistical_test.go @@ -29,6 +29,7 @@ import ( "github.com/stretchr/testify/require" sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" ) const ( @@ -216,7 +217,7 @@ func sampleTrials(t *testing.T, prob float64, degrees testDegrees, upperP pValue WithRandomSource(source), ) - recorder := &testSpanRecorder{} + recorder := &tracetest.InMemoryExporter{} provider := sdktrace.NewTracerProvider( sdktrace.WithSyncer(recorder), sdktrace.WithSampler(sampler), @@ -233,8 +234,8 @@ func sampleTrials(t *testing.T, prob float64, degrees testDegrees, upperP pValue counts := map[pValue]int64{} - for idx, r := range recorder.spans { - ts := r.SpanContext().TraceState() + for idx, r := range recorder.GetSpans() { + ts := r.SpanContext.TraceState() p, _ := parsePR(ts.Get("ot")) pi, err := strconv.ParseUint(p, 10, 64) From d3bc61fc486d2e87ccb2891b330b5805ecafb2fa Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Fri, 1 Apr 2022 12:53:43 -0700 Subject: [PATCH 32/34] changelog & readme --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e39fca8fa4..e2cc73b897c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Added + +- Consistent probability sampler implementation. (#1379) + ## [1.6.0/0.31.0] - 2022-03-28 ### Added From 36f3e67b284b05b2ba4bb981f18340106d404c5d Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Fri, 1 Apr 2022 13:19:35 -0700 Subject: [PATCH 33/34] revert accidental golint fix --- .../astaxie/beego/otelbeego/example_middleware_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/github.com/astaxie/beego/otelbeego/example_middleware_test.go b/instrumentation/github.com/astaxie/beego/otelbeego/example_middleware_test.go index ad67f74e34c..9501bd0f535 100644 --- a/instrumentation/github.com/astaxie/beego/otelbeego/example_middleware_test.go +++ b/instrumentation/github.com/astaxie/beego/otelbeego/example_middleware_test.go @@ -24,7 +24,7 @@ type ExampleController struct { func (c *ExampleController) Get() { // name of the template in the views directory - c.Controller.TplName = "index.tpl" + c.TplName = "index.tpl" // explicit call to Render if err := Render(&c.Controller); err != nil { From d2baa5674ffec59f59fe950b3cb59a01567b4d42 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Fri, 1 Apr 2022 13:49:05 -0700 Subject: [PATCH 34/34] godoc --- samplers/probability/consistent/parent.go | 7 +++++++ samplers/probability/consistent/sampler.go | 10 +++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/samplers/probability/consistent/parent.go b/samplers/probability/consistent/parent.go index 37e874e822a..f21052f77b6 100644 --- a/samplers/probability/consistent/parent.go +++ b/samplers/probability/consistent/parent.go @@ -28,12 +28,16 @@ type ( } ) +// ParentProbabilityBased is an implementation of the OpenTelemetry +// Trace Sampler interface that provides additional checks for tracestate +// Probability Sampling fields. func ParentProbabilityBased(root sdktrace.Sampler, samplers ...sdktrace.ParentBasedSamplerOption) sdktrace.Sampler { return &parentProbabilitySampler{ delegate: sdktrace.ParentBased(root, samplers...), } } +// ShouldSample implements "go.opentelemetry.io/otel/sdk/trace".Sampler. func (p *parentProbabilitySampler) ShouldSample(params sdktrace.SamplingParameters) sdktrace.SamplingResult { psc := trace.SpanContextFromContext(params.ParentContext) @@ -62,6 +66,9 @@ func (p *parentProbabilitySampler) ShouldSample(params sdktrace.SamplingParamete return p.delegate.ShouldSample(params) } +// Description returns the same description as the built-in +// ParentBased sampler, with "ParentBased" replaced by +// "ParentProbabilityBased". func (p *parentProbabilitySampler) Description() string { return "ParentProbabilityBased" + strings.TrimPrefix(p.delegate.Description(), "ParentBased") } diff --git a/samplers/probability/consistent/sampler.go b/samplers/probability/consistent/sampler.go index abe7d596082..814f52c503b 100644 --- a/samplers/probability/consistent/sampler.go +++ b/samplers/probability/consistent/sampler.go @@ -27,6 +27,8 @@ import ( ) type ( + // ProbabilityBasedOption is an option to the + // ConssitentProbabilityBased sampler. ProbabilityBasedOption interface { apply(*consistentProbabilityBasedConfig) } @@ -62,9 +64,7 @@ type ( } ) -// WithRandomSource sets the source of the random number used in this -// prototype of OTEP 170, which assumes the randomness is not derived -// from the TraceID. +// WithRandomSource sets the source of the randomness used by the Sampler. func WithRandomSource(source rand.Source) ProbabilityBasedOption { return consistentProbabilityBasedRandomSource{source} } @@ -120,7 +120,7 @@ func (cs *consistentProbabilityBased) lowChoice() bool { return cs.rnd.Float64() < cs.lowProb } -// ShouldSample implements Sampler. +// ShouldSample implements "go.opentelemetry.io/otel/sdk/trace".Sampler. func (cs *consistentProbabilityBased) ShouldSample(p sdktrace.SamplingParameters) sdktrace.SamplingResult { psc := trace.SpanContextFromContext(p.ParentContext) @@ -168,7 +168,7 @@ func (cs *consistentProbabilityBased) ShouldSample(p sdktrace.SamplingParameters } } -// Description implements Sampler. +// Description returns "ProbabilityBased{%g}" with the configured probability. func (cs *consistentProbabilityBased) Description() string { var prob float64 if cs.lowLAC != pZeroValue {