Skip to content

Commit

Permalink
[chore] Upgrade golangci and fix lint issues (#36913)
Browse files Browse the repository at this point in the history
For the moment, disabling the G115 rule, but will create an issue to ask
for help to enable it.

Signed-off-by: Bogdan Drutu <[email protected]>
  • Loading branch information
bogdandrutu authored Dec 20, 2024
1 parent 856062d commit c413eeb
Show file tree
Hide file tree
Showing 57 changed files with 181 additions and 264 deletions.
5 changes: 4 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,11 @@ linters-settings:

linters:
enable:
- copyloopvar
- decorder
- depguard
- errcheck
- errorlint
- exportloopref
- exhaustive
- gci
- gocritic
Expand Down Expand Up @@ -183,3 +183,6 @@ issues:
- text: "G402:"
linters:
- gosec
- text: "G115:"
linters:
- gosec
2 changes: 1 addition & 1 deletion Makefile.Common
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ fmt: $(GOFUMPT) $(GOIMPORTS)

.PHONY: lint
lint: $(LINT) checklicense misspell
$(LINT) run --allow-parallel-runners --verbose --build-tags integration --timeout=30m --path-prefix $(shell basename "$(CURDIR)")
$(LINT) run --allow-parallel-runners --build-tags integration --timeout=30m --path-prefix $(shell basename "$(CURDIR)")

.PHONY: govulncheck
govulncheck: $(GOVULNCHECK)
Expand Down
3 changes: 0 additions & 3 deletions connector/exceptionsconnector/connector_metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@ func TestConnectorConsumeTraces(t *testing.T) {
}

for _, tc := range testcases {
// Since parallelism is enabled in these tests, to avoid flaky behavior,
// instantiate a copy of the test case for t.Run's closure to use.
tc := tc
t.Run(tc.name, func(t *testing.T) {
msink := &consumertest.MetricsSink{}

Expand Down
4 changes: 0 additions & 4 deletions connector/spanmetricsconnector/connector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -877,9 +877,6 @@ func TestConsumeTraces(t *testing.T) {
}

for _, tc := range testcases {
// Since parallelism is enabled in these tests, to avoid flaky behavior,
// instantiate a copy of the test case for t.Run's closure to use.
tc := tc
t.Run(tc.name, func(t *testing.T) {
// Prepare

Expand Down Expand Up @@ -1070,7 +1067,6 @@ func TestExcludeDimensionsConsumeTraces(t *testing.T) {

excludeDimensions := []string{"span.kind", "span.name", "totallyWrongNameDoesNotAffectAnything"}
for _, tc := range testcases {
tc := tc
t.Run(tc.dsc, func(t *testing.T) {
// Set feature gate value
previousValue := legacyMetricNamesFeatureGate.IsEnabled()
Expand Down
4 changes: 0 additions & 4 deletions connector/spanmetricsconnector/internal/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ func TestNewCache(t *testing.T) {
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
_, err := NewCache[string, string](tt.args.size)
Expand Down Expand Up @@ -122,7 +121,6 @@ func TestCache_Get(t *testing.T) {
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
c := tt.lruCache()
Expand Down Expand Up @@ -162,7 +160,6 @@ func TestCache_RemoveEvictedItems(t *testing.T) {
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
cache, err := tt.lruCache()
Expand Down Expand Up @@ -212,7 +209,6 @@ func TestCache_PurgeItems(t *testing.T) {
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
cache, err := tt.lruCache()
Expand Down
1 change: 0 additions & 1 deletion exporter/awskinesisexporter/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ func TestCreatingExporter(t *testing.T) {
},
},
} {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ func TestBatchedExporter(t *testing.T) {
}

for _, tc := range cases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
be, err := producer.NewBatcher(
SetPutRecordsOperation(tc.PutRecordsOP),
Expand Down
3 changes: 1 addition & 2 deletions exporter/azuredataexplorerexporter/adx_exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,7 @@ func TestCreateKcsb(t *testing.T) {
isAzureAuth: true,
},
}
for i := range tests {
tt := tests[i]
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
wantAppID := tt.applicationID
gotKcsb := createKcsb(&tt.config, "1.0.0")
Expand Down
1 change: 0 additions & 1 deletion exporter/fileexporter/buffered_writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ func BenchmarkWriter(b *testing.B) {
"raw-file": tempfile(b),
"buffered-file": newBufferedWriteCloser(tempfile(b)),
} {
w := w
b.Run(fmt.Sprintf("%s_%d_bytes", name, payloadSize), func(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
Expand Down
3 changes: 0 additions & 3 deletions exporter/kafkaexporter/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ func TestCreateMetricExporter(t *testing.T) {
}

for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -138,7 +137,6 @@ func TestCreateLogExporter(t *testing.T) {
}

for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -203,7 +201,6 @@ func TestCreateTraceExporter(t *testing.T) {
}

for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

Expand Down
9 changes: 0 additions & 9 deletions exporter/loadbalancingexporter/metrics_exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,9 +287,6 @@ func TestSplitMetrics(t *testing.T) {
}

for _, tc := range testCases {
// Purposely make a copy since we're running in a goroutine due to t.Parallel()
tc := tc

t.Run(tc.name, func(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -334,9 +331,6 @@ func TestConsumeMetrics_SingleEndpoint(t *testing.T) {
}

for _, tc := range testCases {
// Purposely make a copy since we're running in a goroutine due to t.Parallel()
tc := tc

t.Run(tc.name, func(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -443,9 +437,6 @@ func TestConsumeMetrics_TripleEndpoint(t *testing.T) {
}

for _, tc := range testCases {
// Purposely make a copy since we're running in a goroutine due to t.Parallel()
tc := tc

t.Run(tc.name, func(t *testing.T) {
t.Parallel()

Expand Down
3 changes: 1 addition & 2 deletions exporter/prometheusremotewriteexporter/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -684,8 +684,7 @@ func Test_PushMetrics(t *testing.T) {
if useWAL {
t.Skip("Flaky test, see https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/9124")
}
for _, ttt := range tests {
tt := ttt
for _, tt := range tests {
if useWAL && tt.skipForWAL {
t.Skip("test not supported when using WAL")
}
Expand Down
1 change: 0 additions & 1 deletion exporter/sapmexporter/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,6 @@ func TestCompression(t *testing.T) {
},
}
for _, tt := range tests {
tt := tt
t.Run(
tt.name, func(t *testing.T) {
tracesReceived := false
Expand Down
1 change: 0 additions & 1 deletion exporter/sentryexporter/sentry_exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,6 @@ func TestSpanEventToSentryEvent(t *testing.T) {
}

for _, test := range testCases {
test := test
t.Run(test.testName, func(t *testing.T) {
sentryEvent, err := sentryEventFromError(test.errorMessage, test.errorType, test.sampleSentrySpan)
if sentryEvent != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,6 @@ func TestCorrelationClient(t *testing.T) {

for _, correlationType := range []Type{Service, Environment} {
for _, op := range []string{http.MethodPut, http.MethodDelete} {
op := op
correlationType := correlationType
t.Run(fmt.Sprintf("%v %v", op, correlationType), func(t *testing.T) {
testData := &Correlation{Type: correlationType, DimName: "host", DimValue: "test-box", Value: "test-service"}
switch op {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ type ActiveServiceTracker struct {
func (a *ActiveServiceTracker) LoadHostIDDimCorrelations() {
// asynchronously fetch all services and environments for each hostIDDim at startup
for dimName, dimValue := range a.hostIDDims {
dimName := dimName
dimValue := dimValue
a.correlationClient.Get(dimName, dimValue, func(correlations map[string][]string) {
if services, ok := correlations["sf_services"]; ok {
for _, service := range services {
Expand Down Expand Up @@ -179,8 +177,6 @@ func (a *ActiveServiceTracker) processEnvironment(res pcommon.Resource, now time
// container / pod level stuff
// this cache is necessary to identify environments associated with a kubernetes pod or container id
for sourceAttr, dimName := range a.dimsToSyncSource {
sourceAttr := sourceAttr
dimName := dimName
if val, ok := attrs.Get(sourceAttr); ok {
// Note that the value is not set on the cache key. We only send the first environment received for a
// given pod/container, and we never delete the values set on the container/pod dimension.
Expand Down Expand Up @@ -239,8 +235,6 @@ func (a *ActiveServiceTracker) processService(res pcommon.Resource, now time.Tim
// container / pod level stuff (this should not directly affect the active service count)
// this cache is necessary to identify services associated with a kubernetes pod or container id
for sourceAttr, dimName := range a.dimsToSyncSource {
sourceAttr := sourceAttr
dimName := dimName
if val, ok := res.Attributes().Get(sourceAttr); ok {
// Note that the value is not set on the cache key. We only send the first service received for a
// given pod/container, and we never delete the values set on the container/pod dimension.
Expand Down
2 changes: 0 additions & 2 deletions exporter/sumologicexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ func TestInitExporterInvalidConfiguration(t *testing.T) {
}

for _, tc := range testcases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
err := component.ValidateConfig(tc.cfg)

Expand Down Expand Up @@ -108,7 +107,6 @@ func TestConfigInvalidTimeout(t *testing.T) {
}

for _, tc := range testcases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
err := tc.cfg.Validate()

Expand Down
3 changes: 0 additions & 3 deletions extension/ackextension/inmemory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ func TestExtensionAckAsync(t *testing.T) {
wg.Add(partitionCount)
// send events through different partitions
for i := 0; i < partitionCount; i++ {
i := i
go func() {
// each partition has 3 events
for j := 0; j < 3; j++ {
Expand All @@ -242,7 +241,6 @@ func TestExtensionAckAsync(t *testing.T) {
wg.Add(partitionCount)
// ack the second event of all even partitions and first and third events of all odd partitions
for i := 0; i < partitionCount; i++ {
i := i
go func() {
if i%2 == 0 {
ext.Ack(fmt.Sprintf("part-%d", i), 2)
Expand Down Expand Up @@ -275,7 +273,6 @@ func TestExtensionAckAsync(t *testing.T) {
resultChan := make(chan map[uint64]bool, partitionCount)
// querying the same acked events should result in false
for i := 0; i < partitionCount; i++ {
i := i
go func() {
resultChan <- ext.QueryAcks(fmt.Sprintf("part-%d", i), []uint64{1, 2, 3})
wg.Done()
Expand Down
2 changes: 0 additions & 2 deletions extension/healthcheckv2extension/internal/grpc/grpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1579,8 +1579,6 @@ func TestWatch(t *testing.T) {
wg.Add(len(watchers))

for svc, watcher := range watchers {
svc := svc
watcher := watcher
go func() {
resp, err := watcher.Recv()
// Ensure there are not any unread messages
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,10 @@ func Test_serviceStrategyCache_Concurrency(t *testing.T) {
wg := sync.WaitGroup{}
wg.Add(numThreads)
for i := 0; i < numThreads; i++ {
ii := i
go func() {
for j := 0; j < numIterationsPerThread; j++ {
for _, svcName := range []string{
fmt.Sprintf("thread-specific-service-%d", ii),
fmt.Sprintf("thread-specific-service-%d", i),
"contended-for-service",
} {
if _, ok := cache.get(context.Background(), svcName); !ok {
Expand Down
1 change: 0 additions & 1 deletion extension/storage/filestorage/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,6 @@ func TestClientConcurrentCompaction(t *testing.T) {
}

for i := 0; i < 10; i++ {
i := i
t.Run(fmt.Sprintf("client-operations-thread-%d", i), func(t *testing.T) {
t.Parallel()
clientOperationsThread(t, i)
Expand Down
2 changes: 0 additions & 2 deletions extension/sumologicextension/extension_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -988,8 +988,6 @@ func TestCollectorCheckingCredentialsFoundInLocalStorage(t *testing.T) {

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
tc := tc

srv, reqCount := tc.srvFn()
t.Cleanup(func() { srv.Close() })

Expand Down
Loading

0 comments on commit c413eeb

Please sign in to comment.