Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize CLI flags to use host:port addresses #1827

Merged
merged 21 commits into from
Mar 31, 2020
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@ Changes by Version
--collector.port
```

* Normalize CLI flags to use host:port addresses ([#1827](https://github.com/jaegertracing/jaeger/pull/1827), [@annanay25](https://github.com/annanay25))
annanay25 marked this conversation as resolved.
Show resolved Hide resolved

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 -
annanay25 marked this conversation as resolved.
Show resolved Hide resolved

* `collector.port` is superseded by `collector.tchan-server.host-port`
* `collector.http-port` is superseded by `collector.http-server.host-port`
* `collector.grpc-port` is superseded by `collector.grpc-server.host-port`
* `collector.zipkin.http-port` is superseded by `collector.zipkin.host-port`

#### New Features

#### Bug fixes, Minor Improvements
Expand Down
31 changes: 25 additions & 6 deletions cmd/collector/app/builder_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,12 @@ const (
collectorNumWorkers = "collector.num-workers"
collectorHTTPPort = "collector.http-port"
collectorGRPCPort = "collector.grpc-port"
collectorTChanHostPort = "collector.tchan-server.host-port"
annanay25 marked this conversation as resolved.
Show resolved Hide resolved
collectorHTTPHostPort = "collector.http-server.host-port"
collectorGRPCHostPort = "collector.grpc-server.host-port"
collectorZipkinHTTPPort = "collector.zipkin.http-port"
collectorZipkinHTTPHostPort = "collector.zipkin.host-port"
collectorTags = "collector.tags"
collectorZipkinHTTPort = "collector.zipkin.http-port"
collectorZipkinAllowedOrigins = "collector.zipkin.allowed-origins"
collectorZipkinAllowedHeaders = "collector.zipkin.allowed-headers"
)
Expand All @@ -55,12 +59,20 @@ type CollectorOptions struct {
CollectorHTTPPort int
// CollectorGRPCPort is the port that the collector service listens in on for gRPC requests
CollectorGRPCPort int
// CollectorTChanHostPort is the host:port address that the collector service listens in on for tchannel requests
CollectorTChanHostPort string
// CollectorHTTPHostPort is the host:port address that the collector service listens in on for http requests
CollectorHTTPHostPort string
// CollectorGRPCHostPort is the host:port address that the collector service listens in on for gRPC requests
CollectorGRPCHostPort string
// TLS configures secure transport
TLS tlscfg.Options
// CollectorTags is the string representing collector tags to append to each and every span
CollectorTags map[string]string
// CollectorZipkinHTTPPort is the port that the Zipkin collector service listens in on for http requests
CollectorZipkinHTTPPort int
// CollectorZipkinHTTPHostPort is the host:port address that the Zipkin collector service listens in on for http requests
CollectorZipkinHTTPHostPort string
// CollectorZipkinAllowedOrigins is a list of origins a cross-domain request to the Zipkin collector service can be executed from
CollectorZipkinAllowedOrigins string
// CollectorZipkinAllowedHeaders is a list of headers that the Zipkin collector service allowes the client to use with cross-domain requests
Expand All @@ -69,13 +81,16 @@ type CollectorOptions struct {

// AddFlags adds flags for CollectorOptions
func AddFlags(flags *flag.FlagSet) {
flags.Uint(collectorDynQueueSizeMemory, 0, "(experimental) The max memory size in MiB to use for the dynamic queue.")
flags.Int(collectorQueueSize, DefaultQueueSize, "The queue size of the collector")
flags.Int(collectorNumWorkers, DefaultNumWorkers, "The number of workers pulling items from the queue")
flags.Int(collectorHTTPPort, ports.CollectorHTTP, "The HTTP port for the collector service")
flags.Int(collectorGRPCPort, ports.CollectorGRPC, "The gRPC port for the collector service")
flags.Int(collectorHTTPPort, 0, "(deprecated) please use -"+collectorHTTPHostPort)
flags.Int(collectorGRPCPort, 0, "(deprecated) please use -"+collectorGRPCHostPort)
flags.Int(collectorZipkinHTTPPort, 0, "(deprecated) please use -"+collectorZipkinHTTPHostPort)
annanay25 marked this conversation as resolved.
Show resolved Hide resolved
flags.String(collectorHTTPHostPort, ports.PortToHostPort(ports.CollectorHTTP), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's HTTP server")
flags.String(collectorGRPCHostPort, ports.PortToHostPort(ports.CollectorGRPC), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's GRPC server")
flags.String(collectorZipkinHTTPHostPort, ports.PortToHostPort(0), "The host:port (e.g. 127.0.0.1:5555 or :5555) of the collector's Zipkin server")
flags.Uint(collectorDynQueueSizeMemory, 0, "(experimental) The max memory size in MiB to use for the dynamic queue.")
flags.String(collectorTags, "", "One or more tags to be added to the Process tags of all spans passing through this collector. Ex: key1=value1,key2=${envVar:defaultValue}")
flags.Int(collectorZipkinHTTPort, 0, "The HTTP port for the Zipkin collector service e.g. 9411")
flags.String(collectorZipkinAllowedOrigins, "*", "Comma separated list of allowed origins for the Zipkin collector service, default accepts all")
flags.String(collectorZipkinAllowedHeaders, "content-type", "Comma separated list of allowed headers for the Zipkin collector service, default content-type")
tlsFlagsConfig.AddFlags(flags)
Expand All @@ -88,8 +103,12 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper) *CollectorOptions {
cOpts.NumWorkers = v.GetInt(collectorNumWorkers)
cOpts.CollectorHTTPPort = v.GetInt(collectorHTTPPort)
cOpts.CollectorGRPCPort = v.GetInt(collectorGRPCPort)
cOpts.CollectorTChanHostPort = v.GetString(collectorTChanHostPort)
cOpts.CollectorHTTPHostPort = v.GetString(collectorHTTPHostPort)
cOpts.CollectorGRPCHostPort = v.GetString(collectorGRPCHostPort)
cOpts.CollectorZipkinHTTPPort = v.GetInt(collectorZipkinHTTPPort)
cOpts.CollectorZipkinHTTPHostPort = v.GetString(collectorZipkinHTTPHostPort)
annanay25 marked this conversation as resolved.
Show resolved Hide resolved
cOpts.CollectorTags = flags.ParseJaegerTags(v.GetString(collectorTags))
cOpts.CollectorZipkinHTTPPort = v.GetInt(collectorZipkinHTTPort)
cOpts.CollectorZipkinAllowedOrigins = v.GetString(collectorZipkinAllowedOrigins)
cOpts.CollectorZipkinAllowedHeaders = v.GetString(collectorZipkinAllowedHeaders)
cOpts.TLS = tlsFlagsConfig.InitFromViper(v)
Expand Down
16 changes: 13 additions & 3 deletions cmd/collector/app/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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
}
70 changes: 70 additions & 0 deletions cmd/collector/app/grpcserver/grpc_server.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright (c) 2018 The Jaeger Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package grpcserver

import (
"io/ioutil"
"net"
"os"
"strconv"

"github.com/pkg/errors"
"go.uber.org/zap"
"google.golang.org/grpc"
"google.golang.org/grpc/grpclog"

"github.com/jaegertracing/jaeger/cmd/collector/app"
"github.com/jaegertracing/jaeger/cmd/collector/app/sampling"
"github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore"
"github.com/jaegertracing/jaeger/proto-gen/api_v2"
)

// StartGRPCCollector configures and starts gRPC endpoints exposed by collector.
func StartGRPCCollector(
addr string,
server *grpc.Server,
handler *app.GRPCHandler,
samplingStrategy strategystore.StrategyStore,
logger *zap.Logger,
serveErr func(error),
) (net.Addr, error) {
lis, err := net.Listen("tcp", addr)
if err != nil {
return nil, errors.Wrap(err, "failed to listen on gRPC port")
}

grpclog.SetLoggerV2(grpclog.NewLoggerV2(ioutil.Discard, os.Stderr, os.Stderr))

api_v2.RegisterCollectorServiceServer(server, handler)
api_v2.RegisterSamplingManagerServer(server, sampling.NewGRPCHandler(samplingStrategy))
startServer(server, lis, logger, serveErr)
return lis.Addr(), nil
}

func startServer(server *grpc.Server, lis net.Listener, logger *zap.Logger, serveErr func(error)) {
var port string
if tcpAddr, ok := lis.Addr().(*net.TCPAddr); ok {
port = strconv.Itoa(tcpAddr.Port)
} else {
port = lis.Addr().Network()
}
logger.Info("Starting jaeger-collector gRPC server", zap.String("grpc-port", port))
go func() {
if err := server.Serve(lis); err != nil {
logger.Error("Could not launch gRPC service", zap.Error(err))
serveErr(err)
}
}()
}
8 changes: 3 additions & 5 deletions cmd/collector/app/server/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package server
import (
"fmt"
"net"
"strconv"

"go.uber.org/zap"
"google.golang.org/grpc"
Expand All @@ -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
Expand All @@ -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)
}
Expand All @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion cmd/collector/app/server/grpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 4 additions & 6 deletions cmd/collector/app/server/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package server
import (
"net"
"net/http"
"strconv"

"github.com/gorilla/mux"
"github.com/uber/jaeger-lib/metrics"
Expand All @@ -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
Expand All @@ -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
Expand Down
12 changes: 5 additions & 7 deletions cmd/collector/app/server/zipkin.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package server
import (
"net"
"net/http"
"strconv"
"strings"

"github.com/gorilla/mux"
Expand All @@ -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
Expand All @@ -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
Expand Down
Loading