Skip to content

Commit

Permalink
Tracing without performance (#833)
Browse files Browse the repository at this point in the history
  • Loading branch information
ribice authored Jul 15, 2024
1 parent 5377e6a commit 00f7f6c
Show file tree
Hide file tree
Showing 14 changed files with 675 additions and 55 deletions.
4 changes: 2 additions & 2 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
50 changes: 41 additions & 9 deletions dynamic_sampling_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
}
}
80 changes: 72 additions & 8 deletions dynamic_sampling_context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
},
Expand Down Expand Up @@ -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)
})
}
}
1 change: 1 addition & 0 deletions http/sentryhttp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
21 changes: 21 additions & 0 deletions hub.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
84 changes: 84 additions & 0 deletions hub_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion mocks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading

0 comments on commit 00f7f6c

Please sign in to comment.