Skip to content

Commit

Permalink
ref: Unify TracesSampler (#498)
Browse files Browse the repository at this point in the history
  • Loading branch information
cleptric committed Nov 29, 2022
1 parent 014331c commit c5a9734
Show file tree
Hide file tree
Showing 9 changed files with 150 additions and 235 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,7 @@
# Changelog

- *[breaking]* ref: Unify TracesSampler (#444)

## 0.15.0

- fix: Scope values should not override Event values (#446)
Expand Down
9 changes: 3 additions & 6 deletions client.go
Expand Up @@ -3,7 +3,6 @@ package sentry
import (
"context"
"crypto/x509"
"errors"
"fmt"
"io"
"log"
Expand Down Expand Up @@ -126,6 +125,8 @@ 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.
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.
Expand Down Expand Up @@ -235,10 +236,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 {
Expand Down Expand Up @@ -323,7 +320,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
Expand Down
23 changes: 12 additions & 11 deletions example/http/main.go
Expand Up @@ -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
Expand Down Expand Up @@ -53,30 +53,31 @@ 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,
TracesSampler: sentry.TracesSamplerFunc(func(ctx sentry.SamplingContext) sentry.Sampled {
// 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.
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 {
Expand Down
23 changes: 0 additions & 23 deletions internal/crypto/randutil/randutil.go

This file was deleted.

16 changes: 0 additions & 16 deletions internal/crypto/randutil/randutil_test.go

This file was deleted.

164 changes: 6 additions & 158 deletions 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.
7 changes: 4 additions & 3 deletions traces_sampler_test.go
Expand Up @@ -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)
}
Expand All @@ -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()
Expand All @@ -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++
}
}
Expand Down

0 comments on commit c5a9734

Please sign in to comment.