Merge cockroachdb#132859

132859: kvserver: only start split tracing if we might record it r=kvoli a=andrewbaptist Previously we always started tracing in the replicate queue but only recorded the span if expensive logging was enabled. Now we check earlier in the process. Part of: cockroachdb#104035 Release note: None Co-authored-by: Andrew Baptist <[email protected]>
mgartner · Oct 18, 2024 · 34429e1 · 34429e1
2 parents ed15efe + 5cc445a
commit 34429e1
Showing 1 changed file with 21 additions and 19 deletions.
diff --git a/pkg/kv/kvserver/split_queue.go b/pkg/kv/kvserver/split_queue.go
@@ -242,33 +242,35 @@ func (sq *splitQueue) processAttemptWithTracing(
 	ctx context.Context, r *Replica, confReader spanconfig.StoreReader,
 ) (processed bool, _ error) {
 	processStart := r.Clock().PhysicalTime()
-	ctx, sp := tracing.EnsureChildSpan(ctx, sq.Tracer, "split",
-		tracing.WithRecording(tracingpb.RecordingVerbose))
+	startTracing := log.ExpensiveLogEnabled(ctx, 1)
+	var opts []tracing.SpanOption
+	if startTracing {
+		opts = append(opts, tracing.WithRecording(tracingpb.RecordingVerbose))
+	}
+	ctx, sp := tracing.EnsureChildSpan(ctx, sq.Tracer, "split", opts...)
 	defer sp.Finish()
 
 	processed, err := sq.processAttempt(ctx, r, confReader)
-
-	// Utilize a new background context (properly annotated) to avoid writing
-	// traces from a child context into its parent.
-	{
-		ctx := r.AnnotateCtx(sq.AnnotateCtx(context.Background()))
-		processDuration := r.Clock().PhysicalTime().Sub(processStart)
-		exceededDuration := sq.logTracesThreshold > time.Duration(0) && processDuration > sq.logTracesThreshold
-
-		var traceOutput redact.RedactableString
-		traceLoggingNeeded := (err != nil || exceededDuration) && log.ExpensiveLogEnabled(ctx, 1)
+	processDuration := r.Clock().PhysicalTime().Sub(processStart)
+	exceededDuration := sq.logTracesThreshold > time.Duration(0) && processDuration > sq.logTracesThreshold
+	var traceOutput redact.RedactableString
+	if startTracing {
+		// Utilize a new background context (properly annotated) to avoid writing
+		// traces from a child context into its parent.
+		ctx = r.AnnotateCtx(sq.AnnotateCtx(context.Background()))
+
+		traceLoggingNeeded := (err != nil || exceededDuration)
 		if traceLoggingNeeded {
 			// Add any trace filtering here if the output is too verbose.
 			rec := sp.GetConfiguredRecording()
 			traceOutput = redact.Sprintf("\ntrace:\n%s", rec)
 		}
-
-		if err != nil {
-			log.Infof(ctx, "error during range split: %v%s", err, traceOutput)
-		} else if exceededDuration {
-			log.Infof(ctx, "range split took %s, exceeding threshold of %s%s",
-				processDuration, sq.logTracesThreshold, traceOutput)
-		}
+	}
+	if err != nil {
+		log.Infof(ctx, "error during range split: %v%s", err, traceOutput)
+	} else if exceededDuration {
+		log.Infof(ctx, "range split took %s, exceeding threshold of %s%s",
+			processDuration, sq.logTracesThreshold, traceOutput)
 	}
 
 	return processed, err