Enable errcheck linter (#4462)

* Check or explicitly ignore all errors * Enable errcheck * Use `t.Cleanup` instead of `defer` * Point to issue on `Set` ignored error
open-telemetry · Nov 23, 2021 · a8ff2dd · a8ff2dd
1 parent 7059878
commit a8ff2dd
Show file tree

Hide file tree

Showing 17 changed files with 68 additions and 56 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -96,9 +96,8 @@ linters-settings:
       - kilometres
 
 linters:
-  disable:
-    - errcheck
   enable:
+    - errcheck
     - exportloopref
     - gocritic
     - gofmt
@@ -129,7 +128,7 @@ issues:
   # The list of ids of default excludes to include or disable. By default it's empty.
   # See the list of default excludes here https://golangci-lint.run/usage/configuration.
   include:
-    - EXC0001
+    # - EXC0001 - errcheck checks that are not usually checked
     - EXC0002
     - EXC0003
     - EXC0004

diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go
@@ -40,7 +40,7 @@ import (
 func TestDefaultGrpcClientSettings(t *testing.T) {
 	tt, err := obsreporttest.SetupTelemetry()
 	require.NoError(t, err)
-	defer tt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
 
 	gcs := &GRPCClientSettings{
 		TLSSetting: configtls.TLSClientSetting{
@@ -55,7 +55,7 @@ func TestDefaultGrpcClientSettings(t *testing.T) {
 func TestAllGrpcClientSettings(t *testing.T) {
 	tt, err := obsreporttest.SetupTelemetry()
 	require.NoError(t, err)
-	defer tt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
 
 	gcs := &GRPCClientSettings{
 		Headers: map[string]string{
@@ -160,7 +160,7 @@ func TestGrpcServerAuthSettings(t *testing.T) {
 func TestGRPCClientSettingsError(t *testing.T) {
 	tt, err := obsreporttest.SetupTelemetry()
 	require.NoError(t, err)
-	defer tt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
 
 	tests := []struct {
 		settings GRPCClientSettings
@@ -251,7 +251,7 @@ func TestGRPCClientSettingsError(t *testing.T) {
 func TestUseSecure(t *testing.T) {
 	tt, err := obsreporttest.SetupTelemetry()
 	require.NoError(t, err)
-	defer tt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
 
 	gcs := &GRPCClientSettings{
 		Headers:     nil,
@@ -371,7 +371,7 @@ func TestGetGRPCCompressionKey(t *testing.T) {
 func TestHttpReception(t *testing.T) {
 	tt, err := obsreporttest.SetupTelemetry()
 	require.NoError(t, err)
-	defer tt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
 
 	tests := []struct {
 		name           string
@@ -526,7 +526,7 @@ func TestReceiveOnUnixDomainSocket(t *testing.T) {
 	}
 	tt, err := obsreporttest.SetupTelemetry()
 	require.NoError(t, err)
-	defer tt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
 
 	socketName := tempSocketName(t)
 	gss := &GRPCServerSettings{

diff --git a/config/configmap.go b/config/configmap.go
@@ -116,7 +116,8 @@ func (l *Map) Set(key string, value interface{}) {
 	// koanf doesn't offer a direct setting mechanism so merging is required.
 	merged := koanf.New(KeyDelimiter)
 	_ = merged.Load(confmap.Provider(map[string]interface{}{key: value}, KeyDelimiter), nil)
-	l.k.Merge(merged)
+	// TODO (issue 4467): return this error on `Set`.
+	_ = l.k.Merge(merged)
 }
 
 // IsSet checks to see if the key has been set in any of the data locations.

diff --git a/exporter/exporterhelper/internal/persistent_queue_test.go b/exporter/exporterhelper/internal/persistent_queue_test.go
@@ -52,7 +52,7 @@ func TestPersistentQueue_Capacity(t *testing.T) {
 
 	for i := 0; i < 100; i++ {
 		ext := createStorageExtension(path)
-		defer ext.Shutdown(context.Background())
+		t.Cleanup(func() { require.NoError(t, ext.Shutdown(context.Background())) })
 
 		wq := createTestQueue(ext, 5)
 		require.Equal(t, 0, wq.Size())
@@ -85,7 +85,7 @@ func TestPersistentQueue_Close(t *testing.T) {
 	defer os.RemoveAll(path)
 
 	ext := createStorageExtension(path)
-	defer ext.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, ext.Shutdown(context.Background())) })
 
 	wq := createTestQueue(ext, 1001)
 	traces := newTraces(1, 10)
@@ -147,7 +147,7 @@ func TestPersistentQueue_ConsumersProducers(t *testing.T) {
 
 			defer os.RemoveAll(path)
 			defer tq.Stop()
-			defer ext.Shutdown(context.Background())
+			t.Cleanup(func() { require.NoError(t, ext.Shutdown(context.Background())) })
 
 			numMessagesConsumed := int32(0)
 			tq.StartConsumers(c.numConsumers, func(item interface{}) {

diff --git a/exporter/exporterhelper/internal/persistent_storage_test.go b/exporter/exporterhelper/internal/persistent_storage_test.go
@@ -216,7 +216,7 @@ func TestPersistentStorage_RepeatPutCloseReadClose(t *testing.T) {
 	ext := createStorageExtension(path)
 	wq := createTestQueue(ext, 5000)
 	require.Equal(t, 0, wq.Size())
-	ext.Shutdown(context.Background())
+	require.NoError(t, ext.Shutdown(context.Background()))
 }
 
 func TestPersistentStorage_EmptyRequest(t *testing.T) {
@@ -279,7 +279,7 @@ func BenchmarkPersistentStorage_TraceSpans(b *testing.B) {
 				req := ps.get()
 				require.NotNil(bb, req)
 			}
-			ext.Shutdown(context.Background())
+			require.NoError(b, ext.Shutdown(context.Background()))
 		})
 	}
 }

diff --git a/exporter/exporterhelper/logs_test.go b/exporter/exporterhelper/logs_test.go
@@ -127,7 +127,7 @@ func TestLogsExporter_WithRecordLogs_ReturnError(t *testing.T) {
 func TestLogsExporter_WithRecordEnqueueFailedMetrics(t *testing.T) {
 	tt, err := obsreporttest.SetupTelemetry()
 	require.NoError(t, err)
-	defer tt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
 
 	rCfg := DefaultRetrySettings()
 	qCfg := DefaultQueueSettings()
@@ -141,7 +141,8 @@ func TestLogsExporter_WithRecordEnqueueFailedMetrics(t *testing.T) {
 	md := testdata.GenerateLogsTwoLogRecordsSameResourceOneDifferent()
 	const numBatches = 7
 	for i := 0; i < numBatches; i++ {
-		te.ConsumeLogs(context.Background(), md)
+		// errors are checked in the checkExporterEnqueueFailedLogsStats function below.
+		_ = te.ConsumeLogs(context.Background(), md)
 	}
 
 	// 2 batched must be in queue, and 5 batches (15 log records) rejected due to queue overflow
@@ -207,7 +208,7 @@ func newPushLogsData(retError error) consumerhelper.ConsumeLogsFunc {
 func checkRecordedMetricsForLogsExporter(t *testing.T, le component.LogsExporter, wantError error) {
 	tt, err := obsreporttest.SetupTelemetry()
 	require.NoError(t, err)
-	defer tt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
 
 	ld := testdata.GenerateLogsTwoLogRecordsSameResource()
 	const numBatches = 7

diff --git a/exporter/exporterhelper/metrics_test.go b/exporter/exporterhelper/metrics_test.go
@@ -126,7 +126,7 @@ func TestMetricsExporter_WithRecordMetrics_ReturnError(t *testing.T) {
 func TestMetricsExporter_WithRecordEnqueueFailedMetrics(t *testing.T) {
 	tt, err := obsreporttest.SetupTelemetry()
 	require.NoError(t, err)
-	defer tt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
 
 	rCfg := DefaultRetrySettings()
 	qCfg := DefaultQueueSettings()
@@ -140,7 +140,8 @@ func TestMetricsExporter_WithRecordEnqueueFailedMetrics(t *testing.T) {
 	md := testdata.GenerateMetricsOneMetric()
 	const numBatches = 7
 	for i := 0; i < numBatches; i++ {
-		te.ConsumeMetrics(context.Background(), md)
+		// errors are checked in the checkExporterEnqueueFailedMetricsStats function below.
+		_ = te.ConsumeMetrics(context.Background(), md)
 	}
 
 	// 2 batched must be in queue, and 10 metric points rejected due to queue overflow
@@ -208,7 +209,7 @@ func newPushMetricsData(retError error) consumerhelper.ConsumeMetricsFunc {
 func checkRecordedMetricsForMetricsExporter(t *testing.T, me component.MetricsExporter, wantError error) {
 	tt, err := obsreporttest.SetupTelemetry()
 	require.NoError(t, err)
-	defer tt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
 
 	md := testdata.GenerateMetricsTwoMetrics()
 	const numBatches = 7

diff --git a/exporter/exporterhelper/obsreport_test.go b/exporter/exporterhelper/obsreport_test.go
@@ -31,7 +31,7 @@ import (
 func TestExportEnqueueFailure(t *testing.T) {
 	tt, err := obsreporttest.SetupTelemetry()
 	require.NoError(t, err)
-	defer tt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
 
 	exporter := config.NewComponentID("fakeExporter")
 

diff --git a/exporter/exporterhelper/queued_retry_test.go b/exporter/exporterhelper/queued_retry_test.go
@@ -302,7 +302,7 @@ func TestQueuedRetry_DropOnFull(t *testing.T) {
 func TestQueuedRetryHappyPath(t *testing.T) {
 	tt, err := obsreporttest.SetupTelemetry()
 	require.NoError(t, err)
-	defer tt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
 
 	qCfg := DefaultQueueSettings()
 	rCfg := DefaultRetrySettings()

diff --git a/exporter/exporterhelper/traces_test.go b/exporter/exporterhelper/traces_test.go
@@ -124,7 +124,7 @@ func TestTracesExporter_WithRecordMetrics_ReturnError(t *testing.T) {
 func TestTracesExporter_WithRecordEnqueueFailedMetrics(t *testing.T) {
 	tt, err := obsreporttest.SetupTelemetry()
 	require.NoError(t, err)
-	defer tt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
 
 	rCfg := DefaultRetrySettings()
 	qCfg := DefaultQueueSettings()
@@ -138,7 +138,8 @@ func TestTracesExporter_WithRecordEnqueueFailedMetrics(t *testing.T) {
 	td := testdata.GenerateTracesTwoSpansSameResource()
 	const numBatches = 7
 	for i := 0; i < numBatches; i++ {
-		te.ConsumeTraces(context.Background(), td)
+		// errors are checked in the checkExporterEnqueueFailedTracesStats function below.
+		_ = te.ConsumeTraces(context.Background(), td)
 	}
 
 	// 2 batched must be in queue, and 5 batches (10 spans) rejected due to queue overflow
@@ -208,7 +209,7 @@ func newTraceDataPusher(retError error) consumerhelper.ConsumeTracesFunc {
 func checkRecordedMetricsForTracesExporter(t *testing.T, te component.TracesExporter, wantError error) {
 	tt, err := obsreporttest.SetupTelemetry()
 	require.NoError(t, err)
-	defer tt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
 
 	td := testdata.GenerateTracesTwoSpansSameResource()
 	const numBatches = 7

diff --git a/obsreport/obsreport_processor.go b/obsreport/obsreport_processor.go
@@ -65,7 +65,8 @@ func NewProcessor(cfg ProcessorSettings) *Processor {
 // TracesAccepted reports that the trace data was accepted.
 func (por *Processor) TracesAccepted(ctx context.Context, numSpans int) {
 	if por.level != configtelemetry.LevelNone {
-		stats.RecordWithTags(
+		// ignore the error for now; should not happen
+		_ = stats.RecordWithTags(
 			ctx,
 			por.mutators,
 			obsmetrics.ProcessorAcceptedSpans.M(int64(numSpans)),
@@ -78,7 +79,8 @@ func (por *Processor) TracesAccepted(ctx context.Context, numSpans int) {
 // TracesRefused reports that the trace data was refused.
 func (por *Processor) TracesRefused(ctx context.Context, numSpans int) {
 	if por.level != configtelemetry.LevelNone {
-		stats.RecordWithTags(
+		// ignore the error for now; should not happen
+		_ = stats.RecordWithTags(
 			ctx,
 			por.mutators,
 			obsmetrics.ProcessorAcceptedSpans.M(0),
@@ -91,7 +93,8 @@ func (por *Processor) TracesRefused(ctx context.Context, numSpans int) {
 // TracesDropped reports that the trace data was dropped.
 func (por *Processor) TracesDropped(ctx context.Context, numSpans int) {
 	if por.level != configtelemetry.LevelNone {
-		stats.RecordWithTags(
+		// ignore the error for now; should not happen
+		_ = stats.RecordWithTags(
 			ctx,
 			por.mutators,
 			obsmetrics.ProcessorAcceptedSpans.M(0),
@@ -104,7 +107,8 @@ func (por *Processor) TracesDropped(ctx context.Context, numSpans int) {
 // MetricsAccepted reports that the metrics were accepted.
 func (por *Processor) MetricsAccepted(ctx context.Context, numPoints int) {
 	if por.level != configtelemetry.LevelNone {
-		stats.RecordWithTags(
+		// ignore the error for now; should not happen
+		_ = stats.RecordWithTags(
 			ctx,
 			por.mutators,
 			obsmetrics.ProcessorAcceptedMetricPoints.M(int64(numPoints)),
@@ -117,7 +121,8 @@ func (por *Processor) MetricsAccepted(ctx context.Context, numPoints int) {
 // MetricsRefused reports that the metrics were refused.
 func (por *Processor) MetricsRefused(ctx context.Context, numPoints int) {
 	if por.level != configtelemetry.LevelNone {
-		stats.RecordWithTags(
+		// ignore the error for now; should not happen
+		_ = stats.RecordWithTags(
 			ctx,
 			por.mutators,
 			obsmetrics.ProcessorAcceptedMetricPoints.M(0),
@@ -130,7 +135,8 @@ func (por *Processor) MetricsRefused(ctx context.Context, numPoints int) {
 // MetricsDropped reports that the metrics were dropped.
 func (por *Processor) MetricsDropped(ctx context.Context, numPoints int) {
 	if por.level != configtelemetry.LevelNone {
-		stats.RecordWithTags(
+		// ignore the error for now; should not happen
+		_ = stats.RecordWithTags(
 			ctx,
 			por.mutators,
 			obsmetrics.ProcessorAcceptedMetricPoints.M(0),
@@ -143,7 +149,8 @@ func (por *Processor) MetricsDropped(ctx context.Context, numPoints int) {
 // LogsAccepted reports that the logs were accepted.
 func (por *Processor) LogsAccepted(ctx context.Context, numRecords int) {
 	if por.level != configtelemetry.LevelNone {
-		stats.RecordWithTags(
+		// ignore the error for now; should not happen
+		_ = stats.RecordWithTags(
 			ctx,
 			por.mutators,
 			obsmetrics.ProcessorAcceptedLogRecords.M(int64(numRecords)),
@@ -156,7 +163,8 @@ func (por *Processor) LogsAccepted(ctx context.Context, numRecords int) {
 // LogsRefused reports that the logs were refused.
 func (por *Processor) LogsRefused(ctx context.Context, numRecords int) {
 	if por.level != configtelemetry.LevelNone {
-		stats.RecordWithTags(
+		// ignore the error for now; should not happen
+		_ = stats.RecordWithTags(
 			ctx,
 			por.mutators,
 			obsmetrics.ProcessorAcceptedLogRecords.M(0),
@@ -169,7 +177,8 @@ func (por *Processor) LogsRefused(ctx context.Context, numRecords int) {
 // LogsDropped reports that the logs were dropped.
 func (por *Processor) LogsDropped(ctx context.Context, numRecords int) {
 	if por.level != configtelemetry.LevelNone {
-		stats.RecordWithTags(
+		// ignore the error for now; should not happen
+		_ = stats.RecordWithTags(
 			ctx,
 			por.mutators,
 			obsmetrics.ProcessorAcceptedLogRecords.M(0),