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,12 @@ const (
 	collectorNumWorkers           = "collector.num-workers"
 	collectorHTTPPort             = "collector.http-port"
 	collectorGRPCPort             = "collector.grpc-port"
+	collectorTChanHostPort        = "collector.tchan-server.host-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 +59,20 @@ type CollectorOptions struct {
 	CollectorHTTPPort int
 	// CollectorGRPCPort is the port that the collector service listens in on for gRPC requests
 	CollectorGRPCPort int
+	// CollectorTChanHostPort is the host:port address that the collector service listens in on for tchannel requests
+	CollectorTChanHostPort string
+	// 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 +81,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 +103,12 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper) *CollectorOptions {
 	cOpts.NumWorkers = v.GetInt(collectorNumWorkers)
 	cOpts.CollectorHTTPPort = v.GetInt(collectorHTTPPort)
 	cOpts.CollectorGRPCPort = v.GetInt(collectorGRPCPort)
+	cOpts.CollectorTChanHostPort = v.GetString(collectorTChanHostPort)
+	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/grpcserver/grpc_server.go

@@ -0,0 +1,70 @@
+// Copyright (c) 2018 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 grpcserver
+
+import (
+	"io/ioutil"
+	"net"
+	"os"
+	"strconv"
+
+	"github.com/pkg/errors"
+	"go.uber.org/zap"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/grpclog"
+
+	"github.com/jaegertracing/jaeger/cmd/collector/app"
+	"github.com/jaegertracing/jaeger/cmd/collector/app/sampling"
+	"github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore"
+	"github.com/jaegertracing/jaeger/proto-gen/api_v2"
+)
+
+// StartGRPCCollector configures and starts gRPC endpoints exposed by collector.
+func StartGRPCCollector(
+	addr string,
+	server *grpc.Server,
+	handler *app.GRPCHandler,
+	samplingStrategy strategystore.StrategyStore,
+	logger *zap.Logger,
+	serveErr func(error),
+) (net.Addr, error) {
+	lis, err := net.Listen("tcp", addr)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to listen on gRPC port")
+	}
+
+	grpclog.SetLoggerV2(grpclog.NewLoggerV2(ioutil.Discard, os.Stderr, os.Stderr))
+
+	api_v2.RegisterCollectorServiceServer(server, handler)
+	api_v2.RegisterSamplingManagerServer(server, sampling.NewGRPCHandler(samplingStrategy))
+	startServer(server, lis, logger, serveErr)
+	return lis.Addr(), nil
+}
+
+func startServer(server *grpc.Server, lis net.Listener, logger *zap.Logger, serveErr func(error)) {
+	var port string
+	if tcpAddr, ok := lis.Addr().(*net.TCPAddr); ok {
+		port = strconv.Itoa(tcpAddr.Port)
+	} else {
+		port = lis.Addr().Network()
+	}
+	logger.Info("Starting jaeger-collector gRPC server", zap.String("grpc-port", port))
+	go func() {
+		if err := server.Serve(lis); err != nil {
+			logger.Error("Could not launch gRPC service", zap.Error(err))
+			serveErr(err)
+		}
+	}()
+}

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