Normalize CLI flags to use host:port addresses

CHANGELOG.md

@@ -26,6 +26,16 @@ Changes by Version
     --collector.port
     ```
 
+* Normalize CLI flags to use host:port addresses ([#1827](https://github.com/jaegertracing/jaeger/pull/1827), [@annanay25](https://github.com/annanay25))
+
+  Jaeger now uses standard host:port addresses as CLI flags. Flags previous accepting listen addresses in any other format have now been deprecated.
+  Deprecated flags and replacements -
+
+  * `collector.port` is superseded by `collector.tchan-server.host-port`
+  * `collector.http-port` is superseded by `collector.http-server.host-port`
+  * `collector.grpc-port` is superseded by `collector.grpc-server.host-port`
+  * `collector.zipkin.http-port` is superseded by `collector.zipkin.host-port`
+
 #### New Features
 
 #### Bug fixes, Minor Improvements

cmd/collector/app/builder_flags.go

@@ -31,8 +31,11 @@ const (
 	collectorNumWorkers           = "collector.num-workers"
 	collectorHTTPPort             = "collector.http-port"
 	collectorGRPCPort             = "collector.grpc-port"
+	collectorHTTPHostPort         = "collector.http-server.host-port"
+	collectorGRPCHostPort         = "collector.grpc-server.host-port"
+	collectorZipkinHTTPPort       = "collector.zipkin.http-port"
+	collectorZipkinHTTPHostPort   = "collector.zipkin.host-port"
 	collectorTags                 = "collector.tags"
-	collectorZipkinHTTPort        = "collector.zipkin.http-port"
 	collectorZipkinAllowedOrigins = "collector.zipkin.allowed-origins"
 	collectorZipkinAllowedHeaders = "collector.zipkin.allowed-headers"
 )
@@ -55,12 +58,18 @@ type CollectorOptions struct {
 	CollectorHTTPPort int
 	// CollectorGRPCPort is the port that the collector service listens in on for gRPC requests
 	CollectorGRPCPort int
+	// CollectorHTTPHostPort is the host:port address that the collector service listens in on for http requests
+	CollectorHTTPHostPort string
+	// CollectorGRPCHostPort is the host:port address that the collector service listens in on for gRPC requests
+	CollectorGRPCHostPort string
 	// TLS configures secure transport
 	TLS tlscfg.Options
 	// CollectorTags is the string representing collector tags to append to each and every span
 	CollectorTags map[string]string
 	// CollectorZipkinHTTPPort is the port that the Zipkin collector service listens in on for http requests
 	CollectorZipkinHTTPPort int
+	// CollectorZipkinHTTPHostPort is the host:port address that the Zipkin collector service listens in on for http requests
+	CollectorZipkinHTTPHostPort string
 	// CollectorZipkinAllowedOrigins is a list of origins a cross-domain request to the Zipkin collector service can be executed from
 	CollectorZipkinAllowedOrigins string
 	// CollectorZipkinAllowedHeaders is a list of headers that the Zipkin collector service allowes the client to use with cross-domain requests
@@ -69,13 +78,16 @@ type CollectorOptions struct {
 
 // AddFlags adds flags for CollectorOptions
 func AddFlags(flags *flag.FlagSet) {
-	flags.Uint(collectorDynQueueSizeMemory, 0, "(experimental) The max memory size in MiB to use for the dynamic queue.")
 	flags.Int(collectorQueueSize, DefaultQueueSize, "The queue size of the collector")
 	flags.Int(collectorNumWorkers, DefaultNumWorkers, "The number of workers pulling items from the queue")
-	flags.Int(collectorHTTPPort, ports.CollectorHTTP, "The HTTP port for the collector service")
-	flags.Int(collectorGRPCPort, ports.CollectorGRPC, "The gRPC port for the collector service")
+	flags.Int(collectorHTTPPort, 0, "(deprecated) please use -"+collectorHTTPHostPort)
+	flags.Int(collectorGRPCPort, 0, "(deprecated) please use -"+collectorGRPCHostPort)
+	flags.Int(collectorZipkinHTTPPort, 0, "(deprecated) please use -"+collectorZipkinHTTPHostPort)
+	flags.String(collectorHTTPHostPort, ports.PortToHostPort(ports.CollectorHTTP), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's HTTP server")
+	flags.String(collectorGRPCHostPort, ports.PortToHostPort(ports.CollectorGRPC), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's GRPC server")
+	flags.String(collectorZipkinHTTPHostPort, ports.PortToHostPort(0), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's Zipkin server")
+	flags.Uint(collectorDynQueueSizeMemory, 0, "(experimental) The max memory size in MiB to use for the dynamic queue.")
 	flags.String(collectorTags, "", "One or more tags to be added to the Process tags of all spans passing through this collector. Ex: key1=value1,key2=${envVar:defaultValue}")
-	flags.Int(collectorZipkinHTTPort, 0, "The HTTP port for the Zipkin collector service e.g. 9411")
 	flags.String(collectorZipkinAllowedOrigins, "*", "Comma separated list of allowed origins for the Zipkin collector service, default accepts all")
 	flags.String(collectorZipkinAllowedHeaders, "content-type", "Comma separated list of allowed headers for the Zipkin collector service, default content-type")
 	tlsFlagsConfig.AddFlags(flags)
@@ -88,8 +100,11 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper) *CollectorOptions {
 	cOpts.NumWorkers = v.GetInt(collectorNumWorkers)
 	cOpts.CollectorHTTPPort = v.GetInt(collectorHTTPPort)
 	cOpts.CollectorGRPCPort = v.GetInt(collectorGRPCPort)
+	cOpts.CollectorHTTPHostPort = v.GetString(collectorHTTPHostPort)
+	cOpts.CollectorGRPCHostPort = v.GetString(collectorGRPCHostPort)
+	cOpts.CollectorZipkinHTTPPort = v.GetInt(collectorZipkinHTTPPort)
+	cOpts.CollectorZipkinHTTPHostPort = v.GetString(collectorZipkinHTTPHostPort)
 	cOpts.CollectorTags = flags.ParseJaegerTags(v.GetString(collectorTags))
-	cOpts.CollectorZipkinHTTPPort = v.GetInt(collectorZipkinHTTPort)
 	cOpts.CollectorZipkinAllowedOrigins = v.GetString(collectorZipkinAllowedOrigins)
 	cOpts.CollectorZipkinAllowedHeaders = v.GetString(collectorZipkinAllowedHeaders)
 	cOpts.TLS = tlsFlagsConfig.InitFromViper(v)

cmd/collector/app/collector.go

@@ -27,6 +27,7 @@ import (
 	"github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore"
 	"github.com/jaegertracing/jaeger/cmd/collector/app/server"
 	"github.com/jaegertracing/jaeger/pkg/healthcheck"
+	"github.com/jaegertracing/jaeger/ports"
 	"github.com/jaegertracing/jaeger/storage/spanstore"
 )
 
@@ -83,7 +84,7 @@ func (c *Collector) Start(builderOpts *CollectorOptions) error {
 	c.spanHandlers = handlerBuilder.BuildHandlers(c.spanProcessor)
 
 	if grpcServer, err := server.StartGRPCServer(&server.GRPCServerParams{
-		Port:          builderOpts.CollectorGRPCPort,
+		HostPort:      getAddressFromCLIOptions(builderOpts.CollectorGRPCPort, builderOpts.CollectorGRPCHostPort, c.logger),
 		Handler:       c.spanHandlers.GRPCHandler,
 		TLSConfig:     builderOpts.TLS,
 		SamplingStore: c.strategyStore,
@@ -95,7 +96,7 @@ func (c *Collector) Start(builderOpts *CollectorOptions) error {
 	}
 
 	if httpServer, err := server.StartHTTPServer(&server.HTTPServerParams{
-		Port:           builderOpts.CollectorHTTPPort,
+		HostPort:       getAddressFromCLIOptions(builderOpts.CollectorHTTPPort, builderOpts.CollectorHTTPHostPort, c.logger),
 		Handler:        c.spanHandlers.JaegerBatchesHandler,
 		HealthCheck:    c.hCheck,
 		MetricsFactory: c.metricsFactory,
@@ -108,7 +109,7 @@ func (c *Collector) Start(builderOpts *CollectorOptions) error {
 	}
 
 	if zkServer, err := server.StartZipkinServer(&server.ZipkinServerParams{
-		Port:           builderOpts.CollectorZipkinHTTPPort,
+		HostPort:       getAddressFromCLIOptions(builderOpts.CollectorZipkinHTTPPort, builderOpts.CollectorZipkinHTTPHostPort, c.logger),
 		Handler:        c.spanHandlers.ZipkinSpansHandler,
 		AllowedHeaders: builderOpts.CollectorZipkinAllowedHeaders,
 		AllowedOrigins: builderOpts.CollectorZipkinAllowedOrigins,
@@ -160,3 +161,12 @@ func (c *Collector) Close() error {
 func (c *Collector) SpanHandlers() *SpanHandlers {
 	return c.spanHandlers
 }
+
+// Utility function to decide listening address based on port (deprecated flags) or host:port (new flags)
+func getAddressFromCLIOptions(port int, hostPort string, logger *zap.Logger) string {
+	if port != 0 {
+		return ports.PortToHostPort(port)
+	}
+
+	return hostPort
+}

cmd/collector/app/server/grpc.go

@@ -17,7 +17,6 @@ package server
 import (
 	"fmt"
 	"net"
-	"strconv"
 
 	"go.uber.org/zap"
 	"google.golang.org/grpc"
@@ -33,7 +32,7 @@ import (
 // GRPCServerParams to construct a new Jaeger Collector gRPC Server
 type GRPCServerParams struct {
 	TLSConfig     tlscfg.Options
-	Port          int
+	HostPort      string
 	Handler       *handler.GRPCHandler
 	SamplingStore strategystore.StrategyStore
 	Logger        *zap.Logger
@@ -58,8 +57,7 @@ func StartGRPCServer(params *GRPCServerParams) (*grpc.Server, error) {
 		server = grpc.NewServer()
 	}
 
-	grpcPortStr := ":" + strconv.Itoa(params.Port)
-	listener, err := net.Listen("tcp", grpcPortStr)
+	listener, err := net.Listen("tcp", params.HostPort)
 	if err != nil {
 		return nil, fmt.Errorf("failed to listen on gRPC port: %w", err)
 	}
@@ -75,7 +73,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.Int("grpc-port", params.Port))
+	params.Logger.Info("Starting jaeger-collector gRPC server", zap.String("grpc.host-port", params.HostPort))
 	go func() {
 		if err := server.Serve(listener); err != nil {
 			params.Logger.Error("Could not launch gRPC service", zap.Error(err))

cmd/collector/app/server/grpc_test.go

@@ -36,7 +36,7 @@ import (
 func TestFailToListen(t *testing.T) {
 	logger, _ := zap.NewDevelopment()
 	server, err := StartGRPCServer(&GRPCServerParams{
-		Port:          -1,
+		HostPort:      ":-1",
 		Handler:       handler.NewGRPCHandler(logger, &mockSpanProcessor{}),
 		SamplingStore: &mockSamplingStore{},
 		Logger:        logger,

cmd/collector/app/server/http.go

@@ -17,7 +17,6 @@ package server
 import (
 	"net"
 	"net/http"
-	"strconv"
 
 	"github.com/gorilla/mux"
 	"github.com/uber/jaeger-lib/metrics"
@@ -32,7 +31,7 @@ import (
 
 // HTTPServerParams to construct a new Jaeger Collector HTTP Server
 type HTTPServerParams struct {
-	Port           int
+	HostPort       string
 	Handler        handler.JaegerBatchesHandler
 	SamplingStore  strategystore.StrategyStore
 	MetricsFactory metrics.Factory
@@ -42,15 +41,14 @@ type HTTPServerParams struct {
 
 // StartHTTPServer based on the given parameters
 func StartHTTPServer(params *HTTPServerParams) (*http.Server, error) {
-	httpPortStr := ":" + strconv.Itoa(params.Port)
-	params.Logger.Info("Starting jaeger-collector HTTP server", zap.String("http-host-port", httpPortStr))
+	params.Logger.Info("Starting jaeger-collector HTTP server", zap.String("http host-port", params.HostPort))
 
-	listener, err := net.Listen("tcp", httpPortStr)
+	listener, err := net.Listen("tcp", params.HostPort)
 	if err != nil {
 		return nil, err
 	}
 
-	server := &http.Server{Addr: httpPortStr}
+	server := &http.Server{Addr: params.HostPort}
 	serveHTTP(server, listener, params)
 
 	return server, nil

cmd/collector/app/server/zipkin.go

@@ -17,7 +17,6 @@ package server
 import (
 	"net"
 	"net/http"
-	"strconv"
 	"strings"
 
 	"github.com/gorilla/mux"
@@ -32,7 +31,7 @@ import (
 
 // ZipkinServerParams to construct a new Jaeger Collector Zipkin Server
 type ZipkinServerParams struct {
-	Port           int
+	HostPort       string
 	Handler        handler.ZipkinSpansHandler
 	AllowedOrigins string
 	AllowedHeaders string
@@ -42,19 +41,18 @@ type ZipkinServerParams struct {
 
 // StartZipkinServer based on the given parameters
 func StartZipkinServer(params *ZipkinServerParams) (*http.Server, error) {
-	if params.Port == 0 {
+	if params.HostPort == "" {
 		return nil, nil
 	}
 
-	httpPortStr := ":" + strconv.Itoa(params.Port)
-	params.Logger.Info("Listening for Zipkin HTTP traffic", zap.Int("zipkin.http-port", params.Port))
+	params.Logger.Info("Listening for Zipkin HTTP traffic", zap.String("zipkin host-port", params.HostPort))
 
-	listener, err := net.Listen("tcp", httpPortStr)
+	listener, err := net.Listen("tcp", params.HostPort)
 	if err != nil {
 		return nil, err
 	}
 
-	server := &http.Server{Addr: httpPortStr}
+	server := &http.Server{Addr: params.HostPort}
 	serveZipkin(server, listener, params)
 
 	return server, nil

cmd/flags/admin.go

@@ -20,26 +20,26 @@ import (
 	"net"
 	"net/http"
 	"net/http/pprof"
-	"strconv"
 
 	"github.com/spf13/viper"
 	"go.uber.org/zap"
 
 	"github.com/jaegertracing/jaeger/pkg/healthcheck"
-	"github.com/jaegertracing/jaeger/pkg/netutils"
 	"github.com/jaegertracing/jaeger/pkg/recoveryhandler"
 	"github.com/jaegertracing/jaeger/pkg/version"
+	"github.com/jaegertracing/jaeger/ports"
 )
 
 const (
-	adminHTTPPort       = "admin-http-port"
 	healthCheckHTTPPort = "health-check-http-port"
+	adminHTTPPort       = "admin-http-port"
+	adminHTTPHostPort   = "admin.http.host-port"
 )
 
 // AdminServer runs an HTTP server with admin endpoints, such as healthcheck at /, /metrics, etc.
 type AdminServer struct {
-	logger    *zap.Logger
-	adminPort int
+	logger        *zap.Logger
+	adminHostPort string
 
 	hc *healthcheck.HealthCheck
 
@@ -48,12 +48,12 @@ type AdminServer struct {
 }
 
 // NewAdminServer creates a new admin server.
-func NewAdminServer(defaultPort int) *AdminServer {
+func NewAdminServer(defaultAddr string) *AdminServer {
 	return &AdminServer{
-		adminPort: defaultPort,
-		logger:    zap.NewNop(),
-		hc:        healthcheck.New(),
-		mux:       http.NewServeMux(),
+		adminHostPort: defaultAddr,
+		logger:        zap.NewNop(),
+		hc:            healthcheck.New(),
+		mux:           http.NewServeMux(),
 	}
 }
 
@@ -70,18 +70,26 @@ func (s *AdminServer) setLogger(logger *zap.Logger) {
 
 // AddFlags registers CLI flags.
 func (s *AdminServer) AddFlags(flagSet *flag.FlagSet) {
-	flagSet.Int(healthCheckHTTPPort, 0, "(deprecated) see --"+adminHTTPPort)
-	flagSet.Int(adminHTTPPort, s.adminPort, "The http port for the admin server, including health check, /metrics, etc.")
+	flagSet.Int(healthCheckHTTPPort, 0, "(deprecated) see --"+adminHTTPHostPort)
+	flagSet.Int(adminHTTPPort, 0, "(deprecated) see --"+adminHTTPHostPort)
+	flagSet.String(adminHTTPHostPort, s.adminHostPort, "The host:port (e.g. 127.0.0.1:5555 or :5555) for the admin server, including health check, /metrics, etc.")
+}
+
+// Util function to check if a deprecated flag is used
+func (s *AdminServer) checkAndUpdate(v *viper.Viper, actualFlagName string, expectedFlagName string) {
+	if v := v.GetInt(actualFlagName); v != 0 {
+		s.logger.Sugar().Warnf("Using deprecated flag %s, please upgrade to %s", actualFlagName, expectedFlagName)
+		s.adminHostPort = ports.PortToHostPort(v)
+	}
 }
 
 // InitFromViper initializes the server with properties retrieved from Viper.
 func (s *AdminServer) initFromViper(v *viper.Viper, logger *zap.Logger) {
 	s.setLogger(logger)
-	s.adminPort = v.GetInt(adminHTTPPort)
-	if v := v.GetInt(healthCheckHTTPPort); v != 0 {
-		logger.Sugar().Warnf("Using deprecated flag %s, please upgrade to %s", healthCheckHTTPPort, adminHTTPPort)
-		s.adminPort = v
-	}
+
+	s.adminHostPort = v.GetString(adminHTTPHostPort)
+	s.checkAndUpdate(v, healthCheckHTTPPort, adminHTTPHostPort)
+	s.checkAndUpdate(v, adminHTTPPort, adminHTTPHostPort)
 }
 
 // Handle adds a new handler to the admin server.
@@ -91,22 +99,16 @@ func (s *AdminServer) Handle(path string, handler http.Handler) {
 
 // Serve starts HTTP server.
 func (s *AdminServer) Serve() error {
-	portStr := ":" + strconv.Itoa(s.adminPort)
-	l, err := net.Listen("tcp", portStr)
+	l, err := net.Listen("tcp", s.adminHostPort)
 	if err != nil {
 		s.logger.Error("Admin server failed to listen", zap.Error(err))
 		return err
 	}
 	s.serveWithListener(l)
 
-	tcpPort := s.adminPort
-	if port, err := netutils.GetPort(l.Addr()); err == nil {
-		tcpPort = port
-	}
-
 	s.logger.Info(
 		"Admin server started",
-		zap.Int("http-port", tcpPort),
+		zap.String("http.host-port", s.adminHostPort),
 		zap.Stringer("health-status", s.hc.Get()))
 	return nil
 }
@@ -118,7 +120,7 @@ func (s *AdminServer) serveWithListener(l net.Listener) {
 	s.registerPprofHandlers()
 	recoveryHandler := recoveryhandler.NewRecoveryHandler(s.logger, true)
 	s.server = &http.Server{Handler: recoveryHandler(s.mux)}
-	s.logger.Info("Starting admin HTTP server", zap.Int("http-port", s.adminPort))
+	s.logger.Info("Starting admin HTTP server", zap.String("http-addr", s.adminHostPort))
 	go func() {
 		switch err := s.server.Serve(l); err {
 		case nil, http.ErrServerClosed:

cmd/flags/admin_test.go

@@ -25,7 +25,7 @@ import (
 )
 
 func TestAdminServerHandlesPortZero(t *testing.T) {
-	adminServer := NewAdminServer(0)
+	adminServer := NewAdminServer(":0")
 
 	v, _ := config.Viperize(adminServer.AddFlags)
 

cmd/flags/service.go

@@ -28,6 +28,7 @@ import (
 
 	"github.com/jaegertracing/jaeger/pkg/healthcheck"
 	pMetrics "github.com/jaegertracing/jaeger/pkg/metrics"
+	"github.com/jaegertracing/jaeger/ports"
 )
 
 // Service represents an abstract Jaeger backend component with some basic shared functionality.
@@ -59,7 +60,7 @@ func NewService(adminPort int) *Service {
 	signal.Notify(signalsChannel, os.Interrupt, syscall.SIGTERM)
 
 	return &Service{
-		Admin:           NewAdminServer(adminPort),
+		Admin:           NewAdminServer(ports.PortToHostPort(adminPort)),
 		signalsChannel:  signalsChannel,
 		hcStatusChannel: hcStatusChannel,
 	}