Fix bug where the error logged when conversion of data fails is alway…

…s nil (#12510)

The problem was that in the logging statement (in all signals) we log
`err` instead of `cErr`. The good part is that this is not a UB, because
if we get to the point where that function is called, the `err` was nil
otherwise and never modified again so not race condition in accessing
it.

Separate the lambda function in a different function so we don't repeat
the same mistake of capturing other params by mistake.

Signed-off-by: Bogdan Drutu <[email protected]>

.chloggen/fix-wrong-error-log.yaml

@@ -0,0 +1,25 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: exporterhelper
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Fix bug where the error logged when conversion of data fails is always nil
+
+# One or more tracking issues or pull requests related to the change
+issues: [12510]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [user]

exporter/exporterhelper/logs.go

@@ -139,19 +139,23 @@ func NewLogsRequest(
 		return nil, err
 	}
 
-	lc, err := consumer.NewLogs(func(ctx context.Context, ld plog.Logs) error {
-		req, cErr := converter(ctx, ld)
-		if cErr != nil {
-			set.Logger.Error("Failed to convert logs. Dropping data.",
+	lc, err := consumer.NewLogs(newConsumeLogs(converter, be, set.Logger), be.ConsumerOptions...)
+	if err != nil {
+		return nil, err
+	}
+
+	return &logsExporter{BaseExporter: be, Logs: lc}, nil
+}
+
+func newConsumeLogs(converter RequestFromLogsFunc, be *internal.BaseExporter, logger *zap.Logger) consumer.ConsumeLogsFunc {
+	return func(ctx context.Context, ld plog.Logs) error {
+		req, err := converter(ctx, ld)
+		if err != nil {
+			logger.Error("Failed to convert metrics. Dropping data.",
 				zap.Int("dropped_log_records", ld.LogRecordCount()),
 				zap.Error(err))
-			return consumererror.NewPermanent(cErr)
+			return consumererror.NewPermanent(err)
 		}
 		return be.Send(ctx, req)
-	}, be.ConsumerOptions...)
-
-	return &logsExporter{
-		BaseExporter: be,
-		Logs:         lc,
-	}, err
+	}
 }

exporter/exporterhelper/metrics.go

@@ -131,19 +131,23 @@ func NewMetricsRequest(
 		return nil, err
 	}
 
-	mc, err := consumer.NewMetrics(func(ctx context.Context, md pmetric.Metrics) error {
-		req, cErr := converter(ctx, md)
-		if cErr != nil {
-			set.Logger.Error("Failed to convert metrics. Dropping data.",
+	mc, err := consumer.NewMetrics(newConsumeMetrics(converter, be, set.Logger), be.ConsumerOptions...)
+	if err != nil {
+		return nil, err
+	}
+
+	return &metricsExporter{BaseExporter: be, Metrics: mc}, nil
+}
+
+func newConsumeMetrics(converter RequestFromMetricsFunc, be *internal.BaseExporter, logger *zap.Logger) consumer.ConsumeMetricsFunc {
+	return func(ctx context.Context, md pmetric.Metrics) error {
+		req, err := converter(ctx, md)
+		if err != nil {
+			logger.Error("Failed to convert metrics. Dropping data.",
 				zap.Int("dropped_data_points", md.DataPointCount()),
 				zap.Error(err))
-			return consumererror.NewPermanent(cErr)
+			return consumererror.NewPermanent(err)
 		}
 		return be.Send(ctx, req)
-	}, be.ConsumerOptions...)
-
-	return &metricsExporter{
-		BaseExporter: be,
-		Metrics:      mc,
-	}, err
+	}
 }

exporter/exporterhelper/traces.go

@@ -131,19 +131,23 @@ func NewTracesRequest(
 		return nil, err
 	}
 
-	tc, err := consumer.NewTraces(func(ctx context.Context, td ptrace.Traces) error {
-		req, cErr := converter(ctx, td)
-		if cErr != nil {
-			set.Logger.Error("Failed to convert traces. Dropping data.",
+	tc, err := consumer.NewTraces(newConsumeTraces(converter, be, set.Logger), be.ConsumerOptions...)
+	if err != nil {
+		return nil, err
+	}
+
+	return &tracesExporter{BaseExporter: be, Traces: tc}, nil
+}
+
+func newConsumeTraces(converter RequestFromTracesFunc, be *internal.BaseExporter, logger *zap.Logger) consumer.ConsumeTracesFunc {
+	return func(ctx context.Context, td ptrace.Traces) error {
+		req, err := converter(ctx, td)
+		if err != nil {
+			logger.Error("Failed to convert metrics. Dropping data.",
 				zap.Int("dropped_spans", td.SpanCount()),
 				zap.Error(err))
-			return consumererror.NewPermanent(cErr)
+			return consumererror.NewPermanent(err)
 		}
 		return be.Send(ctx, req)
-	}, be.ConsumerOptions...)
-
-	return &tracesExporter{
-		BaseExporter: be,
-		Traces:       tc,
-	}, err
+	}
 }

exporter/exporterhelper/xexporterhelper/profiles.go

@@ -134,19 +134,23 @@ func NewProfilesRequestExporter(
 		return nil, err
 	}
 
-	tc, err := xconsumer.NewProfiles(func(ctx context.Context, pd pprofile.Profiles) error {
-		req, cErr := converter(ctx, pd)
-		if cErr != nil {
-			set.Logger.Error("Failed to convert profiles. Dropping data.",
+	tc, err := xconsumer.NewProfiles(newConsumeProfiles(converter, be, set.Logger), be.ConsumerOptions...)
+	if err != nil {
+		return nil, err
+	}
+
+	return &profileExporter{BaseExporter: be, Profiles: tc}, nil
+}
+
+func newConsumeProfiles(converter RequestFromProfilesFunc, be *internal.BaseExporter, logger *zap.Logger) xconsumer.ConsumeProfilesFunc {
+	return func(ctx context.Context, pd pprofile.Profiles) error {
+		req, err := converter(ctx, pd)
+		if err != nil {
+			logger.Error("Failed to convert metrics. Dropping data.",
 				zap.Int("dropped_samples", pd.SampleCount()),
 				zap.Error(err))
-			return consumererror.NewPermanent(cErr)
+			return consumererror.NewPermanent(err)
 		}
 		return be.Send(ctx, req)
-	}, be.ConsumerOptions...)
-
-	return &profileExporter{
-		BaseExporter: be,
-		Profiles:     tc,
-	}, err
+	}
 }