[query] Update v1 query service to check for adapter at construction (#…

…6482)

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

## Description of the changes
- This PR changes the v1 query service to check for the v1 adapter at
construction rather than at method invocation.
- Currently, the constructor will panic if its provided a native v2
storage implementation. This will be remedied in a follow-up PR that
will implement a reverse adapter to go from v2->v1.

## How was this change tested?
- CI and added a unit test

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

---------

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

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,32 +98,20 @@ 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
 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
 	}

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) })
+}

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 {

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