From 54a67f5efc850902e8ea3b454a251097e38ce485 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 16 Jul 2024 13:06:26 -0300 Subject: [PATCH] [v2] Add e2e test with memory store (#5751) ## Which problem is this PR solving? - Even though we were using memory storage in v2-e2e-grpc test, we never had an e2e that would validate `cmd/jaeger/config.yaml` config, which uses memory storage in-process ## Description of the changes - Add the new test workflow an e2e test - Rename other test files for more readable file names / test names ## How was this change tested? - build cmd/jaeger binary - `$ STORAGE=memory go test ./cmd/jaeger/internal/integration` --------- Signed-off-by: Yuri Shkuro --- .github/workflows/ci-e2e-memory.yaml | 41 +++++++++++++++++++ .../internal/integration/e2e_integration.go | 5 ++- .../{es_test.go => elasticsearch_test.go} | 2 +- .../internal/integration/memory_test.go | 27 ++++++++++++ .../{os_test.go => opensearch_test.go} | 2 +- .../integration/storagecleaner/extension.go | 16 ++++---- .../storagecleaner/extension_test.go | 10 ++++- plugin/storage/memory/factory.go | 9 ++++ plugin/storage/memory/memory.go | 7 ++++ 9 files changed, 107 insertions(+), 12 deletions(-) create mode 100644 .github/workflows/ci-e2e-memory.yaml rename cmd/jaeger/internal/integration/{es_test.go => elasticsearch_test.go} (93%) create mode 100644 cmd/jaeger/internal/integration/memory_test.go rename cmd/jaeger/internal/integration/{os_test.go => opensearch_test.go} (93%) diff --git a/.github/workflows/ci-e2e-memory.yaml b/.github/workflows/ci-e2e-memory.yaml new file mode 100644 index 00000000000..3a8dce7d370 --- /dev/null +++ b/.github/workflows/ci-e2e-memory.yaml @@ -0,0 +1,41 @@ +name: CIT Memory + +on: + push: + branches: [main] + + pull_request: + branches: [main] + +concurrency: + group: ${{ github.workflow }}-${{ (github.event.pull_request && github.event.pull_request.number) || github.ref || github.run_id }} + cancel-in-progress: true + +# See https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions +permissions: # added using https://github.com/step-security/secure-workflows + contents: read + +jobs: + memory-v2: + runs-on: ubuntu-latest + steps: + - name: Harden Runner + uses: step-security/harden-runner@17d0e2bd7d51742c71671bd19fa12bdc9d40a3d6 # v2.8.1 + with: + egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs + + - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 + + - uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5.0.2 + with: + go-version: 1.22.x + + - name: Run Memory storage integration tests + run: | + STORAGE=memory_v2 make jaeger-v2-storage-integration-test + + - name: Upload coverage to codecov + uses: ./.github/actions/upload-codecov + with: + files: cover.out + flags: memory_v2 diff --git a/cmd/jaeger/internal/integration/e2e_integration.go b/cmd/jaeger/internal/integration/e2e_integration.go index 6ddcb538cff..670b7b576d4 100644 --- a/cmd/jaeger/internal/integration/e2e_integration.go +++ b/cmd/jaeger/internal/integration/e2e_integration.go @@ -177,6 +177,7 @@ func createStorageCleanerConfig(t *testing.T, configFile string, storage string) func purge(t *testing.T) { addr := fmt.Sprintf("http://0.0.0.0:%s%s", storagecleaner.Port, storagecleaner.URL) + t.Logf("Purging storage via %s", addr) r, err := http.NewRequestWithContext(context.Background(), http.MethodPost, addr, nil) require.NoError(t, err) @@ -185,6 +186,8 @@ func purge(t *testing.T) { resp, err := client.Do(r) require.NoError(t, err) defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) - require.Equal(t, http.StatusOK, resp.StatusCode) + require.Equal(t, http.StatusOK, resp.StatusCode, "body: %s", string(body)) } diff --git a/cmd/jaeger/internal/integration/es_test.go b/cmd/jaeger/internal/integration/elasticsearch_test.go similarity index 93% rename from cmd/jaeger/internal/integration/es_test.go rename to cmd/jaeger/internal/integration/elasticsearch_test.go index 92b9f0c79e0..2c78bfdb7af 100644 --- a/cmd/jaeger/internal/integration/es_test.go +++ b/cmd/jaeger/internal/integration/elasticsearch_test.go @@ -9,7 +9,7 @@ import ( "github.com/jaegertracing/jaeger/plugin/storage/integration" ) -func TestESStorage(t *testing.T) { +func TestElasticsearchStorage(t *testing.T) { integration.SkipUnlessEnv(t, "elasticsearch") s := &E2EStorageIntegration{ diff --git a/cmd/jaeger/internal/integration/memory_test.go b/cmd/jaeger/internal/integration/memory_test.go new file mode 100644 index 00000000000..a2592b04a38 --- /dev/null +++ b/cmd/jaeger/internal/integration/memory_test.go @@ -0,0 +1,27 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + +package integration + +import ( + "testing" + + "github.com/jaegertracing/jaeger/plugin/storage/integration" +) + +func TestMemoryStorage(t *testing.T) { + integration.SkipUnlessEnv(t, "memory_v2") + + s := &E2EStorageIntegration{ + ConfigFile: "../../config.yaml", + StorageIntegration: integration.StorageIntegration{ + SkipArchiveTest: true, + CleanUp: purge, + }, + } + s.e2eInitialize(t, "memory") + t.Cleanup(func() { + s.e2eCleanUp(t) + }) + s.RunAll(t) +} diff --git a/cmd/jaeger/internal/integration/os_test.go b/cmd/jaeger/internal/integration/opensearch_test.go similarity index 93% rename from cmd/jaeger/internal/integration/os_test.go rename to cmd/jaeger/internal/integration/opensearch_test.go index a5d3e945214..909c0be6510 100644 --- a/cmd/jaeger/internal/integration/os_test.go +++ b/cmd/jaeger/internal/integration/opensearch_test.go @@ -9,7 +9,7 @@ import ( "github.com/jaegertracing/jaeger/plugin/storage/integration" ) -func TestOSStorage(t *testing.T) { +func TestOpenSearchStorage(t *testing.T) { integration.SkipUnlessEnv(t, "opensearch") s := &E2EStorageIntegration{ ConfigFile: "../../config-opensearch.yaml", diff --git a/cmd/jaeger/internal/integration/storagecleaner/extension.go b/cmd/jaeger/internal/integration/storagecleaner/extension.go index 2a8b37016e6..c49d9b7bff0 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension.go @@ -13,6 +13,7 @@ import ( "github.com/gorilla/mux" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/extension" + "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage" "github.com/jaegertracing/jaeger/storage" @@ -29,15 +30,15 @@ const ( ) type storageCleaner struct { - config *Config - server *http.Server - settings component.TelemetrySettings + config *Config + server *http.Server + telset component.TelemetrySettings } -func newStorageCleaner(config *Config, telemetrySettings component.TelemetrySettings) *storageCleaner { +func newStorageCleaner(config *Config, telset component.TelemetrySettings) *storageCleaner { return &storageCleaner{ - config: config, - settings: telemetrySettings, + config: config, + telset: telset, } } @@ -74,10 +75,11 @@ func (c *storageCleaner) Start(_ context.Context, host component.Host) error { Handler: r, ReadHeaderTimeout: 3 * time.Second, } + c.telset.Logger.Info("Starting storage cleaner server", zap.String("addr", c.server.Addr)) go func() { 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)) + c.telset.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 23d05c28e5c..b95b21b69f5 100644 --- a/cmd/jaeger/internal/integration/storagecleaner/extension_test.go +++ b/cmd/jaeger/internal/integration/storagecleaner/extension_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" + "go.uber.org/zap/zaptest" "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage" "github.com/jaegertracing/jaeger/storage" @@ -92,7 +93,9 @@ func TestStorageCleanerExtension(t *testing.T) { TraceStorage: "storage", Port: Port, } - s := newStorageCleaner(config, component.TelemetrySettings{}) + s := newStorageCleaner(config, component.TelemetrySettings{ + Logger: zaptest.NewLogger(t), + }) require.NotEmpty(t, s.Dependencies()) host := storagetest.NewStorageHost() host.WithExtension(jaegerstorage.ID, &mockStorageExt{ @@ -118,7 +121,9 @@ func TestStorageCleanerExtension(t *testing.T) { func TestGetStorageFactoryError(t *testing.T) { config := &Config{} - s := newStorageCleaner(config, component.TelemetrySettings{}) + s := newStorageCleaner(config, component.TelemetrySettings{ + Logger: zaptest.NewLogger(t), + }) host := storagetest.NewStorageHost() host.WithExtension(jaegerstorage.ID, &mockStorageExt{ name: "storage", @@ -136,6 +141,7 @@ func TestStorageExtensionStartError(t *testing.T) { } var startStatus atomic.Pointer[component.StatusEvent] s := newStorageCleaner(config, component.TelemetrySettings{ + Logger: zaptest.NewLogger(t), ReportStatus: func(status *component.StatusEvent) { startStatus.Store(status) }, diff --git a/plugin/storage/memory/factory.go b/plugin/storage/memory/factory.go index 6c677b03d95..d9bfd6c113a 100644 --- a/plugin/storage/memory/factory.go +++ b/plugin/storage/memory/factory.go @@ -16,6 +16,7 @@ package memory import ( + "context" "flag" "github.com/spf13/viper" @@ -36,6 +37,7 @@ var ( // interface comformance checks _ storage.ArchiveFactory = (*Factory)(nil) _ storage.SamplingStoreFactory = (*Factory)(nil) _ plugin.Configurable = (*Factory)(nil) + _ storage.Purger = (*Factory)(nil) ) // Factory implements storage.Factory and creates storage components backed by memory store. @@ -126,3 +128,10 @@ func (*Factory) CreateLock() (distributedlock.Lock, error) { func (f *Factory) publishOpts() { safeexpvar.SetInt("jaeger_storage_memory_max_traces", int64(f.options.Configuration.MaxTraces)) } + +// Purge removes all data from the Factory's underlying Memory store. +// This function is intended for testing purposes only and should not be used in production environments. +func (f *Factory) Purge(ctx context.Context) error { + f.store.purge(ctx) + return nil +} diff --git a/plugin/storage/memory/memory.go b/plugin/storage/memory/memory.go index f92ae9949e9..9bac7d2b2bd 100644 --- a/plugin/storage/memory/memory.go +++ b/plugin/storage/memory/memory.go @@ -337,3 +337,10 @@ func flattenTags(span *model.Span) model.KeyValues { } return retMe } + +// purge supports Purger interface. +func (st *Store) purge(context.Context) { + st.Lock() + st.perTenant = make(map[string]*Tenant) + st.Unlock() +}