refactor(tracer): add checklocks annotations to Span struct

Add compile-time lock verification annotations to the Span struct,
the most critical data structure in the tracing library.

Signed-off-by: Kemal Akkoyun <kemal.akkoyun@datadoghq.com>

Author: kakkoyun

ddtrace/tracer/abandonedspans.go

@@ -82,6 +82,7 @@ type abandonedSpanCandidate struct {
 	Integration     string
 }
 
+// +checklocksignore
 func newAbandonedSpanCandidate(s *Span, finished bool) *abandonedSpanCandidate {
 	var component string
 	if v, ok := s.meta[ext.Component]; ok {

ddtrace/tracer/civisibility_tslv.go

@@ -160,6 +160,8 @@ type ciVisibilityEvent struct {
 //
 //	key - The tag key.
 //	value - The tag value.
+//
+// +checklocksignore
 func (e *ciVisibilityEvent) SetTag(key string, value interface{}) {
 	e.span.SetTag(key, value)
 	e.Content.Meta = e.span.meta
@@ -171,6 +173,8 @@ func (e *ciVisibilityEvent) SetTag(key string, value interface{}) {
 // Parameters:
 //
 //	operationName - The new operation name.
+//
+// +checklocksignore
 func (e *ciVisibilityEvent) SetOperationName(operationName string) {
 	e.span.SetOperationName(operationName)
 	e.Content.Name = e.span.name
@@ -246,6 +250,8 @@ type tslvSpan struct {
 // Returns:
 //
 //	A pointer to the created ciVisibilityEvent.
+//
+// +checklocksignore
 func getCiVisibilityEvent(span *Span) *ciVisibilityEvent {
 	switch span.spanType {
 	case constants.SpanTypeTest:
@@ -270,6 +276,8 @@ func getCiVisibilityEvent(span *Span) *ciVisibilityEvent {
 // Returns:
 //
 //	A pointer to the created ciVisibilityEvent.
+//
+// +checklocksignore
 func createTestEventFromSpan(span *Span) *ciVisibilityEvent {
 	tSpan := createTslvSpan(span)
 	tSpan.ParentID = 0
@@ -296,6 +304,8 @@ func createTestEventFromSpan(span *Span) *ciVisibilityEvent {
 // Returns:
 //
 //	A pointer to the created ciVisibilityEvent.
+//
+// +checklocksignore
 func createTestSuiteEventFromSpan(span *Span) *ciVisibilityEvent {
 	tSpan := createTslvSpan(span)
 	tSpan.ParentID = 0
@@ -319,6 +329,8 @@ func createTestSuiteEventFromSpan(span *Span) *ciVisibilityEvent {
 // Returns:
 //
 //	A pointer to the created ciVisibilityEvent.
+//
+// +checklocksignore
 func createTestModuleEventFromSpan(span *Span) *ciVisibilityEvent {
 	tSpan := createTslvSpan(span)
 	tSpan.ParentID = 0
@@ -341,6 +353,8 @@ func createTestModuleEventFromSpan(span *Span) *ciVisibilityEvent {
 // Returns:
 //
 //	A pointer to the created ciVisibilityEvent.
+//
+// +checklocksignore
 func createTestSessionEventFromSpan(span *Span) *ciVisibilityEvent {
 	tSpan := createTslvSpan(span)
 	tSpan.ParentID = 0
@@ -362,6 +376,8 @@ func createTestSessionEventFromSpan(span *Span) *ciVisibilityEvent {
 // Returns:
 //
 //	A pointer to the created ciVisibilityEvent.
+//
+// +checklocksignore
 func createSpanEventFromSpan(span *Span) *ciVisibilityEvent {
 	tSpan := createTslvSpan(span)
 	tSpan.SpanID = span.spanID
@@ -383,6 +399,8 @@ func createSpanEventFromSpan(span *Span) *ciVisibilityEvent {
 // Returns:
 //
 //	The created tslvSpan.
+//
+// +checklocksignore
 func createTslvSpan(span *Span) tslvSpan {
 	return tslvSpan{
 		Name:     span.name,

ddtrace/tracer/context.go

@@ -76,6 +76,7 @@ func SpanFromContext(ctx context.Context) (*Span, bool) {
 // StartSpanFromContext returns a new span with the given operation name and options. If a span
 // is found in the context, it will be used as the parent of the resulting span. If the ChildOf
 // option is passed, it will only be used as the parent if there is no span found in `ctx`.
+// +checklocksignore
 func StartSpanFromContext(ctx context.Context, operationName string, opts ...StartSpanOption) (*Span, context.Context) {
 	// copy opts in case the caller reuses the slice in parallel
 	// we will add at least 1, at most 2 items

ddtrace/tracer/payload_v1.go

@@ -103,6 +103,7 @@ func newPayloadV1() *payloadV1 {
 }
 
 // push pushes a new item (a traceChunk)into the payload.
+// +checklocksignore
 func (p *payloadV1) push(t spanList) (stats payloadStats, err error) {
 	// We need to hydrate the payload with everything we get from the spans.
 	// Conceptually, our `t spanList` corresponds to one `traceChunk`.
@@ -421,6 +422,7 @@ func (p *payloadV1) encodeTraceChunks(bm bitmap, fieldID int, tc []traceChunk, s
 }
 
 // encodeSpans encodes a list of spans associated with fieldID into p.buf in msgp format.
+// +checklocksignore
 func (p *payloadV1) encodeSpans(bm bitmap, fieldID int, spans spanList, st *stringTable) (bool, error) {
 	if len(spans) == 0 || !bm.contains(uint32(fieldID)) {
 		return false, nil
@@ -1069,6 +1071,7 @@ func decodeSpans(b []byte, st *stringTable) (spanList, []byte, error) {
 
 // decode reads a span from a byte slice and populates the associated fields in the span.
 // This should only be used with decoding v1.0 payloads.
+// +checklocksignore
 func (span *Span) decode(b []byte, st *stringTable) ([]byte, error) {
 	numFields, o, err := msgp.ReadMapHeaderBytes(b)
 	for range numFields {

ddtrace/tracer/rules_sampler.go

@@ -187,6 +187,7 @@ func (sr *SamplingRule) EqualsFalseNegative(other *SamplingRule) bool {
 }
 
 // match returns true when the span's details match all the expected values in the rule.
+// +checklocksignore
 func (sr *SamplingRule) match(s *Span) bool {
 	if sr.Service != nil && !sr.Service.MatchString(s.service) {
 		return false
@@ -476,6 +477,7 @@ func (rs *traceRulesSampler) sampleRules(span *Span) bool {
 	return true
 }
 
+// +checklocksignore
 func (rs *traceRulesSampler) applyRate(span *Span, rate float64, now time.Time, sampler samplernames.SamplerName) {
 	// Use the helper method to apply the rate and execute sampling logic with the lock held
 	span.applyRateWithLock(rate, func() {
@@ -547,6 +549,7 @@ func (rs *singleSpanRulesSampler) enabled() bool {
 // apply uses the sampling rules to determine the sampling rate for the
 // provided span. If the rules don't match, then it returns false and the span is not
 // modified.
+// +checklocksignore
 func (rs *singleSpanRulesSampler) apply(span *Span) bool {
 	for _, rule := range rs.rules {
 		if rule.match(span) {

ddtrace/tracer/sampler.go

@@ -92,6 +92,7 @@ func (r *rateSampler) SetRate(rate float64) {
 const knuthFactor = uint64(1111111111111111111)
 
 // Sample returns true if the given span should be sampled.
+// +checklocksignore
 func (r *rateSampler) Sample(s *Span) bool {
 	if r.rate == 1 {
 		// fast path
@@ -185,6 +186,7 @@ func (ps *prioritySampler) readRatesJSON(rc io.ReadCloser) error {
 
 // getRate returns the sampling rate to be used for the given span. Callers must
 // guard the span.
+// +checklocksignore
 func (ps *prioritySampler) getRate(spn *Span) float64 {
 	key := serviceEnvKey{service: spn.service, env: spn.meta[ext.Environment]}
 	ps.mu.RLock()
@@ -197,6 +199,7 @@ func (ps *prioritySampler) getRate(spn *Span) float64 {
 
 // apply applies sampling priority to the given span. Caller must ensure it is safe
 // to modify the span.
+// +checklocksignore
 func (ps *prioritySampler) apply(spn *Span) {
 	rate := ps.getRate(spn)
 	if sampledByRate(spn.traceID, rate) {

ddtrace/tracer/span.go

@@ -5,6 +5,7 @@
 
 //msgp:ignore inheritedData
 //go:generate go run github.com/tinylib/msgp -unexported -marshal=false -o=span_msgp.go -tests=false
+//go:generate go run ../../scripts/msgp_checklocks_ignore.go -type Span -file span_msgp.go
 
 package tracer
 
@@ -70,6 +71,7 @@ type errorConfig struct {
 // properties set at any time during normal operation! This is used for testing only,
 // and should not be used in non-test code, or you may run into performance or other
 // issues.
+// +checklocksignore
 func (s *Span) AsMap() map[string]interface{} {
 	m := make(map[string]interface{})
 	if s == nil {
@@ -100,6 +102,7 @@ func (s *Span) AsMap() map[string]interface{} {
 	return m
 }
 
+// +checklocksignore
 func (s *Span) spanEventsAsJSONString() string {
 	if !s.supportsEvents {
 		return s.meta["events"]
@@ -118,36 +121,56 @@ func (s *Span) spanEventsAsJSONString() string {
 // Span represents a computation. Callers must call Finish when a Span is
 // complete to ensure it's submitted.
 type Span struct {
-	mu locking.RWMutex `msg:"-"` // all fields are protected by this RWMutex
-
-	name       string             `msg:"name"`                  // operation name
-	service    string             `msg:"service"`               // service name (i.e. "grpc.server", "http.request")
-	resource   string             `msg:"resource"`              // resource name (i.e. "/user?id=123", "SELECT * FROM users")
-	spanType   string             `msg:"type"`                  // protocol associated with the span (i.e. "web", "db", "cache")
-	start      int64              `msg:"start"`                 // span start time expressed in nanoseconds since epoch
-	duration   int64              `msg:"duration"`              // duration of the span expressed in nanoseconds
-	meta       map[string]string  `msg:"meta,omitempty"`        // arbitrary map of metadata
-	metaStruct metaStructMap      `msg:"meta_struct,omitempty"` // arbitrary map of metadata with structured values
-	metrics    map[string]float64 `msg:"metrics,omitempty"`     // arbitrary map of numeric metrics
-	spanID     uint64             `msg:"span_id"`               // identifier of this span
-	traceID    uint64             `msg:"trace_id"`              // lower 64-bits of the root span identifier
-	parentID   uint64             `msg:"parent_id"`             // identifier of the span's direct parent
-	error      int32              `msg:"error"`                 // error status of the span; 0 means no errors
-	spanLinks  []SpanLink         `msg:"span_links,omitempty"`  // links to other spans
-	spanEvents []spanEvent        `msg:"span_events,omitempty"` // events produced related to this span
+	// guards below fields
+	mu locking.RWMutex `msg:"-"`
+
+	// +checklocks:mu
+	name string `msg:"name"` // operation name
+	// +checklocks:mu
+	service string `msg:"service"` // service name (i.e. "grpc.server", "http.request")
+	// +checklocks:mu
+	resource string `msg:"resource"` // resource name (i.e. "/user?id=123", "SELECT * FROM users")
+	// +checklocks:mu
+	spanType string `msg:"type"` // protocol associated with the span (i.e. "web", "db", "cache")
+	// +checklocks:mu
+	start int64 `msg:"start"` // span start time expressed in nanoseconds since epoch
+	// +checklocks:mu
+	duration int64 `msg:"duration"` // duration of the span expressed in nanoseconds
+	// +checklocks:mu
+	meta map[string]string `msg:"meta,omitempty"` // arbitrary map of metadata
+	// +checklocks:mu
+	metaStruct metaStructMap `msg:"meta_struct,omitempty"` // arbitrary map of metadata with structured values
+	// +checklocks:mu
+	metrics map[string]float64 `msg:"metrics,omitempty"` // arbitrary map of numeric metrics
+	// +checklocks:mu
+	spanID uint64 `msg:"span_id"` // identifier of this span
+	// +checklocks:mu
+	traceID uint64 `msg:"trace_id"` // lower 64-bits of the root span identifier
+	// +checklocks:mu
+	parentID uint64 `msg:"parent_id"` // identifier of the span's direct parent
+	// +checklocks:mu
+	error int32 `msg:"error"` // error status of the span; 0 means no errors
+	// +checklocks:mu
+	spanLinks []SpanLink `msg:"span_links,omitempty"` // links to other spans
+	// +checklocks:mu
+	spanEvents []spanEvent `msg:"span_events,omitempty"` // events produced related to this span
 
 	goExecTraced   bool         `msg:"-"`
 	noDebugStack   bool         `msg:"-"` // disables debug stack traces
-	finished       bool         `msg:"-"` // true if the span has been submitted to a tracer. Can only be read/modified if the trace is locked.
 	context        *SpanContext `msg:"-"` // span propagation context
-	integration    string       `msg:"-"` // where the span was started from, such as a specific contrib or "manual"
 	supportsEvents bool         `msg:"-"` // whether the span supports native span events or not
 
-	pprofCtxActive  context.Context `msg:"-"` // contains pprof.WithLabel labels to tell the profiler more about this span
+	// +checklocks:mu
+	finished bool `msg:"-"` // true if the span has been submitted to a tracer. Can only be read/modified if the trace is locked.
+	// +checklocks:mu
+	integration string `msg:"-"` // where the span was started from, such as a specific contrib or "manual"
+	// +checklocks:mu
+	pprofCtxActive context.Context `msg:"-"` // contains pprof.WithLabel labels to tell the profiler more about this span
+
 	pprofCtxRestore context.Context `msg:"-"` // contains pprof.WithLabel labels of the parent span (if any) that need to be restored when this span finishes
 
-	// ends execution tracer (runtime/trace) task, if started
-	taskEnd func() // +checklocks:mu
+	// +checklocks:mu
+	taskEnd func() // ends execution tracer (runtime/trace) task, if started
 }
 
 // Context yields the SpanContext for this Span. Note that the return
@@ -540,6 +563,8 @@ func (s *Span) StartChild(operationName string, opts ...StartSpanOption) *Span {
 
 // setSamplingPriorityLocked updates the sampling priority.
 // It also updates the trace's sampling priority.
+// s.mu must be held for writing.
+// +checklocks:s.mu
 func (s *Span) setSamplingPriorityLocked(priority int, sampler samplernames.SamplerName) {
 	assert.RWMutexLocked(&s.mu)
 	// We don't lock spans when flushing, so we could have a data race when
@@ -556,6 +581,8 @@ func (s *Span) setSamplingPriorityLocked(priority int, sampler samplernames.Samp
 // If the trace is locked, the sampling priority is forced to the given value.
 //
 // This function is should only be used when applying a manual keep or drop decision.
+// s.mu must be held for writing.
+// +checklocks:s.mu
 func (s *Span) forceSetSamplingPriorityLocked(priority int, sampler samplernames.SamplerName) {
 	assert.RWMutexLocked(&s.mu)
 	// We don't lock spans when flushing, so we could have a data race when
@@ -569,7 +596,8 @@ func (s *Span) forceSetSamplingPriorityLocked(priority int, sampler samplernames
 }
 
 // setTagErrorLocked sets the error tag. It accounts for various valid scenarios.
-// This method assumes the span lock is already held.
+// s.mu must be held for writing.
+// +checklocks:s.mu
 func (s *Span) setTagErrorLocked(value interface{}, cfg errorConfig) {
 	assert.RWMutexLocked(&s.mu)
 	setError := func(yes bool) {
@@ -659,12 +687,14 @@ func (s *Span) setMeta(key, v string) {
 }
 
 // setMetaLocked sets a string tag. This method assumes the span lock is already held.
+// +checklocks:s.mu
 func (s *Span) setMetaLocked(key, v string) {
 	assert.RWMutexLocked(&s.mu)
 	s.setMetaInit(key, v)
 }
 
 // setMetaInit sets a string tag without acquiring the lock and asserting the lock is held.
+// +checklocksignore
 func (s *Span) setMetaInit(key, v string) {
 	if s.meta == nil {
 		s.meta = make(map[string]string, 1)
@@ -685,6 +715,7 @@ func (s *Span) setMetaInit(key, v string) {
 }
 
 // setMetaStructLocked sets structured metadata. This method assumes the span lock is already held.
+// +checklocks:s.mu
 func (s *Span) setMetaStructLocked(key string, v any) {
 	assert.RWMutexLocked(&s.mu)
 	if s.metaStruct == nil {
@@ -694,6 +725,7 @@ func (s *Span) setMetaStructLocked(key string, v any) {
 }
 
 // setTagBoolLocked sets a boolean tag on the span. This method assumes the span lock is already held.
+// +checklocks:s.mu
 func (s *Span) setTagBoolLocked(key string, v bool) {
 	assert.RWMutexLocked(&s.mu)
 	switch key {
@@ -722,6 +754,7 @@ func (s *Span) setTagBoolLocked(key string, v bool) {
 
 // setMetric sets a numeric tag during span initialization (before the span is published).
 // This method should only be used during span construction in spanStart and StartSpan.
+// +checklocksignore
 func (s *Span) setMetricInit(key string, v float64) {
 	if s.metrics == nil {
 		s.metrics = make(map[string]float64, 1)
@@ -740,6 +773,7 @@ func (s *Span) setMetric(key string, v float64) {
 
 // setMetricLocked sets a numeric tag, in our case called a metric. This method
 // assumes the span lock is already held.
+// +checklocks:s.mu
 func (s *Span) setMetricLocked(key string, v float64) {
 	assert.RWMutexLocked(&s.mu)
 	if s.metrics == nil {
@@ -778,7 +812,9 @@ func (s *Span) AddLink(link SpanLink) {
 }
 
 // serializeSpanLinksInMeta saves span links as a JSON string under `Span[meta][_dd.span_links]`.
+// +checklocks:s.mu
 func (s *Span) serializeSpanLinksInMeta() {
+	assert.RWMutexLocked(&s.mu)
 	if len(s.spanLinks) == 0 {
 		return
 	}
@@ -795,7 +831,9 @@ func (s *Span) serializeSpanLinksInMeta() {
 
 // serializeSpanEvents sets the span events from the current span in the correct transport, depending on whether the
 // agent supports the native method or not.
+// +checklocks:s.mu
 func (s *Span) serializeSpanEvents() {
+	assert.RWMutexLocked(&s.mu)
 	if len(s.spanEvents) == 0 {
 		return
 	}
@@ -988,6 +1026,7 @@ func obfuscatedResource(o *obfuscate.Obfuscator, typ, resource string) string {
 
 // shouldKeep reports whether the trace should be kept.
 // a single span being kept implies the whole trace being kept.
+// +checklocksignore
 func shouldKeep(s *Span) bool {
 	if p, ok := s.context.SamplingPriority(); ok && p > 0 {
 		// positive sampling priorities stay
@@ -1005,6 +1044,7 @@ func shouldKeep(s *Span) bool {
 
 // shouldComputeStats mentions whether this span needs to have stats computed for.
 // Warning: callers must guard!
+// +checklocksignore
 func shouldComputeStats(s *Span) bool {
 	if v, ok := s.metrics[keyMeasured]; ok && v == 1 {
 		return true
@@ -1047,6 +1087,7 @@ func (s *Span) String() string {
 }
 
 // Format implements fmt.Formatter.
+// +checklocksignore
 func (s *Span) Format(f fmt.State, c rune) {
 	if s == nil {
 		fmt.Fprintf(f, "<nil>")