diff --git a/cmd/query/app/querysvc/query_service.go b/cmd/query/app/querysvc/query_service.go index 4eff06eeb7b..57369669830 100644 --- a/cmd/query/app/querysvc/query_service.go +++ b/cmd/query/app/querysvc/query_service.go @@ -42,7 +42,7 @@ type StorageCapabilities struct { // QueryService contains span utils required by the query-service. type QueryService struct { - traceReader tracestore.Reader + spanReader spanstore.Reader dependencyReader depstore.Reader options QueryServiceOptions } @@ -61,8 +61,13 @@ type TraceQueryParameters struct { // NewQueryService returns a new QueryService. func NewQueryService(traceReader tracestore.Reader, dependencyReader depstore.Reader, options QueryServiceOptions) *QueryService { + spanReader, err := v1adapter.GetV1Reader(traceReader) + if err != nil { + // TODO: implement a reverse adapter to convert v2 reader to v1 reader + panic(err) + } qsvc := &QueryService{ - traceReader: traceReader, + spanReader: spanReader, dependencyReader: dependencyReader, options: options, } @@ -75,11 +80,7 @@ func NewQueryService(traceReader tracestore.Reader, dependencyReader depstore.Re // GetTrace is the queryService implementation of spanstore.Reader.GetTrace func (qs QueryService) GetTrace(ctx context.Context, query GetTraceParameters) (*model.Trace, error) { - spanReader, err := v1adapter.GetV1Reader(qs.traceReader) - if err != nil { - return nil, err - } - trace, err := spanReader.GetTrace(ctx, query.GetTraceParameters) + trace, err := qs.spanReader.GetTrace(ctx, query.GetTraceParameters) if errors.Is(err, spanstore.ErrTraceNotFound) { if qs.options.ArchiveSpanReader == nil { return nil, err @@ -97,11 +98,7 @@ func (qs QueryService) GetTrace(ctx context.Context, query GetTraceParameters) ( // GetServices is the queryService implementation of spanstore.Reader.GetServices func (qs QueryService) GetServices(ctx context.Context) ([]string, error) { - spanReader, err := v1adapter.GetV1Reader(qs.traceReader) - if err != nil { - return nil, err - } - return spanReader.GetServices(ctx) + return qs.spanReader.GetServices(ctx) } // GetOperations is the queryService implementation of spanstore.Reader.GetOperations @@ -109,20 +106,12 @@ func (qs QueryService) GetOperations( ctx context.Context, query spanstore.OperationQueryParameters, ) ([]spanstore.Operation, error) { - spanReader, err := v1adapter.GetV1Reader(qs.traceReader) - if err != nil { - return nil, err - } - return spanReader.GetOperations(ctx, query) + return qs.spanReader.GetOperations(ctx, query) } // FindTraces is the queryService implementation of spanstore.Reader.FindTraces func (qs QueryService) FindTraces(ctx context.Context, query *TraceQueryParameters) ([]*model.Trace, error) { - spanReader, err := v1adapter.GetV1Reader(qs.traceReader) - if err != nil { - return nil, err - } - traces, err := spanReader.FindTraces(ctx, &query.TraceQueryParameters) + traces, err := qs.spanReader.FindTraces(ctx, &query.TraceQueryParameters) if err != nil { return nil, err } diff --git a/cmd/query/app/querysvc/query_service_test.go b/cmd/query/app/querysvc/query_service_test.go index 64ef7500e3f..5115d720b99 100644 --- a/cmd/query/app/querysvc/query_service_test.go +++ b/cmd/query/app/querysvc/query_service_test.go @@ -195,20 +195,6 @@ func TestGetTraceNotFound(t *testing.T) { assert.Equal(t, err, spanstore.ErrTraceNotFound) } -func TestGetTrace_V1ReaderNotFound(t *testing.T) { - fr := new(tracestoremocks.Reader) - qs := QueryService{ - traceReader: fr, - } - query := GetTraceParameters{ - GetTraceParameters: spanstore.GetTraceParameters{ - TraceID: mockTraceID, - }, - } - _, err := qs.GetTrace(context.Background(), query) - require.Error(t, err) -} - // Test QueryService.GetTrace() with ArchiveSpanReader func TestGetTraceFromArchiveStorage(t *testing.T) { tqs := initializeTestService(withArchiveSpanReader()) @@ -242,15 +228,6 @@ func TestGetServices(t *testing.T) { assert.Equal(t, expectedServices, actualServices) } -func TestGetServices_V1ReaderNotFound(t *testing.T) { - fr := new(tracestoremocks.Reader) - qs := QueryService{ - traceReader: fr, - } - _, err := qs.GetServices(context.Background()) - require.Error(t, err) -} - // Test QueryService.GetOperations() for success. func TestGetOperations(t *testing.T) { tqs := initializeTestService() @@ -269,16 +246,6 @@ func TestGetOperations(t *testing.T) { assert.Equal(t, expectedOperations, actualOperations) } -func TestGetOperations_V1ReaderNotFound(t *testing.T) { - fr := new(tracestoremocks.Reader) - qs := QueryService{ - traceReader: fr, - } - operationQuery := spanstore.OperationQueryParameters{ServiceName: "abc/trifle"} - _, err := qs.GetOperations(context.Background(), operationQuery) - require.Error(t, err) -} - // Test QueryService.FindTraces() for success. func TestFindTraces(t *testing.T) { tqs := initializeTestService() @@ -362,26 +329,6 @@ func TestFindTracesWithRawTraces(t *testing.T) { } } -func TestFindTraces_V1ReaderNotFound(t *testing.T) { - fr := new(tracestoremocks.Reader) - qs := QueryService{ - traceReader: fr, - } - duration, err := time.ParseDuration("20ms") - require.NoError(t, err) - params := &TraceQueryParameters{ - TraceQueryParameters: spanstore.TraceQueryParameters{ - ServiceName: "service", - OperationName: "operation", - StartTimeMax: time.Now(), - DurationMin: duration, - NumTraces: 200, - }, - } - _, err = qs.FindTraces(context.Background(), params) - require.Error(t, err) -} - func TestFindTracesError(t *testing.T) { tqs := initializeTestService() tqs.spanReader.On("FindTraces", mock.Anything, mock.AnythingOfType("*spanstore.TraceQueryParameters")). @@ -565,3 +512,10 @@ func TestInitArchiveStorage(t *testing.T) { func TestMain(m *testing.M) { testutils.VerifyGoLeaks(m) } + +func TestNewQueryService_PanicsForNonV1AdapterReader(t *testing.T) { + reader := &tracestoremocks.Reader{} + dependencyReader := &depsmocks.Reader{} + options := QueryServiceOptions{} + require.PanicsWithError(t, v1adapter.ErrV1ReaderNotAvailable.Error(), func() { NewQueryService(reader, dependencyReader, options) }) +} diff --git a/storage_v2/v1adapter/reader.go b/storage_v2/v1adapter/reader.go index 7ef6921d5e9..67d7f00c126 100644 --- a/storage_v2/v1adapter/reader.go +++ b/storage_v2/v1adapter/reader.go @@ -18,7 +18,7 @@ import ( "github.com/jaegertracing/jaeger/storage_v2/tracestore" ) -var errV1ReaderNotAvailable = errors.New("spanstore.Reader is not a wrapper around v1 reader") +var ErrV1ReaderNotAvailable = errors.New("spanstore.Reader is not a wrapper around v1 reader") var _ tracestore.Reader = (*TraceReader)(nil) @@ -30,7 +30,7 @@ func GetV1Reader(reader tracestore.Reader) (spanstore.Reader, error) { if tr, ok := reader.(*TraceReader); ok { return tr.spanReader, nil } - return nil, errV1ReaderNotAvailable + return nil, ErrV1ReaderNotAvailable } func NewTraceReader(spanReader spanstore.Reader) *TraceReader { diff --git a/storage_v2/v1adapter/reader_test.go b/storage_v2/v1adapter/reader_test.go index e1952c0a825..4ebceec57c6 100644 --- a/storage_v2/v1adapter/reader_test.go +++ b/storage_v2/v1adapter/reader_test.go @@ -39,7 +39,7 @@ func TestGetV1Reader_NoError(t *testing.T) { func TestGetV1Reader_Error(t *testing.T) { fr := new(tracestoremocks.Reader) _, err := GetV1Reader(fr) - require.ErrorIs(t, err, errV1ReaderNotAvailable) + require.ErrorIs(t, err, ErrV1ReaderNotAvailable) } func TestTraceReader_GetTracesDelegatesSuccessResponse(t *testing.T) {