Propagate Errors

Signed-off-by: Mahad Zaryab <[email protected]>
jaegertracing · Jan 31, 2025 · ecc1036 · ecc1036
1 parent b19e0dd
commit ecc1036
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 45 deletions.
diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go
@@ -27,8 +27,8 @@ import (
 	"github.com/jaegertracing/jaeger/pkg/config"
 	"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
 	"github.com/jaegertracing/jaeger/pkg/tenancy"
+	"github.com/jaegertracing/jaeger/plugin/storage"
 	"github.com/jaegertracing/jaeger/ports"
-	"github.com/jaegertracing/jaeger/storage/spanstore"
 	"github.com/jaegertracing/jaeger/storage_v2/v1adapter"
 )
 
@@ -140,7 +140,7 @@ func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*Q
 	return qOpts, nil
 }
 
-type InitArchiveStorageFn func() (spanstore.Reader, spanstore.Writer)
+type InitArchiveStorageFn func() (*storage.ArchiveStorage, error)
 
 // BuildQueryServiceOptions creates a QueryServiceOptions struct with appropriate adjusters and archive config
 func (qOpts *QueryOptions) BuildQueryServiceOptions(
@@ -153,12 +153,17 @@ func (qOpts *QueryOptions) BuildQueryServiceOptions(
 	v2Opts := &v2querysvc.QueryServiceOptions{
 		MaxClockSkewAdjust: qOpts.MaxClockSkewAdjust,
 	}
-	ar, aw := initArchiveStorageFn()
-	if ar != nil && aw != nil {
-		opts.ArchiveSpanReader = ar
-		opts.ArchiveSpanWriter = aw
-		v2Opts.ArchiveTraceReader = v1adapter.NewTraceReader(ar)
-		v2Opts.ArchiveTraceWriter = v1adapter.NewTraceWriter(aw)
+	as, err := initArchiveStorageFn()
+	if err != nil {
+		logger.Error("Received an error when trying to initialize archive storage", zap.Error(err))
+		return opts, v2Opts
+	}
+
+	if as != nil && as.Reader != nil && as.Writer != nil {
+		opts.ArchiveSpanReader = as.Reader
+		opts.ArchiveSpanWriter = as.Writer
+		v2Opts.ArchiveTraceReader = v1adapter.NewTraceReader(as.Reader)
+		v2Opts.ArchiveTraceWriter = v1adapter.NewTraceWriter(as.Writer)
 	} else {
 		logger.Info("Archive storage not initialized")
 	}

diff --git a/cmd/query/app/flags_test.go b/cmd/query/app/flags_test.go
@@ -16,8 +16,8 @@ import (
 
 	"github.com/jaegertracing/jaeger/pkg/config"
 	"github.com/jaegertracing/jaeger/pkg/testutils"
+	"github.com/jaegertracing/jaeger/plugin/storage"
 	"github.com/jaegertracing/jaeger/ports"
-	"github.com/jaegertracing/jaeger/storage/spanstore"
 	spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks"
 )
 
@@ -86,11 +86,14 @@ func TestStringSliceAsHeader(t *testing.T) {
 	require.NoError(t, err)
 }
 
-func initializedFn() (spanstore.Reader, spanstore.Writer) {
-	return &spanstoremocks.Reader{}, &spanstoremocks.Writer{}
+func initializedFn() (*storage.ArchiveStorage, error) {
+	return &storage.ArchiveStorage{
+		Reader: &spanstoremocks.Reader{},
+		Writer: &spanstoremocks.Writer{},
+	}, nil
 }
 
-func uninitializedFn() (spanstore.Reader, spanstore.Writer) {
+func uninitializedFn() (*storage.ArchiveStorage, error) {
 	return nil, nil
 }
 

diff --git a/plugin/storage/factory.go b/plugin/storage/factory.go
@@ -338,33 +338,34 @@ func (f *Factory) initDownsamplingFromViper(v *viper.Viper) {
 	f.FactoryConfig.DownsamplingHashSalt = v.GetString(downsamplingHashSalt)
 }
 
-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)
-	}
-	return factory.CreateSpanReader()
+type ArchiveStorage struct {
+	Reader spanstore.Reader
+	Writer spanstore.Writer
 }
 
-func (f *Factory) createArchiveSpanWriter() (spanstore.Writer, error) {
-	factory, ok := f.archiveFactories[f.SpanWriterTypes[0]]
+func (f *Factory) InitArchiveStorage() (*ArchiveStorage, error) {
+	factory, ok := f.archiveFactories[f.SpanReaderType]
 	if !ok {
-		return nil, fmt.Errorf("no %s backend registered for span store", f.SpanWriterTypes[0])
+		return nil, nil
 	}
-	return factory.CreateSpanWriter()
-}
-
-func (f *Factory) InitArchiveStorage() (spanstore.Reader, spanstore.Writer) {
-	reader, err := f.createArchiveSpanReader()
+	reader, err := factory.CreateSpanReader()
 	if err != nil {
+		return nil, err
+	}
+
+	factory, ok = f.archiveFactories[f.SpanWriterTypes[0]]
+	if !ok {
 		return nil, nil
 	}
-	writer, err := f.createArchiveSpanWriter()
+	writer, err := factory.CreateSpanWriter()
 	if err != nil {
-		return nil, nil
+		return nil, err
 	}
 
-	return reader, writer
+	return &ArchiveStorage{
+		Reader: reader,
+		Writer: writer,
+	}, nil
 }
 
 var _ io.Closer = (*Factory)(nil)

diff --git a/plugin/storage/factory_test.go b/plugin/storage/factory_test.go
@@ -463,10 +463,11 @@ func TestPublishOpts(t *testing.T) {
 
 func TestInitArchiveStorage(t *testing.T) {
 	tests := []struct {
-		name           string
-		setupMock      func(*mocks.Factory)
-		expectedReader spanstore.Reader
-		expectedWriter spanstore.Writer
+		name            string
+		setupMock       func(*mocks.Factory)
+		factoryCfg      func() FactoryConfig
+		expectedStorage *ArchiveStorage
+		expectedError   error
 	}{
 		{
 			name: "successful initialization",
@@ -476,16 +477,54 @@ func TestInitArchiveStorage(t *testing.T) {
 				mock.On("CreateSpanReader").Return(spanReader, nil)
 				mock.On("CreateSpanWriter").Return(spanWriter, nil)
 			},
-			expectedReader: &spanStoreMocks.Reader{},
-			expectedWriter: &spanStoreMocks.Writer{},
+			factoryCfg: func() FactoryConfig {
+				return defaultCfg()
+			},
+			expectedStorage: &ArchiveStorage{
+				Reader: &spanStoreMocks.Reader{},
+				Writer: &spanStoreMocks.Writer{},
+			},
+		},
+		{
+			name: "no archive span reader",
+			setupMock: func(mock *mocks.Factory) {
+				spanReader := &spanStoreMocks.Reader{}
+				spanWriter := &spanStoreMocks.Writer{}
+				mock.On("CreateSpanReader").Return(spanReader, nil)
+				mock.On("CreateSpanWriter").Return(spanWriter, nil)
+			},
+			factoryCfg: func() FactoryConfig {
+				cfg := defaultCfg()
+				cfg.SpanReaderType = "blackhole"
+				return cfg
+			},
+			expectedStorage: nil,
+		},
+		{
+			name: "no archive span writer",
+			setupMock: func(mock *mocks.Factory) {
+				spanReader := &spanStoreMocks.Reader{}
+				spanWriter := &spanStoreMocks.Writer{}
+				mock.On("CreateSpanReader").Return(spanReader, nil)
+				mock.On("CreateSpanWriter").Return(spanWriter, nil)
+			},
+			factoryCfg: func() FactoryConfig {
+				cfg := defaultCfg()
+				cfg.SpanWriterTypes = []string{"blackhole"}
+				return cfg
+			},
+			expectedStorage: nil,
 		},
 		{
 			name: "error initializing reader",
 			setupMock: func(mock *mocks.Factory) {
 				mock.On("CreateSpanReader").Return(nil, assert.AnError)
 			},
-			expectedReader: nil,
-			expectedWriter: nil,
+			factoryCfg: func() FactoryConfig {
+				return defaultCfg()
+			},
+			expectedStorage: nil,
+			expectedError:   assert.AnError,
 		},
 		{
 			name: "error initializing writer",
@@ -494,25 +533,27 @@ func TestInitArchiveStorage(t *testing.T) {
 				mock.On("CreateSpanReader").Return(spanReader, nil)
 				mock.On("CreateSpanWriter").Return(nil, assert.AnError)
 			},
-			expectedReader: nil,
-			expectedWriter: nil,
+			factoryCfg: func() FactoryConfig {
+				return defaultCfg()
+			},
+			expectedStorage: nil,
+			expectedError:   assert.AnError,
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			f, err := NewFactory(defaultCfg())
+			cfg := test.factoryCfg()
+			f, err := NewFactory(cfg)
 			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()
-			assert.Equal(t, test.expectedReader, reader)
-			assert.Equal(t, test.expectedWriter, writer)
+			storage, err := f.InitArchiveStorage()
+			require.Equal(t, test.expectedStorage, storage)
+			require.ErrorIs(t, err, test.expectedError)
 		})
 	}
 }