Deprecate MaxSpansPerTrace in favour of MaxBytesPerTrace (#612)

* Deprecate MaxSpansPerTrace in favour of MaxBytesPerTrace

Signed-off-by: Annanay <annanayagarwal@gmail.com>

* Lint, CHANGELOG

Signed-off-by: Annanay <annanayagarwal@gmail.com>

* Fix e2e limits test

Signed-off-by: Annanay <annanayagarwal@gmail.com>

CHANGELOG.md

@@ -3,6 +3,8 @@
 * [CHANGE] Update to Go 1.16, latest OpenTelemetry proto definition and collector [#546](https://github.com/grafana/tempo/pull/546)
 * [CHANGE] Tempo Query Frontend now accepts queries at `/tempo/api/traces/{traceID}` as opposed to `/api/traces/{traceID}`.
   This is a **breaking change**, make sure to change the Grafana Datasource endpoint accordingly. [#574](https://github.com/grafana/tempo/pull/574)
+* [CHANGE] `max_spans_per_trace` limit override has been removed in favour of `max_bytes_per_trace`.
+  This is a **breaking change** to the overrides config section. [#612](https://github.com/grafana/tempo/pull/612)
 * [FEATURE] Add page based access to the index file. [#557](https://github.com/grafana/tempo/pull/557)
 * [ENHANCEMENT] Add a Shutdown handler to flush data to backend, at "/shutdown". [#526](https://github.com/grafana/tempo/pull/526)
 * [ENHANCEMENT] Queriers now query all (healthy) ingesters for a trace to mitigate 404s on ingester rollouts/scaleups.

docs/tempo/website/configuration/ingestion-limit.md

@@ -18,7 +18,7 @@ The following options can be used to limit ingestion:
 
    - `ingestion_burst_size` : Burst size used in span ingestion. Default is `100,000`.
    - `ingestion_rate_limit` : Per-user ingestion rate limit in spans per second. Default is `100,000`.
-   - `max_spans_per_trace` : Maximum number of spans per trace.  `0` to disable. Default is `50,000`.
+   - `max_bytes_per_trace` : Maximum size of a single trace in bytes.  `0` to disable. Default is `5,000,000` (~5MB).
    - `max_traces_per_user`: Maximum number of active traces per user, per ingester. `0` to disable. Default is `10,000`.
 
 Both the `ingestion_burst_size` and `ingestion_rate_limit` parameters control the rate limit. When these limits exceed the following message is logged:
@@ -27,10 +27,10 @@ Both the `ingestion_burst_size` and `ingestion_rate_limit` parameters control th
     RATE_LIMITED: ingestion rate limit (100000 spans) exceeded while adding 10 spans
 ```    
 
-When the limit for the `max_spans_per_trace` parameter exceeds the following message is logged:
+When the limit for the `max_bytes_per_trace` parameter exceeds the following message is logged:
 
 ```
-    TRACE_TOO_LARGE: totalSpans (50000) exceeded while adding 2 spans
+    TRACE_TOO_LARGE: max size of trace (5000000) exceeded while adding 387 bytes
 ```
 
 Finally, when the limit for the `max_traces_per_user` parameter exceeds the following message is logged:
@@ -50,7 +50,7 @@ To configure new ingestion limits that applies to all tenants of the cluster:
     overrides:
         ingestion_burst_size: 50000
         ingestion_rate_limit: 50000
-        max_spans_per_trace: 50000
+        max_bytes_per_trace: 5_000_000
         max_traces_per_user: 10000 
 ``` 
 
@@ -75,7 +75,7 @@ Sometimes you don't want all tenants within the cluster to have the same setting
         "<tenant id>":
             ingestion_max_batch_size: 5000
             ingestion_rate_limit: 400000
-            max_spans_per_trace: 250000
+            max_bytes_per_trace: 25_000_000
             max_traces_per_user: 100000
     ```
 
@@ -87,7 +87,7 @@ This overrides file is dynamically loaded.  It can be changed at runtime and wil
         "*":
             ingestion_max_batch_size: 5000
             ingestion_rate_limit: 400000
-            max_spans_per_trace: 250000
+            max_bytes_per_trace: 25_000_000
             max_traces_per_user: 100000
 ```
 

docs/tempo/website/troubleshooting/max-trace-limit-reached.md

@@ -13,6 +13,6 @@ In high volume tracing environments the default trace limits are sometimes not s
 
    - `ingestion_burst_size` : Burst size used in span ingestion. Default is `100,000`.
    - `ingestion_rate_limit` : Per-user ingestion rate limit in spans per second. Default is `100,000`.
-   - `max_spans_per_trace` : Maximum number of spans per trace.  `0` to disable. Default is `50,000`.
+   - `max_bytes_per_trace` : Maximum size of a single trace in bytes.  `0` to disable. Default is `5,000,000` (~5MB).
    - `max_traces_per_user`: Maximum number of active traces per user, per ingester. `0` to disable. Default is `10,000`.
 - Increase the maximum limit to a failsafe value. For example, increase the limit for the `max_traces_per_user` parameter from `10,000` like `50000`.

integration/e2e/config-limits.yaml

@@ -12,7 +12,7 @@ distributor:
         grpc:
 
 overrides:
-  max_spans_per_trace: 1
+  max_bytes_per_trace: 100
   max_traces_per_user: 1
   ingestion_rate_limit: 5
   ingestion_burst_size: 5

integration/e2e/limits_test.go

@@ -29,7 +29,7 @@ func TestLimits(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, c)
 
-	// should fail b/c the trace is too large
+	// should fail b/c the trace is too large. each batch should be ~80 bytes
 	batch := makeThriftBatchWithSpanCount(2)
 	require.Error(t, c.EmitBatch(context.Background(), batch))
 	// should fail b/c this will be too many traces

modules/ingester/instance.go

@@ -332,8 +332,8 @@ func (i *instance) getOrCreateTrace(req *tempopb.PushRequest) (*trace, error) {
 		return nil, status.Errorf(codes.FailedPrecondition, "%s max live traces per tenant exceeded: %v", overrides.ErrorPrefixLiveTracesExceeded, err)
 	}
 
-	maxSpans := i.limiter.limits.MaxSpansPerTrace(i.instanceID)
-	trace = newTrace(maxSpans, fp, traceID)
+	maxBytes := i.limiter.limits.MaxBytesPerTrace(i.instanceID)
+	trace = newTrace(maxBytes, fp, traceID)
 	i.traces[fp] = trace
 	i.tracesCreatedTotal.Inc()
 

modules/ingester/instance_test.go

@@ -215,7 +215,7 @@ func TestInstanceDoesNotRace(t *testing.T) {
 
 func TestInstanceLimits(t *testing.T) {
 	limits, err := overrides.NewOverrides(overrides.Limits{
-		MaxSpansPerTrace: 10,
+		MaxBytesPerTrace: 1000,
 	})
 	assert.NoError(t, err, "unexpected error creating limits")
 	limiter := NewLimiter(limits, &ringCountMock{count: 1}, 1)
@@ -242,39 +242,39 @@ func TestInstanceLimits(t *testing.T) {
 			name: "succeeds",
 			pushes: []push{
 				{
-					req: test.MakeRequest(3, []byte{}),
+					req: test.MakeRequestWithByteLimit(300, []byte{}),
 				},
 				{
-					req: test.MakeRequest(5, []byte{}),
+					req: test.MakeRequestWithByteLimit(500, []byte{}),
 				},
 				{
-					req: test.MakeRequest(9, []byte{}),
+					req: test.MakeRequestWithByteLimit(900, []byte{}),
 				},
 			},
 		},
 		{
 			name: "one fails",
 			pushes: []push{
 				{
-					req: test.MakeRequest(3, []byte{}),
+					req: test.MakeRequestWithByteLimit(300, []byte{}),
 				},
 				{
-					req:          test.MakeRequest(15, []byte{}),
+					req:          test.MakeRequestWithByteLimit(1500, []byte{}),
 					expectsError: true,
 				},
 				{
-					req: test.MakeRequest(9, []byte{}),
+					req: test.MakeRequestWithByteLimit(900, []byte{}),
 				},
 			},
 		},
 		{
 			name: "multiple pushes same trace",
 			pushes: []push{
 				{
-					req: test.MakeRequest(5, []byte{0x01}),
+					req: test.MakeRequestWithByteLimit(500, []byte{0x01}),
 				},
 				{
-					req:          test.MakeRequest(7, []byte{0x01}),
+					req:          test.MakeRequestWithByteLimit(700, []byte{0x01}),
 					expectsError: true,
 				},
 			},

modules/ingester/trace.go

@@ -15,34 +15,29 @@ type trace struct {
 	token        uint32
 	lastAppend   time.Time
 	traceID      []byte
-	maxSpans     int
-	currentSpans int
+	maxBytes     int
+	currentBytes int
 }
 
-func newTrace(maxSpans int, token uint32, traceID []byte) *trace {
+func newTrace(maxBytes int, token uint32, traceID []byte) *trace {
 	return &trace{
 		token:      token,
 		trace:      &tempopb.Trace{},
 		lastAppend: time.Now(),
 		traceID:    traceID,
-		maxSpans:   maxSpans,
+		maxBytes:   maxBytes,
 	}
 }
 
 func (t *trace) Push(_ context.Context, req *tempopb.PushRequest) error {
 	t.lastAppend = time.Now()
-	if t.maxSpans != 0 {
-		// count spans
-		spanCount := 0
-		for _, ils := range req.Batch.InstrumentationLibrarySpans {
-			spanCount += len(ils.Spans)
+	if t.maxBytes != 0 {
+		reqSize := req.Size()
+		if t.currentBytes+reqSize > t.maxBytes {
+			return status.Errorf(codes.FailedPrecondition, "%s max size of trace (%d) exceeded while adding %d bytes", overrides.ErrorPrefixTraceTooLarge, t.maxBytes, reqSize)
 		}
 
-		if t.currentSpans+spanCount > t.maxSpans {
-			return status.Errorf(codes.FailedPrecondition, "%s totalSpans (%d) exceeded while adding %d spans", overrides.ErrorPrefixTraceTooLarge, t.maxSpans, spanCount)
-		}
-
-		t.currentSpans += spanCount
+		t.currentBytes += reqSize
 	}
 
 	t.trace.Batches = append(t.trace.Batches, req.Batch)

modules/overrides/limits.go

@@ -30,7 +30,7 @@ type Limits struct {
 	// Ingester enforced limits.
 	MaxLocalTracesPerUser  int `yaml:"max_traces_per_user"`
 	MaxGlobalTracesPerUser int `yaml:"max_global_traces_per_user"`
-	MaxSpansPerTrace       int `yaml:"max_spans_per_trace"`
+	MaxBytesPerTrace       int `yaml:"max_bytes_per_trace"`
 
 	// Compactor enforced limits.
 	BlockRetention time.Duration `yaml:"block_retention"`
@@ -50,7 +50,7 @@ func (l *Limits) RegisterFlags(f *flag.FlagSet) {
 	// Ingester limits
 	f.IntVar(&l.MaxLocalTracesPerUser, "ingester.max-traces-per-user", 10e3, "Maximum number of active traces per user, per ingester. 0 to disable.")
 	f.IntVar(&l.MaxGlobalTracesPerUser, "ingester.max-global-traces-per-user", 0, "Maximum number of active traces per user, across the cluster. 0 to disable.")
-	f.IntVar(&l.MaxSpansPerTrace, "ingester.max-spans-per-trace", 50e3, "Maximum number of spans per trace.  0 to disable.")
+	f.IntVar(&l.MaxBytesPerTrace, "ingester.max-bytes-per-trace", 50e5, "Maximum size of a trace in bytes.  0 to disable.")
 
 	f.StringVar(&l.PerTenantOverrideConfig, "limits.per-user-override-config", "", "File name of per-user overrides.")
 	f.DurationVar(&l.PerTenantOverridePeriod, "limits.per-user-override-period", 10*time.Second, "Period with this to reload the overrides.")

modules/overrides/overrides.go

@@ -142,9 +142,9 @@ func (o *Overrides) MaxGlobalTracesPerUser(userID string) int {
 	return o.getOverridesForUser(userID).MaxGlobalTracesPerUser
 }
 
-// MaxSpansPerTrace returns the maximum number of spans a user can have in a live trace.
-func (o *Overrides) MaxSpansPerTrace(userID string) int {
-	return o.getOverridesForUser(userID).MaxSpansPerTrace
+// MaxBytesPerTrace returns the maximum size of a single trace in bytes allowed for a user.
+func (o *Overrides) MaxBytesPerTrace(userID string) int {
+	return o.getOverridesForUser(userID).MaxBytesPerTrace
 }
 
 // IngestionRateSpans is the number of spans per second allowed for this tenant

modules/overrides/overrides_test.go

@@ -23,7 +23,7 @@ func TestOverrides(t *testing.T) {
 		overrides                   *perTenantOverrides
 		expectedMaxLocalTraces      map[string]int
 		expectedMaxGlobalTraces     map[string]int
-		expectedMaxSpansPerTrace    map[string]int
+		expectedMaxBytesPerTrace    map[string]int
 		expectedIngestionRateSpans  map[string]int
 		expectedIngestionBurstSpans map[string]int
 	}{
@@ -32,13 +32,13 @@ func TestOverrides(t *testing.T) {
 			limits: Limits{
 				MaxGlobalTracesPerUser: 1,
 				MaxLocalTracesPerUser:  2,
-				MaxSpansPerTrace:       3,
+				MaxBytesPerTrace:       3,
 				IngestionBurstSize:     4,
 				IngestionRateSpans:     5,
 			},
 			expectedMaxGlobalTraces:     map[string]int{"user1": 1, "user2": 1},
 			expectedMaxLocalTraces:      map[string]int{"user1": 2, "user2": 2},
-			expectedMaxSpansPerTrace:    map[string]int{"user1": 3, "user2": 3},
+			expectedMaxBytesPerTrace:    map[string]int{"user1": 3, "user2": 3},
 			expectedIngestionBurstSpans: map[string]int{"user1": 4, "user2": 4},
 			expectedIngestionRateSpans:  map[string]int{"user1": 5, "user2": 5},
 		},
@@ -47,7 +47,7 @@ func TestOverrides(t *testing.T) {
 			limits: Limits{
 				MaxGlobalTracesPerUser: 1,
 				MaxLocalTracesPerUser:  2,
-				MaxSpansPerTrace:       3,
+				MaxBytesPerTrace:       3,
 				IngestionBurstSize:     4,
 				IngestionRateSpans:     5,
 			},
@@ -56,15 +56,15 @@ func TestOverrides(t *testing.T) {
 					"user1": {
 						MaxGlobalTracesPerUser: 6,
 						MaxLocalTracesPerUser:  7,
-						MaxSpansPerTrace:       8,
+						MaxBytesPerTrace:       8,
 						IngestionBurstSize:     9,
 						IngestionRateSpans:     10,
 					},
 				},
 			},
 			expectedMaxGlobalTraces:     map[string]int{"user1": 6, "user2": 1},
 			expectedMaxLocalTraces:      map[string]int{"user1": 7, "user2": 2},
-			expectedMaxSpansPerTrace:    map[string]int{"user1": 8, "user2": 3},
+			expectedMaxBytesPerTrace:    map[string]int{"user1": 8, "user2": 3},
 			expectedIngestionBurstSpans: map[string]int{"user1": 9, "user2": 4},
 			expectedIngestionRateSpans:  map[string]int{"user1": 10, "user2": 5},
 		},
@@ -73,7 +73,7 @@ func TestOverrides(t *testing.T) {
 			limits: Limits{
 				MaxGlobalTracesPerUser: 1,
 				MaxLocalTracesPerUser:  2,
-				MaxSpansPerTrace:       3,
+				MaxBytesPerTrace:       3,
 				IngestionBurstSize:     4,
 				IngestionRateSpans:     5,
 			},
@@ -82,22 +82,22 @@ func TestOverrides(t *testing.T) {
 					"user1": {
 						MaxGlobalTracesPerUser: 6,
 						MaxLocalTracesPerUser:  7,
-						MaxSpansPerTrace:       8,
+						MaxBytesPerTrace:       8,
 						IngestionBurstSize:     9,
 						IngestionRateSpans:     10,
 					},
 					"*": {
 						MaxGlobalTracesPerUser: 11,
 						MaxLocalTracesPerUser:  12,
-						MaxSpansPerTrace:       13,
+						MaxBytesPerTrace:       13,
 						IngestionBurstSize:     14,
 						IngestionRateSpans:     15,
 					},
 				},
 			},
 			expectedMaxGlobalTraces:     map[string]int{"user1": 6, "user2": 11},
 			expectedMaxLocalTraces:      map[string]int{"user1": 7, "user2": 12},
-			expectedMaxSpansPerTrace:    map[string]int{"user1": 8, "user2": 13},
+			expectedMaxBytesPerTrace:    map[string]int{"user1": 8, "user2": 13},
 			expectedIngestionBurstSpans: map[string]int{"user1": 9, "user2": 14},
 			expectedIngestionRateSpans:  map[string]int{"user1": 10, "user2": 15},
 		},

operations/jsonnet/microservices/config.libsonnet

@@ -50,7 +50,7 @@
         max_traces_per_user: 100000,
         ingestion_rate_limit: 150000,
         ingestion_burst_size: 150000,
-        max_spans_per_trace: 200e3,
+        max_bytes_per_trace: 300e5,  // ~30MB
       },
     },
   },

pkg/util/test/req.go

@@ -69,3 +69,36 @@ func MakeTraceWithSpanCount(requests int, spansEach int, traceID []byte) *tempop
 
 	return trace
 }
+
+// Note that this fn will generate a request with size **close to** maxBytes
+func MakeRequestWithByteLimit(maxBytes int, traceID []byte) *tempopb.PushRequest {
+	if len(traceID) == 0 {
+		traceID = make([]byte, 16)
+		rand.Read(traceID)
+	}
+
+	req := &tempopb.PushRequest{
+		Batch: &v1_trace.ResourceSpans{},
+	}
+
+	ils := &v1_trace.InstrumentationLibrarySpans{
+		InstrumentationLibrary: &v1_common.InstrumentationLibrary{
+			Name:    "super library",
+			Version: "0.0.1",
+		},
+	}
+	req.Batch.InstrumentationLibrarySpans = append(req.Batch.InstrumentationLibrarySpans, ils)
+
+	for req.Size() < maxBytes {
+		sampleSpan := v1_trace.Span{
+			Name:    "test",
+			TraceId: traceID,
+			SpanId:  make([]byte, 8),
+		}
+		rand.Read(sampleSpan.SpanId)
+
+		ils.Spans = append(ils.Spans, &sampleSpan)
+	}
+
+	return req
+}