[v2][storage] Add dependency store to v2 storage interface (jaegertra…

…cing#6297) ## Which problem is this PR solving? - Towards jaegertracing#5079 ## Description of the changes - This PR creates a new factory in the `depstore` package for the DependencyReader and exposes a `CreateDependencyReader` function in the `factoryadapter` - This PR also changes some interface return types to structs because some structs implement multiple interfaces. - The new factory was integrated into the query service and the callsites were changed to use the v2 factory instead of the v1 factory to initialize the dependency reader. ## How was this change tested? - CI / Unit Tests ## 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]> Signed-off-by: Mahad Zaryab <[email protected]>
Ayush-Vish · Dec 7, 2024 · b264d2c · b264d2c
1 parent 723202b
commit b264d2c
Show file tree

Hide file tree

Showing 29 changed files with 307 additions and 109 deletions.
diff --git a/.mockery.yaml b/.mockery.yaml
@@ -76,3 +76,6 @@ packages:
   github.com/jaegertracing/jaeger/storage_v2/tracestore:
     config:
       all: true
+  github.com/jaegertracing/jaeger/storage_v2/depstore:
+    config:
+      all: true
diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go
@@ -38,7 +38,7 @@ import (
 	ss "github.com/jaegertracing/jaeger/plugin/sampling/strategyprovider"
 	"github.com/jaegertracing/jaeger/plugin/storage"
 	"github.com/jaegertracing/jaeger/ports"
-	"github.com/jaegertracing/jaeger/storage/dependencystore"
+	"github.com/jaegertracing/jaeger/storage_v2/depstore"
 	"github.com/jaegertracing/jaeger/storage_v2/factoryadapter"
 	"github.com/jaegertracing/jaeger/storage_v2/tracestore"
 )
@@ -115,7 +115,7 @@ by default uses only in-memory database.`,
 			if err != nil {
 				logger.Fatal("Failed to create span writer", zap.Error(err))
 			}
-			dependencyReader, err := storageFactory.CreateDependencyReader()
+			dependencyReader, err := v2Factory.CreateDependencyReader()
 			if err != nil {
 				logger.Fatal("Failed to create dependency reader", zap.Error(err))
 			}
@@ -219,7 +219,7 @@ func startQuery(
 	qOpts *queryApp.QueryOptions,
 	queryOpts *querysvc.QueryServiceOptions,
 	traceReader tracestore.Reader,
-	depReader dependencystore.Reader,
+	depReader depstore.Reader,
 	metricsQueryService querysvc.MetricsQueryService,
 	tm *tenancy.Manager,
 	telset telemetry.Settings,

diff --git a/cmd/anonymizer/app/query/query_test.go b/cmd/anonymizer/app/query/query_test.go
@@ -18,9 +18,9 @@ import (
 	"github.com/jaegertracing/jaeger/model"
 	"github.com/jaegertracing/jaeger/plugin/metricstore/disabled"
 	"github.com/jaegertracing/jaeger/proto-gen/api_v2"
-	dependencyStoreMocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks"
 	"github.com/jaegertracing/jaeger/storage/spanstore"
 	spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks"
+	dependencyStoreMocks "github.com/jaegertracing/jaeger/storage_v2/depstore/mocks"
 	"github.com/jaegertracing/jaeger/storage_v2/factoryadapter"
 )
 

diff --git a/cmd/jaeger/internal/exporters/storageexporter/exporter.go b/cmd/jaeger/internal/exporters/storageexporter/exporter.go
@@ -34,7 +34,7 @@ func newExporter(config *Config, otel component.TelemetrySettings) *storageExpor
 }
 
 func (exp *storageExporter) start(_ context.Context, host component.Host) error {
-	f, err := jaegerstorage.GetStorageFactoryV2(exp.config.TraceStorage, host)
+	f, err := jaegerstorage.GetTraceStoreFactory(exp.config.TraceStorage, host)
 	if err != nil {
 		return fmt.Errorf("cannot find storage factory: %w", err)
 	}

diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go
@@ -21,6 +21,7 @@ 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_v2/depstore"
 )
 
 var (
@@ -71,23 +72,20 @@ func (s *server) Start(ctx context.Context, host component.Host) error {
 	telset.Metrics = telset.Metrics.
 		Namespace(metrics.NSOptions{Name: "jaeger"}).
 		Namespace(metrics.NSOptions{Name: "query"})
-
-	// TODO currently v1 is still needed because of dependency storage
-	v1Factory, err := jaegerstorage.GetStorageFactory(s.config.Storage.TracesPrimary, host)
+	tf, err := jaegerstorage.GetTraceStoreFactory(s.config.Storage.TracesPrimary, host)
 	if err != nil {
-		return fmt.Errorf("cannot find v1 factory for primary storage %s: %w", s.config.Storage.TracesPrimary, err)
+		return fmt.Errorf("cannot find factory for trace storage %s: %w", s.config.Storage.TracesPrimary, err)
 	}
-	f, err := jaegerstorage.GetStorageFactoryV2(s.config.Storage.TracesPrimary, host)
-	if err != nil {
-		return fmt.Errorf("cannot find v2 factory for primary storage %s: %w", s.config.Storage.TracesPrimary, err)
-	}
-
-	traceReader, err := f.CreateTraceReader()
+	traceReader, err := tf.CreateTraceReader()
 	if err != nil {
 		return fmt.Errorf("cannot create trace reader: %w", err)
 	}
 
-	depReader, err := v1Factory.CreateDependencyReader()
+	df, ok := tf.(depstore.Factory)
+	if !ok {
+		return fmt.Errorf("cannot find factory for dependency storage %s: %w", s.config.Storage.TracesPrimary, err)
+	}
+	depReader, err := df.CreateDependencyReader()
 	if err != nil {
 		return fmt.Errorf("cannot create dependencies reader: %w", err)
 	}

diff --git a/cmd/jaeger/internal/extension/jaegerquery/server_test.go b/cmd/jaeger/internal/extension/jaegerquery/server_test.go
@@ -150,7 +150,7 @@ func TestServerStart(t *testing.T) {
 					TracesPrimary: "need-factory-error",
 				},
 			},
-			expectedErr: "cannot find v1 factory for primary storage",
+			expectedErr: "cannot find factory for trace storage",
 		},
 		{
 			name: "span reader error",

diff --git a/cmd/jaeger/internal/extension/jaegerstorage/extension.go b/cmd/jaeger/internal/extension/jaegerstorage/extension.go
@@ -74,7 +74,7 @@ func GetMetricStorageFactory(name string, host component.Host) (storage.MetricSt
 	return mf, nil
 }
 
-func GetStorageFactoryV2(name string, host component.Host) (tracestore.Factory, error) {
+func GetTraceStoreFactory(name string, host component.Host) (tracestore.Factory, error) {
 	f, err := GetStorageFactory(name, host)
 	if err != nil {
 		return nil, err

diff --git a/cmd/jaeger/internal/extension/jaegerstorage/extension_test.go b/cmd/jaeger/internal/extension/jaegerstorage/extension_test.go
@@ -100,7 +100,7 @@ func TestStorageFactoryBadShutdownError(t *testing.T) {
 
 func TestGetFactoryV2Error(t *testing.T) {
 	host := componenttest.NewNopHost()
-	_, err := GetStorageFactoryV2("something", host)
+	_, err := GetTraceStoreFactory("something", host)
 	require.ErrorContains(t, err, "cannot find extension")
 }
 
@@ -112,7 +112,7 @@ func TestGetFactory(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, f)
 
-	f2, err := GetStorageFactoryV2(name, host)
+	f2, err := GetTraceStoreFactory(name, host)
 	require.NoError(t, err)
 	require.NotNil(t, f2)
 

diff --git a/cmd/query/app/apiv3/grpc_handler_test.go b/cmd/query/app/apiv3/grpc_handler_test.go
@@ -20,9 +20,9 @@ import (
 	"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
 	"github.com/jaegertracing/jaeger/model"
 	_ "github.com/jaegertracing/jaeger/pkg/gogocodec" // force gogo codec registration
-	dependencyStoreMocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks"
 	"github.com/jaegertracing/jaeger/storage/spanstore"
 	spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks"
+	dependencyStoreMocks "github.com/jaegertracing/jaeger/storage_v2/depstore/mocks"
 	"github.com/jaegertracing/jaeger/storage_v2/factoryadapter"
 )
 

diff --git a/cmd/query/app/apiv3/http_gateway_test.go b/cmd/query/app/apiv3/http_gateway_test.go
@@ -22,9 +22,9 @@ import (
 	"github.com/jaegertracing/jaeger/model"
 	"github.com/jaegertracing/jaeger/pkg/jtracer"
 	"github.com/jaegertracing/jaeger/pkg/testutils"
-	dependencyStoreMocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks"
 	"github.com/jaegertracing/jaeger/storage/spanstore"
 	spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks"
+	dependencyStoreMocks "github.com/jaegertracing/jaeger/storage_v2/depstore/mocks"
 	"github.com/jaegertracing/jaeger/storage_v2/factoryadapter"
 )
 

diff --git a/cmd/query/app/grpc_handler_test.go b/cmd/query/app/grpc_handler_test.go
@@ -28,11 +28,12 @@ import (
 	"github.com/jaegertracing/jaeger/plugin/metricstore/disabled"
 	"github.com/jaegertracing/jaeger/proto-gen/api_v2"
 	"github.com/jaegertracing/jaeger/proto-gen/api_v2/metrics"
-	depsmocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks"
 	"github.com/jaegertracing/jaeger/storage/metricstore"
 	metricsmocks "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/depstore"
+	depsmocks "github.com/jaegertracing/jaeger/storage_v2/depstore/mocks"
 	"github.com/jaegertracing/jaeger/storage_v2/factoryadapter"
 )
 
@@ -513,8 +514,10 @@ func TestGetDependenciesSuccessGRPC(t *testing.T) {
 		endTs := time.Now().UTC()
 		server.depReader.On("GetDependencies",
 			mock.Anything, // context.Context
-			endTs,
-			defaultDependencyLookbackDuration,
+			depstore.QueryParameters{
+				StartTime: endTs.Add(-defaultDependencyLookbackDuration),
+				EndTime:   endTs,
+			},
 		).Return(expectedDependencies, nil).Times(1)
 
 		res, err := client.GetDependencies(context.Background(), &api_v2.GetDependenciesRequest{
@@ -529,11 +532,13 @@ func TestGetDependenciesSuccessGRPC(t *testing.T) {
 func TestGetDependenciesFailureGRPC(t *testing.T) {
 	withServerAndClient(t, func(server *grpcServer, client *grpcClient) {
 		endTs := time.Now().UTC()
-		server.depReader.On(
-			"GetDependencies",
+		server.depReader.On("GetDependencies",
 			mock.Anything, // context.Context
-			endTs,
-			defaultDependencyLookbackDuration).Return(nil, errStorageGRPC).Times(1)
+			depstore.QueryParameters{
+				StartTime: endTs.Add(-defaultDependencyLookbackDuration),
+				EndTime:   endTs,
+			},
+		).Return(nil, errStorageGRPC).Times(1)
 
 		_, err := client.GetDependencies(context.Background(), &api_v2.GetDependenciesRequest{
 			StartTime: endTs.Add(time.Duration(-1) * defaultDependencyLookbackDuration),

diff --git a/cmd/query/app/handler_deps_test.go b/cmd/query/app/handler_deps_test.go
@@ -15,6 +15,7 @@ import (
 
 	"github.com/jaegertracing/jaeger/model"
 	ui "github.com/jaegertracing/jaeger/model/json"
+	"github.com/jaegertracing/jaeger/storage_v2/depstore"
 )
 
 func TestDeduplicateDependencies(t *testing.T) {
@@ -304,8 +305,10 @@ func TestGetDependenciesSuccess(t *testing.T) {
 	endTs := time.Unix(0, 1476374248550*millisToNanosMultiplier)
 	ts.dependencyReader.On("GetDependencies",
 		mock.Anything, // context
-		endTs,
-		defaultDependencyLookbackDuration,
+		depstore.QueryParameters{
+			StartTime: endTs.Add(-defaultDependencyLookbackDuration),
+			EndTime:   endTs,
+		},
 	).Return(expectedDependencies, nil).Times(1)
 
 	var response structuredResponse
@@ -324,8 +327,11 @@ func TestGetDependenciesCassandraFailure(t *testing.T) {
 	endTs := time.Unix(0, 1476374248550*millisToNanosMultiplier)
 	ts.dependencyReader.On("GetDependencies",
 		mock.Anything, // context
-		endTs,
-		defaultDependencyLookbackDuration).Return(nil, errStorage).Times(1)
+		depstore.QueryParameters{
+			StartTime: endTs.Add(-defaultDependencyLookbackDuration),
+			EndTime:   endTs,
+		},
+	).Return(nil, errStorage).Times(1)
 
 	var response structuredResponse
 	err := getJSON(ts.server.URL+"/api/dependencies?endTs=1476374248550&service=testing", &response)

diff --git a/cmd/query/app/http_handler_test.go b/cmd/query/app/http_handler_test.go
@@ -38,10 +38,10 @@ import (
 	"github.com/jaegertracing/jaeger/pkg/tenancy"
 	"github.com/jaegertracing/jaeger/plugin/metricstore/disabled"
 	"github.com/jaegertracing/jaeger/proto-gen/api_v2/metrics"
-	depsmocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks"
 	metricsmocks "github.com/jaegertracing/jaeger/storage/metricstore/mocks"
 	"github.com/jaegertracing/jaeger/storage/spanstore"
 	spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks"
+	depsmocks "github.com/jaegertracing/jaeger/storage_v2/depstore/mocks"
 	"github.com/jaegertracing/jaeger/storage_v2/factoryadapter"
 )
 

diff --git a/cmd/query/app/querysvc/query_service.go b/cmd/query/app/querysvc/query_service.go
@@ -13,8 +13,8 @@ import (
 	"github.com/jaegertracing/jaeger/model"
 	"github.com/jaegertracing/jaeger/model/adjuster"
 	"github.com/jaegertracing/jaeger/storage"
-	"github.com/jaegertracing/jaeger/storage/dependencystore"
 	"github.com/jaegertracing/jaeger/storage/spanstore"
+	"github.com/jaegertracing/jaeger/storage_v2/depstore"
 	"github.com/jaegertracing/jaeger/storage_v2/factoryadapter"
 	"github.com/jaegertracing/jaeger/storage_v2/tracestore"
 )
@@ -43,12 +43,12 @@ type StorageCapabilities struct {
 // QueryService contains span utils required by the query-service.
 type QueryService struct {
 	traceReader      tracestore.Reader
-	dependencyReader dependencystore.Reader
+	dependencyReader depstore.Reader
 	options          QueryServiceOptions
 }
 
 // NewQueryService returns a new QueryService.
-func NewQueryService(traceReader tracestore.Reader, dependencyReader dependencystore.Reader, options QueryServiceOptions) *QueryService {
+func NewQueryService(traceReader tracestore.Reader, dependencyReader depstore.Reader, options QueryServiceOptions) *QueryService {
 	qsvc := &QueryService{
 		traceReader:      traceReader,
 		dependencyReader: dependencyReader,
@@ -134,7 +134,10 @@ func (qs QueryService) Adjust(trace *model.Trace) (*model.Trace, error) {
 
 // GetDependencies implements dependencystore.Reader.GetDependencies
 func (qs QueryService) GetDependencies(ctx context.Context, endTs time.Time, lookback time.Duration) ([]model.DependencyLink, error) {
-	return qs.dependencyReader.GetDependencies(ctx, endTs, lookback)
+	return qs.dependencyReader.GetDependencies(ctx, depstore.QueryParameters{
+		StartTime: endTs.Add(-lookback),
+		EndTime:   endTs,
+	})
 }
 
 // GetCapabilities returns the features supported by the query service.

diff --git a/cmd/query/app/querysvc/query_service_test.go b/cmd/query/app/querysvc/query_service_test.go
@@ -22,9 +22,10 @@ import (
 	"github.com/jaegertracing/jaeger/pkg/testutils"
 	"github.com/jaegertracing/jaeger/storage"
 	"github.com/jaegertracing/jaeger/storage/dependencystore"
-	depsmocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks"
 	"github.com/jaegertracing/jaeger/storage/spanstore"
 	spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks"
+	"github.com/jaegertracing/jaeger/storage_v2/depstore"
+	depsmocks "github.com/jaegertracing/jaeger/storage_v2/depstore/mocks"
 	"github.com/jaegertracing/jaeger/storage_v2/factoryadapter"
 	"github.com/jaegertracing/jaeger/storage_v2/tracestore"
 )
@@ -345,8 +346,10 @@ func TestGetDependencies(t *testing.T) {
 	tqs.depsReader.On(
 		"GetDependencies",
 		mock.Anything, // context.Context
-		endTs,
-		defaultDependencyLookbackDuration).Return(expectedDependencies, nil).Times(1)
+		depstore.QueryParameters{
+			StartTime: endTs.Add(-defaultDependencyLookbackDuration),
+			EndTime:   endTs,
+		}).Return(expectedDependencies, nil).Times(1)
 
 	actualDependencies, err := tqs.queryService.GetDependencies(context.Background(), time.Unix(0, 1476374248550*millisToNanosMultiplier), defaultDependencyLookbackDuration)
 	require.NoError(t, err)

diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go
@@ -39,8 +39,8 @@ import (
 	"github.com/jaegertracing/jaeger/pkg/tenancy"
 	"github.com/jaegertracing/jaeger/ports"
 	"github.com/jaegertracing/jaeger/proto-gen/api_v2"
-	depsmocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks"
 	spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks"
+	depsmocks "github.com/jaegertracing/jaeger/storage_v2/depstore/mocks"
 	"github.com/jaegertracing/jaeger/storage_v2/factoryadapter"
 )
 

diff --git a/cmd/query/main.go b/cmd/query/main.go
@@ -97,7 +97,7 @@ func main() {
 			if err != nil {
 				logger.Fatal("Failed to create trace reader", zap.Error(err))
 			}
-			dependencyReader, err := storageFactory.CreateDependencyReader()
+			dependencyReader, err := v2Factory.CreateDependencyReader()
 			if err != nil {
 				logger.Fatal("Failed to create dependency reader", zap.Error(err))
 			}

diff --git a/storage_v2/depstore/factory.go b/storage_v2/depstore/factory.go
@@ -0,0 +1,8 @@
+// Copyright (c) 2024 The Jaeger Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package depstore
+
+type Factory interface {
+	CreateDependencyReader() (Reader, error)
+}
diff --git a/storage_v2/depstore/mocks/Factory.go b/storage_v2/depstore/mocks/Factory.go