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.

Sign up for GitHub

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

fix: SpanLimit shouldn't add SendDelay #1272

Merged
merged 4 commits into from
Aug 14, 2024
Merged

fix: SpanLimit shouldn't add SendDelay #1272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1fc97ea
Make sure we test that we decorate late spans with sample rate
kentquirk Aug 14, 2024
0f2a9c9
Don't use SendDelay if we hit SpanLimit
kentquirk Aug 14, 2024
7b0f768
Update docs to explain SendDelay more accurately
kentquirk Aug 14, 2024
eacd8dd
Merge branch 'main' into kent.send_delay_fixes
kentquirk Aug 14, 2024
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
19 changes: 10 additions & 9 deletions collect/collect.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,10 +528,11 @@ func (i *InMemCollector) processSpan(sp *types.Span) {
// great! trace is live. add the span.
trace.AddSpan(sp)

// we may override these values in conditions below
markTraceForSending := false
// if the span count has exceeded our SpanLimit, mark the trace for sending
if tcfg.SpanLimit > 0 && uint(trace.SpanCount()) > tcfg.SpanLimit {
markTraceForSending = true
timeout := tcfg.GetSendDelay()
if timeout == 0 {
timeout = 2 * time.Second // a sensible default
}

// if this is a root span, say so and send the trace
Expand All @@ -540,13 +541,13 @@ func (i *InMemCollector) processSpan(sp *types.Span) {
trace.RootSpan = sp
}

if markTraceForSending {
timeout := tcfg.GetSendDelay()

if timeout == 0 {
timeout = 2 * time.Second
}
// if the span count has exceeded our SpanLimit, send the trace immediately
if tcfg.SpanLimit > 0 && uint(trace.SpanCount()) > tcfg.SpanLimit {
markTraceForSending = true
timeout = 0 // don't use a timeout in this case; this is an "act fast" situation
}

if markTraceForSending {
trace.SendBy = time.Now().Add(timeout)
}
}
Expand Down
8 changes: 5 additions & 3 deletions collect/collect_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1767,7 +1767,7 @@ func TestBigTracesGoEarly(t *testing.T) {
SpanLimit: uint(spanlimit),
MaxBatchSize: 1500,
},
GetSamplerTypeVal: &config.DeterministicSamplerConfig{SampleRate: 1},
GetSamplerTypeVal: &config.DeterministicSamplerConfig{SampleRate: 2},
AddSpanCountToRoot: true,
AddCountsToRoot: true,
ParentIdFieldNames: []string{"trace.parent_id", "parentId"},
Expand All @@ -1790,7 +1790,8 @@ func TestBigTracesGoEarly(t *testing.T) {
go coll.collect()
defer coll.Stop()

var traceID = "mytrace"
// this name was chosen to be Kept with the deterministic/2 sampler
var traceID = "myTrace"

for i := 0; i < spanlimit; i++ {
span := &types.Span{
Expand Down Expand Up @@ -1836,7 +1837,8 @@ func TestBigTracesGoEarly(t *testing.T) {
assert.Equal(t, nil, transmission.Events[0].Data["meta.span_count"], "child span metadata should NOT be populated with span count")
assert.EqualValues(t, spanlimit+1, transmission.Events[spanlimit].Data["meta.span_count"], "root span metadata should be populated with span count")
assert.EqualValues(t, spanlimit+1, transmission.Events[spanlimit].Data["meta.event_count"], "root span metadata should be populated with event count")
assert.Equal(t, "deterministic/always - late arriving span", transmission.Events[spanlimit].Data["meta.refinery.reason"], "the late root span should have meta.refinery.reason set to rules + late arriving span.")
assert.Equal(t, "deterministic/chance - late arriving span", transmission.Events[spanlimit].Data["meta.refinery.reason"], "the late root span should have meta.refinery.reason set to rules + late arriving span.")
assert.EqualValues(t, 2, transmission.Events[spanlimit].SampleRate, "the late root span should sample rate set")
assert.Equal(t, "trace_send_late_span", transmission.Events[spanlimit].Data["meta.refinery.send_reason"], "send reason should indicate span count exceeded")
transmission.Mux.RUnlock()
}
14 changes: 8 additions & 6 deletions config/metadata/configMeta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,12 +306,14 @@ groups:
- type: minimum
arg: 100ms
reload: true
summary: is the duration to wait before sending a trace.
description: >
This setting is a short timer that is triggered when a trace is
complete. Refinery waits for this duration before sending the trace.
The reason for this setting is to allow for asynchronous spans and
small network delays to elapse before sending the trace.
summary: is the duration to wait after the root span arrives before sending a trace.
description: >
This setting is a short timer that is triggered when a trace is marked
complete by the arrival of the root span. Refinery waits for this
duration before sending the trace. This setting exists to allow for
asynchronous spans and small network delays to elapse before sending
the trace. `SendDelay` is not applied if the TraceTimeout expires or
the `SpanLimit` is reached.

- name: BatchTimeout
type: duration
Expand Down
Loading