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

Tracing without performance #833

Merged
merged 22 commits into from
Jul 15, 2024
Merged
Show file tree
Hide file tree
Changes from 17 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
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 @@ -685,7 +685,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 @@ -136,6 +136,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
28 changes: 28 additions & 0 deletions hub.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,34 @@
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.
// Uses the Global Hub.
func ContinueTrace(trace, baggage string) (SpanOption, error) {
ribice marked this conversation as resolved.
Show resolved Hide resolved
return currentHub.ContinueTrace(trace, baggage)

Check warning on line 372 in hub.go

View check run for this annotation

Codecov / codecov/patch

hub.go#L371-L372

Added lines #L371 - L372 were not covered by tests
}

// 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) {
ribice marked this conversation as resolved.
Show resolved Hide resolved
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
Loading