From 936ef35a88bea4acfd100dfbd34fd62127bef53a Mon Sep 17 00:00:00 2001 From: Sindy Li Date: Wed, 18 Dec 2024 15:26:12 -0800 Subject: [PATCH] [chore][exporterhelper] Fix feature gate setting for unit tests. (#11927) #### Description Before fix: `defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)` This means run `setFeatureGateForTest` in the end. After fix: `defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)()` This means: run `setFeatureGateForTest` immediately but execute the function returned from `setFeatureGateForTest` in the end. #### Link to tracking issue Fixes # #### Testing #### Documentation --- .../internal/base_exporter_test.go | 8 ++++---- .../internal/batch_sender_test.go | 18 +++++++++--------- .../internal/queue_sender_test.go | 18 +++++++++--------- .../internal/retry_sender_test.go | 6 +++--- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/exporter/exporterhelper/internal/base_exporter_test.go b/exporter/exporterhelper/internal/base_exporter_test.go index 886f0ac7b58..cca72cd8272 100644 --- a/exporter/exporterhelper/internal/base_exporter_test.go +++ b/exporter/exporterhelper/internal/base_exporter_test.go @@ -40,7 +40,7 @@ func newNoopObsrepSender(*ObsReport) RequestSender { func TestBaseExporter(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() be, err := NewBaseExporter(defaultSettings, defaultSignal, newNoopObsrepSender) require.NoError(t, err) require.NoError(t, be.Start(context.Background(), componenttest.NewNopHost())) @@ -54,7 +54,7 @@ func TestBaseExporter(t *testing.T) { func TestBaseExporterWithOptions(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() want := errors.New("my error") be, err := NewBaseExporter( defaultSettings, defaultSignal, newNoopObsrepSender, @@ -74,7 +74,7 @@ func TestBaseExporterWithOptions(t *testing.T) { func TestQueueOptionsWithRequestExporter(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() bs, err := NewBaseExporter(exportertest.NewNopSettings(), defaultSignal, newNoopObsrepSender, WithRetry(configretry.NewDefaultBackOffConfig())) require.NoError(t, err) @@ -98,7 +98,7 @@ func TestQueueOptionsWithRequestExporter(t *testing.T) { func TestBaseExporterLogging(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() set := exportertest.NewNopSettings() logger, observed := observer.New(zap.DebugLevel) set.Logger = zap.New(logger) diff --git a/exporter/exporterhelper/internal/batch_sender_test.go b/exporter/exporterhelper/internal/batch_sender_test.go index cc9fb4f07c6..061aeea89a3 100644 --- a/exporter/exporterhelper/internal/batch_sender_test.go +++ b/exporter/exporterhelper/internal/batch_sender_test.go @@ -225,7 +225,7 @@ func TestBatchSender_MergeOrSplit(t *testing.T) { func TestBatchSender_Shutdown(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() batchCfg := exporterbatcher.NewDefaultConfig() batchCfg.MinSizeItems = 10 be := queueBatchExporter(t, WithBatcher(batchCfg)) @@ -253,7 +253,7 @@ func TestBatchSender_Shutdown(t *testing.T) { func TestBatchSender_Disabled(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() cfg := exporterbatcher.NewDefaultConfig() cfg.Enabled = false cfg.MaxSizeItems = 5 @@ -311,7 +311,7 @@ func TestBatchSender_Disabled(t *testing.T) { func TestBatchSender_PostShutdown(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() be, err := NewBaseExporter(defaultSettings, defaultSignal, newNoopObsrepSender, WithBatcher(exporterbatcher.NewDefaultConfig())) require.NotNil(t, be) @@ -432,7 +432,7 @@ func TestBatchSender_ConcurrencyLimitReached(t *testing.T) { func TestBatchSender_BatchBlocking(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() bCfg := exporterbatcher.NewDefaultConfig() bCfg.MinSizeItems = 3 be, err := NewBaseExporter(defaultSettings, defaultSignal, newNoopObsrepSender, @@ -469,7 +469,7 @@ func TestBatchSender_BatchBlocking(t *testing.T) { func TestBatchSender_BatchCancelled(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() bCfg := exporterbatcher.NewDefaultConfig() bCfg.MinSizeItems = 2 be, err := NewBaseExporter(defaultSettings, defaultSignal, newNoopObsrepSender, @@ -511,7 +511,7 @@ func TestBatchSender_BatchCancelled(t *testing.T) { func TestBatchSender_DrainActiveRequests(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() bCfg := exporterbatcher.NewDefaultConfig() bCfg.MinSizeItems = 2 be, err := NewBaseExporter(defaultSettings, defaultSignal, newNoopObsrepSender, @@ -551,7 +551,7 @@ func TestBatchSender_DrainActiveRequests(t *testing.T) { func TestBatchSender_UnstartedShutdown(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() be, err := NewBaseExporter(defaultSettings, defaultSignal, newNoopObsrepSender, WithBatcher(exporterbatcher.NewDefaultConfig())) require.NoError(t, err) @@ -614,7 +614,7 @@ func TestBatchSender_UnstartedShutdown(t *testing.T) { func TestBatchSenderWithTimeout(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() bCfg := exporterbatcher.NewDefaultConfig() bCfg.MinSizeItems = 10 tCfg := NewDefaultTimeoutConfig() @@ -709,7 +709,7 @@ func TestBatchSenderWithTimeout(t *testing.T) { func TestBatchSenderTimerFlush(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() if runtime.GOOS == "windows" { t.Skip("skipping flaky test on Windows, see https://github.com/open-telemetry/opentelemetry-collector/issues/10802") } diff --git a/exporter/exporterhelper/internal/queue_sender_test.go b/exporter/exporterhelper/internal/queue_sender_test.go index 95b536cc050..138dc4ebabe 100644 --- a/exporter/exporterhelper/internal/queue_sender_test.go +++ b/exporter/exporterhelper/internal/queue_sender_test.go @@ -30,7 +30,7 @@ import ( func TestQueuedRetry_StopWhileWaiting(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() qCfg := NewDefaultQueueConfig() qCfg.NumConsumers = 1 rCfg := configretry.NewDefaultBackOffConfig() @@ -245,7 +245,7 @@ func TestQueuedRetryHappyPath(t *testing.T) { func TestQueuedRetry_QueueMetricsReported(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() dataTypes := []pipeline.Signal{pipeline.SignalLogs, pipeline.SignalTraces, pipeline.SignalMetrics} for _, dataType := range dataTypes { tt, err := componenttest.SetupTelemetry(defaultID) @@ -284,7 +284,7 @@ func TestQueuedRetry_QueueMetricsReported(t *testing.T) { func TestNoCancellationContext(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() deadline := time.Now().Add(1 * time.Second) ctx, cancelFunc := context.WithDeadline(context.Background(), deadline) cancelFunc() @@ -308,7 +308,7 @@ func TestNoCancellationContext(t *testing.T) { func TestQueueConfig_Validate(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() qCfg := NewDefaultQueueConfig() require.NoError(t, qCfg.Validate()) @@ -365,7 +365,7 @@ func TestQueueRetryWithDisabledQueue(t *testing.T) { }, ) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() set := exportertest.NewNopSettings() logger, observed := observer.New(zap.ErrorLevel) set.Logger = zap.New(logger) @@ -395,7 +395,7 @@ func TestQueueRetryWithDisabledQueue(t *testing.T) { func TestQueueFailedRequestDropped(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() set := exportertest.NewNopSettings() logger, observed := observer.New(zap.ErrorLevel) set.Logger = zap.New(logger) @@ -419,7 +419,7 @@ func TestQueueFailedRequestDropped(t *testing.T) { func TestQueuedRetryPersistenceEnabled(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() tt, err := componenttest.SetupTelemetry(defaultID) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) }) @@ -452,7 +452,7 @@ func TestQueuedRetryPersistenceEnabled(t *testing.T) { func TestQueuedRetryPersistenceEnabledStorageError(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() storageError := errors.New("could not get storage client") tt, err := componenttest.SetupTelemetry(defaultID) require.NoError(t, err) @@ -539,7 +539,7 @@ func TestQueuedRetryPersistentEnabled_NoDataLossOnShutdown(t *testing.T) { func TestQueueSenderNoStartShutdown(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() queue := queue.NewBoundedMemoryQueue[internal.Request](queue.MemoryQueueSettings[internal.Request]{}) set := exportertest.NewNopSettings() obsrep, err := NewExporter(ObsReportSettings{ diff --git a/exporter/exporterhelper/internal/retry_sender_test.go b/exporter/exporterhelper/internal/retry_sender_test.go index e9ff0dbd94b..b990d399f28 100644 --- a/exporter/exporterhelper/internal/retry_sender_test.go +++ b/exporter/exporterhelper/internal/retry_sender_test.go @@ -279,7 +279,7 @@ func TestQueuedRetry_RetryOnError(t *testing.T) { func TestQueueRetryWithNoQueue(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() rCfg := configretry.NewDefaultBackOffConfig() rCfg.MaxElapsedTime = time.Nanosecond // fail fast be, err := NewBaseExporter(exportertest.NewNopSettings(), pipeline.SignalLogs, newObservabilityConsumerSender, WithRetry(rCfg)) @@ -304,7 +304,7 @@ func TestQueueRetryWithNoQueue(t *testing.T) { func TestQueueRetryWithDisabledRetires(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() rCfg := configretry.NewDefaultBackOffConfig() rCfg.Enabled = false set := exportertest.NewNopSettings() @@ -335,7 +335,7 @@ func TestQueueRetryWithDisabledRetires(t *testing.T) { func TestRetryWithContextTimeout(t *testing.T) { runTest := func(testName string, enableQueueBatcher bool) { t.Run(testName, func(t *testing.T) { - defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher) + defer setFeatureGateForTest(t, usePullingBasedExporterQueueBatcher, enableQueueBatcher)() const testTimeout = 10 * time.Second rCfg := configretry.NewDefaultBackOffConfig()