Skip to content

Commit

Permalink
feat(spans): only start 'http.client' op as a child span
Browse files Browse the repository at this point in the history
  • Loading branch information
aldy505 committed Sep 4, 2024
1 parent 4e55daa commit 15aa59a
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 47 deletions.
9 changes: 7 additions & 2 deletions httpclient/sentryhttpclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,15 @@ type SentryRoundTripper struct {
}

func (s *SentryRoundTripper) RoundTrip(request *http.Request) (*http.Response, error) {
ctx := request.Context()
// Only create the `http.client` span only if there is a parent span.
parentSpan := sentry.GetSpanFromContext(request.Context())
if parentSpan == nil {
return s.originalRoundTripper.RoundTrip(request)
}

cleanRequestURL := request.URL.Redacted()

span := sentry.StartSpan(ctx, "http.client", sentry.WithTransactionName(fmt.Sprintf("%s %s", request.Method, cleanRequestURL)))
span := parentSpan.StartChild("http.client", sentry.WithTransactionName(fmt.Sprintf("%s %s", request.Method, cleanRequestURL)))
span.Tags = s.tags
defer span.Finish()

Expand Down
109 changes: 64 additions & 45 deletions httpclient/sentryhttpclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"io"
"net/http"
"strconv"
"strings"
"testing"

"github.com/getsentry/sentry-go"
Expand Down Expand Up @@ -52,15 +53,15 @@ func TestIntegration(t *testing.T) {
TracerOptions []sentryhttpclient.SentryRoundTripTracerOption
WantStatus int
WantResponseLength int
WantTransaction *sentry.Event
WantSpan *sentry.Span
}{
{
RequestMethod: "GET",
RequestURL: "https://example.com/foo",
WantStatus: 200,
WantResponseLength: 0,
WantTransaction: &sentry.Event{
Extra: map[string]interface{}{
WantSpan: &sentry.Span{
Data: map[string]interface{}{
"http.fragment": string(""),
"http.query": string(""),
"http.request.method": string("GET"),
Expand All @@ -69,11 +70,12 @@ func TestIntegration(t *testing.T) {
"server.address": string("example.com"),
"server.port": string(""),
},
Level: sentry.LevelInfo,
Transaction: "GET https://example.com/foo",
Type: "transaction",
TransactionInfo: &sentry.TransactionInfo{Source: "custom"},
Tags: map[string]string{},
Name: "GET https://example.com/foo",
Op: "http.client",
Tags: map[string]string{},
Origin: "manual",
Sampled: sentry.SampledTrue,
Status: sentry.SpanStatusOK,
},
},
{
Expand All @@ -82,8 +84,8 @@ func TestIntegration(t *testing.T) {
TracerOptions: []sentryhttpclient.SentryRoundTripTracerOption{nil, nil, nil},
WantStatus: 200,
WantResponseLength: 0,
WantTransaction: &sentry.Event{
Extra: map[string]interface{}{
WantSpan: &sentry.Span{
Data: map[string]interface{}{
"http.fragment": string("readme"),
"http.query": string("baz=123"),
"http.request.method": string("GET"),
Expand All @@ -92,11 +94,12 @@ func TestIntegration(t *testing.T) {
"server.address": string("example.com"),
"server.port": string("443"),
},
Level: sentry.LevelInfo,
Transaction: "GET https://example.com:443/foo/bar?baz=123#readme",
Type: "transaction",
TransactionInfo: &sentry.TransactionInfo{Source: "custom"},
Tags: map[string]string{},
Name: "GET https://example.com:443/foo/bar?baz=123#readme",
Op: "http.client",
Tags: map[string]string{},
Origin: "manual",
Sampled: sentry.SampledTrue,
Status: sentry.SpanStatusOK,
},
},
{
Expand All @@ -105,8 +108,8 @@ func TestIntegration(t *testing.T) {
TracerOptions: []sentryhttpclient.SentryRoundTripTracerOption{sentryhttpclient.WithTag("user", "def"), sentryhttpclient.WithTags(map[string]string{"domain": "example.com"})},
WantStatus: 400,
WantResponseLength: 0,
WantTransaction: &sentry.Event{
Extra: map[string]interface{}{
WantSpan: &sentry.Span{
Data: map[string]interface{}{
"http.fragment": string(""),
"http.query": string("abc=def&bar=123"),
"http.request.method": string("HEAD"),
Expand All @@ -119,19 +122,20 @@ func TestIntegration(t *testing.T) {
"user": "def",
"domain": "example.com",
},
Level: sentry.LevelInfo,
Transaction: "HEAD https://example.com:8443/foo?bar=123&abc=def",
Type: "transaction",
TransactionInfo: &sentry.TransactionInfo{Source: "custom"},
Name: "HEAD https://example.com:8443/foo?bar=123&abc=def",
Op: "http.client",
Origin: "manual",
Sampled: sentry.SampledTrue,
Status: sentry.SpanStatusInvalidArgument,
},
},
{
RequestMethod: "POST",
RequestURL: "https://john:verysecurepassword@example.com:4321/secret",
WantStatus: 200,
WantResponseLength: 1024,
WantTransaction: &sentry.Event{
Extra: map[string]interface{}{
WantSpan: &sentry.Span{
Data: map[string]interface{}{
"http.fragment": string(""),
"http.query": string(""),
"http.request.method": string("POST"),
Expand All @@ -140,33 +144,35 @@ func TestIntegration(t *testing.T) {
"server.address": string("example.com"),
"server.port": string("4321"),
},
Level: sentry.LevelInfo,
Transaction: "POST https://john:xxxxx@example.com:4321/secret",
Type: "transaction",
TransactionInfo: &sentry.TransactionInfo{Source: "custom"},
Tags: map[string]string{},
Name: "POST https://john:xxxxx@example.com:4321/secret",
Op: "http.client",
Tags: map[string]string{},
Origin: "manual",
Sampled: sentry.SampledTrue,
Status: sentry.SpanStatusOK,
},
},
}

transactionsCh := make(chan *sentry.Event, len(tests))
spansCh := make(chan []*sentry.Span, len(tests))

sentryClient, err := sentry.NewClient(sentry.ClientOptions{
EnableTracing: true,
TracesSampleRate: 1.0,
BeforeSendTransaction: func(event *sentry.Event, hint *sentry.EventHint) *sentry.Event {
transactionsCh <- event
spansCh <- event.Spans
return event
},
})
if err != nil {
t.Fatal(err)
}

var want []*sentry.Event
for _, tt := range tests {
hub := sentry.NewHub(sentryClient, sentry.NewScope())
ctx := sentry.SetHubOnContext(context.Background(), hub)
span := sentry.StartSpan(ctx, "fake_parent", sentry.WithTransactionName("Fake Parent"))
ctx = span.Context()

request, err := http.NewRequestWithContext(ctx, tt.RequestMethod, tt.RequestURL, nil)
if err != nil {
Expand All @@ -188,32 +194,45 @@ func TestIntegration(t *testing.T) {
}

response.Body.Close()
want = append(want, tt.WantTransaction)
span.Finish()
}

if ok := sentryClient.Flush(testutils.FlushTimeout()); !ok {
t.Fatal("sentry.Flush timed out")
}
close(transactionsCh)
var got []*sentry.Event
for e := range transactionsCh {
close(spansCh)

var got [][]*sentry.Span
for e := range spansCh {
got = append(got, e)
}

optstrans := cmp.Options{
cmpopts.IgnoreFields(
sentry.Event{},
"Contexts", "EventID", "Platform", "Modules",
"Release", "Sdk", "ServerName", "Timestamp",
"sdkMetaData", "StartTime", "Spans",
),
cmpopts.IgnoreFields(
sentry.Request{},
"Env",
sentry.Span{},
"TraceID", "SpanID", "ParentSpanID", "StartTime", "EndTime",
"mu", "parent", "sampleRate", "ctx", "dynamicSamplingContext", "recorder", "finishOnce", "collectProfile", "contexts",
),
}
if diff := cmp.Diff(want, got, optstrans); diff != "" {
t.Fatalf("Transaction mismatch (-want +got):\n%s", diff)
for i, tt := range tests {
var foundMatch = false
gotSpans := got[i]

var diffs []string
for _, gotSpan := range gotSpans {
if diff := cmp.Diff(tt.WantSpan, gotSpan, optstrans); diff != "" {
diffs = append(diffs, diff)
} else {
foundMatch = true
break
}
}

if foundMatch {
continue
} else {
t.Errorf("Span mismatch (-want +got):\n%s", strings.Join(diffs, "\n"))
}
}
}

Expand Down
11 changes: 11 additions & 0 deletions tracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,17 @@ func StartSpan(ctx context.Context, operation string, options ...SpanOption) *Sp
return &span
}

// GetSpanFromContext retrieves attached *sentry.Span instance from context.Context.
// If there are no spans, it will return nil.
func GetSpanFromContext(ctx context.Context) *Span {
span, ok := ctx.Value(spanContextKey{}).(*Span)
if ok {
return span
}

return nil
}

// Finish sets the span's end time, unless already set. If the span is the root
// of a span tree, Finish sends the span tree to Sentry as a transaction.
//
Expand Down
22 changes: 22 additions & 0 deletions tracing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1091,3 +1091,25 @@ func TestSpanFinishConcurrentlyWithoutRaces(_ *testing.T) {

time.Sleep(50 * time.Millisecond)
}

func TestGetSpanFromContext(t *testing.T) {
t.Run("Exists", func(t *testing.T) {
span := StartSpan(context.Background(), "something")

value := GetSpanFromContext(span.Context())
if value == nil {
t.Error("expecting `value` to be not nil")
} else {
if span.Op != "something" {
t.Errorf("expecting `span.Op` to be 'something', instead got %q", span.Op)
}
}
})

t.Run("Nil", func(t *testing.T) {
value := GetSpanFromContext(context.Background())
if value != nil {
t.Error("expecting `value` to be nil")
}
})
}

0 comments on commit 15aa59a

Please sign in to comment.