[chore] Return nil from storage factories when there is an error in c…

…reating span reader (#6293)

## Which problem is this PR solving?
- Towards #6219 

## Description of the changes
- This PR makes a minor fix to return nil in the cassandra and es
storage factories when there is an error creating the storage factory
before decorating it

## How was this change tested?
- CI

## 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`: `yarn lint` and `yarn test`

---------

Signed-off-by: Mahad Zaryab <[email protected]>

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
 }

plugin/storage/cassandra/factory_test.go

@@ -106,6 +106,28 @@ 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.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)

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
 }

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) {