Tracing without performance (#833)

Author: ribice

Diff for: client.go

@@ -90,7 +90,7 @@ type EventProcessor func(event *Event, hint *EventHint) *Event
 // ApplyToEvent changes an event based on external data and/or
 // an event hint.
 type EventModifier interface {
-	ApplyToEvent(event *Event, hint *EventHint) *Event
+	ApplyToEvent(event *Event, hint *EventHint, client *Client) *Event
 }
 
 var globalEventProcessors []EventProcessor
@@ -690,7 +690,7 @@ func (client *Client) prepareEvent(event *Event, hint *EventHint, scope EventMod
 	}
 
 	if scope != nil {
-		event = scope.ApplyToEvent(event, hint)
+		event = scope.ApplyToEvent(event, hint, client)
 		if event == nil {
 			return nil
 		}

Diff for: dynamic_sampling_context.go

@@ -78,15 +78,7 @@ func DynamicSamplingContextFromTransaction(span *Span) DynamicSamplingContext {
 		}
 	}
 
-	if userSegment := scope.user.Segment; userSegment != "" {
-		entries["user_segment"] = userSegment
-	}
-
-	if span.Sampled.Bool() {
-		entries["sampled"] = "true"
-	} else {
-		entries["sampled"] = "false"
-	}
+	entries["sampled"] = strconv.FormatBool(span.Sampled.Bool())
 
 	return DynamicSamplingContext{
 		Entries: entries,
@@ -121,3 +113,43 @@ func (d DynamicSamplingContext) String() string {
 
 	return ""
 }
+
+// Constructs a new DynamicSamplingContext using a scope and client. Accessing
+// fields on the scope are not thread safe, and this function should only be
+// called within scope methods.
+func DynamicSamplingContextFromScope(scope *Scope, client *Client) DynamicSamplingContext {
+	entries := map[string]string{}
+
+	if client == nil || scope == nil {
+		return DynamicSamplingContext{
+			Entries: entries,
+			Frozen:  false,
+		}
+	}
+
+	propagationContext := scope.propagationContext
+
+	if traceID := propagationContext.TraceID.String(); traceID != "" {
+		entries["trace_id"] = traceID
+	}
+	if sampleRate := client.options.TracesSampleRate; sampleRate != 0 {
+		entries["sample_rate"] = strconv.FormatFloat(sampleRate, 'f', -1, 64)
+	}
+
+	if dsn := client.dsn; dsn != nil {
+		if publicKey := dsn.publicKey; publicKey != "" {
+			entries["public_key"] = publicKey
+		}
+	}
+	if release := client.options.Release; release != "" {
+		entries["release"] = release
+	}
+	if environment := client.options.Environment; environment != "" {
+		entries["environment"] = environment
+	}
+
+	return DynamicSamplingContext{
+		Entries: entries,
+		Frozen:  true,
+	}
+}

Diff for: dynamic_sampling_context_test.go

@@ -97,14 +97,13 @@ func TestDynamicSamplingContextFromTransaction(t *testing.T) {
 			want: DynamicSamplingContext{
 				Frozen: true,
 				Entries: map[string]string{
-					"sample_rate":  "1",
-					"trace_id":     "d49d9bf66f13450b81f65bc51cf49c03",
-					"public_key":   "public",
-					"release":      "1.0.0",
-					"environment":  "test",
-					"transaction":  "name",
-					"user_segment": "user_segment",
-					"sampled":      "true",
+					"sample_rate": "1",
+					"trace_id":    "d49d9bf66f13450b81f65bc51cf49c03",
+					"public_key":  "public",
+					"release":     "1.0.0",
+					"environment": "test",
+					"transaction": "name",
+					"sampled":     "true",
 				},
 			},
 		},
@@ -181,3 +180,68 @@ func TestString(t *testing.T) {
 	}
 	testutils.AssertBaggageStringsEqual(t, dsc.String(), "sentry-trace_id=d49d9bf66f13450b81f65bc51cf49c03,sentry-public_key=public,sentry-sample_rate=1")
 }
+
+func TestDynamicSamplingContextFromScope(t *testing.T) {
+	tests := map[string]struct {
+		scope    *Scope
+		client   *Client
+		expected DynamicSamplingContext
+	}{
+		"Valid input": {
+			scope: &Scope{
+				propagationContext: PropagationContext{
+					TraceID: TraceIDFromHex("d49d9bf66f13450b81f65bc51cf49c03"),
+					SpanID:  SpanIDFromHex("a9f442f9330b4e09"),
+				},
+			},
+			client: func() *Client {
+				dsn, _ := NewDsn("http://[email protected]/sentry/1")
+				return &Client{
+					options: ClientOptions{
+						Dsn:         dsn.String(),
+						Release:     "1.0.0",
+						Environment: "production",
+					},
+					dsn: dsn,
+				}
+			}(),
+			expected: DynamicSamplingContext{
+				Entries: map[string]string{
+					"trace_id":    "d49d9bf66f13450b81f65bc51cf49c03",
+					"public_key":  "public",
+					"release":     "1.0.0",
+					"environment": "production",
+				},
+				Frozen: true,
+			},
+		},
+		"Nil client": {
+			scope: &Scope{
+				propagationContext: PropagationContext{
+					TraceID: TraceIDFromHex("d49d9bf66f13450b81f65bc51cf49c03"),
+					SpanID:  SpanIDFromHex("a9f442f9330b4e09"),
+				},
+			},
+			client: nil,
+			expected: DynamicSamplingContext{
+				Entries: map[string]string{},
+				Frozen:  false,
+			},
+		},
+		"Nil scope": {
+			scope:  nil,
+			client: &Client{},
+			expected: DynamicSamplingContext{
+				Entries: map[string]string{},
+				Frozen:  false,
+			},
+		},
+	}
+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			result := DynamicSamplingContextFromScope(tt.scope, tt.client)
+			assertEqual(t, tt.expected, result)
+		})
+	}
+}

