From 283552d252f786d9dcf22b2b81b2edd50b9cd871 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Thu, 9 Jan 2025 21:00:43 -0500 Subject: [PATCH 1/8] Fix Tests Signed-off-by: Mahad Zaryab --- .../internal/extension/jaegerquery/server.go | 26 +++++++++++-------- .../extension/jaegerquery/server_test.go | 22 ++-------------- 2 files changed, 17 insertions(+), 31 deletions(-) diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go index 72880fc3fd1..c380d5e6642 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server.go @@ -11,6 +11,7 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/extension" "go.opentelemetry.io/collector/extension/extensioncapabilities" + "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage" queryApp "github.com/jaegertracing/jaeger/cmd/query/app" @@ -23,7 +24,6 @@ import ( "github.com/jaegertracing/jaeger/plugin/metricstore/disabled" "github.com/jaegertracing/jaeger/storage/metricstore" "github.com/jaegertracing/jaeger/storage_v2/depstore" - "github.com/jaegertracing/jaeger/storage_v2/v1adapter" ) var ( @@ -94,8 +94,7 @@ func (s *server) Start(ctx context.Context, host component.Host) error { var opts querysvc.QueryServiceOptions var v2opts v2querysvc.QueryServiceOptions - // TODO archive storage still uses v1 factory - if err := s.addArchiveStorage(&opts, &v2opts, host); err != nil { + if err := s.addArchiveStorage(&v2opts, host); err != nil { return err } qs := querysvc.NewQueryService(traceReader, depReader, opts) @@ -131,7 +130,6 @@ func (s *server) Start(ctx context.Context, host component.Host) error { } func (s *server) addArchiveStorage( - opts *querysvc.QueryServiceOptions, v2opts *v2querysvc.QueryServiceOptions, host component.Host, ) error { @@ -140,19 +138,25 @@ func (s *server) addArchiveStorage( return nil } - f, err := jaegerstorage.GetStorageFactory(s.config.Storage.TracesArchive, host) + f, err := jaegerstorage.GetTraceStoreFactory(s.config.Storage.TracesArchive, host) if err != nil { return fmt.Errorf("cannot find archive storage factory: %w", err) } - if !opts.InitArchiveStorage(f, s.telset.Logger) { - s.telset.Logger.Info("Archive storage not initialized") + reader, err := f.CreateTraceReader() + if err != nil { + s.telset.Logger.Error("Cannot init archive storage reader", zap.Error(err)) + return nil + } + writer, err := f.CreateTraceWriter() + if err != nil { + s.telset.Logger.Error("Cannot init archive storage writer", zap.Error(err)) + return nil } - ar, aw := v1adapter.InitializeArchiveStorage(f, s.telset.Logger) - if ar != nil && aw != nil { - v2opts.ArchiveTraceReader = ar - v2opts.ArchiveTraceWriter = aw + if reader != nil && writer != nil { + v2opts.ArchiveTraceReader = reader + v2opts.ArchiveTraceWriter = writer } else { s.telset.Logger.Info("Archive storage not initialized") } diff --git a/cmd/jaeger/internal/extension/jaegerquery/server_test.go b/cmd/jaeger/internal/extension/jaegerquery/server_test.go index 3f5dc3957a8..0009367acd7 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server_test.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server_test.go @@ -23,7 +23,6 @@ import ( "go.uber.org/zap/zaptest" "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage" - "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" v2querysvc "github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc" "github.com/jaegertracing/jaeger/internal/grpctest" "github.com/jaegertracing/jaeger/pkg/metrics" @@ -271,7 +270,6 @@ func TestServerAddArchiveStorage(t *testing.T) { tests := []struct { name string - qSvcOpts *querysvc.QueryServiceOptions v2qSvcOpts *v2querysvc.QueryServiceOptions config *Config extension component.Component @@ -281,7 +279,6 @@ func TestServerAddArchiveStorage(t *testing.T) { { name: "Archive storage unset", config: &Config{}, - qSvcOpts: &querysvc.QueryServiceOptions{}, v2qSvcOpts: &v2querysvc.QueryServiceOptions{}, expectedOutput: `{"level":"info","msg":"Archive storage not configured"}` + "\n", expectedErr: "", @@ -293,32 +290,17 @@ func TestServerAddArchiveStorage(t *testing.T) { TracesArchive: "random-value", }, }, - qSvcOpts: &querysvc.QueryServiceOptions{}, v2qSvcOpts: &v2querysvc.QueryServiceOptions{}, expectedOutput: "", expectedErr: "cannot find archive storage factory: cannot find extension", }, { - name: "Archive storage not supported", - config: &Config{ - Storage: Storage{ - TracesArchive: "no-archive", - }, - }, - qSvcOpts: &querysvc.QueryServiceOptions{}, - v2qSvcOpts: &v2querysvc.QueryServiceOptions{}, - extension: fakeStorageExt{}, - expectedOutput: "Archive storage not supported by the factory", - expectedErr: "", - }, - { - name: "Archive storage supported", + name: "Archive storage exists", config: &Config{ Storage: Storage{ TracesArchive: "some-archive-storage", }, }, - qSvcOpts: &querysvc.QueryServiceOptions{}, v2qSvcOpts: &v2querysvc.QueryServiceOptions{}, extension: fakeStorageExt{}, expectedOutput: "", @@ -338,7 +320,7 @@ func TestServerAddArchiveStorage(t *testing.T) { if tt.extension != nil { host = storagetest.NewStorageHost().WithExtension(jaegerstorage.ID, tt.extension) } - err := server.addArchiveStorage(tt.qSvcOpts, tt.v2qSvcOpts, host) + err := server.addArchiveStorage(tt.v2qSvcOpts, host) if tt.expectedErr == "" { require.NoError(t, err) } else { From 399bfcb7b79f9b4d35f9df559af825018d036b85 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sat, 11 Jan 2025 12:15:27 -0500 Subject: [PATCH 2/8] Add Unit Tests Signed-off-by: Mahad Zaryab --- .../extension/jaegerquery/server_test.go | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/cmd/jaeger/internal/extension/jaegerquery/server_test.go b/cmd/jaeger/internal/extension/jaegerquery/server_test.go index 0009367acd7..e42ae5a5c07 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server_test.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server_test.go @@ -294,6 +294,28 @@ func TestServerAddArchiveStorage(t *testing.T) { expectedOutput: "", expectedErr: "cannot find archive storage factory: cannot find extension", }, + { + name: "Archive storage span reader error", + config: &Config{ + Storage: Storage{ + TracesArchive: "need-span-reader-error", + }, + }, + v2qSvcOpts: &v2querysvc.QueryServiceOptions{}, + extension: fakeStorageExt{}, + expectedOutput: "Cannot init archive storage reader", + }, + { + name: "Archive storage span writer error", + config: &Config{ + Storage: Storage{ + TracesArchive: "need-span-writer-error", + }, + }, + v2qSvcOpts: &v2querysvc.QueryServiceOptions{}, + extension: fakeStorageExt{}, + expectedOutput: "Cannot init archive storage writer", + }, { name: "Archive storage exists", config: &Config{ From 4090ece2cf167287f2f85af0190feead65f97334 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Wed, 15 Jan 2025 20:43:03 -0500 Subject: [PATCH 3/8] Revert Initialization of Archive For v1 Signed-off-by: Mahad Zaryab --- .../internal/extension/jaegerquery/server.go | 31 +++++++++++-------- .../extension/jaegerquery/server_test.go | 28 +++++++---------- 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go index c380d5e6642..2e4578f6715 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server.go @@ -94,7 +94,7 @@ func (s *server) Start(ctx context.Context, host component.Host) error { var opts querysvc.QueryServiceOptions var v2opts v2querysvc.QueryServiceOptions - if err := s.addArchiveStorage(&v2opts, host); err != nil { + if err := s.addArchiveStorage(&opts, &v2opts, host); err != nil { return err } qs := querysvc.NewQueryService(traceReader, depReader, opts) @@ -130,6 +130,7 @@ func (s *server) Start(ctx context.Context, host component.Host) error { } func (s *server) addArchiveStorage( + opts *querysvc.QueryServiceOptions, v2opts *v2querysvc.QueryServiceOptions, host component.Host, ) error { @@ -138,28 +139,32 @@ func (s *server) addArchiveStorage( return nil } - f, err := jaegerstorage.GetTraceStoreFactory(s.config.Storage.TracesArchive, host) + v1Factory, err := jaegerstorage.GetStorageFactory(s.config.Storage.TracesArchive, host) if err != nil { return fmt.Errorf("cannot find archive storage factory: %w", err) } - reader, err := f.CreateTraceReader() + if !opts.InitArchiveStorage(v1Factory, s.telset.Logger) { + s.telset.Logger.Info("Archive storage not initialized for v1 query service") + } + + v2Factory, err := jaegerstorage.GetTraceStoreFactory(s.config.Storage.TracesArchive, host) if err != nil { - s.telset.Logger.Error("Cannot init archive storage reader", zap.Error(err)) - return nil + return fmt.Errorf("cannot find traces archive storage factory: %w", err) } - writer, err := f.CreateTraceWriter() + + reader, err := v2Factory.CreateTraceReader() if err != nil { - s.telset.Logger.Error("Cannot init archive storage writer", zap.Error(err)) + s.telset.Logger.Error("Cannot init traces archive storage reader", zap.Error(err)) return nil } - - if reader != nil && writer != nil { - v2opts.ArchiveTraceReader = reader - v2opts.ArchiveTraceWriter = writer - } else { - s.telset.Logger.Info("Archive storage not initialized") + writer, err := v2Factory.CreateTraceWriter() + if err != nil { + s.telset.Logger.Error("Cannot init traces archive storage writer", zap.Error(err)) + return nil } + v2opts.ArchiveTraceReader = reader + v2opts.ArchiveTraceWriter = writer return nil } diff --git a/cmd/jaeger/internal/extension/jaegerquery/server_test.go b/cmd/jaeger/internal/extension/jaegerquery/server_test.go index e42ae5a5c07..3f5dc3957a8 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server_test.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server_test.go @@ -23,6 +23,7 @@ import ( "go.uber.org/zap/zaptest" "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage" + "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" v2querysvc "github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc" "github.com/jaegertracing/jaeger/internal/grpctest" "github.com/jaegertracing/jaeger/pkg/metrics" @@ -270,6 +271,7 @@ func TestServerAddArchiveStorage(t *testing.T) { tests := []struct { name string + qSvcOpts *querysvc.QueryServiceOptions v2qSvcOpts *v2querysvc.QueryServiceOptions config *Config extension component.Component @@ -279,6 +281,7 @@ func TestServerAddArchiveStorage(t *testing.T) { { name: "Archive storage unset", config: &Config{}, + qSvcOpts: &querysvc.QueryServiceOptions{}, v2qSvcOpts: &v2querysvc.QueryServiceOptions{}, expectedOutput: `{"level":"info","msg":"Archive storage not configured"}` + "\n", expectedErr: "", @@ -290,39 +293,32 @@ func TestServerAddArchiveStorage(t *testing.T) { TracesArchive: "random-value", }, }, + qSvcOpts: &querysvc.QueryServiceOptions{}, v2qSvcOpts: &v2querysvc.QueryServiceOptions{}, expectedOutput: "", expectedErr: "cannot find archive storage factory: cannot find extension", }, { - name: "Archive storage span reader error", + name: "Archive storage not supported", config: &Config{ Storage: Storage{ - TracesArchive: "need-span-reader-error", + TracesArchive: "no-archive", }, }, + qSvcOpts: &querysvc.QueryServiceOptions{}, v2qSvcOpts: &v2querysvc.QueryServiceOptions{}, extension: fakeStorageExt{}, - expectedOutput: "Cannot init archive storage reader", - }, - { - name: "Archive storage span writer error", - config: &Config{ - Storage: Storage{ - TracesArchive: "need-span-writer-error", - }, - }, - v2qSvcOpts: &v2querysvc.QueryServiceOptions{}, - extension: fakeStorageExt{}, - expectedOutput: "Cannot init archive storage writer", + expectedOutput: "Archive storage not supported by the factory", + expectedErr: "", }, { - name: "Archive storage exists", + name: "Archive storage supported", config: &Config{ Storage: Storage{ TracesArchive: "some-archive-storage", }, }, + qSvcOpts: &querysvc.QueryServiceOptions{}, v2qSvcOpts: &v2querysvc.QueryServiceOptions{}, extension: fakeStorageExt{}, expectedOutput: "", @@ -342,7 +338,7 @@ func TestServerAddArchiveStorage(t *testing.T) { if tt.extension != nil { host = storagetest.NewStorageHost().WithExtension(jaegerstorage.ID, tt.extension) } - err := server.addArchiveStorage(tt.v2qSvcOpts, host) + err := server.addArchiveStorage(tt.qSvcOpts, tt.v2qSvcOpts, host) if tt.expectedErr == "" { require.NoError(t, err) } else { From 3904944e5a746a3ac4833563b09a794bf8b9b5d8 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Wed, 15 Jan 2025 21:13:24 -0500 Subject: [PATCH 4/8] Stop Replying on InitArchiveStorage Signed-off-by: Mahad Zaryab --- .../internal/extension/jaegerquery/server.go | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go index 2e4578f6715..7d3464cc57c 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server.go @@ -24,6 +24,7 @@ import ( "github.com/jaegertracing/jaeger/plugin/metricstore/disabled" "github.com/jaegertracing/jaeger/storage/metricstore" "github.com/jaegertracing/jaeger/storage_v2/depstore" + "github.com/jaegertracing/jaeger/storage_v2/v1adapter" ) var ( @@ -139,26 +140,26 @@ func (s *server) addArchiveStorage( return nil } - v1Factory, err := jaegerstorage.GetStorageFactory(s.config.Storage.TracesArchive, host) - if err != nil { - return fmt.Errorf("cannot find archive storage factory: %w", err) - } + // v1Factory, err := jaegerstorage.GetStorageFactory(s.config.Storage.TracesArchive, host) + // if err != nil { + // return fmt.Errorf("cannot find archive storage factory: %w", err) + // } - if !opts.InitArchiveStorage(v1Factory, s.telset.Logger) { - s.telset.Logger.Info("Archive storage not initialized for v1 query service") - } + // if !opts.InitArchiveStorage(v1Factory, s.telset.Logger) { + // s.telset.Logger.Info("Archive storage not initialized for v1 query service") + // } - v2Factory, err := jaegerstorage.GetTraceStoreFactory(s.config.Storage.TracesArchive, host) + f, err := jaegerstorage.GetTraceStoreFactory(s.config.Storage.TracesArchive, host) if err != nil { return fmt.Errorf("cannot find traces archive storage factory: %w", err) } - reader, err := v2Factory.CreateTraceReader() + reader, err := f.CreateTraceReader() if err != nil { s.telset.Logger.Error("Cannot init traces archive storage reader", zap.Error(err)) return nil } - writer, err := v2Factory.CreateTraceWriter() + writer, err := f.CreateTraceWriter() if err != nil { s.telset.Logger.Error("Cannot init traces archive storage writer", zap.Error(err)) return nil @@ -166,6 +167,12 @@ func (s *server) addArchiveStorage( v2opts.ArchiveTraceReader = reader v2opts.ArchiveTraceWriter = writer + // TODO: handle error + v1Reader, _ := v1adapter.GetV1Reader(reader) + v1Writer, _ := v1adapter.GetV1Writer(writer) + opts.ArchiveSpanReader = v1Reader + opts.ArchiveSpanWriter = v1Writer + return nil } From d18a211650f08e9ad0b68cdb035f9394a2b9c986 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Wed, 15 Jan 2025 22:11:10 -0500 Subject: [PATCH 5/8] Add Downgrades Signed-off-by: Mahad Zaryab --- .../internal/extension/jaegerquery/server.go | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go index 7d3464cc57c..509ea2f908c 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server.go @@ -140,15 +140,6 @@ func (s *server) addArchiveStorage( return nil } - // v1Factory, err := jaegerstorage.GetStorageFactory(s.config.Storage.TracesArchive, host) - // if err != nil { - // return fmt.Errorf("cannot find archive storage factory: %w", err) - // } - - // if !opts.InitArchiveStorage(v1Factory, s.telset.Logger) { - // s.telset.Logger.Info("Archive storage not initialized for v1 query service") - // } - f, err := jaegerstorage.GetTraceStoreFactory(s.config.Storage.TracesArchive, host) if err != nil { return fmt.Errorf("cannot find traces archive storage factory: %w", err) @@ -167,9 +158,19 @@ func (s *server) addArchiveStorage( v2opts.ArchiveTraceReader = reader v2opts.ArchiveTraceWriter = writer - // TODO: handle error - v1Reader, _ := v1adapter.GetV1Reader(reader) - v1Writer, _ := v1adapter.GetV1Writer(writer) + v1Reader, err := v1adapter.GetV1Reader(reader) + if err != nil { + // if the spanstore.Reader is not available, downgrade the native tracestore.Reader to + // a spanstore.Reader + v1Reader = v1adapter.NewSpanReader(reader) + } + + v1Writer, err := v1adapter.GetV1Writer(writer) + if err != nil { + // TODO: implement an adapter to downgrade a tracestore.Writer to a spanstore.Writer + panic("downgrade tracestore.Writer to spanstore.Writer is not implemented") + } + opts.ArchiveSpanReader = v1Reader opts.ArchiveSpanWriter = v1Writer From 676e545f2d33aafa271b8fb1f0d3ba4dc30539ff Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Thu, 16 Jan 2025 19:53:32 -0500 Subject: [PATCH 6/8] Fix Unit Tests And Integrate Reverse Adapter Signed-off-by: Mahad Zaryab --- .../internal/extension/jaegerquery/server.go | 5 +-- .../extension/jaegerquery/server_test.go | 34 +++++++++---------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go index 509ea2f908c..2f97dca1a81 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server.go @@ -167,8 +167,9 @@ func (s *server) addArchiveStorage( v1Writer, err := v1adapter.GetV1Writer(writer) if err != nil { - // TODO: implement an adapter to downgrade a tracestore.Writer to a spanstore.Writer - panic("downgrade tracestore.Writer to spanstore.Writer is not implemented") + // if the spanstore.Writer is not available, downgrade the native tracestore.Writer to + // a spanstore.Writer + v1Writer = v1adapter.NewSpanWriter(writer) } opts.ArchiveSpanReader = v1Reader diff --git a/cmd/jaeger/internal/extension/jaegerquery/server_test.go b/cmd/jaeger/internal/extension/jaegerquery/server_test.go index 3f5dc3957a8..5a29f73ddff 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server_test.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server_test.go @@ -63,14 +63,6 @@ func (ff fakeFactory) CreateSpanWriter() (spanstore.Writer, error) { return &spanstoremocks.Writer{}, nil } -func (fakeFactory) CreateArchiveSpanReader() (spanstore.Reader, error) { - return &spanstoremocks.Reader{}, nil -} - -func (fakeFactory) CreateArchiveSpanWriter() (spanstore.Writer, error) { - return &spanstoremocks.Writer{}, nil -} - func (ff fakeFactory) Initialize(metrics.Factory, *zap.Logger) error { if ff.name == "need-initialize-error" { return errors.New("test-error") @@ -104,11 +96,6 @@ var _ jaegerstorage.Extension = (*fakeStorageExt)(nil) func (fakeStorageExt) TraceStorageFactory(name string) (storage.Factory, bool) { if name == "need-factory-error" { return nil, false - } else if name == "no-archive" { - f := fakeFactory{name: name} - return struct { - storage.Factory - }{f}, true } return fakeFactory{name: name}, true @@ -296,19 +283,32 @@ func TestServerAddArchiveStorage(t *testing.T) { qSvcOpts: &querysvc.QueryServiceOptions{}, v2qSvcOpts: &v2querysvc.QueryServiceOptions{}, expectedOutput: "", - expectedErr: "cannot find archive storage factory: cannot find extension", + expectedErr: "cannot find traces archive storage factory: cannot find extension", + }, + { + name: "Error in trace reader", + config: &Config{ + Storage: Storage{ + TracesArchive: "need-span-reader-error", + }, + }, + qSvcOpts: &querysvc.QueryServiceOptions{}, + v2qSvcOpts: &v2querysvc.QueryServiceOptions{}, + extension: fakeStorageExt{}, + expectedOutput: "Cannot init traces archive storage reader", + expectedErr: "", }, { - name: "Archive storage not supported", + name: "Error in trace writer", config: &Config{ Storage: Storage{ - TracesArchive: "no-archive", + TracesArchive: "need-span-writer-error", }, }, qSvcOpts: &querysvc.QueryServiceOptions{}, v2qSvcOpts: &v2querysvc.QueryServiceOptions{}, extension: fakeStorageExt{}, - expectedOutput: "Archive storage not supported by the factory", + expectedOutput: "Cannot init traces archive storage writer", expectedErr: "", }, { From f0a07aac38ebae0b8e30172c64c56ddaa970d6fb Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Thu, 16 Jan 2025 20:05:45 -0500 Subject: [PATCH 7/8] Fix Test Signed-off-by: Mahad Zaryab --- cmd/jaeger/internal/extension/jaegerquery/server_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/jaeger/internal/extension/jaegerquery/server_test.go b/cmd/jaeger/internal/extension/jaegerquery/server_test.go index 5a29f73ddff..4adbf07a5ca 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server_test.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server_test.go @@ -180,7 +180,7 @@ func TestServerStart(t *testing.T) { TracesPrimary: "jaeger_storage", }, }, - expectedErr: "cannot find archive storage factory", + expectedErr: "cannot find traces archive storage factory", }, { name: "metrics storage error", From b557b319a316baa8520f05f8dc2ac3d829720cf1 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Thu, 16 Jan 2025 22:18:50 -0500 Subject: [PATCH 8/8] Improve Test Coverage Signed-off-by: Mahad Zaryab --- .../internal/extension/jaegerquery/server.go | 40 +++++++++++++------ .../extension/jaegerquery/server_test.go | 36 +++++++++++++++++ 2 files changed, 64 insertions(+), 12 deletions(-) diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go index 2f97dca1a81..a92cb080b0c 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server.go @@ -23,7 +23,9 @@ import ( "github.com/jaegertracing/jaeger/pkg/tenancy" "github.com/jaegertracing/jaeger/plugin/metricstore/disabled" "github.com/jaegertracing/jaeger/storage/metricstore" + "github.com/jaegertracing/jaeger/storage/spanstore" "github.com/jaegertracing/jaeger/storage_v2/depstore" + "github.com/jaegertracing/jaeger/storage_v2/tracestore" "github.com/jaegertracing/jaeger/storage_v2/v1adapter" ) @@ -145,37 +147,51 @@ func (s *server) addArchiveStorage( return fmt.Errorf("cannot find traces archive storage factory: %w", err) } + traceReader, traceWriter := s.initArchiveStorage(f) + if traceReader == nil || traceWriter == nil { + return nil + } + + v2opts.ArchiveTraceReader = traceReader + v2opts.ArchiveTraceWriter = traceWriter + + spanReader, spanWriter := getV1Adapters(traceReader, traceWriter) + + opts.ArchiveSpanReader = spanReader + opts.ArchiveSpanWriter = spanWriter + + return nil +} + +func (s *server) initArchiveStorage(f tracestore.Factory) (tracestore.Reader, tracestore.Writer) { reader, err := f.CreateTraceReader() if err != nil { s.telset.Logger.Error("Cannot init traces archive storage reader", zap.Error(err)) - return nil + return nil, nil } writer, err := f.CreateTraceWriter() if err != nil { s.telset.Logger.Error("Cannot init traces archive storage writer", zap.Error(err)) - return nil + return nil, nil } - v2opts.ArchiveTraceReader = reader - v2opts.ArchiveTraceWriter = writer + return reader, writer +} +func getV1Adapters( + reader tracestore.Reader, + writer tracestore.Writer, +) (spanstore.Reader, spanstore.Writer) { v1Reader, err := v1adapter.GetV1Reader(reader) if err != nil { - // if the spanstore.Reader is not available, downgrade the native tracestore.Reader to - // a spanstore.Reader v1Reader = v1adapter.NewSpanReader(reader) } v1Writer, err := v1adapter.GetV1Writer(writer) if err != nil { - // if the spanstore.Writer is not available, downgrade the native tracestore.Writer to - // a spanstore.Writer v1Writer = v1adapter.NewSpanWriter(writer) } - opts.ArchiveSpanReader = v1Reader - opts.ArchiveSpanWriter = v1Writer - - return nil + return v1Reader, v1Writer } func (s *server) createMetricReader(host component.Host) (metricstore.Reader, error) { diff --git a/cmd/jaeger/internal/extension/jaegerquery/server_test.go b/cmd/jaeger/internal/extension/jaegerquery/server_test.go index 4adbf07a5ca..1a5effd21e1 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server_test.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server_test.go @@ -36,6 +36,9 @@ import ( metricstoremocks "github.com/jaegertracing/jaeger/storage/metricstore/mocks" "github.com/jaegertracing/jaeger/storage/spanstore" spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks" + "github.com/jaegertracing/jaeger/storage_v2/tracestore" + tracestoremocks "github.com/jaegertracing/jaeger/storage_v2/tracestore/mocks" + "github.com/jaegertracing/jaeger/storage_v2/v1adapter" ) type fakeFactory struct { @@ -350,6 +353,39 @@ func TestServerAddArchiveStorage(t *testing.T) { } } +func TestGetV1Adapters(t *testing.T) { + tests := []struct { + name string + reader tracestore.Reader + writer tracestore.Writer + expectedReader spanstore.Reader + expectedWriter spanstore.Writer + }{ + { + name: "native tracestore.Reader and tracestore.Writer", + reader: &tracestoremocks.Reader{}, + writer: &tracestoremocks.Writer{}, + expectedReader: v1adapter.NewSpanReader(&tracestoremocks.Reader{}), + expectedWriter: v1adapter.NewSpanWriter(&tracestoremocks.Writer{}), + }, + { + name: "wrapped spanstore.Reader and spanstore.Writer", + reader: v1adapter.NewTraceReader(&spanstoremocks.Reader{}), + writer: v1adapter.NewTraceWriter(&spanstoremocks.Writer{}), + expectedReader: &spanstoremocks.Reader{}, + expectedWriter: &spanstoremocks.Writer{}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + gotReader, gotWriter := getV1Adapters(test.reader, test.writer) + require.Equal(t, test.expectedReader, gotReader) + require.Equal(t, test.expectedWriter, gotWriter) + }) + } +} + func TestServerAddMetricsStorage(t *testing.T) { host := componenttest.NewNopHost()