From 64d6d87f24a0d0db321f5209da3fefe84020a6ee Mon Sep 17 00:00:00 2001
From: Tobias Grieger <tobias.b.grieger@gmail.com>
Date: Mon, 25 Jan 2021 11:34:48 +0100
Subject: [PATCH] tracing: limit number of direct children

In #59347 we saw an OOM due to more and more children being registered
with a long-running trace Span (held open by a raft scheduler worker
goroutine). This commit adds a simple guardrail: no more than 1000 spans
can be registered directly with a single parent; any in excess are
simply not registered at all.

Fixes #59347.

Release note: None
---
 pkg/util/tracing/crdbspan.go  |  5 ++++-
 pkg/util/tracing/span_test.go | 18 ++++++++++++++++++
 pkg/util/tracing/tracer.go    |  8 ++++++--
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/pkg/util/tracing/crdbspan.go b/pkg/util/tracing/crdbspan.go
index 0ab84dc0ff2a..4ae2211351de 100644
--- a/pkg/util/tracing/crdbspan.go
+++ b/pkg/util/tracing/crdbspan.go
@@ -324,7 +324,10 @@ func (s *crdbSpan) getRecordingLocked(m mode) tracingpb.RecordedSpan {
 
 func (s *crdbSpan) addChild(child *crdbSpan) {
 	s.mu.Lock()
-	s.mu.recording.children = append(s.mu.recording.children, child)
+	// Only record the child if the parent still has room.
+	if len(s.mu.recording.children) < maxChildrenPerSpan {
+		s.mu.recording.children = append(s.mu.recording.children, child)
+	}
 	s.mu.Unlock()
 }
 
diff --git a/pkg/util/tracing/span_test.go b/pkg/util/tracing/span_test.go
index 1e1e72104165..8ab66d94e847 100644
--- a/pkg/util/tracing/span_test.go
+++ b/pkg/util/tracing/span_test.go
@@ -11,6 +11,7 @@
 package tracing
 
 import (
+	"fmt"
 	"regexp"
 	"strings"
 	"testing"
@@ -212,3 +213,20 @@ func TestNonVerboseChildSpanRegisteredWithParent(t *testing.T) {
 	require.Len(t, rec, 2)
 	require.Len(t, rec[1].InternalStructured, 1)
 }
+
+// TestSpanMaxChildren verifies that a Span can
+// track at most maxChildrenPerSpan direct children.
+func TestSpanMaxChildren(t *testing.T) {
+	tr := NewTracer()
+	sp := tr.StartSpan("root", WithForceRealSpan())
+	defer sp.Finish()
+	for i := 0; i < maxChildrenPerSpan+123; i++ {
+		ch := tr.StartSpan(fmt.Sprintf("child %d", i), WithParentAndAutoCollection(sp), WithForceRealSpan())
+		ch.Finish()
+		exp := i + 1
+		if exp > maxChildrenPerSpan {
+			exp = maxChildrenPerSpan
+		}
+		require.Len(t, sp.crdb.mu.recording.children, exp)
+	}
+}
diff --git a/pkg/util/tracing/tracer.go b/pkg/util/tracing/tracer.go
index 430f32762513..446e5dd096bd 100644
--- a/pkg/util/tracing/tracer.go
+++ b/pkg/util/tracing/tracer.go
@@ -37,8 +37,12 @@ import (
 // and since this string goes on the wire, it's a hassle to change it now.
 const verboseTracingBaggageKey = "sb"
 
-// maxLogsPerSpan limits the number of logs in a Span; use a comfortable limit.
-const maxLogsPerSpan = 1000
+const (
+	// maxLogsPerSpan limits the number of logs in a Span; use a comfortable limit.
+	maxLogsPerSpan = 1000
+	// maxChildrenPerSpan limits the number of (direct) child spans in a Span.
+	maxChildrenPerSpan = 1000
+)
 
 // These constants are used to form keys to represent tracing context
 // information in carriers supporting opentracing.HTTPHeaders format.