nits/cleanup comments

mtoffl01 · mtoffl01 · commit c25d80cf6c88 · 2026-05-01T16:53:59.000-04:00
diff --git a/ddtrace/tracer/tracer.go b/ddtrace/tracer/tracer.go
@@ -926,24 +926,24 @@ func (t *tracer) StartSpan(operationName string, options ...StartSpanOption) *Sp
 	}
 	span := spanStart(operationName, &t.sharedAttrs, options...)
 
-	// Snapshot all stable config fields needed below under a single RLock to avoid
+	// Snapshot all internal config fields needed below under a single RLock to avoid
 	// reader-counter contention on Config.mu when many goroutines call StartSpan.
-	snap := t.config.internalConfig.SpanStartSnapshot()
+	cSnap := t.config.internalConfig.SpanStartSnapshot()
 
 	if span.service == "" {
-		span.service = snap.ServiceName
+		span.service = cSnap.ServiceName
 	}
 
 	// For non-universal version, promote main-service spans to the version-inclusive
 	// shared attrs before applying any tags. This makes the subsequent version write
 	// (from config or global tags) a COW no-op instead of triggering a Clone.
-	if !t.config.universalVersion && span.service == snap.ServiceName {
+	if !t.config.universalVersion && span.service == cSnap.ServiceName {
 		span.meta.ReplaceSharedAttrs(&t.sharedAttrs, &t.sharedAttrsForMainSvc)
 	}
 
-	span.noDebugStack = !snap.DebugStack
-	if snap.Hostname != "" && snap.ReportHostname {
-		span.setMetaInit(keyHostname, snap.Hostname)
+	span.noDebugStack = !cSnap.DebugStack
+	if cSnap.Hostname != "" && cSnap.ReportHostname {
+		span.setMetaInit(keyHostname, cSnap.Hostname)
 	}
 	span.supportsEvents = t.config.agent.load().spanEventsAvailable
 
@@ -955,15 +955,15 @@ func (t *tracer) StartSpan(operationName string, options ...StartSpanOption) *Sp
 		span.serviceSource = ext.ServiceSourceMapping
 	}
 
-	if snap.Version != "" {
-		if t.config.universalVersion || span.service == snap.ServiceName {
+	if cSnap.Version != "" {
+		if t.config.universalVersion || span.service == cSnap.ServiceName {
 			delete(span.metrics, ext.Version)
-			span.meta.Set(ext.Version, snap.Version)
+			span.meta.Set(ext.Version, cSnap.Version)
 		}
 	}
-	if snap.Env != "" {
+	if cSnap.Env != "" {
 		delete(span.metrics, ext.Environment)
-		span.meta.Set(ext.Environment, snap.Env)
+		span.meta.Set(ext.Environment, cSnap.Env)
 	}
 	if _, ok := span.context.SamplingPriority(); !ok {
 		// if not already sampled or a brand new trace, sample it
@@ -974,12 +974,12 @@ func (t *tracer) StartSpan(operationName string, options ...StartSpanOption) *Sp
 		log.Debug("Started Span: %v, Operation: %s, Resource: %s, Tags: %v, %v", //nolint:gocritic // Debug logging needs full span representation
 			span, span.name, span.resource, &span.meta, span.metrics)
 	}
-	if snap.ProfilerHotspotsEnabled || snap.ProfilerEndpoints {
-		t.applyPPROFLabels(span.pprofCtxRestore, span, snap)
+	if cSnap.ProfilerHotspotsEnabled || cSnap.ProfilerEndpoints {
+		t.applyPPROFLabels(span.pprofCtxRestore, span, cSnap)
 	} else {
 		span.pprofCtxRestore = nil
 	}
-	if snap.DebugAbandonedSpans {
+	if cSnap.DebugAbandonedSpans {
 		select {
 		case t.abandonedSpansDebugger.In <- newAbandonedSpanCandidate(span, false):
 			// ok
diff --git a/internal/config/README.md b/internal/config/README.md
@@ -62,6 +62,6 @@ for attempt := 0; attempt <= sendRetries; attempt++ {
 When a hot path reads ~3+ `Config` fields, define a snapshot struct + method in `snapshots.go` and have the caller read from the local copy.
 
 - **Why**: at high concurrency the bottleneck isn't blocking — readers don't block each other — but cache-line contention on `sync.RWMutex`'s reader counter. Folding N `RLock` pairs into 1 collapses N atomic ops on a shared cache line into 1.
-- **Convention**: one bespoke struct per caller. Don't build a generic `Snapshot(fields...)` API — see the comment at the top of `snapshots.go` for the reasoning.
+- **Convention**: one bespoke struct per caller (e.g, a calling function `StartSpan` gets a snapshot API called `SpanStartSnapshot`).
 - **Prior art**: `SpanStartSnapshot` for `tracer.StartSpan` (13 → 1 RLock acquisitions, ~60% speedup on `BenchmarkStartSpanConcurrent-8`).
 
diff --git a/internal/config/config.go b/internal/config/config.go
@@ -61,7 +61,7 @@ type programmaticOverride struct {
 // Config represents global configuration properties.
 // Config instances should be obtained via Get() which always returns a non-nil value.
 // Methods on Config assume a non-nil receiver and will panic if called on nil.
-// Hot paths that read many fields should use a snapshot (see snapshots.go) to
+// Hot paths that read many fields within a single function should use a snapshot (see snapshots.go) to
 // avoid per-field RLock contention on the reader counter.
 type Config struct {
 	mu sync.RWMutex
diff --git a/internal/config/snapshots.go b/internal/config/snapshots.go
@@ -9,15 +9,11 @@
 // on the RWMutex's reader counter when many goroutines call the hot path
 // concurrently.
 //
-// Convention: one bespoke struct + method per caller. A generic
-// Snapshot(fields...) API would either box values through interface{} (costly
-// in a hot path) or expose unexported fields through a callback (no win over
-// adding a method here directly). Add a new struct + method below when a new
-// hot path needs more than ~3 fields under lock.
+// Add a new struct + method below when a new hot path needs more than ~3 fields
+// under the lock.
 
 package config
 
-// SpanStartSnapshot holds the config fields read by tracer.StartSpan.
 type SpanStartSnapshot struct {
 	ServiceName             string
 	Env                     string
