Skip to content

Commit c9d6957

Browse files
albertteohalbertteoh
authored
Convert MetricsQueryService to interface (#3089)
* Convert MetricsQueryService to interface Signed-off-by: albertteoh <[email protected]> * Simplify and make consistent Signed-off-by: albertteoh <[email protected]>
1 parent 013efad commit c9d6957

File tree

6 files changed

+25
-165
lines changed

6 files changed

+25
-165
lines changed

‎cmd/all-in-one/main.go

+5-7
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ import (
5050
"github.com/jaegertracing/jaeger/plugin/storage"
5151
"github.com/jaegertracing/jaeger/ports"
5252
"github.com/jaegertracing/jaeger/storage/dependencystore"
53-
"github.com/jaegertracing/jaeger/storage/metricsstore"
5453
"github.com/jaegertracing/jaeger/storage/spanstore"
5554
storageMetrics "github.com/jaegertracing/jaeger/storage/spanstore/metrics"
5655
)
@@ -116,7 +115,7 @@ by default uses only in-memory database.`,
116115
logger.Fatal("Failed to create dependency reader", zap.Error(err))
117116
}
118117

119-
metricsReader, err := createMetricsReader(metricsReaderFactory, v, logger)
118+
metricsQueryService, err := createMetricsQueryService(metricsReaderFactory, v, logger)
120119
if err != nil {
121120
logger.Fatal("Failed to create metrics reader", zap.Error(err))
122121
}
@@ -171,7 +170,7 @@ by default uses only in-memory database.`,
171170
// query
172171
querySrv := startQuery(
173172
svc, qOpts, qOpts.BuildQueryServiceOptions(storageFactory, logger),
174-
spanReader, dependencyReader, metricsReader,
173+
spanReader, dependencyReader, metricsQueryService,
175174
metricsFactory,
176175
)
177176

@@ -244,13 +243,12 @@ func startQuery(
244243
queryOpts *querysvc.QueryServiceOptions,
245244
spanReader spanstore.Reader,
246245
depReader dependencystore.Reader,
247-
metricsReader metricsstore.Reader,
246+
metricsQueryService querysvc.MetricsQueryService,
248247
baseFactory metrics.Factory,
249248
) *queryApp.Server {
250249
spanReader = storageMetrics.NewReadMetricsDecorator(spanReader, baseFactory.Namespace(metrics.NSOptions{Name: "query"}))
251250
qs := querysvc.NewQueryService(spanReader, depReader, *queryOpts)
252-
mqs := querysvc.NewMetricsQueryService(metricsReader)
253-
server, err := queryApp.NewServer(svc.Logger, qs, mqs, qOpts, opentracing.GlobalTracer())
251+
server, err := queryApp.NewServer(svc.Logger, qs, metricsQueryService, qOpts, opentracing.GlobalTracer())
254252
if err != nil {
255253
svc.Logger.Fatal("Could not start jaeger-query service", zap.Error(err))
256254
}
@@ -289,7 +287,7 @@ func initTracer(metricsFactory metrics.Factory, logger *zap.Logger) io.Closer {
289287
return closer
290288
}
291289

292-
func createMetricsReader(factory *metricsPlugin.Factory, v *viper.Viper, logger *zap.Logger) (metricsstore.Reader, error) {
290+
func createMetricsQueryService(factory *metricsPlugin.Factory, v *viper.Viper, logger *zap.Logger) (querysvc.MetricsQueryService, error) {
293291
if err := factory.Initialize(logger); err != nil {
294292
return nil, fmt.Errorf("failed to init metrics reader factory: %w", err)
295293
}

‎cmd/query/app/querysvc/metrics_query_service.go

+3-36
Original file line numberDiff line numberDiff line change
@@ -14,42 +14,9 @@
1414

1515
package querysvc
1616

17-
import (
18-
"context"
19-
"time"
20-
21-
"github.com/jaegertracing/jaeger/proto-gen/api_v2/metrics"
22-
"github.com/jaegertracing/jaeger/storage/metricsstore"
23-
)
17+
import "github.com/jaegertracing/jaeger/storage/metricsstore"
2418

2519
// MetricsQueryService provides a means of querying R.E.D metrics from an underlying metrics store.
26-
type MetricsQueryService struct {
27-
metricsReader metricsstore.Reader
28-
}
29-
30-
// NewMetricsQueryService returns a new MetricsQueryService.
31-
func NewMetricsQueryService(reader metricsstore.Reader) *MetricsQueryService {
32-
return &MetricsQueryService{
33-
metricsReader: reader,
34-
}
35-
}
36-
37-
// GetLatencies is the queryService implementation of metricsstore.Reader.
38-
func (mqs MetricsQueryService) GetLatencies(ctx context.Context, params *metricsstore.LatenciesQueryParameters) (*metrics.MetricFamily, error) {
39-
return mqs.metricsReader.GetLatencies(ctx, params)
40-
}
41-
42-
// GetCallRates is the queryService implementation of metricsstore.Reader.
43-
func (mqs MetricsQueryService) GetCallRates(ctx context.Context, params *metricsstore.CallRateQueryParameters) (*metrics.MetricFamily, error) {
44-
return mqs.metricsReader.GetCallRates(ctx, params)
45-
}
46-
47-
// GetErrorRates is the queryService implementation of metricsstore.Reader.
48-
func (mqs MetricsQueryService) GetErrorRates(ctx context.Context, params *metricsstore.ErrorRateQueryParameters) (*metrics.MetricFamily, error) {
49-
return mqs.metricsReader.GetErrorRates(ctx, params)
50-
}
51-
52-
// GetMinStepDuration is the queryService implementation of metricsstore.Reader.
53-
func (mqs MetricsQueryService) GetMinStepDuration(ctx context.Context, params *metricsstore.MinStepDurationQueryParameters) (time.Duration, error) {
54-
return mqs.metricsReader.GetMinStepDuration(ctx, params)
20+
type MetricsQueryService interface {
21+
metricsstore.Reader
5522
}

‎cmd/query/app/querysvc/metrics_query_service_test.go

-96
This file was deleted.

‎cmd/query/app/server.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ type Server struct {
5252
}
5353

5454
// NewServer creates and initializes Server
55-
func NewServer(logger *zap.Logger, querySvc *querysvc.QueryService, metricsQuerySvc *querysvc.MetricsQueryService, options *QueryOptions, tracer opentracing.Tracer) (*Server, error) {
55+
func NewServer(logger *zap.Logger, querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, options *QueryOptions, tracer opentracing.Tracer) (*Server, error) {
5656

5757
_, httpPort, err := net.SplitHostPort(options.HTTPHostPort)
5858
if err != nil {
@@ -94,7 +94,7 @@ func (s Server) HealthCheckStatus() chan healthcheck.Status {
9494
return s.unavailableChannel
9595
}
9696

97-
func createGRPCServer(querySvc *querysvc.QueryService, metricsQuerySvc *querysvc.MetricsQueryService, options *QueryOptions, logger *zap.Logger, tracer opentracing.Tracer) (*grpc.Server, error) {
97+
func createGRPCServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, options *QueryOptions, logger *zap.Logger, tracer opentracing.Tracer) (*grpc.Server, error) {
9898
var grpcOpts []grpc.ServerOption
9999

100100
if options.TLSGRPC.Enabled {
@@ -118,7 +118,7 @@ func createGRPCServer(querySvc *querysvc.QueryService, metricsQuerySvc *querysvc
118118
return server, nil
119119
}
120120

121-
func createHTTPServer(querySvc *querysvc.QueryService, metricsQuerySvc *querysvc.MetricsQueryService, queryOpts *QueryOptions, tracer opentracing.Tracer, logger *zap.Logger) (*http.Server, error) {
121+
func createHTTPServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, queryOpts *QueryOptions, tracer opentracing.Tracer, logger *zap.Logger) (*http.Server, error) {
122122
// TODO: Add HandlerOptions.MetricsQueryService
123123
apiHandlerOptions := []HandlerOption{
124124
HandlerOptions.Logger(logger),

‎cmd/query/app/server_test.go

+11-16
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func TestCreateTLSServerSinglePortError(t *testing.T) {
6464
ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem",
6565
}
6666

67-
_, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, querysvc.NewMetricsQueryService(nil),
67+
_, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, nil,
6868
&QueryOptions{HTTPHostPort: ":8080", GRPCHostPort: ":8080", TLSGRPC: tlsCfg, TLSHTTP: tlsCfg}, opentracing.NoopTracer{})
6969
assert.NotNil(t, err)
7070
}
@@ -77,7 +77,7 @@ func TestCreateTLSGrpcServerError(t *testing.T) {
7777
ClientCAPath: "invalid/path",
7878
}
7979

80-
_, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, querysvc.NewMetricsQueryService(nil),
80+
_, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, nil,
8181
&QueryOptions{HTTPHostPort: ":8080", GRPCHostPort: ":8081", TLSGRPC: tlsCfg}, opentracing.NoopTracer{})
8282
assert.NotNil(t, err)
8383
}
@@ -90,7 +90,7 @@ func TestCreateTLSHttpServerError(t *testing.T) {
9090
ClientCAPath: "invalid/path",
9191
}
9292

93-
_, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, querysvc.NewMetricsQueryService(nil),
93+
_, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, nil,
9494
&QueryOptions{HTTPHostPort: ":8080", GRPCHostPort: ":8081", TLSHTTP: tlsCfg}, opentracing.NoopTracer{})
9595
assert.NotNil(t, err)
9696
}
@@ -331,8 +331,7 @@ func TestServerHTTPTLS(t *testing.T) {
331331
spanReader.On("GetServices", mock.AnythingOfType("*context.valueCtx")).Return(expectedServices, nil)
332332

333333
querySvc := querysvc.NewQueryService(spanReader, dependencyReader, querysvc.QueryServiceOptions{})
334-
metricsQuerySvc := querysvc.NewMetricsQueryService(nil)
335-
server, err := NewServer(flagsSvc.Logger, querySvc, metricsQuerySvc,
334+
server, err := NewServer(flagsSvc.Logger, querySvc, nil,
336335
serverOptions,
337336
opentracing.NoopTracer{})
338337
assert.Nil(t, err)
@@ -492,8 +491,7 @@ func TestServerGRPCTLS(t *testing.T) {
492491
spanReader.On("GetServices", mock.AnythingOfType("*context.valueCtx")).Return(expectedServices, nil)
493492

494493
querySvc := querysvc.NewQueryService(spanReader, dependencyReader, querysvc.QueryServiceOptions{})
495-
metricsQuerySvc := querysvc.NewMetricsQueryService(nil)
496-
server, err := NewServer(flagsSvc.Logger, querySvc, metricsQuerySvc,
494+
server, err := NewServer(flagsSvc.Logger, querySvc, nil,
497495
serverOptions,
498496
opentracing.NoopTracer{})
499497
assert.Nil(t, err)
@@ -547,12 +545,12 @@ func TestServerGRPCTLS(t *testing.T) {
547545

548546
}
549547
func TestServerBadHostPort(t *testing.T) {
550-
_, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, querysvc.NewMetricsQueryService(nil),
548+
_, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, nil,
551549
&QueryOptions{HTTPHostPort: "8080", GRPCHostPort: "127.0.0.1:8081", BearerTokenPropagation: true},
552550
opentracing.NoopTracer{})
553551

554552
assert.NotNil(t, err)
555-
_, err = NewServer(zap.NewNop(), &querysvc.QueryService{}, querysvc.NewMetricsQueryService(nil),
553+
_, err = NewServer(zap.NewNop(), &querysvc.QueryService{}, nil,
556554
&QueryOptions{HTTPHostPort: "127.0.0.1:8081", GRPCHostPort: "9123", BearerTokenPropagation: true},
557555
opentracing.NoopTracer{})
558556

@@ -578,7 +576,7 @@ func TestServerInUseHostPort(t *testing.T) {
578576
server, err := NewServer(
579577
zap.NewNop(),
580578
&querysvc.QueryService{},
581-
querysvc.NewMetricsQueryService(nil),
579+
nil,
582580
&QueryOptions{
583581
HTTPHostPort: tc.httpHostPort,
584582
GRPCHostPort: tc.grpcHostPort,
@@ -611,8 +609,7 @@ func TestServerSinglePort(t *testing.T) {
611609
spanReader.On("GetServices", mock.AnythingOfType("*context.valueCtx")).Return(expectedServices, nil)
612610

613611
querySvc := querysvc.NewQueryService(spanReader, dependencyReader, querysvc.QueryServiceOptions{})
614-
metricsQuerySvc := querysvc.NewMetricsQueryService(nil)
615-
server, err := NewServer(flagsSvc.Logger, querySvc, metricsQuerySvc,
612+
server, err := NewServer(flagsSvc.Logger, querySvc, nil,
616613
&QueryOptions{GRPCHostPort: hostPort, HTTPHostPort: hostPort, BearerTokenPropagation: true},
617614
opentracing.NoopTracer{})
618615
assert.Nil(t, err)
@@ -661,10 +658,9 @@ func TestServerGracefulExit(t *testing.T) {
661658
hostPort := ports.PortToHostPort(ports.QueryAdminHTTP)
662659

663660
querySvc := &querysvc.QueryService{}
664-
metricsQuerySvc := querysvc.NewMetricsQueryService(nil)
665661
tracer := opentracing.NoopTracer{}
666662

667-
server, err := NewServer(flagsSvc.Logger, querySvc, metricsQuerySvc, &QueryOptions{GRPCHostPort: hostPort, HTTPHostPort: hostPort}, tracer)
663+
server, err := NewServer(flagsSvc.Logger, querySvc, nil, &QueryOptions{GRPCHostPort: hostPort, HTTPHostPort: hostPort}, tracer)
668664
assert.Nil(t, err)
669665
assert.NoError(t, server.Start())
670666
go func() {
@@ -690,9 +686,8 @@ func TestServerHandlesPortZero(t *testing.T) {
690686
flagsSvc.Logger = zap.New(zapCore)
691687

692688
querySvc := &querysvc.QueryService{}
693-
metricsQuerySvc := querysvc.NewMetricsQueryService(nil)
694689
tracer := opentracing.NoopTracer{}
695-
server, err := NewServer(flagsSvc.Logger, querySvc, metricsQuerySvc, &QueryOptions{GRPCHostPort: ":0", HTTPHostPort: ":0"}, tracer)
690+
server, err := NewServer(flagsSvc.Logger, querySvc, nil, &QueryOptions{GRPCHostPort: ":0", HTTPHostPort: ":0"}, tracer)
696691
assert.Nil(t, err)
697692
assert.NoError(t, server.Start())
698693
server.Close()

‎cmd/query/main.go

+3-7
Original file line numberDiff line numberDiff line change
@@ -163,16 +163,12 @@ func main() {
163163
}
164164
}
165165

166-
func createMetricsQueryService(factory *metricsPlugin.Factory, v *viper.Viper, logger *zap.Logger) (*querysvc.MetricsQueryService, error) {
166+
func createMetricsQueryService(factory *metricsPlugin.Factory, v *viper.Viper, logger *zap.Logger) (querysvc.MetricsQueryService, error) {
167167
if err := factory.Initialize(logger); err != nil {
168-
return nil, fmt.Errorf("failed to init metrics factory: %w", err)
168+
return nil, fmt.Errorf("failed to init metrics reader factory: %w", err)
169169
}
170170

171171
// Ensure default parameter values are loaded correctly.
172172
factory.InitFromViper(v)
173-
metricsReader, err := factory.CreateMetricsReader()
174-
if err != nil {
175-
return nil, fmt.Errorf("failed to create metrics reader: %w", err)
176-
}
177-
return querysvc.NewMetricsQueryService(metricsReader), nil
173+
return factory.CreateMetricsReader()
178174
}

0 commit comments

Comments
 (0)