From ac44bee8cb5b02fcd943f12bde8899594ac34fdc Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Sat, 12 Nov 2022 03:56:54 +0100 Subject: [PATCH 1/8] ref: Unify TracesSampler --- example/http/main.go | 15 +- internal/crypto/randutil/randutil.go | 23 --- internal/crypto/randutil/randutil_test.go | 16 --- traces_sampler.go | 164 +--------------------- traces_sampler_test.go | 7 +- tracing.go | 54 ++++--- 6 files changed, 54 insertions(+), 225 deletions(-) delete mode 100644 internal/crypto/randutil/randutil.go delete mode 100644 internal/crypto/randutil/randutil_test.go diff --git a/example/http/main.go b/example/http/main.go index 9a71b4cb..cac33715 100644 --- a/example/http/main.go +++ b/example/http/main.go @@ -3,7 +3,7 @@ // // Try it by running: // -// go run main.go +// go run main.go // // To actually report events to Sentry, set the DSN either by editing the // appropriate line below or setting the environment variable SENTRY_DSN to @@ -56,27 +56,26 @@ func run() error { // Specify either TracesSampleRate or set a TracesSampler to // enable tracing. // TracesSampleRate: 0.5, - TracesSampler: sentry.TracesSamplerFunc(func(ctx sentry.SamplingContext) sentry.Sampled { + TracesSampler: sentry.TracesSampler(func(ctx sentry.SamplingContext) float64 { // As an example, this custom sampler does not send some // transactions to Sentry based on their name. hub := sentry.GetHubFromContext(ctx.Span.Context()) name := hub.Scope().Transaction() if name == "GET /favicon.ico" { - return sentry.SampledFalse + return 0.0 } if strings.HasPrefix(name, "HEAD") { - return sentry.SampledFalse + return 0.0 } - // As an example, sample some transactions with a - // uniform rate. + // As an example, sample some transactions with a uniform rate. if strings.HasPrefix(name, "POST") { - return sentry.UniformTracesSampler(0.2).Sample(ctx) + return 0.2 } // Sample all other transactions for testing. On // production, use TracesSampleRate with a rate adequate // for your traffic, or use the SamplingContext to // customize sampling per-transaction. - return sentry.SampledTrue + return 1.0 }), }) if err != nil { diff --git a/internal/crypto/randutil/randutil.go b/internal/crypto/randutil/randutil.go deleted file mode 100644 index c37cebc8..00000000 --- a/internal/crypto/randutil/randutil.go +++ /dev/null @@ -1,23 +0,0 @@ -package randutil - -import ( - "crypto/rand" - "encoding/binary" -) - -const ( - floatMax = 1 << 53 - floatMask = floatMax - 1 -) - -// Float64 returns a cryptographically secure random number in [0.0, 1.0). -func Float64() float64 { - // The implementation is, in essence: - // return float64(rand.Int63n(1<<53)) / (1<<53) - b := make([]byte, 8) - _, err := rand.Read(b) - if err != nil { - panic(err) - } - return float64(binary.LittleEndian.Uint64(b)&floatMask) / floatMax -} diff --git a/internal/crypto/randutil/randutil_test.go b/internal/crypto/randutil/randutil_test.go deleted file mode 100644 index 6f6a5a4a..00000000 --- a/internal/crypto/randutil/randutil_test.go +++ /dev/null @@ -1,16 +0,0 @@ -package randutil - -import ( - "testing" -) - -func TestFloat64(t *testing.T) { - const total = 1 << 24 - for i := 0; i < total; i++ { - n := Float64() - if !(n >= 0 && n < 1) { - t.Fatalf("out of range [0.0, 1.0): %f", n) - } - } - // TODO: verify that distribution is uniform -} diff --git a/traces_sampler.go b/traces_sampler.go index 3a341aa2..69e7cb7f 100644 --- a/traces_sampler.go +++ b/traces_sampler.go @@ -1,171 +1,19 @@ package sentry -import ( - "fmt" - - "github.com/getsentry/sentry-go/internal/crypto/randutil" -) - -// A TracesSampler makes sampling decisions for spans. -// -// In addition to the sampling context passed to the Sample method, -// implementations may keep and use internal state to make decisions. -// -// Sampling is one of the last steps when starting a new span, such that the -// sampler can inspect most of the state of the span to make a decision. -// -// Implementations must be safe for concurrent use by multiple goroutines. -type TracesSampler interface { - Sample(ctx SamplingContext) Sampled -} - -// Implementation note: -// -// TracesSampler.Sample return type is Sampled (instead of bool or float64), so -// that we can compose samplers by letting a sampler return SampledUndefined to -// defer the decision to the next sampler. -// -// For example, a hypothetical InheritFromParentSampler would return -// SampledUndefined if there is no parent span in the SamplingContext, deferring -// the sampling decision to another sampler, like a UniformSampler. -// -// var _ TracesSampler = sentry.TracesSamplers{ -// sentry.InheritFromParentSampler, -// sentry.UniformTracesSampler(0.1), -// } -// -// Another example, we can provide a sampler that returns SampledFalse if the -// SamplingContext matches some condition, and SampledUndefined otherwise: -// -// var _ TracesSampler = sentry.TracesSamplers{ -// sentry.IgnoreTransaction(regexp.MustCompile(`^\w+ /(favicon.ico|healthz)`), -// sentry.InheritFromParentSampler, -// sentry.UniformTracesSampler(0.1), -// } -// -// If after running all samplers the decision is still undefined, the -// span/transaction is not sampled. - // A SamplingContext is passed to a TracesSampler to determine a sampling // decision. +// +// TODO(tracing): possibly expand SamplingContext to include custom / +// user-provided data. type SamplingContext struct { Span *Span // The current span, always non-nil. Parent *Span // The parent span, may be nil. } -// TODO(tracing): possibly expand SamplingContext to include custom / -// user-provided data. -// -// Unlike in other SDKs, the current http.Request is not part of the -// SamplingContext to avoid bloating it with possibly unnecessary values that -// could confuse people or have negative performance consequences. -// -// For the request to be provided in a SamplingContext, a request pointer would -// most likely need to be stored in the span context and it would open precedent -// for more arbitrary data like fasthttp.Request. -// -// Users wanting to influence the sampling decision based on the request can -// still do so, either by updating the transaction directly on their HTTP -// handler: -// -// func(w http.ResponseWriter, r *http.Request) { -// transaction := sentry.TransactionFromContext(r.Context()) -// if r.Header.Get("X-Custom-Sampling") == "yes" { -// transaction.Sampled = sentry.SampledTrue -// } else { -// transaction.Sampled = sentry.SampledFalse -// } -// } -// -// Or by having their own middleware that stores arbitrary data in the request -// context (a pointer to the request itself included): -// -// type myContextKey struct{} -// type myContextData struct { -// request *http.Request -// // ... -// } -// -// func middleware(h http.Handler) http.Handler { -// return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { -// data := &myContextData{ -// request: r, -// } -// ctx := context.WithValue(r.Context(), myContextKey{}, data) -// h.ServeHTTP(w, r.WithContext(ctx)) -// }) -// } -// -// func main() { -// err := sentry.Init(sentry.ClientOptions{ -// // A custom TracesSampler can access data from the span's context: -// TracesSampler: sentry.TracesSamplerFunc(func(ctx sentry.SamplingContext) bool { -// data, ok := ctx.Span.Context().Value(myContextKey{}).(*myContextData) -// if !ok { -// return false -// } -// return data.request.URL.Hostname() == "example.com" -// }), -// }) -// // ... -// } -// -// Note, however, that for the middleware to be effective, it would have to run -// before sentryhttp's own middleware, meaning the middleware itself is not -// instrumented to send panics to Sentry and it is not part of the timed -// transaction. -// -// If neither of those prove to be sufficient, we can consider including a -// (possibly nil) *http.Request field to SamplingContext. In that case, the SDK -// would need to track the request either in the Scope or the Span.Context. -// -// Alternatively, add a map-like type or simply a generic interface{} similar to -// the CustomSamplingContext type in the Java SDK: -// -// type SamplingContext struct { -// Span *Span // The current span, always non-nil. -// Parent *Span // The parent span, may be nil. -// CustomData interface{} -// } -// -// func CustomSamplingContext(data interface{}) SpanOption { -// return func(s *Span) { -// s.customSamplingContext = data -// } -// } -// -// func main() { -// // ... -// span := sentry.StartSpan(ctx, "op", CustomSamplingContext(data)) -// // ... -// } - -// The TracesSamplerFunc type is an adapter to allow the use of ordinary +// The TracesSample type is an adapter to allow the use of ordinary // functions as a TracesSampler. -type TracesSamplerFunc func(ctx SamplingContext) Sampled - -var _ TracesSampler = TracesSamplerFunc(nil) +type TracesSampler func(ctx SamplingContext) float64 -func (f TracesSamplerFunc) Sample(ctx SamplingContext) Sampled { +func (f TracesSampler) Sample(ctx SamplingContext) float64 { return f(ctx) } - -// UniformTracesSampler is a TracesSampler that samples root spans randomly at a -// uniform rate. -type UniformTracesSampler float64 - -var _ TracesSampler = UniformTracesSampler(0) - -func (s UniformTracesSampler) Sample(ctx SamplingContext) Sampled { - if s < 0.0 || s > 1.0 { - panic(fmt.Errorf("sampling rate out of range [0.0, 1.0]: %f", s)) - } - if randutil.Float64() < float64(s) { - return SampledTrue - } - return SampledFalse -} - -// TODO(tracing): implement and export basic TracesSampler implementations: -// parent-based, span ID / trace ID based, etc. It should be possible to compose -// parent-based with other samplers. diff --git a/traces_sampler_test.go b/traces_sampler_test.go index 18c7806f..ff8519fe 100644 --- a/traces_sampler_test.go +++ b/traces_sampler_test.go @@ -26,7 +26,7 @@ func TestFixedRateSampler(t *testing.T) { for _, tt := range tests { tt := tt t.Run(fmt.Sprint(tt.Rate), func(t *testing.T) { - got := repeatedSample(UniformTracesSampler(tt.Rate), SamplingContext{Span: rootSpan}, 10000) + got := repeatedSample(func(ctx SamplingContext) float64 { return tt.Rate }, SamplingContext{Span: rootSpan}, 10000) if got < tt.Rate*(1-tt.Tolerance) || got > tt.Rate*(1+tt.Tolerance) { t.Errorf("got rootSpan sample rate %.2f, want %.2f±%.0f%%", got, tt.Rate, 100*tt.Tolerance) } @@ -41,7 +41,7 @@ func TestFixedRateSampler(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - repeatedSample(UniformTracesSampler(0.5), SamplingContext{Span: rootSpan}, 10000) + repeatedSample(func(ctx SamplingContext) float64 { return 0.5 }, SamplingContext{Span: rootSpan}, 10000) }() } wg.Wait() @@ -51,7 +51,8 @@ func TestFixedRateSampler(t *testing.T) { func repeatedSample(sampler TracesSampler, ctx SamplingContext, count int) (observedRate float64) { var n float64 for i := 0; i < count; i++ { - if sampler.Sample(ctx).Bool() { + sampleRate := sampler.Sample(ctx) + if rng.Float64() < sampleRate { n++ } } diff --git a/tracing.go b/tracing.go index 118699b0..fa0501ad 100644 --- a/tracing.go +++ b/tracing.go @@ -81,6 +81,7 @@ func StartSpan(ctx context.Context, operation string, options ...SpanOption) *Sp // defaults Op: operation, StartTime: time.Now(), + Sampled: SampledUndefined, ctx: context.WithValue(ctx, spanContextKey{}, &span), parent: parent, @@ -276,38 +277,57 @@ func (s *Span) MarshalJSON() ([]byte, error) { } func (s *Span) sample() Sampled { - // https://develop.sentry.dev/sdk/unified-api/tracing/#sampling + // https://develop.sentry.dev/sdk/performance/#sampling // #1 explicit sampling decision via StartSpan options. if s.Sampled != SampledUndefined { return s.Sampled } + hub := hubFromContext(s.ctx) var clientOptions ClientOptions client := hub.Client() if client != nil { clientOptions = hub.Client().Options() } - samplingContext := SamplingContext{Span: s, Parent: s.parent} - // Variant for non-transaction spans: they inherit the parent decision. - // TracesSampler only runs for the root span. - // Note: non-transaction should always have a parent, but we check both - // conditions anyway -- the first for semantic meaning, the second to - // avoid a nil pointer dereference. - if !s.isTransaction && s.parent != nil { - return s.parent.Sampled - } + // #2 use TracesSampler from ClientOptions. sampler := clientOptions.TracesSampler + samplingContext := SamplingContext{Span: s, Parent: s.parent} if sampler != nil { - return sampler.Sample(samplingContext) + sampleRate := sampler.Sample(samplingContext) + if sampleRate < 0.0 || sampleRate > 1.0 { + Logger.Printf("TracesSampler returned rate is out of range [0.0, 1.0]: %f", sampleRate) + return SampledFalse + } + if sampleRate == 0 { + return SampledFalse + } + + if rng.Float64() < sampleRate { + return SampledTrue + } + return SampledFalse } // #3 inherit parent decision. if s.parent != nil { return s.parent.Sampled } - // #4 uniform sampling using TracesSampleRate. - sampler = UniformTracesSampler(clientOptions.TracesSampleRate) - return sampler.Sample(samplingContext) + + // #4 use TracesSampleRate from ClientOptions. + sampleRate := clientOptions.TracesSampleRate + if sampleRate < 0.0 || sampleRate > 1.0 { + Logger.Printf("TracesSamplerRate out of range [0.0, 1.0]: %f", sampleRate) + return SampledFalse + } + if sampleRate == 0.0 { + return SampledFalse + } + + if rng.Float64() < sampleRate { + return SampledTrue + } + + return SampledFalse } func (s *Span) toEvent() *Event { @@ -530,9 +550,9 @@ type Sampled int8 // The possible trace sampling decisions are: SampledFalse, SampledUndefined // (default) and SampledTrue. const ( - SampledFalse Sampled = -1 + iota - SampledUndefined - SampledTrue + SampledFalse Sampled = -1 + SampledUndefined Sampled = 0 + SampledTrue Sampled = 1 ) func (s Sampled) String() string { From 298a039677a1671db464460d7cd8991d0df2590d Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Mon, 28 Nov 2022 16:41:03 +0100 Subject: [PATCH 2/8] Introduce EnableTracing --- client.go | 11 +++++------ tracing.go | 32 ++++++++++++++++++++++---------- tracing_test.go | 4 ++++ 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/client.go b/client.go index 965b3e75..0d341078 100644 --- a/client.go +++ b/client.go @@ -3,7 +3,6 @@ package sentry import ( "context" "crypto/x509" - "errors" "fmt" "io" "log" @@ -126,6 +125,10 @@ type ClientOptions struct { // 0.0 is treated as if it was 1.0. To drop all events, set the DSN to the // empty string. SampleRate float64 + // Enable performance tracing. + // By default, this is disabled. + // Explicitly set sampling decisions when calling StartSpan/StartTransaction will overwrite this flag. + EnableTracing bool // The sample rate for sampling traces in the range [0.0, 1.0]. TracesSampleRate float64 // Used to customize the sampling of traces, overrides TracesSampleRate. @@ -235,10 +238,6 @@ type Client struct { // single goroutine) or hub methods (for concurrent programs, for example web // servers). func NewClient(options ClientOptions) (*Client, error) { - if options.TracesSampleRate != 0.0 && options.TracesSampler != nil { - return nil, errors.New("TracesSampleRate and TracesSampler are mutually exclusive") - } - if options.Debug { debugWriter := options.DebugWriter if debugWriter == nil { @@ -323,7 +322,7 @@ func (client *Client) setupTransport() { // accommodate more concurrent events. // TODO(tracing): consider using separate buffers per // event type. - if opts.TracesSampleRate != 0 || opts.TracesSampler != nil { + if opts.EnableTracing { httpTransport.BufferSize = 1000 } transport = httpTransport diff --git a/tracing.go b/tracing.go index fa0501ad..b2e33b0c 100644 --- a/tracing.go +++ b/tracing.go @@ -138,6 +138,7 @@ func StartSpan(ctx context.Context, operation string, options ...SpanOption) *Sp option(&span) } + // TODO only sample transactions? span.Sampled = span.sample() if hasParent { @@ -278,8 +279,9 @@ func (s *Span) MarshalJSON() ([]byte, error) { func (s *Span) sample() Sampled { // https://develop.sentry.dev/sdk/performance/#sampling - // #1 explicit sampling decision via StartSpan options. + // #1 explicit sampling decision via StartSpan/StartTransaction options. if s.Sampled != SampledUndefined { + Logger.Printf("Using explicit sampling decision from StartSpan/StartTransaction: %v", s.Sampled) return s.Sampled } @@ -290,36 +292,46 @@ func (s *Span) sample() Sampled { clientOptions = hub.Client().Options() } - // #2 use TracesSampler from ClientOptions. + // #2 sampling is not enabled. + if !clientOptions.EnableTracing { + Logger.Printf("Dropping transaction: EnableTracing is set to %t", clientOptions.EnableTracing) + return SampledFalse + } + + // #3 use TracesSampler from ClientOptions. sampler := clientOptions.TracesSampler samplingContext := SamplingContext{Span: s, Parent: s.parent} if sampler != nil { - sampleRate := sampler.Sample(samplingContext) - if sampleRate < 0.0 || sampleRate > 1.0 { - Logger.Printf("TracesSampler returned rate is out of range [0.0, 1.0]: %f", sampleRate) + tracesSamplerSampleRate := sampler.Sample(samplingContext) + if tracesSamplerSampleRate < 0.0 || tracesSamplerSampleRate > 1.0 { + Logger.Printf("Dropping transaction: Returned TracesSampler rate is out of range [0.0, 1.0]: %f", tracesSamplerSampleRate) return SampledFalse } - if sampleRate == 0 { + if tracesSamplerSampleRate == 0 { + Logger.Printf("Dropping transaction: Returned TracesSampler rate is: %f", tracesSamplerSampleRate) return SampledFalse } - if rng.Float64() < sampleRate { + if rng.Float64() < tracesSamplerSampleRate { return SampledTrue } + Logger.Printf("Dropping transaction: TracesSampler returned rate: %f", tracesSamplerSampleRate) return SampledFalse } - // #3 inherit parent decision. + // #4 inherit parent decision. if s.parent != nil { + Logger.Printf("Using sampling decision from parent: %v", s.parent.Sampled) return s.parent.Sampled } - // #4 use TracesSampleRate from ClientOptions. + // #5 use TracesSampleRate from ClientOptions. sampleRate := clientOptions.TracesSampleRate if sampleRate < 0.0 || sampleRate > 1.0 { - Logger.Printf("TracesSamplerRate out of range [0.0, 1.0]: %f", sampleRate) + Logger.Printf("Dropping transaction: TracesSamplerRate out of range [0.0, 1.0]: %f", sampleRate) return SampledFalse } if sampleRate == 0.0 { + Logger.Printf("Dropping transaction: TracesSampleRate rate is: %f", sampleRate) return SampledFalse } diff --git a/tracing_test.go b/tracing_test.go index 347a6dde..451d2783 100644 --- a/tracing_test.go +++ b/tracing_test.go @@ -168,6 +168,7 @@ func TestStartSpan(t *testing.T) { func TestStartChild(t *testing.T) { transport := &TransportMock{} ctx := NewTestContext(ClientOptions{ + EnableTracing: true, TracesSampleRate: 1.0, Transport: transport, }) @@ -468,7 +469,10 @@ func TestSpanFromContext(t *testing.T) { func TestDoubleSampling(t *testing.T) { transport := &TransportMock{} ctx := NewTestContext(ClientOptions{ + // A SampleRate set to 0.0 will be transformed to 1.0, + // hence we're using math.SmallestNonzeroFloat64. SampleRate: math.SmallestNonzeroFloat64, + EnableTracing: true, TracesSampleRate: 1.0, Transport: transport, }) From adaa4b2de7fbb9eaed02cea8189f7cee4d5bf706 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Mon, 28 Nov 2022 16:49:38 +0100 Subject: [PATCH 3/8] Update net/http example --- example/http/main.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/example/http/main.go b/example/http/main.go index cac33715..25c1bbbd 100644 --- a/example/http/main.go +++ b/example/http/main.go @@ -53,9 +53,11 @@ func run() error { log.Printf("BeforeSend event [%s]", event.EventID) return event }, - // Specify either TracesSampleRate or set a TracesSampler to - // enable tracing. - // TracesSampleRate: 0.5, + // Enable tracing + EnableTracing: true, + // Specify either a TracesSampleRate... + TracesSampleRate: 1.0, + // ... or a TracesSampler TracesSampler: sentry.TracesSampler(func(ctx sentry.SamplingContext) float64 { // As an example, this custom sampler does not send some // transactions to Sentry based on their name. From db01e669e34145576425efe2ba3984bf4db56403 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 29 Nov 2022 11:16:32 +0100 Subject: [PATCH 4/8] Chnage sample order --- tracing.go | 16 ++++++++-------- tracing_test.go | 6 ++++-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/tracing.go b/tracing.go index b2e33b0c..a82dc4c4 100644 --- a/tracing.go +++ b/tracing.go @@ -278,13 +278,6 @@ func (s *Span) MarshalJSON() ([]byte, error) { } func (s *Span) sample() Sampled { - // https://develop.sentry.dev/sdk/performance/#sampling - // #1 explicit sampling decision via StartSpan/StartTransaction options. - if s.Sampled != SampledUndefined { - Logger.Printf("Using explicit sampling decision from StartSpan/StartTransaction: %v", s.Sampled) - return s.Sampled - } - hub := hubFromContext(s.ctx) var clientOptions ClientOptions client := hub.Client() @@ -292,12 +285,19 @@ func (s *Span) sample() Sampled { clientOptions = hub.Client().Options() } - // #2 sampling is not enabled. + // https://develop.sentry.dev/sdk/performance/#sampling + // #1 tracing is not enabled. if !clientOptions.EnableTracing { Logger.Printf("Dropping transaction: EnableTracing is set to %t", clientOptions.EnableTracing) return SampledFalse } + // #2 explicit sampling decision via StartSpan/StartTransaction options. + if s.Sampled != SampledUndefined { + Logger.Printf("Using explicit sampling decision from StartSpan/StartTransaction: %v", s.Sampled) + return s.Sampled + } + // #3 use TracesSampler from ClientOptions. sampler := clientOptions.TracesSampler samplingContext := SamplingContext{Span: s, Parent: s.parent} diff --git a/tracing_test.go b/tracing_test.go index 451d2783..32192cde 100644 --- a/tracing_test.go +++ b/tracing_test.go @@ -91,7 +91,8 @@ func testMarshalJSONOmitEmptyParentSpanID(t *testing.T, v interface{}) { func TestStartSpan(t *testing.T) { transport := &TransportMock{} ctx := NewTestContext(ClientOptions{ - Transport: transport, + EnableTracing: true, + Transport: transport, }) op := "test.op" transaction := "Test Transaction" @@ -230,7 +231,8 @@ func TestStartChild(t *testing.T) { func TestStartTransaction(t *testing.T) { transport := &TransportMock{} ctx := NewTestContext(ClientOptions{ - Transport: transport, + EnableTracing: true, + Transport: transport, }) transactionName := "Test Transaction" description := "A Description" From f66e5d606997dd97371005282c2576687d32aaf1 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 29 Nov 2022 11:41:56 +0100 Subject: [PATCH 5/8] Add sample tets --- tracing_test.go | 59 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/tracing_test.go b/tracing_test.go index 32192cde..c95eec57 100644 --- a/tracing_test.go +++ b/tracing_test.go @@ -496,3 +496,62 @@ func TestDoubleSampling(t *testing.T) { t.Fatalf("got %v event, want %v", got, transactionType) } } + +func TestSample(t *testing.T) { + var ctx context.Context + var span *Span + + // tracing is disabled + ctx = NewTestContext(ClientOptions{ + EnableTracing: false, + }) + span = StartSpan(ctx, "op", TransactionName("name")) + if got := span.Sampled; got != SampledFalse { + t.Fatalf("got %s, want %s", got, SampledFalse) + } + + // explicit sampling decision + ctx = NewTestContext(ClientOptions{ + EnableTracing: true, + TracesSampleRate: 0.0, + }) + span = StartSpan(ctx, "op", TransactionName("name"), func(s *Span) { + s.Sampled = SampledTrue + }) + if got := span.Sampled; got != SampledTrue { + t.Fatalf("got %s, want %s", got, SampledTrue) + } + + // traces sampler + ctx = NewTestContext(ClientOptions{ + EnableTracing: true, + TracesSampler: func(ctx SamplingContext) float64 { + return 1.0 + }, + }) + span = StartSpan(ctx, "op", TransactionName("name")) + if got := span.Sampled; got != SampledTrue { + t.Fatalf("got %s, want %s", got, SampledTrue) + } + + // parent sampling decision + ctx = NewTestContext(ClientOptions{ + EnableTracing: true, + TracesSampleRate: 1.0, + }) + span = StartSpan(ctx, "op", TransactionName("name")) + childSpan := span.StartChild("child") + if got := childSpan.Sampled; got != SampledTrue { + t.Fatalf("got %s, want %s", got, SampledTrue) + } + + // traces sample rate + ctx = NewTestContext(ClientOptions{ + EnableTracing: true, + TracesSampleRate: 1.0, + }) + span = StartSpan(ctx, "op", TransactionName("name")) + if got := span.Sampled; got != SampledTrue { + t.Fatalf("got %s, want %s", got, SampledTrue) + } +} From f8a77f7ad6be8d7f433b1ff2f3cfd193bb0d3061 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 29 Nov 2022 11:46:22 +0100 Subject: [PATCH 6/8] Update doc block --- client.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/client.go b/client.go index 0d341078..9f615b12 100644 --- a/client.go +++ b/client.go @@ -126,8 +126,6 @@ type ClientOptions struct { // empty string. SampleRate float64 // Enable performance tracing. - // By default, this is disabled. - // Explicitly set sampling decisions when calling StartSpan/StartTransaction will overwrite this flag. EnableTracing bool // The sample rate for sampling traces in the range [0.0, 1.0]. TracesSampleRate float64 From 3d52f80755a9c96b4ab187ddb34ed57b70a17e21 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 29 Nov 2022 12:03:47 +0100 Subject: [PATCH 7/8] Add back non-transaction handling --- tracing.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tracing.go b/tracing.go index a82dc4c4..771addef 100644 --- a/tracing.go +++ b/tracing.go @@ -298,6 +298,14 @@ func (s *Span) sample() Sampled { return s.Sampled } + // Variant for non-transaction spans: they inherit the parent decision. + // Note: non-transaction should always have a parent, but we check both + // conditions anyway -- the first for semantic meaning, the second to + // avoid a nil pointer dereference. + if !s.isTransaction && s.parent != nil { + return s.parent.Sampled + } + // #3 use TracesSampler from ClientOptions. sampler := clientOptions.TracesSampler samplingContext := SamplingContext{Span: s, Parent: s.parent} From c01418fd165e2186be5f83f7fba48e19a75e1c0c Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 29 Nov 2022 16:47:23 +0100 Subject: [PATCH 8/8] Add changelog entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5371de1c..164ccce9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Changelog +- *[breaking]* ref: Unify TracesSampler (#444) + ## 0.15.0 - fix: Scope values should not override Event values (#446)