Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[exporterhelper] Deprecate QueueSettings, use QueueConfig instead #9393

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .chloggen/queuesettings_queueconfig.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Use this changelog template to create an entry for release notes.

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

# 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: Deprecate QueueSettings, use QueueConfig instead

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

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# 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]
8 changes: 8 additions & 0 deletions exporter/exporterhelper/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,15 @@
// WithQueue overrides the default QueueSettings for an exporter.
// The default QueueSettings is to disable queueing.
// This option cannot be used with the new exporter helpers New[Traces|Metrics|Logs]RequestExporter.
// Deprecated: [v0.94.0] Use WithQueueConfig
func WithQueue(config QueueSettings) Option {
return WithQueueConfig(QueueConfig(config))

Check warning on line 87 in exporter/exporterhelper/common.go

View check run for this annotation

Codecov / codecov/patch

exporter/exporterhelper/common.go#L87

Added line #L87 was not covered by tests
}

// WithQueueConfig overrides the default QueueConfig for an exporter.
// The default QueueConfig is to disable queueing.
// This option cannot be used with the new exporter helpers New[Traces|Metrics|Logs]RequestExporter.
func WithQueueConfig(config QueueConfig) Option {
return func(o *baseExporter) {
if o.requestExporter {
panic("queueing is not available for the new request exporters yet")
Expand Down
2 changes: 1 addition & 1 deletion exporter/exporterhelper/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func TestQueueRetryOptionsWithRequestExporter(t *testing.T) {
require.True(t, bs.requestExporter)
require.Panics(t, func() {
_, _ = newBaseExporter(exportertest.NewNopCreateSettings(), "", true, nil, nil, newNoopObsrepSender,
WithRetry(configretry.NewDefaultBackOffConfig()), WithQueue(NewDefaultQueueSettings()))
WithRetry(configretry.NewDefaultBackOffConfig()), WithQueueConfig(NewDefaultQueueConfig()))
})
}

Expand Down
8 changes: 4 additions & 4 deletions exporter/exporterhelper/logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,14 +156,14 @@ func TestLogsRequestExporter_Default_ExportError(t *testing.T) {
}

func TestLogsExporter_WithPersistentQueue(t *testing.T) {
qCfg := NewDefaultQueueSettings()
qCfg := NewDefaultQueueConfig()
storageID := component.NewIDWithName("file_storage", "storage")
qCfg.StorageID = &storageID
rCfg := configretry.NewDefaultBackOffConfig()
ts := consumertest.LogsSink{}
set := exportertest.NewNopCreateSettings()
set.ID = component.NewIDWithName("test_logs", "with_persistent_queue")
te, err := NewLogsExporter(context.Background(), set, &fakeLogsExporterConfig, ts.ConsumeLogs, WithRetry(rCfg), WithQueue(qCfg))
te, err := NewLogsExporter(context.Background(), set, &fakeLogsExporterConfig, ts.ConsumeLogs, WithRetry(rCfg), WithQueueConfig(qCfg))
require.NoError(t, err)

host := &mockHost{ext: map[component.ID]component.Component{
Expand Down Expand Up @@ -238,11 +238,11 @@ func TestLogsExporter_WithRecordEnqueueFailedMetrics(t *testing.T) {
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })

rCfg := configretry.NewDefaultBackOffConfig()
qCfg := NewDefaultQueueSettings()
qCfg := NewDefaultQueueConfig()
qCfg.NumConsumers = 1
qCfg.QueueSize = 2
wantErr := errors.New("some-error")
te, err := NewLogsExporter(context.Background(), exporter.CreateSettings{ID: fakeLogsExporterName, TelemetrySettings: tt.TelemetrySettings(), BuildInfo: component.NewDefaultBuildInfo()}, &fakeLogsExporterConfig, newPushLogsData(wantErr), WithRetry(rCfg), WithQueue(qCfg))
te, err := NewLogsExporter(context.Background(), exporter.CreateSettings{ID: fakeLogsExporterName, TelemetrySettings: tt.TelemetrySettings(), BuildInfo: component.NewDefaultBuildInfo()}, &fakeLogsExporterConfig, newPushLogsData(wantErr), WithRetry(rCfg), WithQueueConfig(qCfg))
require.NoError(t, err)
require.NotNil(t, te)

Expand Down
8 changes: 4 additions & 4 deletions exporter/exporterhelper/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,14 +157,14 @@ func TestMetricsRequestExporter_Default_ExportError(t *testing.T) {
}

func TestMetricsExporter_WithPersistentQueue(t *testing.T) {
qCfg := NewDefaultQueueSettings()
qCfg := NewDefaultQueueConfig()
storageID := component.NewIDWithName("file_storage", "storage")
qCfg.StorageID = &storageID
rCfg := configretry.NewDefaultBackOffConfig()
ms := consumertest.MetricsSink{}
set := exportertest.NewNopCreateSettings()
set.ID = component.NewIDWithName("test_metrics", "with_persistent_queue")
te, err := NewMetricsExporter(context.Background(), set, &fakeTracesExporterConfig, ms.ConsumeMetrics, WithRetry(rCfg), WithQueue(qCfg))
te, err := NewMetricsExporter(context.Background(), set, &fakeTracesExporterConfig, ms.ConsumeMetrics, WithRetry(rCfg), WithQueueConfig(qCfg))
require.NoError(t, err)

host := &mockHost{ext: map[component.ID]component.Component{
Expand Down Expand Up @@ -240,11 +240,11 @@ func TestMetricsExporter_WithRecordEnqueueFailedMetrics(t *testing.T) {
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })

rCfg := configretry.NewDefaultBackOffConfig()
qCfg := NewDefaultQueueSettings()
qCfg := NewDefaultQueueConfig()
qCfg.NumConsumers = 1
qCfg.QueueSize = 2
wantErr := errors.New("some-error")
te, err := NewMetricsExporter(context.Background(), exporter.CreateSettings{ID: fakeMetricsExporterName, TelemetrySettings: tt.TelemetrySettings(), BuildInfo: component.NewDefaultBuildInfo()}, &fakeMetricsExporterConfig, newPushMetricsData(wantErr), WithRetry(rCfg), WithQueue(qCfg))
te, err := NewMetricsExporter(context.Background(), exporter.CreateSettings{ID: fakeMetricsExporterName, TelemetrySettings: tt.TelemetrySettings(), BuildInfo: component.NewDefaultBuildInfo()}, &fakeMetricsExporterConfig, newPushMetricsData(wantErr), WithRetry(rCfg), WithQueueConfig(qCfg))
require.NoError(t, err)
require.NotNil(t, te)

Expand Down
27 changes: 22 additions & 5 deletions exporter/exporterhelper/queue_sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@ var (
)

// QueueSettings defines configuration for queueing batches before sending to the consumerSender.
type QueueSettings struct {
// Deprecated: [v0.94.0] Use QueueConfig instead
type QueueSettings QueueConfig

// QueueConfig defines configuration for queueing batches before sending to the consumerSender.
type QueueConfig struct {
// Enabled indicates whether to not enqueue batches before sending to the consumerSender.
Enabled bool `mapstructure:"enabled"`
// NumConsumers is the number of consumers from the queue.
Expand All @@ -40,7 +44,8 @@ type QueueSettings struct {
StorageID *component.ID `mapstructure:"storage"`
}

// NewDefaultQueueSettings returns the default settings for QueueSettings.
// NewDefaultQueueSettings returns the default settings for QueueConfig.
// Deprecated: [v0.94.0] Use NewDefaultQueueConfig instead
func NewDefaultQueueSettings() QueueSettings {
return QueueSettings{
Enabled: true,
Expand All @@ -52,8 +57,20 @@ func NewDefaultQueueSettings() QueueSettings {
}
}

// Validate checks if the QueueSettings configuration is valid
func (qCfg *QueueSettings) Validate() error {
// NewDefaultQueueConfig returns the default settings for QueueConfig.
func NewDefaultQueueConfig() QueueConfig {
return QueueConfig{
Enabled: true,
NumConsumers: 10,
// By default, batches are 8192 spans, for a total of up to 8 million spans in the queue
// This can be estimated at 1-4 GB worth of maximum memory usage
// This default is probably still too high, and may be adjusted further down in a future release
QueueSize: defaultQueueSize,
}
}

// Validate checks if the QueueConfig configuration is valid
func (qCfg *QueueConfig) Validate() error {
if !qCfg.Enabled {
return nil
}
Expand Down Expand Up @@ -82,7 +99,7 @@ type queueSender struct {
metricSize otelmetric.Int64ObservableGauge
}

func newQueueSender(config QueueSettings, set exporter.CreateSettings, signal component.DataType,
func newQueueSender(config QueueConfig, set exporter.CreateSettings, signal component.DataType,
marshaler RequestMarshaler, unmarshaler RequestUnmarshaler, consumeErrHandler func(error, Request)) *queueSender {

isPersistent := config.StorageID != nil
Expand Down
46 changes: 23 additions & 23 deletions exporter/exporterhelper/queue_sender_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ import (
)

func TestQueuedRetry_StopWhileWaiting(t *testing.T) {
qCfg := NewDefaultQueueSettings()
qCfg := NewDefaultQueueConfig()
qCfg.NumConsumers = 1
rCfg := configretry.NewDefaultBackOffConfig()
be, err := newBaseExporter(defaultSettings, "", false, nil, nil, newObservabilityConsumerSender, WithRetry(rCfg), WithQueue(qCfg))
be, err := newBaseExporter(defaultSettings, "", false, nil, nil, newObservabilityConsumerSender, WithRetry(rCfg), WithQueueConfig(qCfg))
require.NoError(t, err)
ocs := be.obsrepSender.(*observabilityConsumerSender)
require.NoError(t, be.Start(context.Background(), componenttest.NewNopHost()))
Expand Down Expand Up @@ -55,10 +55,10 @@ func TestQueuedRetry_StopWhileWaiting(t *testing.T) {
}

func TestQueuedRetry_DoNotPreserveCancellation(t *testing.T) {
qCfg := NewDefaultQueueSettings()
qCfg := NewDefaultQueueConfig()
qCfg.NumConsumers = 1
rCfg := configretry.NewDefaultBackOffConfig()
be, err := newBaseExporter(defaultSettings, "", false, nil, nil, newObservabilityConsumerSender, WithRetry(rCfg), WithQueue(qCfg))
be, err := newBaseExporter(defaultSettings, "", false, nil, nil, newObservabilityConsumerSender, WithRetry(rCfg), WithQueueConfig(qCfg))
require.NoError(t, err)
ocs := be.obsrepSender.(*observabilityConsumerSender)
require.NoError(t, be.Start(context.Background(), componenttest.NewNopHost()))
Expand All @@ -82,13 +82,13 @@ func TestQueuedRetry_DoNotPreserveCancellation(t *testing.T) {
}

func TestQueuedRetry_RejectOnFull(t *testing.T) {
qCfg := NewDefaultQueueSettings()
qCfg := NewDefaultQueueConfig()
qCfg.QueueSize = 0
qCfg.NumConsumers = 0
set := exportertest.NewNopCreateSettings()
logger, observed := observer.New(zap.ErrorLevel)
set.Logger = zap.New(logger)
be, err := newBaseExporter(set, "", false, nil, nil, newNoopObsrepSender, WithQueue(qCfg))
be, err := newBaseExporter(set, "", false, nil, nil, newNoopObsrepSender, WithQueueConfig(qCfg))
require.NoError(t, err)
require.NoError(t, be.Start(context.Background(), componenttest.NewNopHost()))
t.Cleanup(func() {
Expand All @@ -105,10 +105,10 @@ func TestQueuedRetryHappyPath(t *testing.T) {
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })

qCfg := NewDefaultQueueSettings()
qCfg := NewDefaultQueueConfig()
rCfg := configretry.NewDefaultBackOffConfig()
set := exporter.CreateSettings{ID: defaultID, TelemetrySettings: tt.TelemetrySettings(), BuildInfo: component.NewDefaultBuildInfo()}
be, err := newBaseExporter(set, "", false, nil, nil, newObservabilityConsumerSender, WithRetry(rCfg), WithQueue(qCfg))
be, err := newBaseExporter(set, "", false, nil, nil, newObservabilityConsumerSender, WithRetry(rCfg), WithQueueConfig(qCfg))
require.NoError(t, err)
ocs := be.obsrepSender.(*observabilityConsumerSender)
require.NoError(t, be.Start(context.Background(), componenttest.NewNopHost()))
Expand Down Expand Up @@ -141,11 +141,11 @@ func TestQueuedRetry_QueueMetricsReported(t *testing.T) {
tt, err := componenttest.SetupTelemetry(defaultID)
require.NoError(t, err)

qCfg := NewDefaultQueueSettings()
qCfg := NewDefaultQueueConfig()
qCfg.NumConsumers = 0 // to make every request go straight to the queue
rCfg := configretry.NewDefaultBackOffConfig()
set := exporter.CreateSettings{ID: defaultID, TelemetrySettings: tt.TelemetrySettings(), BuildInfo: component.NewDefaultBuildInfo()}
be, err := newBaseExporter(set, "", false, nil, nil, newObservabilityConsumerSender, WithRetry(rCfg), WithQueue(qCfg))
be, err := newBaseExporter(set, "", false, nil, nil, newObservabilityConsumerSender, WithRetry(rCfg), WithQueueConfig(qCfg))
require.NoError(t, err)
require.NoError(t, be.Start(context.Background(), componenttest.NewNopHost()))

Expand Down Expand Up @@ -176,13 +176,13 @@ func TestNoCancellationContext(t *testing.T) {
}

func TestQueueSettings_Validate(t *testing.T) {
qCfg := NewDefaultQueueSettings()
qCfg := NewDefaultQueueConfig()
assert.NoError(t, qCfg.Validate())

qCfg.QueueSize = 0
assert.EqualError(t, qCfg.Validate(), "queue size must be positive")

qCfg = NewDefaultQueueSettings()
qCfg = NewDefaultQueueConfig()
qCfg.NumConsumers = 0

assert.EqualError(t, qCfg.Validate(), "number of queue consumers must be positive")
Expand All @@ -193,13 +193,13 @@ func TestQueueSettings_Validate(t *testing.T) {
}

func TestQueueRetryWithDisabledQueue(t *testing.T) {
qs := NewDefaultQueueSettings()
qs := NewDefaultQueueConfig()
qs.Enabled = false
set := exportertest.NewNopCreateSettings()
logger, observed := observer.New(zap.ErrorLevel)
set.Logger = zap.New(logger)
be, err := newBaseExporter(set, component.DataTypeLogs, false, nil, nil, newObservabilityConsumerSender,
WithQueue(qs))
WithQueueConfig(qs))
require.NoError(t, err)
require.NoError(t, be.Start(context.Background(), componenttest.NewNopHost()))
ocs := be.obsrepSender.(*observabilityConsumerSender)
Expand All @@ -220,7 +220,7 @@ func TestQueueFailedRequestDropped(t *testing.T) {
set := exportertest.NewNopCreateSettings()
logger, observed := observer.New(zap.ErrorLevel)
set.Logger = zap.New(logger)
be, err := newBaseExporter(set, component.DataTypeLogs, false, nil, nil, newNoopObsrepSender, WithQueue(NewDefaultQueueSettings()))
be, err := newBaseExporter(set, component.DataTypeLogs, false, nil, nil, newNoopObsrepSender, WithQueueConfig(NewDefaultQueueConfig()))
require.NoError(t, err)
require.NoError(t, be.Start(context.Background(), componenttest.NewNopHost()))
mockR := newMockRequest(2, errors.New("some error"))
Expand All @@ -236,12 +236,12 @@ func TestQueuedRetryPersistenceEnabled(t *testing.T) {
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })

qCfg := NewDefaultQueueSettings()
qCfg := NewDefaultQueueConfig()
storageID := component.NewIDWithName("file_storage", "storage")
qCfg.StorageID = &storageID // enable persistence
rCfg := configretry.NewDefaultBackOffConfig()
set := exporter.CreateSettings{ID: defaultID, TelemetrySettings: tt.TelemetrySettings(), BuildInfo: component.NewDefaultBuildInfo()}
be, err := newBaseExporter(set, "", false, nil, nil, newObservabilityConsumerSender, WithRetry(rCfg), WithQueue(qCfg))
be, err := newBaseExporter(set, "", false, nil, nil, newObservabilityConsumerSender, WithRetry(rCfg), WithQueueConfig(qCfg))
require.NoError(t, err)

var extensions = map[component.ID]component.Component{
Expand All @@ -260,12 +260,12 @@ func TestQueuedRetryPersistenceEnabledStorageError(t *testing.T) {
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })

qCfg := NewDefaultQueueSettings()
qCfg := NewDefaultQueueConfig()
storageID := component.NewIDWithName("file_storage", "storage")
qCfg.StorageID = &storageID // enable persistence
rCfg := configretry.NewDefaultBackOffConfig()
set := exporter.CreateSettings{ID: defaultID, TelemetrySettings: tt.TelemetrySettings(), BuildInfo: component.NewDefaultBuildInfo()}
be, err := newBaseExporter(set, "", false, mockRequestMarshaler, mockRequestUnmarshaler(&mockRequest{}), newObservabilityConsumerSender, WithRetry(rCfg), WithQueue(qCfg))
be, err := newBaseExporter(set, "", false, mockRequestMarshaler, mockRequestUnmarshaler(&mockRequest{}), newObservabilityConsumerSender, WithRetry(rCfg), WithQueueConfig(qCfg))
require.NoError(t, err)

var extensions = map[component.ID]component.Component{
Expand All @@ -278,7 +278,7 @@ func TestQueuedRetryPersistenceEnabledStorageError(t *testing.T) {
}

func TestQueuedRetryPersistentEnabled_NoDataLossOnShutdown(t *testing.T) {
qCfg := NewDefaultQueueSettings()
qCfg := NewDefaultQueueConfig()
qCfg.NumConsumers = 1
storageID := component.NewIDWithName("file_storage", "storage")
qCfg.StorageID = &storageID // enable persistence to ensure data is re-queued on shutdown
Expand All @@ -289,7 +289,7 @@ func TestQueuedRetryPersistentEnabled_NoDataLossOnShutdown(t *testing.T) {

mockReq := newErrorRequest()
be, err := newBaseExporter(defaultSettings, "", false, mockRequestMarshaler, mockRequestUnmarshaler(mockReq),
newNoopObsrepSender, WithRetry(rCfg), WithQueue(qCfg))
newNoopObsrepSender, WithRetry(rCfg), WithQueueConfig(qCfg))
require.NoError(t, err)

var extensions = map[component.ID]component.Component{
Expand All @@ -313,7 +313,7 @@ func TestQueuedRetryPersistentEnabled_NoDataLossOnShutdown(t *testing.T) {
// start the exporter again replacing the preserved mockRequest in the unmarshaler with a new one that doesn't fail.
replacedReq := newMockRequest(1, nil)
be, err = newBaseExporter(defaultSettings, "", false, mockRequestMarshaler, mockRequestUnmarshaler(replacedReq),
newNoopObsrepSender, WithRetry(rCfg), WithQueue(qCfg))
newNoopObsrepSender, WithRetry(rCfg), WithQueueConfig(qCfg))
require.NoError(t, err)
require.NoError(t, be.Start(context.Background(), host))
t.Cleanup(func() { require.NoError(t, be.Shutdown(context.Background())) })
Expand All @@ -323,7 +323,7 @@ func TestQueuedRetryPersistentEnabled_NoDataLossOnShutdown(t *testing.T) {
}

func TestQueueSenderNoStartShutdown(t *testing.T) {
qs := newQueueSender(NewDefaultQueueSettings(), exportertest.NewNopCreateSettings(), "", nil, nil, nil)
qs := newQueueSender(NewDefaultQueueConfig(), exportertest.NewNopCreateSettings(), "", nil, nil, nil)
assert.NoError(t, qs.Shutdown(context.Background()))
}

Expand Down
Loading
Loading