From 87b191027f73d6301d21ec2bd5f76f30560b0fa7 Mon Sep 17 00:00:00 2001 From: Annanay Agarwal Date: Wed, 1 Apr 2020 00:16:21 +0530 Subject: [PATCH] Normalize CLI flags to use host:port addresses (#1827) * Normalize CLI flags to use host:port addresses Signed-off-by: Annanay * Addressed review comments Signed-off-by: Annanay * Continue support for deprecated flags in all-in-one Signed-off-by: Annanay * Nitpick, fix agent options in all-in-one Signed-off-by: Annanay * Addressed comments, use HostPort in variable names Signed-off-by: Annanay * Address comments Signed-off-by: Annanay * Add Changelog entries Signed-off-by: Annanay * Fix merge changes Signed-off-by: Annanay * Checkpoint Signed-off-by: Annanay Agarwal * Rebased and addressed comments Signed-off-by: Annanay * Remove TChan ports, remove leftover files from merge Signed-off-by: Annanay * Address comments Signed-off-by: Annanay * Fix go.mod, edit util function name Signed-off-by: Annanay * Fix test, print address in log Signed-off-by: Annanay * Make fmt Signed-off-by: Annanay * Add small test Signed-off-by: Yuri Shkuro * Another silly test Signed-off-by: Yuri Shkuro Co-authored-by: Annanay Co-authored-by: Annanay Agarwal Co-authored-by: Yuri Shkuro --- CHANGELOG.md | 10 +++++ cmd/all-in-one/main.go | 2 +- cmd/collector/app/builder_flags.go | 52 ++++++++++++++++------ cmd/collector/app/builder_flags_test.go | 57 +++++++++++++++++++++++++ cmd/collector/app/collector.go | 6 +-- cmd/collector/app/server/grpc.go | 8 ++-- cmd/collector/app/server/grpc_test.go | 2 +- cmd/collector/app/server/http.go | 10 ++--- cmd/collector/app/server/zipkin.go | 12 +++--- cmd/flags/admin.go | 57 ++++++++++++++----------- cmd/flags/admin_test.go | 9 ++-- cmd/flags/service.go | 3 +- ports/ports.go | 9 ++++ ports/{empty_test.go => ports_test.go} | 12 +++++- 14 files changed, 181 insertions(+), 68 deletions(-) create mode 100644 cmd/collector/app/builder_flags_test.go rename ports/{empty_test.go => ports_test.go} (74%) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1ab700ebfa..fceaf944a84 100644 --- a/CHANGELOG.md +++ b/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)) + + Flags previously accepting listen addresses in any other format have been deprecated: + + * `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` + * `admin-http-port` is superseded by `admin.http.host-port` + #### New Features #### Bug fixes, Minor Improvements diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 0a9b79ed01f..bce191d0c81 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -130,7 +130,7 @@ by default uses only in-memory database.`, c.Start(cOpts) // agent - grpcBuilder.CollectorHostPorts = append(grpcBuilder.CollectorHostPorts, fmt.Sprintf("127.0.0.1:%d", cOpts.CollectorGRPCPort)) + grpcBuilder.CollectorHostPorts = append(grpcBuilder.CollectorHostPorts, cOpts.CollectorGRPCHostPort) agentMetricsFactory := metricsFactory.Namespace(metrics.NSOptions{Name: "agent", Tags: nil}) builders := map[agentRep.Type]agentApp.CollectorProxyBuilder{ agentRep.GRPC: agentApp.GRPCCollectorProxyBuilder(grpcBuilder), diff --git a/cmd/collector/app/builder_flags.go b/cmd/collector/app/builder_flags.go index e725a35dfba..06f2768e67e 100644 --- a/cmd/collector/app/builder_flags.go +++ b/cmd/collector/app/builder_flags.go @@ -17,6 +17,7 @@ package app import ( "flag" + "strings" "github.com/spf13/viper" @@ -31,10 +32,17 @@ 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" + + collectorHTTPPortWarning = "(deprecated, will be removed after 2020-06-30 or in release v1.20.0, whichever is later)" + collectorGRPCPortWarning = "(deprecated, will be removed after 2020-06-30 or in release v1.20.0, whichever is later)" + collectorZipkinHTTPPortWarning = "(deprecated, will be removed after 2020-06-30 or in release v1.20.0, whichever is later)" ) var tlsFlagsConfig = tlscfg.ServerFlagsConfig{ @@ -51,16 +59,16 @@ type CollectorOptions struct { QueueSize int // NumWorkers is the number of internal workers in a collector NumWorkers int - // CollectorHTTPPort is the port that the collector service listens in on for http requests - 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 +77,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, collectorHTTPPortWarning+" see --"+collectorHTTPHostPort) + flags.Int(collectorGRPCPort, 0, collectorGRPCPortWarning+" see --"+collectorGRPCHostPort) + flags.Int(collectorZipkinHTTPPort, 0, collectorZipkinHTTPPortWarning+" see --"+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) @@ -86,12 +97,25 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper) *CollectorOptions { cOpts.DynQueueSizeMemory = v.GetUint(collectorDynQueueSizeMemory) * 1024 * 1024 // we receive in MiB and store in bytes cOpts.QueueSize = v.GetInt(collectorQueueSize) cOpts.NumWorkers = v.GetInt(collectorNumWorkers) - cOpts.CollectorHTTPPort = v.GetInt(collectorHTTPPort) - cOpts.CollectorGRPCPort = v.GetInt(collectorGRPCPort) + cOpts.CollectorHTTPHostPort = getAddressFromCLIOptions(v.GetInt(collectorHTTPPort), v.GetString(collectorHTTPHostPort)) + cOpts.CollectorGRPCHostPort = getAddressFromCLIOptions(v.GetInt(collectorGRPCPort), v.GetString(collectorGRPCHostPort)) + cOpts.CollectorZipkinHTTPHostPort = getAddressFromCLIOptions(v.GetInt(collectorZipkinHTTPPort), 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) return cOpts } + +// Utility function to get listening address based on port (deprecated flags) or host:port (new flags) +func getAddressFromCLIOptions(port int, hostPort string) string { + if port != 0 { + return ports.PortToHostPort(port) + } + + if strings.Contains(hostPort, ":") { + return hostPort + } + + return ":" + hostPort +} diff --git a/cmd/collector/app/builder_flags_test.go b/cmd/collector/app/builder_flags_test.go new file mode 100644 index 00000000000..4ee34b0f152 --- /dev/null +++ b/cmd/collector/app/builder_flags_test.go @@ -0,0 +1,57 @@ +// Copyright (c) 2020 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 app + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/jaegertracing/jaeger/pkg/config" +) + +func TestCollectorOptionsWithFlags_CheckHostPort(t *testing.T) { + c := &CollectorOptions{} + v, command := config.Viperize(AddFlags) + command.ParseFlags([]string{ + "--collector.http-server.host-port=5678", + "--collector.grpc-server.host-port=1234", + "--collector.zipkin.host-port=3456", + }) + c.InitFromViper(v) + + assert.Equal(t, ":5678", c.CollectorHTTPHostPort) + assert.Equal(t, ":1234", c.CollectorGRPCHostPort) + assert.Equal(t, ":3456", c.CollectorZipkinHTTPHostPort) +} + +func TestCollectorOptionsWithFlags_CheckFullHostPort(t *testing.T) { + c := &CollectorOptions{} + v, command := config.Viperize(AddFlags) + command.ParseFlags([]string{ + "--collector.http-server.host-port=:5678", + "--collector.grpc-server.host-port=127.0.0.1:1234", + "--collector.zipkin.host-port=0.0.0.0:3456", + }) + c.InitFromViper(v) + + assert.Equal(t, ":5678", c.CollectorHTTPHostPort) + assert.Equal(t, "127.0.0.1:1234", c.CollectorGRPCHostPort) + assert.Equal(t, "0.0.0.0:3456", c.CollectorZipkinHTTPHostPort) +} + +func Test_getAddressFromCLIOptions(t *testing.T) { + assert.Equal(t, ":123", getAddressFromCLIOptions(123, "")) +} diff --git a/cmd/collector/app/collector.go b/cmd/collector/app/collector.go index 7af28c001a4..29e191043c5 100644 --- a/cmd/collector/app/collector.go +++ b/cmd/collector/app/collector.go @@ -83,7 +83,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: builderOpts.CollectorGRPCHostPort, Handler: c.spanHandlers.GRPCHandler, TLSConfig: builderOpts.TLS, SamplingStore: c.strategyStore, @@ -95,7 +95,7 @@ func (c *Collector) Start(builderOpts *CollectorOptions) error { } if httpServer, err := server.StartHTTPServer(&server.HTTPServerParams{ - Port: builderOpts.CollectorHTTPPort, + HostPort: builderOpts.CollectorHTTPHostPort, Handler: c.spanHandlers.JaegerBatchesHandler, HealthCheck: c.hCheck, MetricsFactory: c.metricsFactory, @@ -108,7 +108,7 @@ func (c *Collector) Start(builderOpts *CollectorOptions) error { } if zkServer, err := server.StartZipkinServer(&server.ZipkinServerParams{ - Port: builderOpts.CollectorZipkinHTTPPort, + HostPort: builderOpts.CollectorZipkinHTTPHostPort, Handler: c.spanHandlers.ZipkinSpansHandler, AllowedHeaders: builderOpts.CollectorZipkinAllowedHeaders, AllowedOrigins: builderOpts.CollectorZipkinAllowedOrigins, diff --git a/cmd/collector/app/server/grpc.go b/cmd/collector/app/server/grpc.go index c487b05cadf..ce68e1cf5c1 100644 --- a/cmd/collector/app/server/grpc.go +++ b/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)) diff --git a/cmd/collector/app/server/grpc_test.go b/cmd/collector/app/server/grpc_test.go index 4629452896e..c13ea8c11e6 100644 --- a/cmd/collector/app/server/grpc_test.go +++ b/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, diff --git a/cmd/collector/app/server/http.go b/cmd/collector/app/server/http.go index 6f1a963e0c0..3a5dd7aad6f 100644 --- a/cmd/collector/app/server/http.go +++ b/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 diff --git a/cmd/collector/app/server/zipkin.go b/cmd/collector/app/server/zipkin.go index c779e931171..70faf00f0ae 100644 --- a/cmd/collector/app/server/zipkin.go +++ b/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 diff --git a/cmd/flags/admin.go b/cmd/flags/admin.go index cda3f0447c4..fab53f2afa7 100644 --- a/cmd/flags/admin.go +++ b/cmd/flags/admin.go @@ -20,26 +20,29 @@ 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" + + healthCheckHTTPPortWarning = "(deprecated, will be removed after 2020-03-15 or in release v1.19.0, whichever is later)" + adminHTTPPortWarning = "(deprecated, will be removed after 2020-06-30 or in release v1.20.0, whichever is later)" ) // 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 +51,12 @@ type AdminServer struct { } // NewAdminServer creates a new admin server. -func NewAdminServer(defaultPort int) *AdminServer { +func NewAdminServer(hostPort string) *AdminServer { return &AdminServer{ - adminPort: defaultPort, - logger: zap.NewNop(), - hc: healthcheck.New(), - mux: http.NewServeMux(), + adminHostPort: hostPort, + logger: zap.NewNop(), + hc: healthcheck.New(), + mux: http.NewServeMux(), } } @@ -70,18 +73,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, healthCheckHTTPPortWarning+" see --"+adminHTTPHostPort) + flagSet.Int(adminHTTPPort, 0, adminHTTPPortWarning+" 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 use deprecated flag value if specified +func (s *AdminServer) checkDeprecatedFlag(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.checkDeprecatedFlag(v, healthCheckHTTPPort, adminHTTPHostPort) + s.checkDeprecatedFlag(v, adminHTTPPort, adminHTTPHostPort) } // Handle adds a new handler to the admin server. @@ -91,22 +102,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", l.Addr().String()), zap.Stringer("health-status", s.hc.Get())) return nil } @@ -118,7 +123,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: diff --git a/cmd/flags/admin_test.go b/cmd/flags/admin_test.go index 6d6cef71700..b3564b41bcc 100644 --- a/cmd/flags/admin_test.go +++ b/cmd/flags/admin_test.go @@ -15,6 +15,8 @@ package flags import ( + "strconv" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -25,7 +27,7 @@ import ( ) func TestAdminServerHandlesPortZero(t *testing.T) { - adminServer := NewAdminServer(0) + adminServer := NewAdminServer(":0") v, _ := config.Viperize(adminServer.AddFlags) @@ -41,6 +43,7 @@ func TestAdminServerHandlesPortZero(t *testing.T) { assert.Equal(t, 1, message.Len(), "Expected Admin server started log message.") onlyEntry := message.All()[0] - port := onlyEntry.ContextMap()["http-port"].(int64) - assert.Greater(t, port, int64(0)) + hostPort := onlyEntry.ContextMap()["http.host-port"].(string) + port, _ := strconv.Atoi(strings.Split(hostPort, ":")[3]) + assert.Greater(t, port, 0) } diff --git a/cmd/flags/service.go b/cmd/flags/service.go index 2381e6037b7..d6b02cf3a53 100644 --- a/cmd/flags/service.go +++ b/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, } diff --git a/ports/ports.go b/ports/ports.go index be94961e300..cba71cbbb11 100644 --- a/ports/ports.go +++ b/ports/ports.go @@ -14,6 +14,10 @@ package ports +import ( + "strconv" +) + const ( // AgentJaegerThriftCompactUDP is the default port for receiving Jaeger Thrift over UDP in compact encoding AgentJaegerThriftCompactUDP = 6831 @@ -41,3 +45,8 @@ const ( // IngesterAdminHTTP is the default admin HTTP port (health check, metrics, etc.) IngesterAdminHTTP = 14270 ) + +// PortToHostPort converts the port into a host:port address string +func PortToHostPort(port int) string { + return ":" + strconv.Itoa(port) +} diff --git a/ports/empty_test.go b/ports/ports_test.go similarity index 74% rename from ports/empty_test.go rename to ports/ports_test.go index b3f548cb657..29ff987d8a7 100644 --- a/ports/empty_test.go +++ b/ports/ports_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2019 The Jaeger Authors. +// Copyright (c) 2020 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. @@ -13,3 +13,13 @@ // limitations under the License. package ports + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestPortToHostPort(t *testing.T) { + assert.Equal(t, ":42", PortToHostPort(42)) +}