chore: enable exposedSyncMutex rule from go-critic

.golangci.yml

@@ -58,7 +58,6 @@ linters:
       disabled-checks:
         - appendAssign
         - exitAfterDefer
-        - exposedSyncMutex
         - hugeParam
         - unnamedResult
       enable-all: true

CHANGELOG.md

@@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ## [Unreleased]
 
+### Deprecated
+
+- Deprecate direct exposure and usage of mutex objects via structs: introduce equivalent methods for locking/unlocking, and mark mutex fields as deprecated, specially for `go.opentelemetry.io/contrib/samplers/jaegerremote.Sampler` and `go.opentelemetry.io/contrib/propagators/aws/xray.IDGenerator`. (#7726)
+
 ### Removed
 
 - Drop support for [Go 1.23]. (#7831)

instrumentation/github.com/aws/aws-lambda-go/otellambda/lambdatest_test.go

@@ -29,7 +29,7 @@
 
 	lambdadetector "go.opentelemetry.io/contrib/detectors/aws/lambda"
 	"go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-lambda-go/otellambda"
 	"go.opentelemetry.io/contrib/propagators/aws/xray"
 )
 
 const miB = 1 << 20
@@ -37,22 +37,22 @@
 var errorLogger = log.New(log.Writer(), "OTel Lambda Test Error: ", 0)
 
 type mockIDGenerator struct {
-	sync.Mutex
+	mu         sync.Mutex
 	traceCount int
 	spanCount  int
 }
 
 func (m *mockIDGenerator) NewIDs(context.Context) (trace.TraceID, trace.SpanID) {
-	m.Lock()
-	defer m.Unlock()
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	m.traceCount++
 	m.spanCount++
 	return [16]byte{byte(m.traceCount)}, [8]byte{byte(m.spanCount)}
 }
 
 func (m *mockIDGenerator) NewSpanID(context.Context, trace.TraceID) trace.SpanID {
-	m.Lock()
-	defer m.Unlock()
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	m.spanCount++
 	return [8]byte{byte(m.spanCount)}
 }

instrumentation/go.mongodb.org/mongo-driver/mongo/otelmongo/mongo.go

@@ -23,7 +23,7 @@ type spanKey struct {
 }
 
 type monitor struct {
-	sync.Mutex
+	mu      sync.Mutex
 	spans   map[spanKey]trace.Span
 	cfg     config
 	semconv semconv.EventMonitor
@@ -51,9 +51,9 @@ func (m *monitor) Started(ctx context.Context, evt *event.CommandStartedEvent) {
 		ConnectionID: evt.ConnectionID,
 		RequestID:    evt.RequestID,
 	}
-	m.Lock()
+	m.mu.Lock()
 	m.spans[key] = span
-	m.Unlock()
+	m.mu.Unlock()
 }
 
 func (m *monitor) Succeeded(_ context.Context, evt *event.CommandSucceededEvent) {
@@ -69,12 +69,12 @@ func (m *monitor) Finished(evt *event.CommandFinishedEvent, err error) {
 		ConnectionID: evt.ConnectionID,
 		RequestID:    evt.RequestID,
 	}
-	m.Lock()
+	m.mu.Lock()
 	span, ok := m.spans[key]
 	if ok {
 		delete(m.spans, key)
 	}
-	m.Unlock()
+	m.mu.Unlock()
 	if !ok {
 		return
 	}

instrumentation/go.mongodb.org/mongo-driver/v2/mongo/otelmongo/mongo.go

@@ -24,7 +24,7 @@ type spanKey struct {
 }
 
 type monitor struct {
-	sync.Mutex
+	mu    sync.Mutex
 	spans map[spanKey]trace.Span
 	cfg   config
 }
@@ -59,9 +59,9 @@ func (m *monitor) Started(ctx context.Context, evt *event.CommandStartedEvent) {
 		ConnectionID: evt.ConnectionID,
 		RequestID:    evt.RequestID,
 	}
-	m.Lock()
+	m.mu.Lock()
 	m.spans[key] = span
-	m.Unlock()
+	m.mu.Unlock()
 }
 
 func (m *monitor) Succeeded(_ context.Context, evt *event.CommandSucceededEvent) {
@@ -77,12 +77,12 @@ func (m *monitor) Finished(evt *event.CommandFinishedEvent, err error) {
 		ConnectionID: evt.ConnectionID,
 		RequestID:    evt.RequestID,
 	}
-	m.Lock()
+	m.mu.Lock()
 	span, ok := m.spans[key]
 	if ok {
 		delete(m.spans, key)
 	}
-	m.Unlock()
+	m.mu.Unlock()
 	if !ok {
 		return
 	}

propagators/aws/xray/idgenerator.go

@@ -19,16 +19,36 @@ import (
 
 // IDGenerator is used for generating a new traceID and spanID.
 type IDGenerator struct {
-	sync.Mutex
+	sync.Mutex // Deprecated: mutex is internal and shall not be used.
 	randSource *rand.Rand
 }
 
 var _ sdktrace.IDGenerator = &IDGenerator{}
 
+// Lock is used to lock the IDGenerator for concurrent use.
+// noop method to satisfy the sync.Mutex interface.
+//
+// Deprecated: mutex is internal and shall not be used.
+func (*IDGenerator) Lock() {}
+
+// TryLock attempts to lock the IDGenerator for concurrent use.
+// noop method to satisfy the sync.Mutex interface.
+//
+// Deprecated: mutex is internal and shall not be used.
+func (*IDGenerator) TryLock() bool {
+	return false
+}
+
+// Unlock is used to unlock the IDGenerator for concurrent use.
+// noop method to satisfy the sync.Mutex interface.
+//
+// Deprecated: mutex is internal and shall not be used.
+func (*IDGenerator) Unlock() {}
+
 // NewSpanID returns a non-zero span ID from a randomly-chosen sequence.
 func (gen *IDGenerator) NewSpanID(context.Context, trace.TraceID) trace.SpanID {
-	gen.Lock()
-	defer gen.Unlock()
+	gen.mu.Lock()
+	defer gen.mu.Unlock()
 	sid := trace.SpanID{}
 	_, _ = gen.randSource.Read(sid[:])
 	return sid
@@ -40,8 +60,8 @@ func (gen *IDGenerator) NewSpanID(context.Context, trace.TraceID) trace.SpanID {
 //
 // span ID is from a randomly-chosen sequence.
 func (gen *IDGenerator) NewIDs(context.Context) (trace.TraceID, trace.SpanID) {
-	gen.Lock()
-	defer gen.Unlock()
+	gen.mu.Lock()
+	defer gen.mu.Unlock()
 
 	tid := trace.TraceID{}
 	currentTime := getCurrentTimeHex()

samplers/jaegerremote/sampler.go

@@ -209,8 +209,7 @@ func (*guaranteedThroughputProbabilisticSampler) Description() string {
 // perOperationSampler is a delegating sampler that applies guaranteedThroughputProbabilisticSampler
 // on a per-operation basis.
 type perOperationSampler struct {
-	sync.RWMutex
-
+	mu             sync.RWMutex
 	samplers       map[string]*guaranteedThroughputProbabilisticSampler
 	defaultSampler *probabilisticSampler
 	lowerBound     float64
@@ -265,15 +264,15 @@ func (s *perOperationSampler) ShouldSample(p trace.SamplingParameters) trace.Sam
 }
 
 func (s *perOperationSampler) getSamplerForOperation(operation string) trace.Sampler {
-	s.RLock()
+	s.mu.RLock()
 	sampler, ok := s.samplers[operation]
 	if ok {
-		defer s.RUnlock()
+		defer s.mu.RUnlock()
 		return sampler
 	}
-	s.RUnlock()
-	s.Lock()
-	defer s.Unlock()
+	s.mu.RUnlock()
+	s.mu.Lock()
+	defer s.mu.Unlock()
 
 	// Check if sampler has already been created
 	sampler, ok = s.samplers[operation]
@@ -294,8 +293,8 @@ func (*perOperationSampler) Description() string {
 }
 
 func (s *perOperationSampler) update(strategies *jaeger_api_v2.PerOperationSamplingStrategies) {
-	s.Lock()
-	defer s.Unlock()
+	s.mu.Lock()
+	defer s.mu.Unlock()
 	newSamplers := map[string]*guaranteedThroughputProbabilisticSampler{}
 	for _, strategy := range strategies.PerOperationStrategies {
 		operation := strategy.Operation

samplers/jaegerremote/sampler_remote.go

@@ -71,8 +71,9 @@ type Sampler struct {
 	// These fields must be first in the struct because `sync/atomic` expects 64-bit alignment.
 	// Cf. https://github.com/jaegertracing/jaeger-client-go/issues/155, https://pkg.go.dev/sync/atomic#pkg-note-BUG
 	closed int64 // 0 - not closed, 1 - closed
-
-	sync.RWMutex // used to serialize access to samplerConfig.sampler
+	//nolint:gocritic // ongoing deletion
+	sync.RWMutex // Deprecated: mutex is internal and shall not be used.
+	mu sync.RWMutex // used to serialize access to samplerConfig.sampler
 	config
 
 	serviceName string
@@ -95,11 +96,31 @@ func New(
 	return sampler
 }
 
+// Lock is used to lock the Sampler for concurrent use.
+// noop method to satisfy the sync.RWMutex interface.
+//
+// Deprecated: mutex is internal and shall not be used.
+func (*Sampler) Lock() {}
+
+// TryLock attempts to lock the Sampler for concurrent use.
+// noop method to satisfy the sync.RWMutex interface.
+//
+// Deprecated: mutex is internal and shall not be used.
+func (*Sampler) TryLock() bool {
+	return false
+}
+
+// Unlock is used to unlock the Sampler for concurrent use.
+// noop method to satisfy the sync.RWMutex interface.
+//
+// Deprecated: mutex is internal and shall not be used.
+func (*Sampler) Unlock() {}
+
 // ShouldSample returns a sampling choice based on the passed sampling
 // parameters.
 func (s *Sampler) ShouldSample(p trace.SamplingParameters) trace.SamplingResult {
-	s.RLock()
-	defer s.RUnlock()
+	s.mu.RLock()
+	defer s.mu.RUnlock()
 	return s.sampler.ShouldSample(p)
 }
 
@@ -143,8 +164,8 @@ func (s *Sampler) pollControllerWithTicker(ticker *time.Ticker) {
 }
 
 func (s *Sampler) setSampler(sampler trace.Sampler) {
-	s.Lock()
-	defer s.Unlock()
+	s.mu.Lock()
+	defer s.mu.Unlock()
 	s.sampler = sampler
 }
 
@@ -162,8 +183,8 @@ func (s *Sampler) UpdateSampler() {
 		return
 	}
 
-	s.Lock()
-	defer s.Unlock()
+	s.mu.Lock()
+	defer s.mu.Unlock()
 
 	if err := s.updateSamplerViaUpdaters(strategy); err != nil {
 		s.logger.Error(err, "failed to handle sampling strategy response", "response", res)

zpages/spanprocessor.go

@@ -154,9 +154,9 @@ func (ssm *SpanProcessor) spansByLatency(name string, latencyBucketIndex int) []
 // It contains sample of spans for error requests (status code is codes.Error);
 // and a sample of spans for successful requests, bucketed by latency.
 type sampleStore struct {
-	sync.Mutex // protects everything below.
-	latency    []*bucket
-	errors     *bucket
+	mu      sync.Mutex // protects everything below.
+	latency []*bucket
+	errors  *bucket
 }
 
 // newSampleStore creates a sampleStore.
@@ -172,8 +172,8 @@ func newSampleStore(latencyBucketSize, errorBucketSize uint) *sampleStore {
 }
 
 func (ss *sampleStore) perMethodSummary() *perMethodSummary {
-	ss.Lock()
-	defer ss.Unlock()
+	ss.mu.Lock()
+	defer ss.mu.Unlock()
 	p := &perMethodSummary{}
 	p.errorSpans = ss.errors.len()
 	for _, b := range ss.latency {
@@ -183,26 +183,26 @@ func (ss *sampleStore) perMethodSummary() *perMethodSummary {
 }
 
 func (ss *sampleStore) spansByLatency(latencyBucketIndex int) []sdktrace.ReadOnlySpan {
-	ss.Lock()
-	defer ss.Unlock()
+	ss.mu.Lock()
+	defer ss.mu.Unlock()
 	if latencyBucketIndex < 0 || latencyBucketIndex >= len(ss.latency) {
 		return nil
 	}
 	return ss.latency[latencyBucketIndex].spans()
 }
 
 func (ss *sampleStore) errorSpans() []sdktrace.ReadOnlySpan {
-	ss.Lock()
-	defer ss.Unlock()
+	ss.mu.Lock()
+	defer ss.mu.Unlock()
 	return ss.errors.spans()
 }
 
 // sampleSpan removes adds to the corresponding latency or error bucket.
 func (ss *sampleStore) sampleSpan(span sdktrace.ReadOnlySpan) {
 	code := span.Status().Code
 
-	ss.Lock()
-	defer ss.Unlock()
+	ss.mu.Lock()
+	defer ss.mu.Unlock()
 	if code == codes.Error {
 		ss.errors.add(span)
 		return