From f067287fa8a4c3b64be6750d1e5643fe66eafd8e Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sat, 13 Apr 2024 04:25:15 +0530 Subject: [PATCH 01/69] add badger integration test Signed-off-by: Harshvir Potpose --- .github/workflows/ci-badger.yaml | 15 +++- cmd/jaeger/badger_config.yaml | 2 + .../internal/integration/badger_test.go | 70 +++++++++++++++++++ .../storagereceiver/testdata/config.yaml | 2 +- plugin/storage/badger/spanstore/reader.go | 4 ++ plugin/storage/badger/spanstore/writer.go | 4 ++ 6 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 cmd/jaeger/internal/integration/badger_test.go diff --git a/.github/workflows/ci-badger.yaml b/.github/workflows/ci-badger.yaml index 4f43057ce6d..8be9465152e 100644 --- a/.github/workflows/ci-badger.yaml +++ b/.github/workflows/ci-badger.yaml @@ -18,6 +18,9 @@ permissions: # added using https://github.com/step-security/secure-workflows jobs: badger: runs-on: ubuntu-latest + strategy: + matrix: + version: [v1, v2] steps: - name: Harden Runner uses: step-security/harden-runner@63c24ba6bd7ba022e95695ff85de572c04a18142 # v2.7.0 @@ -31,7 +34,17 @@ jobs: go-version: 1.22.x - name: Run Badger storage integration tests - run: make badger-storage-integration-test + run: | + case ${{ matrix.version }} in + v1) + SPAN_STORAGE_TYPE=badger \ + make badger-storage-integration-test + ;; + v2) + STORAGE=badger \ + make jaeger-v2-storage-integration-test + ;; + esac - name: Setup CODECOV_TOKEN uses: ./.github/actions/setup-codecov diff --git a/cmd/jaeger/badger_config.yaml b/cmd/jaeger/badger_config.yaml index 950be451250..1f898fe8b06 100644 --- a/cmd/jaeger/badger_config.yaml +++ b/cmd/jaeger/badger_config.yaml @@ -20,12 +20,14 @@ extensions: ephemeral: false maintenance_interval: 5 metrics_update_interval: 10 + span_store_ttl: 72h badger_archive: directory_key: "/tmp/jaeger_archive/" directory_value: "/tmp/jaeger_archive/" ephemeral: false maintenance_interval: 5 metrics_update_interval: 10 + span_store_ttl: 72h receivers: otlp: diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go new file mode 100644 index 00000000000..82b32559c38 --- /dev/null +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -0,0 +1,70 @@ +package integration + +import ( + "testing" + + "github.com/jaegertracing/jaeger/pkg/metrics" + "github.com/jaegertracing/jaeger/pkg/testutils" + "github.com/jaegertracing/jaeger/plugin/storage/badger" + "github.com/jaegertracing/jaeger/plugin/storage/integration" + "github.com/stretchr/testify/require" + "go.uber.org/zap" +) + +type BadgerStorageIntegration struct { + E2EStorageIntegration + logger *zap.Logger + factory *badger.Factory +} + +func (s *BadgerStorageIntegration) initialize(t *testing.T) { + s.factory = badger.NewFactory() + + err := s.factory.Initialize(metrics.NullFactory, zap.NewNop()) + require.NoError(t, err) + + s.SpanWriter, err = s.factory.CreateSpanWriter() + require.NoError(t, err) + + s.SpanReader, err = s.factory.CreateSpanReader() + require.NoError(t, err) + + s.SamplingStore, err = s.factory.CreateSamplingStore(0) + require.NoError(t, err) + + s.Refresh = func(_ *testing.T) {} + s.CleanUp = s.cleanUp + + s.logger, _ = testutils.NewLogger() + + // TODO: remove this badger supports returning spanKind from GetOperations + s.GetOperationsMissingSpanKind = true + s.SkipArchiveTest = true +} + +func (s *BadgerStorageIntegration) Close() error { + return s.factory.Close() +} + +func (s *BadgerStorageIntegration) cleanUp(t *testing.T) { + err := s.Close() + require.NoError(t, err) + s.initialize(t) +} + +func TestBadgerStorage(t *testing.T) { + integration.SkipUnlessEnv(t, "badger") + + s := &BadgerStorageIntegration{} + s.ConfigFile = "cmd/jaeger/badger_config.yaml" + s.SkipBinaryAttrs = true + + s.initialize(t) + s.e2eInitialize(t) + t.Cleanup(func() { + s.e2eCleanUp(t) + s.Close() + + }) + s.RunAll(t) +} diff --git a/cmd/jaeger/internal/integration/receivers/storagereceiver/testdata/config.yaml b/cmd/jaeger/internal/integration/receivers/storagereceiver/testdata/config.yaml index a7c0da9218d..e590e8f1694 100644 --- a/cmd/jaeger/internal/integration/receivers/storagereceiver/testdata/config.yaml +++ b/cmd/jaeger/internal/integration/receivers/storagereceiver/testdata/config.yaml @@ -3,4 +3,4 @@ jaeger_storage_receiver/defaults: trace_storage: storage jaeger_storage_receiver/filled: trace_storage: storage - pull_interval: 2s \ No newline at end of file + pull_interval: 2s diff --git a/plugin/storage/badger/spanstore/reader.go b/plugin/storage/badger/spanstore/reader.go index f33dce1136d..10b00e66851 100644 --- a/plugin/storage/badger/spanstore/reader.go +++ b/plugin/storage/badger/spanstore/reader.go @@ -565,6 +565,10 @@ func (r *TraceReader) scanIndexKeys(indexKeyValue []byte, plan *executionPlan) ( return indexResults, err } +func (s *TraceReader) Close() error { + return nil +} + // scanFunction compares the index name as well as the time range in the index key func scanFunction(it *badger.Iterator, indexPrefix []byte, timeBytesEnd []byte) bool { if it.Valid() { diff --git a/plugin/storage/badger/spanstore/writer.go b/plugin/storage/badger/spanstore/writer.go index cd9aa56e19c..2aac77169d7 100644 --- a/plugin/storage/badger/spanstore/writer.go +++ b/plugin/storage/badger/spanstore/writer.go @@ -158,6 +158,10 @@ func (w *SpanWriter) createTraceEntry(span *model.Span, startTime, expireTime ui return e, nil } +func (w *SpanWriter) Close() error { + return nil +} + func createTraceKV(span *model.Span, encodingType byte, startTime uint64) ([]byte, []byte, error) { // TODO Add Hash for Zipkin compatibility? From d64d43d187266fcb00faee4fca0fca7133bbccc0 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sat, 13 Apr 2024 04:36:20 +0530 Subject: [PATCH 02/69] fix lint Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/badger_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go index 82b32559c38..7e34d89d50d 100644 --- a/cmd/jaeger/internal/integration/badger_test.go +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -1,14 +1,18 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + package integration import ( "testing" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + "github.com/jaegertracing/jaeger/pkg/metrics" "github.com/jaegertracing/jaeger/pkg/testutils" "github.com/jaegertracing/jaeger/plugin/storage/badger" "github.com/jaegertracing/jaeger/plugin/storage/integration" - "github.com/stretchr/testify/require" - "go.uber.org/zap" ) type BadgerStorageIntegration struct { @@ -64,7 +68,6 @@ func TestBadgerStorage(t *testing.T) { t.Cleanup(func() { s.e2eCleanUp(t) s.Close() - }) s.RunAll(t) } From 9f42757e53827f79c557819b43196c752e3a9a56 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sun, 14 Apr 2024 17:05:21 +0530 Subject: [PATCH 03/69] use SpanWriter and SpanReader from e2eInitialize() functionf Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/badger_test.go | 9 --------- plugin/storage/badger/spanstore/reader.go | 4 ---- plugin/storage/badger/spanstore/writer.go | 4 ---- 3 files changed, 17 deletions(-) diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go index 7e34d89d50d..c86f618d23a 100644 --- a/cmd/jaeger/internal/integration/badger_test.go +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -27,15 +27,6 @@ func (s *BadgerStorageIntegration) initialize(t *testing.T) { err := s.factory.Initialize(metrics.NullFactory, zap.NewNop()) require.NoError(t, err) - s.SpanWriter, err = s.factory.CreateSpanWriter() - require.NoError(t, err) - - s.SpanReader, err = s.factory.CreateSpanReader() - require.NoError(t, err) - - s.SamplingStore, err = s.factory.CreateSamplingStore(0) - require.NoError(t, err) - s.Refresh = func(_ *testing.T) {} s.CleanUp = s.cleanUp diff --git a/plugin/storage/badger/spanstore/reader.go b/plugin/storage/badger/spanstore/reader.go index 10b00e66851..f33dce1136d 100644 --- a/plugin/storage/badger/spanstore/reader.go +++ b/plugin/storage/badger/spanstore/reader.go @@ -565,10 +565,6 @@ func (r *TraceReader) scanIndexKeys(indexKeyValue []byte, plan *executionPlan) ( return indexResults, err } -func (s *TraceReader) Close() error { - return nil -} - // scanFunction compares the index name as well as the time range in the index key func scanFunction(it *badger.Iterator, indexPrefix []byte, timeBytesEnd []byte) bool { if it.Valid() { diff --git a/plugin/storage/badger/spanstore/writer.go b/plugin/storage/badger/spanstore/writer.go index 2aac77169d7..cd9aa56e19c 100644 --- a/plugin/storage/badger/spanstore/writer.go +++ b/plugin/storage/badger/spanstore/writer.go @@ -158,10 +158,6 @@ func (w *SpanWriter) createTraceEntry(span *model.Span, startTime, expireTime ui return e, nil } -func (w *SpanWriter) Close() error { - return nil -} - func createTraceKV(span *model.Span, encodingType byte, startTime uint64) ([]byte, []byte, error) { // TODO Add Hash for Zipkin compatibility? From ac8dfad35305db8edc02f55833c05ca23cc5fea1 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> Date: Sun, 14 Apr 2024 20:57:17 +0530 Subject: [PATCH 04/69] Update cmd/jaeger/internal/integration/badger_test.go Co-authored-by: Yuri Shkuro Signed-off-by: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> --- cmd/jaeger/internal/integration/badger_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go index c86f618d23a..b8ee000e80a 100644 --- a/cmd/jaeger/internal/integration/badger_test.go +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -50,9 +50,10 @@ func (s *BadgerStorageIntegration) cleanUp(t *testing.T) { func TestBadgerStorage(t *testing.T) { integration.SkipUnlessEnv(t, "badger") - s := &BadgerStorageIntegration{} - s.ConfigFile = "cmd/jaeger/badger_config.yaml" - s.SkipBinaryAttrs = true + s := &BadgerStorageIntegration{ + ConfigFile: "cmd/jaeger/badger_config.yaml" + SkipBinaryAttrs: true, + } s.initialize(t) s.e2eInitialize(t) From 8555ebd5566487a4636659b99ce08eb7a27f2cec Mon Sep 17 00:00:00 2001 From: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> Date: Sun, 14 Apr 2024 20:57:29 +0530 Subject: [PATCH 05/69] Update cmd/jaeger/internal/integration/badger_test.go Co-authored-by: Yuri Shkuro Signed-off-by: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> --- cmd/jaeger/internal/integration/badger_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go index b8ee000e80a..2d871becd2f 100644 --- a/cmd/jaeger/internal/integration/badger_test.go +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -32,7 +32,7 @@ func (s *BadgerStorageIntegration) initialize(t *testing.T) { s.logger, _ = testutils.NewLogger() - // TODO: remove this badger supports returning spanKind from GetOperations + // TODO: remove this once badger supports returning spanKind from GetOperations s.GetOperationsMissingSpanKind = true s.SkipArchiveTest = true } From c6f841e4afbd0a2d2c961b48766d9adbb36b9211 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Mon, 15 Apr 2024 00:06:28 +0530 Subject: [PATCH 06/69] fix Signed-off-by: Harshvir Potpose --- .github/workflows/ci-badger.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/ci-badger.yaml b/.github/workflows/ci-badger.yaml index 8be9465152e..b6b7cd22f32 100644 --- a/.github/workflows/ci-badger.yaml +++ b/.github/workflows/ci-badger.yaml @@ -37,7 +37,6 @@ jobs: run: | case ${{ matrix.version }} in v1) - SPAN_STORAGE_TYPE=badger \ make badger-storage-integration-test ;; v2) From b4bd719dc3bcd0e09f43bac479c1fbb4dd2fc36c Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Mon, 15 Apr 2024 00:06:57 +0530 Subject: [PATCH 07/69] use badger ephemeral Signed-off-by: Harshvir Potpose --- cmd/jaeger/badger_config.yaml | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/cmd/jaeger/badger_config.yaml b/cmd/jaeger/badger_config.yaml index 1f898fe8b06..12031e8b516 100644 --- a/cmd/jaeger/badger_config.yaml +++ b/cmd/jaeger/badger_config.yaml @@ -15,16 +15,12 @@ extensions: jaeger_storage: badger: badger_main: - directory_key: "/tmp/jaeger/" - directory_value: "/tmp/jaeger/" - ephemeral: false + ephemeral: true maintenance_interval: 5 metrics_update_interval: 10 span_store_ttl: 72h badger_archive: - directory_key: "/tmp/jaeger_archive/" - directory_value: "/tmp/jaeger_archive/" - ephemeral: false + ephemeral: true maintenance_interval: 5 metrics_update_interval: 10 span_store_ttl: 72h From 0a771dfdade03832be90f27b1589df0703ea19f1 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Mon, 15 Apr 2024 00:07:39 +0530 Subject: [PATCH 08/69] fix Signed-off-by: Harshvir Potpose --- .../internal/integration/badger_test.go | 30 +++++-------------- .../internal/integration/integration.go | 13 ++++---- 2 files changed, 14 insertions(+), 29 deletions(-) diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go index 2d871becd2f..471c25b821b 100644 --- a/cmd/jaeger/internal/integration/badger_test.go +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -6,60 +6,44 @@ package integration import ( "testing" - "github.com/stretchr/testify/require" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/pkg/metrics" - "github.com/jaegertracing/jaeger/pkg/testutils" - "github.com/jaegertracing/jaeger/plugin/storage/badger" "github.com/jaegertracing/jaeger/plugin/storage/integration" ) type BadgerStorageIntegration struct { E2EStorageIntegration - logger *zap.Logger - factory *badger.Factory + logger *zap.Logger } func (s *BadgerStorageIntegration) initialize(t *testing.T) { - s.factory = badger.NewFactory() - - err := s.factory.Initialize(metrics.NullFactory, zap.NewNop()) - require.NoError(t, err) + s.e2eInitialize(t) s.Refresh = func(_ *testing.T) {} s.CleanUp = s.cleanUp - s.logger, _ = testutils.NewLogger() + s.logger = zap.NewNop() // TODO: remove this once badger supports returning spanKind from GetOperations s.GetOperationsMissingSpanKind = true s.SkipArchiveTest = true } -func (s *BadgerStorageIntegration) Close() error { - return s.factory.Close() -} - func (s *BadgerStorageIntegration) cleanUp(t *testing.T) { - err := s.Close() - require.NoError(t, err) + s.e2eCleanUp(t) s.initialize(t) } func TestBadgerStorage(t *testing.T) { integration.SkipUnlessEnv(t, "badger") - s := &BadgerStorageIntegration{ - ConfigFile: "cmd/jaeger/badger_config.yaml" - SkipBinaryAttrs: true, - } + s := &BadgerStorageIntegration{} + s.ConfigFile = "cmd/jaeger/badger_config.yaml" + s.SkipBinaryAttrs = true s.initialize(t) - s.e2eInitialize(t) t.Cleanup(func() { s.e2eCleanUp(t) - s.Close() }) s.RunAll(t) } diff --git a/cmd/jaeger/internal/integration/integration.go b/cmd/jaeger/internal/integration/integration.go index 67ecd609508..9371b2d61db 100644 --- a/cmd/jaeger/internal/integration/integration.go +++ b/cmd/jaeger/internal/integration/integration.go @@ -30,7 +30,8 @@ const otlpPort = 4317 // (e.g. close remote-storage) type E2EStorageIntegration struct { integration.StorageIntegration - ConfigFile string + ConfigFile string + jaegerBinary *exec.Cmd } // e2eInitialize starts the Jaeger-v2 collector with the provided config file, @@ -39,7 +40,7 @@ type E2EStorageIntegration struct { func (s *E2EStorageIntegration) e2eInitialize(t *testing.T) { logger, _ := testutils.NewLogger() - cmd := exec.Cmd{ + s.jaegerBinary = &exec.Cmd{ Path: "./cmd/jaeger/jaeger", Args: []string{"jaeger", "--config", s.ConfigFile}, // Change the working directory to the root of this project @@ -49,10 +50,8 @@ func (s *E2EStorageIntegration) e2eInitialize(t *testing.T) { Stdout: os.Stderr, Stderr: os.Stderr, } - require.NoError(t, cmd.Start()) - t.Cleanup(func() { - require.NoError(t, cmd.Process.Kill()) - }) + + require.NoError(t, s.jaegerBinary.Start()) var err error s.SpanWriter, err = createSpanWriter(logger, otlpPort) @@ -66,4 +65,6 @@ func (s *E2EStorageIntegration) e2eInitialize(t *testing.T) { func (s *E2EStorageIntegration) e2eCleanUp(t *testing.T) { require.NoError(t, s.SpanReader.(io.Closer).Close()) require.NoError(t, s.SpanWriter.(io.Closer).Close()) + require.NoError(t, s.jaegerBinary.Process.Kill()) + } From 04d6b5532c15b84291b8796fca5c0a4332711821 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Mon, 15 Apr 2024 00:19:25 +0530 Subject: [PATCH 09/69] fix lint Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/integration.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/integration.go b/cmd/jaeger/internal/integration/integration.go index 9371b2d61db..cea588739f3 100644 --- a/cmd/jaeger/internal/integration/integration.go +++ b/cmd/jaeger/internal/integration/integration.go @@ -66,5 +66,4 @@ func (s *E2EStorageIntegration) e2eCleanUp(t *testing.T) { require.NoError(t, s.SpanReader.(io.Closer).Close()) require.NoError(t, s.SpanWriter.(io.Closer).Close()) require.NoError(t, s.jaegerBinary.Process.Kill()) - } From 343a648e93fd3d331db22f9ced4cf9c22af266a0 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Mon, 15 Apr 2024 01:15:18 +0530 Subject: [PATCH 10/69] assigned static vars when creating struct Signed-off-by: Harshvir Potpose --- .../internal/integration/badger_test.go | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go index 471c25b821b..f55b9f9b3cf 100644 --- a/cmd/jaeger/internal/integration/badger_test.go +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -23,10 +23,6 @@ func (s *BadgerStorageIntegration) initialize(t *testing.T) { s.CleanUp = s.cleanUp s.logger = zap.NewNop() - - // TODO: remove this once badger supports returning spanKind from GetOperations - s.GetOperationsMissingSpanKind = true - s.SkipArchiveTest = true } func (s *BadgerStorageIntegration) cleanUp(t *testing.T) { @@ -37,9 +33,19 @@ func (s *BadgerStorageIntegration) cleanUp(t *testing.T) { func TestBadgerStorage(t *testing.T) { integration.SkipUnlessEnv(t, "badger") - s := &BadgerStorageIntegration{} - s.ConfigFile = "cmd/jaeger/badger_config.yaml" - s.SkipBinaryAttrs = true + s := &BadgerStorageIntegration{ + E2EStorageIntegration: E2EStorageIntegration{ + ConfigFile: "cmd/jaeger/badger_config.yaml", + StorageIntegration: integration.StorageIntegration{ + SkipBinaryAttrs: true, + + // TODO: remove this once badger supports returning spanKind from GetOperations + // Cf https://github.com/jaegertracing/jaeger/issues/1922 + SkipArchiveTest: true, + GetOperationsMissingSpanKind: true, + }, + }, + } s.initialize(t) t.Cleanup(func() { From 3795f7077fc92325b3f17e0fc507e2dfb9f5b16c Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Mon, 15 Apr 2024 01:22:24 +0530 Subject: [PATCH 11/69] fix Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/badger_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go index f55b9f9b3cf..af19801c330 100644 --- a/cmd/jaeger/internal/integration/badger_test.go +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -38,10 +38,10 @@ func TestBadgerStorage(t *testing.T) { ConfigFile: "cmd/jaeger/badger_config.yaml", StorageIntegration: integration.StorageIntegration{ SkipBinaryAttrs: true, + SkipArchiveTest: true, // TODO: remove this once badger supports returning spanKind from GetOperations // Cf https://github.com/jaegertracing/jaeger/issues/1922 - SkipArchiveTest: true, GetOperationsMissingSpanKind: true, }, }, From 968f0109718fa1229e5287ccfc8fd6d8eeea83b2 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Mon, 15 Apr 2024 03:56:47 +0530 Subject: [PATCH 12/69] rm refresh Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/badger_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go index af19801c330..3319a198a02 100644 --- a/cmd/jaeger/internal/integration/badger_test.go +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -19,9 +19,7 @@ type BadgerStorageIntegration struct { func (s *BadgerStorageIntegration) initialize(t *testing.T) { s.e2eInitialize(t) - s.Refresh = func(_ *testing.T) {} s.CleanUp = s.cleanUp - s.logger = zap.NewNop() } From 28af7df0124cc2e3573392b0947d67bfca2069c9 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sat, 20 Apr 2024 14:40:53 +0530 Subject: [PATCH 13/69] use normal badger storage Signed-off-by: Harshvir Potpose --- cmd/jaeger/badger_config.yaml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/cmd/jaeger/badger_config.yaml b/cmd/jaeger/badger_config.yaml index 12031e8b516..39f5f633b30 100644 --- a/cmd/jaeger/badger_config.yaml +++ b/cmd/jaeger/badger_config.yaml @@ -14,13 +14,17 @@ extensions: jaeger_storage: badger: - badger_main: - ephemeral: true + badger_archive: + directory_key: /tmp/jaeger_archive/ + directory_value: /tmp/jaeger_archive/ + ephemeral: false maintenance_interval: 5 metrics_update_interval: 10 span_store_ttl: 72h - badger_archive: - ephemeral: true + badger_main: + directory_key: /tmp/jaeger/ + directory_value: /tmp/jaeger/ + ephemeral: false maintenance_interval: 5 metrics_update_interval: 10 span_store_ttl: 72h From cc48bde09511bd83eb89563833cbdfc54bc2980a Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sat, 20 Apr 2024 17:21:12 +0530 Subject: [PATCH 14/69] revert some changes Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/integration.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/jaeger/internal/integration/integration.go b/cmd/jaeger/internal/integration/integration.go index cea588739f3..b40c7334492 100644 --- a/cmd/jaeger/internal/integration/integration.go +++ b/cmd/jaeger/internal/integration/integration.go @@ -30,8 +30,7 @@ const otlpPort = 4317 // (e.g. close remote-storage) type E2EStorageIntegration struct { integration.StorageIntegration - ConfigFile string - jaegerBinary *exec.Cmd + ConfigFile string } // e2eInitialize starts the Jaeger-v2 collector with the provided config file, @@ -40,7 +39,7 @@ type E2EStorageIntegration struct { func (s *E2EStorageIntegration) e2eInitialize(t *testing.T) { logger, _ := testutils.NewLogger() - s.jaegerBinary = &exec.Cmd{ + cmd := &exec.Cmd{ Path: "./cmd/jaeger/jaeger", Args: []string{"jaeger", "--config", s.ConfigFile}, // Change the working directory to the root of this project @@ -50,8 +49,10 @@ func (s *E2EStorageIntegration) e2eInitialize(t *testing.T) { Stdout: os.Stderr, Stderr: os.Stderr, } - - require.NoError(t, s.jaegerBinary.Start()) + require.NoError(t, cmd.Start()) + t.Cleanup(func() { + require.NoError(t, cmd.Process.Kill()) + }) var err error s.SpanWriter, err = createSpanWriter(logger, otlpPort) @@ -65,5 +66,4 @@ func (s *E2EStorageIntegration) e2eInitialize(t *testing.T) { func (s *E2EStorageIntegration) e2eCleanUp(t *testing.T) { require.NoError(t, s.SpanReader.(io.Closer).Close()) require.NoError(t, s.SpanWriter.(io.Closer).Close()) - require.NoError(t, s.jaegerBinary.Process.Kill()) } From a15d977cd694f8c32a4d2b18761294d63a7717e7 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sat, 20 Apr 2024 17:49:21 +0530 Subject: [PATCH 15/69] add badger cleaner extension Signed-off-by: Harshvir Potpose --- .../integration/badgercleaner/README.md | 47 ++++++++++++ .../integration/badgercleaner/config.go | 14 ++++ .../integration/badgercleaner/extension.go | 75 +++++++++++++++++++ .../integration/badgercleaner/factory.go | 31 ++++++++ 4 files changed, 167 insertions(+) create mode 100644 cmd/jaeger/internal/integration/badgercleaner/README.md create mode 100644 cmd/jaeger/internal/integration/badgercleaner/config.go create mode 100644 cmd/jaeger/internal/integration/badgercleaner/extension.go create mode 100644 cmd/jaeger/internal/integration/badgercleaner/factory.go diff --git a/cmd/jaeger/internal/integration/badgercleaner/README.md b/cmd/jaeger/internal/integration/badgercleaner/README.md new file mode 100644 index 00000000000..6def1ce0500 --- /dev/null +++ b/cmd/jaeger/internal/integration/badgercleaner/README.md @@ -0,0 +1,47 @@ +# badger_cleaner + +This module implements an extension that allows purging the Badger storage by exposing a HTTP endpoint for making POST requests. + +The badger_cleaner extension is intended to be used only in tests, providing a way to clear the Badger storage between test runs. Making a POST request to the exposed endpoint will delete all data in the Badger key-value store. + + +```mermaid +flowchart LR + Receiver --> Processor + Processor --> Exporter + JaegerStorageExension -->|"(1) get storage"| Exporter + Exporter -->|"(2) write trace"| Badger + + Badger_e2e_test -->|"(1) POST /purge"| HTTP_endpoint + JaegerStorageExension -->|"(2) getStorage()"| HTTP_endpoint + HTTP_endpoint -.->|"(3) storage.(*Badger).Purge()"| Badger + + subgraph Jaeger Collector + Receiver + Processor + Exporter + + Badger + BadgerCleanerExtension + HTTP_endpoint + subgraph JaegerStorageExension + Badger + end + subgraph BadgerCleanerExtension + HTTP_endpoint + end + end +``` + +# Getting Started + +The following settings are required: + +- `trace_storage` : name of a storage backend defined in `jaegerstorage` extension + +```yaml +extensions: + badger_cleaner: + trace_storage: external-storage +``` + diff --git a/cmd/jaeger/internal/integration/badgercleaner/config.go b/cmd/jaeger/internal/integration/badgercleaner/config.go new file mode 100644 index 00000000000..b2eecb59e3d --- /dev/null +++ b/cmd/jaeger/internal/integration/badgercleaner/config.go @@ -0,0 +1,14 @@ +package badgercleaner + +import ( + "github.com/asaskevich/govalidator" +) + +type Config struct { + TraceStorage string `valid:"required" mapstructure:"trace_storage"` +} + +func (cfg *Config) Validate() error { + _, err := govalidator.ValidateStruct(cfg) + return err +} diff --git a/cmd/jaeger/internal/integration/badgercleaner/extension.go b/cmd/jaeger/internal/integration/badgercleaner/extension.go new file mode 100644 index 00000000000..d069b4d797f --- /dev/null +++ b/cmd/jaeger/internal/integration/badgercleaner/extension.go @@ -0,0 +1,75 @@ +package badgercleaner + +import ( + "context" + "fmt" + "net/http" + + "github.com/gorilla/mux" + "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage" + "github.com/jaegertracing/jaeger/plugin/storage/badger" + "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/extension" +) + +var ( + _ extension.Extension = (*cleaner)(nil) + _ extension.Dependent = (*cleaner)(nil) +) + +const ( + cleanerPort = "9231" + cleanerURL = "/purge" +) + +type cleaner struct { + config *Config +} + +func newBadgerCleaner(config *Config) *cleaner { + return &cleaner{ + config: config, + } +} + +func (e *cleaner) Start(ctx context.Context, host component.Host) error { + storageFactory, err := jaegerstorage.GetStorageFactory(e.config.TraceStorage, host) + if err != nil { + return fmt.Errorf("cannot find storage factory for Badger: %w", err) + } + + purgeBadger := func() error { + badgerFactory := storageFactory.(*badger.Factory) + if err := badgerFactory.Purge(); err != nil { + return fmt.Errorf("error purging Badger storage: %w", err) + } + return nil + } + + cleanerHandler := func(w http.ResponseWriter, r *http.Request) { + if err := purgeBadger(); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + w.WriteHeader(http.StatusCreated) + w.Write([]byte("Purge request processed successfully")) + } + + r := mux.NewRouter() + r.HandleFunc(cleanerURL, cleanerHandler).Methods(http.MethodPost) + go func() { + if err := http.ListenAndServe(":"+cleanerPort, r); err != nil { + fmt.Printf("Error starting cleaner server: %v\n", err) + } + }() + + return nil +} + +func (r *cleaner) Shutdown(ctx context.Context) error { + return nil +} + +func (s *cleaner) Dependencies() []component.ID { + return []component.ID{jaegerstorage.ID} +} diff --git a/cmd/jaeger/internal/integration/badgercleaner/factory.go b/cmd/jaeger/internal/integration/badgercleaner/factory.go new file mode 100644 index 00000000000..6b9485939fe --- /dev/null +++ b/cmd/jaeger/internal/integration/badgercleaner/factory.go @@ -0,0 +1,31 @@ +package badgercleaner + +import ( + "context" + + "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/extension" +) + +// componentType is the name of this extension in configuration. +var componentType = component.MustNewType("badger_cleaner") + +// ID is the identifier of this extension. +var ID = component.NewID(componentType) + +func NewFactory() extension.Factory { + return extension.NewFactory( + componentType, + createDefaultConfig, + createExtension, + component.StabilityLevelBeta, + ) +} + +func createDefaultConfig() component.Config { + return &Config{} +} + +func createExtension(_ context.Context, set extension.CreateSettings, cfg component.Config) (extension.Extension, error) { + return newBadgerCleaner(cfg.(*Config)), nil +} From dc4fefcafef7cce1e573a6d4f30c612666d9379e Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sat, 20 Apr 2024 17:50:02 +0530 Subject: [PATCH 16/69] use badger cleaner extension in the test Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/components.go | 2 ++ .../internal/integration/badger_test.go | 28 +++++++++++++++++-- scripts/prepare-badger-integration-tests.py | 18 ++++++++++++ 3 files changed, 45 insertions(+), 3 deletions(-) create mode 100755 scripts/prepare-badger-integration-tests.py diff --git a/cmd/jaeger/internal/components.go b/cmd/jaeger/internal/components.go index cf9b10a9e71..20e7a39cbbd 100644 --- a/cmd/jaeger/internal/components.go +++ b/cmd/jaeger/internal/components.go @@ -29,6 +29,7 @@ import ( "github.com/jaegertracing/jaeger/cmd/jaeger/internal/exporters/storageexporter" "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerquery" "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage" + "github.com/jaegertracing/jaeger/cmd/jaeger/internal/integration/badgercleaner" ) type builders struct { @@ -60,6 +61,7 @@ func (b builders) build() (otelcol.Factories, error) { // add-ons jaegerquery.NewFactory(), jaegerstorage.NewFactory(), + badgercleaner.NewFactory(), // TODO add adaptive sampling ) if err != nil { diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go index 3319a198a02..cec65d4efbd 100644 --- a/cmd/jaeger/internal/integration/badger_test.go +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -4,13 +4,23 @@ package integration import ( + "net/http" + "os/exec" "testing" "go.uber.org/zap" + "github.com/crossdock/crossdock-go/require" "github.com/jaegertracing/jaeger/plugin/storage/integration" ) +const ( + host = "0.0.0.0" + cleanerPort = "9231" + cleanerURL = "/purge" + cleanerAddr = "http://" + host + ":" + cleanerPort +) + type BadgerStorageIntegration struct { E2EStorageIntegration logger *zap.Logger @@ -24,8 +34,14 @@ func (s *BadgerStorageIntegration) initialize(t *testing.T) { } func (s *BadgerStorageIntegration) cleanUp(t *testing.T) { - s.e2eCleanUp(t) - s.initialize(t) + r, err := http.NewRequest(http.MethodPost, cleanerAddr+cleanerURL, nil) + require.NoError(t, err) + + client := &http.Client{} + + resp, err := client.Do(r) + require.NoError(t, err) + defer resp.Body.Close() } func TestBadgerStorage(t *testing.T) { @@ -44,10 +60,16 @@ func TestBadgerStorage(t *testing.T) { }, }, } - + s.addBadgerCleanerConfig(t) s.initialize(t) t.Cleanup(func() { s.e2eCleanUp(t) }) s.RunAll(t) } + +func (s *BadgerStorageIntegration) addBadgerCleanerConfig(t *testing.T) { + cmd := exec.Command("../../../../scripts/prepare-badger-integration-tests.py") + err := cmd.Run() + require.NoError(t, err) +} diff --git a/scripts/prepare-badger-integration-tests.py b/scripts/prepare-badger-integration-tests.py new file mode 100755 index 00000000000..dd0da86eff8 --- /dev/null +++ b/scripts/prepare-badger-integration-tests.py @@ -0,0 +1,18 @@ +#!/usr/bin/env python3 + +import yaml +import os + +config_file = os.path.join(os.path.dirname(__file__), '../cmd/jaeger/badger_config.yaml') + +with open(config_file, 'r') as f: + config = yaml.safe_load(f) +if 'badger_cleaner' not in config['service']['extensions'] : + config['service']['extensions'].insert(1, 'badger_cleaner') + +config['extensions']['badger_cleaner'] = { + 'trace_storage': 'badger_main' +} + +with open(config_file, 'w') as f: + yaml.dump(config, f, sort_keys=False) From 39b38ae61a4b86171758864316ad6724bc622d27 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sat, 20 Apr 2024 18:25:29 +0530 Subject: [PATCH 17/69] fix Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/badger_test.go | 2 +- cmd/jaeger/internal/integration/badgercleaner/config.go | 3 +++ .../internal/integration/badgercleaner/extension.go | 8 ++++++-- cmd/jaeger/internal/integration/badgercleaner/factory.go | 3 +++ 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go index cec65d4efbd..bb063979f91 100644 --- a/cmd/jaeger/internal/integration/badger_test.go +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -8,9 +8,9 @@ import ( "os/exec" "testing" + "github.com/stretchr/testify/require" "go.uber.org/zap" - "github.com/crossdock/crossdock-go/require" "github.com/jaegertracing/jaeger/plugin/storage/integration" ) diff --git a/cmd/jaeger/internal/integration/badgercleaner/config.go b/cmd/jaeger/internal/integration/badgercleaner/config.go index b2eecb59e3d..058a8176851 100644 --- a/cmd/jaeger/internal/integration/badgercleaner/config.go +++ b/cmd/jaeger/internal/integration/badgercleaner/config.go @@ -1,3 +1,6 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + package badgercleaner import ( diff --git a/cmd/jaeger/internal/integration/badgercleaner/extension.go b/cmd/jaeger/internal/integration/badgercleaner/extension.go index d069b4d797f..e348042f55d 100644 --- a/cmd/jaeger/internal/integration/badgercleaner/extension.go +++ b/cmd/jaeger/internal/integration/badgercleaner/extension.go @@ -1,3 +1,6 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + package badgercleaner import ( @@ -6,10 +9,11 @@ import ( "net/http" "github.com/gorilla/mux" - "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage" - "github.com/jaegertracing/jaeger/plugin/storage/badger" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/extension" + + "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage" + "github.com/jaegertracing/jaeger/plugin/storage/badger" ) var ( diff --git a/cmd/jaeger/internal/integration/badgercleaner/factory.go b/cmd/jaeger/internal/integration/badgercleaner/factory.go index 6b9485939fe..398d17dac75 100644 --- a/cmd/jaeger/internal/integration/badgercleaner/factory.go +++ b/cmd/jaeger/internal/integration/badgercleaner/factory.go @@ -1,3 +1,6 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + package badgercleaner import ( From 914cddfde6a8ac4eb6294603fc38399a6d4712fc Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sat, 20 Apr 2024 19:36:28 +0530 Subject: [PATCH 18/69] fix Signed-off-by: Harshvir Potpose --- cmd/jaeger/badger_config.yaml | 12 ++++++------ .../internal/integration/badgercleaner/.nocover | 1 + 2 files changed, 7 insertions(+), 6 deletions(-) create mode 100644 cmd/jaeger/internal/integration/badgercleaner/.nocover diff --git a/cmd/jaeger/badger_config.yaml b/cmd/jaeger/badger_config.yaml index 39f5f633b30..7ebd4b7a427 100644 --- a/cmd/jaeger/badger_config.yaml +++ b/cmd/jaeger/badger_config.yaml @@ -14,16 +14,16 @@ extensions: jaeger_storage: badger: - badger_archive: - directory_key: /tmp/jaeger_archive/ - directory_value: /tmp/jaeger_archive/ + badger_main: + directory_key: /tmp/jaeger/ + directory_value: /tmp/jaeger/ ephemeral: false maintenance_interval: 5 metrics_update_interval: 10 span_store_ttl: 72h - badger_main: - directory_key: /tmp/jaeger/ - directory_value: /tmp/jaeger/ + badger_archive: + directory_key: /tmp/jaeger_archive/ + directory_value: /tmp/jaeger_archive/ ephemeral: false maintenance_interval: 5 metrics_update_interval: 10 diff --git a/cmd/jaeger/internal/integration/badgercleaner/.nocover b/cmd/jaeger/internal/integration/badgercleaner/.nocover new file mode 100644 index 00000000000..9d6cf4b7fb6 --- /dev/null +++ b/cmd/jaeger/internal/integration/badgercleaner/.nocover @@ -0,0 +1 @@ +FIXME From 5a293f2d2d9c7314176d9e25bbaf41ebdd3ed6f3 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sat, 20 Apr 2024 19:42:09 +0530 Subject: [PATCH 19/69] fix Signed-off-by: Harshvir Potpose --- .../integration/badgercleaner/extension.go | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/cmd/jaeger/internal/integration/badgercleaner/extension.go b/cmd/jaeger/internal/integration/badgercleaner/extension.go index e348042f55d..ce446508a77 100644 --- a/cmd/jaeger/internal/integration/badgercleaner/extension.go +++ b/cmd/jaeger/internal/integration/badgercleaner/extension.go @@ -28,6 +28,7 @@ const ( type cleaner struct { config *Config + server *http.Server } func newBadgerCleaner(config *Config) *cleaner { @@ -36,8 +37,8 @@ func newBadgerCleaner(config *Config) *cleaner { } } -func (e *cleaner) Start(ctx context.Context, host component.Host) error { - storageFactory, err := jaegerstorage.GetStorageFactory(e.config.TraceStorage, host) +func (c *cleaner) Start(ctx context.Context, host component.Host) error { + storageFactory, err := jaegerstorage.GetStorageFactory(c.config.TraceStorage, host) if err != nil { return fmt.Errorf("cannot find storage factory for Badger: %w", err) } @@ -61,8 +62,12 @@ func (e *cleaner) Start(ctx context.Context, host component.Host) error { r := mux.NewRouter() r.HandleFunc(cleanerURL, cleanerHandler).Methods(http.MethodPost) + c.server = &http.Server{ + Addr: ":" + cleanerPort, + Handler: r, + } go func() { - if err := http.ListenAndServe(":"+cleanerPort, r); err != nil { + if err := c.server.ListenAndServe(); err != nil { fmt.Printf("Error starting cleaner server: %v\n", err) } }() @@ -70,10 +75,16 @@ func (e *cleaner) Start(ctx context.Context, host component.Host) error { return nil } -func (r *cleaner) Shutdown(ctx context.Context) error { +func (c *cleaner) Shutdown(ctx context.Context) error { + if c.server != nil { + err := c.server.Shutdown(ctx) + if err != nil { + return fmt.Errorf("error shutting down cleaner server: %w", err) + } + } return nil } -func (s *cleaner) Dependencies() []component.ID { +func (c *cleaner) Dependencies() []component.ID { return []component.ID{jaegerstorage.ID} } From 118f998e51d53d3e46d960afe2327079b983653d Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sat, 20 Apr 2024 19:44:28 +0530 Subject: [PATCH 20/69] fix lint Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/badgercleaner/extension.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/jaeger/internal/integration/badgercleaner/extension.go b/cmd/jaeger/internal/integration/badgercleaner/extension.go index ce446508a77..56ee35ca6d9 100644 --- a/cmd/jaeger/internal/integration/badgercleaner/extension.go +++ b/cmd/jaeger/internal/integration/badgercleaner/extension.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "net/http" + "time" "github.com/gorilla/mux" "go.opentelemetry.io/collector/component" @@ -63,8 +64,9 @@ func (c *cleaner) Start(ctx context.Context, host component.Host) error { r := mux.NewRouter() r.HandleFunc(cleanerURL, cleanerHandler).Methods(http.MethodPost) c.server = &http.Server{ - Addr: ":" + cleanerPort, - Handler: r, + Addr: ":" + cleanerPort, + Handler: r, + ReadHeaderTimeout: 3 * time.Second, } go func() { if err := c.server.ListenAndServe(); err != nil { From b993e312ea52eced0f0d07db36c9291ce809afd7 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sat, 20 Apr 2024 19:50:40 +0530 Subject: [PATCH 21/69] rm unwanted changes Signed-off-by: Harshvir Potpose --- cmd/jaeger/badger_config.yaml | 8 ++++---- cmd/jaeger/internal/integration/integration.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/jaeger/badger_config.yaml b/cmd/jaeger/badger_config.yaml index 7ebd4b7a427..1f898fe8b06 100644 --- a/cmd/jaeger/badger_config.yaml +++ b/cmd/jaeger/badger_config.yaml @@ -15,15 +15,15 @@ extensions: jaeger_storage: badger: badger_main: - directory_key: /tmp/jaeger/ - directory_value: /tmp/jaeger/ + directory_key: "/tmp/jaeger/" + directory_value: "/tmp/jaeger/" ephemeral: false maintenance_interval: 5 metrics_update_interval: 10 span_store_ttl: 72h badger_archive: - directory_key: /tmp/jaeger_archive/ - directory_value: /tmp/jaeger_archive/ + directory_key: "/tmp/jaeger_archive/" + directory_value: "/tmp/jaeger_archive/" ephemeral: false maintenance_interval: 5 metrics_update_interval: 10 diff --git a/cmd/jaeger/internal/integration/integration.go b/cmd/jaeger/internal/integration/integration.go index b40c7334492..67ecd609508 100644 --- a/cmd/jaeger/internal/integration/integration.go +++ b/cmd/jaeger/internal/integration/integration.go @@ -39,7 +39,7 @@ type E2EStorageIntegration struct { func (s *E2EStorageIntegration) e2eInitialize(t *testing.T) { logger, _ := testutils.NewLogger() - cmd := &exec.Cmd{ + cmd := exec.Cmd{ Path: "./cmd/jaeger/jaeger", Args: []string{"jaeger", "--config", s.ConfigFile}, // Change the working directory to the root of this project From e7373d1a8e00d347485251f1221a8d285bda2d9c Mon Sep 17 00:00:00 2001 From: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> Date: Sat, 20 Apr 2024 20:35:24 +0530 Subject: [PATCH 22/69] Update cmd/jaeger/internal/integration/badgercleaner/factory.go Co-authored-by: Yuri Shkuro Signed-off-by: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> --- cmd/jaeger/internal/integration/badgercleaner/factory.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/badgercleaner/factory.go b/cmd/jaeger/internal/integration/badgercleaner/factory.go index 398d17dac75..cbbca32cfc9 100644 --- a/cmd/jaeger/internal/integration/badgercleaner/factory.go +++ b/cmd/jaeger/internal/integration/badgercleaner/factory.go @@ -29,6 +29,10 @@ func createDefaultConfig() component.Config { return &Config{} } -func createExtension(_ context.Context, set extension.CreateSettings, cfg component.Config) (extension.Extension, error) { +func createExtension( + _ context.Context, + set extension.CreateSettings, + cfg component.Config, +) (extension.Extension, error) { return newBadgerCleaner(cfg.(*Config)), nil } From 06ea8aa9266c9844e20a37ade1e0a571ef683787 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sun, 21 Apr 2024 08:58:18 +0530 Subject: [PATCH 23/69] increase span store ttl to 30d Signed-off-by: Harshvir Potpose --- cmd/jaeger/badger_config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/jaeger/badger_config.yaml b/cmd/jaeger/badger_config.yaml index 1f898fe8b06..4f8827367c8 100644 --- a/cmd/jaeger/badger_config.yaml +++ b/cmd/jaeger/badger_config.yaml @@ -20,14 +20,14 @@ extensions: ephemeral: false maintenance_interval: 5 metrics_update_interval: 10 - span_store_ttl: 72h + span_store_ttl: 30d badger_archive: directory_key: "/tmp/jaeger_archive/" directory_value: "/tmp/jaeger_archive/" ephemeral: false maintenance_interval: 5 metrics_update_interval: 10 - span_store_ttl: 72h + span_store_ttl: 30d receivers: otlp: From 2acbefa7a387c462e20c4f76e45b6c889a6ce6ab Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sun, 21 Apr 2024 08:58:46 +0530 Subject: [PATCH 24/69] add check for status code Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/badger_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go index bb063979f91..aee1862e4b8 100644 --- a/cmd/jaeger/internal/integration/badger_test.go +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -42,6 +42,8 @@ func (s *BadgerStorageIntegration) cleanUp(t *testing.T) { resp, err := client.Do(r) require.NoError(t, err) defer resp.Body.Close() + + require.Equal(t, http.StatusOK, resp.StatusCode) } func TestBadgerStorage(t *testing.T) { From 5232516aa36cab81d5d0a3aa227a68afedc020a2 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sun, 21 Apr 2024 13:15:48 +0530 Subject: [PATCH 25/69] fix Signed-off-by: Harshvir Potpose --- cmd/jaeger/badger_config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/jaeger/badger_config.yaml b/cmd/jaeger/badger_config.yaml index 4f8827367c8..a9ecda040f5 100644 --- a/cmd/jaeger/badger_config.yaml +++ b/cmd/jaeger/badger_config.yaml @@ -20,14 +20,14 @@ extensions: ephemeral: false maintenance_interval: 5 metrics_update_interval: 10 - span_store_ttl: 30d + span_store_ttl: 720h badger_archive: directory_key: "/tmp/jaeger_archive/" directory_value: "/tmp/jaeger_archive/" ephemeral: false maintenance_interval: 5 metrics_update_interval: 10 - span_store_ttl: 30d + span_store_ttl: 720h receivers: otlp: From f14fd56a6e16acd9cb0b42c2f4d64db00871c8fe Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sun, 21 Apr 2024 14:32:50 +0530 Subject: [PATCH 26/69] add cleaner port in the config Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/badgercleaner/config.go | 1 + cmd/jaeger/internal/integration/badgercleaner/extension.go | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cmd/jaeger/internal/integration/badgercleaner/config.go b/cmd/jaeger/internal/integration/badgercleaner/config.go index 058a8176851..b6a49be7ace 100644 --- a/cmd/jaeger/internal/integration/badgercleaner/config.go +++ b/cmd/jaeger/internal/integration/badgercleaner/config.go @@ -9,6 +9,7 @@ import ( type Config struct { TraceStorage string `valid:"required" mapstructure:"trace_storage"` + cleanerPort string `mapstructure:"cleaner_port"` } func (cfg *Config) Validate() error { diff --git a/cmd/jaeger/internal/integration/badgercleaner/extension.go b/cmd/jaeger/internal/integration/badgercleaner/extension.go index 56ee35ca6d9..7a739d1891f 100644 --- a/cmd/jaeger/internal/integration/badgercleaner/extension.go +++ b/cmd/jaeger/internal/integration/badgercleaner/extension.go @@ -57,14 +57,17 @@ func (c *cleaner) Start(ctx context.Context, host component.Host) error { http.Error(w, err.Error(), http.StatusInternalServerError) return } - w.WriteHeader(http.StatusCreated) + w.WriteHeader(http.StatusOK) w.Write([]byte("Purge request processed successfully")) } r := mux.NewRouter() r.HandleFunc(cleanerURL, cleanerHandler).Methods(http.MethodPost) + if c.config.cleanerPort == "" { + c.config.cleanerPort = cleanerPort + } c.server = &http.Server{ - Addr: ":" + cleanerPort, + Addr: ":" + c.config.cleanerPort, Handler: r, ReadHeaderTimeout: 3 * time.Second, } From 30f43e6037fbec60d8cf7cae125729b796b25faa Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sun, 21 Apr 2024 14:33:23 +0530 Subject: [PATCH 27/69] use temp file for the config Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/badger_test.go | 18 ++++++++++++++---- scripts/prepare-badger-integration-tests.py | 14 ++++++++++---- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go index aee1862e4b8..212cab6b94a 100644 --- a/cmd/jaeger/internal/integration/badger_test.go +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -5,7 +5,9 @@ package integration import ( "net/http" + "os" "os/exec" + "strings" "testing" "github.com/stretchr/testify/require" @@ -51,7 +53,7 @@ func TestBadgerStorage(t *testing.T) { s := &BadgerStorageIntegration{ E2EStorageIntegration: E2EStorageIntegration{ - ConfigFile: "cmd/jaeger/badger_config.yaml", + ConfigFile: createBadgerCleanerConfig(t), StorageIntegration: integration.StorageIntegration{ SkipBinaryAttrs: true, SkipArchiveTest: true, @@ -62,7 +64,7 @@ func TestBadgerStorage(t *testing.T) { }, }, } - s.addBadgerCleanerConfig(t) + defer s.rmBadgerCeanerConfig(t) s.initialize(t) t.Cleanup(func() { s.e2eCleanUp(t) @@ -70,8 +72,16 @@ func TestBadgerStorage(t *testing.T) { s.RunAll(t) } -func (s *BadgerStorageIntegration) addBadgerCleanerConfig(t *testing.T) { +func createBadgerCleanerConfig(t *testing.T) string { cmd := exec.Command("../../../../scripts/prepare-badger-integration-tests.py") - err := cmd.Run() + data, err := cmd.Output() + require.NoError(t, err) + tempFile := string(data) + tempFile = strings.ReplaceAll(tempFile, "\n", "") + return tempFile +} + +func (s *BadgerStorageIntegration) rmBadgerCeanerConfig(t *testing.T) { + err := os.Remove(s.ConfigFile) require.NoError(t, err) } diff --git a/scripts/prepare-badger-integration-tests.py b/scripts/prepare-badger-integration-tests.py index dd0da86eff8..b699bd82089 100755 --- a/scripts/prepare-badger-integration-tests.py +++ b/scripts/prepare-badger-integration-tests.py @@ -2,17 +2,23 @@ import yaml import os +import tempfile config_file = os.path.join(os.path.dirname(__file__), '../cmd/jaeger/badger_config.yaml') with open(config_file, 'r') as f: config = yaml.safe_load(f) -if 'badger_cleaner' not in config['service']['extensions'] : +temp_config = config.copy() + +if 'badger_cleaner' not in temp_config['service']['extensions'] : config['service']['extensions'].insert(1, 'badger_cleaner') -config['extensions']['badger_cleaner'] = { +temp_config['extensions']['badger_cleaner'] = { 'trace_storage': 'badger_main' } -with open(config_file, 'w') as f: - yaml.dump(config, f, sort_keys=False) +with tempfile.NamedTemporaryFile(mode='w', delete=False, dir=os.path.dirname(config_file),suffix='.yaml') as f: + temp_config_file = f.name + yaml.dump(temp_config, f, sort_keys=False) + +print(temp_config_file) From fa83996eed4697b826dc6943581f84aeafa1a4c9 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Sun, 21 Apr 2024 22:33:25 +0530 Subject: [PATCH 28/69] refactor the extenstion to make it more generic and address open comments Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/components.go | 4 ++-- .../internal/integration/badger_test.go | 14 +++++++------- .../integration/badgercleaner/.nocover | 1 - .../README.md | 8 ++++---- .../config.go | 4 ++-- .../extension.go | 19 +++++++------------ .../factory.go | 10 ++++++---- scripts/prepare-badger-integration-tests.py | 6 +++--- 8 files changed, 31 insertions(+), 35 deletions(-) delete mode 100644 cmd/jaeger/internal/integration/badgercleaner/.nocover rename cmd/jaeger/internal/integration/{badgercleaner => storagecleaner}/README.md (69%) rename cmd/jaeger/internal/integration/{badgercleaner => storagecleaner}/config.go (81%) rename cmd/jaeger/internal/integration/{badgercleaner => storagecleaner}/extension.go (80%) rename cmd/jaeger/internal/integration/{badgercleaner => storagecleaner}/factory.go (80%) diff --git a/cmd/jaeger/internal/components.go b/cmd/jaeger/internal/components.go index 20e7a39cbbd..c7b9957dbd8 100644 --- a/cmd/jaeger/internal/components.go +++ b/cmd/jaeger/internal/components.go @@ -29,7 +29,7 @@ import ( "github.com/jaegertracing/jaeger/cmd/jaeger/internal/exporters/storageexporter" "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerquery" "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage" - "github.com/jaegertracing/jaeger/cmd/jaeger/internal/integration/badgercleaner" + "github.com/jaegertracing/jaeger/cmd/jaeger/internal/integration/storagecleaner" ) type builders struct { @@ -61,7 +61,7 @@ func (b builders) build() (otelcol.Factories, error) { // add-ons jaegerquery.NewFactory(), jaegerstorage.NewFactory(), - badgercleaner.NewFactory(), + storagecleaner.NewFactory(), // TODO add adaptive sampling ) if err != nil { diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go index 212cab6b94a..4a4446a3a00 100644 --- a/cmd/jaeger/internal/integration/badger_test.go +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -17,10 +17,10 @@ import ( ) const ( - host = "0.0.0.0" - cleanerPort = "9231" - cleanerURL = "/purge" - cleanerAddr = "http://" + host + ":" + cleanerPort + host = "0.0.0.0" + Port = "9231" + URL = "/purge" + Addr = "http://" + host + ":" + Port ) type BadgerStorageIntegration struct { @@ -36,7 +36,7 @@ func (s *BadgerStorageIntegration) initialize(t *testing.T) { } func (s *BadgerStorageIntegration) cleanUp(t *testing.T) { - r, err := http.NewRequest(http.MethodPost, cleanerAddr+cleanerURL, nil) + r, err := http.NewRequest(http.MethodPost, Addr+URL, nil) require.NoError(t, err) client := &http.Client{} @@ -64,7 +64,7 @@ func TestBadgerStorage(t *testing.T) { }, }, } - defer s.rmBadgerCeanerConfig(t) + defer s.rmBadgerCleanerConfig(t) s.initialize(t) t.Cleanup(func() { s.e2eCleanUp(t) @@ -81,7 +81,7 @@ func createBadgerCleanerConfig(t *testing.T) string { return tempFile } -func (s *BadgerStorageIntegration) rmBadgerCeanerConfig(t *testing.T) { +func (s *BadgerStorageIntegration) rmBadgerCleanerConfig(t *testing.T) { err := os.Remove(s.ConfigFile) require.NoError(t, err) } diff --git a/cmd/jaeger/internal/integration/badgercleaner/.nocover b/cmd/jaeger/internal/integration/badgercleaner/.nocover deleted file mode 100644 index 9d6cf4b7fb6..00000000000 --- a/cmd/jaeger/internal/integration/badgercleaner/.nocover +++ /dev/null @@ -1 +0,0 @@ -FIXME diff --git a/cmd/jaeger/internal/integration/badgercleaner/README.md b/cmd/jaeger/internal/integration/storagecleaner/README.md similarity index 69% rename from cmd/jaeger/internal/integration/badgercleaner/README.md rename to cmd/jaeger/internal/integration/storagecleaner/README.md index 6def1ce0500..52f4a47ec4f 100644 --- a/cmd/jaeger/internal/integration/badgercleaner/README.md +++ b/cmd/jaeger/internal/integration/storagecleaner/README.md @@ -1,8 +1,8 @@ -# badger_cleaner +# storage_cleaner -This module implements an extension that allows purging the Badger storage by exposing a HTTP endpoint for making POST requests. +This module implements an extension that allows purging the backend storage by exposing a HTTP endpoint for making POST requests. -The badger_cleaner extension is intended to be used only in tests, providing a way to clear the Badger storage between test runs. Making a POST request to the exposed endpoint will delete all data in the Badger key-value store. +The storage_cleaner extension is intended to be used only in tests, providing a way to clear the storage between test runs. Making a POST request to the exposed endpoint will delete all data in storage. ```mermaid @@ -41,7 +41,7 @@ The following settings are required: ```yaml extensions: - badger_cleaner: + storage_cleaner: trace_storage: external-storage ``` diff --git a/cmd/jaeger/internal/integration/badgercleaner/config.go b/cmd/jaeger/internal/integration/storagecleaner/config.go similarity index 81% rename from cmd/jaeger/internal/integration/badgercleaner/config.go rename to cmd/jaeger/internal/integration/storagecleaner/config.go index b6a49be7ace..5b4c9d30812 100644 --- a/cmd/jaeger/internal/integration/badgercleaner/config.go +++ b/cmd/jaeger/internal/integration/storagecleaner/config.go @@ -1,7 +1,7 @@ // Copyright (c) 2024 The Jaeger Authors. // SPDX-License-Identifier: Apache-2.0 -package badgercleaner +package storagecleaner import ( "github.com/asaskevich/govalidator" @@ -9,7 +9,7 @@ import ( type Config struct { TraceStorage string `valid:"required" mapstructure:"trace_storage"` - cleanerPort string `mapstructure:"cleaner_port"` + Port string `mapstructure:"port"` } func (cfg *Config) Validate() error { diff --git a/cmd/jaeger/internal/integration/badgercleaner/extension.go b/cmd/jaeger/internal/integration/storagecleaner/extension.go similarity index 80% rename from cmd/jaeger/internal/integration/badgercleaner/extension.go rename to cmd/jaeger/internal/integration/storagecleaner/extension.go index 7a739d1891f..c6374054d3e 100644 --- a/cmd/jaeger/internal/integration/badgercleaner/extension.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension.go @@ -1,7 +1,7 @@ // Copyright (c) 2024 The Jaeger Authors. // SPDX-License-Identifier: Apache-2.0 -package badgercleaner +package storagecleaner import ( "context" @@ -23,8 +23,7 @@ var ( ) const ( - cleanerPort = "9231" - cleanerURL = "/purge" + URL = "/purge" ) type cleaner struct { @@ -32,7 +31,7 @@ type cleaner struct { server *http.Server } -func newBadgerCleaner(config *Config) *cleaner { +func newStorageCleaner(config *Config) *cleaner { return &cleaner{ config: config, } @@ -52,7 +51,7 @@ func (c *cleaner) Start(ctx context.Context, host component.Host) error { return nil } - cleanerHandler := func(w http.ResponseWriter, r *http.Request) { + purgeHandler := func(w http.ResponseWriter, r *http.Request) { if err := purgeBadger(); err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return @@ -62,12 +61,9 @@ func (c *cleaner) Start(ctx context.Context, host component.Host) error { } r := mux.NewRouter() - r.HandleFunc(cleanerURL, cleanerHandler).Methods(http.MethodPost) - if c.config.cleanerPort == "" { - c.config.cleanerPort = cleanerPort - } + r.HandleFunc(URL, purgeHandler).Methods(http.MethodPost) c.server = &http.Server{ - Addr: ":" + c.config.cleanerPort, + Addr: ":" + c.config.Port, Handler: r, ReadHeaderTimeout: 3 * time.Second, } @@ -82,8 +78,7 @@ func (c *cleaner) Start(ctx context.Context, host component.Host) error { func (c *cleaner) Shutdown(ctx context.Context) error { if c.server != nil { - err := c.server.Shutdown(ctx) - if err != nil { + if err := c.server.Shutdown(ctx); err != nil { return fmt.Errorf("error shutting down cleaner server: %w", err) } } diff --git a/cmd/jaeger/internal/integration/badgercleaner/factory.go b/cmd/jaeger/internal/integration/storagecleaner/factory.go similarity index 80% rename from cmd/jaeger/internal/integration/badgercleaner/factory.go rename to cmd/jaeger/internal/integration/storagecleaner/factory.go index cbbca32cfc9..782c3fb86e4 100644 --- a/cmd/jaeger/internal/integration/badgercleaner/factory.go +++ b/cmd/jaeger/internal/integration/storagecleaner/factory.go @@ -1,7 +1,7 @@ // Copyright (c) 2024 The Jaeger Authors. // SPDX-License-Identifier: Apache-2.0 -package badgercleaner +package storagecleaner import ( "context" @@ -11,7 +11,7 @@ import ( ) // componentType is the name of this extension in configuration. -var componentType = component.MustNewType("badger_cleaner") +var componentType = component.MustNewType("storage_cleaner") // ID is the identifier of this extension. var ID = component.NewID(componentType) @@ -26,7 +26,9 @@ func NewFactory() extension.Factory { } func createDefaultConfig() component.Config { - return &Config{} + return &Config{ + Port: "9231", + } } func createExtension( @@ -34,5 +36,5 @@ func createExtension( set extension.CreateSettings, cfg component.Config, ) (extension.Extension, error) { - return newBadgerCleaner(cfg.(*Config)), nil + return newStorageCleaner(cfg.(*Config)), nil } diff --git a/scripts/prepare-badger-integration-tests.py b/scripts/prepare-badger-integration-tests.py index b699bd82089..f9fe45ceeab 100755 --- a/scripts/prepare-badger-integration-tests.py +++ b/scripts/prepare-badger-integration-tests.py @@ -10,10 +10,10 @@ config = yaml.safe_load(f) temp_config = config.copy() -if 'badger_cleaner' not in temp_config['service']['extensions'] : - config['service']['extensions'].insert(1, 'badger_cleaner') +if 'storage_cleaner' not in temp_config['service']['extensions'] : + config['service']['extensions'].insert(1, 'storage_cleaner') -temp_config['extensions']['badger_cleaner'] = { +temp_config['extensions']['storage_cleaner'] = { 'trace_storage': 'badger_main' } From 15ddf59dc51fae5b3c43aa8366d8b0ab83e26e34 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> Date: Mon, 22 Apr 2024 11:40:34 +0530 Subject: [PATCH 29/69] Update cmd/jaeger/badger_config.yaml Co-authored-by: Yuri Shkuro Signed-off-by: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> --- cmd/jaeger/badger_config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/jaeger/badger_config.yaml b/cmd/jaeger/badger_config.yaml index a9ecda040f5..4643c9cc75a 100644 --- a/cmd/jaeger/badger_config.yaml +++ b/cmd/jaeger/badger_config.yaml @@ -20,7 +20,7 @@ extensions: ephemeral: false maintenance_interval: 5 metrics_update_interval: 10 - span_store_ttl: 720h + span_store_ttl: 72h badger_archive: directory_key: "/tmp/jaeger_archive/" directory_value: "/tmp/jaeger_archive/" From ca6b23dda4f7cd2071bf69ed0d29ccc9c76f7f89 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Mon, 22 Apr 2024 12:14:53 +0530 Subject: [PATCH 30/69] make port and url public consts Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/badger_test.go | 7 +++---- .../internal/integration/storagecleaner/extension.go | 3 ++- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go index 4a4446a3a00..b714e14b84b 100644 --- a/cmd/jaeger/internal/integration/badger_test.go +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -13,14 +13,13 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/cmd/jaeger/internal/integration/storagecleaner" "github.com/jaegertracing/jaeger/plugin/storage/integration" ) const ( host = "0.0.0.0" - Port = "9231" - URL = "/purge" - Addr = "http://" + host + ":" + Port + Addr = "http://" + host + ":" + storagecleaner.Port + storagecleaner.URL ) type BadgerStorageIntegration struct { @@ -36,7 +35,7 @@ func (s *BadgerStorageIntegration) initialize(t *testing.T) { } func (s *BadgerStorageIntegration) cleanUp(t *testing.T) { - r, err := http.NewRequest(http.MethodPost, Addr+URL, nil) + r, err := http.NewRequest(http.MethodPost, Addr, nil) require.NoError(t, err) client := &http.Client{} diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension.go b/cmd/jaeger/internal/integration/storagecleaner/extension.go index c6374054d3e..ac313fcdf54 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension.go @@ -23,7 +23,8 @@ var ( ) const ( - URL = "/purge" + Port = "9231" + URL = "/purge" ) type cleaner struct { From 6d6d87026dad010129a54847deff7316816680f2 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Mon, 22 Apr 2024 12:15:37 +0530 Subject: [PATCH 31/69] add factory test Signed-off-by: Harshvir Potpose --- .../storagecleaner/factory_test.go | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 cmd/jaeger/internal/integration/storagecleaner/factory_test.go diff --git a/cmd/jaeger/internal/integration/storagecleaner/factory_test.go b/cmd/jaeger/internal/integration/storagecleaner/factory_test.go new file mode 100644 index 00000000000..83a333e2b5b --- /dev/null +++ b/cmd/jaeger/internal/integration/storagecleaner/factory_test.go @@ -0,0 +1,25 @@ +package storagecleaner + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/component/componenttest" + "go.opentelemetry.io/collector/extension/extensiontest" +) + +func TestCreateDefaultConfig(t *testing.T) { + cfg := createDefaultConfig().(*Config) + require.NotNil(t, cfg, "failed to create default config") + require.NoError(t, componenttest.CheckConfigStruct(cfg)) +} + +func TestCreateExtension(t *testing.T) { + cfg := createDefaultConfig().(*Config) + f := NewFactory() + r, err := f.CreateExtension(context.Background(), extensiontest.NewNopCreateSettings(), cfg) + require.NoError(t, err) + assert.NotNil(t, r) +} From 031bf15829a906f821610cc726f0c8e7f0adb5a0 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Mon, 22 Apr 2024 12:45:20 +0530 Subject: [PATCH 32/69] add goleak check and fix lint Signed-off-by: Harshvir Potpose --- .../internal/integration/storagecleaner/factory.go | 2 +- .../integration/storagecleaner/factory_test.go | 3 +++ .../integration/storagecleaner/package_test.go | 14 ++++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 cmd/jaeger/internal/integration/storagecleaner/package_test.go diff --git a/cmd/jaeger/internal/integration/storagecleaner/factory.go b/cmd/jaeger/internal/integration/storagecleaner/factory.go index 782c3fb86e4..b4efe135648 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/factory.go +++ b/cmd/jaeger/internal/integration/storagecleaner/factory.go @@ -27,7 +27,7 @@ func NewFactory() extension.Factory { func createDefaultConfig() component.Config { return &Config{ - Port: "9231", + Port: Port, } } diff --git a/cmd/jaeger/internal/integration/storagecleaner/factory_test.go b/cmd/jaeger/internal/integration/storagecleaner/factory_test.go index 83a333e2b5b..7b1ec51d8f3 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/factory_test.go +++ b/cmd/jaeger/internal/integration/storagecleaner/factory_test.go @@ -1,3 +1,6 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + package storagecleaner import ( diff --git a/cmd/jaeger/internal/integration/storagecleaner/package_test.go b/cmd/jaeger/internal/integration/storagecleaner/package_test.go new file mode 100644 index 00000000000..cec15912582 --- /dev/null +++ b/cmd/jaeger/internal/integration/storagecleaner/package_test.go @@ -0,0 +1,14 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + +package storagecleaner + +import ( + "testing" + + "github.com/jaegertracing/jaeger/pkg/testutils" +) + +func TestMain(m *testing.M) { + testutils.VerifyGoLeaks(m) +} From a5a4e56e003f5432743d01e649ed0f3e06088578 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Mon, 22 Apr 2024 18:19:59 +0530 Subject: [PATCH 33/69] fix Signed-off-by: Harshvir Potpose --- .../integration/storagecleaner/extension.go | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension.go b/cmd/jaeger/internal/integration/storagecleaner/extension.go index ac313fcdf54..e8edddd6129 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension.go @@ -18,8 +18,8 @@ import ( ) var ( - _ extension.Extension = (*cleaner)(nil) - _ extension.Dependent = (*cleaner)(nil) + _ extension.Extension = (*storageCleaner)(nil) + _ extension.Dependent = (*storageCleaner)(nil) ) const ( @@ -27,18 +27,18 @@ const ( URL = "/purge" ) -type cleaner struct { +type storageCleaner struct { config *Config server *http.Server } -func newStorageCleaner(config *Config) *cleaner { - return &cleaner{ +func newStorageCleaner(config *Config) *storageCleaner { + return &storageCleaner{ config: config, } } -func (c *cleaner) Start(ctx context.Context, host component.Host) error { +func (c *storageCleaner) Start(ctx context.Context, host component.Host) error { storageFactory, err := jaegerstorage.GetStorageFactory(c.config.TraceStorage, host) if err != nil { return fmt.Errorf("cannot find storage factory for Badger: %w", err) @@ -68,16 +68,17 @@ func (c *cleaner) Start(ctx context.Context, host component.Host) error { Handler: r, ReadHeaderTimeout: 3 * time.Second, } - go func() { + go func() error { if err := c.server.ListenAndServe(); err != nil { - fmt.Printf("Error starting cleaner server: %v\n", err) + return fmt.Errorf("error starting storage cleaner server: %w", err) } + return nil }() return nil } -func (c *cleaner) Shutdown(ctx context.Context) error { +func (c *storageCleaner) Shutdown(ctx context.Context) error { if c.server != nil { if err := c.server.Shutdown(ctx); err != nil { return fmt.Errorf("error shutting down cleaner server: %w", err) @@ -86,6 +87,6 @@ func (c *cleaner) Shutdown(ctx context.Context) error { return nil } -func (c *cleaner) Dependencies() []component.ID { +func (c *storageCleaner) Dependencies() []component.ID { return []component.ID{jaegerstorage.ID} } From ee8479323e921b555b1e64e6883242930050c35a Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Tue, 23 Apr 2024 02:10:40 +0530 Subject: [PATCH 34/69] create storage.Purger interface Signed-off-by: Harshvir Potpose --- .../internal/integration/storagecleaner/extension.go | 10 +++++----- plugin/storage/badger/factory.go | 1 + storage/factory.go | 6 ++++++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension.go b/cmd/jaeger/internal/integration/storagecleaner/extension.go index e8edddd6129..b08111e9b5d 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension.go @@ -14,7 +14,7 @@ import ( "go.opentelemetry.io/collector/extension" "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage" - "github.com/jaegertracing/jaeger/plugin/storage/badger" + "github.com/jaegertracing/jaeger/storage" ) var ( @@ -44,16 +44,16 @@ func (c *storageCleaner) Start(ctx context.Context, host component.Host) error { return fmt.Errorf("cannot find storage factory for Badger: %w", err) } - purgeBadger := func() error { - badgerFactory := storageFactory.(*badger.Factory) - if err := badgerFactory.Purge(); err != nil { + purgeStorage := func() error { + purger := storageFactory.(storage.Purger) + if err := purger.Purge(); err != nil { return fmt.Errorf("error purging Badger storage: %w", err) } return nil } purgeHandler := func(w http.ResponseWriter, r *http.Request) { - if err := purgeBadger(); err != nil { + if err := purgeStorage(); err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return } diff --git a/plugin/storage/badger/factory.go b/plugin/storage/badger/factory.go index 607ab05bf65..1fc2e1dffac 100644 --- a/plugin/storage/badger/factory.go +++ b/plugin/storage/badger/factory.go @@ -50,6 +50,7 @@ var ( // interface comformance checks _ storage.Factory = (*Factory)(nil) _ io.Closer = (*Factory)(nil) _ plugin.Configurable = (*Factory)(nil) + _ storage.Purger = (*Factory)(nil) // TODO badger could implement archive storage // _ storage.ArchiveFactory = (*Factory)(nil) diff --git a/storage/factory.go b/storage/factory.go index 933d0dce27d..91e8c6a3687 100644 --- a/storage/factory.go +++ b/storage/factory.go @@ -49,6 +49,12 @@ type Factory interface { CreateDependencyReader() (dependencystore.Reader, error) } +// Purger defines an interface that is capable of purging the storage. +type Purger interface { + // Purge removes all data from the storage. + Purge() error +} + // SamplingStoreFactory defines an interface that is capable of returning the necessary backends for // adaptive sampling. type SamplingStoreFactory interface { From f35a6a467f4c21bfdea2fd5e41398703a85f4c3e Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Tue, 23 Apr 2024 02:12:26 +0530 Subject: [PATCH 35/69] rm temp file from t.cleanup and use fmt.Sprintf insted of constants Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/badger_test.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go index b714e14b84b..877fb79c0f1 100644 --- a/cmd/jaeger/internal/integration/badger_test.go +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -4,6 +4,7 @@ package integration import ( + "fmt" "net/http" "os" "os/exec" @@ -17,11 +18,6 @@ import ( "github.com/jaegertracing/jaeger/plugin/storage/integration" ) -const ( - host = "0.0.0.0" - Addr = "http://" + host + ":" + storagecleaner.Port + storagecleaner.URL -) - type BadgerStorageIntegration struct { E2EStorageIntegration logger *zap.Logger @@ -35,6 +31,7 @@ func (s *BadgerStorageIntegration) initialize(t *testing.T) { } func (s *BadgerStorageIntegration) cleanUp(t *testing.T) { + Addr := fmt.Sprintf("http://%s:%s%s", "0.0.0.0", storagecleaner.Port, storagecleaner.URL) r, err := http.NewRequest(http.MethodPost, Addr, nil) require.NoError(t, err) @@ -63,7 +60,6 @@ func TestBadgerStorage(t *testing.T) { }, }, } - defer s.rmBadgerCleanerConfig(t) s.initialize(t) t.Cleanup(func() { s.e2eCleanUp(t) @@ -77,10 +73,8 @@ func createBadgerCleanerConfig(t *testing.T) string { require.NoError(t, err) tempFile := string(data) tempFile = strings.ReplaceAll(tempFile, "\n", "") + t.Cleanup(func() { + os.Remove(tempFile) + }) return tempFile } - -func (s *BadgerStorageIntegration) rmBadgerCleanerConfig(t *testing.T) { - err := os.Remove(s.ConfigFile) - require.NoError(t, err) -} From 4ef7ad8e1bc198bd1b4a86250e9910400a2cbfb6 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> Date: Tue, 23 Apr 2024 02:17:45 +0530 Subject: [PATCH 36/69] Update cmd/jaeger/internal/integration/storagecleaner/extension.go Co-authored-by: Yuri Shkuro Signed-off-by: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> --- cmd/jaeger/internal/integration/storagecleaner/extension.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension.go b/cmd/jaeger/internal/integration/storagecleaner/extension.go index b08111e9b5d..43765306b46 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension.go @@ -45,7 +45,10 @@ func (c *storageCleaner) Start(ctx context.Context, host component.Host) error { } purgeStorage := func() error { - purger := storageFactory.(storage.Purger) + purger, ok := storageFactory.(storage.Purger) + if !ok { + return fmt.Errorf("storage %s does not implement Purger interface", c.config.TraceStorage) + } if err := purger.Purge(); err != nil { return fmt.Errorf("error purging Badger storage: %w", err) } From 07c642cee0c06f1aa10c7f060ba8420ca1182c5d Mon Sep 17 00:00:00 2001 From: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> Date: Tue, 23 Apr 2024 02:18:08 +0530 Subject: [PATCH 37/69] Update storage/factory.go Co-authored-by: Yuri Shkuro Signed-off-by: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> --- storage/factory.go | 1 + 1 file changed, 1 insertion(+) diff --git a/storage/factory.go b/storage/factory.go index 91e8c6a3687..b56e6fdc07f 100644 --- a/storage/factory.go +++ b/storage/factory.go @@ -50,6 +50,7 @@ type Factory interface { } // Purger defines an interface that is capable of purging the storage. +// Only meant to be used from integration tests. type Purger interface { // Purge removes all data from the storage. Purge() error From b2dd6566f013a0694b296c73dac30c7c3425718e Mon Sep 17 00:00:00 2001 From: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> Date: Tue, 23 Apr 2024 02:25:57 +0530 Subject: [PATCH 38/69] Update cmd/jaeger/internal/integration/badger_test.go Co-authored-by: Yuri Shkuro Signed-off-by: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> --- cmd/jaeger/internal/integration/badger_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go index 877fb79c0f1..981f85b7cbe 100644 --- a/cmd/jaeger/internal/integration/badger_test.go +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -31,7 +31,7 @@ func (s *BadgerStorageIntegration) initialize(t *testing.T) { } func (s *BadgerStorageIntegration) cleanUp(t *testing.T) { - Addr := fmt.Sprintf("http://%s:%s%s", "0.0.0.0", storagecleaner.Port, storagecleaner.URL) + addr := fmt.Sprintf("http://0.0.0.0:%s%s", storagecleaner.Port, storagecleaner.URL) r, err := http.NewRequest(http.MethodPost, Addr, nil) require.NoError(t, err) From c0ba470292c264cff6bd3fb20fa1b2c0eefc0abb Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Tue, 23 Apr 2024 02:26:58 +0530 Subject: [PATCH 39/69] fix lint Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/storagecleaner/extension.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension.go b/cmd/jaeger/internal/integration/storagecleaner/extension.go index 43765306b46..f6b5bbb630f 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension.go @@ -47,7 +47,7 @@ func (c *storageCleaner) Start(ctx context.Context, host component.Host) error { purgeStorage := func() error { purger, ok := storageFactory.(storage.Purger) if !ok { - return fmt.Errorf("storage %s does not implement Purger interface", c.config.TraceStorage) + return fmt.Errorf("storage %s does not implement Purger interface", c.config.TraceStorage) } if err := purger.Purge(); err != nil { return fmt.Errorf("error purging Badger storage: %w", err) From 794b5b689ab4412fca5a71475a4e26477d6e4f76 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> Date: Wed, 24 Apr 2024 00:13:34 +0530 Subject: [PATCH 40/69] Update cmd/jaeger/internal/integration/storagecleaner/README.md Co-authored-by: Yuri Shkuro Signed-off-by: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> --- cmd/jaeger/internal/integration/storagecleaner/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/README.md b/cmd/jaeger/internal/integration/storagecleaner/README.md index 52f4a47ec4f..ce66e4f4559 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/README.md +++ b/cmd/jaeger/internal/integration/storagecleaner/README.md @@ -42,6 +42,6 @@ The following settings are required: ```yaml extensions: storage_cleaner: - trace_storage: external-storage + trace_storage: storage_name ``` From 0df8a88e4ddf1b781948a3590a374fd91b3f8dda Mon Sep 17 00:00:00 2001 From: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> Date: Wed, 24 Apr 2024 00:13:57 +0530 Subject: [PATCH 41/69] Update cmd/jaeger/internal/integration/storagecleaner/README.md Co-authored-by: Yuri Shkuro Signed-off-by: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> --- cmd/jaeger/internal/integration/storagecleaner/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/README.md b/cmd/jaeger/internal/integration/storagecleaner/README.md index ce66e4f4559..5fbe26ee078 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/README.md +++ b/cmd/jaeger/internal/integration/storagecleaner/README.md @@ -1,6 +1,6 @@ # storage_cleaner -This module implements an extension that allows purging the backend storage by exposing a HTTP endpoint for making POST requests. +This module implements an extension that allows purging the backend storage by making an HTTP POST request to it. The storage_cleaner extension is intended to be used only in tests, providing a way to clear the storage between test runs. Making a POST request to the exposed endpoint will delete all data in storage. From 667a691ef08ff1c7781272aa1076a268d11271ae Mon Sep 17 00:00:00 2001 From: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> Date: Wed, 24 Apr 2024 00:14:20 +0530 Subject: [PATCH 42/69] Update cmd/jaeger/internal/integration/storagecleaner/README.md Co-authored-by: Yuri Shkuro Signed-off-by: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> --- cmd/jaeger/internal/integration/storagecleaner/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/README.md b/cmd/jaeger/internal/integration/storagecleaner/README.md index 5fbe26ee078..69db89906de 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/README.md +++ b/cmd/jaeger/internal/integration/storagecleaner/README.md @@ -22,7 +22,7 @@ flowchart LR Exporter Badger - BadgerCleanerExtension + StorageCleanerExtension HTTP_endpoint subgraph JaegerStorageExension Badger From 3afb7138daf6ab95a0a0aa806b04b69b0f7a2dfa Mon Sep 17 00:00:00 2001 From: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> Date: Wed, 24 Apr 2024 00:14:30 +0530 Subject: [PATCH 43/69] Update cmd/jaeger/internal/integration/storagecleaner/README.md Co-authored-by: Yuri Shkuro Signed-off-by: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> --- cmd/jaeger/internal/integration/storagecleaner/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/README.md b/cmd/jaeger/internal/integration/storagecleaner/README.md index 69db89906de..16437cb8ae3 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/README.md +++ b/cmd/jaeger/internal/integration/storagecleaner/README.md @@ -14,7 +14,7 @@ flowchart LR Badger_e2e_test -->|"(1) POST /purge"| HTTP_endpoint JaegerStorageExension -->|"(2) getStorage()"| HTTP_endpoint - HTTP_endpoint -.->|"(3) storage.(*Badger).Purge()"| Badger + HTTP_endpoint -.->|"(3) storage.(*storage.Purger).Purge()"| Storage subgraph Jaeger Collector Receiver From 87c3f0370e3a2f5fc817b8cd02ee39f1cc9d06bb Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Wed, 24 Apr 2024 00:16:58 +0530 Subject: [PATCH 44/69] enrich config using golang Signed-off-by: Harshvir Potpose --- .../internal/integration/integration.go | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/cmd/jaeger/internal/integration/integration.go b/cmd/jaeger/internal/integration/integration.go index 67ecd609508..c98acc4e8aa 100644 --- a/cmd/jaeger/internal/integration/integration.go +++ b/cmd/jaeger/internal/integration/integration.go @@ -4,12 +4,14 @@ package integration import ( + "fmt" "io" "os" "os/exec" "testing" "github.com/stretchr/testify/require" + "gopkg.in/yaml.v2" "github.com/jaegertracing/jaeger/pkg/testutils" "github.com/jaegertracing/jaeger/plugin/storage/integration" @@ -38,7 +40,9 @@ type E2EStorageIntegration struct { // This function should be called before any of the tests start. func (s *E2EStorageIntegration) e2eInitialize(t *testing.T) { logger, _ := testutils.NewLogger() + s.ConfigFile = "./cmd/jaeger/internal/integration/" + createStorageCleanerConfig(t, s.ConfigFile) + fmt.Println(s.ConfigFile) cmd := exec.Cmd{ Path: "./cmd/jaeger/jaeger", Args: []string{"jaeger", "--config", s.ConfigFile}, @@ -67,3 +71,40 @@ func (s *E2EStorageIntegration) e2eCleanUp(t *testing.T) { require.NoError(t, s.SpanReader.(io.Closer).Close()) require.NoError(t, s.SpanWriter.(io.Closer).Close()) } + +func createStorageCleanerConfig(t *testing.T, configFile string) string { + data, err := os.ReadFile(configFile) + require.NoError(t, err) + var config map[interface{}]interface{} + err = yaml.Unmarshal(data, &config) + require.NoError(t, err) + + service, ok := config["service"].(map[interface{}]interface{}) + require.True(t, ok) + service["extensions"] = []string{"jaeger_storage", "jaeger_query", "storage_cleaner"} + + extensions, ok := config["extensions"].(map[interface{}]interface{}) + require.True(t, ok) + trace_storage := getTraceStorage(extensions) + extensions["storage_cleaner"] = map[string]string{"trace_storage": trace_storage} + + newData, err := yaml.Marshal(config) + require.NoError(t, err) + tempFile, err := os.Create("storageCleaner_config.yaml") + require.NoError(t, err) + os.WriteFile(tempFile.Name(), newData, 0644) + + t.Cleanup(func() { + os.Remove(tempFile.Name()) + }) + return tempFile.Name() +} + +func getTraceStorage(extensions map[interface{}]interface{}) string { + for k := range extensions["jaeger_query"].(map[interface{}]interface{}) { + if k == "trace_storage" { + return extensions["jaeger_query"].(map[interface{}]interface{})[k].(string) + } + } + return "" +} From 57525acd4d12526c04fae1193c6710a89fe35a58 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Wed, 24 Apr 2024 00:17:22 +0530 Subject: [PATCH 45/69] fix Signed-off-by: Harshvir Potpose --- .../internal/integration/badger_test.go | 53 +++++-------------- cmd/jaeger/internal/integration/grpc_test.go | 2 +- 2 files changed, 13 insertions(+), 42 deletions(-) diff --git a/cmd/jaeger/internal/integration/badger_test.go b/cmd/jaeger/internal/integration/badger_test.go index 981f85b7cbe..4f3526197c4 100644 --- a/cmd/jaeger/internal/integration/badger_test.go +++ b/cmd/jaeger/internal/integration/badger_test.go @@ -6,32 +6,16 @@ package integration import ( "fmt" "net/http" - "os" - "os/exec" - "strings" "testing" "github.com/stretchr/testify/require" - "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/jaeger/internal/integration/storagecleaner" "github.com/jaegertracing/jaeger/plugin/storage/integration" ) -type BadgerStorageIntegration struct { - E2EStorageIntegration - logger *zap.Logger -} - -func (s *BadgerStorageIntegration) initialize(t *testing.T) { - s.e2eInitialize(t) - - s.CleanUp = s.cleanUp - s.logger = zap.NewNop() -} - -func (s *BadgerStorageIntegration) cleanUp(t *testing.T) { - addr := fmt.Sprintf("http://0.0.0.0:%s%s", storagecleaner.Port, storagecleaner.URL) +func cleanUp(t *testing.T) { + Addr := fmt.Sprintf("http://0.0.0.0:%s%s", storagecleaner.Port, storagecleaner.URL) r, err := http.NewRequest(http.MethodPost, Addr, nil) require.NoError(t, err) @@ -47,34 +31,21 @@ func (s *BadgerStorageIntegration) cleanUp(t *testing.T) { func TestBadgerStorage(t *testing.T) { integration.SkipUnlessEnv(t, "badger") - s := &BadgerStorageIntegration{ - E2EStorageIntegration: E2EStorageIntegration{ - ConfigFile: createBadgerCleanerConfig(t), - StorageIntegration: integration.StorageIntegration{ - SkipBinaryAttrs: true, - SkipArchiveTest: true, + s := &E2EStorageIntegration{ + ConfigFile: "../../badger_config.yaml", + StorageIntegration: integration.StorageIntegration{ + SkipBinaryAttrs: true, + SkipArchiveTest: true, + CleanUp: cleanUp, - // TODO: remove this once badger supports returning spanKind from GetOperations - // Cf https://github.com/jaegertracing/jaeger/issues/1922 - GetOperationsMissingSpanKind: true, - }, + // TODO: remove this once badger supports returning spanKind from GetOperations + // Cf https://github.com/jaegertracing/jaeger/issues/1922 + GetOperationsMissingSpanKind: true, }, } - s.initialize(t) + s.e2eInitialize(t) t.Cleanup(func() { s.e2eCleanUp(t) }) s.RunAll(t) } - -func createBadgerCleanerConfig(t *testing.T) string { - cmd := exec.Command("../../../../scripts/prepare-badger-integration-tests.py") - data, err := cmd.Output() - require.NoError(t, err) - tempFile := string(data) - tempFile = strings.ReplaceAll(tempFile, "\n", "") - t.Cleanup(func() { - os.Remove(tempFile) - }) - return tempFile -} diff --git a/cmd/jaeger/internal/integration/grpc_test.go b/cmd/jaeger/internal/integration/grpc_test.go index f990e8aec46..22f4bbbd067 100644 --- a/cmd/jaeger/internal/integration/grpc_test.go +++ b/cmd/jaeger/internal/integration/grpc_test.go @@ -33,7 +33,7 @@ func TestGRPCStorage(t *testing.T) { integration.SkipUnlessEnv(t, "grpc") s := &GRPCStorageIntegration{} - s.ConfigFile = "cmd/jaeger/grpc_config.yaml" + s.ConfigFile = "../../grpc_config.yaml" s.SkipBinaryAttrs = true s.initialize(t) From fcee96af6e264d69e8bebb76579bf38bfc79d15b Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Wed, 24 Apr 2024 00:17:44 +0530 Subject: [PATCH 46/69] rm py script Signed-off-by: Harshvir Potpose --- scripts/prepare-badger-integration-tests.py | 24 --------------------- 1 file changed, 24 deletions(-) delete mode 100755 scripts/prepare-badger-integration-tests.py diff --git a/scripts/prepare-badger-integration-tests.py b/scripts/prepare-badger-integration-tests.py deleted file mode 100755 index f9fe45ceeab..00000000000 --- a/scripts/prepare-badger-integration-tests.py +++ /dev/null @@ -1,24 +0,0 @@ -#!/usr/bin/env python3 - -import yaml -import os -import tempfile - -config_file = os.path.join(os.path.dirname(__file__), '../cmd/jaeger/badger_config.yaml') - -with open(config_file, 'r') as f: - config = yaml.safe_load(f) -temp_config = config.copy() - -if 'storage_cleaner' not in temp_config['service']['extensions'] : - config['service']['extensions'].insert(1, 'storage_cleaner') - -temp_config['extensions']['storage_cleaner'] = { - 'trace_storage': 'badger_main' -} - -with tempfile.NamedTemporaryFile(mode='w', delete=False, dir=os.path.dirname(config_file),suffix='.yaml') as f: - temp_config_file = f.name - yaml.dump(temp_config, f, sort_keys=False) - -print(temp_config_file) From df0a4233d293e4433f3e393550447f62f7af3504 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Wed, 24 Apr 2024 00:29:57 +0530 Subject: [PATCH 47/69] fix Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/integration.go | 2 +- .../internal/integration/storagecleaner/README.md | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/jaeger/internal/integration/integration.go b/cmd/jaeger/internal/integration/integration.go index c98acc4e8aa..636f8b27b1a 100644 --- a/cmd/jaeger/internal/integration/integration.go +++ b/cmd/jaeger/internal/integration/integration.go @@ -92,7 +92,7 @@ func createStorageCleanerConfig(t *testing.T, configFile string) string { require.NoError(t, err) tempFile, err := os.Create("storageCleaner_config.yaml") require.NoError(t, err) - os.WriteFile(tempFile.Name(), newData, 0644) + os.WriteFile(tempFile.Name(), newData, 0o644) t.Cleanup(func() { os.Remove(tempFile.Name()) diff --git a/cmd/jaeger/internal/integration/storagecleaner/README.md b/cmd/jaeger/internal/integration/storagecleaner/README.md index 16437cb8ae3..a2011cddb06 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/README.md +++ b/cmd/jaeger/internal/integration/storagecleaner/README.md @@ -10,9 +10,9 @@ flowchart LR Receiver --> Processor Processor --> Exporter JaegerStorageExension -->|"(1) get storage"| Exporter - Exporter -->|"(2) write trace"| Badger + Exporter -->|"(2) write trace"| Storage - Badger_e2e_test -->|"(1) POST /purge"| HTTP_endpoint + E2E_test -->|"(1) POST /purge"| HTTP_endpoint JaegerStorageExension -->|"(2) getStorage()"| HTTP_endpoint HTTP_endpoint -.->|"(3) storage.(*storage.Purger).Purge()"| Storage @@ -21,13 +21,13 @@ flowchart LR Processor Exporter - Badger + Storage StorageCleanerExtension HTTP_endpoint subgraph JaegerStorageExension - Badger + Storage end - subgraph BadgerCleanerExtension + subgraph StorageCleanerExtension HTTP_endpoint end end From f218e63d2964bfa55c34800c6fc48b10d0608e20 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Wed, 24 Apr 2024 00:33:35 +0530 Subject: [PATCH 48/69] fix Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/integration.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/integration.go b/cmd/jaeger/internal/integration/integration.go index 636f8b27b1a..153a536d7c0 100644 --- a/cmd/jaeger/internal/integration/integration.go +++ b/cmd/jaeger/internal/integration/integration.go @@ -92,7 +92,8 @@ func createStorageCleanerConfig(t *testing.T, configFile string) string { require.NoError(t, err) tempFile, err := os.Create("storageCleaner_config.yaml") require.NoError(t, err) - os.WriteFile(tempFile.Name(), newData, 0o644) + err = os.WriteFile(tempFile.Name(), newData, 0o644) + require.NoError(t, err) t.Cleanup(func() { os.Remove(tempFile.Name()) From 697346edeca113761dcbefe5dad7214dd8407ab5 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Wed, 24 Apr 2024 00:53:21 +0530 Subject: [PATCH 49/69] fix lint Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/integration.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/integration.go b/cmd/jaeger/internal/integration/integration.go index 153a536d7c0..0288085d7bf 100644 --- a/cmd/jaeger/internal/integration/integration.go +++ b/cmd/jaeger/internal/integration/integration.go @@ -92,7 +92,7 @@ func createStorageCleanerConfig(t *testing.T, configFile string) string { require.NoError(t, err) tempFile, err := os.Create("storageCleaner_config.yaml") require.NoError(t, err) - err = os.WriteFile(tempFile.Name(), newData, 0o644) + err = os.WriteFile(tempFile.Name(), newData, 0o600) require.NoError(t, err) t.Cleanup(func() { From 62dbdb010608e134952d544d984d68ad305be8ec Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Wed, 24 Apr 2024 01:00:05 +0530 Subject: [PATCH 50/69] add config test Signed-off-by: Harshvir Potpose --- .../integration/storagecleaner/config_test.go | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 cmd/jaeger/internal/integration/storagecleaner/config_test.go diff --git a/cmd/jaeger/internal/integration/storagecleaner/config_test.go b/cmd/jaeger/internal/integration/storagecleaner/config_test.go new file mode 100644 index 00000000000..48b49c66e88 --- /dev/null +++ b/cmd/jaeger/internal/integration/storagecleaner/config_test.go @@ -0,0 +1,23 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + +package storagecleaner + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestStorageExtensionConfig(t *testing.T) { + config := createDefaultConfig().(*Config) + config.TraceStorage = "storage" + err := config.Validate() + require.NoError(t, err) +} + +func TestStorageExtensionConfigError(t *testing.T) { + config := createDefaultConfig().(*Config) + err := config.Validate() + require.ErrorContains(t, err, "non zero value required") +} From 32c116f4307b73225a0cac7159ab19e48be569b2 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> Date: Wed, 24 Apr 2024 01:53:02 +0530 Subject: [PATCH 51/69] Update cmd/jaeger/internal/integration/integration.go Co-authored-by: Yuri Shkuro Signed-off-by: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> --- cmd/jaeger/internal/integration/integration.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/cmd/jaeger/internal/integration/integration.go b/cmd/jaeger/internal/integration/integration.go index 0288085d7bf..dfc33e080c9 100644 --- a/cmd/jaeger/internal/integration/integration.go +++ b/cmd/jaeger/internal/integration/integration.go @@ -95,10 +95,7 @@ func createStorageCleanerConfig(t *testing.T, configFile string) string { err = os.WriteFile(tempFile.Name(), newData, 0o600) require.NoError(t, err) - t.Cleanup(func() { - os.Remove(tempFile.Name()) - }) - return tempFile.Name() + return tempFile } func getTraceStorage(extensions map[interface{}]interface{}) string { From 9a008d7cbbe7dc642bcd406ac29e44ff6af801f0 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> Date: Wed, 24 Apr 2024 01:53:27 +0530 Subject: [PATCH 52/69] Update cmd/jaeger/internal/integration/integration.go Co-authored-by: Yuri Shkuro Signed-off-by: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> --- cmd/jaeger/internal/integration/integration.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/jaeger/internal/integration/integration.go b/cmd/jaeger/internal/integration/integration.go index dfc33e080c9..11bb8c3b99e 100644 --- a/cmd/jaeger/internal/integration/integration.go +++ b/cmd/jaeger/internal/integration/integration.go @@ -90,9 +90,8 @@ func createStorageCleanerConfig(t *testing.T, configFile string) string { newData, err := yaml.Marshal(config) require.NoError(t, err) - tempFile, err := os.Create("storageCleaner_config.yaml") - require.NoError(t, err) - err = os.WriteFile(tempFile.Name(), newData, 0o600) + tempFile := filepath.Join(t.TempDir(), "storageCleaner_config.yaml") + err = os.WriteFile(tempFile, newData, 0o600) require.NoError(t, err) return tempFile From 659c9c7dc9ca176eeea275c6fa9d60f8e43a2207 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> Date: Wed, 24 Apr 2024 01:53:36 +0530 Subject: [PATCH 53/69] Update cmd/jaeger/internal/integration/integration.go Co-authored-by: Yuri Shkuro Signed-off-by: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> --- cmd/jaeger/internal/integration/integration.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/integration.go b/cmd/jaeger/internal/integration/integration.go index 11bb8c3b99e..09c50771ead 100644 --- a/cmd/jaeger/internal/integration/integration.go +++ b/cmd/jaeger/internal/integration/integration.go @@ -40,7 +40,7 @@ type E2EStorageIntegration struct { // This function should be called before any of the tests start. func (s *E2EStorageIntegration) e2eInitialize(t *testing.T) { logger, _ := testutils.NewLogger() - s.ConfigFile = "./cmd/jaeger/internal/integration/" + createStorageCleanerConfig(t, s.ConfigFile) + s.ConfigFile = createStorageCleanerConfig(t, s.ConfigFile) fmt.Println(s.ConfigFile) cmd := exec.Cmd{ From a053b309757ce4544e5f4188ecaa7297ff57caa5 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Wed, 24 Apr 2024 01:59:40 +0530 Subject: [PATCH 54/69] fix Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/integration.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/cmd/jaeger/internal/integration/integration.go b/cmd/jaeger/internal/integration/integration.go index 09c50771ead..54159936575 100644 --- a/cmd/jaeger/internal/integration/integration.go +++ b/cmd/jaeger/internal/integration/integration.go @@ -8,6 +8,7 @@ import ( "io" "os" "os/exec" + "path/filepath" "testing" "github.com/stretchr/testify/require" @@ -81,11 +82,13 @@ func createStorageCleanerConfig(t *testing.T, configFile string) string { service, ok := config["service"].(map[interface{}]interface{}) require.True(t, ok) - service["extensions"] = []string{"jaeger_storage", "jaeger_query", "storage_cleaner"} + service["extensions"] = append(service["extensions"].([]interface{}), "storage_cleaner") extensions, ok := config["extensions"].(map[interface{}]interface{}) require.True(t, ok) - trace_storage := getTraceStorage(extensions) + query, ok := extensions["jaeger_query"].(map[interface{}]interface{}) + require.True(t, ok) + trace_storage := query["trace_storage"].(string) extensions["storage_cleaner"] = map[string]string{"trace_storage": trace_storage} newData, err := yaml.Marshal(config) @@ -96,12 +99,3 @@ func createStorageCleanerConfig(t *testing.T, configFile string) string { return tempFile } - -func getTraceStorage(extensions map[interface{}]interface{}) string { - for k := range extensions["jaeger_query"].(map[interface{}]interface{}) { - if k == "trace_storage" { - return extensions["jaeger_query"].(map[interface{}]interface{})[k].(string) - } - } - return "" -} From 3783de8b0e20727cadb30a648d997d3776e0645a Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Wed, 24 Apr 2024 02:02:47 +0530 Subject: [PATCH 55/69] use yaml.v3 Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/integration.go | 2 +- go.mod | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/jaeger/internal/integration/integration.go b/cmd/jaeger/internal/integration/integration.go index 54159936575..58eac318732 100644 --- a/cmd/jaeger/internal/integration/integration.go +++ b/cmd/jaeger/internal/integration/integration.go @@ -12,7 +12,7 @@ import ( "testing" "github.com/stretchr/testify/require" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" "github.com/jaegertracing/jaeger/pkg/testutils" "github.com/jaegertracing/jaeger/plugin/storage/integration" diff --git a/go.mod b/go.mod index 4387f8c9974..c33a0cd1151 100644 --- a/go.mod +++ b/go.mod @@ -225,7 +225,7 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20240401170217-c3f982113cda // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/ini.v1 v1.67.0 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect + gopkg.in/yaml.v3 v3.0.1 ) replace github.com/Shopify/sarama => github.com/Shopify/sarama v1.33.0 From 2c72071c9dd6a197c730fd15191ba1678388b895 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Wed, 24 Apr 2024 02:47:26 +0530 Subject: [PATCH 56/69] fix Signed-off-by: Harshvir Potpose --- cmd/jaeger/internal/integration/integration.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/cmd/jaeger/internal/integration/integration.go b/cmd/jaeger/internal/integration/integration.go index 58eac318732..6446418746d 100644 --- a/cmd/jaeger/internal/integration/integration.go +++ b/cmd/jaeger/internal/integration/integration.go @@ -4,7 +4,6 @@ package integration import ( - "fmt" "io" "os" "os/exec" @@ -41,12 +40,11 @@ type E2EStorageIntegration struct { // This function should be called before any of the tests start. func (s *E2EStorageIntegration) e2eInitialize(t *testing.T) { logger, _ := testutils.NewLogger() - s.ConfigFile = createStorageCleanerConfig(t, s.ConfigFile) + ConfigFile := createStorageCleanerConfig(t, s.ConfigFile) - fmt.Println(s.ConfigFile) cmd := exec.Cmd{ Path: "./cmd/jaeger/jaeger", - Args: []string{"jaeger", "--config", s.ConfigFile}, + Args: []string{"jaeger", "--config", ConfigFile}, // Change the working directory to the root of this project // since the binary config file jaeger_query's ui_config points to // "./cmd/jaeger/config-ui.json" @@ -76,17 +74,17 @@ func (s *E2EStorageIntegration) e2eCleanUp(t *testing.T) { func createStorageCleanerConfig(t *testing.T, configFile string) string { data, err := os.ReadFile(configFile) require.NoError(t, err) - var config map[interface{}]interface{} + var config map[string]interface{} err = yaml.Unmarshal(data, &config) require.NoError(t, err) - service, ok := config["service"].(map[interface{}]interface{}) + service, ok := config["service"].(map[string]interface{}) require.True(t, ok) service["extensions"] = append(service["extensions"].([]interface{}), "storage_cleaner") - extensions, ok := config["extensions"].(map[interface{}]interface{}) + extensions, ok := config["extensions"].(map[string]interface{}) require.True(t, ok) - query, ok := extensions["jaeger_query"].(map[interface{}]interface{}) + query, ok := extensions["jaeger_query"].(map[string]interface{}) require.True(t, ok) trace_storage := query["trace_storage"].(string) extensions["storage_cleaner"] = map[string]string{"trace_storage": trace_storage} From 2a3750e840afd89df1ece6bae0c03434f9be968d Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Wed, 24 Apr 2024 14:06:13 +0530 Subject: [PATCH 57/69] add extension.go tests Signed-off-by: Harshvir Potpose --- .../internal/integration/integration.go | 4 +- .../integration/storagecleaner/extension.go | 4 +- .../storagecleaner/extension_test.go | 89 +++++++++++++++++++ storage/mocks/Factory.go | 4 + 4 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 cmd/jaeger/internal/integration/storagecleaner/extension_test.go diff --git a/cmd/jaeger/internal/integration/integration.go b/cmd/jaeger/internal/integration/integration.go index 6446418746d..e6404c3b9ed 100644 --- a/cmd/jaeger/internal/integration/integration.go +++ b/cmd/jaeger/internal/integration/integration.go @@ -40,11 +40,11 @@ type E2EStorageIntegration struct { // This function should be called before any of the tests start. func (s *E2EStorageIntegration) e2eInitialize(t *testing.T) { logger, _ := testutils.NewLogger() - ConfigFile := createStorageCleanerConfig(t, s.ConfigFile) + configFile := createStorageCleanerConfig(t, s.ConfigFile) cmd := exec.Cmd{ Path: "./cmd/jaeger/jaeger", - Args: []string{"jaeger", "--config", ConfigFile}, + Args: []string{"jaeger", "--config", configFile}, // Change the working directory to the root of this project // since the binary config file jaeger_query's ui_config points to // "./cmd/jaeger/config-ui.json" diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension.go b/cmd/jaeger/internal/integration/storagecleaner/extension.go index f6b5bbb630f..6ecec20844d 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension.go @@ -41,7 +41,7 @@ func newStorageCleaner(config *Config) *storageCleaner { func (c *storageCleaner) Start(ctx context.Context, host component.Host) error { storageFactory, err := jaegerstorage.GetStorageFactory(c.config.TraceStorage, host) if err != nil { - return fmt.Errorf("cannot find storage factory for Badger: %w", err) + return fmt.Errorf("cannot find storage factory: %w", err) } purgeStorage := func() error { @@ -50,7 +50,7 @@ func (c *storageCleaner) Start(ctx context.Context, host component.Host) error { return fmt.Errorf("storage %s does not implement Purger interface", c.config.TraceStorage) } if err := purger.Purge(); err != nil { - return fmt.Errorf("error purging Badger storage: %w", err) + return fmt.Errorf("error purging storage: %w", err) } return nil } diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go new file mode 100644 index 00000000000..210d79192b5 --- /dev/null +++ b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go @@ -0,0 +1,89 @@ +package storagecleaner + +import ( + "context" + "fmt" + "net/http" + "testing" + "time" + + "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage" + "github.com/jaegertracing/jaeger/storage" + factoryMocks "github.com/jaegertracing/jaeger/storage/mocks" + "github.com/open-telemetry/opentelemetry-collector-contrib/extension/storage/storagetest" + "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/component" +) + +var _ jaegerstorage.Extension = (*mockStorageExt)(nil) + +type mockStorageExt struct { + name string + factory *factoryMocks.Factory +} + +func (m *mockStorageExt) Start(ctx context.Context, host component.Host) error { + panic("not implemented") +} + +func (m *mockStorageExt) Shutdown(ctx context.Context) error { + panic("not implemented") +} + +func (m *mockStorageExt) Factory(name string) (storage.Factory, bool) { + if m.name == name { + return m.factory, true + } + return nil, false +} + +func TestStorageCleanerExtension(t *testing.T) { + config := &Config{ + TraceStorage: "storage", + Port: Port, + } + s := newStorageCleaner(config) + host := storagetest.NewStorageHost() + factory := &factoryMocks.Factory{} + host.WithExtension(jaegerstorage.ID, &mockStorageExt{ + name: "storage", + factory: factory, + }) + err := s.Start(context.Background(), host) + require.NoError(t, err) + + Addr := fmt.Sprintf("http://0.0.0.0:%s%s", Port, URL) + client := &http.Client{} + var ok bool + var resp *http.Response + for i := 0; i < 10; i++ { + r, err := http.NewRequest(http.MethodPost, Addr, nil) + require.NoError(t, err) + + resp, err = client.Do(r) + if err == nil { + ok = true + break + } + time.Sleep(1 * time.Second) + } + require.True(t, ok, "could not reach storage cleaner server") + defer resp.Body.Close() + require.Equal(t, resp.StatusCode, http.StatusOK) + s.Dependencies() + s.Shutdown(context.Background()) +} + +func TestStorageGetStorageFactoryError(t *testing.T) { + config := &Config{} + s := newStorageCleaner(config) + host := storagetest.NewStorageHost() + host.WithExtension(jaegerstorage.ID, &mockStorageExt{ + name: "storage", + factory: nil, + }) + err := s.Start(context.Background(), host) + require.Error(t, err) + require.Contains(t, err.Error(), "cannot find storage factory") + +} diff --git a/storage/mocks/Factory.go b/storage/mocks/Factory.go index 8f2c713ec1e..95c9ae700f9 100644 --- a/storage/mocks/Factory.go +++ b/storage/mocks/Factory.go @@ -111,5 +111,9 @@ func (_m *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger return r0 } +func (_m *Factory) Purge() error { + return nil +} var _ storage.Factory = (*Factory)(nil) +var _ storage.Purger = (*Factory)(nil) From af583592826308d6258de51f97b4371476d160ff Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Wed, 24 Apr 2024 14:08:03 +0530 Subject: [PATCH 58/69] fix Signed-off-by: Harshvir Potpose --- .../internal/integration/storagecleaner/extension_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go index 210d79192b5..e1ca3cd8e30 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go @@ -74,7 +74,7 @@ func TestStorageCleanerExtension(t *testing.T) { s.Shutdown(context.Background()) } -func TestStorageGetStorageFactoryError(t *testing.T) { +func TestGetStorageFactoryError(t *testing.T) { config := &Config{} s := newStorageCleaner(config) host := storagetest.NewStorageHost() From bd3d4d7db232188e415a924b265a288c94cbef99 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Wed, 24 Apr 2024 14:09:05 +0530 Subject: [PATCH 59/69] run makefmt Signed-off-by: Harshvir Potpose --- .../integration/storagecleaner/extension_test.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go index e1ca3cd8e30..58ed212e732 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go @@ -1,3 +1,6 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + package storagecleaner import ( @@ -7,12 +10,13 @@ import ( "testing" "time" - "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage" - "github.com/jaegertracing/jaeger/storage" - factoryMocks "github.com/jaegertracing/jaeger/storage/mocks" "github.com/open-telemetry/opentelemetry-collector-contrib/extension/storage/storagetest" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" + + "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage" + "github.com/jaegertracing/jaeger/storage" + factoryMocks "github.com/jaegertracing/jaeger/storage/mocks" ) var _ jaegerstorage.Extension = (*mockStorageExt)(nil) @@ -85,5 +89,4 @@ func TestGetStorageFactoryError(t *testing.T) { err := s.Start(context.Background(), host) require.Error(t, err) require.Contains(t, err.Error(), "cannot find storage factory") - } From 49406bdf36b19bb0f33eadbe99c2db9c93be4296 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Wed, 24 Apr 2024 14:15:10 +0530 Subject: [PATCH 60/69] fix lint Signed-off-by: Harshvir Potpose --- .../internal/integration/storagecleaner/extension_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go index 58ed212e732..48f80aeee31 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go @@ -73,7 +73,7 @@ func TestStorageCleanerExtension(t *testing.T) { } require.True(t, ok, "could not reach storage cleaner server") defer resp.Body.Close() - require.Equal(t, resp.StatusCode, http.StatusOK) + require.Equal(t, http.StatusOK, resp.StatusCode) s.Dependencies() s.Shutdown(context.Background()) } From 64ae824bb700056bb093a97115141e2960a41cb1 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Thu, 25 Apr 2024 01:33:28 +0530 Subject: [PATCH 61/69] fix open comments Signed-off-by: Harshvir Potpose --- .../integration/storagecleaner/extension.go | 17 ++++++------ .../storagecleaner/extension_test.go | 26 +++++++------------ .../integration/storagecleaner/factory.go | 2 +- 3 files changed, 19 insertions(+), 26 deletions(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension.go b/cmd/jaeger/internal/integration/storagecleaner/extension.go index 6ecec20844d..8b0ceb7b3b4 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension.go @@ -28,13 +28,15 @@ const ( ) type storageCleaner struct { - config *Config - server *http.Server + config *Config + server *http.Server + settings component.TelemetrySettings } -func newStorageCleaner(config *Config) *storageCleaner { +func newStorageCleaner(config *Config, telemetrySettings component.TelemetrySettings) *storageCleaner { return &storageCleaner{ - config: config, + config: config, + settings: telemetrySettings, } } @@ -71,11 +73,10 @@ func (c *storageCleaner) Start(ctx context.Context, host component.Host) error { Handler: r, ReadHeaderTimeout: 3 * time.Second, } - go func() error { - if err := c.server.ListenAndServe(); err != nil { - return fmt.Errorf("error starting storage cleaner server: %w", err) + go func() { + if err := c.server.ListenAndServe(); err != nil && err != http.ErrServerClosed { + c.settings.ReportStatus(component.NewFatalErrorEvent(err)) } - return nil }() return nil diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go index 48f80aeee31..1c58b60e7f2 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go @@ -46,41 +46,33 @@ func TestStorageCleanerExtension(t *testing.T) { TraceStorage: "storage", Port: Port, } - s := newStorageCleaner(config) + s := newStorageCleaner(config, component.TelemetrySettings{}) host := storagetest.NewStorageHost() - factory := &factoryMocks.Factory{} host.WithExtension(jaegerstorage.ID, &mockStorageExt{ name: "storage", - factory: factory, + factory: &factoryMocks.Factory{}, }) - err := s.Start(context.Background(), host) + ctx := context.Background() + err := s.Start(ctx, host) require.NoError(t, err) Addr := fmt.Sprintf("http://0.0.0.0:%s%s", Port, URL) client := &http.Client{} - var ok bool var resp *http.Response - for i := 0; i < 10; i++ { + require.Eventually(t, func() bool { r, err := http.NewRequest(http.MethodPost, Addr, nil) require.NoError(t, err) - resp, err = client.Do(r) - if err == nil { - ok = true - break - } - time.Sleep(1 * time.Second) - } - require.True(t, ok, "could not reach storage cleaner server") + return err == nil + }, 5*time.Second, 100*time.Millisecond) defer resp.Body.Close() - require.Equal(t, http.StatusOK, resp.StatusCode) s.Dependencies() - s.Shutdown(context.Background()) + require.NoError(t, s.Shutdown(ctx)) } func TestGetStorageFactoryError(t *testing.T) { config := &Config{} - s := newStorageCleaner(config) + s := newStorageCleaner(config, component.TelemetrySettings{}) host := storagetest.NewStorageHost() host.WithExtension(jaegerstorage.ID, &mockStorageExt{ name: "storage", diff --git a/cmd/jaeger/internal/integration/storagecleaner/factory.go b/cmd/jaeger/internal/integration/storagecleaner/factory.go index b4efe135648..66ab01324ff 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/factory.go +++ b/cmd/jaeger/internal/integration/storagecleaner/factory.go @@ -36,5 +36,5 @@ func createExtension( set extension.CreateSettings, cfg component.Config, ) (extension.Extension, error) { - return newStorageCleaner(cfg.(*Config)), nil + return newStorageCleaner(cfg.(*Config), set.TelemetrySettings), nil } From 5c6935eb3517fd88292d35086bdccf9f9d544a40 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Fri, 26 Apr 2024 20:37:09 +0530 Subject: [PATCH 62/69] fix Signed-off-by: Harshvir Potpose --- .../storagecleaner/extension_test.go | 79 +++++++++++++------ storage/mocks/Factory.go | 4 - 2 files changed, 53 insertions(+), 30 deletions(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go index 1c58b60e7f2..a07b030b797 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go @@ -20,10 +20,20 @@ import ( ) var _ jaegerstorage.Extension = (*mockStorageExt)(nil) +var _ storage.Factory = (*PurgerFactory)(nil) + +type PurgerFactory struct { + factoryMocks.Factory + err error +} + +func (f *PurgerFactory) Purge() error { + return f.err +} type mockStorageExt struct { name string - factory *factoryMocks.Factory + factory storage.Factory } func (m *mockStorageExt) Start(ctx context.Context, host component.Host) error { @@ -42,32 +52,49 @@ func (m *mockStorageExt) Factory(name string) (storage.Factory, bool) { } func TestStorageCleanerExtension(t *testing.T) { - config := &Config{ - TraceStorage: "storage", - Port: Port, + tests := []struct { + name string + factory storage.Factory + status int + }{ + {"good storage", &PurgerFactory{}, http.StatusOK}, + {"good storage with error", &PurgerFactory{err: fmt.Errorf("error")}, http.StatusInternalServerError}, + {"bad storage", &factoryMocks.Factory{}, http.StatusInternalServerError}, } - s := newStorageCleaner(config, component.TelemetrySettings{}) - host := storagetest.NewStorageHost() - host.WithExtension(jaegerstorage.ID, &mockStorageExt{ - name: "storage", - factory: &factoryMocks.Factory{}, - }) - ctx := context.Background() - err := s.Start(ctx, host) - require.NoError(t, err) - - Addr := fmt.Sprintf("http://0.0.0.0:%s%s", Port, URL) - client := &http.Client{} - var resp *http.Response - require.Eventually(t, func() bool { - r, err := http.NewRequest(http.MethodPost, Addr, nil) - require.NoError(t, err) - resp, err = client.Do(r) - return err == nil - }, 5*time.Second, 100*time.Millisecond) - defer resp.Body.Close() - s.Dependencies() - require.NoError(t, s.Shutdown(ctx)) + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + config := &Config{ + TraceStorage: "storage", + Port: Port, + } + s := newStorageCleaner(config, component.TelemetrySettings{}) + host := storagetest.NewStorageHost() + host.WithExtension(jaegerstorage.ID, &mockStorageExt{ + name: "storage", + factory: test.factory, + }) + ctx := context.Background() + err := s.Start(ctx, host) + defer s.Shutdown(ctx) + require.NoError(t, err) + + addr := fmt.Sprintf("http://0.0.0.0:%s%s", Port, URL) + client := &http.Client{} + var resp *http.Response + require.Eventually(t, func() bool { + r, err := http.NewRequest(http.MethodPost, addr, nil) + require.NoError(t, err) + resp, err = client.Do(r) + require.NoError(t, err) + defer resp.Body.Close() + return err == nil + }, 5*time.Second, 100*time.Millisecond) + require.Equal(t, test.status, resp.StatusCode) + s.Dependencies() + }) + } + } func TestGetStorageFactoryError(t *testing.T) { diff --git a/storage/mocks/Factory.go b/storage/mocks/Factory.go index 95c9ae700f9..8f2c713ec1e 100644 --- a/storage/mocks/Factory.go +++ b/storage/mocks/Factory.go @@ -111,9 +111,5 @@ func (_m *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger return r0 } -func (_m *Factory) Purge() error { - return nil -} var _ storage.Factory = (*Factory)(nil) -var _ storage.Purger = (*Factory)(nil) From e517f0687eedb19f5c0815bb24755b836054c2b8 Mon Sep 17 00:00:00 2001 From: Harshvir Potpose Date: Fri, 26 Apr 2024 20:38:39 +0530 Subject: [PATCH 63/69] fix lint Signed-off-by: Harshvir Potpose --- .../internal/integration/storagecleaner/extension_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go index a07b030b797..43aa71a895b 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go @@ -19,8 +19,10 @@ import ( factoryMocks "github.com/jaegertracing/jaeger/storage/mocks" ) -var _ jaegerstorage.Extension = (*mockStorageExt)(nil) -var _ storage.Factory = (*PurgerFactory)(nil) +var ( + _ jaegerstorage.Extension = (*mockStorageExt)(nil) + _ storage.Factory = (*PurgerFactory)(nil) +) type PurgerFactory struct { factoryMocks.Factory @@ -94,7 +96,6 @@ func TestStorageCleanerExtension(t *testing.T) { s.Dependencies() }) } - } func TestGetStorageFactoryError(t *testing.T) { From 4906bc0cec5c6f6186ccc690c4ae77bb23a8fe1d Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Fri, 26 Apr 2024 13:28:28 -0400 Subject: [PATCH 64/69] clean up test Signed-off-by: Yuri Shkuro --- .../storagecleaner/extension_test.go | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go index 43aa71a895b..316aed7509f 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go @@ -59,9 +59,20 @@ func TestStorageCleanerExtension(t *testing.T) { factory storage.Factory status int }{ - {"good storage", &PurgerFactory{}, http.StatusOK}, - {"good storage with error", &PurgerFactory{err: fmt.Errorf("error")}, http.StatusInternalServerError}, - {"bad storage", &factoryMocks.Factory{}, http.StatusInternalServerError}, + { + name: "good storage", + factory: &PurgerFactory{}, + status: http.StatusOK}, + { + name: "good storage with error", + factory: &PurgerFactory{err: fmt.Errorf("error")}, + status: http.StatusInternalServerError, + }, + { + name: "bad storage", + factory: &factoryMocks.Factory{}, + status: http.StatusInternalServerError, + }, } for _, test := range tests { @@ -71,6 +82,7 @@ func TestStorageCleanerExtension(t *testing.T) { Port: Port, } s := newStorageCleaner(config, component.TelemetrySettings{}) + require.NotEmpty(t, s.Dependencies()) host := storagetest.NewStorageHost() host.WithExtension(jaegerstorage.ID, &mockStorageExt{ name: "storage", @@ -83,17 +95,14 @@ func TestStorageCleanerExtension(t *testing.T) { addr := fmt.Sprintf("http://0.0.0.0:%s%s", Port, URL) client := &http.Client{} - var resp *http.Response require.Eventually(t, func() bool { r, err := http.NewRequest(http.MethodPost, addr, nil) require.NoError(t, err) - resp, err = client.Do(r) + resp, err := client.Do(r) require.NoError(t, err) defer resp.Body.Close() - return err == nil + return test.status == resp.StatusCode }, 5*time.Second, 100*time.Millisecond) - require.Equal(t, test.status, resp.StatusCode) - s.Dependencies() }) } } From 1db0b33bbdbc46c6a130e36d44a3f6bee41cdbb0 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Fri, 26 Apr 2024 13:38:01 -0400 Subject: [PATCH 65/69] make fmt Signed-off-by: Yuri Shkuro --- .../internal/integration/storagecleaner/extension_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go index 316aed7509f..a5c5d845475 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go @@ -62,7 +62,8 @@ func TestStorageCleanerExtension(t *testing.T) { { name: "good storage", factory: &PurgerFactory{}, - status: http.StatusOK}, + status: http.StatusOK, + }, { name: "good storage with error", factory: &PurgerFactory{err: fmt.Errorf("error")}, From a55c88b33fe145d07b1c02cbfec6f38881481e6f Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Fri, 26 Apr 2024 14:08:42 -0400 Subject: [PATCH 66/69] cleanup Signed-off-by: Yuri Shkuro --- .../internal/integration/storagecleaner/extension_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go index a5c5d845475..acbc5fb6351 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go @@ -89,10 +89,8 @@ func TestStorageCleanerExtension(t *testing.T) { name: "storage", factory: test.factory, }) - ctx := context.Background() - err := s.Start(ctx, host) - defer s.Shutdown(ctx) - require.NoError(t, err) + require.NoError(t, s.Start(context.Background(), host)) + defer s.Shutdown(context.Background()) addr := fmt.Sprintf("http://0.0.0.0:%s%s", Port, URL) client := &http.Client{} From cc016877e41eea78ee57f67b08b1520ad6130460 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Fri, 26 Apr 2024 14:52:02 -0400 Subject: [PATCH 67/69] add tests Signed-off-by: Yuri Shkuro --- .../integration/storagecleaner/extension.go | 11 +++- .../storagecleaner/extension_test.go | 52 +++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension.go b/cmd/jaeger/internal/integration/storagecleaner/extension.go index 8b0ceb7b3b4..d39726cb0af 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension.go @@ -27,9 +27,15 @@ const ( URL = "/purge" ) +// allow mocking the server in tests +type httpServer interface { + ListenAndServe() error + Shutdown(ctx context.Context) error +} + type storageCleaner struct { config *Config - server *http.Server + server httpServer settings component.TelemetrySettings } @@ -43,7 +49,7 @@ func newStorageCleaner(config *Config, telemetrySettings component.TelemetrySett func (c *storageCleaner) Start(ctx context.Context, host component.Host) error { storageFactory, err := jaegerstorage.GetStorageFactory(c.config.TraceStorage, host) if err != nil { - return fmt.Errorf("cannot find storage factory: %w", err) + return fmt.Errorf("cannot find storage factory '%s': %w", c.config.TraceStorage, err) } purgeStorage := func() error { @@ -75,6 +81,7 @@ func (c *storageCleaner) Start(ctx context.Context, host component.Host) error { } go func() { if err := c.server.ListenAndServe(); err != nil && err != http.ErrServerClosed { + err = fmt.Errorf("error starting cleaner server: %w", err) c.settings.ReportStatus(component.NewFatalErrorEvent(err)) } }() diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go index acbc5fb6351..e1d0ab5be59 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go @@ -5,12 +5,14 @@ package storagecleaner import ( "context" + "errors" "fmt" "net/http" "testing" "time" "github.com/open-telemetry/opentelemetry-collector-contrib/extension/storage/storagetest" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" @@ -118,3 +120,53 @@ func TestGetStorageFactoryError(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), "cannot find storage factory") } + +func TestStorageExtensionStartError(t *testing.T) { + config := &Config{ + TraceStorage: "storage", + Port: "invalid-port", + } + var startErr error + s := newStorageCleaner(config, component.TelemetrySettings{ + ReportStatus: func(status *component.StatusEvent) { + startErr = status.Err() + }, + }) + host := storagetest.NewStorageHost().WithExtension( + jaegerstorage.ID, + &mockStorageExt{ + name: "storage", + factory: &PurgerFactory{}, + }) + require.NoError(t, s.Start(context.Background(), host)) + assert.Eventually(t, func() bool { + return startErr != nil + }, 5*time.Second, 100*time.Millisecond) + require.Contains(t, startErr.Error(), "error starting cleaner server") +} + +type mockServer struct{} + +func (*mockServer) ListenAndServe() error { return nil } +func (*mockServer) Shutdown(ctx context.Context) error { return errors.New("shutdown error") } + +func TestStorageExtensionShutdownError(t *testing.T) { + config := &Config{ + TraceStorage: "storage", + Port: ":0", + } + s := newStorageCleaner(config, component.TelemetrySettings{}) + host := storagetest.NewStorageHost().WithExtension( + jaegerstorage.ID, + &mockStorageExt{ + name: "storage", + factory: &PurgerFactory{}, + }) + require.NoError(t, s.Start(context.Background(), host)) + require.NoError(t, s.Shutdown(context.Background())) + + s.server = &mockServer{} + err := s.Shutdown(context.Background()) + require.Error(t, err) + require.Contains(t, err.Error(), "shutdown error") +} From 927b25081d4cfa5a951017cc52400f0e97493975 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Fri, 26 Apr 2024 15:22:08 -0400 Subject: [PATCH 68/69] fix tests Signed-off-by: Yuri Shkuro --- .../internal/integration/storagecleaner/extension.go | 3 ++- .../integration/storagecleaner/extension_test.go | 9 +++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension.go b/cmd/jaeger/internal/integration/storagecleaner/extension.go index d39726cb0af..86b70d408ee 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension.go @@ -5,6 +5,7 @@ package storagecleaner import ( "context" + "errors" "fmt" "net/http" "time" @@ -80,7 +81,7 @@ func (c *storageCleaner) Start(ctx context.Context, host component.Host) error { ReadHeaderTimeout: 3 * time.Second, } go func() { - if err := c.server.ListenAndServe(); err != nil && err != http.ErrServerClosed { + if err := c.server.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { err = fmt.Errorf("error starting cleaner server: %w", err) c.settings.ReportStatus(component.NewFatalErrorEvent(err)) } diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go index e1d0ab5be59..ecb9b06a0fa 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "net/http" + "sync/atomic" "testing" "time" @@ -126,10 +127,10 @@ func TestStorageExtensionStartError(t *testing.T) { TraceStorage: "storage", Port: "invalid-port", } - var startErr error + var startStatus atomic.Pointer[component.StatusEvent] s := newStorageCleaner(config, component.TelemetrySettings{ ReportStatus: func(status *component.StatusEvent) { - startErr = status.Err() + startStatus.Store(status) }, }) host := storagetest.NewStorageHost().WithExtension( @@ -140,9 +141,9 @@ func TestStorageExtensionStartError(t *testing.T) { }) require.NoError(t, s.Start(context.Background(), host)) assert.Eventually(t, func() bool { - return startErr != nil + return startStatus.Load() != nil }, 5*time.Second, 100*time.Millisecond) - require.Contains(t, startErr.Error(), "error starting cleaner server") + require.Contains(t, startStatus.Load().Err().Error(), "error starting cleaner server") } type mockServer struct{} From c2fcfe323edfa12b63071ed3e2a5f78b2f4de7e3 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Fri, 26 Apr 2024 15:36:16 -0400 Subject: [PATCH 69/69] rm funky test Signed-off-by: Yuri Shkuro --- .../integration/storagecleaner/extension.go | 8 +----- .../storagecleaner/extension_test.go | 27 ------------------- 2 files changed, 1 insertion(+), 34 deletions(-) diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension.go b/cmd/jaeger/internal/integration/storagecleaner/extension.go index 86b70d408ee..5ef26eb1867 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension.go @@ -28,15 +28,9 @@ const ( URL = "/purge" ) -// allow mocking the server in tests -type httpServer interface { - ListenAndServe() error - Shutdown(ctx context.Context) error -} - type storageCleaner struct { config *Config - server httpServer + server *http.Server settings component.TelemetrySettings } diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go index ecb9b06a0fa..26fb08a47e2 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go @@ -5,7 +5,6 @@ package storagecleaner import ( "context" - "errors" "fmt" "net/http" "sync/atomic" @@ -145,29 +144,3 @@ func TestStorageExtensionStartError(t *testing.T) { }, 5*time.Second, 100*time.Millisecond) require.Contains(t, startStatus.Load().Err().Error(), "error starting cleaner server") } - -type mockServer struct{} - -func (*mockServer) ListenAndServe() error { return nil } -func (*mockServer) Shutdown(ctx context.Context) error { return errors.New("shutdown error") } - -func TestStorageExtensionShutdownError(t *testing.T) { - config := &Config{ - TraceStorage: "storage", - Port: ":0", - } - s := newStorageCleaner(config, component.TelemetrySettings{}) - host := storagetest.NewStorageHost().WithExtension( - jaegerstorage.ID, - &mockStorageExt{ - name: "storage", - factory: &PurgerFactory{}, - }) - require.NoError(t, s.Start(context.Background(), host)) - require.NoError(t, s.Shutdown(context.Background())) - - s.server = &mockServer{} - err := s.Shutdown(context.Background()) - require.Error(t, err) - require.Contains(t, err.Error(), "shutdown error") -}