From 366f27917195cba4a9c49afb82bd453caec22353 Mon Sep 17 00:00:00 2001 From: Annanay Date: Wed, 2 Oct 2019 20:43:28 +0530 Subject: [PATCH 01/17] Normalize CLI flags to use host:port addresses Signed-off-by: Annanay --- cmd/all-in-one/main.go | 30 ++++++------- cmd/collector/app/builder/builder_flags.go | 42 +++++++++---------- cmd/collector/app/grpcserver/grpc_server.go | 5 +-- .../app/grpcserver/grpc_server_test.go | 7 ++-- cmd/collector/main.go | 24 +++++------ cmd/flags/admin.go | 27 ++++++------ cmd/flags/service.go | 3 +- ports/ports.go | 9 ++++ 8 files changed, 74 insertions(+), 73 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 31cf80408f4..52a6e6bf271 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -22,7 +22,6 @@ import ( "net" "net/http" "os" - "strconv" "github.com/gorilla/mux" "github.com/opentracing/opentracing-go" @@ -185,7 +184,7 @@ func startAgent( ) { metricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "agent", Tags: nil}) - grpcBuilder.CollectorHostPorts = append(grpcBuilder.CollectorHostPorts, fmt.Sprintf("127.0.0.1:%d", cOpts.CollectorGRPCPort)) + grpcBuilder.CollectorHostPorts = append(grpcBuilder.CollectorHostPorts, cOpts.CollectorGRPCAddr) cp, err := agentApp.CreateCollectorProxy(repOpts, tchanBuilder, grpcBuilder, logger, metricsFactory) if err != nil { logger.Fatal("Could not create collector proxy", zap.Error(err)) @@ -234,16 +233,15 @@ func startCollector( server.Register(jc.NewTChanCollectorServer(batchHandler)) server.Register(zc.NewTChanZipkinCollectorServer(batchHandler)) server.Register(sc.NewTChanSamplingManagerServer(sampling.NewHandler(strategyStore))) - portStr := ":" + strconv.Itoa(cOpts.CollectorPort) - listener, err := net.Listen("tcp", portStr) + listener, err := net.Listen("tcp", cOpts.CollectorTChanAddr) if err != nil { logger.Fatal("Unable to start listening on channel", zap.Error(err)) } - logger.Info("Starting jaeger-collector TChannel server", zap.Int("port", cOpts.CollectorPort)) + logger.Info("Starting jaeger-collector TChannel server", zap.String("addr", cOpts.CollectorTChanAddr)) ch.Serve(listener) } - server, err := startGRPCServer(cOpts.CollectorGRPCPort, grpcHandler, strategyStore, logger) + server, err := startGRPCServer(cOpts.CollectorGRPCAddr, grpcHandler, strategyStore, logger) if err != nil { logger.Fatal("Could not start gRPC collector", zap.Error(err)) } @@ -252,14 +250,13 @@ func startCollector( r := mux.NewRouter() apiHandler := collectorApp.NewAPIHandler(jaegerBatchesHandler) apiHandler.RegisterRoutes(r) - httpPortStr := ":" + strconv.Itoa(cOpts.CollectorHTTPPort) recoveryHandler := recoveryhandler.NewRecoveryHandler(logger, true) - go startZipkinHTTPAPI(logger, cOpts.CollectorZipkinHTTPPort, zipkinSpansHandler, recoveryHandler) + go startZipkinHTTPAPI(logger, cOpts.CollectorZipkinHTTPAddr, zipkinSpansHandler, recoveryHandler) - logger.Info("Starting jaeger-collector HTTP server", zap.Int("http-port", cOpts.CollectorHTTPPort)) + logger.Info("Starting jaeger-collector HTTP server", zap.String("http-addr", cOpts.CollectorHTTPAddr)) go func() { - if err := http.ListenAndServe(httpPortStr, recoveryHandler(r)); err != nil { + if err := http.ListenAndServe(cOpts.CollectorHTTPAddr, recoveryHandler(r)); err != nil { logger.Fatal("Could not launch jaeger-collector HTTP server", zap.Error(err)) } hc.Set(healthcheck.Unavailable) @@ -269,13 +266,13 @@ func startCollector( } func startGRPCServer( - port int, + addr string, handler *collectorApp.GRPCHandler, samplingStore strategystore.StrategyStore, logger *zap.Logger, ) (*grpc.Server, error) { server := grpc.NewServer() - _, err := grpcserver.StartGRPCCollector(port, server, handler, samplingStore, logger, func(err error) { + _, err := grpcserver.StartGRPCCollector(addr, server, handler, samplingStore, logger, func(err error) { logger.Fatal("gRPC collector failed", zap.Error(err)) }) if err != nil { @@ -286,18 +283,17 @@ func startGRPCServer( func startZipkinHTTPAPI( logger *zap.Logger, - zipkinPort int, + zipkinAddr string, zipkinSpansHandler collectorApp.ZipkinSpansHandler, recoveryHandler func(http.Handler) http.Handler, ) { - if zipkinPort != 0 { + if zipkinAddr != ":0" { r := mux.NewRouter() zHandler := zipkin.NewAPIHandler(zipkinSpansHandler) zHandler.RegisterRoutes(r) - httpPortStr := ":" + strconv.Itoa(zipkinPort) - logger.Info("Listening for Zipkin HTTP traffic", zap.Int("zipkin.http-port", zipkinPort)) + logger.Info("Listening for Zipkin HTTP traffic", zap.String("zipkin.http-addr", zipkinAddr)) - if err := http.ListenAndServe(httpPortStr, recoveryHandler(r)); err != nil { + if err := http.ListenAndServe(zipkinAddr, recoveryHandler(r)); err != nil { logger.Fatal("Could not launch service", zap.Error(err)) } } diff --git a/cmd/collector/app/builder/builder_flags.go b/cmd/collector/app/builder/builder_flags.go index 9787c3b04ca..2954dcf42ac 100644 --- a/cmd/collector/app/builder/builder_flags.go +++ b/cmd/collector/app/builder/builder_flags.go @@ -27,14 +27,14 @@ import ( const ( collectorQueueSize = "collector.queue-size" collectorNumWorkers = "collector.num-workers" - collectorPort = "collector.port" - collectorHTTPPort = "collector.http-port" - collectorGRPCPort = "collector.grpc-port" + collectorTChanAddr = "collector.tchan-addr" + collectorHTTPAddr = "collector.http-addr" + collectorGRPCAddr = "collector.grpc-addr" collectorGRPCTLS = "collector.grpc.tls" collectorGRPCCert = "collector.grpc.tls.cert" collectorGRPCKey = "collector.grpc.tls.key" collectorGRPCClientCA = "collector.grpc.tls.client.ca" - collectorZipkinHTTPort = "collector.zipkin.http-port" + collectorZipkinHTTPAddr = "collector.zipkin.http-addr" collectorZipkinAllowedOrigins = "collector.zipkin.allowed-origins" collectorZipkinAllowedHeaders = "collector.zipkin.allowed-headers" ) @@ -45,12 +45,12 @@ type CollectorOptions struct { QueueSize int // NumWorkers is the number of internal workers in a collector NumWorkers int - // CollectorPort is the port that the collector service listens in on for tchannel requests - CollectorPort 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 + // CollectorTChanAddr is the host:port address that the collector service listens in on for tchannel requests + CollectorTChanAddr string + // CollectorHTTPAddr is the host:port address that the collector service listens in on for http requests + CollectorHTTPAddr string + // CollectorGRPCAddr is the host:port address that the collector service listens in on for gRPC requests + CollectorGRPCAddr string // CollectorGRPCTLS defines if the server is setup with TLS CollectorGRPCTLS bool // CollectorGRPCCert is the path to a TLS certificate file for the server @@ -59,8 +59,8 @@ type CollectorOptions struct { CollectorGRPCClientCA string // CollectorGRPCKey is the path to a TLS key file for the server CollectorGRPCKey string - // CollectorZipkinHTTPPort is the port that the Zipkin collector service listens in on for http requests - CollectorZipkinHTTPPort int + // CollectorZipkinHTTPAddr is the host:port address that the Zipkin collector service listens in on for http requests + CollectorZipkinHTTPAddr 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 @@ -71,11 +71,11 @@ type CollectorOptions struct { func AddFlags(flags *flag.FlagSet) { flags.Int(collectorQueueSize, app.DefaultQueueSize, "The queue size of the collector") flags.Int(collectorNumWorkers, app.DefaultNumWorkers, "The number of workers pulling items from the queue") - flags.Int(collectorPort, ports.CollectorTChannel, "The TChannel port for the collector service") - 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(collectorZipkinHTTPort, 0, "The HTTP port for the Zipkin collector service e.g. 9411") - flags.Bool(collectorGRPCTLS, false, "Enable TLS for the gRPC collector port") + flags.String(collectorTChanAddr, ports.GetAddressFromPort(ports.CollectorTChannel), "The TChannel address for the collector service") + flags.String(collectorHTTPAddr, ports.GetAddressFromPort(ports.CollectorHTTP), "The HTTP address for the collector service") + flags.String(collectorGRPCAddr, ports.GetAddressFromPort(ports.CollectorGRPC), "The gRPC address for the collector service") + flags.String(collectorZipkinHTTPAddr, ports.GetAddressFromPort(0), "The HTTP address for the Zipkin collector service") + flags.Bool(collectorGRPCTLS, false, "Enable TLS for the gRPC collector endpoint") flags.String(collectorGRPCCert, "", "Path to TLS certificate for the gRPC collector TLS service") flags.String(collectorGRPCKey, "", "Path to TLS key for the gRPC collector TLS cert") flags.String(collectorGRPCClientCA, "", "Path to a TLS CA to verify certificates presented by clients (if unset, all clients are permitted)") @@ -87,14 +87,14 @@ func AddFlags(flags *flag.FlagSet) { func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper) *CollectorOptions { cOpts.QueueSize = v.GetInt(collectorQueueSize) cOpts.NumWorkers = v.GetInt(collectorNumWorkers) - cOpts.CollectorPort = v.GetInt(collectorPort) - cOpts.CollectorHTTPPort = v.GetInt(collectorHTTPPort) - cOpts.CollectorGRPCPort = v.GetInt(collectorGRPCPort) + cOpts.CollectorTChanAddr = v.GetString(collectorTChanAddr) + cOpts.CollectorHTTPAddr = v.GetString(collectorHTTPAddr) + cOpts.CollectorGRPCAddr = v.GetString(collectorGRPCAddr) cOpts.CollectorGRPCTLS = v.GetBool(collectorGRPCTLS) cOpts.CollectorGRPCCert = v.GetString(collectorGRPCCert) cOpts.CollectorGRPCClientCA = v.GetString(collectorGRPCClientCA) cOpts.CollectorGRPCKey = v.GetString(collectorGRPCKey) - cOpts.CollectorZipkinHTTPPort = v.GetInt(collectorZipkinHTTPort) + cOpts.CollectorZipkinHTTPAddr = v.GetString(collectorZipkinHTTPAddr) cOpts.CollectorZipkinAllowedOrigins = v.GetString(collectorZipkinAllowedOrigins) cOpts.CollectorZipkinAllowedHeaders = v.GetString(collectorZipkinAllowedHeaders) return cOpts diff --git a/cmd/collector/app/grpcserver/grpc_server.go b/cmd/collector/app/grpcserver/grpc_server.go index 5150c1e4e84..c91f6e08d85 100644 --- a/cmd/collector/app/grpcserver/grpc_server.go +++ b/cmd/collector/app/grpcserver/grpc_server.go @@ -71,15 +71,14 @@ func TLSConfig(cert, key, clientCA string) (*tls.Config, error) { // StartGRPCCollector configures and starts gRPC endpoints exposed by collector. func StartGRPCCollector( - port int, + addr string, server *grpc.Server, handler *app.GRPCHandler, samplingStrategy strategystore.StrategyStore, logger *zap.Logger, serveErr func(error), ) (net.Addr, error) { - grpcPortStr := ":" + strconv.Itoa(port) - lis, err := net.Listen("tcp", grpcPortStr) + lis, err := net.Listen("tcp", addr) if err != nil { return nil, errors.Wrap(err, "failed to listen on gRPC port") } diff --git a/cmd/collector/app/grpcserver/grpc_server_test.go b/cmd/collector/app/grpcserver/grpc_server_test.go index 0e7db20a4a1..d9b90f138b4 100644 --- a/cmd/collector/app/grpcserver/grpc_server_test.go +++ b/cmd/collector/app/grpcserver/grpc_server_test.go @@ -29,6 +29,7 @@ import ( "github.com/jaegertracing/jaeger/cmd/collector/app" "github.com/jaegertracing/jaeger/model" + "github.com/jaegertracing/jaeger/ports" "github.com/jaegertracing/jaeger/proto-gen/api_v2" "github.com/jaegertracing/jaeger/thrift-gen/sampling" ) @@ -38,8 +39,8 @@ func TestFailToListen(t *testing.T) { l, _ := zap.NewDevelopment() handler := app.NewGRPCHandler(l, &mockSpanProcessor{}) server := grpc.NewServer() - const invalidPort = -1 - addr, err := StartGRPCCollector(invalidPort, server, handler, &mockSamplingStore{}, l, func(e error) { + const invalidAddr = ":-1" + addr, err := StartGRPCCollector(invalidAddr, server, handler, &mockSamplingStore{}, l, func(e error) { }) assert.Nil(t, addr) assert.EqualError(t, err, "failed to listen on gRPC port: listen tcp: address -1: invalid port") @@ -63,7 +64,7 @@ func TestSpanCollector(t *testing.T) { l, _ := zap.NewDevelopment() handler := app.NewGRPCHandler(l, &mockSpanProcessor{}) server := grpc.NewServer() - addr, err := StartGRPCCollector(0, server, handler, &mockSamplingStore{}, l, func(e error) { + addr, err := StartGRPCCollector(ports.GetAddressFromPort(0), server, handler, &mockSamplingStore{}, l, func(e error) { }) require.NoError(t, err) diff --git a/cmd/collector/main.go b/cmd/collector/main.go index 0e7c818e7c8..a574dec769a 100644 --- a/cmd/collector/main.go +++ b/cmd/collector/main.go @@ -22,7 +22,6 @@ import ( "net" "net/http" "os" - "strconv" "strings" "github.com/gorilla/mux" @@ -120,12 +119,11 @@ func main() { server.Register(jc.NewTChanCollectorServer(batchHandler)) server.Register(zc.NewTChanZipkinCollectorServer(batchHandler)) server.Register(sc.NewTChanSamplingManagerServer(sampling.NewHandler(strategyStore))) - portStr := ":" + strconv.Itoa(builderOpts.CollectorPort) - listener, err := net.Listen("tcp", portStr) + listener, err := net.Listen("tcp", builderOpts.CollectorTChanAddr) if err != nil { logger.Fatal("Unable to start listening on channel", zap.Error(err)) } - logger.Info("Starting jaeger-collector TChannel server", zap.Int("port", builderOpts.CollectorPort)) + logger.Info("Starting jaeger-collector TChannel server", zap.String("tchan-addr", builderOpts.CollectorTChanAddr)) ch.Serve(listener) } @@ -138,15 +136,14 @@ func main() { r := mux.NewRouter() apiHandler := app.NewAPIHandler(jaegerBatchesHandler) apiHandler.RegisterRoutes(r) - httpPortStr := ":" + strconv.Itoa(builderOpts.CollectorHTTPPort) recoveryHandler := recoveryhandler.NewRecoveryHandler(logger, true) httpHandler := recoveryHandler(r) - go startZipkinHTTPAPI(logger, builderOpts.CollectorZipkinHTTPPort, builderOpts.CollectorZipkinAllowedOrigins, builderOpts.CollectorZipkinAllowedHeaders, zipkinSpansHandler, recoveryHandler) + go startZipkinHTTPAPI(logger, builderOpts.CollectorZipkinHTTPAddr, builderOpts.CollectorZipkinAllowedOrigins, builderOpts.CollectorZipkinAllowedHeaders, zipkinSpansHandler, recoveryHandler) - logger.Info("Starting jaeger-collector HTTP server", zap.Int("http-port", builderOpts.CollectorHTTPPort)) + logger.Info("Starting jaeger-collector HTTP server", zap.String("http-addr", builderOpts.CollectorHTTPAddr)) go func() { - if err := http.ListenAndServe(httpPortStr, httpHandler); err != nil { + if err := http.ListenAndServe(builderOpts.CollectorHTTPAddr, httpHandler); err != nil { logger.Fatal("Could not launch service", zap.Error(err)) } svc.HC().Set(healthcheck.Unavailable) @@ -207,7 +204,7 @@ func startGRPCServer( } else { // server without TLS server = grpc.NewServer() } - _, err := grpcserver.StartGRPCCollector(opts.CollectorGRPCPort, server, handler, samplingStore, logger, func(err error) { + _, err := grpcserver.StartGRPCCollector(opts.CollectorGRPCAddr, server, handler, samplingStore, logger, func(err error) { logger.Fatal("gRPC collector failed", zap.Error(err)) }) if err != nil { @@ -218,13 +215,13 @@ func startGRPCServer( func startZipkinHTTPAPI( logger *zap.Logger, - zipkinPort int, + zipkinAddr string, allowedOrigins string, allowedHeaders string, zipkinSpansHandler app.ZipkinSpansHandler, recoveryHandler func(http.Handler) http.Handler, ) { - if zipkinPort != 0 { + if zipkinAddr != ":0" { zHandler := zipkin.NewAPIHandler(zipkinSpansHandler) r := mux.NewRouter() zHandler.RegisterRoutes(r) @@ -238,10 +235,9 @@ func startZipkinHTTPAPI( AllowedHeaders: headers, }) - httpPortStr := ":" + strconv.Itoa(zipkinPort) - logger.Info("Listening for Zipkin HTTP traffic", zap.Int("zipkin.http-port", zipkinPort)) + logger.Info("Listening for Zipkin HTTP traffic", zap.String("zipkin.http-addr", zipkinAddr)) - if err := http.ListenAndServe(httpPortStr, c.Handler(recoveryHandler(r))); err != nil { + if err := http.ListenAndServe(zipkinAddr, c.Handler(recoveryHandler(r))); err != nil { logger.Fatal("Could not launch service", zap.Error(err)) } } diff --git a/cmd/flags/admin.go b/cmd/flags/admin.go index 5f088b1d299..38065414f17 100644 --- a/cmd/flags/admin.go +++ b/cmd/flags/admin.go @@ -20,7 +20,6 @@ import ( "net" "net/http" "net/http/pprof" - "strconv" "github.com/spf13/viper" "go.uber.org/zap" @@ -28,17 +27,18 @@ import ( "github.com/jaegertracing/jaeger/pkg/healthcheck" "github.com/jaegertracing/jaeger/pkg/recoveryhandler" "github.com/jaegertracing/jaeger/pkg/version" + "github.com/jaegertracing/jaeger/ports" ) const ( - adminHTTPPort = "admin-http-port" + adminHTTPAddr = "admin-http-addr" healthCheckHTTPPort = "health-check-http-port" ) // AdminServer runs an HTTP server with admin endpoints, such as healthcheck at /, /metrics, etc. type AdminServer struct { logger *zap.Logger - adminPort int + adminAddr string hc *healthcheck.HealthCheck @@ -47,9 +47,9 @@ type AdminServer struct { } // NewAdminServer creates a new admin server. -func NewAdminServer(defaultPort int) *AdminServer { +func NewAdminServer(defaultAddr string) *AdminServer { return &AdminServer{ - adminPort: defaultPort, + adminAddr: defaultAddr, logger: zap.NewNop(), hc: healthcheck.New(), mux: http.NewServeMux(), @@ -69,17 +69,17 @@ 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 --"+adminHTTPAddr) + flagSet.String(adminHTTPAddr, s.adminAddr, "The http addr for the admin server, including health check, /metrics, etc.") } // 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) + s.adminAddr = v.GetString(adminHTTPAddr) if v := v.GetInt(healthCheckHTTPPort); v != 0 { - logger.Sugar().Warnf("Using deprecated flag %s, please upgrade to %s", healthCheckHTTPPort, adminHTTPPort) - s.adminPort = v + logger.Sugar().Warnf("Using deprecated flag %s, please upgrade to %s", healthCheckHTTPPort, adminHTTPAddr) + s.adminAddr = ports.GetAddressFromPort(v) } } @@ -90,8 +90,7 @@ 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.adminAddr) if err != nil { s.logger.Error("Admin server failed to listen", zap.Error(err)) return err @@ -99,7 +98,7 @@ func (s *AdminServer) Serve() error { s.serveWithListener(l) s.logger.Info( "Admin server started", - zap.Int("http-port", s.adminPort), + zap.String("http-addr", s.adminAddr), zap.Stringer("health-status", s.hc.Get())) return nil } @@ -111,7 +110,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.adminAddr)) go func() { switch err := s.server.Serve(l); err { case nil, http.ErrServerClosed: diff --git a/cmd/flags/service.go b/cmd/flags/service.go index 2d0972d3c69..b7817bce9b6 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.GetAddressFromPort(adminPort)), signalsChannel: signalsChannel, hcStatusChannel: hcStatusChannel, } diff --git a/ports/ports.go b/ports/ports.go index 9873f191119..2b8153bb87e 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 @@ -43,3 +47,8 @@ const ( // IngesterAdminHTTP is the default admin HTTP port (health check, metrics, etc.) IngesterAdminHTTP = 14270 ) + +// GetAddressFromPort converts the port into a host:port address string +func GetAddressFromPort(port int) string { + return ":" + strconv.Itoa(port) +} \ No newline at end of file From 79a9738dee315e443568dd46fa9d85d260f43a46 Mon Sep 17 00:00:00 2001 From: Annanay Date: Thu, 3 Oct 2019 00:19:46 +0530 Subject: [PATCH 02/17] Addressed review comments Signed-off-by: Annanay --- cmd/all-in-one/main.go | 2 +- cmd/collector/app/builder/builder_flags.go | 36 ++++++++++++++----- .../app/grpcserver/grpc_server_test.go | 2 +- cmd/collector/main.go | 31 ++++++++++++---- cmd/flags/admin.go | 2 +- cmd/flags/service.go | 2 +- ports/ports.go | 4 +-- 7 files changed, 58 insertions(+), 21 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 52a6e6bf271..f4d531a5764 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -287,7 +287,7 @@ func startZipkinHTTPAPI( zipkinSpansHandler collectorApp.ZipkinSpansHandler, recoveryHandler func(http.Handler) http.Handler, ) { - if zipkinAddr != ":0" { + if zipkinAddr != "" { r := mux.NewRouter() zHandler := zipkin.NewAPIHandler(zipkinSpansHandler) zHandler.RegisterRoutes(r) diff --git a/cmd/collector/app/builder/builder_flags.go b/cmd/collector/app/builder/builder_flags.go index 2954dcf42ac..619f7d53426 100644 --- a/cmd/collector/app/builder/builder_flags.go +++ b/cmd/collector/app/builder/builder_flags.go @@ -27,14 +27,18 @@ import ( const ( collectorQueueSize = "collector.queue-size" collectorNumWorkers = "collector.num-workers" - collectorTChanAddr = "collector.tchan-addr" - collectorHTTPAddr = "collector.http-addr" - collectorGRPCAddr = "collector.grpc-addr" + collectorPort = "collector.port" + collectorHTTPPort = "collector.http-port" + collectorGRPCPort = "collector.grpc-port" + collectorTChanAddr = "collector.tchan-server.host-port" + collectorHTTPAddr = "collector.http-server.host-port" + collectorGRPCAddr = "collector.grpc-server.host-port" collectorGRPCTLS = "collector.grpc.tls" collectorGRPCCert = "collector.grpc.tls.cert" collectorGRPCKey = "collector.grpc.tls.key" collectorGRPCClientCA = "collector.grpc.tls.client.ca" - collectorZipkinHTTPAddr = "collector.zipkin.http-addr" + collectorZipkinHTTPPort = "collector.zipkin.http-port" + collectorZipkinHTTPAddr = "collector.zipkin.host-port" collectorZipkinAllowedOrigins = "collector.zipkin.allowed-origins" collectorZipkinAllowedHeaders = "collector.zipkin.allowed-headers" ) @@ -45,6 +49,12 @@ type CollectorOptions struct { QueueSize int // NumWorkers is the number of internal workers in a collector NumWorkers int + // CollectorPort is the port that the collector service listens in on for tchannel requests + CollectorPort 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 // CollectorTChanAddr is the host:port address that the collector service listens in on for tchannel requests CollectorTChanAddr string // CollectorHTTPAddr is the host:port address that the collector service listens in on for http requests @@ -59,6 +69,8 @@ type CollectorOptions struct { CollectorGRPCClientCA string // CollectorGRPCKey is the path to a TLS key file for the server CollectorGRPCKey string + // CollectorZipkinHTTPPort is the port that the Zipkin collector service listens in on for http requests + CollectorZipkinHTTPPort int // CollectorZipkinHTTPAddr is the host:port address that the Zipkin collector service listens in on for http requests CollectorZipkinHTTPAddr string // CollectorZipkinAllowedOrigins is a list of origins a cross-domain request to the Zipkin collector service can be executed from @@ -71,10 +83,14 @@ type CollectorOptions struct { func AddFlags(flags *flag.FlagSet) { flags.Int(collectorQueueSize, app.DefaultQueueSize, "The queue size of the collector") flags.Int(collectorNumWorkers, app.DefaultNumWorkers, "The number of workers pulling items from the queue") - flags.String(collectorTChanAddr, ports.GetAddressFromPort(ports.CollectorTChannel), "The TChannel address for the collector service") - flags.String(collectorHTTPAddr, ports.GetAddressFromPort(ports.CollectorHTTP), "The HTTP address for the collector service") - flags.String(collectorGRPCAddr, ports.GetAddressFromPort(ports.CollectorGRPC), "The gRPC address for the collector service") - flags.String(collectorZipkinHTTPAddr, ports.GetAddressFromPort(0), "The HTTP address for the Zipkin collector service") + flags.Int(collectorPort, ports.CollectorTChannel, "The TChannel port for the collector service - (deprecated) see - "+collectorTChanAddr) + flags.Int(collectorHTTPPort, ports.CollectorHTTP, "The HTTP port for the collector service - (deprecated) see -"+collectorHTTPAddr) + flags.Int(collectorGRPCPort, ports.CollectorGRPC, "The gRPC port for the collector service - (deprecated) see -"+collectorGRPCAddr) + flags.Int(collectorZipkinHTTPPort, 0, "The HTTP port for the Zipkin collector service e.g. 9411 - (deprecated) see -"+collectorZipkinHTTPAddr) + flags.String(collectorTChanAddr, ports.PortToHostPort(ports.CollectorTChannel), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's TChannel server") + flags.String(collectorHTTPAddr, ports.PortToHostPort(ports.CollectorHTTP), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's HTTP server") + flags.String(collectorGRPCAddr, ports.PortToHostPort(ports.CollectorGRPC), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's GRPC server") + flags.String(collectorZipkinHTTPAddr, ports.PortToHostPort(0), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's Zipkin server") flags.Bool(collectorGRPCTLS, false, "Enable TLS for the gRPC collector endpoint") flags.String(collectorGRPCCert, "", "Path to TLS certificate for the gRPC collector TLS service") flags.String(collectorGRPCKey, "", "Path to TLS key for the gRPC collector TLS cert") @@ -87,6 +103,9 @@ func AddFlags(flags *flag.FlagSet) { func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper) *CollectorOptions { cOpts.QueueSize = v.GetInt(collectorQueueSize) cOpts.NumWorkers = v.GetInt(collectorNumWorkers) + cOpts.CollectorPort = v.GetInt(collectorPort) + cOpts.CollectorHTTPPort = v.GetInt(collectorHTTPPort) + cOpts.CollectorGRPCPort = v.GetInt(collectorGRPCPort) cOpts.CollectorTChanAddr = v.GetString(collectorTChanAddr) cOpts.CollectorHTTPAddr = v.GetString(collectorHTTPAddr) cOpts.CollectorGRPCAddr = v.GetString(collectorGRPCAddr) @@ -94,6 +113,7 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper) *CollectorOptions { cOpts.CollectorGRPCCert = v.GetString(collectorGRPCCert) cOpts.CollectorGRPCClientCA = v.GetString(collectorGRPCClientCA) cOpts.CollectorGRPCKey = v.GetString(collectorGRPCKey) + cOpts.CollectorZipkinHTTPPort = v.GetInt(collectorZipkinHTTPPort) cOpts.CollectorZipkinHTTPAddr = v.GetString(collectorZipkinHTTPAddr) cOpts.CollectorZipkinAllowedOrigins = v.GetString(collectorZipkinAllowedOrigins) cOpts.CollectorZipkinAllowedHeaders = v.GetString(collectorZipkinAllowedHeaders) diff --git a/cmd/collector/app/grpcserver/grpc_server_test.go b/cmd/collector/app/grpcserver/grpc_server_test.go index d9b90f138b4..6507662bd70 100644 --- a/cmd/collector/app/grpcserver/grpc_server_test.go +++ b/cmd/collector/app/grpcserver/grpc_server_test.go @@ -64,7 +64,7 @@ func TestSpanCollector(t *testing.T) { l, _ := zap.NewDevelopment() handler := app.NewGRPCHandler(l, &mockSpanProcessor{}) server := grpc.NewServer() - addr, err := StartGRPCCollector(ports.GetAddressFromPort(0), server, handler, &mockSamplingStore{}, l, func(e error) { + addr, err := StartGRPCCollector(ports.PortToHostPort(0), server, handler, &mockSamplingStore{}, l, func(e error) { }) require.NoError(t, err) diff --git a/cmd/collector/main.go b/cmd/collector/main.go index a574dec769a..216b391d5af 100644 --- a/cmd/collector/main.go +++ b/cmd/collector/main.go @@ -119,11 +119,13 @@ func main() { server.Register(jc.NewTChanCollectorServer(batchHandler)) server.Register(zc.NewTChanZipkinCollectorServer(batchHandler)) server.Register(sc.NewTChanSamplingManagerServer(sampling.NewHandler(strategyStore))) - listener, err := net.Listen("tcp", builderOpts.CollectorTChanAddr) + + addr := getAddressFromCLIOptions(builderOpts.CollectorPort, builderOpts.CollectorTChanAddr, "collector.http-port", logger) + listener, err := net.Listen("tcp", addr) if err != nil { logger.Fatal("Unable to start listening on channel", zap.Error(err)) } - logger.Info("Starting jaeger-collector TChannel server", zap.String("tchan-addr", builderOpts.CollectorTChanAddr)) + logger.Info("Starting jaeger-collector TChannel server", zap.String("tchan-addr", addr)) ch.Serve(listener) } @@ -139,11 +141,14 @@ func main() { recoveryHandler := recoveryhandler.NewRecoveryHandler(logger, true) httpHandler := recoveryHandler(r) - go startZipkinHTTPAPI(logger, builderOpts.CollectorZipkinHTTPAddr, builderOpts.CollectorZipkinAllowedOrigins, builderOpts.CollectorZipkinAllowedHeaders, zipkinSpansHandler, recoveryHandler) + zipkinAddr := getAddressFromCLIOptions(builderOpts.CollectorZipkinHTTPPort, builderOpts.CollectorZipkinHTTPAddr, "collector.zipkin.http-port", logger) + + go startZipkinHTTPAPI(logger, zipkinAddr, builderOpts.CollectorZipkinAllowedOrigins, builderOpts.CollectorZipkinAllowedHeaders, zipkinSpansHandler, recoveryHandler) - logger.Info("Starting jaeger-collector HTTP server", zap.String("http-addr", builderOpts.CollectorHTTPAddr)) + httpAddr := getAddressFromCLIOptions(builderOpts.CollectorHTTPPort, builderOpts.CollectorHTTPAddr, "collector.http-port", logger) + logger.Info("Starting jaeger-collector HTTP server", zap.String("http-addr", httpAddr)) go func() { - if err := http.ListenAndServe(builderOpts.CollectorHTTPAddr, httpHandler); err != nil { + if err := http.ListenAndServe(httpAddr, httpHandler); err != nil { logger.Fatal("Could not launch service", zap.Error(err)) } svc.HC().Set(healthcheck.Unavailable) @@ -204,7 +209,9 @@ func startGRPCServer( } else { // server without TLS server = grpc.NewServer() } - _, err := grpcserver.StartGRPCCollector(opts.CollectorGRPCAddr, server, handler, samplingStore, logger, func(err error) { + + addr := getAddressFromCLIOptions(opts.CollectorGRPCPort, opts.CollectorGRPCAddr, "collector.grpc-port", logger) + _, err := grpcserver.StartGRPCCollector(addr, server, handler, samplingStore, logger, func(err error) { logger.Fatal("gRPC collector failed", zap.Error(err)) }) if err != nil { @@ -221,7 +228,7 @@ func startZipkinHTTPAPI( zipkinSpansHandler app.ZipkinSpansHandler, recoveryHandler func(http.Handler) http.Handler, ) { - if zipkinAddr != ":0" { + if zipkinAddr != "" { zHandler := zipkin.NewAPIHandler(zipkinSpansHandler) r := mux.NewRouter() zHandler.RegisterRoutes(r) @@ -257,3 +264,13 @@ func initSamplingStrategyStore( } return strategyStore } + +// Utility function to decide listening address based on port (deprecated flags) or host:port (new flags) +func getAddressFromCLIOptions(port int, addr string, deprecatedFlag string, logger *zap.Logger) string { + if port != 0 { + logger.Warn("Using deprecated configuration", zap.String("option", deprecatedFlag)) + return ports.PortToHostPort(port) + } + + return addr +} \ No newline at end of file diff --git a/cmd/flags/admin.go b/cmd/flags/admin.go index 38065414f17..6ae5b76192a 100644 --- a/cmd/flags/admin.go +++ b/cmd/flags/admin.go @@ -79,7 +79,7 @@ func (s *AdminServer) initFromViper(v *viper.Viper, logger *zap.Logger) { s.adminAddr = v.GetString(adminHTTPAddr) if v := v.GetInt(healthCheckHTTPPort); v != 0 { logger.Sugar().Warnf("Using deprecated flag %s, please upgrade to %s", healthCheckHTTPPort, adminHTTPAddr) - s.adminAddr = ports.GetAddressFromPort(v) + s.adminAddr = ports.PortToHostPort(v) } } diff --git a/cmd/flags/service.go b/cmd/flags/service.go index b7817bce9b6..d68d2b2933f 100644 --- a/cmd/flags/service.go +++ b/cmd/flags/service.go @@ -60,7 +60,7 @@ func NewService(adminPort int) *Service { signal.Notify(signalsChannel, os.Interrupt, syscall.SIGTERM) return &Service{ - Admin: NewAdminServer(ports.GetAddressFromPort(adminPort)), + Admin: NewAdminServer(ports.PortToHostPort(adminPort)), signalsChannel: signalsChannel, hcStatusChannel: hcStatusChannel, } diff --git a/ports/ports.go b/ports/ports.go index 2b8153bb87e..046f0312faa 100644 --- a/ports/ports.go +++ b/ports/ports.go @@ -48,7 +48,7 @@ const ( IngesterAdminHTTP = 14270 ) -// GetAddressFromPort converts the port into a host:port address string -func GetAddressFromPort(port int) string { +// PortToHostPort converts the port into a host:port address string +func PortToHostPort(port int) string { return ":" + strconv.Itoa(port) } \ No newline at end of file From 1ad013f486afdeb39ae788ea49f30bc243139c0e Mon Sep 17 00:00:00 2001 From: Annanay Date: Thu, 3 Oct 2019 00:27:23 +0530 Subject: [PATCH 03/17] Continue support for deprecated flags in all-in-one Signed-off-by: Annanay --- cmd/all-in-one/main.go | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index f4d531a5764..3844309fb16 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -233,7 +233,9 @@ func startCollector( server.Register(jc.NewTChanCollectorServer(batchHandler)) server.Register(zc.NewTChanZipkinCollectorServer(batchHandler)) server.Register(sc.NewTChanSamplingManagerServer(sampling.NewHandler(strategyStore))) - listener, err := net.Listen("tcp", cOpts.CollectorTChanAddr) + + tchanAddr := getAddressFromCLIOptions(cOpts.CollectorPort, cOpts.CollectorTChanAddr, "collector.http-port", logger) + listener, err := net.Listen("tcp", tchanAddr) if err != nil { logger.Fatal("Unable to start listening on channel", zap.Error(err)) } @@ -241,7 +243,8 @@ func startCollector( ch.Serve(listener) } - server, err := startGRPCServer(cOpts.CollectorGRPCAddr, grpcHandler, strategyStore, logger) + grpcAddr := getAddressFromCLIOptions(cOpts.CollectorGRPCPort, cOpts.CollectorGRPCAddr, "collector.grpc-port", logger) + server, err := startGRPCServer(grpcAddr, grpcHandler, strategyStore, logger) if err != nil { logger.Fatal("Could not start gRPC collector", zap.Error(err)) } @@ -252,11 +255,14 @@ func startCollector( apiHandler.RegisterRoutes(r) recoveryHandler := recoveryhandler.NewRecoveryHandler(logger, true) - go startZipkinHTTPAPI(logger, cOpts.CollectorZipkinHTTPAddr, zipkinSpansHandler, recoveryHandler) + zipkinAddr := getAddressFromCLIOptions(cOpts.CollectorZipkinHTTPPort, cOpts.CollectorZipkinHTTPAddr, "collector.zipkin.http-port", logger) + + go startZipkinHTTPAPI(logger, zipkinAddr, zipkinSpansHandler, recoveryHandler) logger.Info("Starting jaeger-collector HTTP server", zap.String("http-addr", cOpts.CollectorHTTPAddr)) go func() { - if err := http.ListenAndServe(cOpts.CollectorHTTPAddr, recoveryHandler(r)); err != nil { + httpAddr := getAddressFromCLIOptions(cOpts.CollectorHTTPPort, cOpts.CollectorHTTPAddr, "collector.http-port", logger) + if err := http.ListenAndServe(httpAddr, recoveryHandler(r)); err != nil { logger.Fatal("Could not launch jaeger-collector HTTP server", zap.Error(err)) } hc.Set(healthcheck.Unavailable) @@ -358,3 +364,13 @@ func initTracer(metricsFactory metrics.Factory, logger *zap.Logger) io.Closer { opentracing.SetGlobalTracer(tracer) return closer } + +// Utility function to decide listening address based on port (deprecated flags) or host:port (new flags) +func getAddressFromCLIOptions(port int, addr string, deprecatedFlag string, logger *zap.Logger) string { + if port != 0 { + logger.Warn("Using deprecated configuration", zap.String("option", deprecatedFlag)) + return ports.PortToHostPort(port) + } + + return addr +} \ No newline at end of file From 1b765c596ce8ce943ea4061432b336803e66827f Mon Sep 17 00:00:00 2001 From: Annanay Date: Thu, 3 Oct 2019 00:48:30 +0530 Subject: [PATCH 04/17] Nitpick, fix agent options in all-in-one Signed-off-by: Annanay --- cmd/all-in-one/main.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 3844309fb16..d01ed12ce09 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -184,7 +184,8 @@ func startAgent( ) { metricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "agent", Tags: nil}) - grpcBuilder.CollectorHostPorts = append(grpcBuilder.CollectorHostPorts, cOpts.CollectorGRPCAddr) + addr := getAddressFromCLIOptions(cOpts.CollectorGRPCPort, cOpts.CollectorGRPCAddr, "collector.grpc-port", logger) + grpcBuilder.CollectorHostPorts = append(grpcBuilder.CollectorHostPorts, addr) cp, err := agentApp.CreateCollectorProxy(repOpts, tchanBuilder, grpcBuilder, logger, metricsFactory) if err != nil { logger.Fatal("Could not create collector proxy", zap.Error(err)) @@ -239,7 +240,7 @@ func startCollector( if err != nil { logger.Fatal("Unable to start listening on channel", zap.Error(err)) } - logger.Info("Starting jaeger-collector TChannel server", zap.String("addr", cOpts.CollectorTChanAddr)) + logger.Info("Starting jaeger-collector TChannel server", zap.String("addr", tchanAddr)) ch.Serve(listener) } @@ -259,9 +260,9 @@ func startCollector( go startZipkinHTTPAPI(logger, zipkinAddr, zipkinSpansHandler, recoveryHandler) - logger.Info("Starting jaeger-collector HTTP server", zap.String("http-addr", cOpts.CollectorHTTPAddr)) + httpAddr := getAddressFromCLIOptions(cOpts.CollectorHTTPPort, cOpts.CollectorHTTPAddr, "collector.http-port", logger) + logger.Info("Starting jaeger-collector HTTP server", zap.String("http-addr", httpAddr)) go func() { - httpAddr := getAddressFromCLIOptions(cOpts.CollectorHTTPPort, cOpts.CollectorHTTPAddr, "collector.http-port", logger) if err := http.ListenAndServe(httpAddr, recoveryHandler(r)); err != nil { logger.Fatal("Could not launch jaeger-collector HTTP server", zap.Error(err)) } From cca9d7e5fc9f846fd78abdf36a6ddceaa994b6aa Mon Sep 17 00:00:00 2001 From: Annanay Date: Fri, 4 Oct 2019 00:46:52 +0530 Subject: [PATCH 05/17] Addressed comments, use HostPort in variable names Signed-off-by: Annanay --- cmd/all-in-one/main.go | 12 +++--- cmd/collector/app/builder/builder_flags.go | 48 +++++++++++----------- cmd/collector/main.go | 10 ++--- ports/ports.go | 2 +- 4 files changed, 36 insertions(+), 36 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index d01ed12ce09..b21b3f63494 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -184,7 +184,7 @@ func startAgent( ) { metricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "agent", Tags: nil}) - addr := getAddressFromCLIOptions(cOpts.CollectorGRPCPort, cOpts.CollectorGRPCAddr, "collector.grpc-port", logger) + addr := getAddressFromCLIOptions(cOpts.CollectorGRPCPort, cOpts.CollectorGRPCHostPort, "collector.grpc-port", logger) grpcBuilder.CollectorHostPorts = append(grpcBuilder.CollectorHostPorts, addr) cp, err := agentApp.CreateCollectorProxy(repOpts, tchanBuilder, grpcBuilder, logger, metricsFactory) if err != nil { @@ -235,7 +235,7 @@ func startCollector( server.Register(zc.NewTChanZipkinCollectorServer(batchHandler)) server.Register(sc.NewTChanSamplingManagerServer(sampling.NewHandler(strategyStore))) - tchanAddr := getAddressFromCLIOptions(cOpts.CollectorPort, cOpts.CollectorTChanAddr, "collector.http-port", logger) + tchanAddr := getAddressFromCLIOptions(cOpts.CollectorPort, cOpts.CollectorTChanHostPort, "collector.http-port", logger) listener, err := net.Listen("tcp", tchanAddr) if err != nil { logger.Fatal("Unable to start listening on channel", zap.Error(err)) @@ -244,7 +244,7 @@ func startCollector( ch.Serve(listener) } - grpcAddr := getAddressFromCLIOptions(cOpts.CollectorGRPCPort, cOpts.CollectorGRPCAddr, "collector.grpc-port", logger) + grpcAddr := getAddressFromCLIOptions(cOpts.CollectorGRPCPort, cOpts.CollectorGRPCHostPort, "collector.grpc-port", logger) server, err := startGRPCServer(grpcAddr, grpcHandler, strategyStore, logger) if err != nil { logger.Fatal("Could not start gRPC collector", zap.Error(err)) @@ -256,11 +256,11 @@ func startCollector( apiHandler.RegisterRoutes(r) recoveryHandler := recoveryhandler.NewRecoveryHandler(logger, true) - zipkinAddr := getAddressFromCLIOptions(cOpts.CollectorZipkinHTTPPort, cOpts.CollectorZipkinHTTPAddr, "collector.zipkin.http-port", logger) + zipkinAddr := getAddressFromCLIOptions(cOpts.CollectorZipkinHTTPPort, cOpts.CollectorZipkinHTTPHostPort, "collector.zipkin.http-port", logger) go startZipkinHTTPAPI(logger, zipkinAddr, zipkinSpansHandler, recoveryHandler) - httpAddr := getAddressFromCLIOptions(cOpts.CollectorHTTPPort, cOpts.CollectorHTTPAddr, "collector.http-port", logger) + httpAddr := getAddressFromCLIOptions(cOpts.CollectorHTTPPort, cOpts.CollectorHTTPHostPort, "collector.http-port", logger) logger.Info("Starting jaeger-collector HTTP server", zap.String("http-addr", httpAddr)) go func() { if err := http.ListenAndServe(httpAddr, recoveryHandler(r)); err != nil { @@ -374,4 +374,4 @@ func getAddressFromCLIOptions(port int, addr string, deprecatedFlag string, logg } return addr -} \ No newline at end of file +} diff --git a/cmd/collector/app/builder/builder_flags.go b/cmd/collector/app/builder/builder_flags.go index 619f7d53426..775a80a8968 100644 --- a/cmd/collector/app/builder/builder_flags.go +++ b/cmd/collector/app/builder/builder_flags.go @@ -30,15 +30,15 @@ const ( collectorPort = "collector.port" collectorHTTPPort = "collector.http-port" collectorGRPCPort = "collector.grpc-port" - collectorTChanAddr = "collector.tchan-server.host-port" - collectorHTTPAddr = "collector.http-server.host-port" - collectorGRPCAddr = "collector.grpc-server.host-port" + collectorTChanHostPort = "collector.tchan-server.host-port" + collectorHTTPHostPort = "collector.http-server.host-port" + collectorGRPCHostPort = "collector.grpc-server.host-port" collectorGRPCTLS = "collector.grpc.tls" collectorGRPCCert = "collector.grpc.tls.cert" collectorGRPCKey = "collector.grpc.tls.key" collectorGRPCClientCA = "collector.grpc.tls.client.ca" collectorZipkinHTTPPort = "collector.zipkin.http-port" - collectorZipkinHTTPAddr = "collector.zipkin.host-port" + collectorZipkinHTTPHostPort = "collector.zipkin.host-port" collectorZipkinAllowedOrigins = "collector.zipkin.allowed-origins" collectorZipkinAllowedHeaders = "collector.zipkin.allowed-headers" ) @@ -55,12 +55,12 @@ type CollectorOptions struct { CollectorHTTPPort int // CollectorGRPCPort is the port that the collector service listens in on for gRPC requests CollectorGRPCPort int - // CollectorTChanAddr is the host:port address that the collector service listens in on for tchannel requests - CollectorTChanAddr string - // CollectorHTTPAddr is the host:port address that the collector service listens in on for http requests - CollectorHTTPAddr string - // CollectorGRPCAddr is the host:port address that the collector service listens in on for gRPC requests - CollectorGRPCAddr string + // 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 // CollectorGRPCTLS defines if the server is setup with TLS CollectorGRPCTLS bool // CollectorGRPCCert is the path to a TLS certificate file for the server @@ -71,8 +71,8 @@ type CollectorOptions struct { CollectorGRPCKey string // CollectorZipkinHTTPPort is the port that the Zipkin collector service listens in on for http requests CollectorZipkinHTTPPort int - // CollectorZipkinHTTPAddr is the host:port address that the Zipkin collector service listens in on for http requests - CollectorZipkinHTTPAddr string + // 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 @@ -83,14 +83,14 @@ type CollectorOptions struct { func AddFlags(flags *flag.FlagSet) { flags.Int(collectorQueueSize, app.DefaultQueueSize, "The queue size of the collector") flags.Int(collectorNumWorkers, app.DefaultNumWorkers, "The number of workers pulling items from the queue") - flags.Int(collectorPort, ports.CollectorTChannel, "The TChannel port for the collector service - (deprecated) see - "+collectorTChanAddr) - flags.Int(collectorHTTPPort, ports.CollectorHTTP, "The HTTP port for the collector service - (deprecated) see -"+collectorHTTPAddr) - flags.Int(collectorGRPCPort, ports.CollectorGRPC, "The gRPC port for the collector service - (deprecated) see -"+collectorGRPCAddr) - flags.Int(collectorZipkinHTTPPort, 0, "The HTTP port for the Zipkin collector service e.g. 9411 - (deprecated) see -"+collectorZipkinHTTPAddr) - flags.String(collectorTChanAddr, ports.PortToHostPort(ports.CollectorTChannel), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's TChannel server") - flags.String(collectorHTTPAddr, ports.PortToHostPort(ports.CollectorHTTP), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's HTTP server") - flags.String(collectorGRPCAddr, ports.PortToHostPort(ports.CollectorGRPC), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's GRPC server") - flags.String(collectorZipkinHTTPAddr, ports.PortToHostPort(0), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's Zipkin server") + flags.Int(collectorPort, ports.CollectorTChannel, "The TChannel port for the collector service - (deprecated) see - "+collectorTChanHostPort) + flags.Int(collectorHTTPPort, ports.CollectorHTTP, "The HTTP port for the collector service - (deprecated) see -"+collectorHTTPHostPort) + flags.Int(collectorGRPCPort, ports.CollectorGRPC, "The gRPC port for the collector service - (deprecated) see -"+collectorGRPCHostPort) + flags.Int(collectorZipkinHTTPPort, 0, "The HTTP port for the Zipkin collector service e.g. 9411 - (deprecated) see -"+collectorZipkinHTTPHostPort) + flags.String(collectorTChanHostPort, ports.PortToHostPort(ports.CollectorTChannel), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's TChannel server") + 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.Bool(collectorGRPCTLS, false, "Enable TLS for the gRPC collector endpoint") flags.String(collectorGRPCCert, "", "Path to TLS certificate for the gRPC collector TLS service") flags.String(collectorGRPCKey, "", "Path to TLS key for the gRPC collector TLS cert") @@ -106,15 +106,15 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper) *CollectorOptions { cOpts.CollectorPort = v.GetInt(collectorPort) cOpts.CollectorHTTPPort = v.GetInt(collectorHTTPPort) cOpts.CollectorGRPCPort = v.GetInt(collectorGRPCPort) - cOpts.CollectorTChanAddr = v.GetString(collectorTChanAddr) - cOpts.CollectorHTTPAddr = v.GetString(collectorHTTPAddr) - cOpts.CollectorGRPCAddr = v.GetString(collectorGRPCAddr) + cOpts.CollectorTChanHostPort = v.GetString(collectorTChanHostPort) + cOpts.CollectorHTTPHostPort = v.GetString(collectorHTTPHostPort) + cOpts.CollectorGRPCHostPort = v.GetString(collectorGRPCHostPort) cOpts.CollectorGRPCTLS = v.GetBool(collectorGRPCTLS) cOpts.CollectorGRPCCert = v.GetString(collectorGRPCCert) cOpts.CollectorGRPCClientCA = v.GetString(collectorGRPCClientCA) cOpts.CollectorGRPCKey = v.GetString(collectorGRPCKey) cOpts.CollectorZipkinHTTPPort = v.GetInt(collectorZipkinHTTPPort) - cOpts.CollectorZipkinHTTPAddr = v.GetString(collectorZipkinHTTPAddr) + cOpts.CollectorZipkinHTTPHostPort = v.GetString(collectorZipkinHTTPHostPort) cOpts.CollectorZipkinAllowedOrigins = v.GetString(collectorZipkinAllowedOrigins) cOpts.CollectorZipkinAllowedHeaders = v.GetString(collectorZipkinAllowedHeaders) return cOpts diff --git a/cmd/collector/main.go b/cmd/collector/main.go index 216b391d5af..e39c4c3e21b 100644 --- a/cmd/collector/main.go +++ b/cmd/collector/main.go @@ -120,7 +120,7 @@ func main() { server.Register(zc.NewTChanZipkinCollectorServer(batchHandler)) server.Register(sc.NewTChanSamplingManagerServer(sampling.NewHandler(strategyStore))) - addr := getAddressFromCLIOptions(builderOpts.CollectorPort, builderOpts.CollectorTChanAddr, "collector.http-port", logger) + addr := getAddressFromCLIOptions(builderOpts.CollectorPort, builderOpts.CollectorTChanHostPort, "collector.http-port", logger) listener, err := net.Listen("tcp", addr) if err != nil { logger.Fatal("Unable to start listening on channel", zap.Error(err)) @@ -141,11 +141,11 @@ func main() { recoveryHandler := recoveryhandler.NewRecoveryHandler(logger, true) httpHandler := recoveryHandler(r) - zipkinAddr := getAddressFromCLIOptions(builderOpts.CollectorZipkinHTTPPort, builderOpts.CollectorZipkinHTTPAddr, "collector.zipkin.http-port", logger) + zipkinAddr := getAddressFromCLIOptions(builderOpts.CollectorZipkinHTTPPort, builderOpts.CollectorZipkinHTTPHostPort, "collector.zipkin.http-port", logger) go startZipkinHTTPAPI(logger, zipkinAddr, builderOpts.CollectorZipkinAllowedOrigins, builderOpts.CollectorZipkinAllowedHeaders, zipkinSpansHandler, recoveryHandler) - httpAddr := getAddressFromCLIOptions(builderOpts.CollectorHTTPPort, builderOpts.CollectorHTTPAddr, "collector.http-port", logger) + httpAddr := getAddressFromCLIOptions(builderOpts.CollectorHTTPPort, builderOpts.CollectorHTTPHostPort, "collector.http-port", logger) logger.Info("Starting jaeger-collector HTTP server", zap.String("http-addr", httpAddr)) go func() { if err := http.ListenAndServe(httpAddr, httpHandler); err != nil { @@ -210,7 +210,7 @@ func startGRPCServer( server = grpc.NewServer() } - addr := getAddressFromCLIOptions(opts.CollectorGRPCPort, opts.CollectorGRPCAddr, "collector.grpc-port", logger) + addr := getAddressFromCLIOptions(opts.CollectorGRPCPort, opts.CollectorGRPCHostPort, "collector.grpc-port", logger) _, err := grpcserver.StartGRPCCollector(addr, server, handler, samplingStore, logger, func(err error) { logger.Fatal("gRPC collector failed", zap.Error(err)) }) @@ -273,4 +273,4 @@ func getAddressFromCLIOptions(port int, addr string, deprecatedFlag string, logg } return addr -} \ No newline at end of file +} diff --git a/ports/ports.go b/ports/ports.go index 046f0312faa..66883185a54 100644 --- a/ports/ports.go +++ b/ports/ports.go @@ -51,4 +51,4 @@ const ( // PortToHostPort converts the port into a host:port address string func PortToHostPort(port int) string { return ":" + strconv.Itoa(port) -} \ No newline at end of file +} From 66273f314393dd6df978c5590bf52ab811a070b1 Mon Sep 17 00:00:00 2001 From: Annanay Date: Fri, 18 Oct 2019 23:16:32 +0530 Subject: [PATCH 06/17] Address comments Signed-off-by: Annanay --- cmd/collector/app/builder/builder_flags.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/collector/app/builder/builder_flags.go b/cmd/collector/app/builder/builder_flags.go index 775a80a8968..fbb5e78032f 100644 --- a/cmd/collector/app/builder/builder_flags.go +++ b/cmd/collector/app/builder/builder_flags.go @@ -83,10 +83,10 @@ type CollectorOptions struct { func AddFlags(flags *flag.FlagSet) { flags.Int(collectorQueueSize, app.DefaultQueueSize, "The queue size of the collector") flags.Int(collectorNumWorkers, app.DefaultNumWorkers, "The number of workers pulling items from the queue") - flags.Int(collectorPort, ports.CollectorTChannel, "The TChannel port for the collector service - (deprecated) see - "+collectorTChanHostPort) - flags.Int(collectorHTTPPort, ports.CollectorHTTP, "The HTTP port for the collector service - (deprecated) see -"+collectorHTTPHostPort) - flags.Int(collectorGRPCPort, ports.CollectorGRPC, "The gRPC port for the collector service - (deprecated) see -"+collectorGRPCHostPort) - flags.Int(collectorZipkinHTTPPort, 0, "The HTTP port for the Zipkin collector service e.g. 9411 - (deprecated) see -"+collectorZipkinHTTPHostPort) + flags.Int(collectorPort, 0, "(deprecated) please use - "+collectorTChanHostPort) + 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(collectorTChanHostPort, ports.PortToHostPort(ports.CollectorTChannel), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's TChannel server") 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") From f6fed00a8599e7d0c9dcca8b343b2d5d68823d1f Mon Sep 17 00:00:00 2001 From: Annanay Date: Sat, 19 Oct 2019 16:23:15 +0530 Subject: [PATCH 07/17] Add Changelog entries Signed-off-by: Annanay --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e534666ad7..9dfee37dfa7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,16 @@ Changes by Version ##### Breaking Changes +* 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` - superseded by `collector.tchan-server.host-port` + * `collector.http-port` - superseded by `collector.http-server.host-port` + * `collector.grpc-port` - superseded by `collector.grpc-server.host-port` + * `collector.zipkin.http-port` - superseded by `collector.zipkin.host-port` + ##### New Features ##### Bug fixes, Minor Improvements From 706e582c1f92cfa3173411d78b8b7d78af670430 Mon Sep 17 00:00:00 2001 From: Annanay Date: Sun, 20 Oct 2019 19:17:58 +0530 Subject: [PATCH 08/17] Fix merge changes Signed-off-by: Annanay --- cmd/collector/app/builder/builder_flags.go | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/cmd/collector/app/builder/builder_flags.go b/cmd/collector/app/builder/builder_flags.go index ce13ef8a3b4..3d2ccddc3cd 100644 --- a/cmd/collector/app/builder/builder_flags.go +++ b/cmd/collector/app/builder/builder_flags.go @@ -34,10 +34,6 @@ const ( collectorTChanHostPort = "collector.tchan-server.host-port" collectorHTTPHostPort = "collector.http-server.host-port" collectorGRPCHostPort = "collector.grpc-server.host-port" - collectorGRPCTLS = "collector.grpc.tls" - collectorGRPCCert = "collector.grpc.tls.cert" - collectorGRPCKey = "collector.grpc.tls.key" - collectorGRPCClientCA = "collector.grpc.tls.client.ca" collectorZipkinHTTPPort = "collector.zipkin.http-port" collectorZipkinHTTPHostPort = "collector.zipkin.host-port" collectorZipkinAllowedOrigins = "collector.zipkin.allowed-origins" @@ -68,14 +64,6 @@ type CollectorOptions struct { CollectorHTTPHostPort string // CollectorGRPCHostPort is the host:port address that the collector service listens in on for gRPC requests CollectorGRPCHostPort string - // CollectorGRPCTLS defines if the server is setup with TLS - CollectorGRPCTLS bool - // CollectorGRPCCert is the path to a TLS certificate file for the server - CollectorGRPCCert string - // CollectorGRPCClientCA is the path to a TLS certificate file for authenticating clients - CollectorGRPCClientCA string - // CollectorGRPCKey is the path to a TLS key file for the server - CollectorGRPCKey string // TLS configures secure transport TLS tlscfg.Options // CollectorZipkinHTTPPort is the port that the Zipkin collector service listens in on for http requests @@ -100,10 +88,6 @@ func AddFlags(flags *flag.FlagSet) { 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.Bool(collectorGRPCTLS, false, "Enable TLS for the gRPC collector endpoint") - flags.String(collectorGRPCCert, "", "Path to TLS certificate for the gRPC collector TLS service") - flags.String(collectorGRPCKey, "", "Path to TLS key for the gRPC collector TLS cert") - flags.String(collectorGRPCClientCA, "", "Path to a TLS CA to verify certificates presented by clients (if unset, all clients are permitted)") 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) @@ -119,10 +103,6 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper) *CollectorOptions { cOpts.CollectorTChanHostPort = v.GetString(collectorTChanHostPort) cOpts.CollectorHTTPHostPort = v.GetString(collectorHTTPHostPort) cOpts.CollectorGRPCHostPort = v.GetString(collectorGRPCHostPort) - cOpts.CollectorGRPCTLS = v.GetBool(collectorGRPCTLS) - cOpts.CollectorGRPCCert = v.GetString(collectorGRPCCert) - cOpts.CollectorGRPCClientCA = v.GetString(collectorGRPCClientCA) - cOpts.CollectorGRPCKey = v.GetString(collectorGRPCKey) cOpts.CollectorZipkinHTTPPort = v.GetInt(collectorZipkinHTTPPort) cOpts.CollectorZipkinHTTPHostPort = v.GetString(collectorZipkinHTTPHostPort) cOpts.CollectorZipkinAllowedOrigins = v.GetString(collectorZipkinAllowedOrigins) From d3e43852764f4f8d3f9371a2a8c4f2b6a2413c04 Mon Sep 17 00:00:00 2001 From: Annanay Agarwal Date: Thu, 19 Dec 2019 22:18:25 +0530 Subject: [PATCH 09/17] Checkpoint Signed-off-by: Annanay Agarwal --- cmd/flags/admin.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/cmd/flags/admin.go b/cmd/flags/admin.go index 6ae5b76192a..949d384dedb 100644 --- a/cmd/flags/admin.go +++ b/cmd/flags/admin.go @@ -31,6 +31,7 @@ import ( ) const ( + adminHTTPPort = "admin-http-port" adminHTTPAddr = "admin-http-addr" healthCheckHTTPPort = "health-check-http-port" ) @@ -70,17 +71,30 @@ func (s *AdminServer) setLogger(logger *zap.Logger) { // AddFlags registers CLI flags. func (s *AdminServer) AddFlags(flagSet *flag.FlagSet) { flagSet.Int(healthCheckHTTPPort, 0, "(deprecated) see --"+adminHTTPAddr) + flagSet.Int(adminHTTPPort, 0, "(deprecated) see --"+adminHTTPAddr) flagSet.String(adminHTTPAddr, s.adminAddr, "The http addr for the admin server, including health check, /metrics, etc.") } +// Util function to check if deprecated flag is used +func checkDeprecatedFlag(v *viper.Viper, string actualFlagName, string expectedFlagName) string { + +} + // InitFromViper initializes the server with properties retrieved from Viper. func (s *AdminServer) initFromViper(v *viper.Viper, logger *zap.Logger) { s.setLogger(logger) s.adminAddr = v.GetString(adminHTTPAddr) + + s.adminAddr = checkDeprecatedFlag(v, healthCheckHTTPPort, ) + if v := v.GetInt(healthCheckHTTPPort); v != 0 { logger.Sugar().Warnf("Using deprecated flag %s, please upgrade to %s", healthCheckHTTPPort, adminHTTPAddr) s.adminAddr = ports.PortToHostPort(v) } + if v := v.GetInt(adminHTTPPort); v != 0 { + logger.Sugar().Warnf("Using deprecated flag %s, please upgrade to %s", adminHTTPPort, adminHTTPAddr) + s.adminAddr = ports.PortToHostPort(v) + } } // Handle adds a new handler to the admin server. From 5fbe038b49836d37340e4b832d2bc6c5b06681be Mon Sep 17 00:00:00 2001 From: Annanay Date: Fri, 13 Mar 2020 17:25:57 +0530 Subject: [PATCH 10/17] Rebased and addressed comments Signed-off-by: Annanay --- cmd/all-in-one/main.go | 10 ---------- cmd/collector/app/builder_flags.go | 6 ++---- cmd/collector/app/collector.go | 16 +++++++++++++--- 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 +++++------- 7 files changed, 28 insertions(+), 36 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 4167ca65bdb..0a9b79ed01f 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -254,13 +254,3 @@ func initTracer(metricsFactory metrics.Factory, logger *zap.Logger) io.Closer { opentracing.SetGlobalTracer(tracer) return closer } - -// Utility function to decide listening address based on port (deprecated flags) or host:port (new flags) -func getAddressFromCLIOptions(port int, addr string, deprecatedFlag string, logger *zap.Logger) string { - if port != 0 { - logger.Warn("Using deprecated configuration", zap.String("option", deprecatedFlag)) - return ports.PortToHostPort(port) - } - - return addr -} diff --git a/cmd/collector/app/builder_flags.go b/cmd/collector/app/builder_flags.go index 69bdea7c8ed..4478f7a9e46 100644 --- a/cmd/collector/app/builder_flags.go +++ b/cmd/collector/app/builder_flags.go @@ -81,13 +81,11 @@ type CollectorOptions struct { // AddFlags adds flags for CollectorOptions func AddFlags(flags *flag.FlagSet) { - flags.Int(collectorQueueSize, app.DefaultQueueSize, "The queue size of the collector") - flags.Int(collectorNumWorkers, app.DefaultNumWorkers, "The number of workers pulling items from the queue") - flags.Int(collectorPort, 0, "(deprecated) please use - "+collectorTChanHostPort) + 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, 0, "(deprecated) please use -"+collectorHTTPHostPort) flags.Int(collectorGRPCPort, 0, "(deprecated) please use -"+collectorGRPCHostPort) flags.Int(collectorZipkinHTTPPort, 0, "(deprecated) please use -"+collectorZipkinHTTPHostPort) - flags.String(collectorTChanHostPort, ports.PortToHostPort(ports.CollectorTChannel), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's TChannel server") 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") diff --git a/cmd/collector/app/collector.go b/cmd/collector/app/collector.go index 7af28c001a4..bc168479bea 100644 --- a/cmd/collector/app/collector.go +++ b/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 +} 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 From a86336234ff2636ec90c51513d968226d96a6f59 Mon Sep 17 00:00:00 2001 From: Annanay Date: Fri, 13 Mar 2020 19:22:53 +0530 Subject: [PATCH 11/17] Remove TChan ports, remove leftover files from merge Signed-off-by: Annanay --- cmd/collector/app/builder_flags.go | 4 -- cmd/collector/app/grpcserver/grpc_server.go | 70 --------------------- cmd/flags/admin.go | 24 +++---- cmd/flags/admin_test.go | 2 +- 4 files changed, 13 insertions(+), 87 deletions(-) delete mode 100644 cmd/collector/app/grpcserver/grpc_server.go diff --git a/cmd/collector/app/builder_flags.go b/cmd/collector/app/builder_flags.go index 4478f7a9e46..a34ebc9ab1d 100644 --- a/cmd/collector/app/builder_flags.go +++ b/cmd/collector/app/builder_flags.go @@ -31,7 +31,6 @@ 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" @@ -59,8 +58,6 @@ 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 @@ -103,7 +100,6 @@ 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) diff --git a/cmd/collector/app/grpcserver/grpc_server.go b/cmd/collector/app/grpcserver/grpc_server.go deleted file mode 100644 index beabbca7d42..00000000000 --- a/cmd/collector/app/grpcserver/grpc_server.go +++ /dev/null @@ -1,70 +0,0 @@ -// 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) - } - }() -} diff --git a/cmd/flags/admin.go b/cmd/flags/admin.go index 1e4d87a0f36..1da07a37e1b 100644 --- a/cmd/flags/admin.go +++ b/cmd/flags/admin.go @@ -38,8 +38,8 @@ const ( // AdminServer runs an HTTP server with admin endpoints, such as healthcheck at /, /metrics, etc. type AdminServer struct { - logger *zap.Logger - adminAddr string + logger *zap.Logger + adminHostPort string hc *healthcheck.HealthCheck @@ -50,10 +50,10 @@ type AdminServer struct { // NewAdminServer creates a new admin server. func NewAdminServer(defaultAddr string) *AdminServer { return &AdminServer{ - adminAddr: defaultAddr, - logger: zap.NewNop(), - hc: healthcheck.New(), - mux: http.NewServeMux(), + adminHostPort: defaultAddr, + logger: zap.NewNop(), + hc: healthcheck.New(), + mux: http.NewServeMux(), } } @@ -72,14 +72,14 @@ func (s *AdminServer) setLogger(logger *zap.Logger) { func (s *AdminServer) AddFlags(flagSet *flag.FlagSet) { flagSet.Int(healthCheckHTTPPort, 0, "(deprecated) see --"+adminHTTPHostPort) flagSet.Int(adminHTTPPort, 0, "(deprecated) see --"+adminHTTPHostPort) - flagSet.String(adminHTTPHostPort, s.adminAddr, "The host:port (e.g. 127.0.0.1:5555 or :5555) for the admin server, including health check, /metrics, etc.") + 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.adminAddr = ports.PortToHostPort(v) + s.adminHostPort = ports.PortToHostPort(v) } } @@ -87,7 +87,7 @@ func (s *AdminServer) checkAndUpdate(v *viper.Viper, actualFlagName string, expe func (s *AdminServer) initFromViper(v *viper.Viper, logger *zap.Logger) { s.setLogger(logger) - s.adminAddr = v.GetString(adminHTTPHostPort) + s.adminHostPort = v.GetString(adminHTTPHostPort) s.checkAndUpdate(v, healthCheckHTTPPort, adminHTTPHostPort) s.checkAndUpdate(v, adminHTTPPort, adminHTTPHostPort) } @@ -99,7 +99,7 @@ func (s *AdminServer) Handle(path string, handler http.Handler) { // Serve starts HTTP server. func (s *AdminServer) Serve() error { - l, err := net.Listen("tcp", s.adminAddr) + l, err := net.Listen("tcp", s.adminHostPort) if err != nil { s.logger.Error("Admin server failed to listen", zap.Error(err)) return err @@ -108,7 +108,7 @@ func (s *AdminServer) Serve() error { s.logger.Info( "Admin server started", - zap.String("http.host-port", s.adminAddr), + zap.String("http.host-port", s.adminHostPort), zap.Stringer("health-status", s.hc.Get())) return nil } @@ -120,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.String("http-addr", s.adminAddr)) + 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..1846a534111 100644 --- a/cmd/flags/admin_test.go +++ b/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) From e8fa878173a8592a6283c0af27f951dc576ff6df Mon Sep 17 00:00:00 2001 From: Annanay Date: Mon, 30 Mar 2020 21:08:11 +0530 Subject: [PATCH 12/17] Address comments Signed-off-by: Annanay --- CHANGELOG.md | 4 +- cmd/all-in-one/main.go | 2 +- cmd/collector/app/builder_flags.go | 39 ++++++++++++------- cmd/collector/app/builder_flags_test.go | 52 +++++++++++++++++++++++++ cmd/collector/app/collector.go | 16 ++------ cmd/flags/admin.go | 11 ++++-- go.mod | 1 - 7 files changed, 89 insertions(+), 36 deletions(-) create mode 100644 cmd/collector/app/builder_flags_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index ab82969acae..fceaf944a84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,13 +28,13 @@ Changes by Version * 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 - + 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 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 a34ebc9ab1d..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" @@ -38,6 +39,10 @@ const ( collectorTags = "collector.tags" 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{ @@ -54,10 +59,6 @@ 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 @@ -66,8 +67,6 @@ type CollectorOptions struct { 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 @@ -80,9 +79,9 @@ type CollectorOptions struct { func AddFlags(flags *flag.FlagSet) { 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, 0, "(deprecated) please use -"+collectorHTTPHostPort) - flags.Int(collectorGRPCPort, 0, "(deprecated) please use -"+collectorGRPCHostPort) - flags.Int(collectorZipkinHTTPPort, 0, "(deprecated) please use -"+collectorZipkinHTTPHostPort) + 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") @@ -98,15 +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 = v.GetString(collectorHTTPHostPort) - cOpts.CollectorGRPCHostPort = v.GetString(collectorGRPCHostPort) - cOpts.CollectorZipkinHTTPPort = v.GetInt(collectorZipkinHTTPPort) - cOpts.CollectorZipkinHTTPHostPort = v.GetString(collectorZipkinHTTPHostPort) + 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.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..e476b68492c --- /dev/null +++ b/cmd/collector/app/builder_flags_test.go @@ -0,0 +1,52 @@ +// 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/jaegertracing/jaeger/pkg/config" + "github.com/stretchr/testify/assert" +) + +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) +} diff --git a/cmd/collector/app/collector.go b/cmd/collector/app/collector.go index bc168479bea..29e191043c5 100644 --- a/cmd/collector/app/collector.go +++ b/cmd/collector/app/collector.go @@ -27,7 +27,6 @@ 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" ) @@ -84,7 +83,7 @@ func (c *Collector) Start(builderOpts *CollectorOptions) error { c.spanHandlers = handlerBuilder.BuildHandlers(c.spanProcessor) if grpcServer, err := server.StartGRPCServer(&server.GRPCServerParams{ - HostPort: getAddressFromCLIOptions(builderOpts.CollectorGRPCPort, builderOpts.CollectorGRPCHostPort, c.logger), + HostPort: builderOpts.CollectorGRPCHostPort, Handler: c.spanHandlers.GRPCHandler, TLSConfig: builderOpts.TLS, SamplingStore: c.strategyStore, @@ -96,7 +95,7 @@ func (c *Collector) Start(builderOpts *CollectorOptions) error { } if httpServer, err := server.StartHTTPServer(&server.HTTPServerParams{ - HostPort: getAddressFromCLIOptions(builderOpts.CollectorHTTPPort, builderOpts.CollectorHTTPHostPort, c.logger), + HostPort: builderOpts.CollectorHTTPHostPort, Handler: c.spanHandlers.JaegerBatchesHandler, HealthCheck: c.hCheck, MetricsFactory: c.metricsFactory, @@ -109,7 +108,7 @@ func (c *Collector) Start(builderOpts *CollectorOptions) error { } if zkServer, err := server.StartZipkinServer(&server.ZipkinServerParams{ - HostPort: getAddressFromCLIOptions(builderOpts.CollectorZipkinHTTPPort, builderOpts.CollectorZipkinHTTPHostPort, c.logger), + HostPort: builderOpts.CollectorZipkinHTTPHostPort, Handler: c.spanHandlers.ZipkinSpansHandler, AllowedHeaders: builderOpts.CollectorZipkinAllowedHeaders, AllowedOrigins: builderOpts.CollectorZipkinAllowedOrigins, @@ -161,12 +160,3 @@ 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 -} diff --git a/cmd/flags/admin.go b/cmd/flags/admin.go index 1da07a37e1b..9de729addd5 100644 --- a/cmd/flags/admin.go +++ b/cmd/flags/admin.go @@ -34,6 +34,9 @@ const ( 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. @@ -48,9 +51,9 @@ type AdminServer struct { } // NewAdminServer creates a new admin server. -func NewAdminServer(defaultAddr string) *AdminServer { +func NewAdminServer(hostPort string) *AdminServer { return &AdminServer{ - adminHostPort: defaultAddr, + adminHostPort: hostPort, logger: zap.NewNop(), hc: healthcheck.New(), mux: http.NewServeMux(), @@ -70,8 +73,8 @@ func (s *AdminServer) setLogger(logger *zap.Logger) { // AddFlags registers CLI flags. func (s *AdminServer) AddFlags(flagSet *flag.FlagSet) { - flagSet.Int(healthCheckHTTPPort, 0, "(deprecated) see --"+adminHTTPHostPort) - flagSet.Int(adminHTTPPort, 0, "(deprecated) see --"+adminHTTPHostPort) + 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.") } diff --git a/go.mod b/go.mod index e6bccf55d1f..04ea3fb9886 100644 --- a/go.mod +++ b/go.mod @@ -52,7 +52,6 @@ require ( github.com/opentracing/opentracing-go v1.1.0 github.com/pelletier/go-toml v1.6.0 // indirect github.com/pierrec/lz4 v2.4.1+incompatible // indirect - github.com/pkg/errors v0.9.1 github.com/prashantv/protectmem v0.0.0-20171002184600-e20412882b3a // indirect github.com/prometheus/client_golang v1.1.0 github.com/prometheus/common v0.9.1 // indirect From 10325bc98fcfdc8561d255fc1d92f735998b5b30 Mon Sep 17 00:00:00 2001 From: Annanay Date: Tue, 31 Mar 2020 12:37:02 +0530 Subject: [PATCH 13/17] Fix go.mod, edit util function name Signed-off-by: Annanay --- cmd/collector/app/builder_flags_test.go | 3 ++- cmd/flags/admin.go | 8 ++++---- go.mod | 1 + 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/cmd/collector/app/builder_flags_test.go b/cmd/collector/app/builder_flags_test.go index e476b68492c..0ffa0e8ec1b 100644 --- a/cmd/collector/app/builder_flags_test.go +++ b/cmd/collector/app/builder_flags_test.go @@ -17,8 +17,9 @@ package app import ( "testing" - "github.com/jaegertracing/jaeger/pkg/config" "github.com/stretchr/testify/assert" + + "github.com/jaegertracing/jaeger/pkg/config" ) func TestCollectorOptionsWithFlags_CheckHostPort(t *testing.T) { diff --git a/cmd/flags/admin.go b/cmd/flags/admin.go index 9de729addd5..060386adf09 100644 --- a/cmd/flags/admin.go +++ b/cmd/flags/admin.go @@ -78,8 +78,8 @@ func (s *AdminServer) AddFlags(flagSet *flag.FlagSet) { 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) { +// 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) @@ -91,8 +91,8 @@ func (s *AdminServer) initFromViper(v *viper.Viper, logger *zap.Logger) { s.setLogger(logger) s.adminHostPort = v.GetString(adminHTTPHostPort) - s.checkAndUpdate(v, healthCheckHTTPPort, adminHTTPHostPort) - s.checkAndUpdate(v, adminHTTPPort, adminHTTPHostPort) + s.checkDeprecatedFlag(v, healthCheckHTTPPort, adminHTTPHostPort) + s.checkDeprecatedFlag(v, adminHTTPPort, adminHTTPHostPort) } // Handle adds a new handler to the admin server. diff --git a/go.mod b/go.mod index 04ea3fb9886..e2b440cdf6d 100644 --- a/go.mod +++ b/go.mod @@ -52,6 +52,7 @@ require ( github.com/opentracing/opentracing-go v1.1.0 github.com/pelletier/go-toml v1.6.0 // indirect github.com/pierrec/lz4 v2.4.1+incompatible // indirect + github.com/pkg/errors v0.9.1 // indirect github.com/prashantv/protectmem v0.0.0-20171002184600-e20412882b3a // indirect github.com/prometheus/client_golang v1.1.0 github.com/prometheus/common v0.9.1 // indirect From 9097ce5431b97350d5ad8fc551ca6e1bae5b6780 Mon Sep 17 00:00:00 2001 From: Annanay Date: Tue, 31 Mar 2020 20:24:56 +0530 Subject: [PATCH 14/17] Fix test, print address in log Signed-off-by: Annanay --- cmd/flags/admin.go | 2 +- cmd/flags/admin_test.go | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/cmd/flags/admin.go b/cmd/flags/admin.go index 060386adf09..fab53f2afa7 100644 --- a/cmd/flags/admin.go +++ b/cmd/flags/admin.go @@ -111,7 +111,7 @@ func (s *AdminServer) Serve() error { s.logger.Info( "Admin server started", - zap.String("http.host-port", s.adminHostPort), + zap.String("http.host-port", l.Addr().String()), zap.Stringer("health-status", s.hc.Get())) return nil } diff --git a/cmd/flags/admin_test.go b/cmd/flags/admin_test.go index 1846a534111..dd04c070eee 100644 --- a/cmd/flags/admin_test.go +++ b/cmd/flags/admin_test.go @@ -15,6 +15,8 @@ package flags import ( + "strings" + "strconv" "testing" "github.com/stretchr/testify/assert" @@ -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) } From 6498408ca31921038e99fd26d9ec51e25197b29f Mon Sep 17 00:00:00 2001 From: Annanay Date: Tue, 31 Mar 2020 20:25:41 +0530 Subject: [PATCH 15/17] Make fmt Signed-off-by: Annanay --- cmd/flags/admin_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/flags/admin_test.go b/cmd/flags/admin_test.go index dd04c070eee..b3564b41bcc 100644 --- a/cmd/flags/admin_test.go +++ b/cmd/flags/admin_test.go @@ -15,8 +15,8 @@ package flags import ( - "strings" "strconv" + "strings" "testing" "github.com/stretchr/testify/assert" From d1e14afc55cf4e2947812f140e8e7c269f21e5d2 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 31 Mar 2020 13:37:41 -0400 Subject: [PATCH 16/17] Add small test Signed-off-by: Yuri Shkuro --- cmd/collector/app/builder_flags_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/collector/app/builder_flags_test.go b/cmd/collector/app/builder_flags_test.go index 0ffa0e8ec1b..4ee34b0f152 100644 --- a/cmd/collector/app/builder_flags_test.go +++ b/cmd/collector/app/builder_flags_test.go @@ -51,3 +51,7 @@ func TestCollectorOptionsWithFlags_CheckFullHostPort(t *testing.T) { 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, "")) +} From 0717a8d9f9a3cc39ea787504c0bc3bcc29d051a9 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 31 Mar 2020 14:08:23 -0400 Subject: [PATCH 17/17] Another silly test Signed-off-by: Yuri Shkuro --- ports/{empty_test.go => ports_test.go} | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) rename ports/{empty_test.go => ports_test.go} (74%) 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)) +}