From abab75cc3aed5582bc4841356b525a7065d95925 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Mon, 2 Dec 2024 11:13:43 -0500 Subject: [PATCH 1/4] Return Nil When Error In Creating Span Reader Signed-off-by: Mahad Zaryab --- plugin/storage/cassandra/factory.go | 4 ++-- plugin/storage/es/factory.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/plugin/storage/cassandra/factory.go b/plugin/storage/cassandra/factory.go index 1d91b8347dd..ebfac8e6f82 100644 --- a/plugin/storage/cassandra/factory.go +++ b/plugin/storage/cassandra/factory.go @@ -221,7 +221,7 @@ func NewSession(c *config.Configuration) (cassandra.Session, error) { func (f *Factory) CreateSpanReader() (spanstore.Reader, error) { sr, err := cSpanStore.NewSpanReader(f.primarySession, f.primaryMetricsFactory, f.logger, f.tracer.Tracer("cSpanStore.SpanReader")) if err != nil { - return sr, err + return nil, err } return spanstoremetrics.NewReaderDecorator(sr, f.primaryMetricsFactory), nil } @@ -248,7 +248,7 @@ func (f *Factory) CreateArchiveSpanReader() (spanstore.Reader, error) { } sr, err := cSpanStore.NewSpanReader(f.archiveSession, f.archiveMetricsFactory, f.logger, f.tracer.Tracer("cSpanStore.SpanReader")) if err != nil { - return sr, err + return nil, err } return spanstoremetrics.NewReaderDecorator(sr, f.archiveMetricsFactory), nil } diff --git a/plugin/storage/es/factory.go b/plugin/storage/es/factory.go index fff8b7e571f..ee00688535e 100644 --- a/plugin/storage/es/factory.go +++ b/plugin/storage/es/factory.go @@ -198,7 +198,7 @@ func (f *Factory) getArchiveClient() es.Client { func (f *Factory) CreateSpanReader() (spanstore.Reader, error) { sr, err := createSpanReader(f.getPrimaryClient, f.primaryConfig, false, f.logger, f.tracer) if err != nil { - return sr, err + return nil, err } return spanstoremetrics.NewReaderDecorator(sr, f.primaryMetricsFactory), nil } @@ -220,7 +220,7 @@ func (f *Factory) CreateArchiveSpanReader() (spanstore.Reader, error) { } sr, err := createSpanReader(f.getArchiveClient, f.archiveConfig, true, f.logger, f.tracer) if err != nil { - return sr, err + return nil, err } return spanstoremetrics.NewReaderDecorator(sr, f.archiveMetricsFactory), nil } From 3863438b487f300b3180699c89e973d150ac15a3 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Mon, 2 Dec 2024 11:29:58 -0500 Subject: [PATCH 2/4] Add Test For ES Archive Span Reader Signed-off-by: Mahad Zaryab --- plugin/storage/es/factory_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/plugin/storage/es/factory_test.go b/plugin/storage/es/factory_test.go index 4da9a6c6f45..7a3412f8e56 100644 --- a/plugin/storage/es/factory_test.go +++ b/plugin/storage/es/factory_test.go @@ -123,7 +123,10 @@ func TestElasticsearchILMUsedWithoutReadWriteAliases(t *testing.T) { f.primaryConfig = &escfg.Configuration{ UseILM: true, } - f.archiveConfig = &escfg.Configuration{} + f.archiveConfig = &escfg.Configuration{ + Enabled: true, + UseILM: true, + } f.newClientFn = (&mockClientBuilder{}).NewClient require.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) defer f.Close() @@ -134,6 +137,10 @@ 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) { From 0f2fb1b49a526c2c304a2ea8611471d5423eb164 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Mon, 2 Dec 2024 12:17:31 -0500 Subject: [PATCH 3/4] Add Missing Tests For Cassandra Signed-off-by: Mahad Zaryab --- plugin/storage/cassandra/factory_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/plugin/storage/cassandra/factory_test.go b/plugin/storage/cassandra/factory_test.go index d8800089a75..71add9cdb93 100644 --- a/plugin/storage/cassandra/factory_test.go +++ b/plugin/storage/cassandra/factory_test.go @@ -106,6 +106,27 @@ func TestCassandraFactory(t *testing.T) { require.NoError(t, f.Close()) } +func TestCreateSpanReaderError(t *testing.T) { + session := &mocks.Session{} + query := &mocks.Query{} + session.On("Query", + mock.AnythingOfType("string"), + mock.Anything).Return(query) + session.On("Query", + mock.AnythingOfType("string"), + mock.Anything).Return(query) + query.On("Exec").Return(errors.New("table does not exist")) + f := NewFactory() + f.sessionBuilderFn = new(mockSessionBuilder).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) From f8afd00f2826a4140d3be3b22e27aa1613e0a2ce Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Mon, 2 Dec 2024 12:46:00 -0500 Subject: [PATCH 4/4] Fix Test Signed-off-by: Mahad Zaryab --- plugin/storage/cassandra/factory_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugin/storage/cassandra/factory_test.go b/plugin/storage/cassandra/factory_test.go index 71add9cdb93..20f7ca54d5f 100644 --- a/plugin/storage/cassandra/factory_test.go +++ b/plugin/storage/cassandra/factory_test.go @@ -117,7 +117,8 @@ func TestCreateSpanReaderError(t *testing.T) { mock.Anything).Return(query) query.On("Exec").Return(errors.New("table does not exist")) f := NewFactory() - f.sessionBuilderFn = new(mockSessionBuilder).add(session, nil).build + 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)