From 5a460e6bcebc892e0799910cc7c124a6f1f65254 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 30 Nov 2022 12:33:46 +0100 Subject: [PATCH 1/6] Head of trace --- dynamic_sampling_context.go | 50 +++++++++++++++++- dynamic_sampling_context_test.go | 87 +++++++++++++++++++++++++++++++- http/sentryhttp.go | 7 +-- interfaces.go | 3 +- tracing.go | 55 ++++++++++++++++---- 5 files changed, 185 insertions(+), 17 deletions(-) diff --git a/dynamic_sampling_context.go b/dynamic_sampling_context.go index a9c9d6b82..5c15b1d89 100644 --- a/dynamic_sampling_context.go +++ b/dynamic_sampling_context.go @@ -1,6 +1,7 @@ package sentry import ( + "strconv" "strings" "github.com/getsentry/sentry-go/internal/otel/baggage" @@ -16,7 +17,7 @@ type DynamicSamplingContext struct { Frozen bool } -func NewDynamicSamplingContext(header []byte) (DynamicSamplingContext, error) { +func DynamicSamplingContextFromHeader(header []byte) (DynamicSamplingContext, error) { bag, err := baggage.Parse(string(header)) if err != nil { return DynamicSamplingContext{}, err @@ -36,10 +37,57 @@ func NewDynamicSamplingContext(header []byte) (DynamicSamplingContext, error) { }, nil } +func DynamicSamplingContextFromTransaction(span *Span) (DynamicSamplingContext, error) { + entries := map[string]string{} + + hub := hubFromContext(span.Context()) + scope := hub.Scope() + options := hub.Client().Options() + + if traceID := span.TraceID.String(); traceID != "" { + entries["trace_id"] = traceID + } + if sampleRate := span.sampleRate; sampleRate != 0 { + entries["sample_rate"] = strconv.FormatFloat(sampleRate, 'f', -1, 64) + } + + if dsn := hub.Client().dsn; dsn != nil { + if publicKey := dsn.publicKey; publicKey != "" { + entries["public_key"] = publicKey + } + } + if release := options.Release; release != "" { + entries["release"] = release + } + if environment := options.Environment; environment != "" { + entries["environment"] = environment + } + + // Only include the transaction name if it's of good quality (not empty and not SourceURL) + if span.Source != "" && span.Source != SourceURL { + if transactioName := scope.Transaction(); transactioName != "" { + entries["transaction"] = transactioName + } + } + + if userSegment := scope.user.Segment; userSegment != "" { + entries["user_segment"] = userSegment + } + + return DynamicSamplingContext{ + Entries: entries, + Frozen: true, + }, nil +} + func (d DynamicSamplingContext) HasEntries() bool { return len(d.Entries) > 0 } +func (d DynamicSamplingContext) IsFrozen() bool { + return d.Frozen +} + func (d DynamicSamplingContext) String() string { // TODO implement me return "" diff --git a/dynamic_sampling_context_test.go b/dynamic_sampling_context_test.go index 0be16d555..36be8c50e 100644 --- a/dynamic_sampling_context_test.go +++ b/dynamic_sampling_context_test.go @@ -4,7 +4,7 @@ import ( "testing" ) -func TestNewDynamicSamplingContext(t *testing.T) { +func TestDynamicSamplingContextFromHeader(t *testing.T) { tests := []struct { input []byte want DynamicSamplingContext @@ -41,10 +41,93 @@ func TestNewDynamicSamplingContext(t *testing.T) { } for _, tc := range tests { - got, err := NewDynamicSamplingContext(tc.input) + got, err := DynamicSamplingContextFromHeader(tc.input) if err != nil { t.Fatal(err) } assertEqual(t, got, tc.want) } } + +func TestDynamicSamplingContextFromTransaction(t *testing.T) { + tests := []struct { + input *Span + want DynamicSamplingContext + }{ + // Normal flow + { + input: func() *Span { + ctx := NewTestContext(ClientOptions{ + EnableTracing: true, + TracesSampleRate: 0.5, + Dsn: "http://public@example.com/sentry/1", + Release: "1.0.0", + Environment: "test", + }) + hubFromContext(ctx).ConfigureScope(func(scope *Scope) { + scope.SetUser(User{Segment: "user_segment"}) + }) + txn := StartTransaction(ctx, "name", TransctionSource(SourceCustom)) + txn.TraceID = TraceIDFromHex("d49d9bf66f13450b81f65bc51cf49c03") + return txn + }(), + want: DynamicSamplingContext{ + Frozen: true, + Entries: map[string]string{ + "sample_rate": "0.5", + "trace_id": "d49d9bf66f13450b81f65bc51cf49c03", + "public_key": "public", + "release": "1.0.0", + "environment": "test", + "transaction": "name", + "user_segment": "user_segment", + }, + }, + }, + // Transaction with source url, do not include in Dynamic Sampling context + { + input: func() *Span { + ctx := NewTestContext(ClientOptions{ + EnableTracing: true, + TracesSampleRate: 0.5, + Dsn: "http://public@example.com/sentry/1", + Release: "1.0.0", + }) + txn := StartTransaction(ctx, "name") + txn.TraceID = TraceIDFromHex("d49d9bf66f13450b81f65bc51cf49c03") + return txn + }(), + want: DynamicSamplingContext{ + Frozen: true, + Entries: map[string]string{ + "sample_rate": "0.5", + "trace_id": "d49d9bf66f13450b81f65bc51cf49c03", + "public_key": "public", + "release": "1.0.0", + }, + }, + }, + } + + for _, tc := range tests { + got, err := DynamicSamplingContextFromTransaction(tc.input) + if err != nil { + t.Fatal(err) + } + assertEqual(t, got, tc.want) + } +} + +func TestHasEntries(t *testing.T) { + var dsc DynamicSamplingContext + + dsc = DynamicSamplingContext{} + assertEqual(t, dsc.HasEntries(), false) + + dsc = DynamicSamplingContext{ + Entries: map[string]string{ + "foo": "bar", + }, + } + assertEqual(t, dsc.HasEntries(), true) +} diff --git a/http/sentryhttp.go b/http/sentryhttp.go index 86c5df380..10515310c 100644 --- a/http/sentryhttp.go +++ b/http/sentryhttp.go @@ -71,9 +71,9 @@ func (h *Handler) Handle(handler http.Handler) http.Handler { // where that is convenient. In particular, use it to wrap a handler function // literal. // -// http.Handle(pattern, h.HandleFunc(func (w http.ResponseWriter, r *http.Request) { -// // handler code here -// })) +// http.Handle(pattern, h.HandleFunc(func (w http.ResponseWriter, r *http.Request) { +// // handler code here +// })) func (h *Handler) HandleFunc(handler http.HandlerFunc) http.HandlerFunc { return h.handle(handler) } @@ -89,6 +89,7 @@ func (h *Handler) handle(handler http.Handler) http.HandlerFunc { options := []sentry.SpanOption{ sentry.OpName("http.server"), sentry.ContinueFromRequest(r), + sentry.TransctionSource(sentry.SourceURL), } // We don't mind getting an existing transaction back so we don't need to // check if it is. diff --git a/interfaces.go b/interfaces.go index 01de6c011..c34ec8937 100644 --- a/interfaces.go +++ b/interfaces.go @@ -218,7 +218,8 @@ type Exception struct { // SDKMetaData is a struct to stash data which is needed at some point in the SDK's event processing pipeline // but which shouldn't get send to Sentry. type SDKMetaData struct { - dsc DynamicSamplingContext + dsc DynamicSamplingContext + source Source } // EventID is a hexadecimal string representing a unique uuid4 for an Event. diff --git a/tracing.go b/tracing.go index 6023cd9de..9b359e192 100644 --- a/tracing.go +++ b/tracing.go @@ -28,25 +28,23 @@ type Span struct { //nolint: maligned // prefer readability over optimal memory StartTime time.Time `json:"start_timestamp"` EndTime time.Time `json:"timestamp"` Data map[string]interface{} `json:"data,omitempty"` + Sampled Sampled `json:"-"` + Source Source `json:"-"` - Sampled Sampled `json:"-"` - + // sample rate the span was sampled with. + sampleRate float64 // ctx is the context where the span was started. Always non-nil. ctx context.Context - // Dynamic Sampling context dynamicSamplingContext DynamicSamplingContext - // parent refers to the immediate local parent span. A remote parent span is // only referenced by setting ParentSpanID. parent *Span - // isTransaction is true only for the root span of a local span tree. The // root span is the first span started in a context. Note that a local root // span may have a remote parent belonging to the same trace, therefore // isTransaction depends on ctx and not on parent. isTransaction bool - // recorder stores all spans in a transaction. Guaranteed to be non-nil. recorder *spanRecorder } @@ -141,7 +139,6 @@ func StartSpan(ctx context.Context, operation string, options ...SpanOption) *Sp option(&span) } - // TODO only sample transactions? span.Sampled = span.sample() if hasParent { @@ -273,7 +270,7 @@ func (s *Span) updateFromSentryTrace(header []byte) { func (s *Span) updateFromBaggage(header []byte) { if s.isTransaction { - dsc, err := NewDynamicSamplingContext(header) + dsc, err := DynamicSamplingContextFromHeader(header) if err != nil { return } @@ -311,12 +308,19 @@ func (s *Span) sample() Sampled { // #1 tracing is not enabled. if !clientOptions.EnableTracing { Logger.Printf("Dropping transaction: EnableTracing is set to %t", clientOptions.EnableTracing) + s.sampleRate = 0.0 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) + switch s.Sampled { + case SampledTrue: + s.sampleRate = 1.0 + case SampledFalse: + s.sampleRate = 0.0 + } return s.Sampled } @@ -333,6 +337,7 @@ func (s *Span) sample() Sampled { samplingContext := SamplingContext{Span: s, Parent: s.parent} if sampler != nil { tracesSamplerSampleRate := sampler.Sample(samplingContext) + s.sampleRate = tracesSamplerSampleRate 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 @@ -356,6 +361,7 @@ func (s *Span) sample() Sampled { // #5 use TracesSampleRate from ClientOptions. sampleRate := clientOptions.TracesSampleRate + s.sampleRate = sampleRate if sampleRate < 0.0 || sampleRate > 1.0 { Logger.Printf("Dropping transaction: TracesSamplerRate out of range [0.0, 1.0]: %f", sampleRate) return SampledFalse @@ -388,6 +394,15 @@ func (s *Span) toEvent() *Event { finished = append(finished, child) } + // Create and attach a DynamicSamplingContext to the transaction. + // If the DynamicSamplingContext is not frozen at this point, we can assume being head of trace. + if !s.dynamicSamplingContext.IsFrozen() { + dsc, err := DynamicSamplingContextFromTransaction(s) + if err == nil { + s.dynamicSamplingContext = dsc + } + } + return &Event{ Type: transactionType, Transaction: hub.Scope().Transaction(), @@ -400,7 +415,8 @@ func (s *Span) toEvent() *Event { StartTime: s.StartTime, Spans: finished, sdkMetaData: SDKMetaData{ - dsc: s.dynamicSamplingContext, + dsc: s.dynamicSamplingContext, + source: s.Source, }, } } @@ -459,6 +475,18 @@ var ( zeroSpanID SpanID ) +// Contains information about how the name of the transaction was determined. +type Source string + +const ( + SourceCustom Source = "custom" + SourceURL Source = "url" + SourceRoute Source = "route" + SourceView Source = "view" + SourceComponent Source = "component" + SourceTask Source = "task" +) + // SpanStatus is the status of a span. type SpanStatus uint8 @@ -639,13 +667,20 @@ func OpName(name string) SpanOption { } } +// TransctionSource sets the source of the transaction name. +func TransctionSource(source Source) SpanOption { + return func(s *Span) { + s.Source = source + } +} + // ContinueFromRequest returns a span option that updates the span to continue // an existing trace. If it cannot detect an existing trace in the request, the // span will be left unchanged. // // ContinueFromRequest is an alias for: // -// ContinueFromHeaders(r.Header.Get("sentry-trace"), r.Header.Get("baggage")) +// ContinueFromHeaders(r.Header.Get("sentry-trace"), r.Header.Get("baggage")). func ContinueFromRequest(r *http.Request) SpanOption { return ContinueFromHeaders(r.Header.Get("sentry-trace"), r.Header.Get("baggage")) } From 89fdf403f1b47dbf3e4e17fec89225a63cb85709 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 30 Nov 2022 12:37:08 +0100 Subject: [PATCH 2/6] Apply sampleRate in case of inherting the parent decision --- tracing.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tracing.go b/tracing.go index 9b359e192..817f169c0 100644 --- a/tracing.go +++ b/tracing.go @@ -356,6 +356,12 @@ func (s *Span) sample() Sampled { // #4 inherit parent decision. if s.parent != nil { Logger.Printf("Using sampling decision from parent: %v", s.parent.Sampled) + switch s.parent.Sampled { + case SampledTrue: + s.sampleRate = 1.0 + case SampledFalse: + s.sampleRate = 0.0 + } return s.parent.Sampled } From 30506ea0cf0b746d42ed347ab8989610699d8bdf Mon Sep 17 00:00:00 2001 From: Michi Hoffmann Date: Wed, 30 Nov 2022 13:42:46 +0100 Subject: [PATCH 3/6] Update dynamic_sampling_context.go Co-authored-by: Abhijeet Prasad --- dynamic_sampling_context.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dynamic_sampling_context.go b/dynamic_sampling_context.go index 5c15b1d89..8f07e747f 100644 --- a/dynamic_sampling_context.go +++ b/dynamic_sampling_context.go @@ -65,8 +65,8 @@ func DynamicSamplingContextFromTransaction(span *Span) (DynamicSamplingContext, // Only include the transaction name if it's of good quality (not empty and not SourceURL) if span.Source != "" && span.Source != SourceURL { - if transactioName := scope.Transaction(); transactioName != "" { - entries["transaction"] = transactioName + if transactionName := scope.Transaction(); transactionName != "" { + entries["transaction"] = transactionName } } From b182fc54e4fa80621648d2a6b006b359593b135b Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 30 Nov 2022 13:46:11 +0100 Subject: [PATCH 4/6] Apply review comments --- dynamic_sampling_context.go | 9 +++++---- dynamic_sampling_context_test.go | 5 +---- http/sentryhttp.go | 2 +- tracing.go | 5 +---- 4 files changed, 8 insertions(+), 13 deletions(-) diff --git a/dynamic_sampling_context.go b/dynamic_sampling_context.go index 8f07e747f..1cdab28fe 100644 --- a/dynamic_sampling_context.go +++ b/dynamic_sampling_context.go @@ -37,12 +37,13 @@ func DynamicSamplingContextFromHeader(header []byte) (DynamicSamplingContext, er }, nil } -func DynamicSamplingContextFromTransaction(span *Span) (DynamicSamplingContext, error) { +func DynamicSamplingContextFromTransaction(span *Span) DynamicSamplingContext { entries := map[string]string{} hub := hubFromContext(span.Context()) scope := hub.Scope() - options := hub.Client().Options() + client := hub.Client() + options := client.Options() if traceID := span.TraceID.String(); traceID != "" { entries["trace_id"] = traceID @@ -51,7 +52,7 @@ func DynamicSamplingContextFromTransaction(span *Span) (DynamicSamplingContext, entries["sample_rate"] = strconv.FormatFloat(sampleRate, 'f', -1, 64) } - if dsn := hub.Client().dsn; dsn != nil { + if dsn := client.dsn; dsn != nil { if publicKey := dsn.publicKey; publicKey != "" { entries["public_key"] = publicKey } @@ -77,7 +78,7 @@ func DynamicSamplingContextFromTransaction(span *Span) (DynamicSamplingContext, return DynamicSamplingContext{ Entries: entries, Frozen: true, - }, nil + } } func (d DynamicSamplingContext) HasEntries() bool { diff --git a/dynamic_sampling_context_test.go b/dynamic_sampling_context_test.go index 36be8c50e..ca8bec70d 100644 --- a/dynamic_sampling_context_test.go +++ b/dynamic_sampling_context_test.go @@ -110,10 +110,7 @@ func TestDynamicSamplingContextFromTransaction(t *testing.T) { } for _, tc := range tests { - got, err := DynamicSamplingContextFromTransaction(tc.input) - if err != nil { - t.Fatal(err) - } + got := DynamicSamplingContextFromTransaction(tc.input) assertEqual(t, got, tc.want) } } diff --git a/http/sentryhttp.go b/http/sentryhttp.go index 10515310c..7c21d1cfe 100644 --- a/http/sentryhttp.go +++ b/http/sentryhttp.go @@ -72,7 +72,7 @@ func (h *Handler) Handle(handler http.Handler) http.Handler { // literal. // // http.Handle(pattern, h.HandleFunc(func (w http.ResponseWriter, r *http.Request) { -// // handler code here +// // handler code here // })) func (h *Handler) HandleFunc(handler http.HandlerFunc) http.HandlerFunc { return h.handle(handler) diff --git a/tracing.go b/tracing.go index 817f169c0..aee965859 100644 --- a/tracing.go +++ b/tracing.go @@ -403,10 +403,7 @@ func (s *Span) toEvent() *Event { // Create and attach a DynamicSamplingContext to the transaction. // If the DynamicSamplingContext is not frozen at this point, we can assume being head of trace. if !s.dynamicSamplingContext.IsFrozen() { - dsc, err := DynamicSamplingContextFromTransaction(s) - if err == nil { - s.dynamicSamplingContext = dsc - } + s.dynamicSamplingContext = DynamicSamplingContextFromTransaction(s) } return &Event{ From 204e87b8bbb7c448b266ab7987d1d370ebdbeb4b Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 30 Nov 2022 13:48:24 +0100 Subject: [PATCH 5/6] Ranme Source to TransactionSource --- interfaces.go | 4 ++-- tracing.go | 22 +++++++++++----------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/interfaces.go b/interfaces.go index c34ec8937..5adf12518 100644 --- a/interfaces.go +++ b/interfaces.go @@ -218,8 +218,8 @@ type Exception struct { // SDKMetaData is a struct to stash data which is needed at some point in the SDK's event processing pipeline // but which shouldn't get send to Sentry. type SDKMetaData struct { - dsc DynamicSamplingContext - source Source + dsc DynamicSamplingContext + transactionSource TransactionSource } // EventID is a hexadecimal string representing a unique uuid4 for an Event. diff --git a/tracing.go b/tracing.go index aee965859..33b4e5aa6 100644 --- a/tracing.go +++ b/tracing.go @@ -29,7 +29,7 @@ type Span struct { //nolint: maligned // prefer readability over optimal memory EndTime time.Time `json:"timestamp"` Data map[string]interface{} `json:"data,omitempty"` Sampled Sampled `json:"-"` - Source Source `json:"-"` + Source TransactionSource `json:"-"` // sample rate the span was sampled with. sampleRate float64 @@ -418,8 +418,8 @@ func (s *Span) toEvent() *Event { StartTime: s.StartTime, Spans: finished, sdkMetaData: SDKMetaData{ - dsc: s.dynamicSamplingContext, - source: s.Source, + dsc: s.dynamicSamplingContext, + transactionSource: s.Source, }, } } @@ -479,15 +479,15 @@ var ( ) // Contains information about how the name of the transaction was determined. -type Source string +type TransactionSource string const ( - SourceCustom Source = "custom" - SourceURL Source = "url" - SourceRoute Source = "route" - SourceView Source = "view" - SourceComponent Source = "component" - SourceTask Source = "task" + SourceCustom TransactionSource = "custom" + SourceURL TransactionSource = "url" + SourceRoute TransactionSource = "route" + SourceView TransactionSource = "view" + SourceComponent TransactionSource = "component" + SourceTask TransactionSource = "task" ) // SpanStatus is the status of a span. @@ -671,7 +671,7 @@ func OpName(name string) SpanOption { } // TransctionSource sets the source of the transaction name. -func TransctionSource(source Source) SpanOption { +func TransctionSource(source TransactionSource) SpanOption { return func(s *Span) { s.Source = source } From 84417c128503867c2df8d589b7f061db64f65813 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 30 Nov 2022 13:49:30 +0100 Subject: [PATCH 6/6] Update comment --- http/sentryhttp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/http/sentryhttp.go b/http/sentryhttp.go index 7c21d1cfe..10515310c 100644 --- a/http/sentryhttp.go +++ b/http/sentryhttp.go @@ -72,7 +72,7 @@ func (h *Handler) Handle(handler http.Handler) http.Handler { // literal. // // http.Handle(pattern, h.HandleFunc(func (w http.ResponseWriter, r *http.Request) { -// // handler code here +// // handler code here // })) func (h *Handler) HandleFunc(handler http.HandlerFunc) http.HandlerFunc { return h.handle(handler)