From 946bdbca27c2d3c4f2f5156f9a15f1e9be5aba14 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab <43658574+mahadzaryab1@users.noreply.github.com> Date: Thu, 31 Oct 2024 19:54:10 -0400 Subject: [PATCH] [fix][query] Remove bifurcation for grpc query server (#6145) ## Which problem is this PR solving? - Resolves #6026 ## Description of the changes - This PR removes the bifurcation in the creation of the GRPC query server. This bifurcation was initially added to handle the case where users were leveraging the same port for starting both the GRPC and HTTP query servers. However, that part is handled by `initListener` and not at the time of creating the server, so we can simply use OTEL's helper to create the server while leaving our logic to start the listeners using cmux if the ports are the same. ## How was this change tested? - CI ## 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 --- cmd/query/app/server.go | 49 ++--------------------------------------- 1 file changed, 2 insertions(+), 47 deletions(-) diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index a26508e4b15..543ede956ee 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -22,7 +22,6 @@ import ( "go.opentelemetry.io/otel/metric/noop" "go.uber.org/zap" "google.golang.org/grpc" - "google.golang.org/grpc/credentials" "google.golang.org/grpc/health" "google.golang.org/grpc/health/grpc_health_v1" "google.golang.org/grpc/reflection" @@ -78,12 +77,7 @@ func NewServer( return nil, errors.New("server with TLS enabled can not use same host ports for gRPC and HTTP. Use dedicated HTTP and gRPC host ports instead") } - var grpcServer *grpc.Server - if !separatePorts { - grpcServer, err = createGRPCServerLegacy(ctx, options, tm) - } else { - grpcServer, err = createGRPCServerOTEL(ctx, options, tm, telset) - } + grpcServer, err := createGRPCServer(ctx, options, tm, telset) if err != nil { return nil, err } @@ -103,45 +97,6 @@ func NewServer( }, nil } -func createGRPCServerLegacy( - ctx context.Context, - options *QueryOptions, - tm *tenancy.Manager, -) (*grpc.Server, error) { - var grpcOpts []grpc.ServerOption - - if options.GRPC.TLSSetting != nil { - tlsCfg, err := options.GRPC.TLSSetting.LoadTLSConfig(ctx) - if err != nil { - return nil, err - } - - creds := credentials.NewTLS(tlsCfg) - - grpcOpts = append(grpcOpts, grpc.Creds(creds)) - } - - unaryInterceptors := []grpc.UnaryServerInterceptor{ - bearertoken.NewUnaryServerInterceptor(), - } - streamInterceptors := []grpc.StreamServerInterceptor{ - bearertoken.NewStreamServerInterceptor(), - } - //nolint:contextcheck - if tm.Enabled { - unaryInterceptors = append(unaryInterceptors, tenancy.NewGuardingUnaryInterceptor(tm)) - streamInterceptors = append(streamInterceptors, tenancy.NewGuardingStreamInterceptor(tm)) - } - - grpcOpts = append(grpcOpts, - grpc.ChainUnaryInterceptor(unaryInterceptors...), - grpc.ChainStreamInterceptor(streamInterceptors...), - ) - - server := grpc.NewServer(grpcOpts...) - return server, nil -} - func registerGRPCHandlers( server *grpc.Server, querySvc *querysvc.QueryService, @@ -165,7 +120,7 @@ func registerGRPCHandlers( grpc_health_v1.RegisterHealthServer(server, healthServer) } -func createGRPCServerOTEL( +func createGRPCServer( ctx context.Context, options *QueryOptions, tm *tenancy.Manager,