fix: SpanLimit shouldn't add SendDelay (#1272)

## Which problem is this PR solving?

- Fixes #1270 
- Adds a test for preserving sample rate for late spans

collect/collect.go

@@ -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
@@ -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)
 	}
 }

collect/collect_test.go

@@ -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"},
@@ -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{
@@ -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()
 }

config/metadata/configMeta.yaml

@@ -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