Diff for: http/sentryhttp.go

@@ -121,6 +121,7 @@ func (h *Handler) handle(handler http.Handler) http.HandlerFunc {
 		// level?, ...).
 		r = r.WithContext(transaction.Context())
 		hub.Scope().SetRequest(r)
+
 		defer h.recoverWithSentry(hub, r)
 		handler.ServeHTTP(rw, r)
 	}

Diff for: hub.go

@@ -365,6 +365,27 @@ func (hub *Hub) Flush(timeout time.Duration) bool {
 	return client.Flush(timeout)
 }
 
+// Continue a trace based on HTTP header values. If performance is enabled this
+// returns a SpanOption that can be used to start a transaction, otherwise nil.
+func (hub *Hub) ContinueTrace(trace, baggage string) (SpanOption, error) {
+	scope := hub.Scope()
+	propagationContext, err := PropagationContextFromHeaders(trace, baggage)
+	if err != nil {
+		return nil, err
+	}
+
+	scope.SetPropagationContext(propagationContext)
+
+	client := hub.Client()
+	if client != nil && client.options.EnableTracing {
+		return ContinueFromHeaders(trace, baggage), nil
+	}
+
+	scope.SetContext("trace", propagationContext.Map())
+
+	return nil, nil
+}
+
 // HasHubOnContext checks whether Hub instance is bound to a given Context struct.
 func HasHubOnContext(ctx context.Context) bool {
 	_, ok := ctx.Value(HubContextKey).(*Hub)

Diff for: hub_test.go

@@ -2,12 +2,14 @@ package sentry
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"sync"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/stretchr/testify/assert"
 )
 
 const testDsn = "http://[email protected]/1337"
@@ -324,6 +326,88 @@ func TestHasHubOnContextReturnsFalseIfHubIsNotThere(t *testing.T) {
 	assertEqual(t, false, HasHubOnContext(ctx))
 }
 
