From d8a3bc117a1d4e2cfed5be42d9df378cd98fc33c Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sat, 12 Feb 2022 22:31:45 -0500 Subject: [PATCH 1/9] Enable gRPC reflection service on collector/query Signed-off-by: Yuri Shkuro --- cmd/collector/app/server/grpc.go | 15 ++++---- cmd/collector/app/server/grpc_test.go | 52 ++++++++++++++++++++++----- cmd/query/app/server.go | 8 ++++- cmd/query/app/server_test.go | 36 ++++++++++++++++--- 4 files changed, 91 insertions(+), 20 deletions(-) diff --git a/cmd/collector/app/server/grpc.go b/cmd/collector/app/server/grpc.go index fae5a4116af..7ceffd6e58e 100644 --- a/cmd/collector/app/server/grpc.go +++ b/cmd/collector/app/server/grpc.go @@ -23,6 +23,7 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/credentials" "google.golang.org/grpc/keepalive" + "google.golang.org/grpc/reflection" "github.com/jaegertracing/jaeger/cmd/collector/app/handler" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling" @@ -42,6 +43,9 @@ type GRPCServerParams struct { MaxReceiveMessageLength int MaxConnectionAge time.Duration MaxConnectionAgeGrace time.Duration + + // Set by the server to indicate the actual host:port of the server. + HostPortActual string } // StartGRPCServer based on the given parameters @@ -64,17 +68,16 @@ func StartGRPCServer(params *GRPCServerParams) (*grpc.Server, error) { creds := credentials.NewTLS(tlsCfg) grpcOpts = append(grpcOpts, grpc.Creds(creds)) - - server = grpc.NewServer(grpcOpts...) - } else { - // server without TLS - server = grpc.NewServer(grpcOpts...) } + server = grpc.NewServer(grpcOpts...) + reflection.Register(server) + listener, err := net.Listen("tcp", params.HostPort) if err != nil { return nil, fmt.Errorf("failed to listen on gRPC port: %w", err) } + params.HostPortActual = listener.Addr().String() if err := serveGRPC(server, listener, params); err != nil { return nil, err @@ -87,7 +90,7 @@ func serveGRPC(server *grpc.Server, listener net.Listener, params *GRPCServerPar api_v2.RegisterCollectorServiceServer(server, params.Handler) api_v2.RegisterSamplingManagerServer(server, sampling.NewGRPCHandler(params.SamplingStore)) - params.Logger.Info("Starting jaeger-collector gRPC server", zap.String("grpc.host-port", params.HostPort)) + params.Logger.Info("Starting jaeger-collector gRPC server", zap.String("grpc.host-port", params.HostPortActual)) go func() { if err := server.Serve(listener); err != nil { params.Logger.Error("Could not launch gRPC service", zap.Error(err)) diff --git a/cmd/collector/app/server/grpc_test.go b/cmd/collector/app/server/grpc_test.go index 22d54a8d4a9..146aa47e715 100644 --- a/cmd/collector/app/server/grpc_test.go +++ b/cmd/collector/app/server/grpc_test.go @@ -16,7 +16,6 @@ package server import ( "context" - "net" "sync" "testing" @@ -27,6 +26,7 @@ import ( "go.uber.org/zap/zaptest/observer" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/reflection/grpc_reflection_v1alpha" "google.golang.org/grpc/test/bufconn" "github.com/jaegertracing/jaeger/cmd/collector/app/handler" @@ -71,21 +71,19 @@ func TestFailServe(t *testing.T) { func TestSpanCollector(t *testing.T) { logger, _ := zap.NewDevelopment() params := &GRPCServerParams{ + HostPort: ":0", Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}), SamplingStore: &mockSamplingStore{}, Logger: logger, } - server := grpc.NewServer() - defer server.Stop() - - listener, err := net.Listen("tcp", ":0") + server, err := StartGRPCServer(params) require.NoError(t, err) - defer listener.Close() - - serveGRPC(server, listener, params) + defer server.Stop() - conn, err := grpc.Dial(listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) + conn, err := grpc.Dial( + params.HostPortActual, + grpc.WithTransportCredentials(insecure.NewCredentials())) require.NoError(t, err) defer conn.Close() @@ -114,3 +112,39 @@ func TestCollectorStartWithTLS(t *testing.T) { require.NoError(t, err) defer server.Stop() } + +func TestCollectorReflection(t *testing.T) { + logger, _ := zap.NewDevelopment() + params := &GRPCServerParams{ + HostPort: ":0", + Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}), + SamplingStore: &mockSamplingStore{}, + Logger: logger, + MaxReceiveMessageLength: 8 * 1024 * 1024, + } + + server, err := StartGRPCServer(params) + require.NoError(t, err) + defer server.Stop() + + conn, err := grpc.Dial( + params.HostPortActual, + grpc.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(t, err) + defer conn.Close() + + client := grpc_reflection_v1alpha.NewServerReflectionClient(conn) + r, err := client.ServerReflectionInfo(context.Background()) + require.NoError(t, err) + require.NotNil(t, r) + + err = r.Send(&grpc_reflection_v1alpha.ServerReflectionRequest{ + MessageRequest: &grpc_reflection_v1alpha.ServerReflectionRequest_ListServices{}, + }) + require.NoError(t, err) + m, err := r.Recv() + require.NoError(t, err) + require.IsType(t, + new(grpc_reflection_v1alpha.ServerReflectionResponse_ListServicesResponse), + m.MessageResponse) +} diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index d1f1b676ebe..7a1b93559cc 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -29,6 +29,7 @@ import ( "go.uber.org/zap/zapcore" "google.golang.org/grpc" "google.golang.org/grpc/credentials" + "google.golang.org/grpc/reflection" "github.com/jaegertracing/jaeger/cmd/query/app/apiv3" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" @@ -119,6 +120,7 @@ func createGRPCServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc. } server := grpc.NewServer(grpcOpts...) + reflection.Register(server) handler := &GRPCHandler{ queryService: querySvc, @@ -195,7 +197,11 @@ func (s *Server) initListener() (cmux.CMux, error) { if err != nil { return nil, err } - s.logger.Info("Query server started") + s.logger.Info( + "Query server started", + zap.String("http_addr", s.httpConn.Addr().String()), + zap.String("grpc_addr", s.grpcConn.Addr().String()), + ) return nil, nil } diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 165ffb5eae9..871f1211534 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -33,6 +33,7 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/reflection/grpc_reflection_v1alpha" "github.com/jaegertracing/jaeger/cmd/flags" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" @@ -688,15 +689,42 @@ func TestServerHandlesPortZero(t *testing.T) { querySvc := &querysvc.QueryService{} tracer := opentracing.NoopTracer{} - server, err := NewServer(flagsSvc.Logger, querySvc, nil, &QueryOptions{GRPCHostPort: ":0", HTTPHostPort: ":0"}, tracer) + server, err := NewServer(flagsSvc.Logger, querySvc, nil, + &QueryOptions{GRPCHostPort: ":0", HTTPHostPort: ":0"}, + tracer) assert.Nil(t, err) assert.NoError(t, server.Start()) - server.Close() + defer server.Close() message := logs.FilterMessage("Query server started") - assert.Equal(t, 1, message.Len(), "Expected query started log message.") + assert.Equal(t, 1, message.Len(), "Expected 'Query server started' log message.") onlyEntry := message.All()[0] - port := onlyEntry.ContextMap()["port"] + port := onlyEntry.ContextMap()["port"].(int64) assert.Greater(t, port, int64(0)) + + verifyGRPCReflection(t, port) +} + +func verifyGRPCReflection(t *testing.T, port int64) { + conn, err := grpc.Dial( + fmt.Sprintf(":%v", port), + grpc.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(t, err) + defer conn.Close() + + client := grpc_reflection_v1alpha.NewServerReflectionClient(conn) + r, err := client.ServerReflectionInfo(context.Background()) + require.NoError(t, err) + require.NotNil(t, r) + + err = r.Send(&grpc_reflection_v1alpha.ServerReflectionRequest{ + MessageRequest: &grpc_reflection_v1alpha.ServerReflectionRequest_ListServices{}, + }) + require.NoError(t, err) + m, err := r.Recv() + require.NoError(t, err) + require.IsType(t, + new(grpc_reflection_v1alpha.ServerReflectionResponse_ListServicesResponse), + m.MessageResponse) } From b47e7b8385e4cfd218eae6000161d44b769e7be0 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sat, 12 Feb 2022 22:47:41 -0500 Subject: [PATCH 2/9] fix Signed-off-by: Yuri Shkuro --- cmd/collector/app/server/grpc.go | 4 +++- cmd/collector/app/server/grpc_test.go | 25 ++++++++++++------------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/cmd/collector/app/server/grpc.go b/cmd/collector/app/server/grpc.go index 7ceffd6e58e..e40655a08be 100644 --- a/cmd/collector/app/server/grpc.go +++ b/cmd/collector/app/server/grpc.go @@ -53,7 +53,9 @@ func StartGRPCServer(params *GRPCServerParams) (*grpc.Server, error) { var server *grpc.Server var grpcOpts []grpc.ServerOption - grpcOpts = append(grpcOpts, grpc.MaxRecvMsgSize(params.MaxReceiveMessageLength)) + if params.MaxReceiveMessageLength > 0 { + grpcOpts = append(grpcOpts, grpc.MaxRecvMsgSize(params.MaxReceiveMessageLength)) + } grpcOpts = append(grpcOpts, grpc.KeepaliveParams(keepalive.ServerParameters{ MaxConnectionAge: params.MaxConnectionAge, MaxConnectionAgeGrace: params.MaxConnectionAgeGrace, diff --git a/cmd/collector/app/server/grpc_test.go b/cmd/collector/app/server/grpc_test.go index 146aa47e715..107ef873284 100644 --- a/cmd/collector/app/server/grpc_test.go +++ b/cmd/collector/app/server/grpc_test.go @@ -71,10 +71,11 @@ func TestFailServe(t *testing.T) { func TestSpanCollector(t *testing.T) { logger, _ := zap.NewDevelopment() params := &GRPCServerParams{ - HostPort: ":0", - Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}), - SamplingStore: &mockSamplingStore{}, - Logger: logger, + HostPort: ":0", + Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}), + SamplingStore: &mockSamplingStore{}, + Logger: logger, + MaxReceiveMessageLength: 1024 * 1024, } server, err := StartGRPCServer(params) @@ -96,10 +97,9 @@ func TestSpanCollector(t *testing.T) { func TestCollectorStartWithTLS(t *testing.T) { logger, _ := zap.NewDevelopment() params := &GRPCServerParams{ - Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}), - SamplingStore: &mockSamplingStore{}, - Logger: logger, - MaxReceiveMessageLength: 8 * 1024 * 1024, + Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}), + SamplingStore: &mockSamplingStore{}, + Logger: logger, TLSConfig: tlscfg.Options{ Enabled: true, CertPath: testCertKeyLocation + "/example-server-cert.pem", @@ -116,11 +116,10 @@ func TestCollectorStartWithTLS(t *testing.T) { func TestCollectorReflection(t *testing.T) { logger, _ := zap.NewDevelopment() params := &GRPCServerParams{ - HostPort: ":0", - Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}), - SamplingStore: &mockSamplingStore{}, - Logger: logger, - MaxReceiveMessageLength: 8 * 1024 * 1024, + HostPort: ":0", + Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}), + SamplingStore: &mockSamplingStore{}, + Logger: logger, } server, err := StartGRPCServer(params) From c9ff1326996762fcc47d74b2650d9d135c3936e8 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sun, 13 Feb 2022 00:24:54 -0500 Subject: [PATCH 3/9] refactor Signed-off-by: Yuri Shkuro --- cmd/collector/app/server/grpc_test.go | 24 ++---------- cmd/query/app/server_test.go | 28 ++------------ internal/grpctest/reflection.go | 56 +++++++++++++++++++++++++++ internal/grpctest/reflection_test.go | 44 +++++++++++++++++++++ 4 files changed, 108 insertions(+), 44 deletions(-) create mode 100644 internal/grpctest/reflection.go create mode 100644 internal/grpctest/reflection_test.go diff --git a/cmd/collector/app/server/grpc_test.go b/cmd/collector/app/server/grpc_test.go index 107ef873284..f5eea9a2c58 100644 --- a/cmd/collector/app/server/grpc_test.go +++ b/cmd/collector/app/server/grpc_test.go @@ -26,10 +26,10 @@ import ( "go.uber.org/zap/zaptest/observer" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" - "google.golang.org/grpc/reflection/grpc_reflection_v1alpha" "google.golang.org/grpc/test/bufconn" "github.com/jaegertracing/jaeger/cmd/collector/app/handler" + "github.com/jaegertracing/jaeger/internal/grpctest" "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/proto-gen/api_v2" ) @@ -126,24 +126,8 @@ func TestCollectorReflection(t *testing.T) { require.NoError(t, err) defer server.Stop() - conn, err := grpc.Dial( - params.HostPortActual, - grpc.WithTransportCredentials(insecure.NewCredentials())) - require.NoError(t, err) - defer conn.Close() - - client := grpc_reflection_v1alpha.NewServerReflectionClient(conn) - r, err := client.ServerReflectionInfo(context.Background()) - require.NoError(t, err) - require.NotNil(t, r) - - err = r.Send(&grpc_reflection_v1alpha.ServerReflectionRequest{ - MessageRequest: &grpc_reflection_v1alpha.ServerReflectionRequest_ListServices{}, + grpctest.VerifyReflectionService(t, grpctest.VerifyReflectionServiceParams{ + HostPort: params.HostPortActual, + Server: server, }) - require.NoError(t, err) - m, err := r.Recv() - require.NoError(t, err) - require.IsType(t, - new(grpc_reflection_v1alpha.ServerReflectionResponse_ListServicesResponse), - m.MessageResponse) } diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 871f1211534..cc414965b98 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -33,10 +33,10 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" - "google.golang.org/grpc/reflection/grpc_reflection_v1alpha" "github.com/jaegertracing/jaeger/cmd/flags" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" + "github.com/jaegertracing/jaeger/internal/grpctest" "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/healthcheck" @@ -703,28 +703,8 @@ func TestServerHandlesPortZero(t *testing.T) { port := onlyEntry.ContextMap()["port"].(int64) assert.Greater(t, port, int64(0)) - verifyGRPCReflection(t, port) -} - -func verifyGRPCReflection(t *testing.T, port int64) { - conn, err := grpc.Dial( - fmt.Sprintf(":%v", port), - grpc.WithTransportCredentials(insecure.NewCredentials())) - require.NoError(t, err) - defer conn.Close() - - client := grpc_reflection_v1alpha.NewServerReflectionClient(conn) - r, err := client.ServerReflectionInfo(context.Background()) - require.NoError(t, err) - require.NotNil(t, r) - - err = r.Send(&grpc_reflection_v1alpha.ServerReflectionRequest{ - MessageRequest: &grpc_reflection_v1alpha.ServerReflectionRequest_ListServices{}, + grpctest.VerifyReflectionService(t, grpctest.VerifyReflectionServiceParams{ + HostPort: fmt.Sprintf(":%v", port), + Server: server.grpcServer, }) - require.NoError(t, err) - m, err := r.Recv() - require.NoError(t, err) - require.IsType(t, - new(grpc_reflection_v1alpha.ServerReflectionResponse_ListServicesResponse), - m.MessageResponse) } diff --git a/internal/grpctest/reflection.go b/internal/grpctest/reflection.go new file mode 100644 index 00000000000..602e301098c --- /dev/null +++ b/internal/grpctest/reflection.go @@ -0,0 +1,56 @@ +// Copyright (c) 2022 The Jaeger Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package grpctest + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/reflection/grpc_reflection_v1alpha" +) + +// VerifyReflectionServiceParams defins inputs to VerifyReflectionService. +type VerifyReflectionServiceParams struct { + Server *grpc.Server + HostPort string +} + +// VerifyReflectionService validates that a gRPC service at a given address +// supports reflection service. +func VerifyReflectionService(t *testing.T, params VerifyReflectionServiceParams) { + conn, err := grpc.Dial( + params.HostPort, + grpc.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(t, err) + defer conn.Close() + + client := grpc_reflection_v1alpha.NewServerReflectionClient(conn) + r, err := client.ServerReflectionInfo(context.Background()) + require.NoError(t, err) + require.NotNil(t, r) + + err = r.Send(&grpc_reflection_v1alpha.ServerReflectionRequest{ + MessageRequest: &grpc_reflection_v1alpha.ServerReflectionRequest_ListServices{}, + }) + require.NoError(t, err) + m, err := r.Recv() + require.NoError(t, err) + require.IsType(t, + new(grpc_reflection_v1alpha.ServerReflectionResponse_ListServicesResponse), + m.MessageResponse) +} diff --git a/internal/grpctest/reflection_test.go b/internal/grpctest/reflection_test.go new file mode 100644 index 00000000000..80005a37f40 --- /dev/null +++ b/internal/grpctest/reflection_test.go @@ -0,0 +1,44 @@ +// Copyright (c) 2022 The Jaeger Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package grpctest + +import ( + "net" + "testing" + + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/reflection" +) + +func TestVerifyReflectionService(t *testing.T) { + server := grpc.NewServer() + reflection.Register(server) + + listener, err := net.Listen("tcp", ":0") + require.NoError(t, err) + defer listener.Close() + + go func() { + err := server.Serve(listener) + require.NoError(t, err) + }() + defer server.Stop() + + VerifyReflectionService(t, VerifyReflectionServiceParams{ + HostPort: listener.Addr().String(), + Server: server, + }) +} From 2880802b3c1a3f8c8e9a39fdce3a3bb10f52a4f7 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sun, 13 Feb 2022 00:36:47 -0500 Subject: [PATCH 4/9] more validation Signed-off-by: Yuri Shkuro --- cmd/collector/app/server/grpc_test.go | 4 ++++ cmd/query/app/server_test.go | 5 +++++ internal/grpctest/reflection.go | 21 ++++++++++++++++++--- internal/grpctest/reflection_test.go | 5 +++-- 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/cmd/collector/app/server/grpc_test.go b/cmd/collector/app/server/grpc_test.go index f5eea9a2c58..49cf2712333 100644 --- a/cmd/collector/app/server/grpc_test.go +++ b/cmd/collector/app/server/grpc_test.go @@ -129,5 +129,9 @@ func TestCollectorReflection(t *testing.T) { grpctest.VerifyReflectionService(t, grpctest.VerifyReflectionServiceParams{ HostPort: params.HostPortActual, Server: server, + ExpectedServices: []string{ + "jaeger.api_v2.CollectorService", + "jaeger.api_v2.SamplingManager", + }, }) } diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index cc414965b98..f73e18079d0 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -706,5 +706,10 @@ func TestServerHandlesPortZero(t *testing.T) { grpctest.VerifyReflectionService(t, grpctest.VerifyReflectionServiceParams{ HostPort: fmt.Sprintf(":%v", port), Server: server.grpcServer, + ExpectedServices: []string{ + "jaeger.api_v2.QueryService", + "jaeger.api_v3.QueryService", + "jaeger.api_v2.metrics.MetricsQueryService", + }, }) } diff --git a/internal/grpctest/reflection.go b/internal/grpctest/reflection.go index 602e301098c..d319288189a 100644 --- a/internal/grpctest/reflection.go +++ b/internal/grpctest/reflection.go @@ -24,10 +24,11 @@ import ( "google.golang.org/grpc/reflection/grpc_reflection_v1alpha" ) -// VerifyReflectionServiceParams defins inputs to VerifyReflectionService. +// VerifyReflectionServiceParams defines inputs to VerifyReflectionService. type VerifyReflectionServiceParams struct { - Server *grpc.Server - HostPort string + Server *grpc.Server + HostPort string + ExpectedServices []string } // VerifyReflectionService validates that a gRPC service at a given address @@ -53,4 +54,18 @@ func VerifyReflectionService(t *testing.T, params VerifyReflectionServiceParams) require.IsType(t, new(grpc_reflection_v1alpha.ServerReflectionResponse_ListServicesResponse), m.MessageResponse) + + resp := m.MessageResponse.(*grpc_reflection_v1alpha.ServerReflectionResponse_ListServicesResponse) + for _, svc := range params.ExpectedServices { + found := false + for _, s := range resp.ListServicesResponse.Service { + if svc == s.Name { + found = true + break + } + } + if !found { + t.Fatalf("expected to find service '%v', got '%+v'", svc, resp.ListServicesResponse.Service) + } + } } diff --git a/internal/grpctest/reflection_test.go b/internal/grpctest/reflection_test.go index 80005a37f40..d720cc3fe27 100644 --- a/internal/grpctest/reflection_test.go +++ b/internal/grpctest/reflection_test.go @@ -38,7 +38,8 @@ func TestVerifyReflectionService(t *testing.T) { defer server.Stop() VerifyReflectionService(t, VerifyReflectionServiceParams{ - HostPort: listener.Addr().String(), - Server: server, + HostPort: listener.Addr().String(), + Server: server, + ExpectedServices: []string{"grpc.reflection.v1alpha.ServerReflection"}, }) } From fd3dba65981aa68b18e07f68134e5336fe9fe63f Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sun, 13 Feb 2022 11:00:37 -0500 Subject: [PATCH 5/9] coverage Signed-off-by: Yuri Shkuro --- internal/grpctest/reflection.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/grpctest/reflection.go b/internal/grpctest/reflection.go index d319288189a..d213c2833af 100644 --- a/internal/grpctest/reflection.go +++ b/internal/grpctest/reflection.go @@ -57,15 +57,15 @@ func VerifyReflectionService(t *testing.T, params VerifyReflectionServiceParams) resp := m.MessageResponse.(*grpc_reflection_v1alpha.ServerReflectionResponse_ListServicesResponse) for _, svc := range params.ExpectedServices { - found := false + var found string for _, s := range resp.ListServicesResponse.Service { if svc == s.Name { - found = true + found = s.Name break } } - if !found { - t.Fatalf("expected to find service '%v', got '%+v'", svc, resp.ListServicesResponse.Service) - } + require.Equalf(t, svc, found, + "service not found, got '%+v'", + resp.ListServicesResponse.Service) } } From 910abee3ce60ea658123472246e3333e36841e04 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sun, 13 Feb 2022 11:03:47 -0500 Subject: [PATCH 6/9] simplify Signed-off-by: Yuri Shkuro --- cmd/collector/app/server/grpc_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/cmd/collector/app/server/grpc_test.go b/cmd/collector/app/server/grpc_test.go index 49cf2712333..ed1d6dcd32e 100644 --- a/cmd/collector/app/server/grpc_test.go +++ b/cmd/collector/app/server/grpc_test.go @@ -71,7 +71,6 @@ func TestFailServe(t *testing.T) { func TestSpanCollector(t *testing.T) { logger, _ := zap.NewDevelopment() params := &GRPCServerParams{ - HostPort: ":0", Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}), SamplingStore: &mockSamplingStore{}, Logger: logger, @@ -116,7 +115,6 @@ func TestCollectorStartWithTLS(t *testing.T) { func TestCollectorReflection(t *testing.T) { logger, _ := zap.NewDevelopment() params := &GRPCServerParams{ - HostPort: ":0", Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}), SamplingStore: &mockSamplingStore{}, Logger: logger, From a413374826b040b7324395bbf692b73e2966a8cb Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sun, 13 Feb 2022 11:10:03 -0500 Subject: [PATCH 7/9] use OOP Signed-off-by: Yuri Shkuro --- cmd/collector/app/server/grpc_test.go | 4 ++-- cmd/query/app/server_test.go | 4 ++-- internal/grpctest/reflection.go | 10 +++++----- internal/grpctest/reflection_test.go | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cmd/collector/app/server/grpc_test.go b/cmd/collector/app/server/grpc_test.go index ed1d6dcd32e..e9c8e81e029 100644 --- a/cmd/collector/app/server/grpc_test.go +++ b/cmd/collector/app/server/grpc_test.go @@ -124,12 +124,12 @@ func TestCollectorReflection(t *testing.T) { require.NoError(t, err) defer server.Stop() - grpctest.VerifyReflectionService(t, grpctest.VerifyReflectionServiceParams{ + grpctest.VerifyReflectionService{ HostPort: params.HostPortActual, Server: server, ExpectedServices: []string{ "jaeger.api_v2.CollectorService", "jaeger.api_v2.SamplingManager", }, - }) + }.Execute(t) } diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index f73e18079d0..24c08bc36ff 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -703,7 +703,7 @@ func TestServerHandlesPortZero(t *testing.T) { port := onlyEntry.ContextMap()["port"].(int64) assert.Greater(t, port, int64(0)) - grpctest.VerifyReflectionService(t, grpctest.VerifyReflectionServiceParams{ + grpctest.VerifyReflectionService{ HostPort: fmt.Sprintf(":%v", port), Server: server.grpcServer, ExpectedServices: []string{ @@ -711,5 +711,5 @@ func TestServerHandlesPortZero(t *testing.T) { "jaeger.api_v3.QueryService", "jaeger.api_v2.metrics.MetricsQueryService", }, - }) + }.Execute(t) } diff --git a/internal/grpctest/reflection.go b/internal/grpctest/reflection.go index d213c2833af..93b4d6de018 100644 --- a/internal/grpctest/reflection.go +++ b/internal/grpctest/reflection.go @@ -24,16 +24,16 @@ import ( "google.golang.org/grpc/reflection/grpc_reflection_v1alpha" ) -// VerifyReflectionServiceParams defines inputs to VerifyReflectionService. -type VerifyReflectionServiceParams struct { +// VerifyReflectionService validates that a gRPC service at a given address +// supports reflection service. Called must invoke Execute func. +type VerifyReflectionService struct { Server *grpc.Server HostPort string ExpectedServices []string } -// VerifyReflectionService validates that a gRPC service at a given address -// supports reflection service. -func VerifyReflectionService(t *testing.T, params VerifyReflectionServiceParams) { +// Execute performs validation. +func (params VerifyReflectionService) Execute(t *testing.T) { conn, err := grpc.Dial( params.HostPort, grpc.WithTransportCredentials(insecure.NewCredentials())) diff --git a/internal/grpctest/reflection_test.go b/internal/grpctest/reflection_test.go index d720cc3fe27..05441596108 100644 --- a/internal/grpctest/reflection_test.go +++ b/internal/grpctest/reflection_test.go @@ -37,9 +37,9 @@ func TestVerifyReflectionService(t *testing.T) { }() defer server.Stop() - VerifyReflectionService(t, VerifyReflectionServiceParams{ + VerifyReflectionService{ HostPort: listener.Addr().String(), Server: server, ExpectedServices: []string{"grpc.reflection.v1alpha.ServerReflection"}, - }) + }.Execute(t) } From 90035ebb2760aea86766c1d26b2ab2b56e05f18e Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sun, 13 Feb 2022 11:11:56 -0500 Subject: [PATCH 8/9] rename Signed-off-by: Yuri Shkuro --- cmd/collector/app/server/grpc_test.go | 2 +- cmd/query/app/server_test.go | 2 +- internal/grpctest/reflection.go | 6 +++--- internal/grpctest/reflection_test.go | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/collector/app/server/grpc_test.go b/cmd/collector/app/server/grpc_test.go index e9c8e81e029..71280e8cfe2 100644 --- a/cmd/collector/app/server/grpc_test.go +++ b/cmd/collector/app/server/grpc_test.go @@ -124,7 +124,7 @@ func TestCollectorReflection(t *testing.T) { require.NoError(t, err) defer server.Stop() - grpctest.VerifyReflectionService{ + grpctest.ReflectionServiceValidator{ HostPort: params.HostPortActual, Server: server, ExpectedServices: []string{ diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 24c08bc36ff..4d803a5a36f 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -703,7 +703,7 @@ func TestServerHandlesPortZero(t *testing.T) { port := onlyEntry.ContextMap()["port"].(int64) assert.Greater(t, port, int64(0)) - grpctest.VerifyReflectionService{ + grpctest.ReflectionServiceValidator{ HostPort: fmt.Sprintf(":%v", port), Server: server.grpcServer, ExpectedServices: []string{ diff --git a/internal/grpctest/reflection.go b/internal/grpctest/reflection.go index 93b4d6de018..24f61e99932 100644 --- a/internal/grpctest/reflection.go +++ b/internal/grpctest/reflection.go @@ -24,16 +24,16 @@ import ( "google.golang.org/grpc/reflection/grpc_reflection_v1alpha" ) -// VerifyReflectionService validates that a gRPC service at a given address +// ReflectionServiceValidator verifies that a gRPC service at a given address // supports reflection service. Called must invoke Execute func. -type VerifyReflectionService struct { +type ReflectionServiceValidator struct { Server *grpc.Server HostPort string ExpectedServices []string } // Execute performs validation. -func (params VerifyReflectionService) Execute(t *testing.T) { +func (params ReflectionServiceValidator) Execute(t *testing.T) { conn, err := grpc.Dial( params.HostPort, grpc.WithTransportCredentials(insecure.NewCredentials())) diff --git a/internal/grpctest/reflection_test.go b/internal/grpctest/reflection_test.go index 05441596108..0d3782bcd3e 100644 --- a/internal/grpctest/reflection_test.go +++ b/internal/grpctest/reflection_test.go @@ -23,7 +23,7 @@ import ( "google.golang.org/grpc/reflection" ) -func TestVerifyReflectionService(t *testing.T) { +func TestReflectionServiceValidator(t *testing.T) { server := grpc.NewServer() reflection.Register(server) @@ -37,7 +37,7 @@ func TestVerifyReflectionService(t *testing.T) { }() defer server.Stop() - VerifyReflectionService{ + ReflectionServiceValidator{ HostPort: listener.Addr().String(), Server: server, ExpectedServices: []string{"grpc.reflection.v1alpha.ServerReflection"}, From 1fef34adb51c424f41e851d83d28c5aed15fb131 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Sun, 13 Feb 2022 11:14:05 -0500 Subject: [PATCH 9/9] rename Signed-off-by: Yuri Shkuro --- internal/grpctest/reflection.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/grpctest/reflection.go b/internal/grpctest/reflection.go index 24f61e99932..e2791feab40 100644 --- a/internal/grpctest/reflection.go +++ b/internal/grpctest/reflection.go @@ -33,9 +33,9 @@ type ReflectionServiceValidator struct { } // Execute performs validation. -func (params ReflectionServiceValidator) Execute(t *testing.T) { +func (v ReflectionServiceValidator) Execute(t *testing.T) { conn, err := grpc.Dial( - params.HostPort, + v.HostPort, grpc.WithTransportCredentials(insecure.NewCredentials())) require.NoError(t, err) defer conn.Close() @@ -56,7 +56,7 @@ func (params ReflectionServiceValidator) Execute(t *testing.T) { m.MessageResponse) resp := m.MessageResponse.(*grpc_reflection_v1alpha.ServerReflectionResponse_ListServicesResponse) - for _, svc := range params.ExpectedServices { + for _, svc := range v.ExpectedServices { var found string for _, s := range resp.ListServicesResponse.Service { if svc == s.Name {