Delete gRPC MetricsQueryService, metricsquery.proto and related code (#6616)

## Which problem is this PR solving?
- No code in Jaeger was dependent on having a gRPC endpoint for reading
metrics. UI's Monitor tab uses JSON endpoint.
- This code is just a pass-through proxy to the underlying metrics
storage with which we communicate using OpenMetrics API.

## Description of the changes
- Delete unnecessary code and proto-gen types.
- 🛑 This is a breaking change: the gRPC `MetricsQueryService` endpoint
is removed.

## How was this change tested?
- CI

---------

Signed-off-by: Yuri Shkuro <[email protected]>

Author: yurishkuro

.github/workflows/ci-lint-checks.yaml

@@ -86,10 +86,10 @@ jobs:
         go-version: 1.23.x
 
     - name: Verify Protobuf types are up to date
-      run: make proto && git diff --name-status --exit-code
+      run: make proto && { if git status --porcelain | grep '??'; then exit 1; else git diff --name-status --exit-code; fi }
 
     - name: Verify Thrift types are up to date
-      run: make thrift && git diff --name-status --exit-code
+      run: make thrift && { if git status --porcelain | grep '??'; then exit 1; else git diff --name-status --exit-code; fi }
 
     - name: Verify Mockery types are up to date
       run: make generate-mocks && { if git status --porcelain | grep '??'; then exit 1; else git diff --name-status --exit-code; fi }

Makefile.Protobuf.mk

@@ -98,8 +98,6 @@ patch-api-v2:
 proto-openmetrics:
 	$(call print_caption, Processing OpenMetrics Protos)
 	$(foreach file,$(OPENMETRICS_PROTO_FILES),$(call proto_compile, proto-gen/api_v2/metrics, $(file)))
-	@# TODO why is this file included in model/proto/metrics/ in the first place?
-	rm proto-gen/api_v2/metrics/otelmetric.pb.go
 
 .PHONY: proto-storage-v1
 proto-storage-v1:

cmd/anonymizer/app/query/query_test.go

@@ -18,7 +18,6 @@ import (
 	"github.com/jaegertracing/jaeger-idl/proto-gen/api_v2"
 	"github.com/jaegertracing/jaeger/cmd/query/app"
 	"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
-	"github.com/jaegertracing/jaeger/plugin/metricstore/disabled"
 	"github.com/jaegertracing/jaeger/storage/spanstore"
 	spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks"
 	dependencyStoreMocks "github.com/jaegertracing/jaeger/storage_v2/depstore/mocks"
@@ -58,15 +57,13 @@ type testServer struct {
 func newTestServer(t *testing.T) *testServer {
 	spanReader := &spanstoremocks.Reader{}
 	traceReader := v1adapter.NewTraceReader(spanReader)
-	metricsReader, err := disabled.NewMetricsReader()
-	require.NoError(t, err)
 
 	q := querysvc.NewQueryService(
 		traceReader,
 		&dependencyStoreMocks.Reader{},
 		querysvc.QueryServiceOptions{},
 	)
-	h := app.NewGRPCHandler(q, metricsReader, app.GRPCHandlerOptions{})
+	h := app.NewGRPCHandler(q, app.GRPCHandlerOptions{})
 
 	server := grpc.NewServer()
 	api_v2.RegisterQueryServiceServer(server, h)

cmd/jaeger/internal/extension/jaegerquery/server_test.go

@@ -245,7 +245,6 @@ func TestServerStart(t *testing.T) {
 					ExpectedServices: []string{
 						"jaeger.api_v2.QueryService",
 						"jaeger.api_v3.QueryService",
-						"jaeger.api_v2.metrics.MetricsQueryService",
 						"grpc.health.v1.Health",
 					},
 				}.Execute(t)

cmd/query/app/grpc_handler.go

@@ -17,9 +17,6 @@ import (
 	"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
 	_ "github.com/jaegertracing/jaeger/pkg/gogocodec" // force gogo codec registration
 	"github.com/jaegertracing/jaeger/pkg/jtracer"
-	"github.com/jaegertracing/jaeger/plugin/metricstore/disabled"
-	"github.com/jaegertracing/jaeger/proto-gen/api_v2/metrics"
-	"github.com/jaegertracing/jaeger/storage/metricstore"
 	"github.com/jaegertracing/jaeger/storage/spanstore"
 )
 
@@ -30,20 +27,16 @@ const (
 )
 
 var (
-	errGRPCMetricsQueryDisabled = status.Error(codes.Unimplemented, "metrics querying is currently disabled")
-	errNilRequest               = status.Error(codes.InvalidArgument, "a nil argument is not allowed")
-	errUninitializedTraceID     = status.Error(codes.InvalidArgument, "uninitialized TraceID is not allowed")
-	errMissingServiceNames      = status.Error(codes.InvalidArgument, "please provide at least one service name")
-	errMissingQuantile          = status.Error(codes.InvalidArgument, "please provide a quantile between (0, 1]")
+	errNilRequest           = status.Error(codes.InvalidArgument, "a nil argument is not allowed")
+	errUninitializedTraceID = status.Error(codes.InvalidArgument, "uninitialized TraceID is not allowed")
 )
 
 // GRPCHandler implements the gRPC endpoint of the query service.
 type GRPCHandler struct {
-	queryService        *querysvc.QueryService
-	metricsQueryService querysvc.MetricsQueryService
-	logger              *zap.Logger
-	tracer              *jtracer.JTracer
-	nowFn               func() time.Time
+	queryService *querysvc.QueryService
+	logger       *zap.Logger
+	tracer       *jtracer.JTracer
+	nowFn        func() time.Time
 }
 
 // GRPCHandlerOptions contains optional members of GRPCHandler.
@@ -55,7 +48,6 @@ type GRPCHandlerOptions struct {
 
 // NewGRPCHandler returns a GRPCHandler.
 func NewGRPCHandler(queryService *querysvc.QueryService,
-	metricsQueryService querysvc.MetricsQueryService,
 	options GRPCHandlerOptions,
 ) *GRPCHandler {
 	if options.Logger == nil {
@@ -71,11 +63,10 @@ func NewGRPCHandler(queryService *querysvc.QueryService,
 	}
 
 	return &GRPCHandler{
-		queryService:        queryService,
-		metricsQueryService: metricsQueryService,
-		logger:              options.Logger,
-		tracer:              options.Tracer,
-		nowFn:               options.NowFn,
+		queryService: queryService,
+		logger:       options.Logger,
+		tracer:       options.Tracer,
+		nowFn:        options.NowFn,
 	}
 }
 
@@ -248,135 +239,3 @@ func (g *GRPCHandler) GetDependencies(ctx context.Context, r *api_v2.GetDependen
 
 	return &api_v2.GetDependenciesResponse{Dependencies: dependencies}, nil
 }
-
-// GetLatencies is the gRPC handler to fetch latency metrics.
-func (g *GRPCHandler) GetLatencies(ctx context.Context, r *metrics.GetLatenciesRequest) (*metrics.GetMetricsResponse, error) {
-	bqp, err := g.newBaseQueryParameters(r)
-	if err := g.handleErr("failed to build parameters", err); err != nil {
-		return nil, err
-	}
-	// Check for cases where clients do not provide the Quantile, which defaults to the float64's zero value.
-	if r.Quantile == 0 {
-		return nil, errMissingQuantile
-	}
-	queryParams := metricstore.LatenciesQueryParameters{
-		BaseQueryParameters: bqp,
-		Quantile:            r.Quantile,
-	}
-	m, err := g.metricsQueryService.GetLatencies(ctx, &queryParams)
-	if err := g.handleErr("failed to fetch latencies", err); err != nil {
-		return nil, err
-	}
-	return &metrics.GetMetricsResponse{Metrics: *m}, nil
-}
-
-// GetCallRates is the gRPC handler to fetch call rate metrics.
-func (g *GRPCHandler) GetCallRates(ctx context.Context, r *metrics.GetCallRatesRequest) (*metrics.GetMetricsResponse, error) {
-	bqp, err := g.newBaseQueryParameters(r)
-	if err := g.handleErr("failed to build parameters", err); err != nil {
-		return nil, err
-	}
-	queryParams := metricstore.CallRateQueryParameters{
-		BaseQueryParameters: bqp,
-	}
-	m, err := g.metricsQueryService.GetCallRates(ctx, &queryParams)
-	if err := g.handleErr("failed to fetch call rates", err); err != nil {
-		return nil, err
-	}
-	return &metrics.GetMetricsResponse{Metrics: *m}, nil
-}
-
-// GetErrorRates is the gRPC handler to fetch error rate metrics.
-func (g *GRPCHandler) GetErrorRates(ctx context.Context, r *metrics.GetErrorRatesRequest) (*metrics.GetMetricsResponse, error) {
-	bqp, err := g.newBaseQueryParameters(r)
-	if err := g.handleErr("failed to build parameters", err); err != nil {
-		return nil, err
-	}
-	queryParams := metricstore.ErrorRateQueryParameters{
-		BaseQueryParameters: bqp,
-	}
-	m, err := g.metricsQueryService.GetErrorRates(ctx, &queryParams)
-	if err := g.handleErr("failed to fetch error rates", err); err != nil {
-		return nil, err
-	}
-	return &metrics.GetMetricsResponse{Metrics: *m}, nil
-}
-
-// GetMinStepDuration is the gRPC handler to fetch the minimum step duration supported by the underlying metrics store.
-func (g *GRPCHandler) GetMinStepDuration(ctx context.Context, _ *metrics.GetMinStepDurationRequest) (*metrics.GetMinStepDurationResponse, error) {
-	minStep, err := g.metricsQueryService.GetMinStepDuration(ctx, &metricstore.MinStepDurationQueryParameters{})
-	if err := g.handleErr("failed to fetch min step duration", err); err != nil {
-		return nil, err
-	}
-	return &metrics.GetMinStepDurationResponse{MinStep: minStep}, nil
-}
-
-func (g *GRPCHandler) handleErr(msg string, err error) error {
-	if err == nil {
-		return nil
-	}
-	g.logger.Error(msg, zap.Error(err))
-
-	// Avoid wrapping "expected" errors with an "Internal Server" error.
-	if errors.Is(err, disabled.ErrDisabled) {
-		return errGRPCMetricsQueryDisabled
-	}
-	if _, ok := status.FromError(err); ok {
-		return err
-	}
-
-	// Received an "unexpected" error.
-	return status.Errorf(codes.Internal, "%s: %v", msg, err)
-}
-
-func (g *GRPCHandler) newBaseQueryParameters(r any) (bqp metricstore.BaseQueryParameters, err error) {
-	if r == nil {
-		return bqp, errNilRequest
-	}
-	var baseRequest *metrics.MetricsQueryBaseRequest
-	switch v := r.(type) {
-	case *metrics.GetLatenciesRequest:
-		baseRequest = v.BaseRequest
-	case *metrics.GetCallRatesRequest:
-		baseRequest = v.BaseRequest
-	case *metrics.GetErrorRatesRequest:
-		baseRequest = v.BaseRequest
-	}
-	if baseRequest == nil || len(baseRequest.ServiceNames) == 0 {
-		return bqp, errMissingServiceNames
-	}
-
-	// Copy non-nullable params.
-	bqp.GroupByOperation = baseRequest.GroupByOperation
-	bqp.ServiceNames = baseRequest.ServiceNames
-
-	// Initialize nullable params with defaults.
-	defaultEndTime := g.nowFn()
-	bqp.EndTime = &defaultEndTime
-	bqp.Lookback = &defaultMetricsQueryLookbackDuration
-	bqp.RatePer = &defaultMetricsQueryRateDuration
-	bqp.SpanKinds = defaultMetricsSpanKinds
-	bqp.Step = &defaultMetricsQueryStepDuration
-
-	// ... and override defaults with any provided request params.
-	if baseRequest.EndTime != nil {
-		bqp.EndTime = baseRequest.EndTime
-	}
-	if baseRequest.Lookback != nil {
-		bqp.Lookback = baseRequest.Lookback
-	}
-	if baseRequest.Step != nil {
-		bqp.Step = baseRequest.Step
-	}
-	if baseRequest.RatePer != nil {
-		bqp.RatePer = baseRequest.RatePer
-	}
-	if len(baseRequest.SpanKinds) > 0 {
-		spanKinds := make([]string, len(baseRequest.SpanKinds))
-		for i, v := range baseRequest.SpanKinds {
-			spanKinds[i] = v.String()
-		}
-		bqp.SpanKinds = spanKinds
-	}
-	return bqp, nil
-}