Enable errcheck linter (open-telemetry#4462)

* Check or explicitly ignore all errors

* Enable errcheck

* Use `t.Cleanup` instead of `defer`

* Point to issue on `Set` ignored error

Author: mx-psi

.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

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{

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.

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{}) {

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()))
 		})
 	}
 }

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

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

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")
 

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()

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