Change NewSplitDriver paramater and initialization (#1798)

* Change NewSplitDriver paramater and initialization * Update CHANGELOG.md Co-authored-by: Anthony Mirabella <[email protected]> * Update CHANGELOG.md Co-authored-by: Anthony Mirabella <[email protected]> * Update exporters/otlp/protocoldriver.go Co-authored-by: Anthony Mirabella <[email protected]> * Update exporters/otlp/protocoldriver.go Co-authored-by: Anthony Mirabella <[email protected]> * Update exporters/otlp/protocoldriver.go Co-authored-by: Anthony Mirabella <[email protected]> * Update exporters/otlp/protocoldriver.go Co-authored-by: Anthony Mirabella <[email protected]> * Move splitdriver option into options.go and rename * Update CHANGELOG.md Co-authored-by: Anthony Mirabella <[email protected]> * Change option name and nil test to snapshots * Update exporters/otlp/protocoldriver.go Co-authored-by: Tyler Yahn <[email protected]> * Update exporters/otlp/protocoldriver.go Co-authored-by: Tyler Yahn <[email protected]> * Update exporters/otlp/protocoldriver.go Co-authored-by: Tyler Yahn <[email protected]> * Update exporters/otlp/protocoldriver.go Co-authored-by: Tyler Yahn <[email protected]> * Update exporters/otlp/options.go Co-authored-by: Tyler Yahn <[email protected]> * Update exporters/otlp/options.go Co-authored-by: Tyler Yahn <[email protected]> * Update exporters/otlp/options.go Co-authored-by: Tyler Yahn <[email protected]> * Update exporters/otlp/options.go Co-authored-by: Tyler Yahn <[email protected]> * Change SplitDriverOption to match spec * Update changelog entry Co-authored-by: Anthony Mirabella <[email protected]> Co-authored-by: Tyler Yahn <[email protected]>
plantfansam · Apr 23, 2021 · 0f4e454 · 0f4e454
1 parent 02d8bdd
commit 0f4e454
Show file tree

Hide file tree

Showing 7 changed files with 178 additions and 61 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ### Changed
 
+- Make `NewSplitDriver` from `go.opentelemetry.io/otel/exporters/otlp` take variadic arguments instead of a `SplitConfig` item. 
+  `NewSplitDriver` now automically implements an internal `noopDriver` for `SplitConfig` fields that are not initialized. (#1798)
+
 ### Deprecated
 
 ### Removed

diff --git a/exporters/otlp/example_test.go b/exporters/otlp/example_test.go
@@ -35,11 +35,7 @@ func ExampleNewExporter() {
 	tracesDriver := otlpgrpc.NewDriver(
 	// Configure traces driver here
 	)
-	config := otlp.SplitConfig{
-		ForMetrics: metricsDriver,
-		ForTraces:  tracesDriver,
-	}
-	driver := otlp.NewSplitDriver(config)
+	driver := otlp.NewSplitDriver(otlp.WithMetricDriver(metricsDriver), otlp.WithTraceDriver(tracesDriver))
 	exporter, err := otlp.NewExporter(ctx, driver) // Configure as needed.
 	if err != nil {
 		log.Fatalf("failed to create exporter: %v", err)

diff --git a/exporters/otlp/options.go b/exporters/otlp/options.go
@@ -43,3 +43,36 @@ func WithMetricExportKindSelector(selector metricsdk.ExportKindSelector) Exporte
 		cfg.exportKindSelector = selector
 	}
 }
+
+// SplitDriverOption provides options for setting up a split driver.
+type SplitDriverOption interface {
+	Apply(*splitDriver)
+}
+
+// WithMetricDriver allows one to set the driver used for metrics
+// in a SplitDriver.
+func WithMetricDriver(dr ProtocolDriver) SplitDriverOption {
+	return metricDriverOption{dr}
+}
+
+type metricDriverOption struct {
+	driver ProtocolDriver
+}
+
+func (o metricDriverOption) Apply(s *splitDriver) {
+	s.metric = o.driver
+}
+
+// WithTraceDriver allows one to set the driver used for traces
+// in a SplitDriver.
+func WithTraceDriver(dr ProtocolDriver) SplitDriverOption {
+	return traceDriverOption{dr}
+}
+
+type traceDriverOption struct {
+	driver ProtocolDriver
+}
+
+func (o traceDriverOption) Apply(s *splitDriver) {
+	s.trace = o.driver
+}
diff --git a/exporters/otlp/otlp_test.go b/exporters/otlp/otlp_test.go
@@ -283,46 +283,112 @@ func TestNewExportPipeline(t *testing.T) {
 }
 
 func TestSplitDriver(t *testing.T) {
-	driverTraces := &stubProtocolDriver{}
-	driverMetrics := &stubProtocolDriver{}
-	config := otlp.SplitConfig{
-		ForMetrics: driverMetrics,
-		ForTraces:  driverTraces,
-	}
-	driver := otlp.NewSplitDriver(config)
-	ctx := context.Background()
-	assert.NoError(t, driver.Start(ctx))
-	assert.Equal(t, 1, driverTraces.started)
-	assert.Equal(t, 1, driverMetrics.started)
-	assert.Equal(t, 0, driverTraces.stopped)
-	assert.Equal(t, 0, driverMetrics.stopped)
-	assert.Equal(t, 0, driverTraces.tracesExported)
-	assert.Equal(t, 0, driverTraces.metricsExported)
-	assert.Equal(t, 0, driverMetrics.tracesExported)
-	assert.Equal(t, 0, driverMetrics.metricsExported)
 
 	recordCount := 5
 	spanCount := 7
-	assert.NoError(t, driver.ExportMetrics(ctx, stubCheckpointSet{recordCount}, metricsdk.StatelessExportKindSelector()))
-	assert.NoError(t, driver.ExportTraces(ctx, stubSpanSnapshot(spanCount)))
-	assert.Len(t, driverTraces.rm, 0)
-	assert.Len(t, driverTraces.rs, spanCount)
-	assert.Len(t, driverMetrics.rm, recordCount)
-	assert.Len(t, driverMetrics.rs, 0)
-	assert.Equal(t, 1, driverTraces.tracesExported)
-	assert.Equal(t, 0, driverTraces.metricsExported)
-	assert.Equal(t, 0, driverMetrics.tracesExported)
-	assert.Equal(t, 1, driverMetrics.metricsExported)
-
-	assert.NoError(t, driver.Stop(ctx))
-	assert.Equal(t, 1, driverTraces.started)
-	assert.Equal(t, 1, driverMetrics.started)
-	assert.Equal(t, 1, driverTraces.stopped)
-	assert.Equal(t, 1, driverMetrics.stopped)
-	assert.Equal(t, 1, driverTraces.tracesExported)
-	assert.Equal(t, 0, driverTraces.metricsExported)
-	assert.Equal(t, 0, driverMetrics.tracesExported)
-	assert.Equal(t, 1, driverMetrics.metricsExported)
+	assertExport := func(t testing.TB, ctx context.Context, driver otlp.ProtocolDriver) {
+		t.Helper()
+		assert.NoError(t, driver.ExportMetrics(ctx, stubCheckpointSet{recordCount}, metricsdk.StatelessExportKindSelector()))
+		assert.NoError(t, driver.ExportTraces(ctx, stubSpanSnapshot(spanCount)))
+	}
+
+	t.Run("with metric/trace drivers configured", func(t *testing.T) {
+		driverTraces := &stubProtocolDriver{}
+		driverMetrics := &stubProtocolDriver{}
+
+		driver := otlp.NewSplitDriver(otlp.WithMetricDriver(driverMetrics), otlp.WithTraceDriver(driverTraces))
+		ctx := context.Background()
+		assert.NoError(t, driver.Start(ctx))
+		assert.Equal(t, 1, driverTraces.started)
+		assert.Equal(t, 1, driverMetrics.started)
+		assert.Equal(t, 0, driverTraces.stopped)
+		assert.Equal(t, 0, driverMetrics.stopped)
+		assert.Equal(t, 0, driverTraces.tracesExported)
+		assert.Equal(t, 0, driverTraces.metricsExported)
+		assert.Equal(t, 0, driverMetrics.tracesExported)
+		assert.Equal(t, 0, driverMetrics.metricsExported)
+
+		assertExport(t, ctx, driver)
+		assert.Len(t, driverTraces.rm, 0)
+		assert.Len(t, driverTraces.rs, spanCount)
+		assert.Len(t, driverMetrics.rm, recordCount)
+		assert.Len(t, driverMetrics.rs, 0)
+		assert.Equal(t, 1, driverTraces.tracesExported)
+		assert.Equal(t, 0, driverTraces.metricsExported)
+		assert.Equal(t, 0, driverMetrics.tracesExported)
+		assert.Equal(t, 1, driverMetrics.metricsExported)
+
+		assert.NoError(t, driver.Stop(ctx))
+		assert.Equal(t, 1, driverTraces.started)
+		assert.Equal(t, 1, driverMetrics.started)
+		assert.Equal(t, 1, driverTraces.stopped)
+		assert.Equal(t, 1, driverMetrics.stopped)
+		assert.Equal(t, 1, driverTraces.tracesExported)
+		assert.Equal(t, 0, driverTraces.metricsExported)
+		assert.Equal(t, 0, driverMetrics.tracesExported)
+		assert.Equal(t, 1, driverMetrics.metricsExported)
+	})
+
+	t.Run("with just metric driver", func(t *testing.T) {
+		driverMetrics := &stubProtocolDriver{}
+
+		driver := otlp.NewSplitDriver(otlp.WithMetricDriver(driverMetrics))
+		ctx := context.Background()
+		assert.NoError(t, driver.Start(ctx))
+
+		assert.Equal(t, 1, driverMetrics.started)
+		assert.Equal(t, 0, driverMetrics.stopped)
+		assert.Equal(t, 0, driverMetrics.tracesExported)
+		assert.Equal(t, 0, driverMetrics.metricsExported)
+
+		assertExport(t, ctx, driver)
+		assert.Len(t, driverMetrics.rm, recordCount)
+		assert.Len(t, driverMetrics.rs, 0)
+		assert.Equal(t, 0, driverMetrics.tracesExported)
+		assert.Equal(t, 1, driverMetrics.metricsExported)
+
+		assert.NoError(t, driver.Stop(ctx))
+		assert.Equal(t, 1, driverMetrics.started)
+		assert.Equal(t, 1, driverMetrics.stopped)
+		assert.Equal(t, 0, driverMetrics.tracesExported)
+		assert.Equal(t, 1, driverMetrics.metricsExported)
+	})
+
+	t.Run("with just trace driver", func(t *testing.T) {
+		driverTraces := &stubProtocolDriver{}
+
+		driver := otlp.NewSplitDriver(otlp.WithTraceDriver(driverTraces))
+		ctx := context.Background()
+		assert.NoError(t, driver.Start(ctx))
+		assert.Equal(t, 1, driverTraces.started)
+		assert.Equal(t, 0, driverTraces.stopped)
+		assert.Equal(t, 0, driverTraces.tracesExported)
+		assert.Equal(t, 0, driverTraces.metricsExported)
+
+		assertExport(t, ctx, driver)
+		assert.Len(t, driverTraces.rm, 0)
+		assert.Len(t, driverTraces.rs, spanCount)
+		assert.Equal(t, 1, driverTraces.tracesExported)
+		assert.Equal(t, 0, driverTraces.metricsExported)
+
+		assert.NoError(t, driver.Stop(ctx))
+		assert.Equal(t, 1, driverTraces.started)
+		assert.Equal(t, 1, driverTraces.stopped)
+		assert.Equal(t, 1, driverTraces.tracesExported)
+		assert.Equal(t, 0, driverTraces.metricsExported)
+	})
+
+	t.Run("with no drivers configured", func(t *testing.T) {
+
+		driver := otlp.NewSplitDriver()
+		ctx := context.Background()
+		assert.NoError(t, driver.Start(ctx))
+
+		assert.NoError(t, driver.ExportMetrics(ctx, stubCheckpointSet{recordCount}, metricsdk.StatelessExportKindSelector()))
+		assert.NoError(t, driver.ExportTraces(ctx, stubSpanSnapshot(spanCount)))
+		assert.NoError(t, driver.Stop(ctx))
+	})
+
 }
 
 func TestSplitDriverFail(t *testing.T) {
@@ -357,11 +423,7 @@ func TestSplitDriverFail(t *testing.T) {
 			injectedStartError: errStartMetric,
 			injectedStopError:  errStopMetric,
 		}
-		config := otlp.SplitConfig{
-			ForMetrics: driverMetrics,
-			ForTraces:  driverTraces,
-		}
-		driver := otlp.NewSplitDriver(config)
+		driver := otlp.NewSplitDriver(otlp.WithMetricDriver(driverMetrics), otlp.WithTraceDriver(driverTraces))
 		errStart := driver.Start(ctx)
 		if shouldStartFail {
 			assert.Error(t, errStart)

diff --git a/exporters/otlp/otlpgrpc/example_test.go b/exporters/otlp/otlpgrpc/example_test.go
@@ -143,11 +143,7 @@ func Example_withDifferentSignalCollectors() {
 		otlpgrpc.WithInsecure(),
 		otlpgrpc.WithEndpoint("localhost:30082"),
 	)
-	splitCfg := otlp.SplitConfig{
-		ForMetrics: metricsDriver,
-		ForTraces:  tracesDriver,
-	}
-	driver := otlp.NewSplitDriver(splitCfg)
+	driver := otlp.NewSplitDriver(otlp.WithMetricDriver(metricsDriver), otlp.WithTraceDriver(tracesDriver))
 	ctx := context.Background()
 	exp, err := otlp.NewExporter(ctx, driver)
 	if err != nil {

diff --git a/exporters/otlp/otlpgrpc/otlp_integration_test.go b/exporters/otlp/otlpgrpc/otlp_integration_test.go
@@ -626,11 +626,7 @@ func TestMultiConnectionDriver(t *testing.T) {
 
 	tracesDriver := otlpgrpc.NewDriver(optsTraces...)
 	metricsDriver := otlpgrpc.NewDriver(optsMetrics...)
-	splitCfg := otlp.SplitConfig{
-		ForMetrics: metricsDriver,
-		ForTraces:  tracesDriver,
-	}
-	driver := otlp.NewSplitDriver(splitCfg)
+	driver := otlp.NewSplitDriver(otlp.WithMetricDriver(metricsDriver), otlp.WithTraceDriver(tracesDriver))
 	ctx := context.Background()
 	exp, err := otlp.NewExporter(ctx, driver)
 	if err != nil {

diff --git a/exporters/otlp/protocoldriver.go b/exporters/otlp/protocoldriver.go
@@ -66,16 +66,27 @@ type splitDriver struct {
 	trace  ProtocolDriver
 }
 
+// noopDriver implements the ProtocolDriver interface and
+// is used internally to implement split drivers that do not have
+// all drivers configured.
+type noopDriver struct{}
+
+var _ ProtocolDriver = (*noopDriver)(nil)
+
 var _ ProtocolDriver = (*splitDriver)(nil)
 
 // NewSplitDriver creates a protocol driver which contains two other
 // protocol drivers and will forward traces to one of them and metrics
 // to another.
-func NewSplitDriver(cfg SplitConfig) ProtocolDriver {
-	return &splitDriver{
-		metric: cfg.ForMetrics,
-		trace:  cfg.ForTraces,
+func NewSplitDriver(opts ...SplitDriverOption) ProtocolDriver {
+	driver := splitDriver{
+		metric: &noopDriver{},
+		trace:  &noopDriver{},
 	}
+	for _, opt := range opts {
+		opt.Apply(&driver)
+	}
+	return &driver
 }
 
 // Start implements ProtocolDriver. It starts both drivers at the same
@@ -143,3 +154,23 @@ func (d *splitDriver) ExportMetrics(ctx context.Context, cps metricsdk.Checkpoin
 func (d *splitDriver) ExportTraces(ctx context.Context, ss []*tracesdk.SpanSnapshot) error {
 	return d.trace.ExportTraces(ctx, ss)
 }
+
+// Start does nothing.
+func (d *noopDriver) Start(ctx context.Context) error {
+	return nil
+}
+
+// Stop does nothing.
+func (d *noopDriver) Stop(ctx context.Context) error {
+	return nil
+}
+
+// ExportMetrics does nothing.
+func (d *noopDriver) ExportMetrics(ctx context.Context, cps metricsdk.CheckpointSet, selector metricsdk.ExportKindSelector) error {
+	return nil
+}
+
+// ExportTraces does nothing.
+func (d *noopDriver) ExportTraces(ctx context.Context, ss []*tracesdk.SpanSnapshot) error {
+	return nil
+}