Fix panic in SpanDeduper when traceID is empty slice (#3668)

* Fix panic in SpanDeduper when traceID is empty slice

* changelog

(cherry picked from commit d8c69d4)

CHANGELOG.md

@@ -33,6 +33,7 @@
 * [BUGFIX] Fix metrics query results when filtering and rating on the same attribute [#3428](https://github.com/grafana/tempo/issues/3428) (@mdisibio)
 * [BUGFIX] Fix metrics query results when series contain empty strings or nil values [#3429](https://github.com/grafana/tempo/issues/3429) (@mdisibio)
 * [BUGFIX] Fix metrics query duration check, add per-tenant override for max metrics query duration [#3479](https://github.com/grafana/tempo/issues/3479) (@mdisibio)
+* [BUGFIX] Fix metrics query panic "index out of range [-1]" when a trace has zero-length ID [](https://github.com/grafana/tempo/pull/3668) (@mdisibio)
 * [BUGFIX] Return unfiltered results when a bad TraceQL query is provided in autocomplete. [#3426](https://github.com/grafana/tempo/pull/3426) (@mapno)
 * [BUGFIX] Add support for dashes, quotes and spaces in attribute names in autocomplete [#3458](https://github.com/grafana/tempo/pull/3458) (@mapno)
 * [BUGFIX] Correctly handle 429s in GRPC search streaming. [#3469](https://github.com/grafana/tempo/pull/3469) (@joe-ellitot)

pkg/traceql/engine_metrics.go

@@ -624,7 +624,15 @@ func (d *SpanDeduper2) Skip(tid []byte, startTime uint64) bool {
 	d.h.Write(d.buf)
 
 	v := d.h.Sum32()
-	m := d.m[tid[len(tid)-1]]
+
+	// Use last byte of the trace to choose the submap.
+	// Empty ID uses submap 0.
+	mapIdx := byte(0)
+	if len(tid) > 0 {
+		mapIdx = tid[len(tid)-1]
+	}
+
+	m := d.m[mapIdx]
 
 	if _, ok := m[v]; ok {
 		return true

pkg/traceql/engine_metrics_test.go

@@ -290,3 +290,26 @@ func TestCompileMetricsQueryRangeFetchSpansRequest(t *testing.T) {
 		})
 	}
 }
+
+func TestSpanDeduper(t *testing.T) {
+	d := NewSpanDeduper2()
+
+	in := []struct {
+		tid []byte
+		ts  uint64
+	}{
+		{nil, 0},
+		{[]byte{1}, 1},
+		{[]byte{1, 1}, 1},
+		{[]byte{1, 2}, 2},
+	}
+
+	for _, tc := range in {
+		// First call is always false
+		require.False(t, d.Skip(tc.tid, tc.ts))
+
+		// Second call is always true
+		require.True(t, d.Skip(tc.tid, tc.ts))
+	}
+	d.Skip(nil, 0)
+}