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.

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

ref: Unify TracesSampler #498

Merged
merged 9 commits into from
Nov 29, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
9 changes: 3 additions & 6 deletions client.go
Original file line number Diff line number Diff line change
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
cleptric marked this conversation as resolved.
Show resolved Hide resolved
// 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 {
cleptric marked this conversation as resolved.
Show resolved Hide resolved
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
cleptric marked this conversation as resolved.
Show resolved Hide resolved
}
transport = httpTransport
Expand Down
23 changes: 12 additions & 11 deletions example/http/main.go
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Loading