Remove old exporter and exporter factory (#1630)

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>

component/exporter.go

@@ -28,71 +28,24 @@ type Exporter interface {
 	Component
 }
 
-// TraceExporterBase defines a common interface for TraceExporter and TraceExporterOld
-type TraceExporterBase interface {
-	Exporter
-}
-
-// TraceExporterOld is a TraceExporter that can consume old-style traces.
-type TraceExporterOld interface {
-	consumer.TraceConsumerOld
-	TraceExporterBase
-}
-
-// TraceExporter is a TraceExporter that can consume new-style traces.
+// TraceExporter is a Exporter that can consume traces.
 type TraceExporter interface {
-	consumer.TraceConsumer
-	TraceExporterBase
-}
-
-// MetricsExporterBase defines a common interface for MetricsExporter and MetricsExporterOld
-type MetricsExporterBase interface {
 	Exporter
+	consumer.TraceConsumer
 }
 
-// MetricsExporterOld is a TraceExporter that can consume old-style metrics.
-type MetricsExporterOld interface {
-	consumer.MetricsConsumerOld
-	MetricsExporterBase
-}
-
-// MetricsExporter is a TraceExporter that can consume new-style metrics.
+// MetricsExporter is an Exporter that can consume metrics.
 type MetricsExporter interface {
+	Exporter
 	consumer.MetricsConsumer
-	MetricsExporterBase
 }
 
-// LogsExporter is a LogsConsumer that is also an Exporter.
+// LogsExporter is an Exporter that can consume logs.
 type LogsExporter interface {
 	Exporter
 	consumer.LogsConsumer
 }
 
-// ExporterFactoryBase defines the common functions for all exporter factories.
-type ExporterFactoryBase interface {
-	Factory
-
-	// CreateDefaultConfig creates the default configuration for the Exporter.
-	// This method can be called multiple times depending on the pipeline
-	// configuration and should not cause side-effects that prevent the creation
-	// of multiple instances of the Exporter.
-	// The object returned by this method needs to pass the checks implemented by
-	// 'configcheck.ValidateConfig'. It is recommended to have such check in the
-	// tests of any implementation of the Factory interface.
-	CreateDefaultConfig() configmodels.Exporter
-}
-
-// ExporterFactoryOld can create TraceExporterOld and MetricsExporterOld.
-type ExporterFactoryOld interface {
-	ExporterFactoryBase
-
-	// CreateTraceExporter creates a trace exporter based on this config.
-	CreateTraceExporter(logger *zap.Logger, cfg configmodels.Exporter) (TraceExporterOld, error)
-
-	// CreateMetricsExporter creates a metrics exporter based on this config.
-	CreateMetricsExporter(logger *zap.Logger, cfg configmodels.Exporter) (MetricsExporterOld, error)
-}
-
 // ExporterCreateParams is passed to Create*Exporter functions.
 type ExporterCreateParams struct {
 	// Logger that the factory can use during creation and can pass to the created
@@ -103,19 +56,34 @@ type ExporterCreateParams struct {
 // ExporterFactory can create TraceExporter and MetricsExporter. This is the
 // new factory type that can create new style exporters.
 type ExporterFactory interface {
-	ExporterFactoryBase
+	Factory
+
+	// CreateDefaultConfig creates the default configuration for the Exporter.
+	// This method can be called multiple times depending on the pipeline
+	// configuration and should not cause side-effects that prevent the creation
+	// of multiple instances of the Exporter.
+	// The object returned by this method needs to pass the checks implemented by
+	// 'configcheck.ValidateConfig'. It is recommended to have such check in the
+	// tests of any implementation of the Factory interface.
+	CreateDefaultConfig() configmodels.Exporter
 
 	// CreateTraceExporter creates a trace exporter based on this config.
 	// If the exporter type does not support tracing or if the config is not valid
 	// error will be returned instead.
-	CreateTraceExporter(ctx context.Context, params ExporterCreateParams,
-		cfg configmodels.Exporter) (TraceExporter, error)
+	CreateTraceExporter(
+		ctx context.Context,
+		params ExporterCreateParams,
+		cfg configmodels.Exporter,
+	) (TraceExporter, error)
 
 	// CreateMetricsExporter creates a metrics exporter based on this config.
 	// If the exporter type does not support metrics or if the config is not valid
 	// error will be returned instead.
-	CreateMetricsExporter(ctx context.Context, params ExporterCreateParams,
-		cfg configmodels.Exporter) (MetricsExporter, error)
+	CreateMetricsExporter(
+		ctx context.Context,
+		params ExporterCreateParams,
+		cfg configmodels.Exporter,
+	) (MetricsExporter, error)
 
 	// CreateLogsExporter creates an exporter based on the config.
 	// If the exporter type does not support logs or if the config is not valid

component/exporter_test.go

@@ -15,11 +15,12 @@
 package component
 
 import (
+	"context"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"go.uber.org/zap"
 
+	"go.opentelemetry.io/collector/config/configerror"
 	"go.opentelemetry.io/collector/config/configmodels"
 )
 
@@ -38,34 +39,39 @@ func (f *TestExporterFactory) CreateDefaultConfig() configmodels.Exporter {
 }
 
 // CreateTraceExporter creates a trace exporter based on this config.
-func (f *TestExporterFactory) CreateTraceExporter(*zap.Logger, configmodels.Exporter) (TraceExporterOld, error) {
-	return nil, nil
+func (f *TestExporterFactory) CreateTraceExporter(context.Context, ExporterCreateParams, configmodels.Exporter) (TraceExporter, error) {
+	return nil, configerror.ErrDataTypeIsNotSupported
 }
 
 // CreateMetricsExporter creates a metrics exporter based on this config.
-func (f *TestExporterFactory) CreateMetricsExporter(*zap.Logger, configmodels.Exporter) (MetricsExporterOld, error) {
-	return nil, nil
+func (f *TestExporterFactory) CreateMetricsExporter(context.Context, ExporterCreateParams, configmodels.Exporter) (MetricsExporter, error) {
+	return nil, configerror.ErrDataTypeIsNotSupported
+}
+
+// CreateMetricsExporter creates a logs exporter based on this config.
+func (f *TestExporterFactory) CreateLogsExporter(context.Context, ExporterCreateParams, configmodels.Exporter) (LogsExporter, error) {
+	return nil, configerror.ErrDataTypeIsNotSupported
 }
 
 func TestBuildExporters(t *testing.T) {
 	type testCase struct {
-		in  []ExporterFactoryBase
-		out map[configmodels.Type]ExporterFactoryBase
+		in  []ExporterFactory
+		out map[configmodels.Type]ExporterFactory
 	}
 
 	testCases := []testCase{
 		{
-			in: []ExporterFactoryBase{
+			in: []ExporterFactory{
 				&TestExporterFactory{"exp1"},
 				&TestExporterFactory{"exp2"},
 			},
-			out: map[configmodels.Type]ExporterFactoryBase{
+			out: map[configmodels.Type]ExporterFactory{
 				"exp1": &TestExporterFactory{"exp1"},
 				"exp2": &TestExporterFactory{"exp2"},
 			},
 		},
 		{
-			in: []ExporterFactoryBase{
+			in: []ExporterFactory{
 				&TestExporterFactory{"exp1"},
 				&TestExporterFactory{"exp1"},
 			},

component/factories.go

@@ -30,7 +30,7 @@ type Factories struct {
 	Processors map[configmodels.Type]ProcessorFactory
 
 	// Exporters maps exporter type names in the config to the respective factory.
-	Exporters map[configmodels.Type]ExporterFactoryBase
+	Exporters map[configmodels.Type]ExporterFactory
 
 	// Extensions maps extension type names in the config to the respective factory.
 	Extensions map[configmodels.Type]ExtensionFactory
@@ -67,8 +67,8 @@ func MakeProcessorFactoryMap(factories ...ProcessorFactory) (map[configmodels.Ty
 // MakeExporterFactoryMap takes a list of exporter factories and returns a map
 // with factory type as keys. It returns a non-nil error when more than one factories
 // have the same type.
-func MakeExporterFactoryMap(factories ...ExporterFactoryBase) (map[configmodels.Type]ExporterFactoryBase, error) {
-	fMap := map[configmodels.Type]ExporterFactoryBase{}
+func MakeExporterFactoryMap(factories ...ExporterFactory) (map[configmodels.Type]ExporterFactory, error) {
+	fMap := map[configmodels.Type]ExporterFactory{}
 	for _, f := range factories {
 		if _, ok := fMap[f.Type()]; ok {
 			return fMap, fmt.Errorf("duplicate exporter factory %q", f.Type())

config/config.go

@@ -367,7 +367,7 @@ func loadReceivers(v *viper.Viper, factories map[configmodels.Type]component.Rec
 	return receivers, nil
 }
 
-func loadExporters(v *viper.Viper, factories map[configmodels.Type]component.ExporterFactoryBase) (configmodels.Exporters, error) {
+func loadExporters(v *viper.Viper, factories map[configmodels.Type]component.ExporterFactory) (configmodels.Exporters, error) {
 	// Get the list of all "exporters" sub vipers from config source.
 	exportersConfig := ViperSub(v, exportersKeyName)
 	expandEnvConfig(exportersConfig)

exporter/exporterhelper/metricshelper.go

@@ -26,55 +26,6 @@ import (
 	"go.opentelemetry.io/collector/obsreport"
 )
 
-// PushMetricsDataOld is a helper function that is similar to ConsumeMetricsData but also returns
-// the number of dropped metrics.
-type PushMetricsDataOld func(ctx context.Context, td consumerdata.MetricsData) (droppedTimeSeries int, err error)
-
-type metricsExporterOld struct {
-	*baseExporter
-	pushMetricsData PushMetricsDataOld
-}
-
-func (mexp *metricsExporterOld) ConsumeMetricsData(ctx context.Context, md consumerdata.MetricsData) error {
-	exporterCtx := obsreport.ExporterContext(ctx, mexp.cfg.Name())
-	_, err := mexp.pushMetricsData(exporterCtx, md)
-	return err
-}
-
-// NewMetricsExporterOld creates an MetricsExporter that records observability metrics and wraps every request with a Span.
-// TODO: Add support for retries.
-func NewMetricsExporterOld(cfg configmodels.Exporter, pushMetricsData PushMetricsDataOld, options ...ExporterOption) (component.MetricsExporterOld, error) {
-	if cfg == nil {
-		return nil, errNilConfig
-	}
-
-	if pushMetricsData == nil {
-		return nil, errNilPushMetricsData
-	}
-
-	pushMetricsData = pushMetricsWithObservabilityOld(pushMetricsData, cfg.Name())
-
-	return &metricsExporterOld{
-		baseExporter:    newBaseExporter(cfg, options...),
-		pushMetricsData: pushMetricsData,
-	}, nil
-}
-
-func pushMetricsWithObservabilityOld(next PushMetricsDataOld, exporterName string) PushMetricsDataOld {
-	return func(ctx context.Context, md consumerdata.MetricsData) (int, error) {
-		ctx = obsreport.StartMetricsExportOp(ctx, exporterName)
-		numDroppedTimeSeries, err := next(ctx, md)
-
-		// TODO: this is not ideal: it should come from the next function itself.
-		// 	temporarily loading it from internal format. Once full switch is done
-		// 	to new metrics will remove this.
-		numReceivedTimeSeries, numPoints := pdatautil.TimeseriesAndPointCount(md)
-
-		obsreport.EndMetricsExportOp(ctx, numPoints, numReceivedTimeSeries, numDroppedTimeSeries, err)
-		return numDroppedTimeSeries, err
-	}
-}
-
 // NumTimeSeries returns the number of timeseries in a MetricsData.
 func NumTimeSeries(md consumerdata.MetricsData) int {
 	receivedTimeSeries := 0