sql: revert "sql: use stmt's span for exec stats propagation"

This is a "by-hand" revert of 842d79b and motivates the next commit.
The only "by-hand" bits comes from wanting to retain the comments
added in that commit.

To reduce the memory overhead for tracing, statements will stop using
real spans unless absolutely required (for e.g. when SET TRACING = on).
The intent behind #61532 was to reduce the overhead of sampled
statements by not creating an additional new span when one was already
available (the statement's span). Well, with the next commit, the
statement's span will be a no-op one, so when we know that we're
sampling we'll want create a new span.

In the end, with respect to spans created, we'll be doing no worse for
sampled statements relative to #61532.

Release note: None

pkg/sql/instrumentation.go

@@ -92,11 +92,9 @@ type instrumentationHelper struct {
 	finishCollectionDiagnostics func()
 	withStatementTrace          func(trace tracing.Recording, stmt string)
 
-	sp *tracing.Span
-	// If true, the instrumentation helper is responsible for finishing sp.
-	finishSpan bool
-	origCtx    context.Context
-	evalCtx    *tree.EvalContext
+	sp      *tracing.Span
+	origCtx context.Context
+	evalCtx *tree.EvalContext
 
 	// If savePlanForStats is true, the explainPlan will be collected and returned
 	// via PlanForStats().
@@ -167,8 +165,6 @@ func (ih *instrumentationHelper) Setup(
 	ih.savePlanForStats = appStats.shouldSaveLogicalPlanDescription(fingerprint, implicitTxn)
 
 	if sp := tracing.SpanFromContext(ctx); sp != nil {
-		ih.sp = sp
-		ih.finishSpan = false
 		if sp.IsVerbose() {
 			// If verbose tracing was enabled at a higher level, stats
 			// collection is enabled so that stats are shown in the traces, but
@@ -186,18 +182,10 @@ func (ih *instrumentationHelper) Setup(
 
 	if !ih.collectBundle && ih.withStatementTrace == nil && ih.outputMode == unmodifiedOutput {
 		if ih.collectExecStats {
+			// If we need to collect stats, create a non-verbose child span. Stats
+			// will be added as structured metadata and processed in Finish.
 			ih.origCtx = ctx
-			if ih.sp != nil {
-				// The span present in the context is sufficient for us to
-				// record stats as structured metadata, so there is nothing else
-				// to do.
-				return ctx, true
-			}
-			// This is an edge case when the span is not present in the original
-			// context for some reason. This should *never* happen, but we
-			// choose to be defensive about it.
 			newCtx, ih.sp = tracing.EnsureChildSpan(ctx, cfg.AmbientCtx.Tracer, "traced statement")
-			ih.finishSpan = true
 			return newCtx, true
 		}
 		return ctx, false
@@ -208,7 +196,6 @@ func (ih *instrumentationHelper) Setup(
 	ih.origCtx = ctx
 	ih.evalCtx = p.EvalContext()
 	newCtx, ih.sp = tracing.StartVerboseTrace(ctx, cfg.AmbientCtx.Tracer, "traced statement")
-	ih.finishSpan = true
 	return newCtx, true
 }
 
@@ -224,9 +211,10 @@ func (ih *instrumentationHelper) Finish(
 	retErr error,
 ) error {
 	ctx := ih.origCtx
-	if ih.finishSpan {
-		ih.sp.Finish()
+	if ih.sp == nil {
+		return retErr
 	}
+	ih.sp.Finish()
 
 	// Record the statement information that we've collected.
 	// Note that in case of implicit transactions, the trace contains the auto-commit too.