chore: enable exposedSyncMutex rule from go-critic

[mmorel-35]

Description

Enable and fixes more rules from go-critic:

• https://go-critic.com/overview.html#exposedsyncmutex

[codecov]

Codecov Report

❌ Patch coverage is 80.48780% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.4%. Comparing base (72e8b77) to head (2cf8caa).

Files with missing lines	Patch %	Lines
propagators/aws/xray/idgenerator.go	50.0%	4 Missing ⚠️
samplers/jaegerremote/sampler_remote.go	60.0%	4 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #7726     +/-   ##
=======================================
- Coverage   78.4%   78.4%   -0.1%     
=======================================
  Files        184     184             
  Lines      14648   14656      +8     
=======================================
  Hits       11496   11496             
- Misses      2801    2809      +8     
  Partials     351     351             

Files with missing lines	Coverage Δ
....mongodb.org/mongo-driver/mongo/otelmongo/mongo.go	90.0% <100.0%> (ø)
...ngodb.org/mongo-driver/v2/mongo/otelmongo/mongo.go	88.5% <100.0%> (ø)
samplers/jaegerremote/sampler.go	91.6% <100.0%> (ø)
zpages/spanprocessor.go	100.0% <100.0%> (ø)
propagators/aws/xray/idgenerator.go	88.5% <50.0%> (-11.5%)	⬇️
samplers/jaegerremote/sampler_remote.go	87.1% <60.0%> (-2.3%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[flc1125]

Overall, it's very good, with an experimental version section (for reference only), pending decision.

[pellared]

Can you please add deperecation entries in the changelog?

stable modules should have the methods implemented as no-op

[pellared]

Please notice https://github.com/open-telemetry/opentelemetry-go/blob/main/CONTRIBUTING.md#how-to-send-pull-requests:

Avoid rebasing and force-pushing to your branch to facilitate reviewing the pull request. Rewriting Git history makes it difficult to keep track of iterations during code review. All pull requests are squashed to a single commit upon merge to main.

[pellared]

The field needs to be kept as removing an exported field is a breaking change.
The field should be marked as deprecated.

[mmorel-35]

Aren't the lign added between L28 and L47 here to handle deprecation ?

[pellared]

https://github.com/open-telemetry/opentelemetry-go-contrib/pull/7726/files#r2334121098

[pellared]

The field needs to be kept as removing an exported field is a breaking change.
The field should be marked as deprecated.

[mmorel-35]

Aren't the lign added between L98 and L117 here to handle deprecation ?

[pellared]

This only handles deprecation of the "embedded methods".
But after making this field not embedded this would no longer compile:

package main

import "go.opentelemetry.io/contrib/samplers/jaegerremote"

func main() {
	s := new(jaegerremote.Sampler)
	s.RWMutex.Lock()
}

Thus this is a breaking change from Go perspective.
More: open-telemetry/opentelemetry-go#6274

[pellared]

Suggested change

	sync.RWMutex // Deprecated: mutex is internal and shall not be used.
	mu sync.RWMutex // used to serialize access to samplerConfig.sampler
	sync.RWMutex // Deprecated: mutex is internal and shall not be used.

[pellared]

Is there any reason for duplicating the fields?

Suggested change

	sync.Mutex // Deprecated: mutex is internal and shall not be used.
	mu sync.Mutex
	sync.Mutex // Deprecated: mutex is internal and shall not be used.

[mmorel-35]

I just wanted to fix exposedSyncMutex... this is a nightmare 😭. Please take it from here. I don’t want to here about this egg or chicken problem!

https://go-critic.com/overview.html#exposedsyncmutex

[pellared]

Cannot you add a //nolint that this is required for backwards compatibility?