fix issue where errors exporting from the collector are not logged

GoogleCloudPlatform · Nov 1, 2023 · 63ebe18 · 63ebe18
1 parent 2f06d89
commit 63ebe18
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 5 deletions.
diff --git a/exporter/collector/integrationtest/inmemoryocexporter.go b/exporter/collector/integrationtest/inmemoryocexporter.go
@@ -136,9 +136,13 @@ func NewTraceTestExporter(
 	cfg.TraceConfig.ClientConfig.UseInsecure = true
 	cfg.ProjectID = "fakeprojectid"
 
+	logger, err := zap.NewDevelopment()
+	require.NoError(t, err)
+
 	exporter, err := collector.NewGoogleCloudTracesExporter(
 		ctx,
 		cfg,
+		logger,
 		"latest",
 		collector.DefaultTimeout,
 	)

diff --git a/exporter/collector/integrationtest/traces_integration_test.go b/exporter/collector/integrationtest/traces_integration_test.go
@@ -26,6 +26,7 @@ import (
 	"github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/collector"
 	"github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/collector/integrationtest/testcases"
 	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
 )
 
 func createTracesExporter(
@@ -35,9 +36,11 @@ func createTracesExporter(
 ) *collector.TraceExporter {
 	cfg := test.CreateTraceConfig()
 	cfg.ProjectID = os.Getenv("PROJECT_ID")
+	logger, _ := zap.NewProduction()
 	exporter, err := collector.NewGoogleCloudTracesExporter(
 		ctx,
 		cfg,
+		logger,
 		"latest",
 		collector.DefaultTimeout,
 	)

diff --git a/exporter/collector/logs.go b/exporter/collector/logs.go
@@ -196,6 +196,17 @@ func (l *LogsExporter) Shutdown(ctx context.Context) error {
 }
 
 func (l *LogsExporter) PushLogs(ctx context.Context, ld plog.Logs) error {
+	err := l.pushLogs(ctx, ld)
+	if err != nil {
+		l.obs.log.Error(
+			"Exporting logs failed",
+			zap.Error(err),
+		)
+	}
+	return err
+}
+
+func (l *LogsExporter) pushLogs(ctx context.Context, ld plog.Logs) error {
 	projectEntries, err := l.mapper.createEntries(ld)
 	if err != nil {
 		return err

diff --git a/exporter/collector/metrics.go b/exporter/collector/metrics.go
@@ -396,7 +396,14 @@ func (me *MetricsExporter) PushMetrics(ctx context.Context, m pmetric.Metrics) e
 			}
 		}
 	}
-	return errors.Join(errs...)
+	err := errors.Join(errs...)
+	if err != nil {
+		me.obs.log.Error(
+			"Exporting metrics failed",
+			zap.Error(err),
+		)
+	}
+	return err
 }
 
 // exportToTimeSeries is the default exporting call to GCM.

diff --git a/exporter/collector/traces.go b/exporter/collector/traces.go
@@ -27,20 +27,22 @@ import (
 	"go.opentelemetry.io/collector/pdata/ptrace"
 	"go.opentelemetry.io/otel/attribute"
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
+	"go.uber.org/zap"
 
 	texporter "github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/trace"
 )
 
 // TraceExporter is a wrapper struct of OT cloud trace exporter.
 type TraceExporter struct {
 	texporter *texporter.Exporter
+	logger    *zap.Logger
 }
 
 func (te *TraceExporter) Shutdown(ctx context.Context) error {
 	return te.texporter.Shutdown(ctx)
 }
 
-func NewGoogleCloudTracesExporter(ctx context.Context, cfg Config, version string, timeout time.Duration) (*TraceExporter, error) {
+func NewGoogleCloudTracesExporter(ctx context.Context, cfg Config, log *zap.Logger, version string, timeout time.Duration) (*TraceExporter, error) {
 	// TODO: https://github.com/GoogleCloudPlatform/opentelemetry-operations-go/pull/537#discussion_r1038290097
 	//nolint:errcheck
 	view.Register(ocgrpc.DefaultClientViews...)
@@ -70,7 +72,7 @@ func NewGoogleCloudTracesExporter(ctx context.Context, cfg Config, version strin
 		return nil, fmt.Errorf("error creating GoogleCloud Trace exporter: %w", err)
 	}
 
-	return &TraceExporter{texporter: exp}, nil
+	return &TraceExporter{texporter: exp, logger: log}, nil
 }
 
 func mappingFuncFromAKM(akm []AttributeMapping) func(attribute.Key) attribute.Key {
@@ -98,5 +100,12 @@ func (te *TraceExporter) PushTraces(ctx context.Context, td ptrace.Traces) error
 		spans = append(spans, sd...)
 	}
 
-	return te.texporter.ExportSpans(ctx, spans)
+	err := te.texporter.ExportSpans(ctx, spans)
+	if err != nil {
+		te.logger.Error(
+			"Exporting spans failed",
+			zap.Error(err),
+		)
+	}
+	return err
 }
diff --git a/exporter/collector/traces_test.go b/exporter/collector/traces_test.go
@@ -25,6 +25,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/pdata/ptrace"
+	"go.uber.org/zap"
 	"google.golang.org/grpc"
 	"google.golang.org/protobuf/types/known/emptypb"
 	"google.golang.org/protobuf/types/known/timestamppb"
@@ -100,7 +101,7 @@ func TestGoogleCloudTraceExport(t *testing.T) {
 
 			//nolint:errcheck
 			go srv.Serve(lis)
-			sde, err := NewGoogleCloudTracesExporter(ctx, test.cfg, "latest", DefaultTimeout)
+			sde, err := NewGoogleCloudTracesExporter(ctx, test.cfg, zap.NewNop(), "latest", DefaultTimeout)
 			if test.expectedErr != "" {
 				assert.EqualError(t, err, test.expectedErr)
 				return