Skip to content

Commit

Permalink
[exporterhelper] Replace experimental converter interface with functi…
Browse files Browse the repository at this point in the history
…on (#8764)

As proposed in
#8122 (comment)

If we need backward conversion, we will use an optional argument to the
helper function instead of an optional interface.
  • Loading branch information
dmitryax committed Nov 17, 2023
1 parent abdeacb commit 1e25ed7
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 67 deletions.
20 changes: 20 additions & 0 deletions .chloggen/dmitryax_use-func-instead-of-converter-interface.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# 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: Replace converter interface with function in the new experimental exporter helper.

# One or more tracking issues or pull requests related to the change
issues: [8122]

# 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: [api]
12 changes: 6 additions & 6 deletions exporter/exporterhelper/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ var (
errNilPushMetricsData = errors.New("nil PushMetrics")
// errNilPushLogsData is returned when a nil PushLogs is given.
errNilPushLogsData = errors.New("nil PushLogs")
// errNilTracesConverter is returned when a nil TracesConverter is given.
errNilTracesConverter = errors.New("nil TracesConverter")
// errNilMetricsConverter is returned when a nil MetricsConverter is given.
errNilMetricsConverter = errors.New("nil MetricsConverter")
// errNilLogsConverter is returned when a nil LogsConverter is given.
errNilLogsConverter = errors.New("nil LogsConverter")
// errNilTracesConverter is returned when a nil RequestFromTracesFunc is given.
errNilTracesConverter = errors.New("nil RequestFromTracesFunc")
// errNilMetricsConverter is returned when a nil RequestFromMetricsFunc is given.
errNilMetricsConverter = errors.New("nil RequestFromMetricsFunc")
// errNilLogsConverter is returned when a nil RequestFromLogsFunc is given.
errNilLogsConverter = errors.New("nil RequestFromLogsFunc")
)
11 changes: 4 additions & 7 deletions exporter/exporterhelper/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,21 +108,18 @@ func NewLogsExporter(
}, err
}

// LogsConverter provides an interface for converting plog.Logs into a request.
// RequestFromLogsFunc converts plog.Logs data into a user-defined request.
// This API is at the early stage of development and may change without backward compatibility
// until https://github.com/open-telemetry/opentelemetry-collector/issues/8122 is resolved.
type LogsConverter interface {
// RequestFromLogs converts plog.Logs data into a request.
RequestFromLogs(context.Context, plog.Logs) (Request, error)
}
type RequestFromLogsFunc func(context.Context, plog.Logs) (Request, error)

