Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a formula for tolerance in sampling tests #298

Merged
merged 1 commit into from
Nov 8, 2019
Merged

Use a formula for tolerance in sampling tests #298

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 27 additions & 20 deletions sdk/trace/trace_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,34 +137,33 @@ func TestRecordingIsOff(t *testing.T) {

func TestSampling(t *testing.T) {
idg := defIDGenerator()
total := 10000
const total = 10000
for name, tc := range map[string]struct {
sampler Sampler
expect float64
tolerance float64
parent bool
sampledParent bool
}{
// Span w/o a parent
"NeverSample": {sampler: NeverSample(), expect: 0, tolerance: 0},
"AlwaysSample": {sampler: AlwaysSample(), expect: 1.0, tolerance: 0},
"ProbabilitySampler_-1": {sampler: ProbabilitySampler(-1.0), expect: 0, tolerance: 0},
"ProbabilitySampler_.25": {sampler: ProbabilitySampler(0.25), expect: .25, tolerance: 0.015},
"ProbabilitySampler_.50": {sampler: ProbabilitySampler(0.50), expect: .5, tolerance: 0.015},
"ProbabilitySampler_.75": {sampler: ProbabilitySampler(0.75), expect: .75, tolerance: 0.015},
"ProbabilitySampler_2.0": {sampler: ProbabilitySampler(2.0), expect: 1, tolerance: 0},
"NeverSample": {sampler: NeverSample(), expect: 0},
"AlwaysSample": {sampler: AlwaysSample(), expect: 1.0},
"ProbabilitySampler_-1": {sampler: ProbabilitySampler(-1.0), expect: 0},
"ProbabilitySampler_.25": {sampler: ProbabilitySampler(0.25), expect: .25},
"ProbabilitySampler_.50": {sampler: ProbabilitySampler(0.50), expect: .5},
"ProbabilitySampler_.75": {sampler: ProbabilitySampler(0.75), expect: .75},
"ProbabilitySampler_2.0": {sampler: ProbabilitySampler(2.0), expect: 1},
// Spans with a parent that is *not* sampled act like spans w/o a parent
"UnsampledParentSpanWithProbabilitySampler_-1": {sampler: ProbabilitySampler(-1.0), expect: 0, tolerance: 0, parent: true},
"UnsampledParentSpanWithProbabilitySampler_.25": {sampler: ProbabilitySampler(.25), expect: .25, tolerance: 0.015, parent: true},
"UnsampledParentSpanWithProbabilitySampler_.50": {sampler: ProbabilitySampler(0.50), expect: .5, tolerance: 0.015, parent: true},
"UnsampledParentSpanWithProbabilitySampler_.75": {sampler: ProbabilitySampler(0.75), expect: .75, tolerance: 0.015, parent: true},
"UnsampledParentSpanWithProbabilitySampler_2.0": {sampler: ProbabilitySampler(2.0), expect: 1, tolerance: 0, parent: true},
"UnsampledParentSpanWithProbabilitySampler_-1": {sampler: ProbabilitySampler(-1.0), expect: 0, parent: true},
"UnsampledParentSpanWithProbabilitySampler_.25": {sampler: ProbabilitySampler(.25), expect: .25, parent: true},
"UnsampledParentSpanWithProbabilitySampler_.50": {sampler: ProbabilitySampler(0.50), expect: .5, parent: true},
"UnsampledParentSpanWithProbabilitySampler_.75": {sampler: ProbabilitySampler(0.75), expect: .75, parent: true},
"UnsampledParentSpanWithProbabilitySampler_2.0": {sampler: ProbabilitySampler(2.0), expect: 1, parent: true},
// Spans with a parent that is sampled, will always sample, regardless of the probability
"SampledParentSpanWithProbabilitySampler_-1": {sampler: ProbabilitySampler(-1.0), expect: 1, tolerance: 0, parent: true, sampledParent: true},
"SampledParentSpanWithProbabilitySampler_.25": {sampler: ProbabilitySampler(.25), expect: 1, tolerance: 0, parent: true, sampledParent: true},
"SampledParentSpanWithProbabilitySampler_2.0": {sampler: ProbabilitySampler(2.0), expect: 1, tolerance: 0, parent: true, sampledParent: true},
"SampledParentSpanWithProbabilitySampler_-1": {sampler: ProbabilitySampler(-1.0), expect: 1, parent: true, sampledParent: true},
"SampledParentSpanWithProbabilitySampler_.25": {sampler: ProbabilitySampler(.25), expect: 1, parent: true, sampledParent: true},
"SampledParentSpanWithProbabilitySampler_2.0": {sampler: ProbabilitySampler(2.0), expect: 1, parent: true, sampledParent: true},
// Spans with a sampled parent, but when using the NeverSample Sampler, aren't sampled
"SampledParentSpanWithNeverSample": {sampler: NeverSample(), expect: 0, tolerance: 0, parent: true, sampledParent: true},
"SampledParentSpanWithNeverSample": {sampler: NeverSample(), expect: 0, parent: true, sampledParent: true},
} {
tc := tc
t.Run(name, func(t *testing.T) {
Expand Down Expand Up @@ -192,10 +191,18 @@ func TestSampling(t *testing.T) {
sampled++
}
}
tolerance := 0.0
got := float64(sampled) / float64(total)

if tc.expect > 0 && tc.expect < 1 {
// See https://en.wikipedia.org/wiki/Binomial_proportion_confidence_interval
const z = 4.75342 // This should succeed 99.9999% of the time
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer if you went with the Wilson score interval with continuity correction.

Joking, of course. ;)

tolerance = z * math.Sqrt(got*(1-got)/total)
}

diff := math.Abs(got - tc.expect)
if diff > tc.tolerance {
t.Errorf("got %f (diff: %f), expected %f (w/tolerance: %f)", got, diff, tc.expect, tc.tolerance)
if diff > tolerance {
t.Errorf("got %f (diff: %f), expected %f (w/tolerance: %f)", got, diff, tc.expect, tolerance)
}
})
}
Expand Down