+func TestHub_ContinueTrace(t *testing.T) {
+	newScope := func() *Scope {
+		return &Scope{contexts: make(map[string]Context)}
+	}
+
+	mockClient := &Client{options: ClientOptions{EnableTracing: true}}
+
+	tests := map[string]struct {
+		hub          *Hub
+		trace        string
+		baggage      string
+		expectedErr  error
+		expectedSpan bool                             // Whether a SpanOption is expected to be returned
+		checkScope   func(t *testing.T, scope *Scope) // Additional checks on the scope
+	}{
+		"Valid trace and baggage": {
+			hub:          NewHub(mockClient, newScope()),
+			trace:        "4fbfb1b884c8532962a3c0b7b834428e-a9f442f9330b4e09",
+			baggage:      "sentry-release=1.0.0,sentry-environment=production",
+			expectedErr:  nil,
+			expectedSpan: true,
+			checkScope: func(t *testing.T, scope *Scope) {
+				assert.Equal(t, "4fbfb1b884c8532962a3c0b7b834428e", scope.propagationContext.TraceID.String())
+			},
+		},
+		"Invalid trace": {
+			hub:          NewHub(mockClient, newScope()),
+			trace:        "invalid",
+			baggage:      "sentry-release=1.0.0,sentry-environment=production",
+			expectedErr:  nil,
+			expectedSpan: true,
+			checkScope: func(t *testing.T, scope *Scope) {
+				assert.NotEmpty(t, scope.propagationContext.TraceID.String())
+			},
+		},
+		"Invalid baggage": {
+			hub:          NewHub(mockClient, newScope()),
+			trace:        "4fbfb1b884c8532962a3c0b7b834428e-a9f442f9330b4e09",
+			baggage:      "invalid_baggage",
+			expectedErr:  errors.New("invalid baggage list-member: \"invalid_baggage\""),
+			expectedSpan: false,
+			checkScope: func(t *testing.T, scope *Scope) {
+				assert.Equal(t, "00000000000000000000000000000000", scope.propagationContext.TraceID.String())
+			},
+		},
+		"Tracing not enabled": {
+			hub:          NewHub(&Client{options: ClientOptions{EnableTracing: false}}, newScope()),
+			trace:        "4fbfb1b884c8532962a3c0b7b834428e-a9f442f9330b4e09",
+			baggage:      "sentry-release=1.0.0,sentry-environment=production",
+			expectedErr:  nil,
+			expectedSpan: false,
+			checkScope: func(t *testing.T, scope *Scope) {
+				assert.Equal(t, "4fbfb1b884c8532962a3c0b7b834428e", scope.propagationContext.TraceID.String())
+				assert.Contains(t, scope.contexts, "trace")
+			},
+		},
+	}
+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			opt, err := tt.hub.ContinueTrace(tt.trace, tt.baggage)
+
+			if tt.expectedErr != nil {
+				assert.Error(t, err, "expected error, got nil")
+				assert.Equal(t, tt.expectedErr.Error(), err.Error())
+			} else {
+				assert.NoError(t, err, "expected no error, got one")
+			}
+
+			// Check for expected SpanOption
+			if tt.expectedSpan {
+				assert.NotNil(t, opt, "expected SpanOption, got nil")
+			} else {
+				assert.Nil(t, opt, "expected no SpanOption, got one")
+			}
+
+			// Additional checks on the scope
+			tt.checkScope(t, tt.hub.Scope())
+		})
+	}
+}
+
 func TestGetHubFromContext(t *testing.T) {
 	hub, _, _ := setupHubTest()
 	ctx := context.Background()

Diff for: mocks_test.go

@@ -14,7 +14,7 @@ func (scope *ScopeMock) AddBreadcrumb(breadcrumb *Breadcrumb, _ int) {
 	scope.breadcrumb = breadcrumb
 }
 
-func (scope *ScopeMock) ApplyToEvent(event *Event, _ *EventHint) *Event {
+func (scope *ScopeMock) ApplyToEvent(event *Event, _ *EventHint, _ *Client) *Event {
 	if scope.shouldDropEvent {
 		return nil
 	}