Skip to content

Commit

Permalink
[fix][query] Remove bifurcation for grpc query server (#6145)
Browse files Browse the repository at this point in the history
## 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 <[email protected]>
  • Loading branch information
mahadzaryab1 authored Oct 31, 2024
1 parent 4f5eca1 commit 946bdbc
Showing 1 changed file with 2 additions and 47 deletions.
49 changes: 2 additions & 47 deletions cmd/query/app/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand All @@ -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,
Expand All @@ -165,7 +120,7 @@ func registerGRPCHandlers(
grpc_health_v1.RegisterHealthServer(server, healthServer)
}

func createGRPCServerOTEL(
func createGRPCServer(
ctx context.Context,
options *QueryOptions,
tm *tenancy.Manager,
Expand Down

0 comments on commit 946bdbc

Please sign in to comment.