1
0
mirror of https://github.com/open-telemetry/opentelemetry-go.git synced 2025-11-27 22:49:15 +02:00

Change name of ProbabilitySampler to TraceIdRatioBased (#1115)

* Change name of ProbabilitySampler to TraceIdRatioBased

* Modify behavior to ignore parent span
* Add test for inclusivity property on TraceIdRatioBased sampler
* Modify tests in `trace_test.go` to reflect change in parent
  span behavior

* Add to CHANGELOG

* Satisfy golint

* Update CHANGELOG.md

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
This commit is contained in:
ET
2020-09-03 07:28:01 -07:00
committed by GitHub
parent f38e1902f9
commit 4d83d5b571
5 changed files with 106 additions and 62 deletions

View File

@@ -22,6 +22,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- The `go.opentelemetry.io/otel/api/trace` `TracerOption` was changed to an interface to conform to project option conventions. (#1109) - The `go.opentelemetry.io/otel/api/trace` `TracerOption` was changed to an interface to conform to project option conventions. (#1109)
- Rename Jaeger tags used for instrumentation library information to reflect changes in OpenTelemetry specification. (#1119) - Rename Jaeger tags used for instrumentation library information to reflect changes in OpenTelemetry specification. (#1119)
### Changed
- Rename `ProbabilitySampler` to `TraceIDRatioBased` and change semantics to ignore parent span sampling status. (#1115)
## [0.11.0] - 2020-08-24 ## [0.11.0] - 2020-08-24
### Added ### Added

View File

@@ -36,8 +36,8 @@ func initTracer(url string) {
// Create Zipkin Exporter and install it as a global tracer. // Create Zipkin Exporter and install it as a global tracer.
// //
// For demoing purposes, always sample. In a production application, you should // For demoing purposes, always sample. In a production application, you should
// configure the sampler to a trace.ProbabilitySampler set at the desired // configure the sampler to a trace.ParentSampler(trace.TraceIDRatioBased) set at the desired
// probability. // ratio.
err := zipkin.InstallNewPipeline( err := zipkin.InstallNewPipeline(
url, url,
"zipkin-test", "zipkin-test",

View File

@@ -55,32 +55,29 @@ type SamplingResult struct {
Attributes []label.KeyValue Attributes []label.KeyValue
} }
type probabilitySampler struct { type traceIDRatioSampler struct {
traceIDUpperBound uint64 traceIDUpperBound uint64
description string description string
} }
func (ps probabilitySampler) ShouldSample(p SamplingParameters) SamplingResult { func (ts traceIDRatioSampler) ShouldSample(p SamplingParameters) SamplingResult {
if p.ParentContext.IsSampled() {
return SamplingResult{Decision: RecordAndSampled}
}
x := binary.BigEndian.Uint64(p.TraceID[0:8]) >> 1 x := binary.BigEndian.Uint64(p.TraceID[0:8]) >> 1
if x < ps.traceIDUpperBound { if x < ts.traceIDUpperBound {
return SamplingResult{Decision: RecordAndSampled} return SamplingResult{Decision: RecordAndSampled}
} }
return SamplingResult{Decision: NotRecord} return SamplingResult{Decision: NotRecord}
} }
func (ps probabilitySampler) Description() string { func (ts traceIDRatioSampler) Description() string {
return ps.description return ts.description
} }
// ProbabilitySampler samples a given fraction of traces. Fractions >= 1 will // TraceIDRatioBased samples a given fraction of traces. Fractions >= 1 will
// always sample. If the parent span is sampled, then it's child spans will // always sample. Fractions < 0 are treated as zero. To respect the
// automatically be sampled. Fractions < 0 are treated as zero, but spans may // parent trace's `SampledFlag`, the `TraceIDRatioBased` sampler should be used
// still be sampled if their parent is. // as a delegate of a `Parent` sampler.
func ProbabilitySampler(fraction float64) Sampler { //nolint:golint // golint complains about stutter of `trace.TraceIDRatioBased`
func TraceIDRatioBased(fraction float64) Sampler {
if fraction >= 1 { if fraction >= 1 {
return AlwaysSample() return AlwaysSample()
} }
@@ -89,9 +86,9 @@ func ProbabilitySampler(fraction float64) Sampler {
fraction = 0 fraction = 0
} }
return &probabilitySampler{ return &traceIDRatioSampler{
traceIDUpperBound: uint64(fraction * (1 << 63)), traceIDUpperBound: uint64(fraction * (1 << 63)),
description: fmt.Sprintf("ProbabilitySampler{%g}", fraction), description: fmt.Sprintf("TraceIDRatioBased{%g}", fraction),
} }
} }

View File

@@ -12,67 +12,98 @@
// See the License for the specific language governing permissions and // See the License for the specific language governing permissions and
// limitations under the License. // limitations under the License.
package trace_test package trace
import ( import (
"math/rand"
"testing" "testing"
"go.opentelemetry.io/otel/api/trace" "github.com/stretchr/testify/require"
sdktrace "go.opentelemetry.io/otel/sdk/trace" api "go.opentelemetry.io/otel/api/trace"
) )
func TestAlwaysParentSampleWithParentSampled(t *testing.T) { func TestAlwaysParentSampleWithParentSampled(t *testing.T) {
sampler := sdktrace.ParentSample(sdktrace.AlwaysSample()) sampler := ParentSample(AlwaysSample())
traceID, _ := trace.IDFromHex("4bf92f3577b34da6a3ce929d0e0e4736") traceID, _ := api.IDFromHex("4bf92f3577b34da6a3ce929d0e0e4736")
spanID, _ := trace.SpanIDFromHex("00f067aa0ba902b7") spanID, _ := api.SpanIDFromHex("00f067aa0ba902b7")
parentCtx := trace.SpanContext{ parentCtx := api.SpanContext{
TraceID: traceID, TraceID: traceID,
SpanID: spanID, SpanID: spanID,
TraceFlags: trace.FlagsSampled, TraceFlags: api.FlagsSampled,
} }
if sampler.ShouldSample(sdktrace.SamplingParameters{ParentContext: parentCtx}).Decision != sdktrace.RecordAndSampled { if sampler.ShouldSample(SamplingParameters{ParentContext: parentCtx}).Decision != RecordAndSampled {
t.Error("Sampling decision should be RecordAndSampled") t.Error("Sampling decision should be RecordAndSampled")
} }
} }
func TestNeverParentSampleWithParentSampled(t *testing.T) { func TestNeverParentSampleWithParentSampled(t *testing.T) {
sampler := sdktrace.ParentSample(sdktrace.NeverSample()) sampler := ParentSample(NeverSample())
traceID, _ := trace.IDFromHex("4bf92f3577b34da6a3ce929d0e0e4736") traceID, _ := api.IDFromHex("4bf92f3577b34da6a3ce929d0e0e4736")
spanID, _ := trace.SpanIDFromHex("00f067aa0ba902b7") spanID, _ := api.SpanIDFromHex("00f067aa0ba902b7")
parentCtx := trace.SpanContext{ parentCtx := api.SpanContext{
TraceID: traceID, TraceID: traceID,
SpanID: spanID, SpanID: spanID,
TraceFlags: trace.FlagsSampled, TraceFlags: api.FlagsSampled,
} }
if sampler.ShouldSample(sdktrace.SamplingParameters{ParentContext: parentCtx}).Decision != sdktrace.RecordAndSampled { if sampler.ShouldSample(SamplingParameters{ParentContext: parentCtx}).Decision != RecordAndSampled {
t.Error("Sampling decision should be RecordAndSampled") t.Error("Sampling decision should be RecordAndSampled")
} }
} }
func TestAlwaysParentSampleWithParentNotSampled(t *testing.T) { func TestAlwaysParentSampleWithParentNotSampled(t *testing.T) {
sampler := sdktrace.ParentSample(sdktrace.AlwaysSample()) sampler := ParentSample(AlwaysSample())
traceID, _ := trace.IDFromHex("4bf92f3577b34da6a3ce929d0e0e4736") traceID, _ := api.IDFromHex("4bf92f3577b34da6a3ce929d0e0e4736")
spanID, _ := trace.SpanIDFromHex("00f067aa0ba902b7") spanID, _ := api.SpanIDFromHex("00f067aa0ba902b7")
parentCtx := trace.SpanContext{ parentCtx := api.SpanContext{
TraceID: traceID, TraceID: traceID,
SpanID: spanID, SpanID: spanID,
} }
if sampler.ShouldSample(sdktrace.SamplingParameters{ParentContext: parentCtx}).Decision != sdktrace.NotRecord { if sampler.ShouldSample(SamplingParameters{ParentContext: parentCtx}).Decision != NotRecord {
t.Error("Sampling decision should be NotRecord") t.Error("Sampling decision should be NotRecord")
} }
} }
func TestParentSampleWithNoParent(t *testing.T) { func TestParentSampleWithNoParent(t *testing.T) {
params := sdktrace.SamplingParameters{} params := SamplingParameters{}
sampler := sdktrace.ParentSample(sdktrace.AlwaysSample()) sampler := ParentSample(AlwaysSample())
if sampler.ShouldSample(params).Decision != sdktrace.RecordAndSampled { if sampler.ShouldSample(params).Decision != RecordAndSampled {
t.Error("Sampling decision should be RecordAndSampled") t.Error("Sampling decision should be RecordAndSampled")
} }
sampler = sdktrace.ParentSample(sdktrace.NeverSample()) sampler = ParentSample(NeverSample())
if sampler.ShouldSample(params).Decision != sdktrace.NotRecord { if sampler.ShouldSample(params).Decision != NotRecord {
t.Error("Sampling decision should be NotRecord") t.Error("Sampling decision should be NotRecord")
} }
} }
// TraceIDRatioBased sampler requirements state
// "A TraceIDRatioBased sampler with a given sampling rate MUST also sample
// all traces that any TraceIDRatioBased sampler with a lower sampling rate
// would sample."
func TestTraceIdRatioSamplesInclusively(t *testing.T) {
const (
numSamplers = 1000
numTraces = 100
)
idg := defIDGenerator()
for i := 0; i < numSamplers; i++ {
ratioLo, ratioHi := rand.Float64(), rand.Float64()
if ratioHi < ratioLo {
ratioLo, ratioHi = ratioHi, ratioLo
}
samplerHi := TraceIDRatioBased(ratioHi)
samplerLo := TraceIDRatioBased(ratioLo)
for j := 0; j < numTraces; j++ {
traceID := idg.NewTraceID()
params := SamplingParameters{TraceID: traceID}
if samplerLo.ShouldSample(params).Decision == RecordAndSampled {
require.Equal(t, RecordAndSampled, samplerHi.ShouldSample(params).Decision,
"%s sampled but %s did not", samplerLo.Description(), samplerHi.Description())
}
}
}
}

View File

@@ -59,7 +59,7 @@ func init() {
} }
func TestTracerFollowsExpectedAPIBehaviour(t *testing.T) { func TestTracerFollowsExpectedAPIBehaviour(t *testing.T) {
tp, err := NewProvider(WithConfig(Config{DefaultSampler: ProbabilitySampler(0)})) tp, err := NewProvider(WithConfig(Config{DefaultSampler: TraceIDRatioBased(0)}))
if err != nil { if err != nil {
t.Fatalf("failed to create provider, err: %v\n", err) t.Fatalf("failed to create provider, err: %v\n", err)
} }
@@ -174,25 +174,37 @@ func TestSampling(t *testing.T) {
sampledParent bool sampledParent bool
}{ }{
// Span w/o a parent // Span w/o a parent
"NeverSample": {sampler: NeverSample(), expect: 0}, "NeverSample": {sampler: NeverSample(), expect: 0},
"AlwaysSample": {sampler: AlwaysSample(), expect: 1.0}, "AlwaysSample": {sampler: AlwaysSample(), expect: 1.0},
"ProbabilitySampler_-1": {sampler: ProbabilitySampler(-1.0), expect: 0}, "TraceIdRatioBased_-1": {sampler: TraceIDRatioBased(-1.0), expect: 0},
"ProbabilitySampler_.25": {sampler: ProbabilitySampler(0.25), expect: .25}, "TraceIdRatioBased_.25": {sampler: TraceIDRatioBased(0.25), expect: .25},
"ProbabilitySampler_.50": {sampler: ProbabilitySampler(0.50), expect: .5}, "TraceIdRatioBased_.50": {sampler: TraceIDRatioBased(0.50), expect: .5},
"ProbabilitySampler_.75": {sampler: ProbabilitySampler(0.75), expect: .75}, "TraceIdRatioBased_.75": {sampler: TraceIDRatioBased(0.75), expect: .75},
"ProbabilitySampler_2.0": {sampler: ProbabilitySampler(2.0), expect: 1}, "TraceIdRatioBased_2.0": {sampler: TraceIDRatioBased(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, parent: true}, // Spans w/o a parent and using ParentSample(DelegateSampler()) Sampler, receive DelegateSampler's sampling decision
"UnsampledParentSpanWithProbabilitySampler_.25": {sampler: ProbabilitySampler(.25), expect: .25, parent: true}, "ParentNeverSample": {sampler: ParentSample(NeverSample()), expect: 0},
"UnsampledParentSpanWithProbabilitySampler_.50": {sampler: ProbabilitySampler(0.50), expect: .5, parent: true}, "ParentAlwaysSample": {sampler: ParentSample(AlwaysSample()), expect: 1},
"UnsampledParentSpanWithProbabilitySampler_.75": {sampler: ProbabilitySampler(0.75), expect: .75, parent: true}, "ParentTraceIdRatioBased_.50": {sampler: ParentSample(TraceIDRatioBased(0.50)), expect: .5},
"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 // An unadorned TraceIDRatioBased sampler ignores parent spans
"SampledParentSpanWithProbabilitySampler_-1": {sampler: ProbabilitySampler(-1.0), expect: 1, parent: true, sampledParent: true}, "UnsampledParentSpanWithTraceIdRatioBased_.25": {sampler: TraceIDRatioBased(0.25), expect: .25, parent: true},
"SampledParentSpanWithProbabilitySampler_.25": {sampler: ProbabilitySampler(.25), expect: 1, parent: true, sampledParent: true}, "SampledParentSpanWithTraceIdRatioBased_.25": {sampler: TraceIDRatioBased(0.25), expect: .25, parent: true, sampledParent: true},
"SampledParentSpanWithProbabilitySampler_2.0": {sampler: ProbabilitySampler(2.0), expect: 1, parent: true, sampledParent: true}, "UnsampledParentSpanWithTraceIdRatioBased_.50": {sampler: TraceIDRatioBased(0.50), expect: .5, parent: true},
// Spans with a sampled parent, but when using the NeverSample Sampler, aren't sampled "SampledParentSpanWithTraceIdRatioBased_.50": {sampler: TraceIDRatioBased(0.50), expect: .5, parent: true, sampledParent: true},
"UnsampledParentSpanWithTraceIdRatioBased_.75": {sampler: TraceIDRatioBased(0.75), expect: .75, parent: true},
"SampledParentSpanWithTraceIdRatioBased_.75": {sampler: TraceIDRatioBased(0.75), expect: .75, parent: true, sampledParent: true},
// Spans with a sampled parent but using NeverSample Sampler, are not sampled
"SampledParentSpanWithNeverSample": {sampler: NeverSample(), expect: 0, parent: true, sampledParent: true}, "SampledParentSpanWithNeverSample": {sampler: NeverSample(), expect: 0, parent: true, sampledParent: true},
// Spans with a sampled parent and using ParentSample(DelegateSampler()) Sampler, inherit the parent span's sampling status
"SampledParentSpanWithParentNeverSample": {sampler: ParentSample(NeverSample()), expect: 1, parent: true, sampledParent: true},
"UnsampledParentSpanWithParentNeverSampler": {sampler: ParentSample(NeverSample()), expect: 0, parent: true, sampledParent: false},
"SampledParentSpanWithParentAlwaysSampler": {sampler: ParentSample(AlwaysSample()), expect: 1, parent: true, sampledParent: true},
"UnsampledParentSpanWithParentAlwaysSampler": {sampler: ParentSample(AlwaysSample()), expect: 0, parent: true, sampledParent: false},
"SampledParentSpanWithParentTraceIdRatioBased_.50": {sampler: ParentSample(TraceIDRatioBased(0.50)), expect: 1, parent: true, sampledParent: true},
"UnsampledParentSpanWithParentTraceIdRatioBased_.50": {sampler: ParentSample(TraceIDRatioBased(0.50)), expect: 0, parent: true, sampledParent: false},
} { } {
tc := tc tc := tc
t.Run(name, func(t *testing.T) { t.Run(name, func(t *testing.T) {