Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Jan 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci-lint-checks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
2 changes: 0 additions & 2 deletions Makefile.Protobuf.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 1 addition & 4 deletions cmd/anonymizer/app/query/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion cmd/jaeger/internal/extension/jaegerquery/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
161 changes: 10 additions & 151 deletions cmd/query/app/grpc_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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.
Expand All @@ -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 {
Expand All @@ -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,
}
}

Expand Down Expand Up @@ -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
}
Loading
Loading