From d7ab0f8c175956c1d36bbc2e18b34469f568e205 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab <43658574+mahadzaryab1@users.noreply.github.com> Date: Sat, 25 Jan 2025 14:04:23 -0500 Subject: [PATCH] [storage] Remove distinction between primary and `archive` storage interfaces (#6567) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Which problem is this PR solving? - Towards #6065 ## Description of the changes - This PR completely removes the interface `storage.ArchiveFactory` by refactoring all the storage implementations to remove the distinction between a primary and archive interface. Note that the concept of archive storage remains the same within Jaeger, we just now use the same interface to handle both primary and archive storages. - 🛑 Breaking change for users of GRPC storage 🛑 - The GRPC storage was changed to only dispatch to the primary storage instead of being able to dispatch to a primary and archive storage. - Mitigation for jaeger-v1 users: In order to restore your archive storage, configure a new GRPC server on a different port and specify it via `--grpc-storage-archive.server`. Archive storage will also need to be enabled via `--grpc-storage-archive.enabled=true` - Mitigation for jaeger-v2 users: In order to restore your archive storage, configured a new GRPC server on a different port and specify it via a new storage backend in `jaeger_storage.backends` (an example of this can be viewed at https://github.com/jaegertracing/jaeger/blob/main/cmd/jaeger/config-remote-storage.yaml) ## How was this change tested? ### gRPC Storage #### On main (establish ground truth) Start the remote storage binary (uses memory storage by default which implements the ArchiveFactory interface) ``` go run ./cmd/remote-storage ``` Start the all in one binary configured with grpc storage (jaeger-v1) ``` SPAN_STORAGE_TYPE=grpc go run ./cmd/all-in-one --grpc-storage.server=http://localhost:17271 ``` Traces can be archived from the UI Screenshot 2025-01-23 at 10 17 32 PM For jaeger-v2, change the extension section of `cmd/jaeger/internal/all-in-one.yaml` to be the following ```yaml jaeger_query: storage: traces: some_storage traces_archive: some_storage jaeger_storage: backends: some_storage: grpc: endpoint: localhost:17271 tls: insecure: true ``` and then start the binary as follows: ``` go run ./cmd/jaeger/ ``` #### For current PR Stop both binaries and checkout this PR ``` gh pr checkout 6567 ``` Start two remote storage binaries (in two separate terminals) ``` go run ./cmd/remote-storage --admin.http.host-port=:17270 --grpc.host-port=:17271 go run ./cmd/remote-storage --admin.http.host-port=:17272 --grpc.host-port=:17273 ``` Start the all-in-one binary with explicit archive configurations ``` SPAN_STORAGE_TYPE=grpc go run ./cmd/all-in-one --grpc-storage.server=http://localhost:17271 --grpc-storage-archive.enabled=true --grpc-storage-archive.server=http://localhost:17273 ``` Traces can be once again archived from the UI Screenshot 2025-01-23 at 10 25 29 PM For jaeger-v2, the configuration was changed to the following: ```yaml extensions: jaeger_query: storage: traces: some_storage traces_archive: another_storage jaeger_storage: backends: some_storage: grpc: endpoint: localhost:17271 tls: insecure: true another_storage: grpc: endpoint: localhost:17273 tls: insecure: true ``` Try running all-in-one without archive-storage enabled ``` SPAN_STORAGE_TYPE=grpc go run ./cmd/all-in-one --grpc-storage.server=http://localhost:17271 ``` We cannot archive traces Screenshot 2025-01-23 at 10 26 53 PM ### CLI Flags #### ElasticSearch CLI Flags ``` git checkout main SPAN_STORAGE_TYPE=elasticsearch go run ./cmd/collector help > es_main git checkout v1-es-archive SPAN_STORAGE_TYPE=elasticsearch go run ./cmd/collector help > es_current ``` The diff can be viewed [here](https://www.diffchecker.com/xXuFqnOc/). There is no difference. #### Cassandra CLI Flags ``` git checkout main SPAN_STORAGE_TYPE=cassandra go run ./cmd/collector help > cassandra_main git checkout v1-es-archive SPAN_STORAGE_TYPE=cassandra go run ./cmd/collector help > cassandra_current ``` The diff can be viewed [here](https://www.diffchecker.com/x74PKvhA/). There are a few here differences here in which `cassandra-archive.*` gains some new configuration options that were previously only existed for the primary storage. `cassandra-archive.*` also gains some defaults that will be the same as the the primary storage. ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: Mahad Zaryab Signed-off-by: Mahad Zaryab <43658574+mahadzaryab1@users.noreply.github.com> --- cmd/all-in-one/main.go | 4 +- cmd/jaeger/config-remote-storage.yaml | 6 + .../extension/jaegerstorage/config.go | 2 +- .../extension/jaegerstorage/config_test.go | 2 +- cmd/jaeger/internal/integration/grpc_test.go | 9 +- cmd/query/app/flags.go | 23 +- cmd/query/app/flags_test.go | 61 ++--- cmd/query/app/querysvc/query_service.go | 33 --- cmd/query/app/querysvc/query_service_test.go | 52 +--- cmd/query/app/token_propagation_test.go | 2 +- cmd/query/main.go | 10 +- cmd/remote-storage/app/server.go | 22 +- cmd/remote-storage/app/server_test.go | 104 ++++---- pkg/es/config/config.go | 8 + plugin/storage/badger/factory.go | 12 +- plugin/storage/blackhole/factory.go | 16 +- plugin/storage/blackhole/factory_test.go | 6 - plugin/storage/cassandra/factory.go | 117 +++------ plugin/storage/cassandra/factory_test.go | 176 ++++++++----- plugin/storage/cassandra/options.go | 74 ++---- plugin/storage/cassandra/options_test.go | 43 +--- plugin/storage/es/factory.go | 190 +++++--------- plugin/storage/es/factory_test.go | 236 +++++++++--------- plugin/storage/es/options.go | 51 +--- plugin/storage/es/options_test.go | 53 +--- plugin/storage/factory.go | 128 +++++++--- plugin/storage/factory_test.go | 212 ++++++++++++---- plugin/storage/grpc/config.go | 1 + plugin/storage/grpc/factory.go | 70 ++---- plugin/storage/grpc/factory_test.go | 98 ++++---- plugin/storage/grpc/options.go | 48 +++- plugin/storage/grpc/options_test.go | 18 +- plugin/storage/grpc/shared/archive.go | 81 ------ plugin/storage/grpc/shared/archive_test.go | 106 -------- plugin/storage/grpc/shared/grpc_client.go | 39 +-- .../storage/grpc/shared/grpc_client_test.go | 149 ++--------- .../storage/grpc/shared/grpc_handler_test.go | 48 ++-- plugin/storage/grpc/shared/interface.go | 9 - plugin/storage/integration/cassandra_test.go | 17 +- .../storage/integration/elasticsearch_test.go | 42 ++-- plugin/storage/integration/grpc_test.go | 39 ++- .../integration/remote_memory_storage.go | 4 +- plugin/storage/memory/factory.go | 18 -- storage/factory.go | 30 +-- storage/mocks/ArchiveCapable.go | 47 ++++ storage/mocks/ArchiveFactory.go | 92 ------- storage/mocks/Inheritable.go | 37 +++ storage_v2/v1adapter/archive.go | 43 ---- storage_v2/v1adapter/archive_test.go | 112 --------- 49 files changed, 1126 insertions(+), 1674 deletions(-) delete mode 100644 plugin/storage/grpc/shared/archive.go delete mode 100644 plugin/storage/grpc/shared/archive_test.go create mode 100644 storage/mocks/ArchiveCapable.go delete mode 100644 storage/mocks/ArchiveFactory.go create mode 100644 storage/mocks/Inheritable.go delete mode 100644 storage_v2/v1adapter/archive.go delete mode 100644 storage_v2/v1adapter/archive_test.go diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index b393a6c962d..502efe2c895 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -171,8 +171,8 @@ by default uses only in-memory database.`, queryTelset := baseTelset // copy queryTelset.Metrics = queryMetricsFactory querySrv := startQuery( - svc, qOpts, qOpts.BuildQueryServiceOptions(storageFactory, logger), - qOpts.BuildQueryServiceOptionsV2(storageFactory, logger), + svc, qOpts, qOpts.BuildQueryServiceOptions(storageFactory.InitArchiveStorage, logger), + qOpts.BuildQueryServiceOptionsV2(storageFactory.InitArchiveStorage, logger), traceReader, dependencyReader, metricsQueryService, tm, queryTelset, ) diff --git a/cmd/jaeger/config-remote-storage.yaml b/cmd/jaeger/config-remote-storage.yaml index 655703b2034..966812aaffb 100644 --- a/cmd/jaeger/config-remote-storage.yaml +++ b/cmd/jaeger/config-remote-storage.yaml @@ -24,6 +24,7 @@ extensions: jaeger_query: storage: traces: some-storage + traces_archive: another-storage ui: config_file: ./cmd/jaeger/config-ui.json @@ -34,6 +35,11 @@ extensions: endpoint: localhost:17271 tls: insecure: true + another-storage: + grpc: + endpoint: localhost:17272 + tls: + insecure: true receivers: otlp: diff --git a/cmd/jaeger/internal/extension/jaegerstorage/config.go b/cmd/jaeger/internal/extension/jaegerstorage/config.go index 460e5cbc78f..0612a805019 100644 --- a/cmd/jaeger/internal/extension/jaegerstorage/config.go +++ b/cmd/jaeger/internal/extension/jaegerstorage/config.go @@ -74,7 +74,7 @@ func (cfg *TraceBackend) Unmarshal(conf *confmap.Conf) error { } if conf.IsSet("cassandra") { cfg.Cassandra = &cassandra.Options{ - Primary: cassandra.NamespaceConfig{ + NamespaceConfig: cassandra.NamespaceConfig{ Configuration: casCfg.DefaultConfiguration(), Enabled: true, }, diff --git a/cmd/jaeger/internal/extension/jaegerstorage/config_test.go b/cmd/jaeger/internal/extension/jaegerstorage/config_test.go index 9f531caf255..c8e02148a7a 100644 --- a/cmd/jaeger/internal/extension/jaegerstorage/config_test.go +++ b/cmd/jaeger/internal/extension/jaegerstorage/config_test.go @@ -83,7 +83,7 @@ backends: `) cfg := createDefaultConfig().(*Config) require.NoError(t, conf.Unmarshal(cfg)) - assert.NotEmpty(t, cfg.TraceBackends["some_storage"].Cassandra.Primary.Connection.Servers) + assert.NotEmpty(t, cfg.TraceBackends["some_storage"].Cassandra.Configuration.Connection.Servers) } func TestConfigDefaultElasticsearch(t *testing.T) { diff --git a/cmd/jaeger/internal/integration/grpc_test.go b/cmd/jaeger/internal/integration/grpc_test.go index cd12984bc62..a3579f350e6 100644 --- a/cmd/jaeger/internal/integration/grpc_test.go +++ b/cmd/jaeger/internal/integration/grpc_test.go @@ -7,20 +7,24 @@ import ( "testing" "github.com/jaegertracing/jaeger/plugin/storage/integration" + "github.com/jaegertracing/jaeger/ports" ) type GRPCStorageIntegration struct { E2EStorageIntegration - remoteStorage *integration.RemoteMemoryStorage + remoteStorage *integration.RemoteMemoryStorage + archiveRemoteStorage *integration.RemoteMemoryStorage } func (s *GRPCStorageIntegration) initialize(t *testing.T) { - s.remoteStorage = integration.StartNewRemoteMemoryStorage(t) + s.remoteStorage = integration.StartNewRemoteMemoryStorage(t, ports.RemoteStorageGRPC) + s.archiveRemoteStorage = integration.StartNewRemoteMemoryStorage(t, ports.RemoteStorageGRPC+1) } func (s *GRPCStorageIntegration) cleanUp(t *testing.T) { s.remoteStorage.Close(t) + s.archiveRemoteStorage.Close(t) s.initialize(t) } @@ -37,6 +41,7 @@ func TestGRPCStorage(t *testing.T) { s.e2eInitialize(t, "grpc") t.Cleanup(func() { s.remoteStorage.Close(t) + s.archiveRemoteStorage.Close(t) }) s.RunSpanStoreTests(t) } diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go index 23072b16d90..2110c9f2a70 100644 --- a/cmd/query/app/flags.go +++ b/cmd/query/app/flags.go @@ -30,7 +30,7 @@ import ( "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/tenancy" "github.com/jaegertracing/jaeger/ports" - "github.com/jaegertracing/jaeger/storage" + "github.com/jaegertracing/jaeger/storage/spanstore" "github.com/jaegertracing/jaeger/storage_v2/v1adapter" ) @@ -138,10 +138,19 @@ func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*Q return qOpts, nil } +type InitArchiveStorageFn func(logger *zap.Logger) (spanstore.Reader, spanstore.Writer) + // BuildQueryServiceOptions creates a QueryServiceOptions struct with appropriate adjusters and archive config -func (qOpts *QueryOptions) BuildQueryServiceOptions(storageFactory storage.BaseFactory, logger *zap.Logger) *querysvc.QueryServiceOptions { +func (qOpts *QueryOptions) BuildQueryServiceOptions( + initArchiveStorageFn InitArchiveStorageFn, + logger *zap.Logger, +) *querysvc.QueryServiceOptions { opts := &querysvc.QueryServiceOptions{} - if !opts.InitArchiveStorage(storageFactory, logger) { + ar, aw := initArchiveStorageFn(logger) + if ar != nil && aw != nil { + opts.ArchiveSpanReader = ar + opts.ArchiveSpanWriter = aw + } else { logger.Info("Archive storage not initialized") } @@ -150,13 +159,13 @@ func (qOpts *QueryOptions) BuildQueryServiceOptions(storageFactory storage.BaseF return opts } -func (qOpts *QueryOptions) BuildQueryServiceOptionsV2(storageFactory storage.BaseFactory, logger *zap.Logger) *v2querysvc.QueryServiceOptions { +func (qOpts *QueryOptions) BuildQueryServiceOptionsV2(initArchiveStorageFn InitArchiveStorageFn, logger *zap.Logger) *v2querysvc.QueryServiceOptions { opts := &v2querysvc.QueryServiceOptions{} - ar, aw := v1adapter.InitializeArchiveStorage(storageFactory, logger) + ar, aw := initArchiveStorageFn(logger) if ar != nil && aw != nil { - opts.ArchiveTraceReader = ar - opts.ArchiveTraceWriter = aw + opts.ArchiveTraceReader = v1adapter.NewTraceReader(ar) + opts.ArchiveTraceWriter = v1adapter.NewTraceWriter(aw) } else { logger.Info("Archive storage not initialized") } diff --git a/cmd/query/app/flags_test.go b/cmd/query/app/flags_test.go index b56a17fc97a..86f07f367fd 100644 --- a/cmd/query/app/flags_test.go +++ b/cmd/query/app/flags_test.go @@ -17,8 +17,8 @@ import ( "github.com/jaegertracing/jaeger/pkg/config" "github.com/jaegertracing/jaeger/pkg/testutils" "github.com/jaegertracing/jaeger/ports" - "github.com/jaegertracing/jaeger/storage/mocks" - spanstore_mocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks" + "github.com/jaegertracing/jaeger/storage/spanstore" + spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks" ) func TestQueryBuilderFlags(t *testing.T) { @@ -86,60 +86,51 @@ func TestStringSliceAsHeader(t *testing.T) { require.NoError(t, err) } +func initializedFn(*zap.Logger) (spanstore.Reader, spanstore.Writer) { + return &spanstoremocks.Reader{}, &spanstoremocks.Writer{} +} + +func uninitializedFn(*zap.Logger) (spanstore.Reader, spanstore.Writer) { + return nil, nil +} + func TestBuildQueryServiceOptions(t *testing.T) { v, _ := config.Viperize(AddFlags) qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop()) require.NoError(t, err) assert.NotNil(t, qOpts) - qSvcOpts := qOpts.BuildQueryServiceOptions(&mocks.Factory{}, zap.NewNop()) - assert.NotNil(t, qSvcOpts) - assert.NotNil(t, qSvcOpts.Adjuster) - assert.Nil(t, qSvcOpts.ArchiveSpanReader) - assert.Nil(t, qSvcOpts.ArchiveSpanWriter) - - comboFactory := struct { - *mocks.Factory - *mocks.ArchiveFactory - }{ - &mocks.Factory{}, - &mocks.ArchiveFactory{}, - } - - comboFactory.ArchiveFactory.On("CreateArchiveSpanReader").Return(&spanstore_mocks.Reader{}, nil) - comboFactory.ArchiveFactory.On("CreateArchiveSpanWriter").Return(&spanstore_mocks.Writer{}, nil) - - qSvcOpts = qOpts.BuildQueryServiceOptions(comboFactory, zap.NewNop()) + qSvcOpts := qOpts.BuildQueryServiceOptions(initializedFn, zap.NewNop()) assert.NotNil(t, qSvcOpts) assert.NotNil(t, qSvcOpts.Adjuster) assert.NotNil(t, qSvcOpts.ArchiveSpanReader) assert.NotNil(t, qSvcOpts.ArchiveSpanWriter) } -func TestBuildQueryServiceOptionsV2(t *testing.T) { +func TestBuildQueryServiceOptions_NoArchiveStorage(t *testing.T) { v, _ := config.Viperize(AddFlags) qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop()) require.NoError(t, err) assert.NotNil(t, qOpts) - qSvcOpts := qOpts.BuildQueryServiceOptionsV2(&mocks.Factory{}, zap.NewNop()) + logger, logBuf := testutils.NewLogger() + qSvcOpts := qOpts.BuildQueryServiceOptions(uninitializedFn, logger) assert.NotNil(t, qSvcOpts) assert.NotNil(t, qSvcOpts.Adjuster) - assert.Nil(t, qSvcOpts.ArchiveTraceReader) - assert.Nil(t, qSvcOpts.ArchiveTraceWriter) + assert.Nil(t, qSvcOpts.ArchiveSpanReader) + assert.Nil(t, qSvcOpts.ArchiveSpanWriter) - comboFactory := struct { - *mocks.Factory - *mocks.ArchiveFactory - }{ - &mocks.Factory{}, - &mocks.ArchiveFactory{}, - } + require.Contains(t, logBuf.String(), "Archive storage not initialized") +} + +func TestBuildQueryServiceOptionsV2(t *testing.T) { + v, _ := config.Viperize(AddFlags) + qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop()) + require.NoError(t, err) + assert.NotNil(t, qOpts) - comboFactory.ArchiveFactory.On("CreateArchiveSpanReader").Return(&spanstore_mocks.Reader{}, nil) - comboFactory.ArchiveFactory.On("CreateArchiveSpanWriter").Return(&spanstore_mocks.Writer{}, nil) + qSvcOpts := qOpts.BuildQueryServiceOptionsV2(initializedFn, zap.NewNop()) - qSvcOpts = qOpts.BuildQueryServiceOptionsV2(comboFactory, zap.NewNop()) assert.NotNil(t, qSvcOpts) assert.NotNil(t, qSvcOpts.Adjuster) assert.NotNil(t, qSvcOpts.ArchiveTraceReader) @@ -153,7 +144,7 @@ func TestBuildQueryServiceOptionsV2_NoArchiveStorage(t *testing.T) { assert.NotNil(t, qOpts) logger, logBuf := testutils.NewLogger() - qSvcOpts := qOpts.BuildQueryServiceOptionsV2(&mocks.Factory{}, logger) + qSvcOpts := qOpts.BuildQueryServiceOptionsV2(uninitializedFn, logger) assert.Nil(t, qSvcOpts.ArchiveTraceReader) assert.Nil(t, qSvcOpts.ArchiveTraceWriter) diff --git a/cmd/query/app/querysvc/query_service.go b/cmd/query/app/querysvc/query_service.go index 9492ab13f61..914a302e30b 100644 --- a/cmd/query/app/querysvc/query_service.go +++ b/cmd/query/app/querysvc/query_service.go @@ -8,11 +8,8 @@ import ( "errors" "time" - "go.uber.org/zap" - "github.com/jaegertracing/jaeger-idl/model/v1" "github.com/jaegertracing/jaeger/model/adjuster" - "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/spanstore" "github.com/jaegertracing/jaeger/storage_v2/depstore" "github.com/jaegertracing/jaeger/storage_v2/tracestore" @@ -164,36 +161,6 @@ func (qs QueryService) GetCapabilities() StorageCapabilities { } } -// InitArchiveStorage tries to initialize archive storage reader/writer if storage factory supports them. -func (opts *QueryServiceOptions) InitArchiveStorage(storageFactory storage.BaseFactory, logger *zap.Logger) bool { - archiveFactory, ok := storageFactory.(storage.ArchiveFactory) - if !ok { - logger.Info("Archive storage not supported by the factory") - return false - } - reader, err := archiveFactory.CreateArchiveSpanReader() - if errors.Is(err, storage.ErrArchiveStorageNotConfigured) || errors.Is(err, storage.ErrArchiveStorageNotSupported) { - logger.Info("Archive storage not created", zap.String("reason", err.Error())) - return false - } - if err != nil { - logger.Error("Cannot init archive storage reader", zap.Error(err)) - return false - } - writer, err := archiveFactory.CreateArchiveSpanWriter() - if errors.Is(err, storage.ErrArchiveStorageNotConfigured) || errors.Is(err, storage.ErrArchiveStorageNotSupported) { - logger.Info("Archive storage not created", zap.String("reason", err.Error())) - return false - } - if err != nil { - logger.Error("Cannot init archive storage writer", zap.Error(err)) - return false - } - opts.ArchiveSpanReader = reader - opts.ArchiveSpanWriter = writer - return true -} - // hasArchiveStorage returns true if archive storage reader/writer are initialized. func (opts *QueryServiceOptions) hasArchiveStorage() bool { return opts.ArchiveSpanReader != nil && opts.ArchiveSpanWriter != nil diff --git a/cmd/query/app/querysvc/query_service_test.go b/cmd/query/app/querysvc/query_service_test.go index f92c21e1b8c..f2706356a3f 100644 --- a/cmd/query/app/querysvc/query_service_test.go +++ b/cmd/query/app/querysvc/query_service_test.go @@ -451,14 +451,6 @@ func TestGetCapabilitiesWithSupportsArchive(t *testing.T) { type fakeStorageFactory1 struct{} -type fakeStorageFactory2 struct { - fakeStorageFactory1 - r spanstore.Reader - w spanstore.Writer - rErr error - wErr error -} - func (*fakeStorageFactory1) Initialize(metrics.Factory, *zap.Logger) error { return nil } @@ -466,49 +458,7 @@ func (*fakeStorageFactory1) CreateSpanReader() (spanstore.Reader, error) func (*fakeStorageFactory1) CreateSpanWriter() (spanstore.Writer, error) { return nil, nil } func (*fakeStorageFactory1) CreateDependencyReader() (dependencystore.Reader, error) { return nil, nil } -func (f *fakeStorageFactory2) CreateArchiveSpanReader() (spanstore.Reader, error) { return f.r, f.rErr } -func (f *fakeStorageFactory2) CreateArchiveSpanWriter() (spanstore.Writer, error) { return f.w, f.wErr } - -var ( - _ storage.Factory = new(fakeStorageFactory1) - _ storage.ArchiveFactory = new(fakeStorageFactory2) -) - -func TestInitArchiveStorageErrors(t *testing.T) { - opts := &QueryServiceOptions{} - logger := zap.NewNop() - - assert.False(t, opts.InitArchiveStorage(new(fakeStorageFactory1), logger)) - assert.False(t, opts.InitArchiveStorage( - &fakeStorageFactory2{rErr: storage.ErrArchiveStorageNotConfigured}, - logger, - )) - assert.False(t, opts.InitArchiveStorage( - &fakeStorageFactory2{rErr: errors.New("error")}, - logger, - )) - assert.False(t, opts.InitArchiveStorage( - &fakeStorageFactory2{wErr: storage.ErrArchiveStorageNotConfigured}, - logger, - )) - assert.False(t, opts.InitArchiveStorage( - &fakeStorageFactory2{wErr: errors.New("error")}, - logger, - )) -} - -func TestInitArchiveStorage(t *testing.T) { - opts := &QueryServiceOptions{} - logger := zap.NewNop() - reader := &spanstoremocks.Reader{} - writer := &spanstoremocks.Writer{} - assert.True(t, opts.InitArchiveStorage( - &fakeStorageFactory2{r: reader, w: writer}, - logger, - )) - assert.Equal(t, reader, opts.ArchiveSpanReader) - assert.Equal(t, writer, opts.ArchiveSpanWriter) -} +var _ storage.Factory = new(fakeStorageFactory1) func TestMain(m *testing.M) { testutils.VerifyGoLeaks(m) diff --git a/cmd/query/app/token_propagation_test.go b/cmd/query/app/token_propagation_test.go index 6c7177cb535..2af20349b2c 100644 --- a/cmd/query/app/token_propagation_test.go +++ b/cmd/query/app/token_propagation_test.go @@ -81,7 +81,7 @@ func runQueryService(t *testing.T, esURL string) *Server { })) f.InitFromViper(v, flagsSvc.Logger) // set AllowTokenFromContext manually because we don't register the respective CLI flag from query svc - f.Options.Primary.Authentication.BearerTokenAuthentication.AllowFromContext = true + f.Options.Config.Authentication.BearerTokenAuthentication.AllowFromContext = true require.NoError(t, f.Initialize(telset.Metrics, telset.Logger)) defer f.Close() diff --git a/cmd/query/main.go b/cmd/query/main.go index 2cced74fb30..8ee34ae6188 100644 --- a/cmd/query/main.go +++ b/cmd/query/main.go @@ -107,13 +107,19 @@ func main() { if err != nil { logger.Fatal("Failed to create metrics query service", zap.Error(err)) } - queryServiceOptions := queryOpts.BuildQueryServiceOptions(storageFactory, logger) + queryServiceOptions := queryOpts.BuildQueryServiceOptions( + storageFactory.InitArchiveStorage, + logger, + ) queryService := querysvc.NewQueryService( traceReader, dependencyReader, *queryServiceOptions) - queryServiceOptionsV2 := queryOpts.BuildQueryServiceOptionsV2(storageFactory, logger) + queryServiceOptionsV2 := queryOpts.BuildQueryServiceOptionsV2( + storageFactory.InitArchiveStorage, + logger, + ) queryServiceV2 := querysvcv2.NewQueryService( traceReader, dependencyReader, diff --git a/cmd/remote-storage/app/server.go b/cmd/remote-storage/app/server.go index 66529595a88..577df153f20 100644 --- a/cmd/remote-storage/app/server.go +++ b/cmd/remote-storage/app/server.go @@ -17,12 +17,10 @@ import ( "google.golang.org/grpc/health" "google.golang.org/grpc/reflection" - "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" "github.com/jaegertracing/jaeger/pkg/bearertoken" "github.com/jaegertracing/jaeger/pkg/telemetry" "github.com/jaegertracing/jaeger/pkg/tenancy" "github.com/jaegertracing/jaeger/plugin/storage/grpc/shared" - "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" "github.com/jaegertracing/jaeger/storage/spanstore" ) @@ -36,8 +34,15 @@ type Server struct { telset telemetry.Settings } +type storageFactory interface { + CreateSpanReader() (spanstore.Reader, error) + CreateSpanWriter() (spanstore.Writer, error) + CreateDependencyReader() (dependencystore.Reader, error) + InitArchiveStorage(logger *zap.Logger) (spanstore.Reader, spanstore.Writer) +} + // NewServer creates and initializes Server. -func NewServer(options *Options, storageFactory storage.BaseFactory, tm *tenancy.Manager, telset telemetry.Settings) (*Server, error) { +func NewServer(options *Options, storageFactory storageFactory, tm *tenancy.Manager, telset telemetry.Settings) (*Server, error) { handler, err := createGRPCHandler(storageFactory, telset.Logger) if err != nil { return nil, err @@ -55,7 +60,7 @@ func NewServer(options *Options, storageFactory storage.BaseFactory, tm *tenancy }, nil } -func createGRPCHandler(f storage.BaseFactory, logger *zap.Logger) (*shared.GRPCHandler, error) { +func createGRPCHandler(f storageFactory, logger *zap.Logger) (*shared.GRPCHandler, error) { reader, err := f.CreateSpanReader() if err != nil { return nil, err @@ -76,12 +81,9 @@ func createGRPCHandler(f storage.BaseFactory, logger *zap.Logger) (*shared.GRPCH StreamingSpanWriter: func() spanstore.Writer { return nil }, } - // borrow code from Query service for archive storage - qOpts := &querysvc.QueryServiceOptions{} - // when archive storage not initialized (returns false), the reader/writer will be nil - _ = qOpts.InitArchiveStorage(f, logger) - impl.ArchiveSpanReader = func() spanstore.Reader { return qOpts.ArchiveSpanReader } - impl.ArchiveSpanWriter = func() spanstore.Writer { return qOpts.ArchiveSpanWriter } + ar, aw := f.InitArchiveStorage(logger) + impl.ArchiveSpanReader = func() spanstore.Reader { return ar } + impl.ArchiveSpanWriter = func() spanstore.Writer { return aw } handler := shared.NewGRPCHandler(impl) return handler, nil diff --git a/cmd/remote-storage/app/server_test.go b/cmd/remote-storage/app/server_test.go index e9c245c4cc4..25603dc701d 100644 --- a/cmd/remote-storage/app/server_test.go +++ b/cmd/remote-storage/app/server_test.go @@ -28,22 +28,16 @@ import ( "github.com/jaegertracing/jaeger/pkg/tenancy" "github.com/jaegertracing/jaeger/ports" "github.com/jaegertracing/jaeger/proto-gen/storage_v1" + "github.com/jaegertracing/jaeger/storage/dependencystore" depStoreMocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks" - factoryMocks "github.com/jaegertracing/jaeger/storage/mocks" + "github.com/jaegertracing/jaeger/storage/spanstore" spanStoreMocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks" ) var testCertKeyLocation = "../../../pkg/config/tlscfg/testdata" func TestNewServer_CreateStorageErrors(t *testing.T) { - factory := new(factoryMocks.Factory) - factory.On("CreateSpanReader").Return(nil, errors.New("no reader")).Once() - factory.On("CreateSpanReader").Return(nil, nil) - factory.On("CreateSpanWriter").Return(nil, errors.New("no writer")).Once() - factory.On("CreateSpanWriter").Return(nil, nil) - factory.On("CreateDependencyReader").Return(nil, errors.New("no deps")).Once() - factory.On("CreateDependencyReader").Return(nil, nil) - f := func() (*Server, error) { + createServer := func(factory *fakeFactory) (*Server, error) { return NewServer( &Options{ ServerConfig: configgrpc.ServerConfig{ @@ -57,16 +51,21 @@ func TestNewServer_CreateStorageErrors(t *testing.T) { telemetry.NoopSettings(), ) } - _, err := f() + + factory := &fakeFactory{readerErr: errors.New("no reader")} + _, err := createServer(factory) require.ErrorContains(t, err, "no reader") - _, err = f() + factory = &fakeFactory{writerErr: errors.New("no writer")} + _, err = createServer(factory) require.ErrorContains(t, err, "no writer") - _, err = f() + factory = &fakeFactory{depReaderErr: errors.New("no deps")} + _, err = createServer(factory) require.ErrorContains(t, err, "no deps") - s, err := f() + factory = &fakeFactory{} + s, err := createServer(factory) require.NoError(t, err) require.NoError(t, s.Start()) validateGRPCServer(t, s.grpcConn.Addr().String()) @@ -86,29 +85,39 @@ func TestServerStart_BadPortErrors(t *testing.T) { require.Error(t, srv.Start()) } -type storageMocks struct { - factory *factoryMocks.Factory - reader *spanStoreMocks.Reader - writer *spanStoreMocks.Writer - depReader *depStoreMocks.Reader +type fakeFactory struct { + reader spanstore.Reader + writer spanstore.Writer + depReader dependencystore.Reader + + readerErr error + writerErr error + depReaderErr error } -func newStorageMocks() *storageMocks { - reader := new(spanStoreMocks.Reader) - writer := new(spanStoreMocks.Writer) - depReader := new(depStoreMocks.Reader) +func (f *fakeFactory) CreateSpanReader() (spanstore.Reader, error) { + if f.readerErr != nil { + return nil, f.readerErr + } + return f.reader, nil +} - factory := new(factoryMocks.Factory) - factory.On("CreateSpanReader").Return(reader, nil) - factory.On("CreateSpanWriter").Return(writer, nil) - factory.On("CreateDependencyReader").Return(depReader, nil) +func (f *fakeFactory) CreateSpanWriter() (spanstore.Writer, error) { + if f.writerErr != nil { + return nil, f.writerErr + } + return f.writer, nil +} - return &storageMocks{ - factory: factory, - reader: reader, - writer: writer, - depReader: depReader, +func (f *fakeFactory) CreateDependencyReader() (dependencystore.Reader, error) { + if f.depReaderErr != nil { + return nil, f.depReaderErr } + return f.depReader, nil +} + +func (*fakeFactory) InitArchiveStorage(*zap.Logger) (spanstore.Reader, spanstore.Writer) { + return nil, nil } func TestNewServer_TLSConfigError(t *testing.T) { @@ -123,7 +132,7 @@ func TestNewServer_TLSConfigError(t *testing.T) { Logger: zap.NewNop(), ReportStatus: telemetry.HCAdapter(healthcheck.New()), } - storageMocks := newStorageMocks() + _, err := NewServer( &Options{ ServerConfig: configgrpc.ServerConfig{ @@ -133,7 +142,7 @@ func TestNewServer_TLSConfigError(t *testing.T) { TLSSetting: tlsCfg, }, }, - storageMocks.factory, + &fakeFactory{}, tenancy.NewManager(&tenancy.Options{}), telset, ) @@ -141,15 +150,24 @@ func TestNewServer_TLSConfigError(t *testing.T) { } func TestCreateGRPCHandler(t *testing.T) { - storageMocks := newStorageMocks() - h, err := createGRPCHandler(storageMocks.factory, zap.NewNop()) + reader := new(spanStoreMocks.Reader) + writer := new(spanStoreMocks.Writer) + depReader := new(depStoreMocks.Reader) + + f := &fakeFactory{ + reader: reader, + writer: writer, + depReader: depReader, + } + + h, err := createGRPCHandler(f, zap.NewNop()) require.NoError(t, err) - storageMocks.writer.On("WriteSpan", mock.Anything, mock.Anything).Return(errors.New("writer error")) + writer.On("WriteSpan", mock.Anything, mock.Anything).Return(errors.New("writer error")) _, err = h.WriteSpan(context.Background(), &storage_v1.WriteSpanRequest{}) require.ErrorContains(t, err, "writer error") - storageMocks.depReader.On( + depReader.On( "GetDependencies", mock.Anything, // context mock.Anything, // time @@ -338,9 +356,12 @@ func TestServerGRPCTLS(t *testing.T) { flagsSvc := flags.NewService(ports.QueryAdminHTTP) flagsSvc.Logger = zap.NewNop() - storageMocks := newStorageMocks() + reader := new(spanStoreMocks.Reader) + f := &fakeFactory{ + reader: reader, + } expectedServices := []string{"test"} - storageMocks.reader.On("GetServices", mock.AnythingOfType("*context.valueCtx")).Return(expectedServices, nil) + reader.On("GetServices", mock.AnythingOfType("*context.valueCtx")).Return(expectedServices, nil) tm := tenancy.NewManager(&tenancy.Options{Enabled: true}) telset := telemetry.Settings{ @@ -349,7 +370,7 @@ func TestServerGRPCTLS(t *testing.T) { } server, err := NewServer( serverOptions, - storageMocks.factory, + f, tm, telset, ) @@ -391,7 +412,6 @@ func TestServerHandlesPortZero(t *testing.T) { flagsSvc := flags.NewService(ports.QueryAdminHTTP) zapCore, logs := observer.New(zap.InfoLevel) flagsSvc.Logger = zap.New(zapCore) - storageMocks := newStorageMocks() telset := telemetry.Settings{ Logger: flagsSvc.Logger, ReportStatus: telemetry.HCAdapter(flagsSvc.HC()), @@ -400,7 +420,7 @@ func TestServerHandlesPortZero(t *testing.T) { &Options{ServerConfig: configgrpc.ServerConfig{ NetAddr: confignet.AddrConfig{Endpoint: ":0"}, }}, - storageMocks.factory, + &fakeFactory{}, tenancy.NewManager(&tenancy.Options{}), telset, ) diff --git a/pkg/es/config/config.go b/pkg/es/config/config.go index d0ae9c6ad3d..a1916e95d48 100644 --- a/pkg/es/config/config.go +++ b/pkg/es/config/config.go @@ -115,6 +115,14 @@ type Configuration struct { // Use this option with Elasticsearch rollover API. It requires an external component // to create aliases before startup and then performing its management. UseReadWriteAliases bool `mapstructure:"use_aliases"` + // ReadAliasSuffix is the suffix to append to the index name used for reading. + // This configuration only exists to provide backwards compatibility for jaeger-v1 + // which is why it is not exposed as a configuration option for jaeger-v2 + ReadAliasSuffix string `mapstructure:"-"` + // WriteAliasSuffix is the suffix to append to the write index name. + // This configuration only exists to provide backwards compatibility for jaeger-v1 + // which is why it is not exposed as a configuration option for jaeger-v2 + WriteAliasSuffix string `mapstructure:"-"` // CreateIndexTemplates, if set to true, creates index templates at application startup. // This configuration should be set to false when templates are installed manually. CreateIndexTemplates bool `mapstructure:"create_mappings"` diff --git a/plugin/storage/badger/factory.go b/plugin/storage/badger/factory.go index 10de32d8e8e..b8fbe06770f 100644 --- a/plugin/storage/badger/factory.go +++ b/plugin/storage/badger/factory.go @@ -38,14 +38,10 @@ const ( ) 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) - + _ storage.Factory = (*Factory)(nil) + _ io.Closer = (*Factory)(nil) + _ plugin.Configurable = (*Factory)(nil) + _ storage.Purger = (*Factory)(nil) _ storage.SamplingStoreFactory = (*Factory)(nil) ) diff --git a/plugin/storage/blackhole/factory.go b/plugin/storage/blackhole/factory.go index ad149593633..7cbdd4ae2eb 100644 --- a/plugin/storage/blackhole/factory.go +++ b/plugin/storage/blackhole/factory.go @@ -13,10 +13,8 @@ import ( "github.com/jaegertracing/jaeger/storage/spanstore" ) -var ( // interface comformance checks - _ storage.Factory = (*Factory)(nil) - _ storage.ArchiveFactory = (*Factory)(nil) -) +// interface comformance checks +var _ storage.Factory = (*Factory)(nil) // Factory implements storage.Factory and creates blackhole storage components. type Factory struct { @@ -48,16 +46,6 @@ func (f *Factory) CreateSpanWriter() (spanstore.Writer, error) { return f.store, nil } -// CreateArchiveSpanReader implements storage.ArchiveFactory -func (f *Factory) CreateArchiveSpanReader() (spanstore.Reader, error) { - return f.store, nil -} - -// CreateArchiveSpanWriter implements storage.ArchiveFactory -func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { - return f.store, nil -} - // CreateDependencyReader implements storage.Factory func (f *Factory) CreateDependencyReader() (dependencystore.Reader, error) { return f.store, nil diff --git a/plugin/storage/blackhole/factory_test.go b/plugin/storage/blackhole/factory_test.go index 0ef1a783176..134e44047c1 100644 --- a/plugin/storage/blackhole/factory_test.go +++ b/plugin/storage/blackhole/factory_test.go @@ -27,12 +27,6 @@ func TestStorageFactory(t *testing.T) { writer, err := f.CreateSpanWriter() require.NoError(t, err) assert.Equal(t, f.store, writer) - reader, err = f.CreateArchiveSpanReader() - require.NoError(t, err) - assert.Equal(t, f.store, reader) - writer, err = f.CreateArchiveSpanWriter() - require.NoError(t, err) - assert.Equal(t, f.store, writer) depReader, err := f.CreateDependencyReader() require.NoError(t, err) assert.Equal(t, f.store, depReader) diff --git a/plugin/storage/cassandra/factory.go b/plugin/storage/cassandra/factory.go index 884eed5a772..5937f92d08a 100644 --- a/plugin/storage/cassandra/factory.go +++ b/plugin/storage/cassandra/factory.go @@ -36,17 +36,18 @@ import ( ) const ( - primaryStorageConfig = "cassandra" - archiveStorageConfig = "cassandra-archive" + primaryStorageNamespace = "cassandra" + archiveStorageNamespace = "cassandra-archive" ) var ( // interface comformance checks _ storage.Factory = (*Factory)(nil) _ storage.Purger = (*Factory)(nil) - _ storage.ArchiveFactory = (*Factory)(nil) _ storage.SamplingStoreFactory = (*Factory)(nil) _ io.Closer = (*Factory)(nil) _ plugin.Configurable = (*Factory)(nil) + _ storage.Inheritable = (*Factory)(nil) + _ storage.ArchiveCapable = (*Factory)(nil) ) // Factory implements storage.Factory for Cassandra backend. @@ -57,11 +58,9 @@ type Factory struct { logger *zap.Logger tracer trace.TracerProvider - primaryConfig config.Configuration - archiveConfig *config.Configuration + config config.Configuration - primarySession cassandra.Session - archiveSession cassandra.Session + session cassandra.Session // tests can override this sessionBuilderFn func(*config.Configuration) (cassandra.Session, error) @@ -71,7 +70,15 @@ type Factory struct { func NewFactory() *Factory { return &Factory{ tracer: otel.GetTracerProvider(), - Options: NewOptions(primaryStorageConfig, archiveStorageConfig), + Options: NewOptions(primaryStorageNamespace), + sessionBuilderFn: NewSession, + } +} + +func NewArchiveFactory() *Factory { + return &Factory{ + tracer: otel.GetTracerProvider(), + Options: NewOptions(archiveStorageNamespace), sessionBuilderFn: NewSession, } } @@ -104,7 +111,7 @@ type withConfigBuilder struct { func (b *withConfigBuilder) build() (*Factory, error) { b.f.configureFromOptions(b.opts) - if err := b.opts.Primary.Validate(); err != nil { + if err := b.opts.NamespaceConfig.Validate(); err != nil { return nil, err } err := b.initializer(b.metricsFactory, b.logger) @@ -128,12 +135,7 @@ func (f *Factory) InitFromViper(v *viper.Viper, _ *zap.Logger) { // InitFromOptions initializes factory from options. func (f *Factory) configureFromOptions(o *Options) { f.Options = o - // TODO this is a hack because we do not define defaults in Options - if o.others == nil { - o.others = make(map[string]*NamespaceConfig) - } - f.primaryConfig = o.GetPrimary() - f.archiveConfig = f.Options.Get(archiveStorageConfig) + f.config = o.GetConfig() } // Initialize implements storage.Factory @@ -141,21 +143,12 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) f.metricsFactory = metricsFactory f.logger = logger - primarySession, err := f.sessionBuilderFn(&f.primaryConfig) + session, err := f.sessionBuilderFn(&f.config) if err != nil { return err } - f.primarySession = primarySession - - if f.archiveConfig != nil { - archiveSession, err := f.sessionBuilderFn(f.archiveConfig) - if err != nil { - return err - } - f.archiveSession = archiveSession - } else { - logger.Info("Cassandra archive storage configuration is empty, skipping") - } + f.session = session + return nil } @@ -203,7 +196,7 @@ func NewSession(c *config.Configuration) (cassandra.Session, error) { // CreateSpanReader implements storage.Factory func (f *Factory) CreateSpanReader() (spanstore.Reader, error) { - sr, err := cSpanStore.NewSpanReader(f.primarySession, f.metricsFactory, f.logger, f.tracer.Tracer("cSpanStore.SpanReader")) + sr, err := cSpanStore.NewSpanReader(f.session, f.metricsFactory, f.logger, f.tracer.Tracer("cSpanStore.SpanReader")) if err != nil { return nil, err } @@ -216,51 +209,13 @@ func (f *Factory) CreateSpanWriter() (spanstore.Writer, error) { if err != nil { return nil, err } - return cSpanStore.NewSpanWriter(f.primarySession, f.Options.SpanStoreWriteCacheTTL, f.metricsFactory, f.logger, options...) + return cSpanStore.NewSpanWriter(f.session, f.Options.SpanStoreWriteCacheTTL, f.metricsFactory, f.logger, options...) } // CreateDependencyReader implements storage.Factory func (f *Factory) CreateDependencyReader() (dependencystore.Reader, error) { - version := cDepStore.GetDependencyVersion(f.primarySession) - return cDepStore.NewDependencyStore(f.primarySession, f.metricsFactory, f.logger, version) -} - -// CreateArchiveSpanReader implements storage.ArchiveFactory -func (f *Factory) CreateArchiveSpanReader() (spanstore.Reader, error) { - if f.archiveSession == nil { - return nil, storage.ErrArchiveStorageNotConfigured - } - archiveMetricsFactory := f.metricsFactory.Namespace( - metrics.NSOptions{ - Tags: map[string]string{ - "role": "archive", - }, - }, - ) - sr, err := cSpanStore.NewSpanReader(f.archiveSession, archiveMetricsFactory, f.logger, f.tracer.Tracer("cSpanStore.SpanReader")) - if err != nil { - return nil, err - } - return spanstoremetrics.NewReaderDecorator(sr, archiveMetricsFactory), nil -} - -// CreateArchiveSpanWriter implements storage.ArchiveFactory -func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { - if f.archiveSession == nil { - return nil, storage.ErrArchiveStorageNotConfigured - } - options, err := writerOptions(f.Options) - if err != nil { - return nil, err - } - archiveMetricsFactory := f.metricsFactory.Namespace( - metrics.NSOptions{ - Tags: map[string]string{ - "role": "archive", - }, - }, - ) - return cSpanStore.NewSpanWriter(f.archiveSession, f.Options.SpanStoreWriteCacheTTL, archiveMetricsFactory, f.logger, options...) + version := cDepStore.GetDependencyVersion(f.session) + return cDepStore.NewDependencyStore(f.session, f.metricsFactory, f.logger, version) } // CreateLock implements storage.SamplingStoreFactory @@ -271,7 +226,7 @@ func (f *Factory) CreateLock() (distributedlock.Lock, error) { } f.logger.Info("Using unique participantName in the distributed lock", zap.String("participantName", hostId)) - return cLock.NewLock(f.primarySession, hostId), nil + return cLock.NewLock(f.session, hostId), nil } // CreateSamplingStore implements storage.SamplingStoreFactory @@ -283,7 +238,7 @@ func (f *Factory) CreateSamplingStore(int /* maxBuckets */) (samplingstore.Store }, }, ) - return cSamplingStore.New(f.primarySession, samplingMetricsFactory, f.logger), nil + return cSamplingStore.New(f.session, samplingMetricsFactory, f.logger), nil } func writerOptions(opts *Options) ([]cSpanStore.Option, error) { @@ -319,16 +274,24 @@ var _ io.Closer = (*Factory)(nil) // Close closes the resources held by the factory func (f *Factory) Close() error { - if f.primarySession != nil { - f.primarySession.Close() - } - if f.archiveSession != nil { - f.archiveSession.Close() + if f.session != nil { + f.session.Close() } return nil } func (f *Factory) Purge(_ context.Context) error { - return f.primarySession.Query("TRUNCATE traces").Exec() + return f.session.Query("TRUNCATE traces").Exec() +} + +func (f *Factory) InheritSettingsFrom(other storage.Factory) { + if otherFactory, ok := other.(*Factory); ok { + f.config.ApplyDefaults(&otherFactory.config) + } +} + +func (f *Factory) IsArchiveCapable() bool { + return f.Options.NamespaceConfig.namespace == archiveStorageNamespace && + f.Options.NamespaceConfig.Enabled } diff --git a/plugin/storage/cassandra/factory_test.go b/plugin/storage/cassandra/factory_test.go index 20f7ca54d5f..bc0af147e1c 100644 --- a/plugin/storage/cassandra/factory_test.go +++ b/plugin/storage/cassandra/factory_test.go @@ -46,64 +46,68 @@ func (m *mockSessionBuilder) build(*config.Configuration) (cassandra.Session, er } func TestCassandraFactory(t *testing.T) { - logger, logBuf := testutils.NewLogger() - f := NewFactory() - v, command := viperize.Viperize(f.AddFlags) - command.ParseFlags([]string{"--cassandra-archive.enabled=true"}) - f.InitFromViper(v, zap.NewNop()) - - f.sessionBuilderFn = new(mockSessionBuilder).add(nil, errors.New("made-up primary error")).build - require.EqualError(t, f.Initialize(metrics.NullFactory, zap.NewNop()), "made-up primary error") + logger, _ := testutils.NewLogger() + + tests := []struct { + name string + factoryFn func() *Factory + namespace string + }{ + { + name: "CassandraFactory", + factoryFn: NewFactory, + namespace: primaryStorageNamespace, + }, + { + name: "CassandraArchiveFactory", + factoryFn: NewArchiveFactory, + namespace: archiveStorageNamespace, + }, + } - var ( - session = &mocks.Session{} - query = &mocks.Query{} - ) - session.On("Query", mock.AnythingOfType("string"), mock.Anything).Return(query) - session.On("Close").Return() - query.On("Exec").Return(nil) - f.sessionBuilderFn = new(mockSessionBuilder). - add(session, nil). - add(nil, errors.New("made-up archive error")).build - require.EqualError(t, f.Initialize(metrics.NullFactory, zap.NewNop()), "made-up archive error") + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + f := test.factoryFn() + require.Equal(t, test.namespace, f.Options.namespace) + v, command := viperize.Viperize(f.AddFlags) + command.ParseFlags([]string{}) + f.InitFromViper(v, zap.NewNop()) - f.archiveConfig = nil - f.sessionBuilderFn = new(mockSessionBuilder).add(session, nil).build - require.NoError(t, f.Initialize(metrics.NullFactory, logger)) - assert.Contains(t, logBuf.String(), "Cassandra archive storage configuration is empty, skipping") + f.sessionBuilderFn = new(mockSessionBuilder).add(nil, errors.New("made-up primary error")).build + require.EqualError(t, f.Initialize(metrics.NullFactory, zap.NewNop()), "made-up primary error") - _, err := f.CreateSpanReader() - require.NoError(t, err) + var ( + session = &mocks.Session{} + query = &mocks.Query{} + ) + session.On("Query", mock.AnythingOfType("string"), mock.Anything).Return(query) + session.On("Close").Return() + query.On("Exec").Return(nil) - _, err = f.CreateSpanWriter() - require.NoError(t, err) + f.sessionBuilderFn = new(mockSessionBuilder).add(session, nil).build + require.NoError(t, f.Initialize(metrics.NullFactory, logger)) - _, err = f.CreateDependencyReader() - require.NoError(t, err) + _, err := f.CreateSpanReader() + require.NoError(t, err) - _, err = f.CreateArchiveSpanReader() - require.EqualError(t, err, "archive storage not configured") + _, err = f.CreateSpanWriter() + require.NoError(t, err) - _, err = f.CreateArchiveSpanWriter() - require.EqualError(t, err, "archive storage not configured") + _, err = f.CreateDependencyReader() + require.NoError(t, err) - f.archiveConfig = &config.Configuration{} - f.sessionBuilderFn = new(mockSessionBuilder).add(session, nil).add(session, nil).build - require.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) + f.sessionBuilderFn = new(mockSessionBuilder).add(session, nil).add(session, nil).build + require.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) - _, err = f.CreateArchiveSpanReader() - require.NoError(t, err) + _, err = f.CreateLock() + require.NoError(t, err) - _, err = f.CreateArchiveSpanWriter() - require.NoError(t, err) + _, err = f.CreateSamplingStore(0) + require.NoError(t, err) - _, err = f.CreateLock() - require.NoError(t, err) - - _, err = f.CreateSamplingStore(0) - require.NoError(t, err) - - require.NoError(t, f.Close()) + require.NoError(t, f.Close()) + }) + } } func TestCreateSpanReaderError(t *testing.T) { @@ -117,22 +121,17 @@ func TestCreateSpanReaderError(t *testing.T) { mock.Anything).Return(query) query.On("Exec").Return(errors.New("table does not exist")) f := NewFactory() - f.archiveConfig = &config.Configuration{} f.sessionBuilderFn = new(mockSessionBuilder).add(session, nil).add(session, nil).build require.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) r, err := f.CreateSpanReader() require.Error(t, err) require.Nil(t, r) - ar, err := f.CreateArchiveSpanReader() - require.Error(t, err) - require.Nil(t, ar) } func TestExclusiveWhitelistBlacklist(t *testing.T) { f := NewFactory() v, command := viperize.Viperize(f.AddFlags) command.ParseFlags([]string{ - "--cassandra-archive.enabled=true", "--cassandra.index.tag-whitelist=a,b,c", "--cassandra.index.tag-blacklist=a,b,c", }) @@ -149,12 +148,8 @@ func TestExclusiveWhitelistBlacklist(t *testing.T) { _, err := f.CreateSpanWriter() require.EqualError(t, err, "only one of TagIndexBlacklist and TagIndexWhitelist can be specified") - f.archiveConfig = &config.Configuration{} f.sessionBuilderFn = new(mockSessionBuilder).add(session, nil).add(session, nil).build require.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) - - _, err = f.CreateArchiveSpanWriter() - require.EqualError(t, err, "only one of TagIndexBlacklist and TagIndexWhitelist can be specified") } func TestWriterOptions(t *testing.T) { @@ -201,18 +196,16 @@ func TestWriterOptions(t *testing.T) { func TestConfigureFromOptions(t *testing.T) { f := NewFactory() - o := NewOptions("foo", archiveStorageConfig) - o.others[archiveStorageConfig].Enabled = true + o := NewOptions("foo") f.configureFromOptions(o) assert.Equal(t, o, f.Options) - assert.Equal(t, o.GetPrimary(), f.primaryConfig) - assert.Equal(t, o.Get(archiveStorageConfig), f.archiveConfig) + assert.Equal(t, o.GetConfig(), f.config) } func TestNewFactoryWithConfig(t *testing.T) { t.Run("valid configuration", func(t *testing.T) { opts := &Options{ - Primary: NamespaceConfig{ + NamespaceConfig: NamespaceConfig{ Configuration: config.DefaultConfiguration(), }, } @@ -230,7 +223,7 @@ func TestNewFactoryWithConfig(t *testing.T) { t.Run("connection error", func(t *testing.T) { expErr := errors.New("made-up error") opts := &Options{ - Primary: NamespaceConfig{ + NamespaceConfig: NamespaceConfig{ Configuration: config.DefaultConfiguration(), }, } @@ -260,7 +253,7 @@ func TestFactory_Purge(t *testing.T) { ) session.On("Query", mock.AnythingOfType("string"), mock.Anything).Return(query) query.On("Exec").Return(nil) - f.primarySession = session + f.session = session err := f.Purge(context.Background()) require.NoError(t, err) @@ -298,3 +291,60 @@ func TestNewSessionErrors(t *testing.T) { require.ErrorContains(t, err, "no hosts provided") }) } + +func TestInheritSettingsFrom(t *testing.T) { + primaryFactory := NewFactory() + primaryFactory.config.Schema.Keyspace = "foo" + primaryFactory.config.Query.MaxRetryAttempts = 99 + + archiveFactory := NewArchiveFactory() + archiveFactory.config.Schema.Keyspace = "bar" + + archiveFactory.InheritSettingsFrom(primaryFactory) + + require.Equal(t, "bar", archiveFactory.config.Schema.Keyspace) + require.Equal(t, 99, archiveFactory.config.Query.MaxRetryAttempts) +} + +func TestIsArchiveCapable(t *testing.T) { + tests := []struct { + name string + namespace string + enabled bool + expected bool + }{ + { + name: "archive capable", + namespace: "cassandra-archive", + enabled: true, + expected: true, + }, + { + name: "not capable", + namespace: "cassandra-archive", + enabled: false, + expected: false, + }, + { + name: "capable + wrong namespace", + namespace: "cassandra", + enabled: true, + expected: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + factory := &Factory{ + Options: &Options{ + NamespaceConfig: NamespaceConfig{ + namespace: test.namespace, + Enabled: test.enabled, + }, + }, + } + result := factory.IsArchiveCapable() + require.Equal(t, test.expected, result) + }) + } +} diff --git a/plugin/storage/cassandra/options.go b/plugin/storage/cassandra/options.go index ae2ba2d1d05..8745c4c8fbb 100644 --- a/plugin/storage/cassandra/options.go +++ b/plugin/storage/cassandra/options.go @@ -48,8 +48,7 @@ const ( // to bind them to command line flag and apply overlays, so that some configurations // (e.g. archive) may be underspecified and infer the rest of its parameters from primary. type Options struct { - Primary NamespaceConfig `mapstructure:",squash"` - others map[string]*NamespaceConfig + NamespaceConfig `mapstructure:",squash"` SpanStoreWriteCacheTTL time.Duration `mapstructure:"span_store_write_cache_ttl"` Index IndexConfig `mapstructure:"index"` } @@ -74,52 +73,44 @@ type NamespaceConfig struct { } // NewOptions creates a new Options struct. -func NewOptions(primaryNamespace string, otherNamespaces ...string) *Options { +func NewOptions(namespace string) *Options { // TODO all default values should be defined via cobra flags options := &Options{ - Primary: NamespaceConfig{ + NamespaceConfig: NamespaceConfig{ Configuration: config.DefaultConfiguration(), - namespace: primaryNamespace, + namespace: namespace, Enabled: true, }, - others: make(map[string]*NamespaceConfig, len(otherNamespaces)), SpanStoreWriteCacheTTL: time.Hour * 12, } - for _, namespace := range otherNamespaces { - options.others[namespace] = &NamespaceConfig{namespace: namespace} - } - return options } // AddFlags adds flags for Options func (opt *Options) AddFlags(flagSet *flag.FlagSet) { - addFlags(flagSet, opt.Primary) - for _, cfg := range opt.others { - addFlags(flagSet, *cfg) - } - flagSet.Duration(opt.Primary.namespace+suffixSpanStoreWriteCacheTTL, + addFlags(flagSet, opt.NamespaceConfig) + flagSet.Duration(opt.namespace+suffixSpanStoreWriteCacheTTL, opt.SpanStoreWriteCacheTTL, "The duration to wait before rewriting an existing service or operation name") flagSet.String( - opt.Primary.namespace+suffixIndexTagsBlacklist, + opt.namespace+suffixIndexTagsBlacklist, opt.Index.TagBlackList, "The comma-separated list of span tags to blacklist from being indexed. All other tags will be indexed. Mutually exclusive with the whitelist option.") flagSet.String( - opt.Primary.namespace+suffixIndexTagsWhitelist, + opt.namespace+suffixIndexTagsWhitelist, opt.Index.TagWhiteList, "The comma-separated list of span tags to whitelist for being indexed. All other tags will not be indexed. Mutually exclusive with the blacklist option.") flagSet.Bool( - opt.Primary.namespace+suffixIndexLogs, + opt.namespace+suffixIndexLogs, !opt.Index.Logs, "Controls log field indexing. Set to false to disable.") flagSet.Bool( - opt.Primary.namespace+suffixIndexTags, + opt.namespace+suffixIndexTags, !opt.Index.Tags, "Controls tag indexing. Set to false to disable.") flagSet.Bool( - opt.Primary.namespace+suffixIndexProcessTags, + opt.namespace+suffixIndexProcessTags, !opt.Index.ProcessTags, "Controls process tag indexing. Set to false to disable.") } @@ -128,7 +119,7 @@ func addFlags(flagSet *flag.FlagSet, nsConfig NamespaceConfig) { tlsFlagsConfig := tlsFlagsConfig(nsConfig.namespace) tlsFlagsConfig.AddFlags(flagSet) - if nsConfig.namespace != primaryStorageConfig { + if nsConfig.namespace != primaryStorageNamespace { flagSet.Bool( nsConfig.namespace+suffixEnabled, false, @@ -205,16 +196,13 @@ func addFlags(flagSet *flag.FlagSet, nsConfig NamespaceConfig) { // InitFromViper initializes Options with properties from viper func (opt *Options) InitFromViper(v *viper.Viper) { - opt.Primary.initFromViper(v) - for _, cfg := range opt.others { - cfg.initFromViper(v) - } - opt.SpanStoreWriteCacheTTL = v.GetDuration(opt.Primary.namespace + suffixSpanStoreWriteCacheTTL) - opt.Index.TagBlackList = stripWhiteSpace(v.GetString(opt.Primary.namespace + suffixIndexTagsBlacklist)) - opt.Index.TagWhiteList = stripWhiteSpace(v.GetString(opt.Primary.namespace + suffixIndexTagsWhitelist)) - opt.Index.Tags = v.GetBool(opt.Primary.namespace + suffixIndexTags) - opt.Index.Logs = v.GetBool(opt.Primary.namespace + suffixIndexLogs) - opt.Index.ProcessTags = v.GetBool(opt.Primary.namespace + suffixIndexProcessTags) + opt.NamespaceConfig.initFromViper(v) + opt.SpanStoreWriteCacheTTL = v.GetDuration(opt.NamespaceConfig.namespace + suffixSpanStoreWriteCacheTTL) + opt.Index.TagBlackList = stripWhiteSpace(v.GetString(opt.NamespaceConfig.namespace + suffixIndexTagsBlacklist)) + opt.Index.TagWhiteList = stripWhiteSpace(v.GetString(opt.NamespaceConfig.namespace + suffixIndexTagsWhitelist)) + opt.Index.Tags = v.GetBool(opt.NamespaceConfig.namespace + suffixIndexTags) + opt.Index.Logs = v.GetBool(opt.NamespaceConfig.namespace + suffixIndexLogs) + opt.Index.ProcessTags = v.GetBool(opt.NamespaceConfig.namespace + suffixIndexProcessTags) } func tlsFlagsConfig(namespace string) tlscfg.ClientFlagsConfig { @@ -225,7 +213,7 @@ func tlsFlagsConfig(namespace string) tlscfg.ClientFlagsConfig { func (cfg *NamespaceConfig) initFromViper(v *viper.Viper) { tlsFlagsConfig := tlsFlagsConfig(cfg.namespace) - if cfg.namespace != primaryStorageConfig { + if cfg.namespace != primaryStorageNamespace { cfg.Enabled = v.GetBool(cfg.namespace + suffixEnabled) } cfg.Connection.ConnectionsPerHost = v.GetInt(cfg.namespace + suffixConnPerHost) @@ -255,26 +243,8 @@ func (cfg *NamespaceConfig) initFromViper(v *viper.Viper) { cfg.Connection.TLS = tlsCfg } -// GetPrimary returns primary configuration. -func (opt *Options) GetPrimary() config.Configuration { - return opt.Primary.Configuration -} - -// Get returns auxiliary named configuration. -func (opt *Options) Get(namespace string) *config.Configuration { - nsCfg, ok := opt.others[namespace] - if !ok { - nsCfg = &NamespaceConfig{} - opt.others[namespace] = nsCfg - } - if !nsCfg.Enabled { - return nil - } - nsCfg.Configuration.ApplyDefaults(&opt.Primary.Configuration) - if len(nsCfg.Connection.Servers) == 0 { - nsCfg.Connection.Servers = opt.Primary.Connection.Servers - } - return &nsCfg.Configuration +func (opt *Options) GetConfig() config.Configuration { + return opt.NamespaceConfig.Configuration } // TagIndexBlacklist returns the list of blacklisted tags diff --git a/plugin/storage/cassandra/options_test.go b/plugin/storage/cassandra/options_test.go index 8ccd813c838..35976ebe8fb 100644 --- a/plugin/storage/cassandra/options_test.go +++ b/plugin/storage/cassandra/options_test.go @@ -6,36 +6,22 @@ package cassandra import ( "testing" - "time" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/jaegertracing/jaeger/pkg/config" ) func TestOptions(t *testing.T) { opts := NewOptions("foo") - primary := opts.GetPrimary() + primary := opts.GetConfig() assert.NotEmpty(t, primary.Schema.Keyspace) assert.NotEmpty(t, primary.Connection.Servers) assert.Equal(t, 2, primary.Connection.ConnectionsPerHost) - - aux := opts.Get("archive") - assert.Nil(t, aux) - - assert.NotNil(t, opts.others["archive"]) - opts.others["archive"].Enabled = true - aux = opts.Get("archive") - require.NotNil(t, aux) - assert.Equal(t, primary.Schema.Keyspace, aux.Schema.Keyspace) - assert.Equal(t, primary.Connection.Servers, aux.Connection.Servers) - assert.Equal(t, primary.Connection.ConnectionsPerHost, aux.Connection.ConnectionsPerHost) - assert.Equal(t, primary.Connection.ReconnectInterval, aux.Connection.ReconnectInterval) } func TestOptionsWithFlags(t *testing.T) { - opts := NewOptions("cas", "cas-aux") + opts := NewOptions("cas") v, command := config.Viperize(opts.AddFlags) command.ParseFlags([]string{ "--cas.keyspace=jaeger", @@ -56,17 +42,10 @@ func TestOptionsWithFlags(t *testing.T) { "--cas.basic.allowed-authenticators=org.apache.cassandra.auth.PasswordAuthenticator,com.datastax.bdp.cassandra.auth.DseAuthenticator", "--cas.username=username", "--cas.password=password", - // enable aux with a couple overrides - "--cas-aux.enabled=true", - "--cas-aux.keyspace=jaeger-archive", - "--cas-aux.servers=3.3.3.3, 4.4.4.4", - "--cas-aux.username=username", - "--cas-aux.password=password", - "--cas-aux.basic.allowed-authenticators=org.apache.cassandra.auth.PasswordAuthenticator,com.ericsson.bss.cassandra.ecaudit.auth.AuditAuthenticator", }) opts.InitFromViper(v) - primary := opts.GetPrimary() + primary := opts.GetConfig() assert.Equal(t, "jaeger", primary.Schema.Keyspace) assert.Equal(t, "mojave", primary.Connection.LocalDC) assert.Equal(t, []string{"1.1.1.1", "2.2.2.2"}, primary.Connection.Servers) @@ -77,20 +56,6 @@ func TestOptionsWithFlags(t *testing.T) { assert.True(t, opts.Index.Tags) assert.False(t, opts.Index.ProcessTags) assert.True(t, opts.Index.Logs) - - aux := opts.Get("cas-aux") - require.NotNil(t, aux) - assert.Equal(t, "jaeger-archive", aux.Schema.Keyspace) - assert.Equal(t, []string{"3.3.3.3", "4.4.4.4"}, aux.Connection.Servers) - assert.Equal(t, []string{"org.apache.cassandra.auth.PasswordAuthenticator", "com.ericsson.bss.cassandra.ecaudit.auth.AuditAuthenticator"}, aux.Connection.Authenticator.Basic.AllowedAuthenticators) - assert.Equal(t, 42, aux.Connection.ConnectionsPerHost) - assert.Equal(t, 42, aux.Query.MaxRetryAttempts) - assert.Equal(t, 42*time.Second, aux.Query.Timeout) - assert.Equal(t, 42*time.Second, aux.Connection.ReconnectInterval) - assert.Equal(t, 4242, aux.Connection.Port) - assert.Equal(t, "", aux.Query.Consistency, "aux storage does not inherit consistency from primary") - assert.Equal(t, 3, aux.Connection.ProtoVersion) - assert.Equal(t, 42*time.Second, aux.Connection.SocketKeepAlive) } func TestDefaultTlsHostVerify(t *testing.T) { @@ -101,7 +66,7 @@ func TestDefaultTlsHostVerify(t *testing.T) { }) opts.InitFromViper(v) - primary := opts.GetPrimary() + primary := opts.GetConfig() assert.False(t, primary.Connection.TLS.InsecureSkipVerify) } diff --git a/plugin/storage/es/factory.go b/plugin/storage/es/factory.go index e1c7ae807a4..595e81ffe29 100644 --- a/plugin/storage/es/factory.go +++ b/plugin/storage/es/factory.go @@ -43,10 +43,11 @@ const ( var ( // interface comformance checks _ storage.Factory = (*Factory)(nil) - _ storage.ArchiveFactory = (*Factory)(nil) _ io.Closer = (*Factory)(nil) _ plugin.Configurable = (*Factory)(nil) + _ storage.Inheritable = (*Factory)(nil) _ storage.Purger = (*Factory)(nil) + _ storage.ArchiveCapable = (*Factory)(nil) ) // Factory implements storage.Factory for Elasticsearch backend. @@ -59,19 +60,25 @@ type Factory struct { newClientFn func(c *config.Configuration, logger *zap.Logger, metricsFactory metrics.Factory) (es.Client, error) - primaryConfig *config.Configuration - archiveConfig *config.Configuration + config *config.Configuration - primaryClient atomic.Pointer[es.Client] - archiveClient atomic.Pointer[es.Client] + client atomic.Pointer[es.Client] - watchers []*fswatcher.FSWatcher + pwdFileWatcher *fswatcher.FSWatcher } // NewFactory creates a new Factory. func NewFactory() *Factory { return &Factory{ - Options: NewOptions(primaryNamespace, archiveNamespace), + Options: NewOptions(primaryNamespace), + newClientFn: config.NewClient, + tracer: otel.GetTracerProvider(), + } +} + +func NewArchiveFactory() *Factory { + return &Factory{ + Options: NewOptions(archiveNamespace), newClientFn: config.NewClient, tracer: otel.GetTracerProvider(), } @@ -89,20 +96,11 @@ func NewFactoryWithConfig( defaultConfig := DefaultConfig() cfg.ApplyDefaults(&defaultConfig) - archive := make(map[string]*namespaceConfig) - archive[archiveNamespace] = &namespaceConfig{ - Configuration: cfg, - namespace: archiveNamespace, + f := &Factory{ + config: &cfg, + newClientFn: config.NewClient, + tracer: otel.GetTracerProvider(), } - - f := NewFactory() - f.configureFromOptions(&Options{ - Primary: namespaceConfig{ - Configuration: cfg, - namespace: primaryNamespace, - }, - others: archive, - }) err := f.Initialize(metricsFactory, logger) if err != nil { return nil, err @@ -124,8 +122,7 @@ func (f *Factory) InitFromViper(v *viper.Viper, _ *zap.Logger) { // configureFromOptions configures factory from Options struct. func (f *Factory) configureFromOptions(o *Options) { f.Options = o - f.primaryConfig = f.Options.GetPrimary() - f.archiveConfig = f.Options.Get(archiveNamespace) + f.config = f.Options.GetConfig() } // Initialize implements storage.Factory. @@ -133,48 +130,37 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) f.metricsFactory = metricsFactory f.logger = logger - primaryClient, err := f.newClientFn(f.primaryConfig, logger, metricsFactory) + client, err := f.newClientFn(f.config, logger, metricsFactory) if err != nil { - return fmt.Errorf("failed to create primary Elasticsearch client: %w", err) + return fmt.Errorf("failed to create Elasticsearch client: %w", err) } - f.primaryClient.Store(&primaryClient) + f.client.Store(&client) - if f.primaryConfig.Authentication.BasicAuthentication.PasswordFilePath != "" { - primaryWatcher, err := fswatcher.New([]string{f.primaryConfig.Authentication.BasicAuthentication.PasswordFilePath}, f.onPrimaryPasswordChange, f.logger) + if f.config.Authentication.BasicAuthentication.PasswordFilePath != "" { + watcher, err := fswatcher.New([]string{f.config.Authentication.BasicAuthentication.PasswordFilePath}, f.onPasswordChange, f.logger) if err != nil { - return fmt.Errorf("failed to create watcher for primary ES client's password: %w", err) + return fmt.Errorf("failed to create watcher for ES client's password: %w", err) } - f.watchers = append(f.watchers, primaryWatcher) + f.pwdFileWatcher = watcher } - if f.archiveConfig.Enabled { - archiveClient, err := f.newClientFn(f.archiveConfig, logger, metricsFactory) - if err != nil { - return fmt.Errorf("failed to create archive Elasticsearch client: %w", err) - } - f.archiveClient.Store(&archiveClient) - - if f.archiveConfig.Authentication.BasicAuthentication.PasswordFilePath != "" { - archiveWatcher, err := fswatcher.New([]string{f.archiveConfig.Authentication.BasicAuthentication.PasswordFilePath}, f.onArchivePasswordChange, f.logger) - if err != nil { - return fmt.Errorf("failed to create watcher for archive ES client's password: %w", err) - } - f.watchers = append(f.watchers, archiveWatcher) + if f.Options != nil && f.Options.Config.namespace == archiveNamespace { + aliasSuffix := "archive" + if f.config.UseReadWriteAliases { + f.config.ReadAliasSuffix = aliasSuffix + "-read" + f.config.WriteAliasSuffix = aliasSuffix + "-write" + } else { + f.config.ReadAliasSuffix = aliasSuffix + f.config.WriteAliasSuffix = aliasSuffix } + f.config.UseReadWriteAliases = true } return nil } -func (f *Factory) getPrimaryClient() es.Client { - if c := f.primaryClient.Load(); c != nil { - return *c - } - return nil -} - -func (f *Factory) getArchiveClient() es.Client { - if c := f.archiveClient.Load(); c != nil { +func (f *Factory) getClient() es.Client { + if c := f.client.Load(); c != nil { return *c } return nil @@ -182,7 +168,7 @@ func (f *Factory) getArchiveClient() es.Client { // CreateSpanReader implements storage.Factory func (f *Factory) CreateSpanReader() (spanstore.Reader, error) { - sr, err := createSpanReader(f.getPrimaryClient, f.primaryConfig, f.logger, f.tracer, "", f.primaryConfig.UseReadWriteAliases) + sr, err := createSpanReader(f.getClient, f.config, f.logger, f.tracer, f.config.ReadAliasSuffix, f.config.UseReadWriteAliases) if err != nil { return nil, err } @@ -191,54 +177,12 @@ func (f *Factory) CreateSpanReader() (spanstore.Reader, error) { // CreateSpanWriter implements storage.Factory func (f *Factory) CreateSpanWriter() (spanstore.Writer, error) { - return createSpanWriter(f.getPrimaryClient, f.primaryConfig, f.metricsFactory, f.logger, "", f.primaryConfig.UseReadWriteAliases) + return createSpanWriter(f.getClient, f.config, f.metricsFactory, f.logger, f.config.WriteAliasSuffix, f.config.UseReadWriteAliases) } // CreateDependencyReader implements storage.Factory func (f *Factory) CreateDependencyReader() (dependencystore.Reader, error) { - return createDependencyReader(f.getPrimaryClient, f.primaryConfig, f.logger) -} - -// CreateArchiveSpanReader implements storage.ArchiveFactory -func (f *Factory) CreateArchiveSpanReader() (spanstore.Reader, error) { - if !f.archiveConfig.Enabled { - return nil, nil - } - readAliasSuffix := "archive" - if f.archiveConfig.UseReadWriteAliases { - readAliasSuffix += "-read" - } - sr, err := createSpanReader(f.getArchiveClient, f.archiveConfig, f.logger, f.tracer, readAliasSuffix, true) - if err != nil { - return nil, err - } - archiveMetricsFactory := f.metricsFactory.Namespace( - metrics.NSOptions{ - Tags: map[string]string{ - "role": "archive", - }, - }, - ) - return spanstoremetrics.NewReaderDecorator(sr, archiveMetricsFactory), nil -} - -// CreateArchiveSpanWriter implements storage.ArchiveFactory -func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { - if !f.archiveConfig.Enabled { - return nil, nil - } - writeAliasSuffix := "archive" - if f.archiveConfig.UseReadWriteAliases { - writeAliasSuffix += "-write" - } - archiveMetricsFactory := f.metricsFactory.Namespace( - metrics.NSOptions{ - Tags: map[string]string{ - "role": "archive", - }, - }, - ) - return createSpanWriter(f.getArchiveClient, f.archiveConfig, archiveMetricsFactory, f.logger, writeAliasSuffix, true) + return createDependencyReader(f.getClient, f.config, f.logger) } func createSpanReader( @@ -317,23 +261,23 @@ func createSpanWriter( func (f *Factory) CreateSamplingStore(int /* maxBuckets */) (samplingstore.Store, error) { params := esSampleStore.Params{ - Client: f.getPrimaryClient, + Client: f.getClient, Logger: f.logger, - IndexPrefix: f.primaryConfig.Indices.IndexPrefix, - IndexDateLayout: f.primaryConfig.Indices.Sampling.DateLayout, - IndexRolloverFrequency: config.RolloverFrequencyAsNegativeDuration(f.primaryConfig.Indices.Sampling.RolloverFrequency), - Lookback: f.primaryConfig.AdaptiveSamplingLookback, - MaxDocCount: f.primaryConfig.MaxDocCount, + IndexPrefix: f.config.Indices.IndexPrefix, + IndexDateLayout: f.config.Indices.Sampling.DateLayout, + IndexRolloverFrequency: config.RolloverFrequencyAsNegativeDuration(f.config.Indices.Sampling.RolloverFrequency), + Lookback: f.config.AdaptiveSamplingLookback, + MaxDocCount: f.config.MaxDocCount, } store := esSampleStore.NewSamplingStore(params) - if f.primaryConfig.CreateIndexTemplates && !f.primaryConfig.UseILM { - mappingBuilder := mappingBuilderFromConfig(f.primaryConfig) + if f.config.CreateIndexTemplates && !f.config.UseILM { + mappingBuilder := mappingBuilderFromConfig(f.config) samplingMapping, err := mappingBuilder.GetSamplingMappings() if err != nil { return nil, err } - if _, err := f.getPrimaryClient().CreateTemplate(params.PrefixedIndexName()).Body(samplingMapping).Do(context.Background()); err != nil { + if _, err := f.getClient().CreateTemplate(params.PrefixedIndexName()).Body(samplingMapping).Do(context.Background()); err != nil { return nil, fmt.Errorf("failed to create template: %w", err) } } @@ -372,30 +316,16 @@ var _ io.Closer = (*Factory)(nil) func (f *Factory) Close() error { var errs []error - for _, w := range f.watchers { - errs = append(errs, w.Close()) - } - errs = append(errs, f.getPrimaryClient().Close()) - if client := f.getArchiveClient(); client != nil { - errs = append(errs, client.Close()) + if f.pwdFileWatcher != nil { + errs = append(errs, f.pwdFileWatcher.Close()) } + errs = append(errs, f.getClient().Close()) return errors.Join(errs...) } -func (f *Factory) onPrimaryPasswordChange() { - f.onClientPasswordChange(f.primaryConfig, &f.primaryClient, f.metricsFactory) -} - -func (f *Factory) onArchivePasswordChange() { - archiveMetricsFactory := f.metricsFactory.Namespace( - metrics.NSOptions{ - Tags: map[string]string{ - "role": "archive", - }, - }, - ) - f.onClientPasswordChange(f.archiveConfig, &f.archiveClient, archiveMetricsFactory) +func (f *Factory) onPasswordChange() { + f.onClientPasswordChange(f.config, &f.client, f.metricsFactory) } func (f *Factory) onClientPasswordChange(cfg *config.Configuration, client *atomic.Pointer[es.Client], mf metrics.Factory) { @@ -422,7 +352,7 @@ func (f *Factory) onClientPasswordChange(cfg *config.Configuration, client *atom } func (f *Factory) Purge(ctx context.Context) error { - esClient := f.getPrimaryClient() + esClient := f.getClient() _, err := esClient.DeleteIndex("*").Do(ctx) return err } @@ -434,3 +364,13 @@ func loadTokenFromFile(path string) (string, error) { } return strings.TrimRight(string(b), "\r\n"), nil } + +func (f *Factory) InheritSettingsFrom(other storage.Factory) { + if otherFactory, ok := other.(*Factory); ok { + f.config.ApplyDefaults(otherFactory.config) + } +} + +func (f *Factory) IsArchiveCapable() bool { + return f.Options.Config.namespace == archiveNamespace && f.Options.Config.Enabled +} diff --git a/plugin/storage/es/factory_test.go b/plugin/storage/es/factory_test.go index 9b35119420c..07887d0587c 100644 --- a/plugin/storage/es/factory_test.go +++ b/plugin/storage/es/factory_test.go @@ -8,7 +8,6 @@ import ( "context" "encoding/base64" "errors" - "fmt" "net/http" "net/http/httptest" "os" @@ -68,16 +67,7 @@ func TestElasticsearchFactory(t *testing.T) { f.InitFromViper(v, zap.NewNop()) f.newClientFn = (&mockClientBuilder{err: errors.New("made-up error")}).NewClient - require.EqualError(t, f.Initialize(metrics.NullFactory, zap.NewNop()), "failed to create primary Elasticsearch client: made-up error") - - f.archiveConfig.Enabled = true - f.newClientFn = func(c *escfg.Configuration, logger *zap.Logger, metricsFactory metrics.Factory) (es.Client, error) { - // to test archive storage error, pretend that primary client creation is successful - // but override newClientFn so it fails for the next invocation - f.newClientFn = (&mockClientBuilder{err: errors.New("made-up error2")}).NewClient - return (&mockClientBuilder{}).NewClient(c, logger, metricsFactory) - } - require.EqualError(t, f.Initialize(metrics.NullFactory, zap.NewNop()), "failed to create archive Elasticsearch client: made-up error2") + require.EqualError(t, f.Initialize(metrics.NullFactory, zap.NewNop()), "failed to create Elasticsearch client: made-up error") f.newClientFn = (&mockClientBuilder{}).NewClient require.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) @@ -91,26 +81,57 @@ func TestElasticsearchFactory(t *testing.T) { _, err = f.CreateDependencyReader() require.NoError(t, err) - _, err = f.CreateArchiveSpanReader() - require.NoError(t, err) - - _, err = f.CreateArchiveSpanWriter() - require.NoError(t, err) - _, err = f.CreateSamplingStore(1) require.NoError(t, err) require.NoError(t, f.Close()) } +func TestArchiveFactory(t *testing.T) { + tests := []struct { + name string + args []string + expectedReadAlias string + expectedWriteAlias string + }{ + { + name: "default settings", + args: []string{}, + expectedReadAlias: "archive", + expectedWriteAlias: "archive", + }, + { + name: "use read write aliases", + args: []string{"--es-archive.use-aliases=true"}, + expectedReadAlias: "archive-read", + expectedWriteAlias: "archive-write", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + f := NewArchiveFactory() + v, command := config.Viperize(f.AddFlags) + command.ParseFlags(test.args) + f.InitFromViper(v, zap.NewNop()) + + f.newClientFn = (&mockClientBuilder{}).NewClient + require.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) + + require.Equal(t, test.expectedReadAlias, f.config.ReadAliasSuffix) + require.Equal(t, test.expectedWriteAlias, f.config.WriteAliasSuffix) + require.True(t, f.config.UseReadWriteAliases) + }) + } +} + func TestElasticsearchTagsFileDoNotExist(t *testing.T) { f := NewFactory() - f.primaryConfig = &escfg.Configuration{ + f.config = &escfg.Configuration{ Tags: escfg.TagsAsFields{ File: "fixtures/file-does-not-exist.txt", }, } - f.archiveConfig = &escfg.Configuration{} f.newClientFn = (&mockClientBuilder{}).NewClient require.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) defer f.Close() @@ -121,13 +142,9 @@ func TestElasticsearchTagsFileDoNotExist(t *testing.T) { func TestElasticsearchILMUsedWithoutReadWriteAliases(t *testing.T) { f := NewFactory() - f.primaryConfig = &escfg.Configuration{ + f.config = &escfg.Configuration{ UseILM: true, } - f.archiveConfig = &escfg.Configuration{ - Enabled: true, - UseILM: true, - } f.newClientFn = (&mockClientBuilder{}).NewClient require.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) defer f.Close() @@ -138,10 +155,6 @@ func TestElasticsearchILMUsedWithoutReadWriteAliases(t *testing.T) { r, err := f.CreateSpanReader() require.EqualError(t, err, "--es.use-ilm must always be used in conjunction with --es.use-aliases to ensure ES writers and readers refer to the single index mapping") assert.Nil(t, r) - - ar, err := f.CreateArchiveSpanReader() - require.EqualError(t, err, "--es.use-ilm must always be used in conjunction with --es.use-aliases to ensure ES writers and readers refer to the single index mapping") - assert.Nil(t, ar) } func TestTagKeysAsFields(t *testing.T) { @@ -203,8 +216,7 @@ func TestTagKeysAsFields(t *testing.T) { func TestCreateTemplateError(t *testing.T) { f := NewFactory() - f.primaryConfig = &escfg.Configuration{CreateIndexTemplates: true} - f.archiveConfig = &escfg.Configuration{} + f.config = &escfg.Configuration{CreateIndexTemplates: true} f.newClientFn = (&mockClientBuilder{createTemplateError: errors.New("template-error")}).NewClient err := f.Initialize(metrics.NullFactory, zap.NewNop()) require.NoError(t, err) @@ -221,8 +233,7 @@ func TestCreateTemplateError(t *testing.T) { func TestILMDisableTemplateCreation(t *testing.T) { f := NewFactory() - f.primaryConfig = &escfg.Configuration{UseILM: true, UseReadWriteAliases: true, CreateIndexTemplates: true} - f.archiveConfig = &escfg.Configuration{} + f.config = &escfg.Configuration{UseILM: true, UseReadWriteAliases: true, CreateIndexTemplates: true} f.newClientFn = (&mockClientBuilder{createTemplateError: errors.New("template-error")}).NewClient err := f.Initialize(metrics.NullFactory, zap.NewNop()) defer f.Close() @@ -231,57 +242,13 @@ func TestILMDisableTemplateCreation(t *testing.T) { require.NoError(t, err) // as the createTemplate is not called, CreateSpanWriter should not return an error } -func TestArchiveDisabled(t *testing.T) { - f := NewFactory() - f.archiveConfig = &escfg.Configuration{Enabled: false} - f.newClientFn = (&mockClientBuilder{}).NewClient - w, err := f.CreateArchiveSpanWriter() - assert.Nil(t, w) - require.NoError(t, err) - r, err := f.CreateArchiveSpanReader() - assert.Nil(t, r) - require.NoError(t, err) -} - -func TestArchiveEnabled(t *testing.T) { - tests := []struct { - useReadWriteAliases bool - }{ - { - useReadWriteAliases: false, - }, - { - useReadWriteAliases: true, - }, - } - for _, test := range tests { - t.Run(fmt.Sprintf("useReadWriteAliases=%v", test.useReadWriteAliases), func(t *testing.T) { - f := NewFactory() - f.primaryConfig = &escfg.Configuration{} - f.archiveConfig = &escfg.Configuration{Enabled: true, UseReadWriteAliases: test.useReadWriteAliases} - f.newClientFn = (&mockClientBuilder{}).NewClient - err := f.Initialize(metrics.NullFactory, zap.NewNop()) - require.NoError(t, err) - defer f.Close() // Ensure resources are cleaned up if initialization is successful - w, err := f.CreateArchiveSpanWriter() - require.NoError(t, err) - assert.NotNil(t, w) - r, err := f.CreateArchiveSpanReader() - require.NoError(t, err) - assert.NotNil(t, r) - }) - } -} - func TestConfigureFromOptions(t *testing.T) { f := NewFactory() o := &Options{ - Primary: namespaceConfig{Configuration: escfg.Configuration{Servers: []string{"server"}}}, - others: map[string]*namespaceConfig{"es-archive": {Configuration: escfg.Configuration{Servers: []string{"server2"}}}}, + Config: namespaceConfig{Configuration: escfg.Configuration{Servers: []string{"server"}}}, } f.configureFromOptions(o) - assert.Equal(t, o.GetPrimary(), f.primaryConfig) - assert.Equal(t, o.Get(archiveNamespace), f.archiveConfig) + assert.Equal(t, o.GetConfig(), f.config) } func TestESStorageFactoryWithConfig(t *testing.T) { @@ -339,19 +306,14 @@ func TestESStorageFactoryWithConfigError(t *testing.T) { LogLevel: "error", } _, err := NewFactoryWithConfig(cfg, metrics.NullFactory, zap.NewNop()) - require.ErrorContains(t, err, "failed to create primary Elasticsearch client") + require.ErrorContains(t, err, "failed to create Elasticsearch client") } func TestPasswordFromFile(t *testing.T) { defer testutils.VerifyGoLeaksOnce(t) t.Run("primary client", func(t *testing.T) { f := NewFactory() - testPasswordFromFile(t, f, f.getPrimaryClient, f.CreateSpanWriter) - }) - - t.Run("archive client", func(t *testing.T) { - f2 := NewFactory() - testPasswordFromFile(t, f2, f2.getArchiveClient, f2.CreateArchiveSpanWriter) + testPasswordFromFile(t, f, f.getClient, f.CreateSpanWriter) }) t.Run("load token error", func(t *testing.T) { @@ -391,21 +353,7 @@ func testPasswordFromFile(t *testing.T, f *Factory, getClient func() es.Client, pwdFile := filepath.Join(t.TempDir(), "pwd") require.NoError(t, os.WriteFile(pwdFile, []byte(pwd1), 0o600)) - f.primaryConfig = &escfg.Configuration{ - Servers: []string{server.URL}, - LogLevel: "debug", - Authentication: escfg.Authentication{ - BasicAuthentication: escfg.BasicAuthentication{ - Username: "user", - PasswordFilePath: pwdFile, - }, - }, - BulkProcessing: escfg.BulkProcessing{ - MaxBytes: -1, // disable bulk; we want immediate flush - }, - } - f.archiveConfig = &escfg.Configuration{ - Enabled: true, + f.config = &escfg.Configuration{ Servers: []string{server.URL}, LogLevel: "debug", Authentication: escfg.Authentication{ @@ -464,8 +412,7 @@ func testPasswordFromFile(t *testing.T, f *Factory, getClient func() es.Client, func TestFactoryESClientsAreNil(t *testing.T) { f := &Factory{} - assert.Nil(t, f.getPrimaryClient()) - assert.Nil(t, f.getArchiveClient()) + assert.Nil(t, f.getClient()) } func TestPasswordFromFileErrors(t *testing.T) { @@ -479,16 +426,7 @@ func TestPasswordFromFileErrors(t *testing.T) { require.NoError(t, os.WriteFile(pwdFile, []byte("first password"), 0o600)) f := NewFactory() - f.primaryConfig = &escfg.Configuration{ - Servers: []string{server.URL}, - LogLevel: "debug", - Authentication: escfg.Authentication{ - BasicAuthentication: escfg.BasicAuthentication{ - PasswordFilePath: pwdFile, - }, - }, - } - f.archiveConfig = &escfg.Configuration{ + f.config = &escfg.Configuration{ Servers: []string{server.URL}, LogLevel: "debug", Authentication: escfg.Authentication{ @@ -502,16 +440,72 @@ func TestPasswordFromFileErrors(t *testing.T) { require.NoError(t, f.Initialize(metrics.NullFactory, logger)) defer f.Close() - f.primaryConfig.Servers = []string{} - f.onPrimaryPasswordChange() - assert.Contains(t, buf.String(), "no servers specified") - - f.archiveConfig.Servers = []string{} - buf.Reset() - f.onArchivePasswordChange() + f.config.Servers = []string{} + f.onPasswordChange() assert.Contains(t, buf.String(), "no servers specified") require.NoError(t, os.Remove(pwdFile)) - f.onPrimaryPasswordChange() - f.onArchivePasswordChange() + f.onPasswordChange() +} + +func TestInheritSettingsFrom(t *testing.T) { + primaryFactory := NewFactory() + primaryFactory.config = &escfg.Configuration{ + MaxDocCount: 99, + } + + archiveFactory := NewArchiveFactory() + archiveFactory.config = &escfg.Configuration{ + SendGetBodyAs: "PUT", + } + + archiveFactory.InheritSettingsFrom(primaryFactory) + + require.Equal(t, "PUT", archiveFactory.config.SendGetBodyAs) + require.Equal(t, 99, primaryFactory.config.MaxDocCount) +} + +func TestIsArchiveCapable(t *testing.T) { + tests := []struct { + name string + namespace string + enabled bool + expected bool + }{ + { + name: "archive capable", + namespace: "es-archive", + enabled: true, + expected: true, + }, + { + name: "not capable", + namespace: "es-archive", + enabled: false, + expected: false, + }, + { + name: "capable + wrong namespace", + namespace: "es", + enabled: true, + expected: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + factory := &Factory{ + Options: &Options{ + Config: namespaceConfig{ + namespace: test.namespace, + Configuration: escfg.Configuration{ + Enabled: test.enabled, + }, + }, + }, + } + result := factory.IsArchiveCapable() + require.Equal(t, test.expected, result) + }) + } } diff --git a/plugin/storage/es/options.go b/plugin/storage/es/options.go index ee52d718032..264bf3393fa 100644 --- a/plugin/storage/es/options.go +++ b/plugin/storage/es/options.go @@ -84,9 +84,8 @@ var defaultIndexOptions = config.IndexOptions{ // to bind them to command line flag and apply overlays, so that some configurations // (e.g. archive) may be underspecified and infer the rest of its parameters from primary. type Options struct { - Primary namespaceConfig `mapstructure:",squash"` - - others map[string]*namespaceConfig + // TODO: remove indirection + Config namespaceConfig `mapstructure:",squash"` } type namespaceConfig struct { @@ -95,24 +94,14 @@ type namespaceConfig struct { } // NewOptions creates a new Options struct. -func NewOptions(primaryNamespace string, otherNamespaces ...string) *Options { +func NewOptions(namespace string) *Options { // TODO all default values should be defined via cobra flags defaultConfig := DefaultConfig() options := &Options{ - Primary: namespaceConfig{ - Configuration: defaultConfig, - namespace: primaryNamespace, - }, - others: make(map[string]*namespaceConfig, len(otherNamespaces)), - } - - // Other namespaces need to be explicitly enabled. - defaultConfig.Enabled = false - for _, namespace := range otherNamespaces { - options.others[namespace] = &namespaceConfig{ + Config: namespaceConfig{ Configuration: defaultConfig, namespace: namespace, - } + }, } return options @@ -126,10 +115,7 @@ func (cfg *namespaceConfig) getTLSFlagsConfig() tlscfg.ClientFlagsConfig { // AddFlags adds flags for Options func (opt *Options) AddFlags(flagSet *flag.FlagSet) { - addFlags(flagSet, &opt.Primary) - for _, cfg := range opt.others { - addFlags(flagSet, cfg) - } + addFlags(flagSet, &opt.Config) } func addFlags(flagSet *flag.FlagSet, nsConfig *namespaceConfig) { @@ -289,7 +275,7 @@ func addFlags(flagSet *flag.FlagSet, nsConfig *namespaceConfig) { if nsConfig.namespace == archiveNamespace { flagSet.Bool( nsConfig.namespace+suffixEnabled, - nsConfig.Enabled, + false, "Enable extra storage") } else { // MaxSpanAge is only relevant when searching for unarchived traces. @@ -304,10 +290,7 @@ func addFlags(flagSet *flag.FlagSet, nsConfig *namespaceConfig) { // InitFromViper initializes Options with properties from viper func (opt *Options) InitFromViper(v *viper.Viper) { - initFromViper(&opt.Primary, v) - for _, cfg := range opt.others { - initFromViper(cfg, v) - } + initFromViper(&opt.Config, v) } func initFromViper(cfg *namespaceConfig, v *viper.Viper) { @@ -388,22 +371,8 @@ func initFromViper(cfg *namespaceConfig, v *viper.Viper) { } // GetPrimary returns primary configuration. -func (opt *Options) GetPrimary() *config.Configuration { - return &opt.Primary.Configuration -} - -// Get returns auxiliary named configuration. -func (opt *Options) Get(namespace string) *config.Configuration { - nsCfg, ok := opt.others[namespace] - if !ok { - nsCfg = &namespaceConfig{} - opt.others[namespace] = nsCfg - } - nsCfg.Configuration.ApplyDefaults(&opt.Primary.Configuration) - if len(nsCfg.Configuration.Servers) == 0 { - nsCfg.Servers = opt.Primary.Servers - } - return &nsCfg.Configuration +func (opt *Options) GetConfig() *config.Configuration { + return &opt.Config.Configuration } // stripWhiteSpace removes all whitespace characters from a string diff --git a/plugin/storage/es/options_test.go b/plugin/storage/es/options_test.go index 87b2ce11d7f..cb3b05af479 100644 --- a/plugin/storage/es/options_test.go +++ b/plugin/storage/es/options_test.go @@ -5,7 +5,6 @@ package es import ( - "net/http" "testing" "time" @@ -18,7 +17,7 @@ import ( func TestOptions(t *testing.T) { opts := NewOptions("foo") - primary := opts.GetPrimary() + primary := opts.GetConfig() assert.Empty(t, primary.Authentication.BasicAuthentication.Username) assert.Empty(t, primary.Authentication.BasicAuthentication.Password) assert.Empty(t, primary.Authentication.BasicAuthentication.PasswordFilePath) @@ -35,16 +34,10 @@ func TestOptions(t *testing.T) { assert.Equal(t, 72*time.Hour, primary.MaxSpanAge) assert.False(t, primary.Sniffing.Enabled) assert.False(t, primary.Sniffing.UseHTTPS) - - aux := opts.Get("archive") - assert.Equal(t, primary.Authentication.BasicAuthentication.Username, aux.Authentication.BasicAuthentication.Username) - assert.Equal(t, primary.Authentication.BasicAuthentication.Password, aux.Authentication.BasicAuthentication.Password) - assert.Equal(t, primary.Authentication.BasicAuthentication.PasswordFilePath, aux.Authentication.BasicAuthentication.PasswordFilePath) - assert.Equal(t, primary.Servers, aux.Servers) } func TestOptionsWithFlags(t *testing.T) { - opts := NewOptions("es", "es.aux") + opts := NewOptions("es") v, command := config.Viperize(opts.AddFlags) err := command.ParseFlags([]string{ "--es.server-urls=1.1.1.1, 2.2.2.2", @@ -62,11 +55,6 @@ func TestOptionsWithFlags(t *testing.T) { "--es.index-rollover-frequency-services=day", // a couple overrides "--es.remote-read-clusters=cluster_one,cluster_two", - "--es.aux.server-urls=3.3.3.3, 4.4.4.4", - "--es.aux.max-span-age=24h", - "--es.aux.num-replicas=10", - "--es.aux.index-date-separator=.", - "--es.aux.index-rollover-frequency-spans=hour", "--es.tls.enabled=true", "--es.tls.skip-host-verify=true", "--es.tags-as-fields.all=true", @@ -79,7 +67,7 @@ func TestOptionsWithFlags(t *testing.T) { require.NoError(t, err) opts.InitFromViper(v) - primary := opts.GetPrimary() + primary := opts.GetConfig() assert.Equal(t, "hello", primary.Authentication.BasicAuthentication.Username) assert.Equal(t, "world", primary.Authentication.BasicAuthentication.Password) assert.Equal(t, "/foo/bar", primary.Authentication.BearerTokenAuthentication.FilePath) @@ -97,32 +85,11 @@ func TestOptionsWithFlags(t *testing.T) { assert.Equal(t, "test,tags", primary.Tags.Include) assert.Equal(t, "20060102", primary.Indices.Services.DateLayout) assert.Equal(t, "2006010215", primary.Indices.Spans.DateLayout) - aux := opts.Get("es.aux") - assert.Equal(t, []string{"3.3.3.3", "4.4.4.4"}, aux.Servers) - assert.Equal(t, "hello", aux.Authentication.BasicAuthentication.Username) - assert.Equal(t, "world", aux.Authentication.BasicAuthentication.Password) - assert.EqualValues(t, 5, aux.Indices.Spans.Shards) - assert.EqualValues(t, 5, aux.Indices.Services.Shards) - assert.EqualValues(t, 5, aux.Indices.Sampling.Shards) - assert.EqualValues(t, 5, aux.Indices.Dependencies.Shards) - assert.EqualValues(t, 10, aux.Indices.Spans.Replicas) - assert.EqualValues(t, 10, aux.Indices.Services.Replicas) - assert.EqualValues(t, 10, aux.Indices.Sampling.Replicas) - assert.EqualValues(t, 10, aux.Indices.Dependencies.Replicas) - assert.Equal(t, 24*time.Hour, aux.MaxSpanAge) - assert.True(t, aux.Sniffing.Enabled) - assert.True(t, aux.Tags.AllAsFields) - assert.Equal(t, "@", aux.Tags.DotReplacement) - assert.Equal(t, "./file.txt", aux.Tags.File) - assert.Equal(t, "test,tags", aux.Tags.Include) - assert.Equal(t, "2006.01.02", aux.Indices.Services.DateLayout) - assert.Equal(t, "2006.01.02.15", aux.Indices.Spans.DateLayout) assert.True(t, primary.UseILM) - assert.Equal(t, http.MethodPost, aux.SendGetBodyAs) } func TestEmptyRemoteReadClusters(t *testing.T) { - opts := NewOptions("es", "es.aux") + opts := NewOptions("es") v, command := config.Viperize(opts.AddFlags) err := command.ParseFlags([]string{ "--es.remote-read-clusters=", @@ -130,12 +97,12 @@ func TestEmptyRemoteReadClusters(t *testing.T) { require.NoError(t, err) opts.InitFromViper(v) - primary := opts.GetPrimary() + primary := opts.GetConfig() assert.Equal(t, []string{}, primary.RemoteReadClusters) } func TestMaxSpanAgeSetErrorInArchiveMode(t *testing.T) { - opts := NewOptions("es", archiveNamespace) + opts := NewOptions(archiveNamespace) _, command := config.Viperize(opts.AddFlags) flags := []string{"--es-archive.max-span-age=24h"} err := command.ParseFlags(flags) @@ -153,12 +120,12 @@ func TestMaxDocCount(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - opts := NewOptions("es", "es.aux") + opts := NewOptions("es") v, command := config.Viperize(opts.AddFlags) command.ParseFlags(tc.flags) opts.InitFromViper(v) - primary := opts.GetPrimary() + primary := opts.GetConfig() assert.Equal(t, tc.wantMaxDocCount, primary.MaxDocCount) }) } @@ -184,7 +151,7 @@ func TestIndexDateSeparator(t *testing.T) { command.ParseFlags(tc.flags) opts.InitFromViper(v) - primary := opts.GetPrimary() + primary := opts.GetConfig() assert.Equal(t, tc.wantDateLayout, primary.Indices.Spans.DateLayout) }) } @@ -238,7 +205,7 @@ func TestIndexRollover(t *testing.T) { v, command := config.Viperize(opts.AddFlags) command.ParseFlags(tc.flags) opts.InitFromViper(v) - primary := opts.GetPrimary() + primary := opts.GetConfig() assert.Equal(t, tc.wantSpanDateLayout, primary.Indices.Spans.DateLayout) assert.Equal(t, tc.wantServiceDateLayout, primary.Indices.Services.DateLayout) assert.Equal(t, tc.wantSpanIndexRolloverFrequency, escfg.RolloverFrequencyAsNegativeDuration(primary.Indices.Spans.RolloverFrequency)) diff --git a/plugin/storage/factory.go b/plugin/storage/factory.go index be1b808c46e..6142c299456 100644 --- a/plugin/storage/factory.go +++ b/plugin/storage/factory.go @@ -74,10 +74,9 @@ func AllSamplingStorageTypes() []string { } var ( // interface comformance checks - _ storage.Factory = (*Factory)(nil) - _ storage.ArchiveFactory = (*Factory)(nil) - _ io.Closer = (*Factory)(nil) - _ plugin.Configurable = (*Factory)(nil) + _ storage.Factory = (*Factory)(nil) + _ io.Closer = (*Factory)(nil) + _ plugin.Configurable = (*Factory)(nil) ) // Factory implements storage.Factory interface as a meta-factory for storage components. @@ -85,6 +84,7 @@ type Factory struct { FactoryConfig metricsFactory metrics.Factory factories map[string]storage.Factory + archiveFactories map[string]storage.Factory downsamplingFlagsAdded bool } @@ -103,12 +103,17 @@ func NewFactory(config FactoryConfig) (*Factory, error) { uniqueTypes[f.SamplingStorageType] = struct{}{} } f.factories = make(map[string]storage.Factory) + f.archiveFactories = make(map[string]storage.Factory) for t := range uniqueTypes { ff, err := f.getFactoryOfType(t) if err != nil { return nil, err } f.factories[t] = ff + + if af, ok := f.getArchiveFactoryOfType(t); ok { + f.archiveFactories[t] = af + } } return f, nil } @@ -134,23 +139,50 @@ func (*Factory) getFactoryOfType(factoryType string) (storage.Factory, error) { } } -// Initialize implements storage.Factory. +func (*Factory) getArchiveFactoryOfType(factoryType string) (storage.Factory, bool) { + switch factoryType { + case cassandraStorageType: + return cassandra.NewArchiveFactory(), true + case elasticsearchStorageType, opensearchStorageType: + return es.NewArchiveFactory(), true + case grpcStorageType: + return grpc.NewArchiveFactory(), true + default: + return nil, false + } +} + func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error { f.metricsFactory = metricsFactory - for kind, factory := range f.factories { + + initializeFactory := func(kind string, factory storage.Factory, role string) error { mf := metricsFactory.Namespace(metrics.NSOptions{ Name: "storage", Tags: map[string]string{ "kind": kind, - "role": "primary", // can be overiden in the storage factory for archive/sampling stores + "role": role, }, }) - if err := factory.Initialize(mf, logger); err != nil { + return factory.Initialize(mf, logger) + } + + for kind, factory := range f.factories { + if err := initializeFactory(kind, factory, "primary"); err != nil { return err } } - f.publishOpts() + for kind, factory := range f.archiveFactories { + if archivable, ok := factory.(storage.ArchiveCapable); ok && archivable.IsArchiveCapable() { + if err := initializeFactory(kind, factory, "archive"); err != nil { + return err + } + } else { + delete(f.archiveFactories, kind) + } + } + + f.publishOpts() return nil } @@ -233,11 +265,15 @@ func (f *Factory) CreateDependencyReader() (dependencystore.Reader, error) { // AddFlags implements plugin.Configurable func (f *Factory) AddFlags(flagSet *flag.FlagSet) { - for _, factory := range f.factories { - if conf, ok := factory.(plugin.Configurable); ok { - conf.AddFlags(flagSet) + addFlags := func(factories map[string]storage.Factory) { + for _, factory := range factories { + if conf, ok := factory.(plugin.Configurable); ok { + conf.AddFlags(flagSet) + } } } + addFlags(f.factories) + addFlags(f.archiveFactories) } // AddPipelineFlags adds all the standard flags as well as the downsampling @@ -265,11 +301,23 @@ func (f *Factory) addDownsamplingFlags(flagSet *flag.FlagSet) { // InitFromViper implements plugin.Configurable func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) { - for _, factory := range f.factories { + initializeConfigurable := func(factory storage.Factory) { if conf, ok := factory.(plugin.Configurable); ok { conf.InitFromViper(v, logger) } } + for _, factory := range f.factories { + initializeConfigurable(factory) + } + for kind, factory := range f.archiveFactories { + initializeConfigurable(factory) + + if primaryFactory, ok := f.factories[kind]; ok { + if inheritable, ok := factory.(storage.Inheritable); ok { + inheritable.InheritSettingsFrom(primaryFactory) + } + } + } f.initDownsamplingFromViper(v) } @@ -290,30 +338,37 @@ func (f *Factory) initDownsamplingFromViper(v *viper.Viper) { f.FactoryConfig.DownsamplingHashSalt = v.GetString(downsamplingHashSalt) } -// CreateArchiveSpanReader implements storage.ArchiveFactory -func (f *Factory) CreateArchiveSpanReader() (spanstore.Reader, error) { - factory, ok := f.factories[f.SpanReaderType] +func (f *Factory) createArchiveSpanReader() (spanstore.Reader, error) { + factory, ok := f.archiveFactories[f.SpanReaderType] if !ok { return nil, fmt.Errorf("no %s backend registered for span store", f.SpanReaderType) } - archive, ok := factory.(storage.ArchiveFactory) - if !ok { - return nil, storage.ErrArchiveStorageNotSupported - } - return archive.CreateArchiveSpanReader() + return factory.CreateSpanReader() } -// CreateArchiveSpanWriter implements storage.ArchiveFactory -func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { - factory, ok := f.factories[f.SpanWriterTypes[0]] +func (f *Factory) createArchiveSpanWriter() (spanstore.Writer, error) { + factory, ok := f.archiveFactories[f.SpanWriterTypes[0]] if !ok { return nil, fmt.Errorf("no %s backend registered for span store", f.SpanWriterTypes[0]) } - archive, ok := factory.(storage.ArchiveFactory) - if !ok { - return nil, storage.ErrArchiveStorageNotSupported + return factory.CreateSpanWriter() +} + +func (f *Factory) InitArchiveStorage( + logger *zap.Logger, +) (spanstore.Reader, spanstore.Writer) { + reader, err := f.createArchiveSpanReader() + if err != nil { + logger.Error("Cannot init archive storage reader", zap.Error(err)) + return nil, nil + } + writer, err := f.createArchiveSpanWriter() + if err != nil { + logger.Error("Cannot init archive storage writer", zap.Error(err)) + return nil, nil } - return archive.CreateArchiveSpanWriter() + + return reader, writer } var _ io.Closer = (*Factory)(nil) @@ -321,14 +376,19 @@ var _ io.Closer = (*Factory)(nil) // Close closes the resources held by the factory func (f *Factory) Close() error { var errs []error + closeFactory := func(factory storage.Factory) { + if closer, ok := factory.(io.Closer); ok { + if err := closer.Close(); err != nil { + errs = append(errs, err) + } + } + } for _, storageType := range f.SpanWriterTypes { if factory, ok := f.factories[storageType]; ok { - if closer, ok := factory.(io.Closer); ok { - err := closer.Close() - if err != nil { - errs = append(errs, err) - } - } + closeFactory(factory) + } + if factory, ok := f.archiveFactories[storageType]; ok { + closeFactory(factory) } } return errors.Join(errs...) diff --git a/plugin/storage/factory_test.go b/plugin/storage/factory_test.go index c09f8e7504c..a9aaa4490d3 100644 --- a/plugin/storage/factory_test.go +++ b/plugin/storage/factory_test.go @@ -20,6 +20,7 @@ import ( "github.com/jaegertracing/jaeger/pkg/config" "github.com/jaegertracing/jaeger/pkg/metrics" + "github.com/jaegertracing/jaeger/pkg/testutils" "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" depStoreMocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks" @@ -90,6 +91,7 @@ func TestInitialize(t *testing.T) { mock := new(mocks.Factory) f.factories[cassandraStorageType] = mock + f.archiveFactories[cassandraStorageType] = mock m := metrics.NullFactory l := zap.NewNop() @@ -131,12 +133,6 @@ func TestCreate(t *testing.T) { assert.Equal(t, depReader, d) require.EqualError(t, err, "dep-reader-error") - _, err = f.CreateArchiveSpanReader() - require.EqualError(t, err, "archive storage not supported") - - _, err = f.CreateArchiveSpanWriter() - require.EqualError(t, err, "archive storage not supported") - mock.On("CreateSpanWriter").Return(spanWriter, nil) m := metrics.NullFactory l := zap.NewNop() @@ -190,7 +186,9 @@ func TestCreateMulti(t *testing.T) { mock := new(mocks.Factory) mock2 := new(mocks.Factory) f.factories[cassandraStorageType] = mock + f.archiveFactories[cassandraStorageType] = mock f.factories[elasticsearchStorageType] = mock2 + f.archiveFactories[elasticsearchStorageType] = mock2 spanWriter := new(spanStoreMocks.Writer) spanWriter2 := new(spanStoreMocks.Writer) @@ -213,32 +211,6 @@ func TestCreateMulti(t *testing.T) { assert.Equal(t, spanstore.NewCompositeWriter(spanWriter, spanWriter2), w) } -func TestCreateArchive(t *testing.T) { - f, err := NewFactory(defaultCfg()) - require.NoError(t, err) - assert.NotEmpty(t, f.factories[cassandraStorageType]) - - mock := &struct { - mocks.Factory - mocks.ArchiveFactory - }{} - f.factories[cassandraStorageType] = mock - - archiveSpanReader := new(spanStoreMocks.Reader) - archiveSpanWriter := new(spanStoreMocks.Writer) - - mock.ArchiveFactory.On("CreateArchiveSpanReader").Return(archiveSpanReader, errors.New("archive-span-reader-error")) - mock.ArchiveFactory.On("CreateArchiveSpanWriter").Return(archiveSpanWriter, errors.New("archive-span-writer-error")) - - ar, err := f.CreateArchiveSpanReader() - assert.Equal(t, archiveSpanReader, ar) - require.EqualError(t, err, "archive-span-reader-error") - - aw, err := f.CreateArchiveSpanWriter() - assert.Equal(t, archiveSpanWriter, aw) - require.EqualError(t, err, "archive-span-writer-error") -} - func TestCreateError(t *testing.T) { f, err := NewFactory(defaultCfg()) require.NoError(t, err) @@ -265,18 +237,6 @@ func TestCreateError(t *testing.T) { assert.Nil(t, d) require.EqualError(t, err, expectedErr) } - - { - r, err := f.CreateArchiveSpanReader() - assert.Nil(t, r) - require.EqualError(t, err, expectedErr) - } - - { - w, err := f.CreateArchiveSpanWriter() - assert.Nil(t, w) - require.EqualError(t, err, expectedErr) - } } func TestAllSamplingStorageTypes(t *testing.T) { @@ -355,6 +315,107 @@ func TestConfigurable(t *testing.T) { assert.Equal(t, v, mock.viper) } +type inheritable struct { + mocks.Factory + calledWith storage.Factory +} + +func (i *inheritable) InheritSettingsFrom(other storage.Factory) { + i.calledWith = other +} + +func TestInheritable(t *testing.T) { + f, err := NewFactory(defaultCfg()) + require.NoError(t, err) + assert.NotEmpty(t, f.factories) + assert.NotEmpty(t, f.factories[cassandraStorageType]) + assert.NotEmpty(t, f.archiveFactories[cassandraStorageType]) + + mockInheritable := new(inheritable) + f.factories[cassandraStorageType] = &mocks.Factory{} + f.archiveFactories[cassandraStorageType] = mockInheritable + + fs := new(flag.FlagSet) + v := viper.New() + + f.AddFlags(fs) + f.InitFromViper(v, zap.NewNop()) + + assert.Equal(t, f.factories[cassandraStorageType], mockInheritable.calledWith) +} + +type archiveConfigurable struct { + isConfigurable bool + *mocks.Factory +} + +func (ac *archiveConfigurable) IsArchiveCapable() bool { + return ac.isConfigurable +} + +func TestArchiveConfigurable(t *testing.T) { + tests := []struct { + name string + isArchiveCapable bool + archiveInitError error + expectedError error + expectedArchiveSize int + }{ + { + name: "Archive factory initializes successfully", + isArchiveCapable: true, + archiveInitError: nil, + expectedError: nil, + expectedArchiveSize: 1, + }, + { + name: "Archive factory initialization fails", + isArchiveCapable: true, + archiveInitError: assert.AnError, + expectedError: assert.AnError, + expectedArchiveSize: 1, + }, + { + name: "Archive factory is not archive-capable", + isArchiveCapable: false, + archiveInitError: nil, + expectedError: nil, + expectedArchiveSize: 0, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + f, err := NewFactory(defaultCfg()) + require.NoError(t, err) + + primaryFactory := &mocks.Factory{} + archiveFactory := &mocks.Factory{} + archiveConfigurable := &archiveConfigurable{ + isConfigurable: test.isArchiveCapable, + Factory: archiveFactory, + } + + f.factories[cassandraStorageType] = primaryFactory + f.archiveFactories[cassandraStorageType] = archiveConfigurable + + m := metrics.NullFactory + l := zap.NewNop() + primaryFactory.On("Initialize", m, l).Return(nil).Once() + archiveFactory.On("Initialize", m, l).Return(test.archiveInitError).Once() + + err = f.Initialize(m, l) + if test.expectedError != nil { + require.ErrorIs(t, err, test.expectedError) + } else { + require.NoError(t, err) + } + + assert.Len(t, f.archiveFactories, test.expectedArchiveSize) + }) + } +} + func TestParsingDownsamplingRatio(t *testing.T) { f := Factory{} v, command := config.Viperize(f.AddPipelineFlags) @@ -401,6 +462,69 @@ func TestPublishOpts(t *testing.T) { assert.EqualValues(t, 1, expvar.Get(spanStorageType+"-"+f.SpanReaderType).(*expvar.Int).Value()) } +func TestInitArchiveStorage(t *testing.T) { + tests := []struct { + name string + setupMock func(*mocks.Factory) + expectedReader spanstore.Reader + expectedWriter spanstore.Writer + expectedLog string + }{ + { + name: "successful initialization", + setupMock: func(mock *mocks.Factory) { + spanReader := &spanStoreMocks.Reader{} + spanWriter := &spanStoreMocks.Writer{} + mock.On("CreateSpanReader").Return(spanReader, nil) + mock.On("CreateSpanWriter").Return(spanWriter, nil) + }, + expectedReader: &spanStoreMocks.Reader{}, + expectedWriter: &spanStoreMocks.Writer{}, + }, + { + name: "error initializing reader", + setupMock: func(mock *mocks.Factory) { + mock.On("CreateSpanReader").Return(nil, assert.AnError) + }, + expectedReader: nil, + expectedWriter: nil, + expectedLog: "Cannot init archive storage reader", + }, + { + name: "error initializing writer", + setupMock: func(mock *mocks.Factory) { + spanReader := new(spanStoreMocks.Reader) + mock.On("CreateSpanReader").Return(spanReader, nil) + mock.On("CreateSpanWriter").Return(nil, assert.AnError) + }, + expectedReader: nil, + expectedWriter: nil, + expectedLog: "Cannot init archive storage writer", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + logger, buf := testutils.NewLogger() + + f, err := NewFactory(defaultCfg()) + require.NoError(t, err) + assert.NotEmpty(t, f.factories) + assert.NotEmpty(t, f.factories[cassandraStorageType]) + + mock := new(mocks.Factory) + f.archiveFactories[cassandraStorageType] = mock + test.setupMock(mock) + + reader, writer := f.InitArchiveStorage(logger) + assert.Equal(t, test.expectedReader, reader) + assert.Equal(t, test.expectedWriter, writer) + + require.Contains(t, buf.String(), test.expectedLog) + }) + } +} + type errorFactory struct { closeErr error } diff --git a/plugin/storage/grpc/config.go b/plugin/storage/grpc/config.go index 37a5d293fa2..d9150bcba33 100644 --- a/plugin/storage/grpc/config.go +++ b/plugin/storage/grpc/config.go @@ -16,6 +16,7 @@ type Config struct { Tenancy tenancy.Options `mapstructure:"multi_tenancy"` configgrpc.ClientConfig `mapstructure:",squash"` exporterhelper.TimeoutConfig `mapstructure:",squash"` + enabled bool } func DefaultConfig() Config { diff --git a/plugin/storage/grpc/factory.go b/plugin/storage/grpc/factory.go index e316ceb7d3e..dd6c68a9a40 100644 --- a/plugin/storage/grpc/factory.go +++ b/plugin/storage/grpc/factory.go @@ -35,14 +35,14 @@ import ( var ( // interface comformance checks _ storage.Factory = (*Factory)(nil) - _ storage.ArchiveFactory = (*Factory)(nil) _ io.Closer = (*Factory)(nil) _ plugin.Configurable = (*Factory)(nil) + _ storage.ArchiveCapable = (*Factory)(nil) ) // Factory implements storage.Factory and creates storage components backed by a storage plugin. type Factory struct { - config Config + options *options telset telemetry.Settings services *ClientPluginServices tracedRemoteConn *grpc.ClientConn @@ -52,7 +52,15 @@ type Factory struct { // NewFactory creates a new Factory. func NewFactory() *Factory { return &Factory{ - telset: telemetry.NoopSettings(), + options: newOptions(remotePrefix), + telset: telemetry.NoopSettings(), + } +} + +func NewArchiveFactory() *Factory { + return &Factory{ + options: newOptions(archiveRemotePrefix), + telset: telemetry.NoopSettings(), } } @@ -62,7 +70,7 @@ func NewFactoryWithConfig( telset telemetry.Settings, ) (*Factory, error) { f := NewFactory() - f.config = cfg + f.options.Config = cfg f.telset = telset if err := f.Initialize(telset.Metrics, telset.Logger); err != nil { return nil, err @@ -71,13 +79,13 @@ func NewFactoryWithConfig( } // AddFlags implements plugin.Configurable -func (*Factory) AddFlags(flagSet *flag.FlagSet) { - addFlags(flagSet) +func (f *Factory) AddFlags(flagSet *flag.FlagSet) { + f.options.addFlags(flagSet) } // InitFromViper implements plugin.Configurable func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) { - if err := initFromViper(&f.config, v); err != nil { + if err := f.options.initFromViper(&f.options.Config, v); err != nil { logger.Fatal("unable to initialize gRPC storage factory", zap.Error(err)) } } @@ -95,7 +103,7 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) for _, opt := range opts { clientOpts = append(clientOpts, configgrpc.WithGrpcDialOption(opt)) } - return f.config.ToClientConn(context.Background(), f.telset.Host, telset, clientOpts...) + return f.options.Config.ToClientConn(context.Background(), f.telset.Host, telset, clientOpts...) } var err error @@ -103,7 +111,7 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) if err != nil { return fmt.Errorf("grpc storage builder failed to create a store: %w", err) } - logger.Info("Remote storage configuration", zap.Any("configuration", f.config)) + logger.Info("Remote storage configuration", zap.Any("configuration", f.options.Config)) return nil } @@ -114,7 +122,7 @@ func (f *Factory) newRemoteStorage( untracedTelset component.TelemetrySettings, newClient newClientFn, ) (*ClientPluginServices, error) { - c := f.config + c := f.options.Config if c.Auth != nil { return nil, errors.New("authenticator is not supported") } @@ -158,7 +166,6 @@ func (f *Factory) newRemoteStorage( return &ClientPluginServices{ PluginServices: shared.PluginServices{ Store: grpcClient, - ArchiveStore: grpcClient, StreamingSpanWriter: grpcClient, }, Capabilities: grpcClient, @@ -185,43 +192,6 @@ func (f *Factory) CreateDependencyReader() (dependencystore.Reader, error) { return f.services.Store.DependencyReader(), nil } -// CreateArchiveSpanReader implements storage.ArchiveFactory -func (f *Factory) CreateArchiveSpanReader() (spanstore.Reader, error) { - if f.services.Capabilities == nil { - return nil, storage.ErrArchiveStorageNotSupported - } - capabilities, err := f.services.Capabilities.Capabilities() - if err != nil { - return nil, err - } - if capabilities == nil || !capabilities.ArchiveSpanReader { - return nil, storage.ErrArchiveStorageNotSupported - } - archiveMetricsFactory := f.telset.Metrics.Namespace( - metrics.NSOptions{ - Tags: map[string]string{ - "role": "archive", - }, - }, - ) - return spanstoremetrics.NewReaderDecorator(f.services.ArchiveStore.ArchiveSpanReader(), archiveMetricsFactory), nil -} - -// CreateArchiveSpanWriter implements storage.ArchiveFactory -func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { - if f.services.Capabilities == nil { - return nil, storage.ErrArchiveStorageNotSupported - } - capabilities, err := f.services.Capabilities.Capabilities() - if err != nil { - return nil, err - } - if capabilities == nil || !capabilities.ArchiveSpanWriter { - return nil, storage.ErrArchiveStorageNotSupported - } - return f.services.ArchiveStore.ArchiveSpanWriter(), nil -} - // Close closes the resources held by the factory func (f *Factory) Close() error { var errs []error @@ -241,3 +211,7 @@ func getTelset(logger *zap.Logger, tracerProvider trace.TracerProvider, meterPro MeterProvider: meterProvider, } } + +func (f *Factory) IsArchiveCapable() bool { + return f.options.namespace == archiveRemotePrefix && f.options.enabled +} diff --git a/plugin/storage/grpc/factory_test.go b/plugin/storage/grpc/factory_test.go index a9bf26c49c4..89d30731e00 100644 --- a/plugin/storage/grpc/factory_test.go +++ b/plugin/storage/grpc/factory_test.go @@ -28,7 +28,6 @@ import ( "github.com/jaegertracing/jaeger/pkg/tenancy" "github.com/jaegertracing/jaeger/plugin/storage/grpc/shared" "github.com/jaegertracing/jaeger/plugin/storage/grpc/shared/mocks" - "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" dependencyStoreMocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks" "github.com/jaegertracing/jaeger/storage/spanstore" @@ -73,10 +72,6 @@ func makeMockServices() *ClientPluginServices { reader: new(spanStoreMocks.Reader), deps: new(dependencyStoreMocks.Reader), }, - ArchiveStore: &store{ - writer: new(spanStoreMocks.Writer), - reader: new(spanStoreMocks.Reader), - }, StreamingSpanWriter: &store{ writer: new(spanStoreMocks.Writer), }, @@ -88,7 +83,7 @@ func makeMockServices() *ClientPluginServices { func makeFactory(t *testing.T) *Factory { f := NewFactory() f.InitFromViper(viper.New(), zap.NewNop()) - f.config.ClientConfig.Endpoint = ":0" + f.options.ClientConfig.Endpoint = ":0" require.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) t.Cleanup(func() { @@ -115,7 +110,7 @@ func TestNewFactoryError(t *testing.T) { t.Run("viper", func(t *testing.T) { f := NewFactory() f.InitFromViper(viper.New(), zap.NewNop()) - f.config = *cfg + f.options.Config = *cfg err := f.Initialize(metrics.NullFactory, zap.NewNop()) assert.ErrorContains(t, err, "authenticator") }) @@ -189,20 +184,10 @@ func TestGRPCStorageFactory_Capabilities(t *testing.T) { capabilities := f.services.Capabilities.(*mocks.PluginCapabilities) capabilities.On("Capabilities"). Return(&shared.Capabilities{ - ArchiveSpanReader: true, - ArchiveSpanWriter: true, StreamingSpanWriter: true, - }, nil).Times(3) + }, nil).Times(1) - reader, err := f.CreateArchiveSpanReader() - require.NoError(t, err) - assert.NotNil(t, reader) - - writer, err := f.CreateArchiveSpanWriter() - require.NoError(t, err) - assert.NotNil(t, writer) - - writer, err = f.CreateSpanWriter() + writer, err := f.CreateSpanWriter() require.NoError(t, err) assert.NotNil(t, writer) } @@ -213,18 +198,10 @@ func TestGRPCStorageFactory_CapabilitiesDisabled(t *testing.T) { capabilities := f.services.Capabilities.(*mocks.PluginCapabilities) capabilities.On("Capabilities"). Return(&shared.Capabilities{ - ArchiveSpanReader: false, - ArchiveSpanWriter: false, StreamingSpanWriter: false, - }, nil).Times(3) - - reader, err := f.CreateArchiveSpanReader() - require.EqualError(t, err, storage.ErrArchiveStorageNotSupported.Error()) - assert.Nil(t, reader) - writer, err := f.CreateArchiveSpanWriter() - require.EqualError(t, err, storage.ErrArchiveStorageNotSupported.Error()) - assert.Nil(t, writer) - writer, err = f.CreateSpanWriter() + }, nil).Times(1) + + writer, err := f.CreateSpanWriter() require.NoError(t, err) assert.NotNil(t, writer, "regular span writer is available") } @@ -236,13 +213,7 @@ func TestGRPCStorageFactory_CapabilitiesError(t *testing.T) { customError := errors.New("made-up error") capabilities.On("Capabilities").Return(nil, customError) - reader, err := f.CreateArchiveSpanReader() - require.EqualError(t, err, customError.Error()) - assert.Nil(t, reader) - writer, err := f.CreateArchiveSpanWriter() - require.EqualError(t, err, customError.Error()) - assert.Nil(t, writer) - writer, err = f.CreateSpanWriter() + writer, err := f.CreateSpanWriter() require.NoError(t, err) assert.NotNil(t, writer, "regular span writer is available") } @@ -251,13 +222,7 @@ func TestGRPCStorageFactory_CapabilitiesNil(t *testing.T) { f := makeFactory(t) f.services.Capabilities = nil - reader, err := f.CreateArchiveSpanReader() - assert.Equal(t, err, storage.ErrArchiveStorageNotSupported) - assert.Nil(t, reader) - writer, err := f.CreateArchiveSpanWriter() - assert.Equal(t, err, storage.ErrArchiveStorageNotSupported) - assert.Nil(t, writer) - writer, err = f.CreateSpanWriter() + writer, err := f.CreateSpanWriter() require.NoError(t, err) assert.NotNil(t, writer, "regular span writer is available") } @@ -270,7 +235,7 @@ func TestWithCLIFlags(t *testing.T) { }) require.NoError(t, err) f.InitFromViper(v, zap.NewNop()) - assert.Equal(t, "foo:1234", f.config.ClientConfig.Endpoint) + assert.Equal(t, "foo:1234", f.options.Config.ClientConfig.Endpoint) require.NoError(t, f.Close()) } @@ -322,3 +287,46 @@ func TestStreamingSpanWriterFactory_Capabilities(t *testing.T) { err = writer.WriteSpan(context.Background(), nil) assert.ErrorContains(t, err, "I am streaming writer", "streaming writer when Capabilities return true") } + +func TestIsArchiveCapable(t *testing.T) { + tests := []struct { + name string + namespace string + enabled bool + expected bool + }{ + { + name: "archive capable", + namespace: "grpc-storage-archive", + enabled: true, + expected: true, + }, + { + name: "not capable", + namespace: "grpc-storage-archive", + enabled: false, + expected: false, + }, + { + name: "capable + wrong namespace", + namespace: "grpc-storage", + enabled: true, + expected: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + factory := &Factory{ + options: &options{ + namespace: test.namespace, + Config: Config{ + enabled: test.enabled, + }, + }, + } + result := factory.IsArchiveCapable() + require.Equal(t, test.expected, result) + }) + } +} diff --git a/plugin/storage/grpc/options.go b/plugin/storage/grpc/options.go index 3461cfc28a4..747e4a924ea 100644 --- a/plugin/storage/grpc/options.go +++ b/plugin/storage/grpc/options.go @@ -16,33 +16,57 @@ import ( const ( remotePrefix = "grpc-storage" - remoteServer = remotePrefix + ".server" - remoteConnectionTimeout = remotePrefix + ".connection-timeout" + archiveRemotePrefix = "grpc-storage-archive" + remoteServer = ".server" + remoteConnectionTimeout = ".connection-timeout" + enabled = ".enabled" defaultConnectionTimeout = time.Duration(5 * time.Second) ) -func tlsFlagsConfig() tlscfg.ClientFlagsConfig { +type options struct { + Config + namespace string +} + +func newOptions(namespace string) *options { + options := &options{ + Config: DefaultConfig(), + namespace: namespace, + } + return options +} + +func (opts *options) tlsFlagsConfig() tlscfg.ClientFlagsConfig { return tlscfg.ClientFlagsConfig{ - Prefix: remotePrefix, + Prefix: opts.namespace, } } // addFlags adds flags for Options -func addFlags(flagSet *flag.FlagSet) { - tlsFlagsConfig().AddFlags(flagSet) +func (opts *options) addFlags(flagSet *flag.FlagSet) { + opts.tlsFlagsConfig().AddFlags(flagSet) - flagSet.String(remoteServer, "", "The remote storage gRPC server address as host:port") - flagSet.Duration(remoteConnectionTimeout, defaultConnectionTimeout, "The remote storage gRPC server connection timeout") + flagSet.String(opts.namespace+remoteServer, "", "The remote storage gRPC server address as host:port") + flagSet.Duration(opts.namespace+remoteConnectionTimeout, defaultConnectionTimeout, "The remote storage gRPC server connection timeout") + if opts.namespace == archiveRemotePrefix { + flagSet.Bool( + opts.namespace+enabled, + false, + "Enable extra storage") + } } -func initFromViper(cfg *Config, v *viper.Viper) error { - cfg.ClientConfig.Endpoint = v.GetString(remoteServer) - remoteTLSCfg, err := tlsFlagsConfig().InitFromViper(v) +func (opts *options) initFromViper(cfg *Config, v *viper.Viper) error { + cfg.ClientConfig.Endpoint = v.GetString(opts.namespace + remoteServer) + remoteTLSCfg, err := opts.tlsFlagsConfig().InitFromViper(v) if err != nil { return fmt.Errorf("failed to parse gRPC storage TLS options: %w", err) } cfg.ClientConfig.TLSSetting = remoteTLSCfg - cfg.TimeoutConfig.Timeout = v.GetDuration(remoteConnectionTimeout) + cfg.TimeoutConfig.Timeout = v.GetDuration(opts.namespace + remoteConnectionTimeout) cfg.Tenancy = tenancy.InitFromViper(v) + if opts.namespace == archiveRemotePrefix { + cfg.enabled = v.GetBool(opts.namespace + enabled) + } return nil } diff --git a/plugin/storage/grpc/options_test.go b/plugin/storage/grpc/options_test.go index a3e8953f098..dc715e4119f 100644 --- a/plugin/storage/grpc/options_test.go +++ b/plugin/storage/grpc/options_test.go @@ -18,14 +18,15 @@ import ( ) func TestOptionsWithFlags(t *testing.T) { - v, command := config.Viperize(addFlags, tenancy.AddFlags) + opts := newOptions("grpc-storage") + v, command := config.Viperize(opts.addFlags, tenancy.AddFlags) err := command.ParseFlags([]string{ "--grpc-storage.server=foo:12345", "--multi-tenancy.header=x-scope-orgid", }) require.NoError(t, err) var cfg Config - require.NoError(t, initFromViper(&cfg, v)) + require.NoError(t, opts.initFromViper(&cfg, v)) assert.Equal(t, "foo:12345", cfg.ClientConfig.Endpoint) assert.False(t, cfg.Tenancy.Enabled) @@ -33,7 +34,8 @@ func TestOptionsWithFlags(t *testing.T) { } func TestRemoteOptionsWithFlags(t *testing.T) { - v, command := config.Viperize(addFlags) + opts := newOptions("grpc-storage") + v, command := config.Viperize(opts.addFlags) err := command.ParseFlags([]string{ "--grpc-storage.server=localhost:2001", "--grpc-storage.tls.enabled=true", @@ -41,7 +43,7 @@ func TestRemoteOptionsWithFlags(t *testing.T) { }) require.NoError(t, err) var cfg Config - require.NoError(t, initFromViper(&cfg, v)) + require.NoError(t, opts.initFromViper(&cfg, v)) assert.Equal(t, "localhost:2001", cfg.ClientConfig.Endpoint) assert.False(t, cfg.ClientConfig.TLSSetting.Insecure) @@ -49,7 +51,8 @@ func TestRemoteOptionsWithFlags(t *testing.T) { } func TestRemoteOptionsNoTLSWithFlags(t *testing.T) { - v, command := config.Viperize(addFlags) + opts := newOptions("grpc-storage") + v, command := config.Viperize(opts.addFlags) err := command.ParseFlags([]string{ "--grpc-storage.server=localhost:2001", "--grpc-storage.tls.enabled=false", @@ -57,7 +60,7 @@ func TestRemoteOptionsNoTLSWithFlags(t *testing.T) { }) require.NoError(t, err) var cfg Config - require.NoError(t, initFromViper(&cfg, v)) + require.NoError(t, opts.initFromViper(&cfg, v)) assert.Equal(t, "localhost:2001", cfg.ClientConfig.Endpoint) assert.True(t, cfg.ClientConfig.TLSSetting.Insecure) @@ -65,7 +68,8 @@ func TestRemoteOptionsNoTLSWithFlags(t *testing.T) { } func TestFailedTLSFlags(t *testing.T) { - v, command := config.Viperize(addFlags) + opts := newOptions("grpc-storage") + v, command := config.Viperize(opts.addFlags) err := command.ParseFlags([]string{ "--grpc-storage.tls.enabled=false", "--grpc-storage.tls.cert=blah", // invalid unless tls.enabled=true diff --git a/plugin/storage/grpc/shared/archive.go b/plugin/storage/grpc/shared/archive.go deleted file mode 100644 index 8776f7542b6..00000000000 --- a/plugin/storage/grpc/shared/archive.go +++ /dev/null @@ -1,81 +0,0 @@ -// Copyright (c) 2020 The Jaeger Authors. -// SPDX-License-Identifier: Apache-2.0 - -package shared - -import ( - "context" - "errors" - "fmt" - - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" - - "github.com/jaegertracing/jaeger-idl/model/v1" - "github.com/jaegertracing/jaeger/proto-gen/storage_v1" - "github.com/jaegertracing/jaeger/storage/spanstore" -) - -var ( - _ spanstore.Reader = (*archiveReader)(nil) - _ spanstore.Writer = (*archiveWriter)(nil) -) - -// archiveReader wraps storage_v1.ArchiveSpanReaderPluginClient into spanstore.Reader -type archiveReader struct { - client storage_v1.ArchiveSpanReaderPluginClient -} - -// ArchiveWriter wraps storage_v1.ArchiveSpanWriterPluginClient into spanstore.Writer -type archiveWriter struct { - client storage_v1.ArchiveSpanWriterPluginClient -} - -// GetTrace takes a traceID and returns a Trace associated with that traceID from Archive Storage -func (r *archiveReader) GetTrace(ctx context.Context, q spanstore.GetTraceParameters) (*model.Trace, error) { - stream, err := r.client.GetArchiveTrace(ctx, &storage_v1.GetTraceRequest{ - TraceID: q.TraceID, - StartTime: q.StartTime, - EndTime: q.EndTime, - }) - if status.Code(err) == codes.NotFound { - return nil, spanstore.ErrTraceNotFound - } - if err != nil { - return nil, fmt.Errorf("plugin error: %w", err) - } - - return readTrace(stream) -} - -// GetServices not used in archiveReader -func (*archiveReader) GetServices(context.Context) ([]string, error) { - return nil, errors.New("GetServices not implemented") -} - -// GetOperations not used in archiveReader -func (*archiveReader) GetOperations(context.Context, spanstore.OperationQueryParameters) ([]spanstore.Operation, error) { - return nil, errors.New("GetOperations not implemented") -} - -// FindTraces not used in archiveReader -func (*archiveReader) FindTraces(context.Context, *spanstore.TraceQueryParameters) ([]*model.Trace, error) { - return nil, errors.New("FindTraces not implemented") -} - -// FindTraceIDs not used in archiveReader -func (*archiveReader) FindTraceIDs(context.Context, *spanstore.TraceQueryParameters) ([]model.TraceID, error) { - return nil, errors.New("FindTraceIDs not implemented") -} - -// WriteSpan saves the span into Archive Storage -func (w *archiveWriter) WriteSpan(ctx context.Context, span *model.Span) error { - _, err := w.client.WriteArchiveSpan(ctx, &storage_v1.WriteSpanRequest{ - Span: span, - }) - if err != nil { - return fmt.Errorf("plugin error: %w", err) - } - - return nil -} diff --git a/plugin/storage/grpc/shared/archive_test.go b/plugin/storage/grpc/shared/archive_test.go deleted file mode 100644 index 100d4b9ba26..00000000000 --- a/plugin/storage/grpc/shared/archive_test.go +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright (c) 2020 The Jaeger Authors. -// SPDX-License-Identifier: Apache-2.0 - -package shared - -import ( - "context" - "io" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" - - "github.com/jaegertracing/jaeger-idl/model/v1" - "github.com/jaegertracing/jaeger/proto-gen/storage_v1" - "github.com/jaegertracing/jaeger/proto-gen/storage_v1/mocks" - "github.com/jaegertracing/jaeger/storage/spanstore" -) - -func TestArchiveWriter_WriteSpan(t *testing.T) { - mockSpan := &model.Span{ - TraceID: mockTraceID, - SpanID: model.NewSpanID(1), - Process: &model.Process{}, - } - - archiveSpanWriter := new(mocks.ArchiveSpanWriterPluginClient) - archiveSpanWriter.On("WriteArchiveSpan", mock.Anything, &storage_v1.WriteSpanRequest{Span: mockSpan}). - Return(&storage_v1.WriteSpanResponse{}, nil) - writer := &archiveWriter{client: archiveSpanWriter} - - err := writer.WriteSpan(context.Background(), mockSpan) - require.NoError(t, err) -} - -func TestArchiveReader_GetTrace(t *testing.T) { - mockTraceID := model.NewTraceID(0, 123456) - mockSpan := model.Span{ - TraceID: mockTraceID, - SpanID: model.NewSpanID(1), - Process: &model.Process{}, - } - expected := &model.Trace{ - Spans: []*model.Span{&mockSpan}, - } - - traceClient := new(mocks.ArchiveSpanReaderPlugin_GetArchiveTraceClient) - traceClient.On("Recv").Return(&storage_v1.SpansResponseChunk{ - Spans: []model.Span{mockSpan}, - }, nil).Once() - traceClient.On("Recv").Return(nil, io.EOF) - - archiveSpanReader := new(mocks.ArchiveSpanReaderPluginClient) - archiveSpanReader.On("GetArchiveTrace", mock.Anything, &storage_v1.GetTraceRequest{ - TraceID: mockTraceID, - }).Return(traceClient, nil) - reader := &archiveReader{client: archiveSpanReader} - - trace, err := reader.GetTrace(context.Background(), spanstore.GetTraceParameters{ - TraceID: mockTraceID, - }) - require.NoError(t, err) - assert.Equal(t, expected, trace) -} - -func TestArchiveReaderGetTrace_NoTrace(t *testing.T) { - mockTraceID := model.NewTraceID(0, 123456) - - archiveSpanReader := new(mocks.ArchiveSpanReaderPluginClient) - archiveSpanReader.On("GetArchiveTrace", mock.Anything, &storage_v1.GetTraceRequest{ - TraceID: mockTraceID, - }).Return(nil, status.Errorf(codes.NotFound, "")) - reader := &archiveReader{client: archiveSpanReader} - - _, err := reader.GetTrace(context.Background(), spanstore.GetTraceParameters{ - TraceID: mockTraceID, - }) - assert.Equal(t, spanstore.ErrTraceNotFound, err) -} - -func TestArchiveReader_FindTraceIDs(t *testing.T) { - reader := archiveReader{client: &mocks.ArchiveSpanReaderPluginClient{}} - _, err := reader.FindTraceIDs(context.Background(), nil) - require.Error(t, err) -} - -func TestArchiveReader_FindTraces(t *testing.T) { - reader := archiveReader{client: &mocks.ArchiveSpanReaderPluginClient{}} - _, err := reader.FindTraces(context.Background(), nil) - require.Error(t, err) -} - -func TestArchiveReader_GetOperations(t *testing.T) { - reader := archiveReader{client: &mocks.ArchiveSpanReaderPluginClient{}} - _, err := reader.GetOperations(context.Background(), spanstore.OperationQueryParameters{}) - require.Error(t, err) -} - -func TestArchiveReader_GetServices(t *testing.T) { - reader := archiveReader{client: &mocks.ArchiveSpanReaderPluginClient{}} - _, err := reader.GetServices(context.Background()) - require.Error(t, err) -} diff --git a/plugin/storage/grpc/shared/grpc_client.go b/plugin/storage/grpc/shared/grpc_client.go index 7f464104b0e..b5a17383bd4 100644 --- a/plugin/storage/grpc/shared/grpc_client.go +++ b/plugin/storage/grpc/shared/grpc_client.go @@ -25,31 +25,26 @@ import ( const BearerTokenKey = "bearer.token" var ( - _ StoragePlugin = (*GRPCClient)(nil) - _ ArchiveStoragePlugin = (*GRPCClient)(nil) - _ PluginCapabilities = (*GRPCClient)(nil) + _ StoragePlugin = (*GRPCClient)(nil) + _ PluginCapabilities = (*GRPCClient)(nil) ) // GRPCClient implements shared.StoragePlugin and reads/writes spans and dependencies type GRPCClient struct { - readerClient storage_v1.SpanReaderPluginClient - writerClient storage_v1.SpanWriterPluginClient - archiveReaderClient storage_v1.ArchiveSpanReaderPluginClient - archiveWriterClient storage_v1.ArchiveSpanWriterPluginClient - capabilitiesClient storage_v1.PluginCapabilitiesClient - depsReaderClient storage_v1.DependenciesReaderPluginClient - streamWriterClient storage_v1.StreamingSpanWriterPluginClient + readerClient storage_v1.SpanReaderPluginClient + writerClient storage_v1.SpanWriterPluginClient + capabilitiesClient storage_v1.PluginCapabilitiesClient + depsReaderClient storage_v1.DependenciesReaderPluginClient + streamWriterClient storage_v1.StreamingSpanWriterPluginClient } func NewGRPCClient(tracedConn *grpc.ClientConn, untracedConn *grpc.ClientConn) *GRPCClient { return &GRPCClient{ - readerClient: storage_v1.NewSpanReaderPluginClient(tracedConn), - writerClient: storage_v1.NewSpanWriterPluginClient(untracedConn), - archiveReaderClient: storage_v1.NewArchiveSpanReaderPluginClient(tracedConn), - archiveWriterClient: storage_v1.NewArchiveSpanWriterPluginClient(untracedConn), - capabilitiesClient: storage_v1.NewPluginCapabilitiesClient(tracedConn), - depsReaderClient: storage_v1.NewDependenciesReaderPluginClient(tracedConn), - streamWriterClient: storage_v1.NewStreamingSpanWriterPluginClient(untracedConn), + readerClient: storage_v1.NewSpanReaderPluginClient(tracedConn), + writerClient: storage_v1.NewSpanWriterPluginClient(untracedConn), + capabilitiesClient: storage_v1.NewPluginCapabilitiesClient(tracedConn), + depsReaderClient: storage_v1.NewDependenciesReaderPluginClient(tracedConn), + streamWriterClient: storage_v1.NewStreamingSpanWriterPluginClient(untracedConn), } } @@ -72,14 +67,6 @@ func (c *GRPCClient) StreamingSpanWriter() spanstore.Writer { return newStreamingSpanWriter(c.streamWriterClient) } -func (c *GRPCClient) ArchiveSpanReader() spanstore.Reader { - return &archiveReader{client: c.archiveReaderClient} -} - -func (c *GRPCClient) ArchiveSpanWriter() spanstore.Writer { - return &archiveWriter{client: c.archiveWriterClient} -} - // GetTrace takes a traceID and returns a Trace associated with that traceID func (c *GRPCClient) GetTrace(ctx context.Context, query spanstore.GetTraceParameters) (*model.Trace, error) { stream, err := c.readerClient.GetTrace(ctx, &storage_v1.GetTraceRequest{ @@ -243,8 +230,6 @@ func (c *GRPCClient) Capabilities() (*Capabilities, error) { } return &Capabilities{ - ArchiveSpanReader: capabilities.ArchiveSpanReader, - ArchiveSpanWriter: capabilities.ArchiveSpanWriter, StreamingSpanWriter: capabilities.StreamingSpanWriter, }, nil } diff --git a/plugin/storage/grpc/shared/grpc_client_test.go b/plugin/storage/grpc/shared/grpc_client_test.go index bc96a77cec1..bfcd601896e 100644 --- a/plugin/storage/grpc/shared/grpc_client_test.go +++ b/plugin/storage/grpc/shared/grpc_client_test.go @@ -60,42 +60,34 @@ var ( ) type grpcClientTest struct { - client *GRPCClient - spanReader *grpcMocks.SpanReaderPluginClient - spanWriter *grpcMocks.SpanWriterPluginClient - archiveReader *grpcMocks.ArchiveSpanReaderPluginClient - archiveWriter *grpcMocks.ArchiveSpanWriterPluginClient - capabilities *grpcMocks.PluginCapabilitiesClient - depsReader *grpcMocks.DependenciesReaderPluginClient - streamWriter *grpcMocks.StreamingSpanWriterPluginClient + client *GRPCClient + spanReader *grpcMocks.SpanReaderPluginClient + spanWriter *grpcMocks.SpanWriterPluginClient + capabilities *grpcMocks.PluginCapabilitiesClient + depsReader *grpcMocks.DependenciesReaderPluginClient + streamWriter *grpcMocks.StreamingSpanWriterPluginClient } func withGRPCClient(fn func(r *grpcClientTest)) { spanReader := new(grpcMocks.SpanReaderPluginClient) - archiveReader := new(grpcMocks.ArchiveSpanReaderPluginClient) spanWriter := new(grpcMocks.SpanWriterPluginClient) - archiveWriter := new(grpcMocks.ArchiveSpanWriterPluginClient) depReader := new(grpcMocks.DependenciesReaderPluginClient) streamWriter := new(grpcMocks.StreamingSpanWriterPluginClient) capabilities := new(grpcMocks.PluginCapabilitiesClient) r := &grpcClientTest{ client: &GRPCClient{ - readerClient: spanReader, - writerClient: spanWriter, - archiveReaderClient: archiveReader, - archiveWriterClient: archiveWriter, - capabilitiesClient: capabilities, - depsReaderClient: depReader, - streamWriterClient: streamWriter, + readerClient: spanReader, + writerClient: spanWriter, + capabilitiesClient: capabilities, + depsReaderClient: depReader, + streamWriterClient: streamWriter, }, - spanReader: spanReader, - spanWriter: spanWriter, - archiveReader: archiveReader, - archiveWriter: archiveWriter, - depsReader: depReader, - capabilities: capabilities, - streamWriter: streamWriter, + spanReader: spanReader, + spanWriter: spanWriter, + depsReader: depReader, + capabilities: capabilities, + streamWriter: streamWriter, } fn(r) } @@ -107,8 +99,6 @@ func TestNewGRPCClient(t *testing.T) { assert.Implements(t, (*storage_v1.SpanReaderPluginClient)(nil), client.readerClient) assert.Implements(t, (*storage_v1.SpanWriterPluginClient)(nil), client.writerClient) - assert.Implements(t, (*storage_v1.ArchiveSpanReaderPluginClient)(nil), client.archiveReaderClient) - assert.Implements(t, (*storage_v1.ArchiveSpanWriterPluginClient)(nil), client.archiveWriterClient) assert.Implements(t, (*storage_v1.PluginCapabilitiesClient)(nil), client.capabilitiesClient) assert.Implements(t, (*storage_v1.DependenciesReaderPluginClient)(nil), client.depsReaderClient) assert.Implements(t, (*storage_v1.StreamingSpanWriterPluginClient)(nil), client.streamWriterClient) @@ -338,28 +328,6 @@ func TestGRPCClientGetDependencies(t *testing.T) { }) } -func TestGrpcClientWriteArchiveSpan(t *testing.T) { - withGRPCClient(func(r *grpcClientTest) { - r.archiveWriter.On("WriteArchiveSpan", mock.Anything, &storage_v1.WriteSpanRequest{ - Span: &mockTraceSpans[0], - }).Return(&storage_v1.WriteSpanResponse{}, nil) - - err := r.client.ArchiveSpanWriter().WriteSpan(context.Background(), &mockTraceSpans[0]) - require.NoError(t, err) - }) -} - -func TestGrpcClientWriteArchiveSpan_Error(t *testing.T) { - withGRPCClient(func(r *grpcClientTest) { - r.archiveWriter.On("WriteArchiveSpan", mock.Anything, &storage_v1.WriteSpanRequest{ - Span: &mockTraceSpans[0], - }).Return(nil, status.Error(codes.Internal, "internal error")) - - err := r.client.ArchiveSpanWriter().WriteSpan(context.Background(), &mockTraceSpans[0]) - require.Error(t, err) - }) -} - func TestGrpcClientStreamWriterWriteSpan(t *testing.T) { withGRPCClient(func(r *grpcClientTest) { stream := new(grpcMocks.StreamingSpanWriterPlugin_WriteSpanStreamClient) @@ -370,87 +338,6 @@ func TestGrpcClientStreamWriterWriteSpan(t *testing.T) { }) } -func TestGrpcClientGetArchiveTrace(t *testing.T) { - withGRPCClient(func(r *grpcClientTest) { - startTime := time.Date(2020, time.January, 1, 13, 0, 0, 0, time.UTC) - endTime := time.Date(2020, time.January, 1, 14, 0, 0, 0, time.UTC) - traceClient := new(grpcMocks.ArchiveSpanReaderPlugin_GetArchiveTraceClient) - traceClient.On("Recv").Return(&storage_v1.SpansResponseChunk{ - Spans: mockTraceSpans, - }, nil).Once() - traceClient.On("Recv").Return(nil, io.EOF) - r.archiveReader.On("GetArchiveTrace", mock.Anything, &storage_v1.GetTraceRequest{ - TraceID: mockTraceID, - StartTime: startTime, - EndTime: endTime, - }).Return(traceClient, nil) - - var expectedSpans []*model.Span - for i := range mockTraceSpans { - expectedSpans = append(expectedSpans, &mockTraceSpans[i]) - } - - s, err := r.client.ArchiveSpanReader().GetTrace(context.Background(), spanstore.GetTraceParameters{ - TraceID: mockTraceID, - StartTime: startTime, - EndTime: endTime, - }) - require.NoError(t, err) - assert.Equal(t, &model.Trace{ - Spans: expectedSpans, - }, s) - }) -} - -func TestGrpcClientGetArchiveTrace_StreamError(t *testing.T) { - withGRPCClient(func(r *grpcClientTest) { - traceClient := new(grpcMocks.ArchiveSpanReaderPlugin_GetArchiveTraceClient) - traceClient.On("Recv").Return(nil, errors.New("an error")) - r.archiveReader.On("GetArchiveTrace", mock.Anything, &storage_v1.GetTraceRequest{ - TraceID: mockTraceID, - }).Return(traceClient, nil) - - s, err := r.client.ArchiveSpanReader().GetTrace(context.Background(), spanstore.GetTraceParameters{ - TraceID: mockTraceID, - }) - require.Error(t, err) - assert.Nil(t, s) - }) -} - -func TestGrpcClientGetArchiveTrace_NoTrace(t *testing.T) { - withGRPCClient(func(r *grpcClientTest) { - r.archiveReader.On("GetArchiveTrace", mock.Anything, &storage_v1.GetTraceRequest{ - TraceID: mockTraceID, - }).Return(nil, spanstore.ErrTraceNotFound) - - s, err := r.client.ArchiveSpanReader().GetTrace( - context.Background(), - spanstore.GetTraceParameters{ - TraceID: mockTraceID, - }, - ) - require.Error(t, err) - assert.Nil(t, s) - }) -} - -func TestGrpcClientGetArchiveTrace_StreamErrorTraceNotFound(t *testing.T) { - withGRPCClient(func(r *grpcClientTest) { - traceClient := new(grpcMocks.ArchiveSpanReaderPlugin_GetArchiveTraceClient) - traceClient.On("Recv").Return(nil, spanstore.ErrTraceNotFound) - r.archiveReader.On("GetArchiveTrace", mock.Anything, &storage_v1.GetTraceRequest{ - TraceID: mockTraceID, - }).Return(traceClient, nil) - - s, err := r.client.ArchiveSpanReader().GetTrace(context.Background(), spanstore.GetTraceParameters{ - TraceID: mockTraceID, - }) - assert.Equal(t, spanstore.ErrTraceNotFound, err) - assert.Nil(t, s) - }) -} - func TestGrpcClientCapabilities(t *testing.T) { withGRPCClient(func(r *grpcClientTest) { r.capabilities.On("Capabilities", mock.Anything, &storage_v1.CapabilitiesRequest{}). @@ -459,8 +346,6 @@ func TestGrpcClientCapabilities(t *testing.T) { capabilities, err := r.client.Capabilities() require.NoError(t, err) assert.Equal(t, &Capabilities{ - ArchiveSpanReader: true, - ArchiveSpanWriter: true, StreamingSpanWriter: true, }, capabilities) }) @@ -474,8 +359,6 @@ func TestGrpcClientCapabilities_NotSupported(t *testing.T) { capabilities, err := r.client.Capabilities() require.NoError(t, err) assert.Equal(t, &Capabilities{ - ArchiveSpanReader: false, - ArchiveSpanWriter: false, StreamingSpanWriter: false, }, capabilities) }) diff --git a/plugin/storage/grpc/shared/grpc_handler_test.go b/plugin/storage/grpc/shared/grpc_handler_test.go index d8942a86c81..1c3cc7cc838 100644 --- a/plugin/storage/grpc/shared/grpc_handler_test.go +++ b/plugin/storage/grpc/shared/grpc_handler_test.go @@ -26,20 +26,10 @@ import ( ) type mockStoragePlugin struct { - spanReader *spanStoreMocks.Reader - spanWriter *spanStoreMocks.Writer - archiveReader *spanStoreMocks.Reader - archiveWriter *spanStoreMocks.Writer - depsReader *dependencyStoreMocks.Reader - streamWriter *spanStoreMocks.Writer -} - -func (plugin *mockStoragePlugin) ArchiveSpanReader() spanstore.Reader { - return plugin.archiveReader -} - -func (plugin *mockStoragePlugin) ArchiveSpanWriter() spanstore.Writer { - return plugin.archiveWriter + spanReader *spanStoreMocks.Reader + spanWriter *spanStoreMocks.Writer + depsReader *dependencyStoreMocks.Reader + streamWriter *spanStoreMocks.Writer } func (plugin *mockStoragePlugin) SpanReader() spanstore.Reader { @@ -66,18 +56,14 @@ type grpcServerTest struct { func withGRPCServer(fn func(r *grpcServerTest)) { spanReader := new(spanStoreMocks.Reader) spanWriter := new(spanStoreMocks.Writer) - archiveReader := new(spanStoreMocks.Reader) - archiveWriter := new(spanStoreMocks.Writer) depReader := new(dependencyStoreMocks.Reader) streamWriter := new(spanStoreMocks.Writer) mockPlugin := &mockStoragePlugin{ - spanReader: spanReader, - spanWriter: spanWriter, - archiveReader: archiveReader, - archiveWriter: archiveWriter, - depsReader: depReader, - streamWriter: streamWriter, + spanReader: spanReader, + spanWriter: spanWriter, + depsReader: depReader, + streamWriter: streamWriter, } impl := &GRPCHandlerStorageImpl{ @@ -91,10 +77,10 @@ func withGRPCServer(fn func(r *grpcServerTest)) { return mockPlugin.depsReader }, ArchiveSpanReader: func() spanstore.Reader { - return mockPlugin.archiveReader + return mockPlugin.spanReader }, ArchiveSpanWriter: func() spanstore.Writer { - return mockPlugin.archiveWriter + return mockPlugin.spanWriter }, StreamingSpanWriter: func() spanstore.Writer { return mockPlugin.streamWriter @@ -305,7 +291,7 @@ func TestGRPCServerGetArchiveTrace(t *testing.T) { for i := range mockTraceSpans { traceSpans = append(traceSpans, &mockTraceSpans[i]) } - r.impl.archiveReader.On("GetTrace", mock.Anything, spanstore.GetTraceParameters{TraceID: mockTraceID}). + r.impl.spanReader.On("GetTrace", mock.Anything, spanstore.GetTraceParameters{TraceID: mockTraceID}). Return(&model.Trace{Spans: traceSpans}, nil) err := r.server.GetArchiveTrace(&storage_v1.GetTraceRequest{ @@ -320,7 +306,7 @@ func TestGRPCServerGetArchiveTrace_NotFound(t *testing.T) { traceSteam := new(grpcMocks.SpanReaderPlugin_GetTraceServer) traceSteam.On("Context").Return(context.Background()) - r.impl.archiveReader.On("GetTrace", mock.Anything, spanstore.GetTraceParameters{TraceID: mockTraceID}). + r.impl.spanReader.On("GetTrace", mock.Anything, spanstore.GetTraceParameters{TraceID: mockTraceID}). Return(nil, spanstore.ErrTraceNotFound) err := r.server.GetArchiveTrace(&storage_v1.GetTraceRequest{ @@ -335,7 +321,7 @@ func TestGRPCServerGetArchiveTrace_Error(t *testing.T) { traceSteam := new(grpcMocks.SpanReaderPlugin_GetTraceServer) traceSteam.On("Context").Return(context.Background()) - r.impl.archiveReader.On("GetTrace", mock.Anything, spanstore.GetTraceParameters{TraceID: mockTraceID}). + r.impl.spanReader.On("GetTrace", mock.Anything, spanstore.GetTraceParameters{TraceID: mockTraceID}). Return(nil, errors.New("some error")) err := r.server.GetArchiveTrace(&storage_v1.GetTraceRequest{ @@ -350,7 +336,7 @@ func TestGRPCServerGetArchiveTrace_NoImpl(t *testing.T) { r.server.impl.ArchiveSpanReader = func() spanstore.Reader { return nil } traceSteam := new(grpcMocks.SpanReaderPlugin_GetTraceServer) - r.impl.archiveReader.On("GetTrace", mock.Anything, spanstore.GetTraceParameters{TraceID: mockTraceID}). + r.impl.spanReader.On("GetTrace", mock.Anything, spanstore.GetTraceParameters{TraceID: mockTraceID}). Return(nil, errors.New("some error")) err := r.server.GetArchiveTrace(&storage_v1.GetTraceRequest{ @@ -371,7 +357,7 @@ func TestGRPCServerGetArchiveTrace_StreamError(t *testing.T) { for i := range mockTraceSpans { traceSpans = append(traceSpans, &mockTraceSpans[i]) } - r.impl.archiveReader.On("GetTrace", mock.Anything, spanstore.GetTraceParameters{TraceID: mockTraceID}). + r.impl.spanReader.On("GetTrace", mock.Anything, spanstore.GetTraceParameters{TraceID: mockTraceID}). Return(&model.Trace{Spans: traceSpans}, nil) err := r.server.GetArchiveTrace(&storage_v1.GetTraceRequest{ @@ -394,7 +380,7 @@ func TestGRPCServerWriteArchiveSpan_NoImpl(t *testing.T) { func TestGRPCServerWriteArchiveSpan(t *testing.T) { withGRPCServer(func(r *grpcServerTest) { - r.impl.archiveWriter.On("WriteSpan", mock.Anything, &mockTraceSpans[0]). + r.impl.spanWriter.On("WriteSpan", mock.Anything, &mockTraceSpans[0]). Return(nil) s, err := r.server.WriteArchiveSpan(context.Background(), &storage_v1.WriteSpanRequest{ @@ -407,7 +393,7 @@ func TestGRPCServerWriteArchiveSpan(t *testing.T) { func TestGRPCServerWriteArchiveSpan_Error(t *testing.T) { withGRPCServer(func(r *grpcServerTest) { - r.impl.archiveWriter.On("WriteSpan", mock.Anything, &mockTraceSpans[0]). + r.impl.spanWriter.On("WriteSpan", mock.Anything, &mockTraceSpans[0]). Return(errors.New("some error")) _, err := r.server.WriteArchiveSpan(context.Background(), &storage_v1.WriteSpanRequest{ diff --git a/plugin/storage/grpc/shared/interface.go b/plugin/storage/grpc/shared/interface.go index 9abc2f75dce..42a7f91937f 100644 --- a/plugin/storage/grpc/shared/interface.go +++ b/plugin/storage/grpc/shared/interface.go @@ -15,12 +15,6 @@ type StoragePlugin interface { DependencyReader() dependencystore.Reader } -// ArchiveStoragePlugin is the interface we're exposing as a plugin. -type ArchiveStoragePlugin interface { - ArchiveSpanReader() spanstore.Reader - ArchiveSpanWriter() spanstore.Writer -} - // StreamingSpanWriterPlugin is the interface we're exposing as a plugin. type StreamingSpanWriterPlugin interface { StreamingSpanWriter() spanstore.Writer @@ -33,14 +27,11 @@ type PluginCapabilities interface { // Capabilities contains information about plugin capabilities type Capabilities struct { - ArchiveSpanReader bool - ArchiveSpanWriter bool StreamingSpanWriter bool } // PluginServices defines services plugin can expose type PluginServices struct { Store StoragePlugin - ArchiveStore ArchiveStoragePlugin StreamingSpanWriter StreamingSpanWriterPlugin } diff --git a/plugin/storage/integration/cassandra_test.go b/plugin/storage/integration/cassandra_test.go index 161a695cc03..a3aeddcc9a8 100644 --- a/plugin/storage/integration/cassandra_test.go +++ b/plugin/storage/integration/cassandra_test.go @@ -24,7 +24,8 @@ import ( type CassandraStorageIntegration struct { StorageIntegration - factory *cassandra.Factory + factory *cassandra.Factory + archiveFactory *cassandra.Factory } func newCassandraStorageIntegration() *CassandraStorageIntegration { @@ -41,11 +42,12 @@ func newCassandraStorageIntegration() *CassandraStorageIntegration { func (s *CassandraStorageIntegration) cleanUp(t *testing.T) { require.NoError(t, s.factory.Purge(context.Background())) + require.NoError(t, s.archiveFactory.Purge(context.Background())) } -func (*CassandraStorageIntegration) initializeCassandraFactory(t *testing.T, flags []string) *cassandra.Factory { +func (*CassandraStorageIntegration) initializeCassandraFactory(t *testing.T, flags []string, factoryInit func() *cassandra.Factory) *cassandra.Factory { logger := zaptest.NewLogger(t, zaptest.WrapOptions(zap.AddCaller())) - f := cassandra.NewFactory() + f := factoryInit() v, command := config.Viperize(f.AddFlags) require.NoError(t, command.ParseFlags(flags)) f.InitFromViper(v, logger) @@ -64,14 +66,17 @@ func (s *CassandraStorageIntegration) initializeCassandra(t *testing.T) { "--cassandra.password=" + password, "--cassandra.username=" + username, "--cassandra.keyspace=jaeger_v1_dc1", + }, cassandra.NewFactory) + af := s.initializeCassandraFactory(t, []string{ "--cassandra-archive.keyspace=jaeger_v1_dc1_archive", "--cassandra-archive.enabled=true", "--cassandra-archive.servers=127.0.0.1", "--cassandra-archive.basic.allowed-authenticators=org.apache.cassandra.auth.PasswordAuthenticator", "--cassandra-archive.password=" + password, "--cassandra-archive.username=" + username, - }) + }, cassandra.NewArchiveFactory) s.factory = f + s.archiveFactory = af var err error spanWriter, err := f.CreateSpanWriter() require.NoError(t, err) @@ -79,9 +84,9 @@ func (s *CassandraStorageIntegration) initializeCassandra(t *testing.T) { spanReader, err := f.CreateSpanReader() require.NoError(t, err) s.TraceReader = v1adapter.NewTraceReader(spanReader) - s.ArchiveSpanReader, err = f.CreateArchiveSpanReader() + s.ArchiveSpanReader, err = af.CreateSpanReader() require.NoError(t, err) - s.ArchiveSpanWriter, err = f.CreateArchiveSpanWriter() + s.ArchiveSpanWriter, err = af.CreateSpanWriter() require.NoError(t, err) s.SamplingStore, err = f.CreateSamplingStore(0) require.NoError(t, err) diff --git a/plugin/storage/integration/elasticsearch_test.go b/plugin/storage/integration/elasticsearch_test.go index 016c7fb87ef..b0d2c84bc25 100644 --- a/plugin/storage/integration/elasticsearch_test.go +++ b/plugin/storage/integration/elasticsearch_test.go @@ -52,7 +52,8 @@ type ESStorageIntegration struct { client *elastic.Client v8Client *elasticsearch8.Client - factory *es.Factory + factory *es.Factory + archiveFactory *es.Factory } func (s *ESStorageIntegration) getVersion() (uint, error) { @@ -100,25 +101,13 @@ func (s *ESStorageIntegration) initializeES(t *testing.T, c *http.Client, allTag func (s *ESStorageIntegration) esCleanUp(t *testing.T) { require.NoError(t, s.factory.Purge(context.Background())) + require.NoError(t, s.archiveFactory.Purge(context.Background())) } -func (*ESStorageIntegration) initializeESFactory(t *testing.T, allTagsAsFields bool) *es.Factory { +func (*ESStorageIntegration) initializeESFactory(t *testing.T, args []string, factoryInit func() *es.Factory) *es.Factory { logger := zaptest.NewLogger(t, zaptest.WrapOptions(zap.AddCaller())) - f := es.NewFactory() + f := factoryInit() v, command := config.Viperize(f.AddFlags) - args := []string{ - fmt.Sprintf("--es.num-shards=%v", 5), - fmt.Sprintf("--es.num-replicas=%v", 1), - fmt.Sprintf("--es.index-prefix=%v", indexPrefix), - fmt.Sprintf("--es.use-ilm=%v", false), - fmt.Sprintf("--es.service-cache-ttl=%v", 1*time.Second), - fmt.Sprintf("--es.tags-as-fields.all=%v", allTagsAsFields), - fmt.Sprintf("--es.bulk.actions=%v", 1), - fmt.Sprintf("--es.bulk.flush-interval=%v", time.Nanosecond), - "--es-archive.enabled=true", - fmt.Sprintf("--es-archive.tags-as-fields.all=%v", allTagsAsFields), - fmt.Sprintf("--es-archive.index-prefix=%v", indexPrefix), - } require.NoError(t, command.ParseFlags(args)) f.InitFromViper(v, logger) require.NoError(t, f.Initialize(metrics.NullFactory, logger)) @@ -130,8 +119,23 @@ func (*ESStorageIntegration) initializeESFactory(t *testing.T, allTagsAsFields b } func (s *ESStorageIntegration) initSpanstore(t *testing.T, allTagsAsFields bool) { - f := s.initializeESFactory(t, allTagsAsFields) + f := s.initializeESFactory(t, []string{ + fmt.Sprintf("--es.num-shards=%v", 5), + fmt.Sprintf("--es.num-replicas=%v", 1), + fmt.Sprintf("--es.index-prefix=%v", indexPrefix), + fmt.Sprintf("--es.use-ilm=%v", false), + fmt.Sprintf("--es.service-cache-ttl=%v", 1*time.Second), + fmt.Sprintf("--es.tags-as-fields.all=%v", allTagsAsFields), + fmt.Sprintf("--es.bulk.actions=%v", 1), + fmt.Sprintf("--es.bulk.flush-interval=%v", time.Nanosecond), + }, es.NewFactory) + af := s.initializeESFactory(t, []string{ + "--es-archive.enabled=true", + fmt.Sprintf("--es-archive.tags-as-fields.all=%v", allTagsAsFields), + fmt.Sprintf("--es-archive.index-prefix=%v", indexPrefix), + }, es.NewArchiveFactory) s.factory = f + s.archiveFactory = af var err error spanWriter, err := f.CreateSpanWriter() require.NoError(t, err) @@ -139,9 +143,9 @@ func (s *ESStorageIntegration) initSpanstore(t *testing.T, allTagsAsFields bool) spanReader, err := f.CreateSpanReader() require.NoError(t, err) s.TraceReader = v1adapter.NewTraceReader(spanReader) - s.ArchiveSpanReader, err = f.CreateArchiveSpanReader() + s.ArchiveSpanReader, err = af.CreateSpanReader() require.NoError(t, err) - s.ArchiveSpanWriter, err = f.CreateArchiveSpanWriter() + s.ArchiveSpanWriter, err = af.CreateSpanWriter() require.NoError(t, err) dependencyReader, err := f.CreateDependencyReader() diff --git a/plugin/storage/integration/grpc_test.go b/plugin/storage/integration/grpc_test.go index 9d8e85bc29e..76e089b1e16 100644 --- a/plugin/storage/integration/grpc_test.go +++ b/plugin/storage/integration/grpc_test.go @@ -15,27 +15,37 @@ import ( "github.com/jaegertracing/jaeger/pkg/metrics" "github.com/jaegertracing/jaeger/pkg/testutils" "github.com/jaegertracing/jaeger/plugin/storage/grpc" + "github.com/jaegertracing/jaeger/ports" "github.com/jaegertracing/jaeger/storage_v2/v1adapter" ) type GRPCStorageIntegrationTestSuite struct { StorageIntegration - flags []string - factory *grpc.Factory - remoteStorage *RemoteMemoryStorage + flags []string + archiveFlags []string + factory *grpc.Factory + archiveFactory *grpc.Factory + remoteStorage *RemoteMemoryStorage + archiveRemoteStorage *RemoteMemoryStorage } func (s *GRPCStorageIntegrationTestSuite) initialize(t *testing.T) { logger := zaptest.NewLogger(t, zaptest.WrapOptions(zap.AddCaller())) - s.remoteStorage = StartNewRemoteMemoryStorage(t) + s.remoteStorage = StartNewRemoteMemoryStorage(t, ports.RemoteStorageGRPC) + s.archiveRemoteStorage = StartNewRemoteMemoryStorage(t, ports.RemoteStorageGRPC+1) + initFactory := func(f *grpc.Factory, flags []string) { + v, command := config.Viperize(f.AddFlags) + require.NoError(t, command.ParseFlags(flags)) + f.InitFromViper(v, logger) + require.NoError(t, f.Initialize(metrics.NullFactory, logger)) + } f := grpc.NewFactory() - v, command := config.Viperize(f.AddFlags) - err := command.ParseFlags(s.flags) - require.NoError(t, err) - f.InitFromViper(v, logger) - require.NoError(t, f.Initialize(metrics.NullFactory, logger)) + af := grpc.NewArchiveFactory() + initFactory(f, s.flags) + initFactory(af, s.archiveFlags) s.factory = f + s.archiveFactory = af spanWriter, err := f.CreateSpanWriter() require.NoError(t, err) @@ -43,9 +53,9 @@ func (s *GRPCStorageIntegrationTestSuite) initialize(t *testing.T) { spanReader, err := f.CreateSpanReader() require.NoError(t, err) s.TraceReader = v1adapter.NewTraceReader(spanReader) - s.ArchiveSpanReader, err = f.CreateArchiveSpanReader() + s.ArchiveSpanReader, err = af.CreateSpanReader() require.NoError(t, err) - s.ArchiveSpanWriter, err = f.CreateArchiveSpanWriter() + s.ArchiveSpanWriter, err = af.CreateSpanWriter() require.NoError(t, err) // TODO DependencyWriter is not implemented in grpc store @@ -55,7 +65,9 @@ func (s *GRPCStorageIntegrationTestSuite) initialize(t *testing.T) { func (s *GRPCStorageIntegrationTestSuite) close(t *testing.T) { require.NoError(t, s.factory.Close()) + require.NoError(t, s.archiveFactory.Close()) s.remoteStorage.Close(t) + s.archiveRemoteStorage.Close(t) } func (s *GRPCStorageIntegrationTestSuite) cleanUp(t *testing.T) { @@ -73,6 +85,11 @@ func TestGRPCRemoteStorage(t *testing.T) { "--grpc-storage.server=localhost:17271", "--grpc-storage.tls.enabled=false", }, + archiveFlags: []string{ + "--grpc-storage-archive.enabled=true", + "--grpc-storage-archive.server=localhost:17272", + "--grpc-storage-archive.tls.enabled=false", + }, } s.initialize(t) defer s.close(t) diff --git a/plugin/storage/integration/remote_memory_storage.go b/plugin/storage/integration/remote_memory_storage.go index 8e72fadf47c..d569abbc65f 100644 --- a/plugin/storage/integration/remote_memory_storage.go +++ b/plugin/storage/integration/remote_memory_storage.go @@ -33,12 +33,12 @@ type RemoteMemoryStorage struct { storageFactory *storage.Factory } -func StartNewRemoteMemoryStorage(t *testing.T) *RemoteMemoryStorage { +func StartNewRemoteMemoryStorage(t *testing.T, port int) *RemoteMemoryStorage { logger := zaptest.NewLogger(t, zaptest.WrapOptions(zap.AddCaller())) opts := &app.Options{ ServerConfig: configgrpc.ServerConfig{ NetAddr: confignet.AddrConfig{ - Endpoint: ports.PortToHostPort(ports.RemoteStorageGRPC), + Endpoint: ports.PortToHostPort(port), }, }, Tenancy: tenancy.Options{ diff --git a/plugin/storage/memory/factory.go b/plugin/storage/memory/factory.go index 0a7b7b34a24..a784c5e22dc 100644 --- a/plugin/storage/memory/factory.go +++ b/plugin/storage/memory/factory.go @@ -24,7 +24,6 @@ import ( var ( // interface comformance checks _ storage.Factory = (*Factory)(nil) - _ storage.ArchiveFactory = (*Factory)(nil) _ storage.SamplingStoreFactory = (*Factory)(nil) _ plugin.Configurable = (*Factory)(nil) _ storage.Purger = (*Factory)(nil) @@ -90,23 +89,6 @@ func (f *Factory) CreateSpanWriter() (spanstore.Writer, error) { return f.store, nil } -// CreateArchiveSpanReader implements storage.ArchiveFactory -func (f *Factory) CreateArchiveSpanReader() (spanstore.Reader, error) { - archiveMetricsFactory := f.metricsFactory.Namespace( - metrics.NSOptions{ - Tags: map[string]string{ - "role": "archive", - }, - }, - ) - return spanstoremetrics.NewReaderDecorator(f.store, archiveMetricsFactory), nil -} - -// CreateArchiveSpanWriter implements storage.ArchiveFactory -func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { - return f.store, nil -} - // CreateDependencyReader implements storage.Factory func (f *Factory) CreateDependencyReader() (dependencystore.Reader, error) { return f.store, nil diff --git a/storage/factory.go b/storage/factory.go index fdac5e9d17b..da07bcd4ca8 100644 --- a/storage/factory.go +++ b/storage/factory.go @@ -6,7 +6,6 @@ package storage import ( "context" - "errors" "go.uber.org/zap" @@ -61,23 +60,6 @@ type SamplingStoreFactory interface { CreateSamplingStore(maxBuckets int) (samplingstore.Store, error) } -var ( - // ErrArchiveStorageNotConfigured can be returned by the ArchiveFactory when the archive storage is not configured. - ErrArchiveStorageNotConfigured = errors.New("archive storage not configured") - - // ErrArchiveStorageNotSupported can be returned by the ArchiveFactory when the archive storage is not supported by the backend. - ErrArchiveStorageNotSupported = errors.New("archive storage not supported") -) - -// ArchiveFactory is an additional interface that can be implemented by a factory to support trace archiving. -type ArchiveFactory interface { - // CreateArchiveSpanReader creates a spanstore.Reader. - CreateArchiveSpanReader() (spanstore.Reader, error) - - // CreateArchiveSpanWriter creates a spanstore.Writer. - CreateArchiveSpanWriter() (spanstore.Writer, error) -} - // MetricStoreFactory defines an interface for a factory that can create implementations of different metrics storage components. // Implementations are also encouraged to implement plugin.Configurable interface. // @@ -92,3 +74,15 @@ type MetricStoreFactory interface { // CreateMetricsReader creates a metricstore.Reader. CreateMetricsReader() (metricstore.Reader, error) } + +// Inheritable is an interface that can be implement by some storage implementations +// to provide a way to inherit configuration settings from another factory. +type Inheritable interface { + InheritSettingsFrom(other Factory) +} + +// ArchiveCapable is an interface that can be implemented by some storage implementations +// to indicate that they are capable of archiving data. +type ArchiveCapable interface { + IsArchiveCapable() bool +} diff --git a/storage/mocks/ArchiveCapable.go b/storage/mocks/ArchiveCapable.go new file mode 100644 index 00000000000..aef23e66bf4 --- /dev/null +++ b/storage/mocks/ArchiveCapable.go @@ -0,0 +1,47 @@ +// Copyright (c) The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 +// +// Run 'make generate-mocks' to regenerate. + +// Code generated by mockery. DO NOT EDIT. + +package mocks + +import mock "github.com/stretchr/testify/mock" + +// ArchiveCapable is an autogenerated mock type for the ArchiveCapable type +type ArchiveCapable struct { + mock.Mock +} + +// IsArchiveCapable provides a mock function with no fields +func (_m *ArchiveCapable) IsArchiveCapable() bool { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for IsArchiveCapable") + } + + var r0 bool + if rf, ok := ret.Get(0).(func() bool); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} + +// NewArchiveCapable creates a new instance of ArchiveCapable. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewArchiveCapable(t interface { + mock.TestingT + Cleanup(func()) +}) *ArchiveCapable { + mock := &ArchiveCapable{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/storage/mocks/ArchiveFactory.go b/storage/mocks/ArchiveFactory.go deleted file mode 100644 index 294b46dec58..00000000000 --- a/storage/mocks/ArchiveFactory.go +++ /dev/null @@ -1,92 +0,0 @@ -// Copyright (c) The Jaeger Authors. -// SPDX-License-Identifier: Apache-2.0 -// -// Run 'make generate-mocks' to regenerate. - -// Code generated by mockery. DO NOT EDIT. - -package mocks - -import ( - spanstore "github.com/jaegertracing/jaeger/storage/spanstore" - mock "github.com/stretchr/testify/mock" -) - -// ArchiveFactory is an autogenerated mock type for the ArchiveFactory type -type ArchiveFactory struct { - mock.Mock -} - -// CreateArchiveSpanReader provides a mock function with no fields -func (_m *ArchiveFactory) CreateArchiveSpanReader() (spanstore.Reader, error) { - ret := _m.Called() - - if len(ret) == 0 { - panic("no return value specified for CreateArchiveSpanReader") - } - - var r0 spanstore.Reader - var r1 error - if rf, ok := ret.Get(0).(func() (spanstore.Reader, error)); ok { - return rf() - } - if rf, ok := ret.Get(0).(func() spanstore.Reader); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(spanstore.Reader) - } - } - - if rf, ok := ret.Get(1).(func() error); ok { - r1 = rf() - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// CreateArchiveSpanWriter provides a mock function with no fields -func (_m *ArchiveFactory) CreateArchiveSpanWriter() (spanstore.Writer, error) { - ret := _m.Called() - - if len(ret) == 0 { - panic("no return value specified for CreateArchiveSpanWriter") - } - - var r0 spanstore.Writer - var r1 error - if rf, ok := ret.Get(0).(func() (spanstore.Writer, error)); ok { - return rf() - } - if rf, ok := ret.Get(0).(func() spanstore.Writer); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(spanstore.Writer) - } - } - - if rf, ok := ret.Get(1).(func() error); ok { - r1 = rf() - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// NewArchiveFactory creates a new instance of ArchiveFactory. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -// The first argument is typically a *testing.T value. -func NewArchiveFactory(t interface { - mock.TestingT - Cleanup(func()) -}) *ArchiveFactory { - mock := &ArchiveFactory{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock -} diff --git a/storage/mocks/Inheritable.go b/storage/mocks/Inheritable.go new file mode 100644 index 00000000000..150bf34ab1e --- /dev/null +++ b/storage/mocks/Inheritable.go @@ -0,0 +1,37 @@ +// Copyright (c) The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 +// +// Run 'make generate-mocks' to regenerate. + +// Code generated by mockery. DO NOT EDIT. + +package mocks + +import ( + storage "github.com/jaegertracing/jaeger/storage" + mock "github.com/stretchr/testify/mock" +) + +// Inheritable is an autogenerated mock type for the Inheritable type +type Inheritable struct { + mock.Mock +} + +// InheritSettingsFrom provides a mock function with given fields: other +func (_m *Inheritable) InheritSettingsFrom(other storage.Factory) { + _m.Called(other) +} + +// NewInheritable creates a new instance of Inheritable. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewInheritable(t interface { + mock.TestingT + Cleanup(func()) +}) *Inheritable { + mock := &Inheritable{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/storage_v2/v1adapter/archive.go b/storage_v2/v1adapter/archive.go deleted file mode 100644 index bdf31fbf6bc..00000000000 --- a/storage_v2/v1adapter/archive.go +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright (c) 2025 The Jaeger Authors. -// SPDX-License-Identifier: Apache-2.0 - -package v1adapter - -import ( - "errors" - - "go.uber.org/zap" - - "github.com/jaegertracing/jaeger/storage" -) - -func InitializeArchiveStorage( - storageFactory storage.BaseFactory, - logger *zap.Logger, -) (*TraceReader, *TraceWriter) { - archiveFactory, ok := storageFactory.(storage.ArchiveFactory) - if !ok { - logger.Info("Archive storage not supported by the factory") - return nil, nil - } - reader, err := archiveFactory.CreateArchiveSpanReader() - if errors.Is(err, storage.ErrArchiveStorageNotConfigured) || errors.Is(err, storage.ErrArchiveStorageNotSupported) { - logger.Info("Archive storage not created", zap.String("reason", err.Error())) - return nil, nil - } - if err != nil { - logger.Error("Cannot init archive storage reader", zap.Error(err)) - return nil, nil - } - writer, err := archiveFactory.CreateArchiveSpanWriter() - if errors.Is(err, storage.ErrArchiveStorageNotConfigured) || errors.Is(err, storage.ErrArchiveStorageNotSupported) { - logger.Info("Archive storage not created", zap.String("reason", err.Error())) - return nil, nil - } - if err != nil { - logger.Error("Cannot init archive storage writer", zap.Error(err)) - return nil, nil - } - - return NewTraceReader(reader), NewTraceWriter(writer) -} diff --git a/storage_v2/v1adapter/archive_test.go b/storage_v2/v1adapter/archive_test.go deleted file mode 100644 index 1dc1c854851..00000000000 --- a/storage_v2/v1adapter/archive_test.go +++ /dev/null @@ -1,112 +0,0 @@ -// Copyright (c) 2025 The Jaeger Authors. -// SPDX-License-Identifier: Apache-2.0 - -package v1adapter - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "go.uber.org/zap" - - "github.com/jaegertracing/jaeger/pkg/metrics" - "github.com/jaegertracing/jaeger/storage" - "github.com/jaegertracing/jaeger/storage/dependencystore" - "github.com/jaegertracing/jaeger/storage/spanstore" - spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks" -) - -type fakeStorageFactory1 struct{} - -type fakeStorageFactory2 struct { - fakeStorageFactory1 - r spanstore.Reader - w spanstore.Writer - rErr error - wErr error -} - -func (*fakeStorageFactory1) Initialize(metrics.Factory, *zap.Logger) error { - return nil -} -func (*fakeStorageFactory1) CreateSpanReader() (spanstore.Reader, error) { return nil, nil } -func (*fakeStorageFactory1) CreateSpanWriter() (spanstore.Writer, error) { return nil, nil } -func (*fakeStorageFactory1) CreateDependencyReader() (dependencystore.Reader, error) { return nil, nil } - -func (f *fakeStorageFactory2) CreateArchiveSpanReader() (spanstore.Reader, error) { return f.r, f.rErr } -func (f *fakeStorageFactory2) CreateArchiveSpanWriter() (spanstore.Writer, error) { return f.w, f.wErr } - -var ( - _ storage.Factory = new(fakeStorageFactory1) - _ storage.ArchiveFactory = new(fakeStorageFactory2) -) - -func TestInitializeArchiveStorage(t *testing.T) { - tests := []struct { - name string - factory storage.Factory - wantReader spanstore.Reader - wantWriter spanstore.Writer - }{ - { - name: "Archive storage not supported by the factory", - factory: new(fakeStorageFactory1), - wantReader: nil, - wantWriter: nil, - }, - { - name: "Archive storage not configured", - factory: &fakeStorageFactory2{rErr: storage.ErrArchiveStorageNotConfigured}, - wantReader: nil, - wantWriter: nil, - }, - { - name: "Archive storage not supported for reader", - factory: &fakeStorageFactory2{rErr: storage.ErrArchiveStorageNotSupported}, - wantReader: nil, - wantWriter: nil, - }, - { - name: "Error initializing archive span reader", - factory: &fakeStorageFactory2{rErr: assert.AnError}, - wantReader: nil, - wantWriter: nil, - }, - { - name: "Archive storage not supported for writer", - factory: &fakeStorageFactory2{wErr: storage.ErrArchiveStorageNotSupported}, - wantReader: nil, - wantWriter: nil, - }, - { - name: "Error initializing archive span writer", - factory: &fakeStorageFactory2{wErr: assert.AnError}, - wantReader: nil, - wantWriter: nil, - }, - { - name: "Successfully initialize archive storage", - factory: &fakeStorageFactory2{r: &spanstoremocks.Reader{}, w: &spanstoremocks.Writer{}}, - wantReader: &spanstoremocks.Reader{}, - wantWriter: &spanstoremocks.Writer{}, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - logger := zap.NewNop() - reader, writer := InitializeArchiveStorage(test.factory, logger) - if test.wantReader != nil { - require.Equal(t, test.wantReader, reader.spanReader) - } else { - require.Nil(t, reader) - } - if test.wantWriter != nil { - require.Equal(t, test.wantWriter, writer.spanWriter) - } else { - require.Nil(t, writer) - } - }) - } -}