fix errcheck issue for aws related exporter (#9785)

* fix errcheck issue for aws related exporter

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>

* Update exporter/awsxrayexporter/internal/translator/writer_pool_test.go

Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>

* Update exporter/awsxrayexporter/awsxray_test.go

Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>

* fix comments and gocritic error

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>

* fix for reviews

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>

* Apply suggestions from code review

Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>

* fix failed unit test

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>

* fix unit test failed

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>

* fix unit test failed

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>

Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>

exporter/awscloudwatchlogsexporter/exporter.go

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// nolint:errcheck
 package awscloudwatchlogsexporter // import "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/awscloudwatchlogsexporter"
 
 import (
@@ -66,8 +65,6 @@ func newCwLogsPusher(expConfig *Config, params component.ExporterCreateSettings)
 		return nil, err
 	}
 
-	expConfig.Validate()
-
 	pusher := cwlogs.NewPusher(aws.String(expConfig.LogGroupName), aws.String(expConfig.LogStreamName), *awsConfig.MaxRetries, *svcStructuredLog, params.Logger)
 
 	logsExporter := &exporter{

exporter/awscloudwatchlogsexporter/exporter_test.go

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// nolint:errcheck
 package awscloudwatchlogsexporter
 
 import (
@@ -110,7 +109,11 @@ func BenchmarkLogToCWLog(b *testing.B) {
 	resource := testResource()
 	log := testLogRecord()
 	for i := 0; i < b.N; i++ {
-		logToCWLog(attrsValue(resource.Attributes()), log)
+		_, err := logToCWLog(attrsValue(resource.Attributes()), log)
+		if err != nil {
+			b.Errorf("logToCWLog() failed %v", err)
+			return
+		}
 	}
 }
 

exporter/awsemfexporter/emf_exporter.go

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// nolint:errcheck,gocritic
 package awsemfexporter // import "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/awsemfexporter"
 
 import (
@@ -46,7 +45,7 @@ const (
 )
 
 type emfExporter struct {
-	//Each (log group, log stream) keeps a separate pusher because of each (log group, log stream) requires separate stream token.
+	// Each (log group, log stream) keeps a separate pusher because of each (log group, log stream) requires separate stream token.
 	groupStreamToPusherMap map[string]map[string]cwlogs.Pusher
 	svcStructuredLog       *cwlogs.Client
 	config                 config.Exporter
@@ -82,8 +81,6 @@ func newEmfPusher(
 	svcStructuredLog := cwlogs.NewClient(logger, awsConfig, params.BuildInfo, expConfig.LogGroupName, session)
 	collectorIdentifier, _ := uuid.NewRandom()
 
-	expConfig.Validate()
-
 	emfExporter := &emfExporter{
 		svcStructuredLog: svcStructuredLog,
 		config:           config,
@@ -174,7 +171,7 @@ func (emf *emfExporter) pushMetricsData(_ context.Context, md pmetric.Metrics) e
 		for _, emfPusher := range emf.listPushers() {
 			returnError := emfPusher.ForceFlush()
 			if returnError != nil {
-				//TODO now we only have one logPusher, so it's ok to return after first error occurred
+				// TODO now we only have one logPusher, so it's ok to return after first error occurred
 				err := wrapErrorIfBadRequest(returnError)
 				if err != nil {
 					emf.logger.Error("Error force flushing logs. Skipping to next logPusher.", zap.Error(err))

exporter/awsemfexporter/emf_exporter_test.go

@@ -588,6 +588,8 @@ func TestNewExporterWithMetricDeclarations(t *testing.T) {
 	exp, err := newEmfPusher(expCfg, params)
 	assert.Nil(t, err)
 	assert.NotNil(t, exp)
+	err = expCfg.Validate()
+	assert.Nil(t, err)
 
 	emfExporter := exp.(*emfExporter)
 	config := emfExporter.config.(*Config)

exporter/awsemfexporter/grouped_metric_test.go

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// nolint:errcheck,gocritic
 package awsemfexporter
 
 import (
@@ -156,7 +155,8 @@ func TestAddToGroupedMetric(t *testing.T) {
 
 			for i := 0; i < metrics.Len(); i++ {
 				metric := metrics.At(i)
-				addToGroupedMetric(&metric, groupedMetrics, metadata, true, zap.NewNop(), nil, nil)
+				err := addToGroupedMetric(&metric, groupedMetrics, metadata, true, zap.NewNop(), nil, nil)
+				assert.Nil(t, err)
 			}
 
 			expectedLabels := map[string]string{
@@ -204,7 +204,8 @@ func TestAddToGroupedMetric(t *testing.T) {
 
 		for i := 0; i < metrics.Len(); i++ {
 			metric := metrics.At(i)
-			addToGroupedMetric(&metric, groupedMetrics, metadata, true, logger, nil, nil)
+			err := addToGroupedMetric(&metric, groupedMetrics, metadata, true, logger, nil, nil)
+			assert.Nil(t, err)
 		}
 
 		assert.Equal(t, 1, len(groupedMetrics))
@@ -267,24 +268,27 @@ func TestAddToGroupedMetric(t *testing.T) {
 
 		for i := 0; i < metrics.Len(); i++ {
 			metric := metrics.At(i)
-			addToGroupedMetric(&metric, groupedMetrics, metadata, true, logger, nil, nil)
+			err := addToGroupedMetric(&metric, groupedMetrics, metadata, true, logger, nil, nil)
+			assert.Nil(t, err)
 		}
 
 		assert.Equal(t, 3, len(groupedMetrics))
 		for _, group := range groupedMetrics {
 			for metricName := range group.metrics {
-				if metricName == "int-gauge" || metricName == "int-sum" {
+				switch metricName {
+				case "int-gauge", "int-sum":
 					assert.Equal(t, 2, len(group.metrics))
 					assert.Equal(t, int64(1608068109347), group.metadata.timestampMs)
-				} else if metricName == "summary" {
+				case "summary":
 					assert.Equal(t, 1, len(group.metrics))
 					assert.Equal(t, int64(1608068110347), group.metadata.timestampMs)
-				} else {
+				default:
 					// double-gauge should use the default timestamp
 					assert.Equal(t, 1, len(group.metrics))
 					assert.Equal(t, "double-gauge", metricName)
 					assert.Equal(t, timestamp, group.metadata.timestampMs)
 				}
+
 			}
 			expectedLabels := map[string]string{
 				oTellibDimensionKey: "cloudwatch-otel",
@@ -314,7 +318,8 @@ func TestAddToGroupedMetric(t *testing.T) {
 			},
 			instrumentationLibraryName: instrumentationLibName,
 		}
-		addToGroupedMetric(&metric, groupedMetrics, metricMetadata1, true, logger, nil, nil)
+		err := addToGroupedMetric(&metric, groupedMetrics, metricMetadata1, true, logger, nil, nil)
+		assert.Nil(t, err)
 
 		metricMetadata2 := cWMetricMetadata{
 			groupedMetricMetadata: groupedMetricMetadata{
@@ -325,7 +330,8 @@ func TestAddToGroupedMetric(t *testing.T) {
 			},
 			instrumentationLibraryName: instrumentationLibName,
 		}
-		addToGroupedMetric(&metric, groupedMetrics, metricMetadata2, true, logger, nil, nil)
+		err = addToGroupedMetric(&metric, groupedMetrics, metricMetadata2, true, logger, nil, nil)
+		assert.Nil(t, err)
 
 		assert.Equal(t, 2, len(groupedMetrics))
 		seenLogGroup1 := false
@@ -380,7 +386,8 @@ func TestAddToGroupedMetric(t *testing.T) {
 
 		for i := 0; i < metrics.Len(); i++ {
 			metric := metrics.At(i)
-			addToGroupedMetric(&metric, groupedMetrics, metadata, true, obsLogger, nil, nil)
+			err := addToGroupedMetric(&metric, groupedMetrics, metadata, true, obsLogger, nil, nil)
+			assert.Nil(t, err)
 		}
 		assert.Equal(t, 1, len(groupedMetrics))
 
@@ -413,7 +420,8 @@ func TestAddToGroupedMetric(t *testing.T) {
 
 		obs, logs := observer.New(zap.WarnLevel)
 		obsLogger := zap.New(obs)
-		addToGroupedMetric(&metric, groupedMetrics, metadata, true, obsLogger, nil, nil)
+		err := addToGroupedMetric(&metric, groupedMetrics, metadata, true, obsLogger, nil, nil)
+		assert.Nil(t, err)
 		assert.Equal(t, 0, len(groupedMetrics))
 
 		// Test output warning logs
@@ -433,7 +441,8 @@ func TestAddToGroupedMetric(t *testing.T) {
 
 	t.Run("Nil metric", func(t *testing.T) {
 		groupedMetrics := make(map[interface{}]*groupedMetric)
-		addToGroupedMetric(nil, groupedMetrics, metadata, true, logger, nil, nil)
+		err := addToGroupedMetric(nil, groupedMetrics, metadata, true, logger, nil, nil)
+		assert.Nil(t, err)
 		assert.Equal(t, 0, len(groupedMetrics))
 	})
 
@@ -502,7 +511,8 @@ func BenchmarkAddToGroupedMetric(b *testing.B) {
 		groupedMetrics := make(map[interface{}]*groupedMetric)
 		for i := 0; i < numMetrics; i++ {
 			metric := metrics.At(i)
-			addToGroupedMetric(&metric, groupedMetrics, metadata, true, logger, nil, nil)
+			err := addToGroupedMetric(&metric, groupedMetrics, metadata, true, logger, nil, nil)
+			assert.Nil(b, err)
 		}
 	}
 }

exporter/awsemfexporter/metric_declaration_test.go

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// nolint:errcheck
 package awsemfexporter
 
 import (
@@ -115,7 +114,8 @@ func TestGetConcatenatedLabels(t *testing.T) {
 			Separator:  tc.separator,
 			Regex:      ".+",
 		}
-		lm.init()
+		err := lm.init()
+		assert.Nil(t, err)
 		t.Run(tc.testName, func(t *testing.T) {
 			concatenatedLabels := lm.getConcatenatedLabels(labels)
 			assert.Equal(t, tc.expected, concatenatedLabels)
@@ -220,7 +220,8 @@ func TestLabelMatcherMatches(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		tc.labelMatcher.init()
+		err := tc.labelMatcher.init()
+		assert.Nil(t, err)
 		t.Run(tc.testName, func(t *testing.T) {
 			matches := tc.labelMatcher.Matches(tc.labels)
 			assert.Equal(t, tc.expected, matches)

exporter/awsemfexporter/metric_translator_test.go

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// nolint:errcheck,gocritic
 package awsemfexporter
 
 import (
@@ -497,7 +496,8 @@ func TestTranslateOtToGroupedMetric(t *testing.T) {
 			setupDataPointCache()
 
 			groupedMetrics := make(map[interface{}]*groupedMetric)
-			translator.translateOTelToGroupedMetric(tc.metric, groupedMetrics, config)
+			err := translator.translateOTelToGroupedMetric(tc.metric, groupedMetrics, config)
+			assert.Nil(t, err)
 			assert.NotNil(t, groupedMetrics)
 			assert.Equal(t, 2, len(groupedMetrics))
 
@@ -529,7 +529,8 @@ func TestTranslateOtToGroupedMetric(t *testing.T) {
 		}
 		rm := internaldata.OCToMetrics(oc.Node, oc.Resource, oc.Metrics).ResourceMetrics().At(0)
 		groupedMetrics := make(map[interface{}]*groupedMetric)
-		translator.translateOTelToGroupedMetric(&rm, groupedMetrics, config)
+		err := translator.translateOTelToGroupedMetric(&rm, groupedMetrics, config)
+		assert.Nil(t, err)
 		assert.Equal(t, 0, len(groupedMetrics))
 	})
 }
@@ -548,12 +549,12 @@ func TestTranslateCWMetricToEMF(t *testing.T) {
 	fields[oTellibDimensionKey] = "cloudwatch-otel"
 	fields["spanName"] = "test"
 	fields["spanCounter"] = 0
-	//add stringified json as attribute values
+	// add stringified json as attribute values
 	fields["kubernetes"] = "{\"container_name\":\"cloudwatch-agent\",\"docker\":{\"container_id\":\"fc1b0a4c3faaa1808e187486a3a90cbea883dccaf2e2c46d4069d663b032a1ca\"},\"host\":\"ip-192-168-58-245.ec2.internal\",\"labels\":{\"controller-revision-hash\":\"5bdbf497dc\",\"name\":\"cloudwatch-agent\",\"pod-template-generation\":\"1\"},\"namespace_name\":\"amazon-cloudwatch\",\"pod_id\":\"e23f3413-af2e-4a98-89e0-5df2251e7f05\",\"pod_name\":\"cloudwatch-agent-26bl6\",\"pod_owners\":[{\"owner_kind\":\"DaemonSet\",\"owner_name\":\"cloudwatch-agent\"}]}"
 	fields["Sources"] = "[\"cadvisor\",\"pod\",\"calculated\"]"
 
 	config := &Config{
-		//include valid json string, a non-existing key, and keys whose value are not json/string
+		// include valid json string, a non-existing key, and keys whose value are not json/string
 		ParseJSONEncodedAttributeValues: []string{"kubernetes", "Sources", "NonExistingAttributeKey", "spanName", "spanCounter"},
 		logger:                          zap.NewNop(),
 	}
@@ -883,7 +884,8 @@ func TestTranslateGroupedMetricToCWMetric(t *testing.T) {
 				logger:                logger,
 			}
 			for _, decl := range tc.metricDeclarations {
-				decl.init(logger)
+				err := decl.init(logger)
+				assert.Nil(t, err)
 			}
 			cWMetric := translateGroupedMetricToCWMetric(tc.groupedMetric, config)
 			assert.NotNil(t, cWMetric)
@@ -2070,7 +2072,8 @@ func BenchmarkTranslateOtToGroupedMetricWithInstrLibrary(b *testing.B) {
 	b.ResetTimer()
 	for n := 0; n < b.N; n++ {
 		groupedMetric := make(map[interface{}]*groupedMetric)
-		translator.translateOTelToGroupedMetric(&rm, groupedMetric, config)
+		err := translator.translateOTelToGroupedMetric(&rm, groupedMetric, config)
+		assert.Nil(b, err)
 	}
 }
 
@@ -2092,7 +2095,8 @@ func BenchmarkTranslateOtToGroupedMetricWithoutConfigReplacePattern(b *testing.B
 	b.ResetTimer()
 	for n := 0; n < b.N; n++ {
 		groupedMetrics := make(map[interface{}]*groupedMetric)
-		translator.translateOTelToGroupedMetric(&rm, groupedMetrics, config)
+		err := translator.translateOTelToGroupedMetric(&rm, groupedMetrics, config)
+		assert.Nil(b, err)
 	}
 }
 
@@ -2114,7 +2118,8 @@ func BenchmarkTranslateOtToGroupedMetricWithConfigReplaceWithResource(b *testing
 	b.ResetTimer()
 	for n := 0; n < b.N; n++ {
 		groupedMetrics := make(map[interface{}]*groupedMetric)
-		translator.translateOTelToGroupedMetric(&rm, groupedMetrics, config)
+		err := translator.translateOTelToGroupedMetric(&rm, groupedMetrics, config)
+		assert.Nil(b, err)
 	}
 }
 
@@ -2136,7 +2141,8 @@ func BenchmarkTranslateOtToGroupedMetricWithConfigReplaceWithLabel(b *testing.B)
 	b.ResetTimer()
 	for n := 0; n < b.N; n++ {
 		groupedMetrics := make(map[interface{}]*groupedMetric)
-		translator.translateOTelToGroupedMetric(&rm, groupedMetrics, config)
+		err := translator.translateOTelToGroupedMetric(&rm, groupedMetrics, config)
+		assert.Nil(b, err)
 	}
 }
 
@@ -2153,7 +2159,8 @@ func BenchmarkTranslateOtToGroupedMetricWithoutInstrLibrary(b *testing.B) {
 	b.ResetTimer()
 	for n := 0; n < b.N; n++ {
 		groupedMetrics := make(map[interface{}]*groupedMetric)
-		translator.translateOTelToGroupedMetric(&rm, groupedMetrics, config)
+		err := translator.translateOTelToGroupedMetric(&rm, groupedMetrics, config)
+		assert.Nil(b, err)
 	}
 }
 
@@ -2221,7 +2228,8 @@ func BenchmarkTranslateGroupedMetricToCWMetricWithFiltering(b *testing.B) {
 		MetricNameSelectors: []string{"metric1", "metric2"},
 	}
 	logger := zap.NewNop()
-	m.init(logger)
+	err := m.init(logger)
+	assert.Nil(b, err)
 	config := &Config{
 		MetricDeclarations:    []*MetricDeclaration{m},
 		DimensionRollupOption: zeroAndSingleDimensionRollup,
@@ -2410,7 +2418,8 @@ func TestTranslateOtToGroupedMetricForLogGroupAndStream(t *testing.T) {
 			groupedMetrics := make(map[interface{}]*groupedMetric)
 
 			rm := test.inputMetrics.ResourceMetrics().At(0)
-			translator.translateOTelToGroupedMetric(&rm, groupedMetrics, config)
+			err := translator.translateOTelToGroupedMetric(&rm, groupedMetrics, config)
+			assert.Nil(t, err)
 
 			assert.NotNil(t, groupedMetrics)
 			assert.Equal(t, 1, len(groupedMetrics))

exporter/awsxrayexporter/awsxray_test.go

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// nolint:errcheck
 package awsxrayexporter
 
 import (
@@ -81,7 +80,8 @@ func BenchmarkForTracesExporter(b *testing.B) {
 		ctx := context.Background()
 		td := constructSpanData()
 		b.StartTimer()
-		traceExporter.ConsumeTraces(ctx, td)
+		err := traceExporter.ConsumeTraces(ctx, td)
+		assert.Error(b, err)
 	}
 }