From 67126912632aa6b8a3d84b24aee385d9310758f2 Mon Sep 17 00:00:00 2001 From: Florian Bauer Date: Tue, 19 Jul 2022 13:41:02 +0200 Subject: [PATCH 1/8] make maximum ammount of spans configurable with client options --- client.go | 11 +++++++++++ span_recorder.go | 6 +----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/client.go b/client.go index 1cc33690f..90acffe46 100644 --- a/client.go +++ b/client.go @@ -29,6 +29,11 @@ import ( // stack trace is often the most useful information. const maxErrorDepth = 10 +// maxSpans limits the number of recorded spans per transaction. The limit is +// meant to bound memory usage and prevent too large transaction events that +// would be rejected by Sentry. +const defaultMaxSpans = 1000 + // hostname is the host name reported by the kernel. It is precomputed once to // avoid syscalls when capturing events. // @@ -175,6 +180,8 @@ type ClientOptions struct { Environment string // Maximum number of breadcrumbs. MaxBreadcrumbs int + // Maximum number of spans. + MaxSpans int // An optional pointer to http.Client that will be used with a default // HTTPTransport. Using your own client will make HTTPTransport, HTTPProxy, // HTTPSProxy and CaCerts options ignored. @@ -250,6 +257,10 @@ func NewClient(options ClientOptions) (*Client, error) { options.MaxErrorDepth = maxErrorDepth } + if options.MaxSpans == 0 { + options.MaxSpans = defaultMaxSpans + } + // SENTRYGODEBUG is a comma-separated list of key=value pairs (similar // to GODEBUG). It is not a supported feature: recognized debug options // may change any time. diff --git a/span_recorder.go b/span_recorder.go index 9e611ca62..86812213f 100644 --- a/span_recorder.go +++ b/span_recorder.go @@ -4,11 +4,6 @@ import ( "sync" ) -// maxSpans limits the number of recorded spans per transaction. The limit is -// meant to bound memory usage and prevent too large transaction events that -// would be rejected by Sentry. -const maxSpans = 1000 - // A spanRecorder stores a span tree that makes up a transaction. Safe for // concurrent use. It is okay to add child spans from multiple goroutines. type spanRecorder struct { @@ -20,6 +15,7 @@ type spanRecorder struct { // record stores a span. The first stored span is assumed to be the root of a // span tree. func (r *spanRecorder) record(s *Span) { + maxSpans := CurrentHub().Client().options.MaxSpans r.mu.Lock() defer r.mu.Unlock() if len(r.spans) >= maxSpans { From 6859dd02d14b72c4341f95f624933811a73af4d3 Mon Sep 17 00:00:00 2001 From: Florian Bauer Date: Tue, 19 Jul 2022 13:44:20 +0200 Subject: [PATCH 2/8] adjust help comment --- client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client.go b/client.go index 90acffe46..ee0b68387 100644 --- a/client.go +++ b/client.go @@ -29,7 +29,7 @@ import ( // stack trace is often the most useful information. const maxErrorDepth = 10 -// maxSpans limits the number of recorded spans per transaction. The limit is +// defaultMaxSpans limits the default number of recorded spans per transaction. The limit is // meant to bound memory usage and prevent too large transaction events that // would be rejected by Sentry. const defaultMaxSpans = 1000 From 25785a36d144b22fc074333652273f9a7a9f295c Mon Sep 17 00:00:00 2001 From: Florian Bauer Date: Tue, 19 Jul 2022 14:45:04 +0200 Subject: [PATCH 3/8] fix unittest issue --- span_recorder.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/span_recorder.go b/span_recorder.go index 86812213f..46cca7fd2 100644 --- a/span_recorder.go +++ b/span_recorder.go @@ -15,7 +15,10 @@ type spanRecorder struct { // record stores a span. The first stored span is assumed to be the root of a // span tree. func (r *spanRecorder) record(s *Span) { - maxSpans := CurrentHub().Client().options.MaxSpans + maxSpans := defaultMaxSpans + if CurrentHub().Client() != nil { + maxSpans = CurrentHub().Client().options.MaxSpans + } r.mu.Lock() defer r.mu.Unlock() if len(r.spans) >= maxSpans { From 22d4bfbe2e253f9ff34b15925b73cd37b034ed02 Mon Sep 17 00:00:00 2001 From: Florian Bauer Date: Thu, 27 Oct 2022 17:08:59 +0200 Subject: [PATCH 4/8] add unit tests for MaxSpans related code --- client_test.go | 14 ++++++++++ span_recorder_test.go | 63 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 span_recorder_test.go diff --git a/client_test.go b/client_test.go index 2388ecac1..88a988245 100644 --- a/client_test.go +++ b/client_test.go @@ -520,3 +520,17 @@ func TestRecover(t *testing.T) { }) } } + +func TestCustomMaxSpansProperty(t *testing.T) { + client, _, _ := setupClientTest() + assertEqual(t, client.Options().MaxSpans, defaultMaxSpans) + + client.options.MaxSpans = 2000 + assertEqual(t, client.Options().MaxSpans, 2000) + + properClient, _ := NewClient(ClientOptions{ + MaxSpans: 3000, + }) + + assertEqual(t, properClient.Options().MaxSpans, 3000) +} diff --git a/span_recorder_test.go b/span_recorder_test.go new file mode 100644 index 000000000..2005fdfd9 --- /dev/null +++ b/span_recorder_test.go @@ -0,0 +1,63 @@ +package sentry + +import ( + "bytes" + "context" + "fmt" + "io" + "testing" +) + +func Test_spanRecorder_record(t *testing.T) { + testRootSpan := StartSpan(context.Background(), "test", TransactionName("test transaction")) + + for _, tt := range []struct { + name string + maxSpans int + toRecordSpans int + expectOverflow bool + }{ + { + name: "record span without problems", + maxSpans: defaultMaxSpans, + toRecordSpans: 1, + expectOverflow: false, + }, + { + name: "record span with overflow", + maxSpans: 2, + toRecordSpans: 4, + expectOverflow: true, + }, + } { + t.Run(tt.name, func(t *testing.T) { + logBuffer := bytes.Buffer{} + Logger.SetOutput(&logBuffer) + defer Logger.SetOutput(io.Discard) + spanRecorder := spanRecorder{} + + currentHub.BindClient(&Client{ + options: ClientOptions{ + MaxSpans: tt.maxSpans, + }, + }) + // unbind client after test for not affecting other tests + defer currentHub.stackTop().SetClient(nil) + + for i := 0; i < tt.toRecordSpans; i++ { + child := testRootSpan.StartChild(fmt.Sprintf("test %d", i)) + spanRecorder.record(child) + } + + if tt.expectOverflow { + assertNotEqual(t, len(spanRecorder.spans), tt.toRecordSpans, "expected overflow") + } else { + assertEqual(t, len(spanRecorder.spans), tt.toRecordSpans, "expected no overflow") + } + // check if Logger was called for overflow messages + if bytes.Contains(logBuffer.Bytes(), []byte("Too many spans")) && !tt.expectOverflow { + t.Error("unexpected overflow log") + } + }) + } +} From 7e658612c90cf02582bb6aeea4e748ab884cd802 Mon Sep 17 00:00:00 2001 From: Florian Bauer <35347506+fsrv-xyz@users.noreply.github.com> Date: Wed, 2 Nov 2022 07:15:40 +0100 Subject: [PATCH 5/8] apply suggested changes to client.go Co-authored-by: Michi Hoffmann --- client.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/client.go b/client.go index ee0b68387..108b7cc77 100644 --- a/client.go +++ b/client.go @@ -181,6 +181,9 @@ type ClientOptions struct { // Maximum number of breadcrumbs. MaxBreadcrumbs int // Maximum number of spans. + // + // See https://develop.sentry.dev/sdk/envelopes/#size-limits for size limits + // applied during event ingestion. Events that exceed these limits might get dropped. MaxSpans int // An optional pointer to http.Client that will be used with a default // HTTPTransport. Using your own client will make HTTPTransport, HTTPProxy, From 30413f91092b42a2cb7382f96c3d088c1568ed8d Mon Sep 17 00:00:00 2001 From: Florian Bauer <35347506+fsrv-xyz@users.noreply.github.com> Date: Wed, 2 Nov 2022 07:17:09 +0100 Subject: [PATCH 6/8] apply suggested changes to span_recorder_test.go Co-authored-by: Michi Hoffmann --- span_recorder_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/span_recorder_test.go b/span_recorder_test.go index 2005fdfd9..70c464378 100644 --- a/span_recorder_test.go +++ b/span_recorder_test.go @@ -41,7 +41,7 @@ func Test_spanRecorder_record(t *testing.T) { MaxSpans: tt.maxSpans, }, }) - // unbind client after test for not affecting other tests + // Unbind the client afterwards, to not affect other tests defer currentHub.stackTop().SetClient(nil) for i := 0; i < tt.toRecordSpans; i++ { From b83ab8f0623098151e43eb90ca720e5ba14615c7 Mon Sep 17 00:00:00 2001 From: Florian Bauer <35347506+fsrv-xyz@users.noreply.github.com> Date: Wed, 2 Nov 2022 07:19:56 +0100 Subject: [PATCH 7/8] apply suggested changes to span_recorder.go Co-authored-by: Michi Hoffmann --- span_recorder.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/span_recorder.go b/span_recorder.go index 46cca7fd2..137aa233e 100644 --- a/span_recorder.go +++ b/span_recorder.go @@ -16,8 +16,8 @@ type spanRecorder struct { // span tree. func (r *spanRecorder) record(s *Span) { maxSpans := defaultMaxSpans - if CurrentHub().Client() != nil { - maxSpans = CurrentHub().Client().options.MaxSpans + if client := CurrentHub().Client(); client != nil { + maxSpans = client.Options().MaxSpans } r.mu.Lock() defer r.mu.Unlock() From 9b8175081d241430e4846eeb2cf2954c03891d39 Mon Sep 17 00:00:00 2001 From: Florian Bauer Date: Wed, 2 Nov 2022 07:31:49 +0100 Subject: [PATCH 8/8] doc: add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b62de10c..a9843f975 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ - fix: Scope values should not override Event values (#446) - feat: Extend User inteface by adding Data, Name and Segment (#483) +- feat: Make maximum amount of spans configurable (#460) ## 0.14.0