// NewLogsRequestExporter creates new logs exporter based on custom LogsConverter and RequestSender.
// This API is at the early stage of development and may change without backward compatibility
// until https://github.com/open-telemetry/opentelemetry-collector/issues/8122 is resolved.
func NewLogsRequestExporter(
_ context.Context,
set exporter.CreateSettings,
converter LogsConverter,
converter RequestFromLogsFunc,
options ...Option,
) (exporter.Logs, error) {
if set.Logger == nil {
Expand All @@ -139,7 +136,7 @@ func NewLogsRequestExporter(
}

lc, err := consumer.NewLogs(func(ctx context.Context, ld plog.Logs) error {
req, cErr := converter.RequestFromLogs(ctx, ld)
req, cErr := converter(ctx, ld)
if cErr != nil {
set.Logger.Error("Failed to convert logs. Dropping data.",
zap.Int("dropped_log_records", ld.LogRecordCount()),
Expand Down
29 changes: 18 additions & 11 deletions exporter/exporterhelper/logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func TestLogsExporter_NilLogger(t *testing.T) {
}

func TestLogsRequestExporter_NilLogger(t *testing.T) {
le, err := NewLogsRequestExporter(context.Background(), exporter.CreateSettings{}, &fakeRequestConverter{})
le, err := NewLogsRequestExporter(context.Background(), exporter.CreateSettings{}, (&fakeRequestConverter{}).requestFromLogsFunc)
require.Nil(t, le)
require.Equal(t, errNilLogger, err)
}
Expand Down Expand Up @@ -96,7 +96,8 @@ func TestLogsExporter_Default(t *testing.T) {

func TestLogsRequestExporter_Default(t *testing.T) {
ld := plog.NewLogs()
le, err := NewLogsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(), &fakeRequestConverter{})
le, err := NewLogsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(),
(&fakeRequestConverter{}).requestFromLogsFunc)
assert.NotNil(t, le)
assert.NoError(t, err)

Expand All @@ -117,7 +118,8 @@ func TestLogsExporter_WithCapabilities(t *testing.T) {

func TestLogsRequestExporter_WithCapabilities(t *testing.T) {
capabilities := consumer.Capabilities{MutatesData: true}
le, err := NewLogsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(), &fakeRequestConverter{}, WithCapabilities(capabilities))
le, err := NewLogsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(),
(&fakeRequestConverter{}).requestFromLogsFunc, WithCapabilities(capabilities))
require.NoError(t, err)
require.NotNil(t, le)

Expand All @@ -136,7 +138,8 @@ func TestLogsExporter_Default_ReturnError(t *testing.T) {
func TestLogsRequestExporter_Default_ConvertError(t *testing.T) {
ld := plog.NewLogs()
want := errors.New("convert_error")
le, err := NewLogsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(), &fakeRequestConverter{logsError: want})
le, err := NewLogsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(),
(&fakeRequestConverter{logsError: want}).requestFromLogsFunc)
require.NoError(t, err)
require.NotNil(t, le)
require.Equal(t, consumererror.NewPermanent(want), le.ConsumeLogs(context.Background(), ld))
Expand All @@ -146,7 +149,7 @@ func TestLogsRequestExporter_Default_ExportError(t *testing.T) {
ld := plog.NewLogs()
want := errors.New("export_error")
le, err := NewLogsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(),
&fakeRequestConverter{requestError: want})
(&fakeRequestConverter{requestError: want}).requestFromLogsFunc)
require.NoError(t, err)
require.NotNil(t, le)
require.Equal(t, want, le.ConsumeLogs(context.Background(), ld))
Expand Down Expand Up @@ -193,7 +196,9 @@ func TestLogsRequestExporter_WithRecordMetrics(t *testing.T) {
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })

le, err := NewLogsRequestExporter(context.Background(), exporter.CreateSettings{ID: fakeLogsExporterName, TelemetrySettings: tt.TelemetrySettings, BuildInfo: component.NewDefaultBuildInfo()}, &fakeRequestConverter{})
le, err := NewLogsRequestExporter(context.Background(),
exporter.CreateSettings{ID: fakeLogsExporterName, TelemetrySettings: tt.TelemetrySettings, BuildInfo: component.NewDefaultBuildInfo()},
(&fakeRequestConverter{}).requestFromLogsFunc)
require.NoError(t, err)
require.NotNil(t, le)

Expand All @@ -220,7 +225,7 @@ func TestLogsRequestExporter_WithRecordMetrics_ExportError(t *testing.T) {
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })

le, err := NewLogsRequestExporter(context.Background(), exporter.CreateSettings{ID: fakeLogsExporterName, TelemetrySettings: tt.TelemetrySettings, BuildInfo: component.NewDefaultBuildInfo()},
&fakeRequestConverter{requestError: want})
(&fakeRequestConverter{requestError: want}).requestFromLogsFunc)
require.Nil(t, err)
require.NotNil(t, le)

Expand Down Expand Up @@ -272,7 +277,7 @@ func TestLogsRequestExporter_WithSpan(t *testing.T) {
otel.SetTracerProvider(set.TracerProvider)
defer otel.SetTracerProvider(nooptrace.NewTracerProvider())

le, err := NewLogsRequestExporter(context.Background(), set, &fakeRequestConverter{})
le, err := NewLogsRequestExporter(context.Background(), set, (&fakeRequestConverter{}).requestFromLogsFunc)
require.Nil(t, err)
require.NotNil(t, le)
checkWrapSpanForLogsExporter(t, sr, set.TracerProvider.Tracer("test"), le, nil, 1)
Expand Down Expand Up @@ -300,7 +305,7 @@ func TestLogsRequestExporter_WithSpan_ReturnError(t *testing.T) {
defer otel.SetTracerProvider(nooptrace.NewTracerProvider())

want := errors.New("my_error")
le, err := NewLogsRequestExporter(context.Background(), set, &fakeRequestConverter{requestError: want})
le, err := NewLogsRequestExporter(context.Background(), set, (&fakeRequestConverter{requestError: want}).requestFromLogsFunc)
require.Nil(t, err)
require.NotNil(t, le)
checkWrapSpanForLogsExporter(t, sr, set.TracerProvider.Tracer("test"), le, want, 1)
Expand All @@ -322,7 +327,8 @@ func TestLogsRequestExporter_WithShutdown(t *testing.T) {
shutdownCalled := false
shutdown := func(context.Context) error { shutdownCalled = true; return nil }

le, err := NewLogsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(), &fakeRequestConverter{}, WithShutdown(shutdown))
le, err := NewLogsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(),
(&fakeRequestConverter{}).requestFromLogsFunc, WithShutdown(shutdown))
assert.NotNil(t, le)
assert.NoError(t, err)

Expand All @@ -345,7 +351,8 @@ func TestLogsRequestExporter_WithShutdown_ReturnError(t *testing.T) {
want := errors.New("my_error")
shutdownErr := func(context.Context) error { return want }

le, err := NewLogsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(), &fakeRequestConverter{}, WithShutdown(shutdownErr))
le, err := NewLogsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(),
(&fakeRequestConverter{}).requestFromLogsFunc, WithShutdown(shutdownErr))
assert.NotNil(t, le)
assert.NoError(t, err)

Expand Down
11 changes: 4 additions & 7 deletions exporter/exporterhelper/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,21 +108,18 @@ func NewMetricsExporter(
}, err
}

// MetricsConverter provides an interface for converting pmetric.Metrics into a request.
// RequestFromMetricsFunc converts pdata.Metrics into a user-defined request.
// This API is at the early stage of development and may change without backward compatibility
// until https://github.com/open-telemetry/opentelemetry-collector/issues/8122 is resolved.
type MetricsConverter interface {
// RequestFromMetrics converts pdata.Metrics into a request.
RequestFromMetrics(context.Context, pmetric.Metrics) (Request, error)
}
type RequestFromMetricsFunc func(context.Context, pmetric.Metrics) (Request, error)

// NewMetricsRequestExporter creates a new metrics exporter based on a custom MetricsConverter and RequestSender.
// This API is at the early stage of development and may change without backward compatibility
// until https://github.com/open-telemetry/opentelemetry-collector/issues/8122 is resolved.
func NewMetricsRequestExporter(
_ context.Context,
set exporter.CreateSettings,
converter MetricsConverter,
converter RequestFromMetricsFunc,
options ...Option,
) (exporter.Metrics, error) {
if set.Logger == nil {
Expand All @@ -139,7 +136,7 @@ func NewMetricsRequestExporter(
}

mc, err := consumer.NewMetrics(func(ctx context.Context, md pmetric.Metrics) error {
req, cErr := converter.RequestFromMetrics(ctx, md)
req, cErr := converter(ctx, md)
if cErr != nil {
set.Logger.Error("Failed to convert metrics. Dropping data.",
zap.Int("dropped_data_points", md.DataPointCount()),
Expand Down
31 changes: 19 additions & 12 deletions exporter/exporterhelper/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ func TestMetricsExporter_NilLogger(t *testing.T) {
}

func TestMetricsRequestExporter_NilLogger(t *testing.T) {
me, err := NewMetricsRequestExporter(context.Background(), exporter.CreateSettings{}, fakeRequestConverter{})
me, err := NewMetricsRequestExporter(context.Background(), exporter.CreateSettings{},
(&fakeRequestConverter{}).requestFromMetricsFunc)
require.Nil(t, me)
require.Equal(t, errNilLogger, err)
}
Expand Down Expand Up @@ -96,7 +97,8 @@ func TestMetricsExporter_Default(t *testing.T) {

func TestMetricsRequestExporter_Default(t *testing.T) {
md := pmetric.NewMetrics()
me, err := NewMetricsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(), fakeRequestConverter{})
me, err := NewMetricsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(),
(&fakeRequestConverter{}).requestFromMetricsFunc)
assert.NoError(t, err)
assert.NotNil(t, me)

Expand All @@ -117,8 +119,8 @@ func TestMetricsExporter_WithCapabilities(t *testing.T) {

func TestMetricsRequestExporter_WithCapabilities(t *testing.T) {
capabilities := consumer.Capabilities{MutatesData: true}
me, err := NewMetricsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(), fakeRequestConverter{},
WithCapabilities(capabilities))
me, err := NewMetricsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(),
(&fakeRequestConverter{}).requestFromMetricsFunc, WithCapabilities(capabilities))
assert.NoError(t, err)
assert.NotNil(t, me)

Expand All @@ -137,7 +139,8 @@ func TestMetricsExporter_Default_ReturnError(t *testing.T) {
func TestMetricsRequestExporter_Default_ConvertError(t *testing.T) {
md := pmetric.NewMetrics()
want := errors.New("convert_error")
me, err := NewMetricsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(), fakeRequestConverter{metricsError: want})
me, err := NewMetricsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(),
(&fakeRequestConverter{metricsError: want}).requestFromMetricsFunc)
require.NoError(t, err)
require.NotNil(t, me)
require.Equal(t, consumererror.NewPermanent(want), me.ConsumeMetrics(context.Background(), md))
Expand All @@ -147,7 +150,7 @@ func TestMetricsRequestExporter_Default_ExportError(t *testing.T) {
md := pmetric.NewMetrics()
want := errors.New("export_error")
me, err := NewMetricsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(),
fakeRequestConverter{requestError: want})
(&fakeRequestConverter{requestError: want}).requestFromMetricsFunc)
require.NoError(t, err)
require.NotNil(t, me)
require.Equal(t, want, me.ConsumeMetrics(context.Background(), md))
Expand Down Expand Up @@ -194,7 +197,9 @@ func TestMetricsRequestExporter_WithRecordMetrics(t *testing.T) {
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })

me, err := NewMetricsRequestExporter(context.Background(), exporter.CreateSettings{ID: fakeMetricsExporterName, TelemetrySettings: tt.TelemetrySettings, BuildInfo: component.NewDefaultBuildInfo()}, fakeRequestConverter{})
me, err := NewMetricsRequestExporter(context.Background(),
exporter.CreateSettings{ID: fakeMetricsExporterName, TelemetrySettings: tt.TelemetrySettings, BuildInfo: component.NewDefaultBuildInfo()},
(&fakeRequestConverter{}).requestFromMetricsFunc)
require.NoError(t, err)
require.NotNil(t, me)

Expand All @@ -220,7 +225,9 @@ func TestMetricsRequestExporter_WithRecordMetrics_ExportError(t *testing.T) {
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })

me, err := NewMetricsRequestExporter(context.Background(), exporter.CreateSettings{ID: fakeMetricsExporterName, TelemetrySettings: tt.TelemetrySettings, BuildInfo: component.NewDefaultBuildInfo()}, fakeRequestConverter{requestError: want})
me, err := NewMetricsRequestExporter(context.Background(),
exporter.CreateSettings{ID: fakeMetricsExporterName, TelemetrySettings: tt.TelemetrySettings, BuildInfo: component.NewDefaultBuildInfo()},
(&fakeRequestConverter{requestError: want}).requestFromMetricsFunc)
require.NoError(t, err)
require.NotNil(t, me)

Expand Down Expand Up @@ -272,7 +279,7 @@ func TestMetricsRequestExporter_WithSpan(t *testing.T) {
otel.SetTracerProvider(set.TracerProvider)
defer otel.SetTracerProvider(nooptrace.NewTracerProvider())

me, err := NewMetricsRequestExporter(context.Background(), set, fakeRequestConverter{})
me, err := NewMetricsRequestExporter(context.Background(), set, (&fakeRequestConverter{}).requestFromMetricsFunc)
require.NoError(t, err)
require.NotNil(t, me)
checkWrapSpanForMetricsExporter(t, sr, set.TracerProvider.Tracer("test"), me, nil, 2)
Expand Down Expand Up @@ -300,7 +307,7 @@ func TestMetricsRequestExporter_WithSpan_ExportError(t *testing.T) {
defer otel.SetTracerProvider(nooptrace.NewTracerProvider())

want := errors.New("my_error")
me, err := NewMetricsRequestExporter(context.Background(), set, fakeRequestConverter{requestError: want})
me, err := NewMetricsRequestExporter(context.Background(), set, (&fakeRequestConverter{requestError: want}).requestFromMetricsFunc)
require.NoError(t, err)
require.NotNil(t, me)
checkWrapSpanForMetricsExporter(t, sr, set.TracerProvider.Tracer("test"), me, want, 2)
Expand All @@ -324,7 +331,7 @@ func TestMetricsRequestExporter_WithShutdown(t *testing.T) {
shutdown := func(context.Context) error { shutdownCalled = true; return nil }

me, err := NewMetricsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(),
&fakeRequestConverter{}, WithShutdown(shutdown))
(&fakeRequestConverter{}).requestFromMetricsFunc, WithShutdown(shutdown))
assert.NotNil(t, me)
assert.NoError(t, err)

Expand All @@ -350,7 +357,7 @@ func TestMetricsRequestExporter_WithShutdown_ReturnError(t *testing.T) {
shutdownErr := func(context.Context) error { return want }

me, err := NewMetricsRequestExporter(context.Background(), exportertest.NewNopCreateSettings(),
&fakeRequestConverter{}, WithShutdown(shutdownErr))
(&fakeRequestConverter{}).requestFromMetricsFunc, WithShutdown(shutdownErr))
assert.NotNil(t, me)
assert.NoError(t, err)

Expand Down
12 changes: 6 additions & 6 deletions exporter/exporterhelper/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@ type fakeRequestConverter struct {
requestError error
}

func (c fakeRequestConverter) RequestFromMetrics(_ context.Context, md pmetric.Metrics) (Request, error) {
return fakeRequest{items: md.DataPointCount(), err: c.requestError}, c.metricsError
func (frc *fakeRequestConverter) requestFromMetricsFunc(_ context.Context, md pmetric.Metrics) (Request, error) {
return fakeRequest{items: md.DataPointCount(), err: frc.requestError}, frc.metricsError
}

func (c fakeRequestConverter) RequestFromTraces(_ context.Context, td ptrace.Traces) (Request, error) {
return fakeRequest{items: td.SpanCount(), err: c.requestError}, c.tracesError
func (frc *fakeRequestConverter) requestFromTracesFunc(_ context.Context, md ptrace.Traces) (Request, error) {
return fakeRequest{items: md.SpanCount(), err: frc.requestError}, frc.tracesError
}

func (c fakeRequestConverter) RequestFromLogs(_ context.Context, ld plog.Logs) (Request, error) {
return fakeRequest{items: ld.LogRecordCount(), err: c.requestError}, c.logsError
func (frc *fakeRequestConverter) requestFromLogsFunc(_ context.Context, md plog.Logs) (Request, error) {
return fakeRequest{items: md.LogRecordCount(), err: frc.requestError}, frc.logsError
}
11 changes: 4 additions & 7 deletions exporter/exporterhelper/traces.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,21 +108,18 @@ func NewTracesExporter(
}, err
}

// TracesConverter provides an interface for converting ptrace.Traces into a request.
// RequestFromTracesFunc converts ptrace.Traces into a user-defined Request.
// This API is at the early stage of development and may change without backward compatibility
// until https://github.com/open-telemetry/opentelemetry-collector/issues/8122 is resolved.
type TracesConverter interface {
// RequestFromTraces converts ptrace.Traces into a Request.
RequestFromTraces(context.Context, ptrace.Traces) (Request, error)
}
type RequestFromTracesFunc func(context.Context, ptrace.Traces) (Request, error)

// NewTracesRequestExporter creates a new traces exporter based on a custom TracesConverter and RequestSender.
// This API is at the early stage of development and may change without backward compatibility
// until https://github.com/open-telemetry/opentelemetry-collector/issues/8122 is resolved.
func NewTracesRequestExporter(
_ context.Context,
set exporter.CreateSettings,
converter TracesConverter,
converter RequestFromTracesFunc,
options ...Option,
) (exporter.Traces, error) {
if set.Logger == nil {
Expand All @@ -139,7 +136,7 @@ func NewTracesRequestExporter(
}

tc, err := consumer.NewTraces(func(ctx context.Context, td ptrace.Traces) error {
req, cErr := converter.RequestFromTraces(ctx, td)
req, cErr := converter(ctx, td)
if cErr != nil {
set.Logger.Error("Failed to convert traces. Dropping data.",
zap.Int("dropped_spans", td.SpanCount()),
Expand Down
Loading

0 comments on commit 1e25ed7

Please sign in to comment.