Revert "[query] Use OTEL's helpers for grpc server (jaegertracing#6055)"

This reverts commit c96790a.

Author: yurishkuro

cmd/all-in-one/main.go

@@ -133,8 +133,7 @@ by default uses only in-memory database.`,
 			if err != nil {
 				logger.Fatal("Failed to initialize collector", zap.Error(err))
 			}
-			defaultOpts := queryApp.DefaultQueryOptions()
-			qOpts, err := defaultOpts.InitFromViper(v, logger)
+			qOpts, err := new(queryApp.QueryOptions).InitFromViper(v, logger)
 			if err != nil {
 				logger.Fatal("Failed to configure query service", zap.Error(err))
 			}
@@ -221,11 +220,11 @@ func startQuery(
 	spanReader = storageMetrics.NewReadMetricsDecorator(spanReader, telset.Metrics)
 	qs := querysvc.NewQueryService(spanReader, depReader, *queryOpts)
 
-	server, err := queryApp.NewServer(context.Background(), qs, metricsQueryService, qOpts, tm, telset)
+	server, err := queryApp.NewServer(qs, metricsQueryService, qOpts, tm, telset)
 	if err != nil {
 		svc.Logger.Fatal("Could not create jaeger-query", zap.Error(err))
 	}
-	if err := server.Start(context.Background()); err != nil {
+	if err := server.Start(); err != nil {
 		svc.Logger.Fatal("Could not start jaeger-query", zap.Error(err))
 	}
 

cmd/jaeger/internal/extension/jaegerquery/factory.go

@@ -7,9 +7,13 @@ import (
 	"context"
 
 	"go.opentelemetry.io/collector/component"
+	"go.opentelemetry.io/collector/config/configgrpc"
+	"go.opentelemetry.io/collector/config/confighttp"
+	"go.opentelemetry.io/collector/config/confignet"
 	"go.opentelemetry.io/collector/extension"
 
 	"github.com/jaegertracing/jaeger/cmd/query/app"
+	"github.com/jaegertracing/jaeger/ports"
 )
 
 // componentType is the name of this extension in configuration.
@@ -24,7 +28,17 @@ func NewFactory() extension.Factory {
 
 func createDefaultConfig() component.Config {
 	return &Config{
-		QueryOptions: app.DefaultQueryOptions(),
+		QueryOptions: app.QueryOptions{
+			HTTP: confighttp.ServerConfig{
+				Endpoint: ports.PortToHostPort(ports.QueryHTTP),
+			},
+			GRPC: configgrpc.ServerConfig{
+				NetAddr: confignet.AddrConfig{
+					Endpoint:  ports.PortToHostPort(ports.QueryGRPC),
+					Transport: confignet.TransportTypeTCP,
+				},
+			},
+		},
 	}
 }
 

cmd/jaeger/internal/extension/jaegerquery/server.go

@@ -51,7 +51,7 @@ func (*server) Dependencies() []component.ID {
 	return []component.ID{jaegerstorage.ID}
 }
 
-func (s *server) Start(ctx context.Context, host component.Host) error {
+func (s *server) Start(_ context.Context, host component.Host) error {
 	mf := otelmetrics.NewFactory(s.telset.MeterProvider)
 	baseFactory := mf.Namespace(metrics.NSOptions{Name: "jaeger"})
 	queryMetricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "query"})
@@ -100,11 +100,11 @@ func (s *server) Start(ctx context.Context, host component.Host) error {
 		ReportStatus: func(event *componentstatus.Event) {
 			componentstatus.ReportStatus(host, event)
 		},
-		Host: host,
 	}
 
+	// TODO contextcheck linter complains about next line that context is not passed. It is not wrong.
+	//nolint
 	s.server, err = queryApp.NewServer(
-		ctx,
 		// TODO propagate healthcheck updates up to the collector's runtime
 		qs,
 		mqs,
@@ -116,7 +116,7 @@ func (s *server) Start(ctx context.Context, host component.Host) error {
 		return fmt.Errorf("could not create jaeger-query: %w", err)
 	}
 
-	if err := s.server.Start(ctx); err != nil {
+	if err := s.server.Start(); err != nil {
 		return fmt.Errorf("could not start jaeger-query: %w", err)
 	}
 

cmd/jaeger/internal/extension/jaegerquery/server_test.go

@@ -15,7 +15,6 @@ import (
 	"github.com/stretchr/testify/require"
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/component/componenttest"
-	"go.opentelemetry.io/collector/config/confignet"
 	"go.opentelemetry.io/collector/config/configtelemetry"
 	"go.opentelemetry.io/otel/metric"
 	noopmetric "go.opentelemetry.io/otel/metric/noop"
@@ -212,7 +211,6 @@ func TestServerStart(t *testing.T) {
 			}
 			tt.config.HTTP.Endpoint = ":0"
 			tt.config.GRPC.NetAddr.Endpoint = ":0"
-			tt.config.GRPC.NetAddr.Transport = confignet.TransportTypeTCP
 			server := newServer(tt.config, telemetrySettings)
 			err := server.Start(context.Background(), host)
 			if tt.expectedErr == "" {

cmd/query/app/flags.go

@@ -18,7 +18,6 @@ import (
 	"github.com/spf13/viper"
 	"go.opentelemetry.io/collector/config/configgrpc"
 	"go.opentelemetry.io/collector/config/confighttp"
-	"go.opentelemetry.io/collector/config/confignet"
 	"go.opentelemetry.io/collector/config/configopaque"
 	"go.uber.org/zap"
 
@@ -101,11 +100,6 @@ func AddFlags(flagSet *flag.FlagSet) {
 func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*QueryOptions, error) {
 	qOpts.HTTP.Endpoint = v.GetString(queryHTTPHostPort)
 	qOpts.GRPC.NetAddr.Endpoint = v.GetString(queryGRPCHostPort)
-	// TODO: drop support for same host ports
-	// https://github.com/jaegertracing/jaeger/issues/6117
-	if qOpts.HTTP.Endpoint == qOpts.GRPC.NetAddr.Endpoint {
-		logger.Warn("using the same port for gRPC and HTTP is deprecated; please use dedicated ports instead; support for shared ports will be removed in Feb 2025")
-	}
 	tlsGrpc, err := tlsGRPCFlagsConfig.InitFromViper(v)
 	if err != nil {
 		return qOpts, fmt.Errorf("failed to process gRPC TLS options: %w", err)
@@ -175,17 +169,3 @@ func mapHTTPHeaderToOTELHeaders(h http.Header) map[string]configopaque.String {
 
 	return otelHeaders
 }
-
-func DefaultQueryOptions() QueryOptions {
-	return QueryOptions{
-		HTTP: confighttp.ServerConfig{
-			Endpoint: ports.PortToHostPort(ports.QueryHTTP),
-		},
-		GRPC: configgrpc.ServerConfig{
-			NetAddr: confignet.AddrConfig{
-				Endpoint:  ports.PortToHostPort(ports.QueryGRPC),
-				Transport: confignet.TransportTypeTCP,
-			},
-		},
-	}
-}

cmd/query/app/flags_test.go

@@ -15,7 +15,6 @@ import (
 	"go.uber.org/zap"
 
 	"github.com/jaegertracing/jaeger/pkg/config"
-	"github.com/jaegertracing/jaeger/pkg/testutils"
 	"github.com/jaegertracing/jaeger/ports"
 	"github.com/jaegertracing/jaeger/storage/mocks"
 	spanstore_mocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks"
@@ -184,27 +183,3 @@ func TestQueryOptions_FailedTLSFlags(t *testing.T) {
 		})
 	}
 }
-
-func TestQueryOptions_SamePortsLogsWarning(t *testing.T) {
-	logger, logBuf := testutils.NewLogger()
-	v, command := config.Viperize(AddFlags)
-	command.ParseFlags([]string{
-		"--query.http-server.host-port=127.0.0.1:8081",
-		"--query.grpc-server.host-port=127.0.0.1:8081",
-	})
-	_, err := new(QueryOptions).InitFromViper(v, logger)
-	require.NoError(t, err)
-
-	require.Contains(
-		t,
-		logBuf.String(),
-		"using the same port for gRPC and HTTP is deprecated",
-	)
-}
-
-func TestDefaultQueryOptions(t *testing.T) {
-	qo := DefaultQueryOptions()
-	require.Equal(t, ":16686", qo.HTTP.Endpoint)
-	require.Equal(t, ":16685", qo.GRPC.NetAddr.Endpoint)
-	require.EqualValues(t, "tcp", qo.GRPC.NetAddr.Transport)
-}

cmd/query/app/server.go

@@ -16,12 +16,7 @@ import (
 
 	"github.com/gorilla/handlers"
 	"github.com/soheilhy/cmux"
-	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/component/componentstatus"
-	"go.opentelemetry.io/collector/config/configgrpc"
-	"go.opentelemetry.io/collector/config/configtelemetry"
-	"go.opentelemetry.io/otel/metric"
-	"go.opentelemetry.io/otel/metric/noop"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
 	"google.golang.org/grpc"
@@ -59,9 +54,7 @@ type Server struct {
 }
 
 // NewServer creates and initializes Server
-func NewServer(
-	ctx context.Context,
-	querySvc *querysvc.QueryService,
+func NewServer(querySvc *querysvc.QueryService,
 	metricsQuerySvc querysvc.MetricsQueryService,
 	options *QueryOptions,
 	tm *tenancy.Manager,
@@ -81,18 +74,12 @@ 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(querySvc, metricsQuerySvc, options, tm, telset)
 	if err != nil {
 		return nil, err
 	}
-	registerGRPCHandlers(grpcServer, querySvc, metricsQuerySvc, telset)
 
-	httpServer, err := createHTTPServer(ctx, querySvc, metricsQuerySvc, options, tm, telset)
+	httpServer, err := createHTTPServer(querySvc, metricsQuerySvc, options, tm, telset)
 	if err != nil {
 		return nil, err
 	}
@@ -107,15 +94,11 @@ func NewServer(
 	}, nil
 }
 
-func createGRPCServerLegacy(
-	ctx context.Context,
-	options *QueryOptions,
-	tm *tenancy.Manager,
-) (*grpc.Server, error) {
+func createGRPCServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc.MetricsQueryService, options *QueryOptions, tm *tenancy.Manager, telset telemetery.Setting) (*grpc.Server, error) {
 	var grpcOpts []grpc.ServerOption
 
 	if options.GRPC.TLSSetting != nil {
-		tlsCfg, err := options.GRPC.TLSSetting.LoadTLSConfig(ctx)
+		tlsCfg, err := options.GRPC.TLSSetting.LoadTLSConfig(context.Background())
 		if err != nil {
 			return nil, err
 		}
@@ -125,24 +108,15 @@ func createGRPCServerLegacy(
 		grpcOpts = append(grpcOpts, grpc.Creds(creds))
 	}
 	if tm.Enabled {
-		//nolint:contextcheck
 		grpcOpts = append(grpcOpts,
 			grpc.StreamInterceptor(tenancy.NewGuardingStreamInterceptor(tm)),
 			grpc.UnaryInterceptor(tenancy.NewGuardingUnaryInterceptor(tm)),
 		)
 	}
 
 	server := grpc.NewServer(grpcOpts...)
-	return server, nil
-}
-
-func registerGRPCHandlers(
-	server *grpc.Server,
-	querySvc *querysvc.QueryService,
-	metricsQuerySvc querysvc.MetricsQueryService,
-	telset telemetery.Setting,
-) {
 	reflection.Register(server)
+
 	handler := NewGRPCHandler(querySvc, metricsQuerySvc, GRPCHandlerOptions{
 		Logger: telset.Logger,
 	})
@@ -157,33 +131,7 @@ func registerGRPCHandlers(
 	healthServer.SetServingStatus("jaeger.api_v3.QueryService", grpc_health_v1.HealthCheckResponse_SERVING)
 
 	grpc_health_v1.RegisterHealthServer(server, healthServer)
-}
-
-func createGRPCServerOTEL(
-	ctx context.Context,
-	options *QueryOptions,
-	tm *tenancy.Manager,
-	telset telemetery.Setting,
-) (*grpc.Server, error) {
-	var grpcOpts []configgrpc.ToServerOption
-	if tm.Enabled {
-		//nolint:contextcheck
-		grpcOpts = append(grpcOpts,
-			configgrpc.WithGrpcServerOption(grpc.StreamInterceptor(tenancy.NewGuardingStreamInterceptor(tm))),
-			configgrpc.WithGrpcServerOption(grpc.UnaryInterceptor(tenancy.NewGuardingUnaryInterceptor(tm))),
-		)
-	}
-	return options.GRPC.ToServer(
-		ctx,
-		telset.Host,
-		component.TelemetrySettings{
-			Logger:         telset.Logger,
-			TracerProvider: telset.TracerProvider,
-			LeveledMeterProvider: func(_ configtelemetry.Level) metric.MeterProvider {
-				return noop.NewMeterProvider()
-			},
-		},
-		grpcOpts...)
+	return server, nil
 }
 
 type httpServer struct {
@@ -194,7 +142,6 @@ type httpServer struct {
 var _ io.Closer = (*httpServer)(nil)
 
 func createHTTPServer(
-	ctx context.Context,
 	querySvc *querysvc.QueryService,
 	metricsQuerySvc querysvc.MetricsQueryService,
 	queryOpts *QueryOptions,
@@ -242,7 +189,7 @@ func createHTTPServer(
 	}
 
 	if queryOpts.HTTP.TLSSetting != nil {
-		tlsCfg, err := queryOpts.HTTP.TLSSetting.LoadTLSConfig(ctx) // This checks if the certificates are correctly provided
+		tlsCfg, err := queryOpts.HTTP.TLSSetting.LoadTLSConfig(context.Background()) // This checks if the certificates are correctly provided
 		if err != nil {
 			return nil, err
 		}
@@ -262,10 +209,10 @@ func (hS httpServer) Close() error {
 }
 
 // initListener initialises listeners of the server
-func (s *Server) initListener(ctx context.Context) (cmux.CMux, error) {
+func (s *Server) initListener() (cmux.CMux, error) {
 	if s.separatePorts { // use separate ports and listeners each for gRPC and HTTP requests
 		var err error
-		s.grpcConn, err = s.queryOptions.GRPC.NetAddr.Listen(ctx)
+		s.grpcConn, err = net.Listen("tcp", s.queryOptions.GRPC.NetAddr.Endpoint)
 		if err != nil {
 			return nil, err
 		}
@@ -313,8 +260,8 @@ func (s *Server) initListener(ctx context.Context) (cmux.CMux, error) {
 }
 
 // Start http, GRPC and cmux servers concurrently
-func (s *Server) Start(ctx context.Context) error {
-	cmuxServer, err := s.initListener(ctx)
+func (s *Server) Start() error {
+	cmuxServer, err := s.initListener()
 	if err != nil {
 		return fmt.Errorf("query server failed to initialize listener: %w", err)
 	}