diff --git a/component/exporter.go b/component/exporter.go index bf71ca8119d..65aa25cbd9d 100644 --- a/component/exporter.go +++ b/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 diff --git a/component/exporter_test.go b/component/exporter_test.go index 3f5d356f747..a4dc9aeb80e 100644 --- a/component/exporter_test.go +++ b/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"}, }, diff --git a/component/factories.go b/component/factories.go index bb883d1a1fa..860b5c57063 100644 --- a/component/factories.go +++ b/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()) diff --git a/config/config.go b/config/config.go index 9366c22bcdb..c73d93883b9 100644 --- a/config/config.go +++ b/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) diff --git a/exporter/exporterhelper/metricshelper.go b/exporter/exporterhelper/metricshelper.go index bf8d9884a44..b50e54bcd44 100644 --- a/exporter/exporterhelper/metricshelper.go +++ b/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 diff --git a/exporter/exporterhelper/metricshelper_test.go b/exporter/exporterhelper/metricshelper_test.go index 63ea9d48a26..990e92226f3 100644 --- a/exporter/exporterhelper/metricshelper_test.go +++ b/exporter/exporterhelper/metricshelper_test.go @@ -18,14 +18,12 @@ import ( "errors" "testing" - metricspb "github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opencensus.io/trace" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configmodels" - "go.opentelemetry.io/collector/consumer/consumerdata" "go.opentelemetry.io/collector/consumer/consumererror" "go.opentelemetry.io/collector/consumer/pdata" "go.opentelemetry.io/collector/consumer/pdatautil" @@ -156,107 +154,6 @@ func TestMetricsExporter_WithShutdown_ReturnError(t *testing.T) { assert.Equal(t, me.Shutdown(context.Background()), want) } -func TestMetricsExporterOld_InvalidName(t *testing.T) { - me, err := NewMetricsExporterOld(nil, newPushMetricsDataOld(0, nil)) - require.Nil(t, me) - require.Equal(t, errNilConfig, err) -} - -func TestMetricsExporterOld_NilPushMetricsData(t *testing.T) { - me, err := NewMetricsExporterOld(fakeMetricsExporterConfig, nil) - require.Nil(t, me) - require.Equal(t, errNilPushMetricsData, err) -} - -func TestMetricsExporterOld_Default(t *testing.T) { - md := consumerdata.MetricsData{} - me, err := NewMetricsExporterOld(fakeMetricsExporterConfig, newPushMetricsDataOld(0, nil)) - assert.NotNil(t, me) - assert.NoError(t, err) - - assert.Nil(t, me.ConsumeMetricsData(context.Background(), md)) - assert.Nil(t, me.Shutdown(context.Background())) -} - -func TestMetricsExporterOld_Default_ReturnError(t *testing.T) { - md := consumerdata.MetricsData{} - want := errors.New("my_error") - me, err := NewMetricsExporterOld(fakeMetricsExporterConfig, newPushMetricsDataOld(0, want)) - require.Nil(t, err) - require.NotNil(t, me) - require.Equal(t, want, me.ConsumeMetricsData(context.Background(), md)) -} - -func TestMetricsExporterOld_WithRecordMetrics(t *testing.T) { - me, err := NewMetricsExporterOld(fakeMetricsExporterConfig, newPushMetricsDataOld(0, nil)) - require.Nil(t, err) - require.NotNil(t, me) - - checkRecordedMetricsForMetricsExporterOld(t, me, nil, 0) -} - -func TestMetricsExporterOld_WithRecordMetrics_NonZeroDropped(t *testing.T) { - me, err := NewMetricsExporterOld(fakeMetricsExporterConfig, newPushMetricsDataOld(1, nil)) - require.Nil(t, err) - require.NotNil(t, me) - - checkRecordedMetricsForMetricsExporterOld(t, me, nil, 1) -} - -func TestMetricsExporterOld_WithRecordMetrics_ReturnError(t *testing.T) { - want := errors.New("my_error") - me, err := NewMetricsExporterOld(fakeMetricsExporterConfig, newPushMetricsDataOld(0, want)) - require.Nil(t, err) - require.NotNil(t, me) - - checkRecordedMetricsForMetricsExporterOld(t, me, want, 0) -} - -func TestMetricsExporterOld_WithSpan(t *testing.T) { - me, err := NewMetricsExporterOld(fakeMetricsExporterConfig, newPushMetricsDataOld(0, nil)) - require.Nil(t, err) - require.NotNil(t, me) - checkWrapSpanForMetricsExporterOld(t, me, nil, 1) -} - -func TestMetricsExporterOld_WithSpan_NonZeroDropped(t *testing.T) { - me, err := NewMetricsExporterOld(fakeMetricsExporterConfig, newPushMetricsDataOld(1, nil)) - require.Nil(t, err) - require.NotNil(t, me) - checkWrapSpanForMetricsExporterOld(t, me, nil, 1) -} - -func TestMetricsExporterOld_WithSpan_ReturnError(t *testing.T) { - want := errors.New("my_error") - me, err := NewMetricsExporterOld(fakeMetricsExporterConfig, newPushMetricsDataOld(0, want)) - require.Nil(t, err) - require.NotNil(t, me) - checkWrapSpanForMetricsExporterOld(t, me, want, 1) -} - -func TestMetricsExporterOld_WithShutdown(t *testing.T) { - shutdownCalled := false - shutdown := func(context.Context) error { shutdownCalled = true; return nil } - - me, err := NewMetricsExporterOld(fakeMetricsExporterConfig, newPushMetricsDataOld(0, nil), WithShutdown(shutdown)) - assert.NotNil(t, me) - assert.NoError(t, err) - - assert.Nil(t, me.Shutdown(context.Background())) - assert.True(t, shutdownCalled) -} - -func TestMetricsExporterOld_WithShutdown_ReturnError(t *testing.T) { - want := errors.New("my_error") - shutdownErr := func(context.Context) error { return want } - - me, err := NewMetricsExporterOld(fakeMetricsExporterConfig, newPushMetricsDataOld(0, nil), WithShutdown(shutdownErr)) - assert.NotNil(t, me) - assert.NoError(t, err) - - assert.Equal(t, me.Shutdown(context.Background()), want) -} - func newPushMetricsData(droppedTimeSeries int, retError error) PushMetricsData { return func(ctx context.Context, td pdata.Metrics) (int, error) { return droppedTimeSeries, retError @@ -325,89 +222,3 @@ func checkWrapSpanForMetricsExporter(t *testing.T, me component.MetricsExporter, require.Equalf(t, failedToSendMetricPoints, sd.Attributes[obsreport.FailedToSendMetricPointsKey], "SpanData %v", sd) } } - -func newPushMetricsDataOld(droppedTimeSeries int, retError error) PushMetricsDataOld { - return func(ctx context.Context, td consumerdata.MetricsData) (int, error) { - return droppedTimeSeries, retError - } -} - -func checkRecordedMetricsForMetricsExporterOld(t *testing.T, me component.MetricsExporterOld, wantError error, droppedTimeSeries int) { - doneFn, err := obsreporttest.SetupRecordedMetricsTest() - require.NoError(t, err) - defer doneFn() - - metrics := []*metricspb.Metric{ - { - Timeseries: []*metricspb.TimeSeries{{Points: []*metricspb.Point{{Value: &metricspb.Point_Int64Value{}}}}}, - }, - { - Timeseries: []*metricspb.TimeSeries{{Points: []*metricspb.Point{{Value: &metricspb.Point_Int64Value{}}}}}, - }, - } - md := consumerdata.MetricsData{Metrics: metrics} - const numBatches = 7 - for i := 0; i < numBatches; i++ { - require.Equal(t, wantError, me.ConsumeMetricsData(context.Background(), md)) - } - - // TODO: When the new metrics correctly count partial dropped fix this. - numPoints := int64(numBatches * len(md.Metrics)) - if wantError != nil { - obsreporttest.CheckExporterMetricsViews(t, fakeMetricsExporterName, 0, numPoints) - } else { - obsreporttest.CheckExporterMetricsViews(t, fakeMetricsExporterName, numPoints, 0) - } -} - -func generateMetricsTrafficOld(t *testing.T, me component.MetricsExporterOld, numRequests int, wantError error) { - md := consumerdata.MetricsData{Metrics: []*metricspb.Metric{ - { - // Create a empty timeseries with one point. - Timeseries: []*metricspb.TimeSeries{ - { - Points: []*metricspb.Point{{}}, - }, - }, - }, - }} - ctx, span := trace.StartSpan(context.Background(), fakeMetricsParentSpanName, trace.WithSampler(trace.AlwaysSample())) - defer span.End() - for i := 0; i < numRequests; i++ { - require.Equal(t, wantError, me.ConsumeMetricsData(ctx, md)) - } -} - -func checkWrapSpanForMetricsExporterOld(t *testing.T, me component.MetricsExporterOld, wantError error, numMetricPoints int64) { - ocSpansSaver := new(testOCTraceExporter) - trace.RegisterExporter(ocSpansSaver) - defer trace.UnregisterExporter(ocSpansSaver) - - const numRequests = 5 - generateMetricsTrafficOld(t, me, numRequests, wantError) - - // Inspection time! - ocSpansSaver.mu.Lock() - defer ocSpansSaver.mu.Unlock() - - require.NotEqual(t, 0, len(ocSpansSaver.spanData), "No exported span data") - - gotSpanData := ocSpansSaver.spanData - require.Equal(t, numRequests+1, len(gotSpanData)) - - parentSpan := gotSpanData[numRequests] - require.Equalf(t, fakeMetricsParentSpanName, parentSpan.Name, "SpanData %v", parentSpan) - for _, sd := range gotSpanData[:numRequests] { - require.Equalf(t, parentSpan.SpanContext.SpanID, sd.ParentSpanID, "Exporter span not a child\nSpanData %v", sd) - require.Equalf(t, errToStatus(wantError), sd.Status, "SpanData %v", sd) - - sentMetricPoints := numMetricPoints - var failedToSendMetricPoints int64 - if wantError != nil { - sentMetricPoints = 0 - failedToSendMetricPoints = numMetricPoints - } - require.Equalf(t, sentMetricPoints, sd.Attributes[obsreport.SentMetricPointsKey], "SpanData %v", sd) - require.Equalf(t, failedToSendMetricPoints, sd.Attributes[obsreport.FailedToSendMetricPointsKey], "SpanData %v", sd) - } -} diff --git a/exporter/exporterhelper/tracehelper.go b/exporter/exporterhelper/tracehelper.go index a876e0a71e4..f45e4c80f06 100644 --- a/exporter/exporterhelper/tracehelper.go +++ b/exporter/exporterhelper/tracehelper.go @@ -19,67 +19,11 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configmodels" - "go.opentelemetry.io/collector/consumer/consumerdata" "go.opentelemetry.io/collector/consumer/consumererror" "go.opentelemetry.io/collector/consumer/pdata" "go.opentelemetry.io/collector/obsreport" ) -// traceDataPusherOld is a helper function that is similar to ConsumeTraceData but also -// returns the number of dropped spans. -type traceDataPusherOld func(ctx context.Context, td consumerdata.TraceData) (droppedSpans int, err error) - -type traceExporterOld struct { - *baseExporter - dataPusher traceDataPusherOld -} - -func (texp *traceExporterOld) ConsumeTraceData(ctx context.Context, td consumerdata.TraceData) error { - exporterCtx := obsreport.ExporterContext(ctx, texp.cfg.Name()) - _, err := texp.dataPusher(exporterCtx, td) - return err -} - -// NewTraceExporterOld creates an TraceExporterOld that records observability metrics and wraps every request with a Span. -func NewTraceExporterOld( - cfg configmodels.Exporter, - dataPusher traceDataPusherOld, - options ...ExporterOption, -) (component.TraceExporterOld, error) { - - if cfg == nil { - return nil, errNilConfig - } - - if dataPusher == nil { - return nil, errNilPushTraceData - } - - dataPusher = dataPusher.withObservability(cfg.Name()) - - return &traceExporterOld{ - baseExporter: newBaseExporter(cfg, options...), - dataPusher: dataPusher, - }, nil -} - -// withObservability wraps the current pusher into a function that records -// the observability signals during the pusher execution. -func (p traceDataPusherOld) withObservability(exporterName string) traceDataPusherOld { - return func(ctx context.Context, td consumerdata.TraceData) (int, error) { - ctx = obsreport.StartTraceDataExportOp(ctx, exporterName) - // Forward the data to the next consumer (this pusher is the next). - droppedSpans, err := p(ctx, td) - - // 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. - numSpans := len(td.Spans) - obsreport.EndTraceDataExportOp(ctx, numSpans, droppedSpans, err) - return droppedSpans, err - } -} - // traceDataPusher is a helper function that is similar to ConsumeTraceData but also // returns the number of dropped spans. type traceDataPusher func(ctx context.Context, td pdata.Traces) (droppedSpans int, err error) diff --git a/exporter/exporterhelper/tracehelper_test.go b/exporter/exporterhelper/tracehelper_test.go index 003677cbdd1..dcd4fd69f59 100644 --- a/exporter/exporterhelper/tracehelper_test.go +++ b/exporter/exporterhelper/tracehelper_test.go @@ -19,14 +19,12 @@ import ( "sync" "testing" - tracepb "github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opencensus.io/trace" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configmodels" - "go.opentelemetry.io/collector/consumer/consumerdata" "go.opentelemetry.io/collector/consumer/consumererror" "go.opentelemetry.io/collector/consumer/pdata" "go.opentelemetry.io/collector/internal/data/testdata" @@ -54,184 +52,6 @@ func TestTracesRequest(t *testing.T) { assert.EqualValues(t, newTracesRequest(context.Background(), testdata.GenerateTraceDataEmpty(), nil), mr.onPartialError(partialErr.(consumererror.PartialError))) } -// TODO https://go.opentelemetry.io/collector/issues/266 -// Migrate tests to use testify/assert instead of t.Fatal pattern. -func TestTraceExporterOld_InvalidName(t *testing.T) { - te, err := NewTraceExporterOld(nil, newTraceDataPusherOld(0, nil)) - require.Nil(t, te) - require.Equal(t, errNilConfig, err) -} - -func TestTraceExporterOld_NilPushTraceData(t *testing.T) { - te, err := NewTraceExporterOld(fakeTraceExporterConfig, nil) - require.Nil(t, te) - require.Equal(t, errNilPushTraceData, err) -} - -func TestTraceExporterOld_Default(t *testing.T) { - td := consumerdata.TraceData{} - te, err := NewTraceExporterOld(fakeTraceExporterConfig, newTraceDataPusherOld(0, nil)) - assert.NotNil(t, te) - assert.NoError(t, err) - - assert.Nil(t, te.ConsumeTraceData(context.Background(), td)) - assert.Nil(t, te.Shutdown(context.Background())) -} - -func TestTraceExporterOld_Default_ReturnError(t *testing.T) { - td := consumerdata.TraceData{} - want := errors.New("my_error") - te, err := NewTraceExporterOld(fakeTraceExporterConfig, newTraceDataPusherOld(0, want)) - require.Nil(t, err) - require.NotNil(t, te) - - err = te.ConsumeTraceData(context.Background(), td) - require.Equalf(t, want, err, "ConsumeTraceData returns: Want %v Got %v", want, err) -} - -func TestTraceExporterOld_WithRecordMetrics(t *testing.T) { - te, err := NewTraceExporterOld(fakeTraceExporterConfig, newTraceDataPusherOld(0, nil)) - require.Nil(t, err) - require.NotNil(t, te) - - checkRecordedMetricsForTraceExporterOld(t, te, nil, 0) -} - -func TestTraceExporterOld_WithRecordMetrics_NonZeroDropped(t *testing.T) { - te, err := NewTraceExporterOld(fakeTraceExporterConfig, newTraceDataPusherOld(1, nil)) - require.Nil(t, err) - require.NotNil(t, te) - - checkRecordedMetricsForTraceExporterOld(t, te, nil, 1) -} - -func TestTraceExporterOld_WithRecordMetrics_ReturnError(t *testing.T) { - want := errors.New("my_error") - te, err := NewTraceExporterOld(fakeTraceExporterConfig, newTraceDataPusherOld(0, want)) - require.Nil(t, err) - require.NotNil(t, te) - - checkRecordedMetricsForTraceExporterOld(t, te, want, 0) -} - -func TestTraceExporterOld_WithSpan(t *testing.T) { - te, err := NewTraceExporterOld(fakeTraceExporterConfig, newTraceDataPusherOld(0, nil)) - require.Nil(t, err) - require.NotNil(t, te) - - checkWrapSpanForTraceExporterOld(t, te, nil, 1) -} - -func TestTraceExporterOld_WithSpan_NonZeroDropped(t *testing.T) { - te, err := NewTraceExporterOld(fakeTraceExporterConfig, newTraceDataPusherOld(1, nil)) - require.Nil(t, err) - require.NotNil(t, te) - - checkWrapSpanForTraceExporterOld(t, te, nil, 1) -} - -func TestTraceExporterOld_WithSpan_ReturnError(t *testing.T) { - want := errors.New("my_error") - te, err := NewTraceExporterOld(fakeTraceExporterConfig, newTraceDataPusherOld(0, want)) - require.Nil(t, err) - require.NotNil(t, te) - - checkWrapSpanForTraceExporterOld(t, te, want, 1) -} - -func TestTraceExporterOld_WithShutdown(t *testing.T) { - shutdownCalled := false - shutdown := func(context.Context) error { shutdownCalled = true; return nil } - - te, err := NewTraceExporterOld(fakeTraceExporterConfig, newTraceDataPusherOld(0, nil), WithShutdown(shutdown)) - assert.NotNil(t, te) - assert.NoError(t, err) - - assert.Nil(t, te.Shutdown(context.Background())) - assert.True(t, shutdownCalled) -} - -func TestTraceExporterOld_WithShutdown_ReturnError(t *testing.T) { - want := errors.New("my_error") - shutdownErr := func(context.Context) error { return want } - - te, err := NewTraceExporterOld(fakeTraceExporterConfig, newTraceDataPusherOld(0, nil), WithShutdown(shutdownErr)) - assert.NotNil(t, te) - assert.NoError(t, err) - - assert.Equal(t, te.Shutdown(context.Background()), want) -} - -func newTraceDataPusherOld(droppedSpans int, retError error) traceDataPusherOld { - return func(ctx context.Context, td consumerdata.TraceData) (int, error) { - return droppedSpans, retError - } -} - -func checkRecordedMetricsForTraceExporterOld(t *testing.T, te component.TraceExporterOld, wantError error, droppedSpans int) { - doneFn, err := obsreporttest.SetupRecordedMetricsTest() - require.NoError(t, err) - defer doneFn() - - spans := make([]*tracepb.Span, 2) - td := consumerdata.TraceData{Spans: spans} - const numBatches = 7 - for i := 0; i < numBatches; i++ { - require.Equal(t, wantError, te.ConsumeTraceData(context.Background(), td)) - } - - // TODO: When the new metrics correctly count partial dropped fix this. - if wantError != nil { - obsreporttest.CheckExporterTracesViews(t, fakeTraceExporterName, 0, int64(numBatches*len(spans))) - } else { - obsreporttest.CheckExporterTracesViews(t, fakeTraceExporterName, int64(numBatches*len(spans)), 0) - } -} - -func generateTraceTrafficOld(t *testing.T, te component.TraceExporterOld, numRequests int, wantError error) { - td := consumerdata.TraceData{Spans: make([]*tracepb.Span, 1)} - ctx, span := trace.StartSpan(context.Background(), fakeTraceParentSpanName, trace.WithSampler(trace.AlwaysSample())) - defer span.End() - for i := 0; i < numRequests; i++ { - require.Equal(t, wantError, te.ConsumeTraceData(ctx, td)) - } -} - -func checkWrapSpanForTraceExporterOld(t *testing.T, te component.TraceExporterOld, wantError error, numSpans int64) { - ocSpansSaver := new(testOCTraceExporter) - trace.RegisterExporter(ocSpansSaver) - defer trace.UnregisterExporter(ocSpansSaver) - - const numRequests = 5 - generateTraceTrafficOld(t, te, numRequests, wantError) - - // Inspection time! - ocSpansSaver.mu.Lock() - defer ocSpansSaver.mu.Unlock() - - require.NotEqual(t, 0, len(ocSpansSaver.spanData), "No exported span data.") - - gotSpanData := ocSpansSaver.spanData - require.Equal(t, numRequests+1, len(gotSpanData)) - - parentSpan := gotSpanData[numRequests] - require.Equalf(t, fakeTraceParentSpanName, parentSpan.Name, "SpanData %v", parentSpan) - - for _, sd := range gotSpanData[:numRequests] { - require.Equalf(t, parentSpan.SpanContext.SpanID, sd.ParentSpanID, "Exporter span not a child\nSpanData %v", sd) - require.Equalf(t, errToStatus(wantError), sd.Status, "SpanData %v", sd) - - sentSpans := numSpans - var failedToSendSpans int64 - if wantError != nil { - sentSpans = 0 - failedToSendSpans = numSpans - } - require.Equalf(t, sentSpans, sd.Attributes[obsreport.SentSpansKey], "SpanData %v", sd) - require.Equalf(t, failedToSendSpans, sd.Attributes[obsreport.FailedToSendSpansKey], "SpanData %v", sd) - } -} - type testOCTraceExporter struct { mu sync.Mutex spanData []*trace.SpanData diff --git a/exporter/exportertest/nop_exporter.go b/exporter/exportertest/nop_exporter.go index a49ba34a743..02eea8c70df 100644 --- a/exporter/exportertest/nop_exporter.go +++ b/exporter/exportertest/nop_exporter.go @@ -18,54 +18,15 @@ import ( "context" "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/consumer/consumerdata" "go.opentelemetry.io/collector/consumer/pdata" ) -type nopExporterOld struct { - name string - retError error -} - -func (ne *nopExporterOld) Start(context.Context, component.Host) error { - return nil -} - -func (ne *nopExporterOld) ConsumeTraceData(context.Context, consumerdata.TraceData) error { - return ne.retError -} - -func (ne *nopExporterOld) ConsumeMetricsData(context.Context, consumerdata.MetricsData) error { - return ne.retError -} - -// Shutdown stops the exporter and is invoked during shutdown. -func (ne *nopExporterOld) Shutdown(context.Context) error { - return nil -} - const ( nopTraceExporterName = "nop_trace" nopMetricsExporterName = "nop_metrics" nopLogsExporterName = "nop_log" ) -// NewNopTraceExporterOld creates an TraceExporter that just drops the received data. -func NewNopTraceExporterOld() component.TraceExporterOld { - ne := &nopExporterOld{ - name: nopTraceExporterName, - } - return ne -} - -// NewNopMetricsExporterOld creates an MetricsExporter that just drops the received data. -func NewNopMetricsExporterOld() component.MetricsExporterOld { - ne := &nopExporterOld{ - name: nopMetricsExporterName, - } - return ne -} - type nopExporter struct { name string retError error diff --git a/exporter/exportertest/nop_exporter_test.go b/exporter/exportertest/nop_exporter_test.go index 1227cb911fa..c985184833c 100644 --- a/exporter/exportertest/nop_exporter_test.go +++ b/exporter/exportertest/nop_exporter_test.go @@ -17,36 +17,13 @@ import ( "context" "testing" - metricspb "github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1" - tracepb "github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1" "github.com/stretchr/testify/require" - "go.opentelemetry.io/collector/consumer/consumerdata" "go.opentelemetry.io/collector/consumer/pdata" "go.opentelemetry.io/collector/consumer/pdatautil" "go.opentelemetry.io/collector/internal/data" ) -func TestNopTraceExporterOld(t *testing.T) { - nte := NewNopTraceExporterOld() - require.NoError(t, nte.Start(context.Background(), nil)) - td := consumerdata.TraceData{ - Spans: make([]*tracepb.Span, 7), - } - require.NoError(t, nte.ConsumeTraceData(context.Background(), td)) - require.NoError(t, nte.Shutdown(context.Background())) -} - -func TestNopMetricsExporterOld(t *testing.T) { - nme := NewNopMetricsExporterOld() - require.NoError(t, nme.Start(context.Background(), nil)) - md := consumerdata.MetricsData{ - Metrics: make([]*metricspb.Metric, 7), - } - require.NoError(t, nme.ConsumeMetricsData(context.Background(), md)) - require.NoError(t, nme.Shutdown(context.Background())) -} - func TestNopTraceExporter(t *testing.T) { nte := NewNopTraceExporter() require.NoError(t, nte.Start(context.Background(), nil)) diff --git a/service/builder/exporters_builder.go b/service/builder/exporters_builder.go index 6a46f9903df..267047c91dd 100644 --- a/service/builder/exporters_builder.go +++ b/service/builder/exporters_builder.go @@ -30,8 +30,8 @@ import ( // a trace and/or a metrics consumer and have a shutdown function. type builtExporter struct { logger *zap.Logger - te component.TraceExporterBase - me component.MetricsExporterBase + te component.TraceExporter + me component.MetricsExporter le component.LogsExporter } @@ -73,11 +73,11 @@ func (exp *builtExporter) Shutdown(ctx context.Context) error { return componenterror.CombineErrors(errors) } -func (exp *builtExporter) GetTraceExporter() component.TraceExporterBase { +func (exp *builtExporter) GetTraceExporter() component.TraceExporter { return exp.te } -func (exp *builtExporter) GetMetricExporter() component.MetricsExporterBase { +func (exp *builtExporter) GetMetricExporter() component.MetricsExporter { return exp.me } @@ -146,14 +146,14 @@ type exportersRequiredDataTypes map[configmodels.Exporter]dataTypeRequirements type ExportersBuilder struct { logger *zap.Logger config *configmodels.Config - factories map[configmodels.Type]component.ExporterFactoryBase + factories map[configmodels.Type]component.ExporterFactory } // NewExportersBuilder creates a new ExportersBuilder. Call BuildExporters() on the returned value. func NewExportersBuilder( logger *zap.Logger, config *configmodels.Config, - factories map[configmodels.Type]component.ExporterFactoryBase, + factories map[configmodels.Type]component.ExporterFactory, ) *ExportersBuilder { return &ExportersBuilder{logger.With(zap.String(kindLogKey, kindLogsExporter)), config, factories} } @@ -314,46 +314,35 @@ func exporterTypeMismatchErr( // createTraceProcessor creates a trace exporter based on provided factory type. func createTraceExporter( - factoryBase component.ExporterFactoryBase, + factory component.ExporterFactory, logger *zap.Logger, cfg configmodels.Exporter, -) (component.TraceExporterBase, error) { - if factory, ok := factoryBase.(component.ExporterFactory); ok { - creationParams := component.ExporterCreateParams{Logger: logger} - ctx := context.Background() - - // If exporter is of the new type (can manipulate on internal data structure), - // use ExporterFactory.CreateTraceExporter. - return factory.CreateTraceExporter(ctx, creationParams, cfg) - } +) (component.TraceExporter, error) { + creationParams := component.ExporterCreateParams{Logger: logger} + ctx := context.Background() - // If exporter is of the old type (can manipulate on OC traces only), - // use ExporterFactoryOld.CreateTraceExporter. - return factoryBase.(component.ExporterFactoryOld).CreateTraceExporter(logger, cfg) + // If exporter is of the new type (can manipulate on internal data structure), + // use ExporterFactory.CreateTraceExporter. + return factory.CreateTraceExporter(ctx, creationParams, cfg) } // createMetricsExporter creates a metrics exporter based on provided factory type. -func createMetricsExporter(factoryBase component.ExporterFactoryBase, +func createMetricsExporter( + factory component.ExporterFactory, logger *zap.Logger, cfg configmodels.Exporter, -) (component.MetricsExporterBase, error) { - if factory, ok := factoryBase.(component.ExporterFactory); ok { - creationParams := component.ExporterCreateParams{Logger: logger} - ctx := context.Background() - - // If exporter is of the new type (can manipulate on internal data structure), - // use ExporterFactory.CreateMetricsExporter. - return factory.CreateMetricsExporter(ctx, creationParams, cfg) - } +) (component.MetricsExporter, error) { + creationParams := component.ExporterCreateParams{Logger: logger} + ctx := context.Background() - // If exporter is of the old type (can manipulate on OC metrics only), - // use ExporterFactoryOld.CreateMetricsExporter. - return factoryBase.(component.ExporterFactoryOld).CreateMetricsExporter(logger, cfg) + // If exporter is of the new type (can manipulate on internal data structure), + // use ExporterFactory.CreateMetricsExporter. + return factory.CreateMetricsExporter(ctx, creationParams, cfg) } // createLogsExporter creates a data exporter based on provided factory type. func createLogsExporter( - factoryBase component.ExporterFactoryBase, + factoryBase component.ExporterFactory, logger *zap.Logger, cfg configmodels.Exporter, ) (component.LogsExporter, error) { diff --git a/service/builder/exporters_builder_test.go b/service/builder/exporters_builder_test.go index f6bbfbdee48..623fdbc761b 100644 --- a/service/builder/exporters_builder_test.go +++ b/service/builder/exporters_builder_test.go @@ -26,6 +26,7 @@ import ( "go.opentelemetry.io/collector/component/componenttest" "go.opentelemetry.io/collector/config/configgrpc" "go.opentelemetry.io/collector/config/configmodels" + "go.opentelemetry.io/collector/exporter/exporterhelper" "go.opentelemetry.io/collector/exporter/opencensusexporter" ) @@ -205,8 +206,8 @@ func TestExportersBuilder_StopAll(t *testing.T) { } func TestExportersBuilder_ErrorOnNilExporter(t *testing.T) { - bf := &badExporterFactory{} - fm := map[configmodels.Type]component.ExporterFactoryBase{ + bf := newBadExporterFactory() + fm := map[configmodels.Type]component.ExporterFactory{ bf.Type(): bf, } @@ -247,21 +248,8 @@ func TestExportersBuilder_ErrorOnNilExporter(t *testing.T) { } } -// badExporterFactory is a factory that returns no error but returns a nil object. -type badExporterFactory struct{} - -func (b *badExporterFactory) Type() configmodels.Type { - return "bf" -} - -func (b *badExporterFactory) CreateDefaultConfig() configmodels.Exporter { - return &configmodels.ExporterSettings{} -} - -func (b *badExporterFactory) CreateTraceExporter(_ *zap.Logger, _ configmodels.Exporter) (component.TraceExporterOld, error) { - return nil, nil -} - -func (b *badExporterFactory) CreateMetricsExporter(_ *zap.Logger, _ configmodels.Exporter) (component.MetricsExporterOld, error) { - return nil, nil +func newBadExporterFactory() component.ExporterFactory { + return exporterhelper.NewFactory("bf", func() configmodels.Exporter { + return &configmodels.ExporterSettings{} + }) } diff --git a/service/builder/pipelines_builder_test.go b/service/builder/pipelines_builder_test.go index 053bd7f0886..692c762418f 100644 --- a/service/builder/pipelines_builder_test.go +++ b/service/builder/pipelines_builder_test.go @@ -70,7 +70,7 @@ func createExampleFactories() component.Factories { Processors: map[configmodels.Type]component.ProcessorFactory{ exampleProcessorFactory.Type(): exampleProcessorFactory, }, - Exporters: map[configmodels.Type]component.ExporterFactoryBase{ + Exporters: map[configmodels.Type]component.ExporterFactory{ exampleExporterFactory.Type(): exampleExporterFactory, }, } diff --git a/service/service_test.go b/service/service_test.go index 5417f7b18d1..ba8c9afb712 100644 --- a/service/service_test.go +++ b/service/service_test.go @@ -360,7 +360,7 @@ func TestApplication_GetFactory(t *testing.T) { Processors: map[configmodels.Type]component.ProcessorFactory{ exampleProcessorFactory.Type(): exampleProcessorFactory, }, - Exporters: map[configmodels.Type]component.ExporterFactoryBase{ + Exporters: map[configmodels.Type]component.ExporterFactory{ exampleExporterFactory.Type(): exampleExporterFactory, }, Extensions: map[configmodels.Type]component.ExtensionFactory{ @@ -408,7 +408,7 @@ func createExampleApplication(t *testing.T) *Application { Processors: map[configmodels.Type]component.ProcessorFactory{ exampleProcessorFactory.Type(): exampleProcessorFactory, }, - Exporters: map[configmodels.Type]component.ExporterFactoryBase{ + Exporters: map[configmodels.Type]component.ExporterFactory{ exampleExporterFactory.Type(): exampleExporterFactory, }, Extensions: map[configmodels.Type]component.ExtensionFactory{