diff --git a/.golangci.yml b/.golangci.yml index f66808babf7..b70fc28d33d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -58,7 +58,6 @@ linters: disabled-checks: - appendAssign - exitAfterDefer - - exposedSyncMutex - hugeParam - unnamedResult enable-all: true diff --git a/CHANGELOG.md b/CHANGELOG.md index 8de84b88cd9..28dff6702fd 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/instrumentation/github.com/aws/aws-lambda-go/otellambda/lambdatest_test.go b/instrumentation/github.com/aws/aws-lambda-go/otellambda/lambdatest_test.go index a865b5b3ca8..0ec1a1cdb14 100644 --- a/instrumentation/github.com/aws/aws-lambda-go/otellambda/lambdatest_test.go +++ b/instrumentation/github.com/aws/aws-lambda-go/otellambda/lambdatest_test.go @@ -37,22 +37,22 @@ const miB = 1 << 20 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)} } diff --git a/instrumentation/go.mongodb.org/mongo-driver/mongo/otelmongo/mongo.go b/instrumentation/go.mongodb.org/mongo-driver/mongo/otelmongo/mongo.go index 53e100df1fe..58c01204c3a 100644 --- a/instrumentation/go.mongodb.org/mongo-driver/mongo/otelmongo/mongo.go +++ b/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 } diff --git a/instrumentation/go.mongodb.org/mongo-driver/v2/mongo/otelmongo/mongo.go b/instrumentation/go.mongodb.org/mongo-driver/v2/mongo/otelmongo/mongo.go index e60df23f396..261338202f3 100644 --- a/instrumentation/go.mongodb.org/mongo-driver/v2/mongo/otelmongo/mongo.go +++ b/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 } diff --git a/propagators/aws/xray/idgenerator.go b/propagators/aws/xray/idgenerator.go index df439448b57..c75a662927d 100644 --- a/propagators/aws/xray/idgenerator.go +++ b/propagators/aws/xray/idgenerator.go @@ -19,16 +19,37 @@ 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. + mu sync.Mutex 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 +61,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() diff --git a/samplers/jaegerremote/sampler.go b/samplers/jaegerremote/sampler.go index 259f7a669a7..ed4503e2719 100644 --- a/samplers/jaegerremote/sampler.go +++ b/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 diff --git a/samplers/jaegerremote/sampler_remote.go b/samplers/jaegerremote/sampler_remote.go index 4cc1531cf53..aae8652646b 100644 --- a/samplers/jaegerremote/sampler_remote.go +++ b/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) diff --git a/zpages/spanprocessor.go b/zpages/spanprocessor.go index 9480bda85d9..7837bfb4c26 100644 --- a/zpages/spanprocessor.go +++ b/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,8 +183,8 @@ 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 } @@ -192,8 +192,8 @@ func (ss *sampleStore) spansByLatency(latencyBucketIndex int) []sdktrace.ReadOnl } func (ss *sampleStore) errorSpans() []sdktrace.ReadOnlySpan { - ss.Lock() - defer ss.Unlock() + ss.mu.Lock() + defer ss.mu.Unlock() return ss.errors.spans() } @@ -201,8 +201,8 @@ func (ss *sampleStore) errorSpans() []sdktrace.ReadOnlySpan { 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