From 641ddf2860288975518c37c3efdac2761d39b0a6 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Fri, 29 Nov 2024 22:19:34 +0530 Subject: [PATCH 01/18] use otel helper in collector Signed-off-by: chahatsagarmain --- cmd/collector/app/handler/otlp_receiver.go | 20 ++----------------- .../app/handler/zipkin_receiver_tls_test.go | 5 +---- cmd/collector/app/server/grpc.go | 3 ++- cmd/collector/app/server/grpc_test.go | 1 - cmd/collector/app/server/http.go | 3 ++- cmd/collector/app/server/http_test.go | 6 ++---- 6 files changed, 9 insertions(+), 29 deletions(-) diff --git a/cmd/collector/app/handler/otlp_receiver.go b/cmd/collector/app/handler/otlp_receiver.go index fb54d931bf8..c69956dbdfb 100644 --- a/cmd/collector/app/handler/otlp_receiver.go +++ b/cmd/collector/app/handler/otlp_receiver.go @@ -13,7 +13,6 @@ import ( "go.opentelemetry.io/collector/config/configgrpc" "go.opentelemetry.io/collector/config/confighttp" "go.opentelemetry.io/collector/config/configtelemetry" - "go.opentelemetry.io/collector/config/configtls" "go.opentelemetry.io/collector/consumer" "go.opentelemetry.io/collector/extension" "go.opentelemetry.io/collector/pdata/ptrace" @@ -27,7 +26,6 @@ import ( "github.com/jaegertracing/jaeger/cmd/collector/app/flags" "github.com/jaegertracing/jaeger/cmd/collector/app/processor" - "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/tenancy" ) @@ -106,7 +104,7 @@ func applyGRPCSettings(cfg *configgrpc.ServerConfig, opts *flags.GRPCOptions) { cfg.NetAddr.Endpoint = opts.HostPort } if opts.TLS.Enabled { - cfg.TLSSetting = applyTLSSettings(&opts.TLS) + cfg.TLSSetting = opts.TLS.ToOtelServerConfig() } if opts.MaxReceiveMessageLength > 0 { cfg.MaxRecvMsgSizeMiB = int(opts.MaxReceiveMessageLength / (1024 * 1024)) @@ -126,7 +124,7 @@ func applyHTTPSettings(cfg *confighttp.ServerConfig, opts *flags.HTTPOptions) { cfg.Endpoint = opts.HostPort } if opts.TLS.Enabled { - cfg.TLSSetting = applyTLSSettings(&opts.TLS) + cfg.TLSSetting = opts.TLS.ToOtelServerConfig() } cfg.CORS = &confighttp.CORSConfig{ @@ -135,20 +133,6 @@ func applyHTTPSettings(cfg *confighttp.ServerConfig, opts *flags.HTTPOptions) { } } -func applyTLSSettings(opts *tlscfg.Options) *configtls.ServerConfig { - return &configtls.ServerConfig{ - Config: configtls.Config{ - CAFile: opts.CAPath, - CertFile: opts.CertPath, - KeyFile: opts.KeyPath, - MinVersion: opts.MinVersion, - MaxVersion: opts.MaxVersion, - ReloadInterval: opts.ReloadInterval, - }, - ClientCAFile: opts.ClientCAPath, - } -} - func newConsumerDelegate(logger *zap.Logger, spanProcessor processor.SpanProcessor, tm *tenancy.Manager) *consumerDelegate { return &consumerDelegate{ batchConsumer: newBatchConsumer(logger, diff --git a/cmd/collector/app/handler/zipkin_receiver_tls_test.go b/cmd/collector/app/handler/zipkin_receiver_tls_test.go index 060327360a6..2a52097f22d 100644 --- a/cmd/collector/app/handler/zipkin_receiver_tls_test.go +++ b/cmd/collector/app/handler/zipkin_receiver_tls_test.go @@ -13,7 +13,6 @@ import ( "time" "github.com/stretchr/testify/require" - "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/collector/app/flags" "github.com/jaegertracing/jaeger/pkg/config/tlscfg" @@ -165,7 +164,6 @@ func TestSpanCollectorZipkinTLS(t *testing.T) { opts := &flags.CollectorOptions{} opts.Zipkin.HTTPHostPort = ports.PortToHostPort(ports.CollectorZipkin) opts.Zipkin.TLS = test.serverTLS - defer test.serverTLS.Close() server, err := StartZipkinReceiver(opts, logger, spanProcessor, tm) if test.expectServerFail { @@ -177,8 +175,7 @@ func TestSpanCollectorZipkinTLS(t *testing.T) { require.NoError(t, server.Shutdown(context.Background())) }() - clientTLSCfg, err0 := test.clientTLS.Config(zap.NewNop()) - defer test.clientTLS.Close() + clientTLSCfg, err0 := test.clientTLS.ToOtelClientConfig().LoadTLSConfig(context.Background()) require.NoError(t, err0) dialer := &net.Dialer{Timeout: 2 * time.Second} conn, clientError := tls.DialWithDialer(dialer, "tcp", fmt.Sprintf("localhost:%d", ports.CollectorZipkin), clientTLSCfg) diff --git a/cmd/collector/app/server/grpc.go b/cmd/collector/app/server/grpc.go index a740fe5b8fe..d08ec69fc99 100644 --- a/cmd/collector/app/server/grpc.go +++ b/cmd/collector/app/server/grpc.go @@ -4,6 +4,7 @@ package server import ( + "context" "fmt" "net" "time" @@ -54,7 +55,7 @@ func StartGRPCServer(params *GRPCServerParams) (*grpc.Server, error) { if params.TLSConfig.Enabled { // user requested a server with TLS, setup creds - tlsCfg, err := params.TLSConfig.Config(params.Logger) + tlsCfg, err := params.TLSConfig.ToOtelServerConfig().LoadTLSConfig(context.Background()) if err != nil { return nil, err } diff --git a/cmd/collector/app/server/grpc_test.go b/cmd/collector/app/server/grpc_test.go index 19594ebcfa7..620a1668e85 100644 --- a/cmd/collector/app/server/grpc_test.go +++ b/cmd/collector/app/server/grpc_test.go @@ -101,7 +101,6 @@ func TestCollectorStartWithTLS(t *testing.T) { server, err := StartGRPCServer(params) require.NoError(t, err) defer server.Stop() - defer params.TLSConfig.Close() } func TestCollectorReflection(t *testing.T) { diff --git a/cmd/collector/app/server/http.go b/cmd/collector/app/server/http.go index c519e6df495..b02a4319b36 100644 --- a/cmd/collector/app/server/http.go +++ b/cmd/collector/app/server/http.go @@ -4,6 +4,7 @@ package server import ( + "context" "net" "net/http" "time" @@ -53,7 +54,7 @@ func StartHTTPServer(params *HTTPServerParams) (*http.Server, error) { ErrorLog: errorLog, } if params.TLSConfig.Enabled { - tlsCfg, err := params.TLSConfig.Config(params.Logger) // This checks if the certificates are correctly provided + tlsCfg, err := params.TLSConfig.ToOtelServerConfig().LoadTLSConfig(context.Background()) if err != nil { return nil, err } diff --git a/cmd/collector/app/server/http_test.go b/cmd/collector/app/server/http_test.go index 46184bd8b89..82b14b77916 100644 --- a/cmd/collector/app/server/http_test.go +++ b/cmd/collector/app/server/http_test.go @@ -4,6 +4,7 @@ package server import ( + "context" "crypto/tls" "fmt" "net" @@ -54,7 +55,6 @@ func TestCreateTLSHTTPServerError(t *testing.T) { } _, err := StartHTTPServer(params) require.Error(t, err) - defer params.TLSConfig.Close() } func TestSpanCollectorHTTP(t *testing.T) { @@ -196,7 +196,6 @@ func TestSpanCollectorHTTPS(t *testing.T) { Logger: logger, TLSConfig: test.TLS, } - defer params.TLSConfig.Close() server, err := StartHTTPServer(params) require.NoError(t, err) @@ -204,8 +203,7 @@ func TestSpanCollectorHTTPS(t *testing.T) { require.NoError(t, server.Close()) }() - clientTLSCfg, err0 := test.clientTLS.Config(logger) - defer test.clientTLS.Close() + clientTLSCfg, err0 := test.clientTLS.ToOtelClientConfig().LoadTLSConfig(context.Background()) require.NoError(t, err0) dialer := &net.Dialer{Timeout: 2 * time.Second} conn, clientError := tls.DialWithDialer(dialer, "tcp", "localhost:"+strconv.Itoa(ports.CollectorHTTP), clientTLSCfg) From 9bb39e276742e8224e86af3ab1541df7b8f16c42 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Fri, 29 Nov 2024 23:49:47 +0530 Subject: [PATCH 02/18] add server config instead of options Signed-off-by: chahatsagarmain --- cmd/collector/app/collector.go | 27 ++----------- cmd/collector/app/flags/flags.go | 13 +++--- cmd/collector/app/handler/otlp_receiver.go | 8 ++-- .../app/handler/otlp_receiver_test.go | 40 ++++++++++--------- .../app/handler/zipkin_receiver_tls_test.go | 2 +- cmd/collector/app/server/grpc.go | 8 ++-- cmd/collector/app/server/grpc_test.go | 13 +++--- cmd/collector/app/server/http.go | 10 ++--- cmd/collector/app/server/http_test.go | 4 +- 9 files changed, 55 insertions(+), 70 deletions(-) diff --git a/cmd/collector/app/collector.go b/cmd/collector/app/collector.go index 44b1ef47803..ef1122e9341 100644 --- a/cmd/collector/app/collector.go +++ b/cmd/collector/app/collector.go @@ -6,7 +6,6 @@ package app import ( "context" "fmt" - "io" "net/http" "time" @@ -47,13 +46,10 @@ type Collector struct { tenancyMgr *tenancy.Manager // state, read only - hServer *http.Server - grpcServer *grpc.Server - otlpReceiver receiver.Traces - zipkinReceiver receiver.Traces - tlsGRPCCertWatcherCloser io.Closer - tlsHTTPCertWatcherCloser io.Closer - tlsZipkinCertWatcherCloser io.Closer + hServer *http.Server + grpcServer *grpc.Server + otlpReceiver receiver.Traces + zipkinReceiver receiver.Traces } // CollectorParams to construct a new Jaeger Collector. @@ -131,10 +127,6 @@ func (c *Collector) Start(options *flags.CollectorOptions) error { } c.hServer = httpServer - c.tlsGRPCCertWatcherCloser = &options.GRPC.TLS - c.tlsHTTPCertWatcherCloser = &options.HTTP.TLS - c.tlsZipkinCertWatcherCloser = &options.Zipkin.TLS - if options.Zipkin.HTTPHostPort == "" { c.logger.Info("Not listening for Zipkin HTTP traffic, port not configured") } else { @@ -209,17 +201,6 @@ func (c *Collector) Close() error { } } - // watchers actually never return errors from Close - if c.tlsGRPCCertWatcherCloser != nil { - _ = c.tlsGRPCCertWatcherCloser.Close() - } - if c.tlsHTTPCertWatcherCloser != nil { - _ = c.tlsHTTPCertWatcherCloser.Close() - } - if c.tlsZipkinCertWatcherCloser != nil { - _ = c.tlsZipkinCertWatcherCloser.Close() - } - return nil } diff --git a/cmd/collector/app/flags/flags.go b/cmd/collector/app/flags/flags.go index 5c68169c881..f6b556b0f2b 100644 --- a/cmd/collector/app/flags/flags.go +++ b/cmd/collector/app/flags/flags.go @@ -10,6 +10,7 @@ import ( "time" "github.com/spf13/viper" + "go.opentelemetry.io/collector/config/configtls" "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/internal/flags" @@ -120,7 +121,7 @@ type CollectorOptions struct { // HTTPHostPort is the host:port address that the Zipkin collector service listens in on for http requests HTTPHostPort string // TLS configures secure transport for Zipkin endpoint to collect spans - TLS tlscfg.Options + TLS *configtls.ServerConfig // CORS allows CORS requests , sets the values for Allowed Headers and Allowed Origins. CORS corscfg.Options // KeepAlive configures allow Keep-Alive for Zipkin HTTP server @@ -142,7 +143,7 @@ type HTTPOptions struct { // HostPort is the host:port address that the server listens on HostPort string // TLS configures secure transport for HTTP endpoint - TLS tlscfg.Options + TLS *configtls.ServerConfig // ReadTimeout sets the respective parameter of http.Server ReadTimeout time.Duration // ReadHeaderTimeout sets the respective parameter of http.Server @@ -158,7 +159,7 @@ type GRPCOptions struct { // HostPort is the host:port address that the collector service listens in on for gRPC requests HostPort string // TLS configures secure transport for gRPC endpoint to collect spans - TLS tlscfg.Options + TLS *configtls.ServerConfig // MaxReceiveMessageLength is the maximum message size receivable by the gRPC Collector. MaxReceiveMessageLength int // MaxConnectionAge is a duration for the maximum amount of time a connection may exist. @@ -232,7 +233,7 @@ func (opts *HTTPOptions) initFromViper(v *viper.Viper, _ *zap.Logger, cfg server if err != nil { return fmt.Errorf("failed to parse HTTP TLS options: %w", err) } - opts.TLS = tlsOpts + opts.TLS = tlsOpts.ToOtelServerConfig() return nil } @@ -245,7 +246,7 @@ func (opts *GRPCOptions) initFromViper(v *viper.Viper, _ *zap.Logger, cfg server if err != nil { return fmt.Errorf("failed to parse gRPC TLS options: %w", err) } - opts.TLS = tlsOpts + opts.TLS = tlsOpts.ToOtelServerConfig() opts.Tenancy = tenancy.InitFromViper(v) return nil @@ -282,7 +283,7 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) if err != nil { return cOpts, fmt.Errorf("failed to parse Zipkin TLS options: %w", err) } - cOpts.Zipkin.TLS = tlsZipkin + cOpts.Zipkin.TLS = tlsZipkin.ToOtelServerConfig() cOpts.Zipkin.CORS = corsZipkinFlags.InitFromViper(v) return cOpts, nil diff --git a/cmd/collector/app/handler/otlp_receiver.go b/cmd/collector/app/handler/otlp_receiver.go index c69956dbdfb..1517d3de2df 100644 --- a/cmd/collector/app/handler/otlp_receiver.go +++ b/cmd/collector/app/handler/otlp_receiver.go @@ -103,8 +103,8 @@ func applyGRPCSettings(cfg *configgrpc.ServerConfig, opts *flags.GRPCOptions) { if opts.HostPort != "" { cfg.NetAddr.Endpoint = opts.HostPort } - if opts.TLS.Enabled { - cfg.TLSSetting = opts.TLS.ToOtelServerConfig() + if opts.TLS != nil { + cfg.TLSSetting = opts.TLS } if opts.MaxReceiveMessageLength > 0 { cfg.MaxRecvMsgSizeMiB = int(opts.MaxReceiveMessageLength / (1024 * 1024)) @@ -123,8 +123,8 @@ func applyHTTPSettings(cfg *confighttp.ServerConfig, opts *flags.HTTPOptions) { if opts.HostPort != "" { cfg.Endpoint = opts.HostPort } - if opts.TLS.Enabled { - cfg.TLSSetting = opts.TLS.ToOtelServerConfig() + if opts.TLS != nil { + cfg.TLSSetting = opts.TLS } cfg.CORS = &confighttp.CORSConfig{ diff --git a/cmd/collector/app/handler/otlp_receiver_test.go b/cmd/collector/app/handler/otlp_receiver_test.go index 1f18c3d07bb..cdc67603533 100644 --- a/cmd/collector/app/handler/otlp_receiver_test.go +++ b/cmd/collector/app/handler/otlp_receiver_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/config/configtls" "go.opentelemetry.io/collector/consumer" "go.opentelemetry.io/collector/pdata/ptrace" "go.opentelemetry.io/collector/pipeline" @@ -20,7 +21,6 @@ import ( "github.com/jaegertracing/jaeger/cmd/collector/app/flags" "github.com/jaegertracing/jaeger/pkg/config/corscfg" - "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/tenancy" "github.com/jaegertracing/jaeger/pkg/testutils" ) @@ -138,15 +138,16 @@ func TestApplyOTLPGRPCServerSettings(t *testing.T) { MaxReceiveMessageLength: 42 * 1024 * 1024, MaxConnectionAge: 33 * time.Second, MaxConnectionAgeGrace: 37 * time.Second, - TLS: tlscfg.Options{ - Enabled: true, - CAPath: "ca", - CertPath: "cert", - KeyPath: "key", - ClientCAPath: "clientca", - MinVersion: "1.1", - MaxVersion: "1.3", - ReloadInterval: 24 * time.Hour, + TLS: &configtls.ServerConfig{ + ClientCAFile: "clientca", + Config: configtls.Config{ + CAFile: "ca", + CertFile: "cert", + KeyFile: "key", + MinVersion: "1.1", + MaxVersion: "1.3", + ReloadInterval: 24 * time.Hour, + }, }, } applyGRPCSettings(otlpReceiverConfig.GRPC, grpcOpts) @@ -173,15 +174,16 @@ func TestApplyOTLPHTTPServerSettings(t *testing.T) { httpOpts := &flags.HTTPOptions{ HostPort: ":12345", - TLS: tlscfg.Options{ - Enabled: true, - CAPath: "ca", - CertPath: "cert", - KeyPath: "key", - ClientCAPath: "clientca", - MinVersion: "1.1", - MaxVersion: "1.3", - ReloadInterval: 24 * time.Hour, + TLS: &configtls.ServerConfig{ + ClientCAFile: "clientca", + Config: configtls.Config{ + CAFile: "ca", + CertFile: "cert", + KeyFile: "key", + MinVersion: "1.1", + MaxVersion: "1.3", + ReloadInterval: 24 * time.Hour, + }, }, CORS: corscfg.Options{ AllowedOrigins: []string{"http://example.domain.com", "http://*.domain.com"}, diff --git a/cmd/collector/app/handler/zipkin_receiver_tls_test.go b/cmd/collector/app/handler/zipkin_receiver_tls_test.go index 2a52097f22d..5da0b6694d5 100644 --- a/cmd/collector/app/handler/zipkin_receiver_tls_test.go +++ b/cmd/collector/app/handler/zipkin_receiver_tls_test.go @@ -163,7 +163,7 @@ func TestSpanCollectorZipkinTLS(t *testing.T) { opts := &flags.CollectorOptions{} opts.Zipkin.HTTPHostPort = ports.PortToHostPort(ports.CollectorZipkin) - opts.Zipkin.TLS = test.serverTLS + opts.Zipkin.TLS = test.serverTLS.ToOtelServerConfig() server, err := StartZipkinReceiver(opts, logger, spanProcessor, tm) if test.expectServerFail { diff --git a/cmd/collector/app/server/grpc.go b/cmd/collector/app/server/grpc.go index d08ec69fc99..b9fbdbf41da 100644 --- a/cmd/collector/app/server/grpc.go +++ b/cmd/collector/app/server/grpc.go @@ -9,6 +9,7 @@ import ( "net" "time" + "go.opentelemetry.io/collector/config/configtls" "go.uber.org/zap" "google.golang.org/grpc" "google.golang.org/grpc/credentials" @@ -20,13 +21,12 @@ import ( "github.com/jaegertracing/jaeger/cmd/collector/app/handler" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/samplingstrategy" - "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/proto-gen/api_v2" ) // GRPCServerParams to construct a new Jaeger Collector gRPC Server type GRPCServerParams struct { - TLSConfig tlscfg.Options + TLSConfig *configtls.ServerConfig HostPort string Handler *handler.GRPCHandler SamplingProvider samplingstrategy.Provider @@ -53,9 +53,9 @@ func StartGRPCServer(params *GRPCServerParams) (*grpc.Server, error) { MaxConnectionAgeGrace: params.MaxConnectionAgeGrace, })) - if params.TLSConfig.Enabled { + if params.TLSConfig != nil { // user requested a server with TLS, setup creds - tlsCfg, err := params.TLSConfig.ToOtelServerConfig().LoadTLSConfig(context.Background()) + tlsCfg, err := params.TLSConfig.LoadTLSConfig(context.Background()) if err != nil { return nil, err } diff --git a/cmd/collector/app/server/grpc_test.go b/cmd/collector/app/server/grpc_test.go index 620a1668e85..d16623138dc 100644 --- a/cmd/collector/app/server/grpc_test.go +++ b/cmd/collector/app/server/grpc_test.go @@ -87,16 +87,17 @@ func TestSpanCollector(t *testing.T) { func TestCollectorStartWithTLS(t *testing.T) { logger, _ := zap.NewDevelopment() + opts := tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", + } params := &GRPCServerParams{ Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}, &tenancy.Manager{}), SamplingProvider: &mockSamplingProvider{}, Logger: logger, - TLSConfig: tlscfg.Options{ - Enabled: true, - CertPath: testCertKeyLocation + "/example-server-cert.pem", - KeyPath: testCertKeyLocation + "/example-server-key.pem", - ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", - }, + TLSConfig: opts.ToOtelServerConfig(), } server, err := StartGRPCServer(params) require.NoError(t, err) diff --git a/cmd/collector/app/server/http.go b/cmd/collector/app/server/http.go index b02a4319b36..411df873b2e 100644 --- a/cmd/collector/app/server/http.go +++ b/cmd/collector/app/server/http.go @@ -10,13 +10,13 @@ import ( "time" "github.com/gorilla/mux" + "go.opentelemetry.io/collector/config/configtls" "go.uber.org/zap" "go.uber.org/zap/zapcore" "github.com/jaegertracing/jaeger/cmd/collector/app/handler" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/samplingstrategy" clientcfgHandler "github.com/jaegertracing/jaeger/pkg/clientcfg/clientcfghttp" - "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/healthcheck" "github.com/jaegertracing/jaeger/pkg/httpmetrics" "github.com/jaegertracing/jaeger/pkg/metrics" @@ -25,7 +25,7 @@ import ( // HTTPServerParams to construct a new Jaeger Collector HTTP Server type HTTPServerParams struct { - TLSConfig tlscfg.Options + TLSConfig *configtls.ServerConfig HostPort string Handler handler.JaegerBatchesHandler SamplingProvider samplingstrategy.Provider @@ -53,8 +53,8 @@ func StartHTTPServer(params *HTTPServerParams) (*http.Server, error) { IdleTimeout: params.IdleTimeout, ErrorLog: errorLog, } - if params.TLSConfig.Enabled { - tlsCfg, err := params.TLSConfig.ToOtelServerConfig().LoadTLSConfig(context.Background()) + if params.TLSConfig != nil { + tlsCfg, err := params.TLSConfig.LoadTLSConfig(context.Background()) if err != nil { return nil, err } @@ -91,7 +91,7 @@ func serveHTTP(server *http.Server, listener net.Listener, params *HTTPServerPar server.Handler = httpmetrics.Wrap(recoveryHandler(r), params.MetricsFactory, params.Logger) go func() { var err error - if params.TLSConfig.Enabled { + if params.TLSConfig != nil { err = server.ServeTLS(listener, "", "") } else { err = server.Serve(listener) diff --git a/cmd/collector/app/server/http_test.go b/cmd/collector/app/server/http_test.go index 82b14b77916..b72cfc54039 100644 --- a/cmd/collector/app/server/http_test.go +++ b/cmd/collector/app/server/http_test.go @@ -51,7 +51,7 @@ func TestCreateTLSHTTPServerError(t *testing.T) { HostPort: fmt.Sprintf(":%d", ports.CollectorHTTP), HealthCheck: healthcheck.New(), Logger: logger, - TLSConfig: tlsCfg, + TLSConfig: tlsCfg.ToOtelServerConfig(), } _, err := StartHTTPServer(params) require.Error(t, err) @@ -194,7 +194,7 @@ func TestSpanCollectorHTTPS(t *testing.T) { MetricsFactory: mFact, HealthCheck: healthcheck.New(), Logger: logger, - TLSConfig: test.TLS, + TLSConfig: test.TLS.ToOtelServerConfig(), } server, err := StartHTTPServer(params) From 4bf8eaa33defa3dfaf7b5a97ddce56bca7cd3451 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Sat, 30 Nov 2024 02:35:19 +0530 Subject: [PATCH 03/18] migrate to otel config Signed-off-by: chahatsagarmain --- cmd/collector/app/collector.go | 14 ++-- cmd/collector/app/collector_test.go | 16 ++-- cmd/collector/app/flags/flags.go | 78 ++++++----------- cmd/collector/app/flags/flags_test.go | 84 +++++++++---------- cmd/collector/app/handler/otlp_receiver.go | 30 +++---- .../app/handler/otlp_receiver_test.go | 40 +++++---- cmd/collector/app/handler/zipkin_receiver.go | 7 +- .../app/handler/zipkin_receiver_tls_test.go | 2 +- pkg/config/corscfg/options.go | 9 ++ 9 files changed, 135 insertions(+), 145 deletions(-) diff --git a/cmd/collector/app/collector.go b/cmd/collector/app/collector.go index ef1122e9341..67469c8bb79 100644 --- a/cmd/collector/app/collector.go +++ b/cmd/collector/app/collector.go @@ -99,14 +99,14 @@ func (c *Collector) Start(options *flags.CollectorOptions) error { c.spanHandlers = handlerBuilder.BuildHandlers(c.spanProcessor) grpcServer, err := server.StartGRPCServer(&server.GRPCServerParams{ - HostPort: options.GRPC.HostPort, + HostPort: options.GRPC.NetAddr.Endpoint, Handler: c.spanHandlers.GRPCHandler, - TLSConfig: options.GRPC.TLS, + TLSConfig: options.GRPC.TLSSetting, SamplingProvider: c.samplingProvider, Logger: c.logger, - MaxReceiveMessageLength: options.GRPC.MaxReceiveMessageLength, - MaxConnectionAge: options.GRPC.MaxConnectionAge, - MaxConnectionAgeGrace: options.GRPC.MaxConnectionAgeGrace, + MaxReceiveMessageLength: options.GRPC.MaxRecvMsgSizeMiB, + MaxConnectionAge: options.GRPC.Keepalive.ServerParameters.MaxConnectionAge, + MaxConnectionAgeGrace: options.GRPC.Keepalive.ServerParameters.MaxConnectionAgeGrace, }) if err != nil { return fmt.Errorf("could not start gRPC server: %w", err) @@ -114,9 +114,9 @@ func (c *Collector) Start(options *flags.CollectorOptions) error { c.grpcServer = grpcServer httpServer, err := server.StartHTTPServer(&server.HTTPServerParams{ - HostPort: options.HTTP.HostPort, + HostPort: options.HTTP.Endpoint, Handler: c.spanHandlers.JaegerBatchesHandler, - TLSConfig: options.HTTP.TLS, + TLSConfig: options.HTTP.TLSSetting, HealthCheck: c.hCheck, MetricsFactory: c.metricsFactory, SamplingProvider: c.samplingProvider, diff --git a/cmd/collector/app/collector_test.go b/cmd/collector/app/collector_test.go index 35c0576d630..0089e846e0f 100644 --- a/cmd/collector/app/collector_test.go +++ b/cmd/collector/app/collector_test.go @@ -28,11 +28,11 @@ var _ (io.Closer) = (*Collector)(nil) func optionsForEphemeralPorts() *flags.CollectorOptions { collectorOpts := &flags.CollectorOptions{} - collectorOpts.GRPC.HostPort = ":0" - collectorOpts.HTTP.HostPort = ":0" + collectorOpts.GRPC.NetAddr.Endpoint = ":0" + collectorOpts.HTTP.Endpoint = ":0" collectorOpts.OTLP.Enabled = true - collectorOpts.OTLP.GRPC.HostPort = ":0" - collectorOpts.OTLP.HTTP.HostPort = ":0" + collectorOpts.OTLP.GRPC.NetAddr.Endpoint = ":0" + collectorOpts.OTLP.HTTP.Endpoint = ":0" collectorOpts.Zipkin.HTTPHostPort = ":0" return collectorOpts } @@ -112,11 +112,11 @@ func TestCollector_StartErrors(t *testing.T) { var options *flags.CollectorOptions options = optionsForEphemeralPorts() - options.GRPC.HostPort = ":-1" + options.GRPC.NetAddr.Endpoint = ":-1" run("gRPC", options, "could not start gRPC server") options = optionsForEphemeralPorts() - options.HTTP.HostPort = ":-1" + options.HTTP.Endpoint = ":-1" run("HTTP", options, "could not start HTTP server") options = optionsForEphemeralPorts() @@ -124,11 +124,11 @@ func TestCollector_StartErrors(t *testing.T) { run("Zipkin", options, "could not start Zipkin receiver") options = optionsForEphemeralPorts() - options.OTLP.GRPC.HostPort = ":-1" + options.OTLP.GRPC.NetAddr.Endpoint = ":-1" run("OTLP/GRPC", options, "could not start OTLP receiver") options = optionsForEphemeralPorts() - options.OTLP.HTTP.HostPort = ":-1" + options.OTLP.HTTP.Endpoint = ":-1" run("OTLP/HTTP", options, "could not start OTLP receiver") } diff --git a/cmd/collector/app/flags/flags.go b/cmd/collector/app/flags/flags.go index f6b556b0f2b..dfd3d95a212 100644 --- a/cmd/collector/app/flags/flags.go +++ b/cmd/collector/app/flags/flags.go @@ -10,6 +10,8 @@ import ( "time" "github.com/spf13/viper" + "go.opentelemetry.io/collector/config/configgrpc" + "go.opentelemetry.io/collector/config/confighttp" "go.opentelemetry.io/collector/config/configtls" "go.uber.org/zap" @@ -107,21 +109,21 @@ type CollectorOptions struct { // NumWorkers is the number of internal workers in a collector NumWorkers int // HTTP section defines options for HTTP server - HTTP HTTPOptions + HTTP confighttp.ServerConfig // GRPC section defines options for gRPC server - GRPC GRPCOptions + GRPC configgrpc.ServerConfig // OTLP section defines options for servers accepting OpenTelemetry OTLP format OTLP struct { Enabled bool - GRPC GRPCOptions - HTTP HTTPOptions + GRPC configgrpc.ServerConfig + HTTP confighttp.ServerConfig } // Zipkin section defines options for Zipkin HTTP server Zipkin struct { // HTTPHostPort is the host:port address that the Zipkin collector service listens in on for http requests HTTPHostPort string // TLS configures secure transport for Zipkin endpoint to collect spans - TLS *configtls.ServerConfig + TLS configtls.ServerConfig // CORS allows CORS requests , sets the values for Allowed Headers and Allowed Origins. CORS corscfg.Options // KeepAlive configures allow Keep-Alive for Zipkin HTTP server @@ -138,39 +140,6 @@ type serverFlagsConfig struct { tls tlscfg.ServerFlagsConfig } -// HTTPOptions defines options for an HTTP server -type HTTPOptions struct { - // HostPort is the host:port address that the server listens on - HostPort string - // TLS configures secure transport for HTTP endpoint - TLS *configtls.ServerConfig - // ReadTimeout sets the respective parameter of http.Server - ReadTimeout time.Duration - // ReadHeaderTimeout sets the respective parameter of http.Server - ReadHeaderTimeout time.Duration - // IdleTimeout sets the respective parameter of http.Server - IdleTimeout time.Duration - // CORS allows CORS requests , sets the values for Allowed Headers and Allowed Origins. - CORS corscfg.Options -} - -// GRPCOptions defines options for a gRPC server -type GRPCOptions struct { - // HostPort is the host:port address that the collector service listens in on for gRPC requests - HostPort string - // TLS configures secure transport for gRPC endpoint to collect spans - TLS *configtls.ServerConfig - // MaxReceiveMessageLength is the maximum message size receivable by the gRPC Collector. - MaxReceiveMessageLength int - // MaxConnectionAge is a duration for the maximum amount of time a connection may exist. - // See gRPC's keepalive.ServerParameters#MaxConnectionAge. - MaxConnectionAge time.Duration - // MaxConnectionAgeGrace is an additive period after MaxConnectionAge after which the connection will be forcibly closed. - // See gRPC's keepalive.ServerParameters#MaxConnectionAgeGrace. - MaxConnectionAgeGrace time.Duration - // Tenancy configures tenancy for endpoints that collect spans - Tenancy tenancy.Options -} // AddFlags adds flags for CollectorOptions func AddFlags(flagSet *flag.FlagSet) { @@ -224,8 +193,8 @@ func addGRPCFlags(flagSet *flag.FlagSet, cfg serverFlagsConfig, defaultHostPort cfg.tls.AddFlags(flagSet) } -func (opts *HTTPOptions) initFromViper(v *viper.Viper, _ *zap.Logger, cfg serverFlagsConfig) error { - opts.HostPort = ports.FormatHostPort(v.GetString(cfg.prefix + "." + flagSuffixHostPort)) +func initHTTPFromViper(v *viper.Viper, _ *zap.Logger, opts *confighttp.ServerConfig,cfg serverFlagsConfig) error { + opts.Endpoint = ports.FormatHostPort(v.GetString(cfg.prefix + "." + flagSuffixHostPort)) opts.IdleTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPIdleTimeout) opts.ReadTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadTimeout) opts.ReadHeaderTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadHeaderTimeout) @@ -233,21 +202,21 @@ func (opts *HTTPOptions) initFromViper(v *viper.Viper, _ *zap.Logger, cfg server if err != nil { return fmt.Errorf("failed to parse HTTP TLS options: %w", err) } - opts.TLS = tlsOpts.ToOtelServerConfig() + opts.TLSSetting = tlsOpts.ToOtelServerConfig() return nil } -func (opts *GRPCOptions) initFromViper(v *viper.Viper, _ *zap.Logger, cfg serverFlagsConfig) error { - opts.HostPort = ports.FormatHostPort(v.GetString(cfg.prefix + "." + flagSuffixHostPort)) - opts.MaxReceiveMessageLength = v.GetInt(cfg.prefix + "." + flagSuffixGRPCMaxReceiveMessageLength) - opts.MaxConnectionAge = v.GetDuration(cfg.prefix + "." + flagSuffixGRPCMaxConnectionAge) - opts.MaxConnectionAgeGrace = v.GetDuration(cfg.prefix + "." + flagSuffixGRPCMaxConnectionAgeGrace) +func initGRPCFromViper(v *viper.Viper, _ *zap.Logger, opts *configgrpc.ServerConfig,cfg serverFlagsConfig) error { + opts.NetAddr.Endpoint = ports.FormatHostPort(v.GetString(cfg.prefix + "." + flagSuffixHostPort)) + opts.MaxRecvMsgSizeMiB = v.GetInt(cfg.prefix + "." + flagSuffixGRPCMaxReceiveMessageLength) + opts.Keepalive.ServerParameters.MaxConnectionAge = v.GetDuration(cfg.prefix + "." + flagSuffixGRPCMaxConnectionAge) + opts.Keepalive.ServerParameters.MaxConnectionAgeGrace = v.GetDuration(cfg.prefix + "." + flagSuffixGRPCMaxConnectionAgeGrace) tlsOpts, err := cfg.tls.InitFromViper(v) if err != nil { return fmt.Errorf("failed to parse gRPC TLS options: %w", err) } - opts.TLS = tlsOpts.ToOtelServerConfig() - opts.Tenancy = tenancy.InitFromViper(v) + opts.TLSSetting = tlsOpts.ToOtelServerConfig() + // opts.Tenancy = tenancy.InitFromViper(v) return nil } @@ -260,20 +229,21 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) cOpts.DynQueueSizeMemory = v.GetUint(flagDynQueueSizeMemory) * 1024 * 1024 // we receive in MiB and store in bytes cOpts.SpanSizeMetricsEnabled = v.GetBool(flagSpanSizeMetricsEnabled) - if err := cOpts.HTTP.initFromViper(v, logger, httpServerFlagsCfg); err != nil { + if err := initHTTPFromViper(v, logger, &cOpts.HTTP,httpServerFlagsCfg); err != nil { return cOpts, fmt.Errorf("failed to parse HTTP server options: %w", err) } - if err := cOpts.GRPC.initFromViper(v, logger, grpcServerFlagsCfg); err != nil { + if err := initGRPCFromViper(v, logger, &cOpts.GRPC,grpcServerFlagsCfg); err != nil { return cOpts, fmt.Errorf("failed to parse gRPC server options: %w", err) } cOpts.OTLP.Enabled = v.GetBool(flagCollectorOTLPEnabled) - if err := cOpts.OTLP.HTTP.initFromViper(v, logger, otlpServerFlagsCfg.HTTP); err != nil { + if err := initHTTPFromViper(v, logger, &cOpts.OTLP.HTTP,otlpServerFlagsCfg.HTTP); err != nil { return cOpts, fmt.Errorf("failed to parse OTLP/HTTP server options: %w", err) } - cOpts.OTLP.HTTP.CORS = corsOTLPFlags.InitFromViper(v) - if err := cOpts.OTLP.GRPC.initFromViper(v, logger, otlpServerFlagsCfg.GRPC); err != nil { + corsOpts := corsOTLPFlags.InitFromViper(v) + cOpts.OTLP.HTTP.CORS = corsOpts.ToOTELCorsConfig() + if err := initGRPCFromViper(v, logger, &cOpts.GRPC,otlpServerFlagsCfg.GRPC); err != nil { return cOpts, fmt.Errorf("failed to parse OTLP/gRPC server options: %w", err) } @@ -283,7 +253,7 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) if err != nil { return cOpts, fmt.Errorf("failed to parse Zipkin TLS options: %w", err) } - cOpts.Zipkin.TLS = tlsZipkin.ToOtelServerConfig() + cOpts.Zipkin.TLS = *tlsZipkin.ToOtelServerConfig() cOpts.Zipkin.CORS = corsZipkinFlags.InitFromViper(v) return cOpts, nil diff --git a/cmd/collector/app/flags/flags_test.go b/cmd/collector/app/flags/flags_test.go index 9d261465c09..e303cbbe7af 100644 --- a/cmd/collector/app/flags/flags_test.go +++ b/cmd/collector/app/flags/flags_test.go @@ -26,8 +26,8 @@ func TestCollectorOptionsWithFlags_CheckHostPort(t *testing.T) { _, err := c.InitFromViper(v, zap.NewNop()) require.NoError(t, err) - assert.Equal(t, ":5678", c.HTTP.HostPort) - assert.Equal(t, ":1234", c.GRPC.HostPort) + assert.Equal(t, ":5678", c.HTTP.Endpoint) + assert.Equal(t, ":1234", c.GRPC.NetAddr.Endpoint) assert.Equal(t, ":3456", c.Zipkin.HTTPHostPort) } @@ -42,8 +42,8 @@ func TestCollectorOptionsWithFlags_CheckFullHostPort(t *testing.T) { _, err := c.InitFromViper(v, zap.NewNop()) require.NoError(t, err) - assert.Equal(t, ":5678", c.HTTP.HostPort) - assert.Equal(t, "127.0.0.1:1234", c.GRPC.HostPort) + assert.Equal(t, ":5678", c.HTTP.Endpoint) + assert.Equal(t, "127.0.0.1:1234", c.GRPC.NetAddr.Endpoint) assert.Equal(t, "0.0.0.0:3456", c.Zipkin.HTTPHostPort) } @@ -107,7 +107,7 @@ func TestCollectorOptionsWithFlags_CheckMaxReceiveMessageLength(t *testing.T) { _, err := c.InitFromViper(v, zap.NewNop()) require.NoError(t, err) - assert.Equal(t, 8388608, c.GRPC.MaxReceiveMessageLength) + assert.Equal(t, 8388608, c.GRPC.MaxRecvMsgSizeMiB) } func TestCollectorOptionsWithFlags_CheckMaxConnectionAge(t *testing.T) { @@ -123,48 +123,48 @@ func TestCollectorOptionsWithFlags_CheckMaxConnectionAge(t *testing.T) { _, err := c.InitFromViper(v, zap.NewNop()) require.NoError(t, err) - assert.Equal(t, 5*time.Minute, c.GRPC.MaxConnectionAge) - assert.Equal(t, time.Minute, c.GRPC.MaxConnectionAgeGrace) + assert.Equal(t, 5*time.Minute, c.GRPC.Keepalive.ServerParameters.MaxConnectionAge) + assert.Equal(t, time.Minute, c.GRPC.Keepalive.ServerParameters.MaxConnectionAgeGrace) assert.Equal(t, 5*time.Minute, c.HTTP.IdleTimeout) assert.Equal(t, 6*time.Minute, c.HTTP.ReadTimeout) assert.Equal(t, 5*time.Second, c.HTTP.ReadHeaderTimeout) } -func TestCollectorOptionsWithFlags_CheckNoTenancy(t *testing.T) { - c := &CollectorOptions{} - v, command := config.Viperize(AddFlags) - command.ParseFlags([]string{}) - c.InitFromViper(v, zap.NewNop()) - - assert.False(t, c.GRPC.Tenancy.Enabled) -} - -func TestCollectorOptionsWithFlags_CheckSimpleTenancy(t *testing.T) { - c := &CollectorOptions{} - v, command := config.Viperize(AddFlags) - command.ParseFlags([]string{ - "--multi-tenancy.enabled=true", - }) - c.InitFromViper(v, zap.NewNop()) - - assert.True(t, c.GRPC.Tenancy.Enabled) - assert.Equal(t, "x-tenant", c.GRPC.Tenancy.Header) -} - -func TestCollectorOptionsWithFlags_CheckFullTenancy(t *testing.T) { - c := &CollectorOptions{} - v, command := config.Viperize(AddFlags) - command.ParseFlags([]string{ - "--multi-tenancy.enabled=true", - "--multi-tenancy.header=custom-tenant-header", - "--multi-tenancy.tenants=acme,hardware-store", - }) - c.InitFromViper(v, zap.NewNop()) - - assert.True(t, c.GRPC.Tenancy.Enabled) - assert.Equal(t, "custom-tenant-header", c.GRPC.Tenancy.Header) - assert.Equal(t, []string{"acme", "hardware-store"}, c.GRPC.Tenancy.Tenants) -} +// func TestCollectorOptionsWithFlags_CheckNoTenancy(t *testing.T) { +// c := &CollectorOptions{} +// v, command := config.Viperize(AddFlags) +// command.ParseFlags([]string{}) +// c.InitFromViper(v, zap.NewNop()) + +// assert.False(t, c.GRPC.Tenancy.Enabled) +// } + +// func TestCollectorOptionsWithFlags_CheckSimpleTenancy(t *testing.T) { +// c := &CollectorOptions{} +// v, command := config.Viperize(AddFlags) +// command.ParseFlags([]string{ +// "--multi-tenancy.enabled=true", +// }) +// c.InitFromViper(v, zap.NewNop()) + +// assert.True(t, c.GRPC.Tenancy.Enabled) +// assert.Equal(t, "x-tenant", c.GRPC.Tenancy.Header) +// } + +// func TestCollectorOptionsWithFlags_CheckFullTenancy(t *testing.T) { +// c := &CollectorOptions{} +// v, command := config.Viperize(AddFlags) +// command.ParseFlags([]string{ +// "--multi-tenancy.enabled=true", +// "--multi-tenancy.header=custom-tenant-header", +// "--multi-tenancy.tenants=acme,hardware-store", +// }) +// c.InitFromViper(v, zap.NewNop()) + +// assert.True(t, c.GRPC.Tenancy.Enabled) +// assert.Equal(t, "custom-tenant-header", c.GRPC.Tenancy.Header) +// assert.Equal(t, []string{"acme", "hardware-store"}, c.GRPC.Tenancy.Tenants) +// } func TestCollectorOptionsWithFlags_CheckZipkinKeepAlive(t *testing.T) { c := &CollectorOptions{} diff --git a/cmd/collector/app/handler/otlp_receiver.go b/cmd/collector/app/handler/otlp_receiver.go index 1517d3de2df..0ad27c0c302 100644 --- a/cmd/collector/app/handler/otlp_receiver.go +++ b/cmd/collector/app/handler/otlp_receiver.go @@ -99,32 +99,32 @@ func startOTLPReceiver( return otlpReceiver, nil } -func applyGRPCSettings(cfg *configgrpc.ServerConfig, opts *flags.GRPCOptions) { - if opts.HostPort != "" { - cfg.NetAddr.Endpoint = opts.HostPort +func applyGRPCSettings(cfg *configgrpc.ServerConfig, opts *configgrpc.ServerConfig) { + if opts.NetAddr.Endpoint != "" { + cfg.NetAddr.Endpoint = opts.NetAddr.Endpoint } - if opts.TLS != nil { - cfg.TLSSetting = opts.TLS + if opts.TLSSetting != nil { + cfg.TLSSetting = opts.TLSSetting } - if opts.MaxReceiveMessageLength > 0 { - cfg.MaxRecvMsgSizeMiB = int(opts.MaxReceiveMessageLength / (1024 * 1024)) + if opts.MaxRecvMsgSizeMiB > 0 { + cfg.MaxRecvMsgSizeMiB = opts.MaxRecvMsgSizeMiB } - if opts.MaxConnectionAge != 0 || opts.MaxConnectionAgeGrace != 0 { + if opts.Keepalive.ServerParameters.MaxConnectionAge != 0 || opts.Keepalive.ServerParameters.MaxConnectionAgeGrace != 0 { cfg.Keepalive = &configgrpc.KeepaliveServerConfig{ ServerParameters: &configgrpc.KeepaliveServerParameters{ - MaxConnectionAge: opts.MaxConnectionAge, - MaxConnectionAgeGrace: opts.MaxConnectionAgeGrace, + MaxConnectionAge: opts.Keepalive.ServerParameters.MaxConnectionAge, + MaxConnectionAgeGrace: opts.Keepalive.ServerParameters.MaxConnectionAgeGrace, }, } } } -func applyHTTPSettings(cfg *confighttp.ServerConfig, opts *flags.HTTPOptions) { - if opts.HostPort != "" { - cfg.Endpoint = opts.HostPort +func applyHTTPSettings(cfg *confighttp.ServerConfig, opts *confighttp.ServerConfig) { + if opts.Endpoint != "" { + cfg.Endpoint = opts.Endpoint } - if opts.TLS != nil { - cfg.TLSSetting = opts.TLS + if opts.TLSSetting != nil { + cfg.TLSSetting = opts.TLSSetting } cfg.CORS = &confighttp.CORSConfig{ diff --git a/cmd/collector/app/handler/otlp_receiver_test.go b/cmd/collector/app/handler/otlp_receiver_test.go index cdc67603533..361daed0645 100644 --- a/cmd/collector/app/handler/otlp_receiver_test.go +++ b/cmd/collector/app/handler/otlp_receiver_test.go @@ -12,6 +12,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/config/configgrpc" + "go.opentelemetry.io/collector/config/confighttp" + "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/config/configtls" "go.opentelemetry.io/collector/consumer" "go.opentelemetry.io/collector/pdata/ptrace" @@ -20,18 +23,19 @@ import ( "go.opentelemetry.io/collector/receiver/otlpreceiver" "github.com/jaegertracing/jaeger/cmd/collector/app/flags" - "github.com/jaegertracing/jaeger/pkg/config/corscfg" "github.com/jaegertracing/jaeger/pkg/tenancy" "github.com/jaegertracing/jaeger/pkg/testutils" ) func optionsWithPorts(port string) *flags.CollectorOptions { opts := &flags.CollectorOptions{} - opts.OTLP.GRPC = flags.GRPCOptions{ - HostPort: port, + opts.OTLP.HTTP = confighttp.ServerConfig{ + Endpoint: port, } - opts.OTLP.HTTP = flags.HTTPOptions{ - HostPort: port, + opts.OTLP.GRPC = configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: port, + }, } return opts } @@ -133,12 +137,18 @@ func TestApplyOTLPGRPCServerSettings(t *testing.T) { otlpFactory := otlpreceiver.NewFactory() otlpReceiverConfig := otlpFactory.CreateDefaultConfig().(*otlpreceiver.Config) - grpcOpts := &flags.GRPCOptions{ - HostPort: ":54321", - MaxReceiveMessageLength: 42 * 1024 * 1024, - MaxConnectionAge: 33 * time.Second, - MaxConnectionAgeGrace: 37 * time.Second, - TLS: &configtls.ServerConfig{ + grpcOpts := &configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: ":54321", + }, + MaxRecvMsgSizeMiB: 42, + Keepalive: &configgrpc.KeepaliveServerConfig{ + ServerParameters: &configgrpc.KeepaliveServerParameters{ + MaxConnectionAge: 33 * time.Second, + MaxConnectionAgeGrace: 37 * time.Second, + }, + }, + TLSSetting: &configtls.ServerConfig{ ClientCAFile: "clientca", Config: configtls.Config{ CAFile: "ca", @@ -172,9 +182,9 @@ func TestApplyOTLPHTTPServerSettings(t *testing.T) { otlpFactory := otlpreceiver.NewFactory() otlpReceiverConfig := otlpFactory.CreateDefaultConfig().(*otlpreceiver.Config) - httpOpts := &flags.HTTPOptions{ - HostPort: ":12345", - TLS: &configtls.ServerConfig{ + httpOpts := &confighttp.ServerConfig{ + Endpoint: ":12345", + TLSSetting: &configtls.ServerConfig{ ClientCAFile: "clientca", Config: configtls.Config{ CAFile: "ca", @@ -185,7 +195,7 @@ func TestApplyOTLPHTTPServerSettings(t *testing.T) { ReloadInterval: 24 * time.Hour, }, }, - CORS: corscfg.Options{ + CORS: &confighttp.CORSConfig{ AllowedOrigins: []string{"http://example.domain.com", "http://*.domain.com"}, AllowedHeaders: []string{"Content-Type", "Accept", "X-Requested-With"}, }, diff --git a/cmd/collector/app/handler/zipkin_receiver.go b/cmd/collector/app/handler/zipkin_receiver.go index 983ee4bd2a5..7f965839a1d 100644 --- a/cmd/collector/app/handler/zipkin_receiver.go +++ b/cmd/collector/app/handler/zipkin_receiver.go @@ -9,6 +9,7 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/zipkinreceiver" "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/config/confighttp" "go.opentelemetry.io/collector/config/configtelemetry" "go.opentelemetry.io/collector/consumer" "go.opentelemetry.io/collector/receiver" @@ -56,9 +57,9 @@ func startZipkinReceiver( cfg component.Config, nextConsumer consumer.Traces) (receiver.Traces, error), ) (receiver.Traces, error) { receiverConfig := zipkinFactory.CreateDefaultConfig().(*zipkinreceiver.Config) - applyHTTPSettings(&receiverConfig.ServerConfig, &flags.HTTPOptions{ - HostPort: options.Zipkin.HTTPHostPort, - TLS: options.Zipkin.TLS, + applyHTTPSettings(&receiverConfig.ServerConfig, &confighttp.ServerConfig{ + Endpoint: options.Zipkin.HTTPHostPort, + TLSSetting: &options.Zipkin.TLS, CORS: options.HTTP.CORS, // TODO keepAlive not supported? }) diff --git a/cmd/collector/app/handler/zipkin_receiver_tls_test.go b/cmd/collector/app/handler/zipkin_receiver_tls_test.go index 5da0b6694d5..e0376034daf 100644 --- a/cmd/collector/app/handler/zipkin_receiver_tls_test.go +++ b/cmd/collector/app/handler/zipkin_receiver_tls_test.go @@ -163,7 +163,7 @@ func TestSpanCollectorZipkinTLS(t *testing.T) { opts := &flags.CollectorOptions{} opts.Zipkin.HTTPHostPort = ports.PortToHostPort(ports.CollectorZipkin) - opts.Zipkin.TLS = test.serverTLS.ToOtelServerConfig() + opts.Zipkin.TLS = *test.serverTLS.ToOtelServerConfig() server, err := StartZipkinReceiver(opts, logger, spanProcessor, tm) if test.expectServerFail { diff --git a/pkg/config/corscfg/options.go b/pkg/config/corscfg/options.go index 9dd1f7ffd5a..eee1f557d74 100644 --- a/pkg/config/corscfg/options.go +++ b/pkg/config/corscfg/options.go @@ -3,7 +3,16 @@ package corscfg +import "go.opentelemetry.io/collector/config/confighttp" + type Options struct { AllowedOrigins []string AllowedHeaders []string } + +func (o *Options) ToOTELCorsConfig() *confighttp.CORSConfig{ + return &confighttp.CORSConfig{ + AllowedOrigins: o.AllowedOrigins, + AllowedHeaders: o.AllowedHeaders, + } +} \ No newline at end of file From 31588eff068d70652c64b2cd565a58d598d58ae9 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Sat, 30 Nov 2024 03:11:21 +0530 Subject: [PATCH 04/18] deletion of unused functions Signed-off-by: chahatsagarmain --- cmd/collector/app/collector.go | 2 +- cmd/collector/app/collector_test.go | 4 +- cmd/collector/app/flags/flags.go | 23 ++--- cmd/collector/app/flags/flags_test.go | 22 ++--- cmd/collector/app/handler/otlp_receiver.go | 38 -------- .../app/handler/otlp_receiver_test.go | 87 ------------------- cmd/collector/app/handler/zipkin_receiver.go | 9 +- .../app/handler/zipkin_receiver_test.go | 4 +- .../app/handler/zipkin_receiver_tls_test.go | 4 +- cmd/collector/app/server/http.go | 6 ++ 10 files changed, 32 insertions(+), 167 deletions(-) diff --git a/cmd/collector/app/collector.go b/cmd/collector/app/collector.go index 67469c8bb79..ed65d444b35 100644 --- a/cmd/collector/app/collector.go +++ b/cmd/collector/app/collector.go @@ -127,7 +127,7 @@ func (c *Collector) Start(options *flags.CollectorOptions) error { } c.hServer = httpServer - if options.Zipkin.HTTPHostPort == "" { + if options.Zipkin.Endpoint == "" { c.logger.Info("Not listening for Zipkin HTTP traffic, port not configured") } else { zipkinReceiver, err := handler.StartZipkinReceiver(options, c.logger, c.spanProcessor, c.tenancyMgr) diff --git a/cmd/collector/app/collector_test.go b/cmd/collector/app/collector_test.go index 0089e846e0f..4faf2beea6d 100644 --- a/cmd/collector/app/collector_test.go +++ b/cmd/collector/app/collector_test.go @@ -33,7 +33,7 @@ func optionsForEphemeralPorts() *flags.CollectorOptions { collectorOpts.OTLP.Enabled = true collectorOpts.OTLP.GRPC.NetAddr.Endpoint = ":0" collectorOpts.OTLP.HTTP.Endpoint = ":0" - collectorOpts.Zipkin.HTTPHostPort = ":0" + collectorOpts.Zipkin.Endpoint = ":0" return collectorOpts } @@ -120,7 +120,7 @@ func TestCollector_StartErrors(t *testing.T) { run("HTTP", options, "could not start HTTP server") options = optionsForEphemeralPorts() - options.Zipkin.HTTPHostPort = ":-1" + options.Zipkin.Endpoint = ":-1" run("Zipkin", options, "could not start Zipkin receiver") options = optionsForEphemeralPorts() diff --git a/cmd/collector/app/flags/flags.go b/cmd/collector/app/flags/flags.go index dfd3d95a212..4d46649bb1d 100644 --- a/cmd/collector/app/flags/flags.go +++ b/cmd/collector/app/flags/flags.go @@ -12,7 +12,6 @@ import ( "github.com/spf13/viper" "go.opentelemetry.io/collector/config/configgrpc" "go.opentelemetry.io/collector/config/confighttp" - "go.opentelemetry.io/collector/config/configtls" "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/internal/flags" @@ -119,16 +118,7 @@ type CollectorOptions struct { HTTP confighttp.ServerConfig } // Zipkin section defines options for Zipkin HTTP server - Zipkin struct { - // HTTPHostPort is the host:port address that the Zipkin collector service listens in on for http requests - HTTPHostPort string - // TLS configures secure transport for Zipkin endpoint to collect spans - TLS configtls.ServerConfig - // CORS allows CORS requests , sets the values for Allowed Headers and Allowed Origins. - CORS corscfg.Options - // KeepAlive configures allow Keep-Alive for Zipkin HTTP server - KeepAlive bool - } + Zipkin confighttp.ServerConfig // CollectorTags is the string representing collector tags to append to each and every span CollectorTags map[string]string // SpanSizeMetricsEnabled determines whether to enable metrics based on processed span size @@ -208,7 +198,7 @@ func initHTTPFromViper(v *viper.Viper, _ *zap.Logger, opts *confighttp.ServerCon func initGRPCFromViper(v *viper.Viper, _ *zap.Logger, opts *configgrpc.ServerConfig,cfg serverFlagsConfig) error { opts.NetAddr.Endpoint = ports.FormatHostPort(v.GetString(cfg.prefix + "." + flagSuffixHostPort)) - opts.MaxRecvMsgSizeMiB = v.GetInt(cfg.prefix + "." + flagSuffixGRPCMaxReceiveMessageLength) + opts.MaxRecvMsgSizeMiB = v.GetInt(cfg.prefix + "." + flagSuffixGRPCMaxReceiveMessageLength) * 1024 * 1024 opts.Keepalive.ServerParameters.MaxConnectionAge = v.GetDuration(cfg.prefix + "." + flagSuffixGRPCMaxConnectionAge) opts.Keepalive.ServerParameters.MaxConnectionAgeGrace = v.GetDuration(cfg.prefix + "." + flagSuffixGRPCMaxConnectionAgeGrace) tlsOpts, err := cfg.tls.InitFromViper(v) @@ -247,14 +237,15 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) return cOpts, fmt.Errorf("failed to parse OTLP/gRPC server options: %w", err) } - cOpts.Zipkin.KeepAlive = v.GetBool(flagZipkinKeepAliveEnabled) - cOpts.Zipkin.HTTPHostPort = ports.FormatHostPort(v.GetString(flagZipkinHTTPHostPort)) + // cOpts.Zipkin. = v.GetBool(flagZipkinKeepAliveEnabled) + cOpts.Zipkin.Endpoint = ports.FormatHostPort(v.GetString(flagZipkinHTTPHostPort)) tlsZipkin, err := tlsZipkinFlagsConfig.InitFromViper(v) if err != nil { return cOpts, fmt.Errorf("failed to parse Zipkin TLS options: %w", err) } - cOpts.Zipkin.TLS = *tlsZipkin.ToOtelServerConfig() - cOpts.Zipkin.CORS = corsZipkinFlags.InitFromViper(v) + cOpts.Zipkin.TLSSetting = tlsZipkin.ToOtelServerConfig() + corsOpts = corsZipkinFlags.InitFromViper(v) + cOpts.Zipkin.CORS = corsOpts.ToOTELCorsConfig() return cOpts, nil } diff --git a/cmd/collector/app/flags/flags_test.go b/cmd/collector/app/flags/flags_test.go index e303cbbe7af..c112e679f4a 100644 --- a/cmd/collector/app/flags/flags_test.go +++ b/cmd/collector/app/flags/flags_test.go @@ -28,7 +28,7 @@ func TestCollectorOptionsWithFlags_CheckHostPort(t *testing.T) { assert.Equal(t, ":5678", c.HTTP.Endpoint) assert.Equal(t, ":1234", c.GRPC.NetAddr.Endpoint) - assert.Equal(t, ":3456", c.Zipkin.HTTPHostPort) + assert.Equal(t, ":3456", c.Zipkin.Endpoint) } func TestCollectorOptionsWithFlags_CheckFullHostPort(t *testing.T) { @@ -44,7 +44,7 @@ func TestCollectorOptionsWithFlags_CheckFullHostPort(t *testing.T) { assert.Equal(t, ":5678", c.HTTP.Endpoint) assert.Equal(t, "127.0.0.1:1234", c.GRPC.NetAddr.Endpoint) - assert.Equal(t, "0.0.0.0:3456", c.Zipkin.HTTPHostPort) + assert.Equal(t, "0.0.0.0:3456", c.Zipkin.Endpoint) } func TestCollectorOptionsWithFailedTLSFlags(t *testing.T) { @@ -166,16 +166,16 @@ func TestCollectorOptionsWithFlags_CheckMaxConnectionAge(t *testing.T) { // assert.Equal(t, []string{"acme", "hardware-store"}, c.GRPC.Tenancy.Tenants) // } -func TestCollectorOptionsWithFlags_CheckZipkinKeepAlive(t *testing.T) { - c := &CollectorOptions{} - v, command := config.Viperize(AddFlags) - command.ParseFlags([]string{ - "--collector.zipkin.keep-alive=false", - }) - c.InitFromViper(v, zap.NewNop()) +// func TestCollectorOptionsWithFlags_CheckZipkinKeepAlive(t *testing.T) { +// c := &CollectorOptions{} +// v, command := config.Viperize(AddFlags) +// command.ParseFlags([]string{ +// "--collector.zipkin.keep-alive=false", +// }) +// c.InitFromViper(v, zap.NewNop()) - assert.False(t, c.Zipkin.KeepAlive) -} +// assert.False(t, c.Zipkin.KeepAlive) +// } func TestMain(m *testing.M) { testutils.VerifyGoLeaks(m) diff --git a/cmd/collector/app/handler/otlp_receiver.go b/cmd/collector/app/handler/otlp_receiver.go index 0ad27c0c302..8704e958153 100644 --- a/cmd/collector/app/handler/otlp_receiver.go +++ b/cmd/collector/app/handler/otlp_receiver.go @@ -10,8 +10,6 @@ import ( otlp2jaeger "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/component/componentstatus" - "go.opentelemetry.io/collector/config/configgrpc" - "go.opentelemetry.io/collector/config/confighttp" "go.opentelemetry.io/collector/config/configtelemetry" "go.opentelemetry.io/collector/consumer" "go.opentelemetry.io/collector/extension" @@ -60,8 +58,6 @@ func startOTLPReceiver( cfg component.Config, nextConsumer consumer.Traces) (receiver.Traces, error), ) (receiver.Traces, error) { otlpReceiverConfig := otlpFactory.CreateDefaultConfig().(*otlpreceiver.Config) - applyGRPCSettings(otlpReceiverConfig.GRPC, &options.OTLP.GRPC) - applyHTTPSettings(otlpReceiverConfig.HTTP.ServerConfig, &options.OTLP.HTTP) statusReporter := func(ev *componentstatus.Event) { // TODO this could be wired into changing healthcheck.HealthCheck logger.Info("OTLP receiver status change", zap.Stringer("status", ev.Status())) @@ -99,40 +95,6 @@ func startOTLPReceiver( return otlpReceiver, nil } -func applyGRPCSettings(cfg *configgrpc.ServerConfig, opts *configgrpc.ServerConfig) { - if opts.NetAddr.Endpoint != "" { - cfg.NetAddr.Endpoint = opts.NetAddr.Endpoint - } - if opts.TLSSetting != nil { - cfg.TLSSetting = opts.TLSSetting - } - if opts.MaxRecvMsgSizeMiB > 0 { - cfg.MaxRecvMsgSizeMiB = opts.MaxRecvMsgSizeMiB - } - if opts.Keepalive.ServerParameters.MaxConnectionAge != 0 || opts.Keepalive.ServerParameters.MaxConnectionAgeGrace != 0 { - cfg.Keepalive = &configgrpc.KeepaliveServerConfig{ - ServerParameters: &configgrpc.KeepaliveServerParameters{ - MaxConnectionAge: opts.Keepalive.ServerParameters.MaxConnectionAge, - MaxConnectionAgeGrace: opts.Keepalive.ServerParameters.MaxConnectionAgeGrace, - }, - } - } -} - -func applyHTTPSettings(cfg *confighttp.ServerConfig, opts *confighttp.ServerConfig) { - if opts.Endpoint != "" { - cfg.Endpoint = opts.Endpoint - } - if opts.TLSSetting != nil { - cfg.TLSSetting = opts.TLSSetting - } - - cfg.CORS = &confighttp.CORSConfig{ - AllowedOrigins: opts.CORS.AllowedOrigins, - AllowedHeaders: opts.CORS.AllowedHeaders, - } -} - func newConsumerDelegate(logger *zap.Logger, spanProcessor processor.SpanProcessor, tm *tenancy.Manager) *consumerDelegate { return &consumerDelegate{ batchConsumer: newBatchConsumer(logger, diff --git a/cmd/collector/app/handler/otlp_receiver_test.go b/cmd/collector/app/handler/otlp_receiver_test.go index 361daed0645..c36abac0b09 100644 --- a/cmd/collector/app/handler/otlp_receiver_test.go +++ b/cmd/collector/app/handler/otlp_receiver_test.go @@ -7,7 +7,6 @@ import ( "context" "errors" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -15,7 +14,6 @@ import ( "go.opentelemetry.io/collector/config/configgrpc" "go.opentelemetry.io/collector/config/confighttp" "go.opentelemetry.io/collector/config/confignet" - "go.opentelemetry.io/collector/config/configtls" "go.opentelemetry.io/collector/consumer" "go.opentelemetry.io/collector/pdata/ptrace" "go.opentelemetry.io/collector/pipeline" @@ -132,88 +130,3 @@ func TestOtelHost(t *testing.T) { assert.Nil(t, host.GetExtensions()) assert.Nil(t, host.GetExporters()) } - -func TestApplyOTLPGRPCServerSettings(t *testing.T) { - otlpFactory := otlpreceiver.NewFactory() - otlpReceiverConfig := otlpFactory.CreateDefaultConfig().(*otlpreceiver.Config) - - grpcOpts := &configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: ":54321", - }, - MaxRecvMsgSizeMiB: 42, - Keepalive: &configgrpc.KeepaliveServerConfig{ - ServerParameters: &configgrpc.KeepaliveServerParameters{ - MaxConnectionAge: 33 * time.Second, - MaxConnectionAgeGrace: 37 * time.Second, - }, - }, - TLSSetting: &configtls.ServerConfig{ - ClientCAFile: "clientca", - Config: configtls.Config{ - CAFile: "ca", - CertFile: "cert", - KeyFile: "key", - MinVersion: "1.1", - MaxVersion: "1.3", - ReloadInterval: 24 * time.Hour, - }, - }, - } - applyGRPCSettings(otlpReceiverConfig.GRPC, grpcOpts) - out := otlpReceiverConfig.GRPC - assert.Equal(t, ":54321", out.NetAddr.Endpoint) - assert.EqualValues(t, 42, out.MaxRecvMsgSizeMiB) - require.NotNil(t, out.Keepalive) - require.NotNil(t, out.Keepalive.ServerParameters) - assert.Equal(t, 33*time.Second, out.Keepalive.ServerParameters.MaxConnectionAge) - assert.Equal(t, 37*time.Second, out.Keepalive.ServerParameters.MaxConnectionAgeGrace) - require.NotNil(t, out.TLSSetting) - assert.Equal(t, "ca", out.TLSSetting.CAFile) - assert.Equal(t, "cert", out.TLSSetting.CertFile) - assert.Equal(t, "key", out.TLSSetting.KeyFile) - assert.Equal(t, "clientca", out.TLSSetting.ClientCAFile) - assert.Equal(t, "1.1", out.TLSSetting.MinVersion) - assert.Equal(t, "1.3", out.TLSSetting.MaxVersion) - assert.Equal(t, 24*time.Hour, out.TLSSetting.ReloadInterval) -} - -func TestApplyOTLPHTTPServerSettings(t *testing.T) { - otlpFactory := otlpreceiver.NewFactory() - otlpReceiverConfig := otlpFactory.CreateDefaultConfig().(*otlpreceiver.Config) - - httpOpts := &confighttp.ServerConfig{ - Endpoint: ":12345", - TLSSetting: &configtls.ServerConfig{ - ClientCAFile: "clientca", - Config: configtls.Config{ - CAFile: "ca", - CertFile: "cert", - KeyFile: "key", - MinVersion: "1.1", - MaxVersion: "1.3", - ReloadInterval: 24 * time.Hour, - }, - }, - CORS: &confighttp.CORSConfig{ - AllowedOrigins: []string{"http://example.domain.com", "http://*.domain.com"}, - AllowedHeaders: []string{"Content-Type", "Accept", "X-Requested-With"}, - }, - } - - applyHTTPSettings(otlpReceiverConfig.HTTP.ServerConfig, httpOpts) - - out := otlpReceiverConfig.HTTP - - assert.Equal(t, ":12345", out.Endpoint) - require.NotNil(t, out.TLSSetting) - assert.Equal(t, "ca", out.TLSSetting.CAFile) - assert.Equal(t, "cert", out.TLSSetting.CertFile) - assert.Equal(t, "key", out.TLSSetting.KeyFile) - assert.Equal(t, "clientca", out.TLSSetting.ClientCAFile) - assert.Equal(t, "1.1", out.TLSSetting.MinVersion) - assert.Equal(t, "1.3", out.TLSSetting.MaxVersion) - assert.Equal(t, 24*time.Hour, out.TLSSetting.ReloadInterval) - assert.Equal(t, []string{"Content-Type", "Accept", "X-Requested-With"}, out.CORS.AllowedHeaders) - assert.Equal(t, []string{"http://example.domain.com", "http://*.domain.com"}, out.CORS.AllowedOrigins) -} diff --git a/cmd/collector/app/handler/zipkin_receiver.go b/cmd/collector/app/handler/zipkin_receiver.go index 7f965839a1d..c28a5adbc06 100644 --- a/cmd/collector/app/handler/zipkin_receiver.go +++ b/cmd/collector/app/handler/zipkin_receiver.go @@ -9,7 +9,6 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/zipkinreceiver" "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/config/confighttp" "go.opentelemetry.io/collector/config/configtelemetry" "go.opentelemetry.io/collector/consumer" "go.opentelemetry.io/collector/receiver" @@ -46,7 +45,7 @@ func StartZipkinReceiver( // which is a situation we cannot reproduce. To test our own error handling, this // function allows to mock those constructors. func startZipkinReceiver( - options *flags.CollectorOptions, + _ *flags.CollectorOptions, logger *zap.Logger, spanProcessor processor.SpanProcessor, tm *tenancy.Manager, @@ -57,12 +56,6 @@ func startZipkinReceiver( cfg component.Config, nextConsumer consumer.Traces) (receiver.Traces, error), ) (receiver.Traces, error) { receiverConfig := zipkinFactory.CreateDefaultConfig().(*zipkinreceiver.Config) - applyHTTPSettings(&receiverConfig.ServerConfig, &confighttp.ServerConfig{ - Endpoint: options.Zipkin.HTTPHostPort, - TLSSetting: &options.Zipkin.TLS, - CORS: options.HTTP.CORS, - // TODO keepAlive not supported? - }) receiverSettings := receiver.Settings{ TelemetrySettings: component.TelemetrySettings{ Logger: logger, diff --git a/cmd/collector/app/handler/zipkin_receiver_test.go b/cmd/collector/app/handler/zipkin_receiver_test.go index 584dbc56cda..d6ad98c4319 100644 --- a/cmd/collector/app/handler/zipkin_receiver_test.go +++ b/cmd/collector/app/handler/zipkin_receiver_test.go @@ -37,7 +37,7 @@ func TestZipkinReceiver(t *testing.T) { tm := &tenancy.Manager{} opts := &flags.CollectorOptions{} - opts.Zipkin.HTTPHostPort = ":11911" + opts.Zipkin.Endpoint = ":11911" rec, err := StartZipkinReceiver(opts, logger, spanProcessor, tm) require.NoError(t, err) @@ -138,7 +138,7 @@ func TestStartZipkinReceiver_Error(t *testing.T) { tm := &tenancy.Manager{} opts := &flags.CollectorOptions{} - opts.Zipkin.HTTPHostPort = ":-1" + opts.Zipkin.Endpoint = ":-1" _, err := StartZipkinReceiver(opts, logger, spanProcessor, tm) require.ErrorContains(t, err, "could not start Zipkin receiver") diff --git a/cmd/collector/app/handler/zipkin_receiver_tls_test.go b/cmd/collector/app/handler/zipkin_receiver_tls_test.go index e0376034daf..5d31d435715 100644 --- a/cmd/collector/app/handler/zipkin_receiver_tls_test.go +++ b/cmd/collector/app/handler/zipkin_receiver_tls_test.go @@ -162,8 +162,8 @@ func TestSpanCollectorZipkinTLS(t *testing.T) { tm := &tenancy.Manager{} opts := &flags.CollectorOptions{} - opts.Zipkin.HTTPHostPort = ports.PortToHostPort(ports.CollectorZipkin) - opts.Zipkin.TLS = *test.serverTLS.ToOtelServerConfig() + opts.Zipkin.Endpoint = ports.PortToHostPort(ports.CollectorZipkin) + opts.Zipkin.TLSSetting = test.serverTLS.ToOtelServerConfig() server, err := StartZipkinReceiver(opts, logger, spanProcessor, tm) if test.expectServerFail { diff --git a/cmd/collector/app/server/http.go b/cmd/collector/app/server/http.go index 411df873b2e..1dcba13e5d0 100644 --- a/cmd/collector/app/server/http.go +++ b/cmd/collector/app/server/http.go @@ -5,6 +5,7 @@ package server import ( "context" + "io" "net" "net/http" "time" @@ -41,6 +42,11 @@ type HTTPServerParams struct { IdleTimeout time.Duration } +type httpServer struct { + *http.Server + staticHandlerCloser io.Closer +} + // StartHTTPServer based on the given parameters func StartHTTPServer(params *HTTPServerParams) (*http.Server, error) { params.Logger.Info("Starting jaeger-collector HTTP server", zap.String("http host-port", params.HostPort)) From 624bb6924622dc5b0dce693b2822d4b92552b71c Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Sun, 1 Dec 2024 03:49:25 +0530 Subject: [PATCH 05/18] tenancy and fixes Signed-off-by: chahatsagarmain --- cmd/all-in-one/main.go | 2 +- cmd/collector/app/collector.go | 26 ++++---- cmd/collector/app/collector_test.go | 15 +++++ cmd/collector/app/flags/flags.go | 40 +++++++------ cmd/collector/app/flags/flags_test.go | 60 +++++++++---------- cmd/collector/app/handler/otlp_receiver.go | 4 ++ cmd/collector/app/handler/zipkin_receiver.go | 3 +- .../app/handler/zipkin_receiver_tls_test.go | 3 +- cmd/collector/app/server/grpc.go | 39 ++++++------ cmd/collector/app/server/grpc_test.go | 22 +++++-- cmd/collector/app/server/http.go | 6 -- cmd/collector/main.go | 2 +- pkg/config/corscfg/options.go | 4 +- 13 files changed, 130 insertions(+), 96 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 10578721248..6524d20da25 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -142,7 +142,7 @@ by default uses only in-memory database.`, logger.Fatal("Failed to configure query service", zap.Error(err)) } - tm := tenancy.NewManager(&cOpts.GRPC.Tenancy) + tm := tenancy.NewManager(&cOpts.Tenancy) // collector c := collectorApp.New(&collectorApp.CollectorParams{ diff --git a/cmd/collector/app/collector.go b/cmd/collector/app/collector.go index ed65d444b35..be9e08c9fd1 100644 --- a/cmd/collector/app/collector.go +++ b/cmd/collector/app/collector.go @@ -9,6 +9,8 @@ import ( "net/http" "time" + "go.opentelemetry.io/collector/config/configgrpc" + "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/receiver" "go.uber.org/zap" "google.golang.org/grpc" @@ -97,22 +99,24 @@ func (c *Collector) Start(options *flags.CollectorOptions) error { c.spanProcessor = handlerBuilder.BuildSpanProcessor(additionalProcessors...) c.spanHandlers = handlerBuilder.BuildHandlers(c.spanProcessor) - + // fmt.Printf("%v \n",options.GRPC.NetAddr.Endpoint) grpcServer, err := server.StartGRPCServer(&server.GRPCServerParams{ - HostPort: options.GRPC.NetAddr.Endpoint, - Handler: c.spanHandlers.GRPCHandler, - TLSConfig: options.GRPC.TLSSetting, - SamplingProvider: c.samplingProvider, - Logger: c.logger, - MaxReceiveMessageLength: options.GRPC.MaxRecvMsgSizeMiB, - MaxConnectionAge: options.GRPC.Keepalive.ServerParameters.MaxConnectionAge, - MaxConnectionAgeGrace: options.GRPC.Keepalive.ServerParameters.MaxConnectionAgeGrace, + Handler: c.spanHandlers.GRPCHandler, + SamplingProvider: c.samplingProvider, + Logger: c.logger, + ServerConfig: &configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: options.GRPC.NetAddr.Endpoint, + }, + TLSSetting: options.GRPC.TLSSetting, + MaxRecvMsgSizeMiB: options.GRPC.MaxRecvMsgSizeMiB, + Keepalive: options.GRPC.Keepalive, + }, }) if err != nil { return fmt.Errorf("could not start gRPC server: %w", err) } c.grpcServer = grpcServer - httpServer, err := server.StartHTTPServer(&server.HTTPServerParams{ HostPort: options.HTTP.Endpoint, Handler: c.spanHandlers.JaegerBatchesHandler, @@ -130,6 +134,7 @@ func (c *Collector) Start(options *flags.CollectorOptions) error { if options.Zipkin.Endpoint == "" { c.logger.Info("Not listening for Zipkin HTTP traffic, port not configured") } else { + fmt.Printf("%v zipkin\n", options.Zipkin.Endpoint) zipkinReceiver, err := handler.StartZipkinReceiver(options, c.logger, c.spanProcessor, c.tenancyMgr) if err != nil { return fmt.Errorf("could not start Zipkin receiver: %w", err) @@ -138,6 +143,7 @@ func (c *Collector) Start(options *flags.CollectorOptions) error { } if options.OTLP.Enabled { + fmt.Printf("otlp enabled \n") otlpReceiver, err := handler.StartOTLPReceiver(options, c.logger, c.spanProcessor, c.tenancyMgr) if err != nil { return fmt.Errorf("could not start OTLP receiver: %w", err) diff --git a/cmd/collector/app/collector_test.go b/cmd/collector/app/collector_test.go index 4faf2beea6d..96605f73135 100644 --- a/cmd/collector/app/collector_test.go +++ b/cmd/collector/app/collector_test.go @@ -6,6 +6,7 @@ package app import ( "context" "expvar" + "fmt" "io" "sync/atomic" "testing" @@ -13,6 +14,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/config/configgrpc" "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/collector/app/flags" @@ -29,11 +31,22 @@ var _ (io.Closer) = (*Collector)(nil) func optionsForEphemeralPorts() *flags.CollectorOptions { collectorOpts := &flags.CollectorOptions{} collectorOpts.GRPC.NetAddr.Endpoint = ":0" + collectorOpts.GRPC.Keepalive = &configgrpc.KeepaliveServerConfig{ + ServerParameters: &configgrpc.KeepaliveServerParameters{ + MaxConnectionIdle: 10, + }, + } collectorOpts.HTTP.Endpoint = ":0" collectorOpts.OTLP.Enabled = true collectorOpts.OTLP.GRPC.NetAddr.Endpoint = ":0" + collectorOpts.OTLP.GRPC.Keepalive = &configgrpc.KeepaliveServerConfig{ + ServerParameters: &configgrpc.KeepaliveServerParameters{ + MaxConnectionIdle: 10, + }, + } collectorOpts.OTLP.HTTP.Endpoint = ":0" collectorOpts.Zipkin.Endpoint = ":0" + collectorOpts.Tenancy = tenancy.Options{} return collectorOpts } @@ -78,6 +91,8 @@ func TestNewCollector(t *testing.T) { }) collectorOpts := optionsForEphemeralPorts() + fmt.Printf("Collector Options: %+v\n", collectorOpts.OTLP.GRPC) + fmt.Print(collectorOpts.OTLP.HTTP.Endpoint) require.NoError(t, c.Start(collectorOpts)) assert.NotNil(t, c.SpanHandlers()) require.NoError(t, c.Close()) diff --git a/cmd/collector/app/flags/flags.go b/cmd/collector/app/flags/flags.go index 4d46649bb1d..54c1141d975 100644 --- a/cmd/collector/app/flags/flags.go +++ b/cmd/collector/app/flags/flags.go @@ -123,6 +123,8 @@ type CollectorOptions struct { CollectorTags map[string]string // SpanSizeMetricsEnabled determines whether to enable metrics based on processed span size SpanSizeMetricsEnabled bool + + Tenancy tenancy.Options } type serverFlagsConfig struct { @@ -130,7 +132,6 @@ type serverFlagsConfig struct { tls tlscfg.ServerFlagsConfig } - // AddFlags adds flags for CollectorOptions func AddFlags(flagSet *flag.FlagSet) { flagSet.Int(flagNumWorkers, DefaultNumWorkers, "The number of workers pulling items from the queue") @@ -183,30 +184,34 @@ func addGRPCFlags(flagSet *flag.FlagSet, cfg serverFlagsConfig, defaultHostPort cfg.tls.AddFlags(flagSet) } -func initHTTPFromViper(v *viper.Viper, _ *zap.Logger, opts *confighttp.ServerConfig,cfg serverFlagsConfig) error { - opts.Endpoint = ports.FormatHostPort(v.GetString(cfg.prefix + "." + flagSuffixHostPort)) - opts.IdleTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPIdleTimeout) - opts.ReadTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadTimeout) - opts.ReadHeaderTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadHeaderTimeout) +func initHTTPFromViper(v *viper.Viper, _ *zap.Logger, opts *confighttp.ServerConfig, cfg serverFlagsConfig) error { tlsOpts, err := cfg.tls.InitFromViper(v) if err != nil { return fmt.Errorf("failed to parse HTTP TLS options: %w", err) } opts.TLSSetting = tlsOpts.ToOtelServerConfig() + opts.Endpoint = ports.FormatHostPort(v.GetString(cfg.prefix + "." + flagSuffixHostPort)) + opts.IdleTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPIdleTimeout) + opts.ReadTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadTimeout) + opts.ReadHeaderTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadHeaderTimeout) + return nil } -func initGRPCFromViper(v *viper.Viper, _ *zap.Logger, opts *configgrpc.ServerConfig,cfg serverFlagsConfig) error { - opts.NetAddr.Endpoint = ports.FormatHostPort(v.GetString(cfg.prefix + "." + flagSuffixHostPort)) - opts.MaxRecvMsgSizeMiB = v.GetInt(cfg.prefix + "." + flagSuffixGRPCMaxReceiveMessageLength) * 1024 * 1024 - opts.Keepalive.ServerParameters.MaxConnectionAge = v.GetDuration(cfg.prefix + "." + flagSuffixGRPCMaxConnectionAge) - opts.Keepalive.ServerParameters.MaxConnectionAgeGrace = v.GetDuration(cfg.prefix + "." + flagSuffixGRPCMaxConnectionAgeGrace) +func initGRPCFromViper(v *viper.Viper, _ *zap.Logger, opts *configgrpc.ServerConfig, cfg serverFlagsConfig) error { tlsOpts, err := cfg.tls.InitFromViper(v) if err != nil { - return fmt.Errorf("failed to parse gRPC TLS options: %w", err) + return fmt.Errorf("failed to parse GRPC TLS options: %w", err) } opts.TLSSetting = tlsOpts.ToOtelServerConfig() - // opts.Tenancy = tenancy.InitFromViper(v) + opts.NetAddr.Endpoint = ports.FormatHostPort(v.GetString(cfg.prefix + "." + flagSuffixHostPort)) + opts.MaxRecvMsgSizeMiB = v.GetInt(cfg.prefix+"."+flagSuffixGRPCMaxReceiveMessageLength) * 1024 * 1024 + opts.Keepalive = &configgrpc.KeepaliveServerConfig{ + ServerParameters: &configgrpc.KeepaliveServerParameters{ + MaxConnectionAge: v.GetDuration(cfg.prefix + "." + flagSuffixGRPCMaxConnectionAge), + MaxConnectionAgeGrace: v.GetDuration(cfg.prefix + "." + flagSuffixGRPCMaxConnectionAgeGrace), + }, + } return nil } @@ -218,22 +223,23 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) cOpts.QueueSize = v.GetUint(flagQueueSize) cOpts.DynQueueSizeMemory = v.GetUint(flagDynQueueSizeMemory) * 1024 * 1024 // we receive in MiB and store in bytes cOpts.SpanSizeMetricsEnabled = v.GetBool(flagSpanSizeMetricsEnabled) + cOpts.Tenancy = tenancy.InitFromViper(v) - if err := initHTTPFromViper(v, logger, &cOpts.HTTP,httpServerFlagsCfg); err != nil { + if err := initHTTPFromViper(v, logger, &cOpts.HTTP, httpServerFlagsCfg); err != nil { return cOpts, fmt.Errorf("failed to parse HTTP server options: %w", err) } - if err := initGRPCFromViper(v, logger, &cOpts.GRPC,grpcServerFlagsCfg); err != nil { + if err := initGRPCFromViper(v, logger, &cOpts.GRPC, grpcServerFlagsCfg); err != nil { return cOpts, fmt.Errorf("failed to parse gRPC server options: %w", err) } cOpts.OTLP.Enabled = v.GetBool(flagCollectorOTLPEnabled) - if err := initHTTPFromViper(v, logger, &cOpts.OTLP.HTTP,otlpServerFlagsCfg.HTTP); err != nil { + if err := initHTTPFromViper(v, logger, &cOpts.OTLP.HTTP, otlpServerFlagsCfg.HTTP); err != nil { return cOpts, fmt.Errorf("failed to parse OTLP/HTTP server options: %w", err) } corsOpts := corsOTLPFlags.InitFromViper(v) cOpts.OTLP.HTTP.CORS = corsOpts.ToOTELCorsConfig() - if err := initGRPCFromViper(v, logger, &cOpts.GRPC,otlpServerFlagsCfg.GRPC); err != nil { + if err := initGRPCFromViper(v, logger, &cOpts.GRPC, otlpServerFlagsCfg.GRPC); err != nil { return cOpts, fmt.Errorf("failed to parse OTLP/gRPC server options: %w", err) } diff --git a/cmd/collector/app/flags/flags_test.go b/cmd/collector/app/flags/flags_test.go index c112e679f4a..bc8b3b81367 100644 --- a/cmd/collector/app/flags/flags_test.go +++ b/cmd/collector/app/flags/flags_test.go @@ -130,41 +130,41 @@ func TestCollectorOptionsWithFlags_CheckMaxConnectionAge(t *testing.T) { assert.Equal(t, 5*time.Second, c.HTTP.ReadHeaderTimeout) } -// func TestCollectorOptionsWithFlags_CheckNoTenancy(t *testing.T) { -// c := &CollectorOptions{} -// v, command := config.Viperize(AddFlags) -// command.ParseFlags([]string{}) -// c.InitFromViper(v, zap.NewNop()) +func TestCollectorOptionsWithFlags_CheckNoTenancy(t *testing.T) { + c := &CollectorOptions{} + v, command := config.Viperize(AddFlags) + command.ParseFlags([]string{}) + c.InitFromViper(v, zap.NewNop()) -// assert.False(t, c.GRPC.Tenancy.Enabled) -// } + assert.False(t, c.Tenancy.Enabled) +} -// func TestCollectorOptionsWithFlags_CheckSimpleTenancy(t *testing.T) { -// c := &CollectorOptions{} -// v, command := config.Viperize(AddFlags) -// command.ParseFlags([]string{ -// "--multi-tenancy.enabled=true", -// }) -// c.InitFromViper(v, zap.NewNop()) +func TestCollectorOptionsWithFlags_CheckSimpleTenancy(t *testing.T) { + c := &CollectorOptions{} + v, command := config.Viperize(AddFlags) + command.ParseFlags([]string{ + "--multi-tenancy.enabled=true", + }) + c.InitFromViper(v, zap.NewNop()) -// assert.True(t, c.GRPC.Tenancy.Enabled) -// assert.Equal(t, "x-tenant", c.GRPC.Tenancy.Header) -// } + assert.True(t, c.Tenancy.Enabled) + assert.Equal(t, "x-tenant", c.Tenancy.Header) +} -// func TestCollectorOptionsWithFlags_CheckFullTenancy(t *testing.T) { -// c := &CollectorOptions{} -// v, command := config.Viperize(AddFlags) -// command.ParseFlags([]string{ -// "--multi-tenancy.enabled=true", -// "--multi-tenancy.header=custom-tenant-header", -// "--multi-tenancy.tenants=acme,hardware-store", -// }) -// c.InitFromViper(v, zap.NewNop()) +func TestCollectorOptionsWithFlags_CheckFullTenancy(t *testing.T) { + c := &CollectorOptions{} + v, command := config.Viperize(AddFlags) + command.ParseFlags([]string{ + "--multi-tenancy.enabled=true", + "--multi-tenancy.header=custom-tenant-header", + "--multi-tenancy.tenants=acme,hardware-store", + }) + c.InitFromViper(v, zap.NewNop()) -// assert.True(t, c.GRPC.Tenancy.Enabled) -// assert.Equal(t, "custom-tenant-header", c.GRPC.Tenancy.Header) -// assert.Equal(t, []string{"acme", "hardware-store"}, c.GRPC.Tenancy.Tenants) -// } + assert.True(t, c.Tenancy.Enabled) + assert.Equal(t, "custom-tenant-header", c.Tenancy.Header) + assert.Equal(t, []string{"acme", "hardware-store"}, c.Tenancy.Tenants) +} // func TestCollectorOptionsWithFlags_CheckZipkinKeepAlive(t *testing.T) { // c := &CollectorOptions{} diff --git a/cmd/collector/app/handler/otlp_receiver.go b/cmd/collector/app/handler/otlp_receiver.go index 8704e958153..afa7ff8b96d 100644 --- a/cmd/collector/app/handler/otlp_receiver.go +++ b/cmd/collector/app/handler/otlp_receiver.go @@ -58,6 +58,10 @@ func startOTLPReceiver( cfg component.Config, nextConsumer consumer.Traces) (receiver.Traces, error), ) (receiver.Traces, error) { otlpReceiverConfig := otlpFactory.CreateDefaultConfig().(*otlpreceiver.Config) + otlpReceiverConfig.GRPC = &options.OTLP.GRPC + otlpReceiverConfig.HTTP.ServerConfig = &options.OTLP.HTTP + fmt.Printf("OTLP HTTP Endpoint: %s\n", otlpReceiverConfig.HTTP.ServerConfig.Endpoint) + fmt.Printf("OTLP GRPC Endpoint: %s\n", otlpReceiverConfig.GRPC.NetAddr.Endpoint) statusReporter := func(ev *componentstatus.Event) { // TODO this could be wired into changing healthcheck.HealthCheck logger.Info("OTLP receiver status change", zap.Stringer("status", ev.Status())) diff --git a/cmd/collector/app/handler/zipkin_receiver.go b/cmd/collector/app/handler/zipkin_receiver.go index c28a5adbc06..357df299c58 100644 --- a/cmd/collector/app/handler/zipkin_receiver.go +++ b/cmd/collector/app/handler/zipkin_receiver.go @@ -45,7 +45,7 @@ func StartZipkinReceiver( // which is a situation we cannot reproduce. To test our own error handling, this // function allows to mock those constructors. func startZipkinReceiver( - _ *flags.CollectorOptions, + options *flags.CollectorOptions, logger *zap.Logger, spanProcessor processor.SpanProcessor, tm *tenancy.Manager, @@ -56,6 +56,7 @@ func startZipkinReceiver( cfg component.Config, nextConsumer consumer.Traces) (receiver.Traces, error), ) (receiver.Traces, error) { receiverConfig := zipkinFactory.CreateDefaultConfig().(*zipkinreceiver.Config) + receiverConfig.ServerConfig = options.Zipkin receiverSettings := receiver.Settings{ TelemetrySettings: component.TelemetrySettings{ Logger: logger, diff --git a/cmd/collector/app/handler/zipkin_receiver_tls_test.go b/cmd/collector/app/handler/zipkin_receiver_tls_test.go index 5d31d435715..f0f8e80a07e 100644 --- a/cmd/collector/app/handler/zipkin_receiver_tls_test.go +++ b/cmd/collector/app/handler/zipkin_receiver_tls_test.go @@ -194,7 +194,8 @@ func TestSpanCollectorZipkinTLS(t *testing.T) { } response, requestError := client.Post(fmt.Sprintf("https://localhost:%d", ports.CollectorZipkin), "", nil) - + fmt.Print(response) + fmt.Print(requestError) if test.expectZipkinClientErr { require.Error(t, requestError) } else { diff --git a/cmd/collector/app/server/grpc.go b/cmd/collector/app/server/grpc.go index b9fbdbf41da..8100158a943 100644 --- a/cmd/collector/app/server/grpc.go +++ b/cmd/collector/app/server/grpc.go @@ -7,9 +7,8 @@ import ( "context" "fmt" "net" - "time" - "go.opentelemetry.io/collector/config/configtls" + "go.opentelemetry.io/collector/config/configgrpc" "go.uber.org/zap" "google.golang.org/grpc" "google.golang.org/grpc/credentials" @@ -26,15 +25,11 @@ import ( // GRPCServerParams to construct a new Jaeger Collector gRPC Server type GRPCServerParams struct { - TLSConfig *configtls.ServerConfig - HostPort string - Handler *handler.GRPCHandler - SamplingProvider samplingstrategy.Provider - Logger *zap.Logger - OnError func(error) - MaxReceiveMessageLength int - MaxConnectionAge time.Duration - MaxConnectionAgeGrace time.Duration + *configgrpc.ServerConfig + Handler *handler.GRPCHandler + SamplingProvider samplingstrategy.Provider + Logger *zap.Logger + OnError func(error) // Set by the server to indicate the actual host:port of the server. HostPortActual string @@ -45,17 +40,19 @@ func StartGRPCServer(params *GRPCServerParams) (*grpc.Server, error) { var server *grpc.Server var grpcOpts []grpc.ServerOption - if params.MaxReceiveMessageLength > 0 { - grpcOpts = append(grpcOpts, grpc.MaxRecvMsgSize(params.MaxReceiveMessageLength)) + if params.MaxRecvMsgSizeMiB > 0 { + grpcOpts = append(grpcOpts, grpc.MaxRecvMsgSize(params.MaxRecvMsgSizeMiB)) + } + if params.Keepalive != nil { + grpcOpts = append(grpcOpts, grpc.KeepaliveParams(keepalive.ServerParameters{ + MaxConnectionAge: params.Keepalive.ServerParameters.MaxConnectionAge, + MaxConnectionAgeGrace: params.Keepalive.ServerParameters.MaxConnectionAgeGrace, + })) } - grpcOpts = append(grpcOpts, grpc.KeepaliveParams(keepalive.ServerParameters{ - MaxConnectionAge: params.MaxConnectionAge, - MaxConnectionAgeGrace: params.MaxConnectionAgeGrace, - })) - if params.TLSConfig != nil { + if params.TLSSetting != nil { // user requested a server with TLS, setup creds - tlsCfg, err := params.TLSConfig.LoadTLSConfig(context.Background()) + tlsCfg, err := params.TLSSetting.LoadTLSConfig(context.Background()) if err != nil { return nil, err } @@ -66,8 +63,8 @@ func StartGRPCServer(params *GRPCServerParams) (*grpc.Server, error) { server = grpc.NewServer(grpcOpts...) reflection.Register(server) - - listener, err := net.Listen("tcp", params.HostPort) + fmt.Printf("grpc endpoint %v \n", params.NetAddr.Endpoint) + listener, err := net.Listen("tcp", params.NetAddr.Endpoint) if err != nil { return nil, fmt.Errorf("failed to listen on gRPC port: %w", err) } diff --git a/cmd/collector/app/server/grpc_test.go b/cmd/collector/app/server/grpc_test.go index d16623138dc..b0fa1cd40ca 100644 --- a/cmd/collector/app/server/grpc_test.go +++ b/cmd/collector/app/server/grpc_test.go @@ -10,6 +10,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/config/configgrpc" + "go.opentelemetry.io/collector/config/confignet" "go.uber.org/zap" "go.uber.org/zap/zapcore" "go.uber.org/zap/zaptest/observer" @@ -28,7 +30,11 @@ import ( func TestFailToListen(t *testing.T) { logger, _ := zap.NewDevelopment() server, err := StartGRPCServer(&GRPCServerParams{ - HostPort: ":-1", + ServerConfig: &configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: ":-1", + }, + }, Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}, &tenancy.Manager{}), SamplingProvider: &mockSamplingProvider{}, Logger: logger, @@ -63,10 +69,12 @@ func TestFailServe(t *testing.T) { func TestSpanCollector(t *testing.T) { logger, _ := zap.NewDevelopment() params := &GRPCServerParams{ - Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}, &tenancy.Manager{}), - SamplingProvider: &mockSamplingProvider{}, - Logger: logger, - MaxReceiveMessageLength: 1024 * 1024, + Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}, &tenancy.Manager{}), + SamplingProvider: &mockSamplingProvider{}, + Logger: logger, + ServerConfig: &configgrpc.ServerConfig{ + MaxRecvMsgSizeMiB: 1, + }, } server, err := StartGRPCServer(params) @@ -97,7 +105,9 @@ func TestCollectorStartWithTLS(t *testing.T) { Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}, &tenancy.Manager{}), SamplingProvider: &mockSamplingProvider{}, Logger: logger, - TLSConfig: opts.ToOtelServerConfig(), + ServerConfig: &configgrpc.ServerConfig{ + TLSSetting: opts.ToOtelServerConfig(), + }, } server, err := StartGRPCServer(params) require.NoError(t, err) diff --git a/cmd/collector/app/server/http.go b/cmd/collector/app/server/http.go index 1dcba13e5d0..411df873b2e 100644 --- a/cmd/collector/app/server/http.go +++ b/cmd/collector/app/server/http.go @@ -5,7 +5,6 @@ package server import ( "context" - "io" "net" "net/http" "time" @@ -42,11 +41,6 @@ type HTTPServerParams struct { IdleTimeout time.Duration } -type httpServer struct { - *http.Server - staticHandlerCloser io.Closer -} - // StartHTTPServer based on the given parameters func StartHTTPServer(params *HTTPServerParams) (*http.Server, error) { params.Logger.Info("Starting jaeger-collector HTTP server", zap.String("http host-port", params.HostPort)) diff --git a/cmd/collector/main.go b/cmd/collector/main.go index db658c209b4..5c10bbfaca7 100644 --- a/cmd/collector/main.go +++ b/cmd/collector/main.go @@ -89,7 +89,7 @@ func main() { if err != nil { logger.Fatal("Failed to initialize collector", zap.Error(err)) } - tm := tenancy.NewManager(&collectorOpts.GRPC.Tenancy) + tm := tenancy.NewManager(&collectorOpts.Tenancy) collector := app.New(&app.CollectorParams{ ServiceName: serviceName, diff --git a/pkg/config/corscfg/options.go b/pkg/config/corscfg/options.go index eee1f557d74..3f7979b07f8 100644 --- a/pkg/config/corscfg/options.go +++ b/pkg/config/corscfg/options.go @@ -10,9 +10,9 @@ type Options struct { AllowedHeaders []string } -func (o *Options) ToOTELCorsConfig() *confighttp.CORSConfig{ +func (o *Options) ToOTELCorsConfig() *confighttp.CORSConfig { return &confighttp.CORSConfig{ AllowedOrigins: o.AllowedOrigins, AllowedHeaders: o.AllowedHeaders, } -} \ No newline at end of file +} From a55d3a287eeeeeec4b25851fd5784104d3d2dfa7 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Tue, 3 Dec 2024 19:00:25 +0530 Subject: [PATCH 06/18] changes Signed-off-by: chahatsagarmain --- cmd/collector/app/collector.go | 20 ++------ cmd/collector/app/collector_test.go | 16 ++++++- cmd/collector/app/flags/flags.go | 25 ++++++---- cmd/collector/app/flags/flags_test.go | 4 +- .../app/handler/otlp_receiver_test.go | 1 + cmd/collector/app/server/grpc.go | 29 ++++++++---- cmd/collector/app/server/grpc_test.go | 22 +++++++-- cmd/collector/app/server/http.go | 47 ++++++++----------- cmd/collector/app/server/http_test.go | 39 +++++++++------ 9 files changed, 117 insertions(+), 86 deletions(-) diff --git a/cmd/collector/app/collector.go b/cmd/collector/app/collector.go index be9e08c9fd1..b601b6e42e8 100644 --- a/cmd/collector/app/collector.go +++ b/cmd/collector/app/collector.go @@ -9,8 +9,6 @@ import ( "net/http" "time" - "go.opentelemetry.io/collector/config/configgrpc" - "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/receiver" "go.uber.org/zap" "google.golang.org/grpc" @@ -104,23 +102,15 @@ func (c *Collector) Start(options *flags.CollectorOptions) error { Handler: c.spanHandlers.GRPCHandler, SamplingProvider: c.samplingProvider, Logger: c.logger, - ServerConfig: &configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: options.GRPC.NetAddr.Endpoint, - }, - TLSSetting: options.GRPC.TLSSetting, - MaxRecvMsgSizeMiB: options.GRPC.MaxRecvMsgSizeMiB, - Keepalive: options.GRPC.Keepalive, - }, + ServerConfig: options.GRPC, }) if err != nil { return fmt.Errorf("could not start gRPC server: %w", err) } c.grpcServer = grpcServer httpServer, err := server.StartHTTPServer(&server.HTTPServerParams{ - HostPort: options.HTTP.Endpoint, + ServerConfig: options.HTTP, Handler: c.spanHandlers.JaegerBatchesHandler, - TLSConfig: options.HTTP.TLSSetting, HealthCheck: c.hCheck, MetricsFactory: c.metricsFactory, SamplingProvider: c.samplingProvider, @@ -171,11 +161,11 @@ func (c *Collector) Close() error { // Stop HTTP server if c.hServer != nil { - timeout, cancel := context.WithTimeout(context.Background(), 5*time.Second) - if err := c.hServer.Shutdown(timeout); err != nil { + // timeout, cancel := context.WithTimeout(context.Background(), 5*time.Second) + if err := c.hServer.Close(); err != nil { c.logger.Fatal("failed to stop the main HTTP server", zap.Error(err)) } - defer cancel() + // defer cancel() } // Stop Zipkin receiver diff --git a/cmd/collector/app/collector_test.go b/cmd/collector/app/collector_test.go index 96605f73135..dc14d8f7e23 100644 --- a/cmd/collector/app/collector_test.go +++ b/cmd/collector/app/collector_test.go @@ -15,6 +15,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/config/configgrpc" + "go.opentelemetry.io/collector/config/confighttp" + "go.opentelemetry.io/collector/config/confignet" + "go.opentelemetry.io/collector/config/configtls" "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/collector/app/flags" @@ -36,7 +39,11 @@ func optionsForEphemeralPorts() *flags.CollectorOptions { MaxConnectionIdle: 10, }, } - collectorOpts.HTTP.Endpoint = ":0" + collectorOpts.GRPC.NetAddr.Transport = confignet.TransportTypeTCP + collectorOpts.HTTP = confighttp.ServerConfig{ + Endpoint: ":0", + TLSSetting: &configtls.ServerConfig{}, + } collectorOpts.OTLP.Enabled = true collectorOpts.OTLP.GRPC.NetAddr.Endpoint = ":0" collectorOpts.OTLP.GRPC.Keepalive = &configgrpc.KeepaliveServerConfig{ @@ -44,7 +51,12 @@ func optionsForEphemeralPorts() *flags.CollectorOptions { MaxConnectionIdle: 10, }, } - collectorOpts.OTLP.HTTP.Endpoint = ":0" + collectorOpts.OTLP.GRPC.NetAddr.Transport = confignet.TransportTypeTCP + + collectorOpts.OTLP.HTTP = confighttp.ServerConfig{ + Endpoint: ":0", + TLSSetting: &configtls.ServerConfig{}, + } collectorOpts.Zipkin.Endpoint = ":0" collectorOpts.Tenancy = tenancy.Options{} return collectorOpts diff --git a/cmd/collector/app/flags/flags.go b/cmd/collector/app/flags/flags.go index 54c1141d975..690ccfb04a6 100644 --- a/cmd/collector/app/flags/flags.go +++ b/cmd/collector/app/flags/flags.go @@ -87,8 +87,15 @@ var otlpServerFlagsCfg = struct { }, } -var tlsZipkinFlagsConfig = tlscfg.ServerFlagsConfig{ - Prefix: "collector.zipkin", +var zipkinServerFlagsCfg = struct { + HTTP serverFlagsConfig +}{ + HTTP: serverFlagsConfig{ + prefix: "collector.zipkin", + tls: tlscfg.ServerFlagsConfig{ + Prefix: "collector.zipkin", + }, + }, } var corsZipkinFlags = corscfg.Flags{ @@ -150,7 +157,7 @@ func AddFlags(flagSet *flag.FlagSet) { flagSet.String(flagZipkinHTTPHostPort, "", "The host:port (e.g. 127.0.0.1:9411 or :9411) of the collector's Zipkin server (disabled by default)") flagSet.Bool(flagZipkinKeepAliveEnabled, true, "KeepAlive configures allow Keep-Alive for Zipkin HTTP server (enabled by default)") - tlsZipkinFlagsConfig.AddFlags(flagSet) + zipkinServerFlagsCfg.HTTP.tls.AddFlags(flagSet) corsZipkinFlags.AddFlags(flagSet) tenancy.AddFlags(flagSet) @@ -205,7 +212,7 @@ func initGRPCFromViper(v *viper.Viper, _ *zap.Logger, opts *configgrpc.ServerCon } opts.TLSSetting = tlsOpts.ToOtelServerConfig() opts.NetAddr.Endpoint = ports.FormatHostPort(v.GetString(cfg.prefix + "." + flagSuffixHostPort)) - opts.MaxRecvMsgSizeMiB = v.GetInt(cfg.prefix+"."+flagSuffixGRPCMaxReceiveMessageLength) * 1024 * 1024 + opts.MaxRecvMsgSizeMiB = v.GetInt(cfg.prefix+"."+flagSuffixGRPCMaxReceiveMessageLength) / (1024 * 1024) opts.Keepalive = &configgrpc.KeepaliveServerConfig{ ServerParameters: &configgrpc.KeepaliveServerParameters{ MaxConnectionAge: v.GetDuration(cfg.prefix + "." + flagSuffixGRPCMaxConnectionAge), @@ -239,17 +246,15 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) } corsOpts := corsOTLPFlags.InitFromViper(v) cOpts.OTLP.HTTP.CORS = corsOpts.ToOTELCorsConfig() - if err := initGRPCFromViper(v, logger, &cOpts.GRPC, otlpServerFlagsCfg.GRPC); err != nil { + if err := initGRPCFromViper(v, logger, &cOpts.OTLP.GRPC, otlpServerFlagsCfg.GRPC); err != nil { return cOpts, fmt.Errorf("failed to parse OTLP/gRPC server options: %w", err) } // cOpts.Zipkin. = v.GetBool(flagZipkinKeepAliveEnabled) - cOpts.Zipkin.Endpoint = ports.FormatHostPort(v.GetString(flagZipkinHTTPHostPort)) - tlsZipkin, err := tlsZipkinFlagsConfig.InitFromViper(v) - if err != nil { - return cOpts, fmt.Errorf("failed to parse Zipkin TLS options: %w", err) + + if err := initHTTPFromViper(v, logger, &cOpts.Zipkin, zipkinServerFlagsCfg.HTTP); err != nil { + return cOpts, fmt.Errorf("failed to parse Zipkin server options: %w", err) } - cOpts.Zipkin.TLSSetting = tlsZipkin.ToOtelServerConfig() corsOpts = corsZipkinFlags.InitFromViper(v) cOpts.Zipkin.CORS = corsOpts.ToOTELCorsConfig() diff --git a/cmd/collector/app/flags/flags_test.go b/cmd/collector/app/flags/flags_test.go index bc8b3b81367..0297672b69e 100644 --- a/cmd/collector/app/flags/flags_test.go +++ b/cmd/collector/app/flags/flags_test.go @@ -107,7 +107,7 @@ func TestCollectorOptionsWithFlags_CheckMaxReceiveMessageLength(t *testing.T) { _, err := c.InitFromViper(v, zap.NewNop()) require.NoError(t, err) - assert.Equal(t, 8388608, c.GRPC.MaxRecvMsgSizeMiB) + assert.Equal(t, 8, c.GRPC.MaxRecvMsgSizeMiB) } func TestCollectorOptionsWithFlags_CheckMaxConnectionAge(t *testing.T) { @@ -174,7 +174,7 @@ func TestCollectorOptionsWithFlags_CheckFullTenancy(t *testing.T) { // }) // c.InitFromViper(v, zap.NewNop()) -// assert.False(t, c.Zipkin.KeepAlive) +// assert.False(t, c.Zipkin.) // } func TestMain(m *testing.M) { diff --git a/cmd/collector/app/handler/otlp_receiver_test.go b/cmd/collector/app/handler/otlp_receiver_test.go index c36abac0b09..8cb046ffba0 100644 --- a/cmd/collector/app/handler/otlp_receiver_test.go +++ b/cmd/collector/app/handler/otlp_receiver_test.go @@ -35,6 +35,7 @@ func optionsWithPorts(port string) *flags.CollectorOptions { Endpoint: port, }, } + opts.OTLP.GRPC.NetAddr.Transport = confignet.TransportTypeTCP return opts } diff --git a/cmd/collector/app/server/grpc.go b/cmd/collector/app/server/grpc.go index 8100158a943..17301ac184c 100644 --- a/cmd/collector/app/server/grpc.go +++ b/cmd/collector/app/server/grpc.go @@ -8,7 +8,11 @@ import ( "fmt" "net" + "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configgrpc" + "go.opentelemetry.io/collector/config/configtelemetry" + "go.opentelemetry.io/otel/metric" + "go.opentelemetry.io/otel/metric/noop" "go.uber.org/zap" "google.golang.org/grpc" "google.golang.org/grpc/credentials" @@ -25,7 +29,7 @@ import ( // GRPCServerParams to construct a new Jaeger Collector gRPC Server type GRPCServerParams struct { - *configgrpc.ServerConfig + configgrpc.ServerConfig Handler *handler.GRPCHandler SamplingProvider samplingstrategy.Provider Logger *zap.Logger @@ -38,16 +42,16 @@ type GRPCServerParams struct { // StartGRPCServer based on the given parameters func StartGRPCServer(params *GRPCServerParams) (*grpc.Server, error) { var server *grpc.Server - var grpcOpts []grpc.ServerOption + var grpcOpts []configgrpc.ToServerOption if params.MaxRecvMsgSizeMiB > 0 { - grpcOpts = append(grpcOpts, grpc.MaxRecvMsgSize(params.MaxRecvMsgSizeMiB)) + grpcOpts = append(grpcOpts, configgrpc.WithGrpcServerOption(grpc.MaxRecvMsgSize(params.MaxRecvMsgSizeMiB))) } if params.Keepalive != nil { - grpcOpts = append(grpcOpts, grpc.KeepaliveParams(keepalive.ServerParameters{ + grpcOpts = append(grpcOpts, configgrpc.WithGrpcServerOption(grpc.KeepaliveParams(keepalive.ServerParameters{ MaxConnectionAge: params.Keepalive.ServerParameters.MaxConnectionAge, MaxConnectionAgeGrace: params.Keepalive.ServerParameters.MaxConnectionAgeGrace, - })) + }))) } if params.TLSSetting != nil { @@ -58,13 +62,20 @@ func StartGRPCServer(params *GRPCServerParams) (*grpc.Server, error) { } creds := credentials.NewTLS(tlsCfg) - grpcOpts = append(grpcOpts, grpc.Creds(creds)) + grpcOpts = append(grpcOpts, configgrpc.WithGrpcServerOption(grpc.Creds(creds))) } - server = grpc.NewServer(grpcOpts...) + server, err := params.ToServer(context.Background(), nil, component.TelemetrySettings{ + Logger: params.Logger, + LeveledMeterProvider: func(_ configtelemetry.Level) metric.MeterProvider { + return noop.NewMeterProvider() + }, + }, grpcOpts...) + if err != nil { + return nil, err + } reflection.Register(server) - fmt.Printf("grpc endpoint %v \n", params.NetAddr.Endpoint) - listener, err := net.Listen("tcp", params.NetAddr.Endpoint) + listener, err := params.NetAddr.Listen(context.Background()) if err != nil { return nil, fmt.Errorf("failed to listen on gRPC port: %w", err) } diff --git a/cmd/collector/app/server/grpc_test.go b/cmd/collector/app/server/grpc_test.go index b0fa1cd40ca..3ac6351d628 100644 --- a/cmd/collector/app/server/grpc_test.go +++ b/cmd/collector/app/server/grpc_test.go @@ -30,9 +30,10 @@ import ( func TestFailToListen(t *testing.T) { logger, _ := zap.NewDevelopment() server, err := StartGRPCServer(&GRPCServerParams{ - ServerConfig: &configgrpc.ServerConfig{ + ServerConfig: configgrpc.ServerConfig{ NetAddr: confignet.AddrConfig{ - Endpoint: ":-1", + Endpoint: ":-1", + Transport: confignet.TransportTypeTCP, }, }, Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}, &tenancy.Manager{}), @@ -72,8 +73,11 @@ func TestSpanCollector(t *testing.T) { Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}, &tenancy.Manager{}), SamplingProvider: &mockSamplingProvider{}, Logger: logger, - ServerConfig: &configgrpc.ServerConfig{ - MaxRecvMsgSizeMiB: 1, + ServerConfig: configgrpc.ServerConfig{ + MaxRecvMsgSizeMiB: 2, + NetAddr: confignet.AddrConfig{ + Transport: confignet.TransportTypeTCP, + }, }, } @@ -105,7 +109,10 @@ func TestCollectorStartWithTLS(t *testing.T) { Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}, &tenancy.Manager{}), SamplingProvider: &mockSamplingProvider{}, Logger: logger, - ServerConfig: &configgrpc.ServerConfig{ + ServerConfig: configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Transport: confignet.TransportTypeTCP, + }, TLSSetting: opts.ToOtelServerConfig(), }, } @@ -120,6 +127,11 @@ func TestCollectorReflection(t *testing.T) { Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}, &tenancy.Manager{}), SamplingProvider: &mockSamplingProvider{}, Logger: logger, + ServerConfig: configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Transport: confignet.TransportTypeTCP, + }, + }, } server, err := StartGRPCServer(params) diff --git a/cmd/collector/app/server/http.go b/cmd/collector/app/server/http.go index 411df873b2e..28f63b4387e 100644 --- a/cmd/collector/app/server/http.go +++ b/cmd/collector/app/server/http.go @@ -7,10 +7,13 @@ import ( "context" "net" "net/http" - "time" "github.com/gorilla/mux" - "go.opentelemetry.io/collector/config/configtls" + "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/config/confighttp" + "go.opentelemetry.io/collector/config/configtelemetry" + "go.opentelemetry.io/otel/metric" + "go.opentelemetry.io/otel/metric/noop" "go.uber.org/zap" "go.uber.org/zap/zapcore" @@ -25,43 +28,31 @@ import ( // HTTPServerParams to construct a new Jaeger Collector HTTP Server type HTTPServerParams struct { - TLSConfig *configtls.ServerConfig - HostPort string + confighttp.ServerConfig Handler handler.JaegerBatchesHandler SamplingProvider samplingstrategy.Provider MetricsFactory metrics.Factory HealthCheck *healthcheck.HealthCheck Logger *zap.Logger - - // ReadTimeout sets the respective parameter of http.Server - ReadTimeout time.Duration - // ReadHeaderTimeout sets the respective parameter of http.Server - ReadHeaderTimeout time.Duration - // IdleTimeout sets the respective parameter of http.Server - IdleTimeout time.Duration } // StartHTTPServer based on the given parameters func StartHTTPServer(params *HTTPServerParams) (*http.Server, error) { - params.Logger.Info("Starting jaeger-collector HTTP server", zap.String("http host-port", params.HostPort)) + params.Logger.Info("Starting jaeger-collector HTTP server", zap.String("http host-port", params.Endpoint)) errorLog, _ := zap.NewStdLogAt(params.Logger, zapcore.ErrorLevel) - server := &http.Server{ - Addr: params.HostPort, - ReadTimeout: params.ReadTimeout, - ReadHeaderTimeout: params.ReadHeaderTimeout, - IdleTimeout: params.IdleTimeout, - ErrorLog: errorLog, - } - if params.TLSConfig != nil { - tlsCfg, err := params.TLSConfig.LoadTLSConfig(context.Background()) - if err != nil { - return nil, err - } - server.TLSConfig = tlsCfg + server, err := params.ToServer(context.Background(), nil, component.TelemetrySettings{ + Logger: params.Logger, + LeveledMeterProvider: func(_ configtelemetry.Level) metric.MeterProvider { + return noop.NewMeterProvider() + }, + }, nil) + server.ErrorLog = errorLog + if err != nil { + return nil, err } - listener, err := net.Listen("tcp", params.HostPort) + listener, err := params.ToListener(context.Background()) if err != nil { return nil, err } @@ -91,8 +82,8 @@ func serveHTTP(server *http.Server, listener net.Listener, params *HTTPServerPar server.Handler = httpmetrics.Wrap(recoveryHandler(r), params.MetricsFactory, params.Logger) go func() { var err error - if params.TLSConfig != nil { - err = server.ServeTLS(listener, "", "") + if params.TLSSetting != nil { + err = server.ServeTLS(listener,params.TLSSetting.CertFile, params.TLSSetting.KeyFile) } else { err = server.Serve(listener) } diff --git a/cmd/collector/app/server/http_test.go b/cmd/collector/app/server/http_test.go index b72cfc54039..9978e856db8 100644 --- a/cmd/collector/app/server/http_test.go +++ b/cmd/collector/app/server/http_test.go @@ -16,6 +16,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/config/confighttp" "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/collector/app/handler" @@ -31,8 +32,10 @@ var testCertKeyLocation = "../../../../pkg/config/tlscfg/testdata" func TestFailToListenHTTP(t *testing.T) { logger, _ := zap.NewDevelopment() server, err := StartHTTPServer(&HTTPServerParams{ - HostPort: ":-1", - Logger: logger, + ServerConfig: confighttp.ServerConfig{ + Endpoint: ":-1", + }, + Logger: logger, }) assert.Nil(t, server) require.EqualError(t, err, "listen tcp: address -1: invalid port") @@ -48,10 +51,12 @@ func TestCreateTLSHTTPServerError(t *testing.T) { } params := &HTTPServerParams{ - HostPort: fmt.Sprintf(":%d", ports.CollectorHTTP), + ServerConfig: confighttp.ServerConfig{ + Endpoint: fmt.Sprintf(":%d", ports.CollectorHTTP), + TLSSetting: tlsCfg.ToOtelServerConfig(), + }, HealthCheck: healthcheck.New(), Logger: logger, - TLSConfig: tlsCfg.ToOtelServerConfig(), } _, err := StartHTTPServer(params) require.Error(t, err) @@ -188,13 +193,15 @@ func TestSpanCollectorHTTPS(t *testing.T) { mFact := metricstest.NewFactory(time.Hour) defer mFact.Backend.Stop() params := &HTTPServerParams{ - HostPort: fmt.Sprintf(":%d", ports.CollectorHTTP), + ServerConfig: confighttp.ServerConfig{ + Endpoint: fmt.Sprintf(":%d", ports.CollectorHTTP), + TLSSetting: test.TLS.ToOtelServerConfig(), + }, Handler: handler.NewJaegerSpanHandler(logger, &mockSpanProcessor{}), SamplingProvider: &mockSamplingProvider{}, MetricsFactory: mFact, HealthCheck: healthcheck.New(), Logger: logger, - TLSConfig: test.TLS.ToOtelServerConfig(), } server, err := StartHTTPServer(params) @@ -248,15 +255,17 @@ func TestStartHTTPServerParams(t *testing.T) { mFact := metricstest.NewFactory(time.Hour) defer mFact.Stop() params := &HTTPServerParams{ - HostPort: fmt.Sprintf(":%d", ports.CollectorHTTP), - Handler: handler.NewJaegerSpanHandler(logger, &mockSpanProcessor{}), - SamplingProvider: &mockSamplingProvider{}, - MetricsFactory: mFact, - HealthCheck: healthcheck.New(), - Logger: logger, - IdleTimeout: 5 * time.Minute, - ReadTimeout: 6 * time.Minute, - ReadHeaderTimeout: 7 * time.Second, + ServerConfig: confighttp.ServerConfig{ + Endpoint: fmt.Sprintf(":%d", ports.CollectorHTTP), + IdleTimeout: 5 * time.Minute, + ReadTimeout: 6 * time.Minute, + ReadHeaderTimeout: 7 * time.Second, + }, + Handler: handler.NewJaegerSpanHandler(logger, &mockSpanProcessor{}), + SamplingProvider: &mockSamplingProvider{}, + MetricsFactory: mFact, + HealthCheck: healthcheck.New(), + Logger: logger, } server, err := StartHTTPServer(params) From 01798c7cb3eb7f841041dada3e263a77665b7a42 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Tue, 3 Dec 2024 19:01:34 +0530 Subject: [PATCH 07/18] shutdown Signed-off-by: chahatsagarmain --- cmd/collector/app/collector.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/collector/app/collector.go b/cmd/collector/app/collector.go index b601b6e42e8..d830b152462 100644 --- a/cmd/collector/app/collector.go +++ b/cmd/collector/app/collector.go @@ -161,11 +161,11 @@ func (c *Collector) Close() error { // Stop HTTP server if c.hServer != nil { - // timeout, cancel := context.WithTimeout(context.Background(), 5*time.Second) - if err := c.hServer.Close(); err != nil { + timeout, cancel := context.WithTimeout(context.Background(), 5*time.Second) + if err := c.hServer.Shutdown(timeout); err != nil { c.logger.Fatal("failed to stop the main HTTP server", zap.Error(err)) } - // defer cancel() + defer cancel() } // Stop Zipkin receiver From 7ebd4a02c5e9931c705fcc3608d9bbfe0f82a53c Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Thu, 5 Dec 2024 21:36:05 +0530 Subject: [PATCH 08/18] changes Signed-off-by: chahatsagarmain --- cmd/collector/app/collector.go | 1 - cmd/collector/app/flags/flags.go | 9 +++++--- cmd/collector/app/flags/flags_test.go | 20 ++++++++-------- cmd/collector/app/handler/zipkin_receiver.go | 2 +- cmd/collector/app/server/grpc.go | 24 +------------------- cmd/collector/app/server/http.go | 13 +++++------ cmd/collector/app/server/http_test.go | 16 ++++++++----- 7 files changed, 34 insertions(+), 51 deletions(-) diff --git a/cmd/collector/app/collector.go b/cmd/collector/app/collector.go index d830b152462..d867f348db5 100644 --- a/cmd/collector/app/collector.go +++ b/cmd/collector/app/collector.go @@ -97,7 +97,6 @@ func (c *Collector) Start(options *flags.CollectorOptions) error { c.spanProcessor = handlerBuilder.BuildSpanProcessor(additionalProcessors...) c.spanHandlers = handlerBuilder.BuildHandlers(c.spanProcessor) - // fmt.Printf("%v \n",options.GRPC.NetAddr.Endpoint) grpcServer, err := server.StartGRPCServer(&server.GRPCServerParams{ Handler: c.spanHandlers.GRPCHandler, SamplingProvider: c.samplingProvider, diff --git a/cmd/collector/app/flags/flags.go b/cmd/collector/app/flags/flags.go index 690ccfb04a6..9e5d1050563 100644 --- a/cmd/collector/app/flags/flags.go +++ b/cmd/collector/app/flags/flags.go @@ -125,7 +125,10 @@ type CollectorOptions struct { HTTP confighttp.ServerConfig } // Zipkin section defines options for Zipkin HTTP server - Zipkin confighttp.ServerConfig + Zipkin struct { + confighttp.ServerConfig + KeepAlive bool `mapstructure:"keep-alive"` // Add keepalive setting + } // CollectorTags is the string representing collector tags to append to each and every span CollectorTags map[string]string // SpanSizeMetricsEnabled determines whether to enable metrics based on processed span size @@ -250,9 +253,9 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) return cOpts, fmt.Errorf("failed to parse OTLP/gRPC server options: %w", err) } - // cOpts.Zipkin. = v.GetBool(flagZipkinKeepAliveEnabled) + cOpts.Zipkin.KeepAlive = v.GetBool(flagZipkinKeepAliveEnabled) - if err := initHTTPFromViper(v, logger, &cOpts.Zipkin, zipkinServerFlagsCfg.HTTP); err != nil { + if err := initHTTPFromViper(v, logger, &cOpts.Zipkin.ServerConfig, zipkinServerFlagsCfg.HTTP); err != nil { return cOpts, fmt.Errorf("failed to parse Zipkin server options: %w", err) } corsOpts = corsZipkinFlags.InitFromViper(v) diff --git a/cmd/collector/app/flags/flags_test.go b/cmd/collector/app/flags/flags_test.go index 0297672b69e..39727d1bce8 100644 --- a/cmd/collector/app/flags/flags_test.go +++ b/cmd/collector/app/flags/flags_test.go @@ -166,16 +166,16 @@ func TestCollectorOptionsWithFlags_CheckFullTenancy(t *testing.T) { assert.Equal(t, []string{"acme", "hardware-store"}, c.Tenancy.Tenants) } -// func TestCollectorOptionsWithFlags_CheckZipkinKeepAlive(t *testing.T) { -// c := &CollectorOptions{} -// v, command := config.Viperize(AddFlags) -// command.ParseFlags([]string{ -// "--collector.zipkin.keep-alive=false", -// }) -// c.InitFromViper(v, zap.NewNop()) - -// assert.False(t, c.Zipkin.) -// } +func TestCollectorOptionsWithFlags_CheckZipkinKeepAlive(t *testing.T) { + c := &CollectorOptions{} + v, command := config.Viperize(AddFlags) + command.ParseFlags([]string{ + "--collector.zipkin.keep-alive=false", + }) + c.InitFromViper(v, zap.NewNop()) + + assert.False(t, c.Zipkin.KeepAlive) +} func TestMain(m *testing.M) { testutils.VerifyGoLeaks(m) diff --git a/cmd/collector/app/handler/zipkin_receiver.go b/cmd/collector/app/handler/zipkin_receiver.go index 357df299c58..b7fc18619e3 100644 --- a/cmd/collector/app/handler/zipkin_receiver.go +++ b/cmd/collector/app/handler/zipkin_receiver.go @@ -56,7 +56,7 @@ func startZipkinReceiver( cfg component.Config, nextConsumer consumer.Traces) (receiver.Traces, error), ) (receiver.Traces, error) { receiverConfig := zipkinFactory.CreateDefaultConfig().(*zipkinreceiver.Config) - receiverConfig.ServerConfig = options.Zipkin + receiverConfig.ServerConfig = options.Zipkin.ServerConfig receiverSettings := receiver.Settings{ TelemetrySettings: component.TelemetrySettings{ Logger: logger, diff --git a/cmd/collector/app/server/grpc.go b/cmd/collector/app/server/grpc.go index 17301ac184c..e79791c60d0 100644 --- a/cmd/collector/app/server/grpc.go +++ b/cmd/collector/app/server/grpc.go @@ -15,10 +15,8 @@ import ( "go.opentelemetry.io/otel/metric/noop" "go.uber.org/zap" "google.golang.org/grpc" - "google.golang.org/grpc/credentials" "google.golang.org/grpc/health" "google.golang.org/grpc/health/grpc_health_v1" - "google.golang.org/grpc/keepalive" "google.golang.org/grpc/reflection" "github.com/jaegertracing/jaeger/cmd/collector/app/handler" @@ -44,27 +42,6 @@ func StartGRPCServer(params *GRPCServerParams) (*grpc.Server, error) { var server *grpc.Server var grpcOpts []configgrpc.ToServerOption - if params.MaxRecvMsgSizeMiB > 0 { - grpcOpts = append(grpcOpts, configgrpc.WithGrpcServerOption(grpc.MaxRecvMsgSize(params.MaxRecvMsgSizeMiB))) - } - if params.Keepalive != nil { - grpcOpts = append(grpcOpts, configgrpc.WithGrpcServerOption(grpc.KeepaliveParams(keepalive.ServerParameters{ - MaxConnectionAge: params.Keepalive.ServerParameters.MaxConnectionAge, - MaxConnectionAgeGrace: params.Keepalive.ServerParameters.MaxConnectionAgeGrace, - }))) - } - - if params.TLSSetting != nil { - // user requested a server with TLS, setup creds - tlsCfg, err := params.TLSSetting.LoadTLSConfig(context.Background()) - if err != nil { - return nil, err - } - - creds := credentials.NewTLS(tlsCfg) - grpcOpts = append(grpcOpts, configgrpc.WithGrpcServerOption(grpc.Creds(creds))) - } - server, err := params.ToServer(context.Background(), nil, component.TelemetrySettings{ Logger: params.Logger, LeveledMeterProvider: func(_ configtelemetry.Level) metric.MeterProvider { @@ -75,6 +52,7 @@ func StartGRPCServer(params *GRPCServerParams) (*grpc.Server, error) { return nil, err } reflection.Register(server) + listener, err := params.NetAddr.Listen(context.Background()) if err != nil { return nil, fmt.Errorf("failed to listen on gRPC port: %w", err) diff --git a/cmd/collector/app/server/http.go b/cmd/collector/app/server/http.go index 28f63b4387e..eaa22ca8b54 100644 --- a/cmd/collector/app/server/http.go +++ b/cmd/collector/app/server/http.go @@ -39,7 +39,10 @@ type HTTPServerParams struct { // StartHTTPServer based on the given parameters func StartHTTPServer(params *HTTPServerParams) (*http.Server, error) { params.Logger.Info("Starting jaeger-collector HTTP server", zap.String("http host-port", params.Endpoint)) - + listener, err := params.ToListener(context.Background()) + if err != nil { + return nil, err + } errorLog, _ := zap.NewStdLogAt(params.Logger, zapcore.ErrorLevel) server, err := params.ToServer(context.Background(), nil, component.TelemetrySettings{ Logger: params.Logger, @@ -47,12 +50,8 @@ func StartHTTPServer(params *HTTPServerParams) (*http.Server, error) { return noop.NewMeterProvider() }, }, nil) - server.ErrorLog = errorLog - if err != nil { - return nil, err - } - listener, err := params.ToListener(context.Background()) + server.ErrorLog = errorLog if err != nil { return nil, err } @@ -83,7 +82,7 @@ func serveHTTP(server *http.Server, listener net.Listener, params *HTTPServerPar go func() { var err error if params.TLSSetting != nil { - err = server.ServeTLS(listener,params.TLSSetting.CertFile, params.TLSSetting.KeyFile) + err = server.ServeTLS(listener, params.TLSSetting.CertFile, params.TLSSetting.KeyFile) } else { err = server.Serve(listener) } diff --git a/cmd/collector/app/server/http_test.go b/cmd/collector/app/server/http_test.go index 9978e856db8..b85f63d024c 100644 --- a/cmd/collector/app/server/http_test.go +++ b/cmd/collector/app/server/http_test.go @@ -9,13 +9,13 @@ import ( "fmt" "net" "net/http" - "net/http/httptest" "strconv" "testing" "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/confighttp" "go.uber.org/zap" @@ -52,7 +52,7 @@ func TestCreateTLSHTTPServerError(t *testing.T) { params := &HTTPServerParams{ ServerConfig: confighttp.ServerConfig{ - Endpoint: fmt.Sprintf(":%d", ports.CollectorHTTP), + Endpoint: ":0", TLSSetting: tlsCfg.ToOtelServerConfig(), }, HealthCheck: healthcheck.New(), @@ -74,11 +74,15 @@ func TestSpanCollectorHTTP(t *testing.T) { Logger: logger, } - server := httptest.NewServer(nil) + server, _ := params.ToServer(context.Background(), nil, component.TelemetrySettings{}, nil) + listener, _ := params.ToListener(context.Background()) - serveHTTP(server.Config, server.Listener, params) - - response, err := http.Post(server.URL, "", nil) + serveHTTP(server, listener, params) + addr := listener.Addr().String() + host, port, err := net.SplitHostPort(addr) + require.NoError(t, err) + url := fmt.Sprintf("http://%s:%s", host, port) + response, err := http.Post(url, "", nil) require.NoError(t, err) assert.NotNil(t, response) defer response.Body.Close() From b63dcd56967add5dbddae7beb32096c3559bcd98 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Fri, 6 Dec 2024 08:29:03 +0530 Subject: [PATCH 09/18] reviewed changes Signed-off-by: chahatsagarmain --- cmd/collector/app/collector.go | 1 - cmd/collector/app/collector_test.go | 69 +++++++++++++------ cmd/collector/app/flags/flags.go | 34 +++++---- cmd/collector/app/handler/otlp_receiver.go | 2 - .../app/handler/otlp_receiver_test.go | 24 ++++--- .../app/handler/zipkin_receiver_tls_test.go | 2 - cmd/collector/app/server/grpc.go | 1 - cmd/collector/app/server/http.go | 7 +- cmd/collector/app/server/http_test.go | 17 ++--- 9 files changed, 90 insertions(+), 67 deletions(-) diff --git a/cmd/collector/app/collector.go b/cmd/collector/app/collector.go index d867f348db5..4125ad790c3 100644 --- a/cmd/collector/app/collector.go +++ b/cmd/collector/app/collector.go @@ -132,7 +132,6 @@ func (c *Collector) Start(options *flags.CollectorOptions) error { } if options.OTLP.Enabled { - fmt.Printf("otlp enabled \n") otlpReceiver, err := handler.StartOTLPReceiver(options, c.logger, c.spanProcessor, c.tenancyMgr) if err != nil { return fmt.Errorf("could not start OTLP receiver: %w", err) diff --git a/cmd/collector/app/collector_test.go b/cmd/collector/app/collector_test.go index dc14d8f7e23..2b86b7756af 100644 --- a/cmd/collector/app/collector_test.go +++ b/cmd/collector/app/collector_test.go @@ -32,33 +32,60 @@ import ( var _ (io.Closer) = (*Collector)(nil) func optionsForEphemeralPorts() *flags.CollectorOptions { - collectorOpts := &flags.CollectorOptions{} - collectorOpts.GRPC.NetAddr.Endpoint = ":0" - collectorOpts.GRPC.Keepalive = &configgrpc.KeepaliveServerConfig{ - ServerParameters: &configgrpc.KeepaliveServerParameters{ - MaxConnectionIdle: 10, + collectorOpts := &flags.CollectorOptions{ + HTTP: confighttp.ServerConfig{ + Endpoint: ":0", + TLSSetting: &configtls.ServerConfig{}, }, + GRPC: configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: ":0", + Transport: confignet.TransportTypeTCP, + }, + Keepalive: &configgrpc.KeepaliveServerConfig{ + ServerParameters: &configgrpc.KeepaliveServerParameters{ + MaxConnectionIdle: 10, + }, + }, + }, + OTLP: struct { + Enabled bool + GRPC configgrpc.ServerConfig + HTTP confighttp.ServerConfig + }{ + Enabled: true, + HTTP: confighttp.ServerConfig{ + Endpoint: ":0", + TLSSetting: &configtls.ServerConfig{}, + }, + GRPC: configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: ":0", + Transport: confignet.TransportTypeTCP, + }, + Keepalive: &configgrpc.KeepaliveServerConfig{ + ServerParameters: &configgrpc.KeepaliveServerParameters{ + MaxConnectionIdle: 10, + }, + }, + }, + }, + Zipkin: struct { + confighttp.ServerConfig + KeepAlive bool + }{ + ServerConfig: confighttp.ServerConfig{ + Endpoint: ":0", + }, + }, + Tenancy: tenancy.Options{}, } - collectorOpts.GRPC.NetAddr.Transport = confignet.TransportTypeTCP - collectorOpts.HTTP = confighttp.ServerConfig{ - Endpoint: ":0", - TLSSetting: &configtls.ServerConfig{}, - } - collectorOpts.OTLP.Enabled = true - collectorOpts.OTLP.GRPC.NetAddr.Endpoint = ":0" - collectorOpts.OTLP.GRPC.Keepalive = &configgrpc.KeepaliveServerConfig{ + collectorOpts.GRPC.NetAddr.Endpoint = ":0" + collectorOpts.GRPC.Keepalive = &configgrpc.KeepaliveServerConfig{ ServerParameters: &configgrpc.KeepaliveServerParameters{ MaxConnectionIdle: 10, }, } - collectorOpts.OTLP.GRPC.NetAddr.Transport = confignet.TransportTypeTCP - - collectorOpts.OTLP.HTTP = confighttp.ServerConfig{ - Endpoint: ":0", - TLSSetting: &configtls.ServerConfig{}, - } - collectorOpts.Zipkin.Endpoint = ":0" - collectorOpts.Tenancy = tenancy.Options{} return collectorOpts } diff --git a/cmd/collector/app/flags/flags.go b/cmd/collector/app/flags/flags.go index 9e5d1050563..14efe28d60f 100644 --- a/cmd/collector/app/flags/flags.go +++ b/cmd/collector/app/flags/flags.go @@ -87,14 +87,10 @@ var otlpServerFlagsCfg = struct { }, } -var zipkinServerFlagsCfg = struct { - HTTP serverFlagsConfig -}{ - HTTP: serverFlagsConfig{ - prefix: "collector.zipkin", - tls: tlscfg.ServerFlagsConfig{ - Prefix: "collector.zipkin", - }, +var zipkinServerFlagsCfg = serverFlagsConfig{ + prefix: "collector.zipkin", + tls: tlscfg.ServerFlagsConfig{ + Prefix: "collector.zipkin", }, } @@ -127,7 +123,7 @@ type CollectorOptions struct { // Zipkin section defines options for Zipkin HTTP server Zipkin struct { confighttp.ServerConfig - KeepAlive bool `mapstructure:"keep-alive"` // Add keepalive setting + KeepAlive bool } // CollectorTags is the string representing collector tags to append to each and every span CollectorTags map[string]string @@ -160,7 +156,7 @@ func AddFlags(flagSet *flag.FlagSet) { flagSet.String(flagZipkinHTTPHostPort, "", "The host:port (e.g. 127.0.0.1:9411 or :9411) of the collector's Zipkin server (disabled by default)") flagSet.Bool(flagZipkinKeepAliveEnabled, true, "KeepAlive configures allow Keep-Alive for Zipkin HTTP server (enabled by default)") - zipkinServerFlagsCfg.HTTP.tls.AddFlags(flagSet) + zipkinServerFlagsCfg.tls.AddFlags(flagSet) corsZipkinFlags.AddFlags(flagSet) tenancy.AddFlags(flagSet) @@ -194,7 +190,7 @@ func addGRPCFlags(flagSet *flag.FlagSet, cfg serverFlagsConfig, defaultHostPort cfg.tls.AddFlags(flagSet) } -func initHTTPFromViper(v *viper.Viper, _ *zap.Logger, opts *confighttp.ServerConfig, cfg serverFlagsConfig) error { +func initHTTPFromViper(v *viper.Viper, _ *zap.Logger, opts *confighttp.ServerConfig, corsOpts *corscfg.Options, cfg serverFlagsConfig) error { tlsOpts, err := cfg.tls.InitFromViper(v) if err != nil { return fmt.Errorf("failed to parse HTTP TLS options: %w", err) @@ -204,6 +200,9 @@ func initHTTPFromViper(v *viper.Viper, _ *zap.Logger, opts *confighttp.ServerCon opts.IdleTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPIdleTimeout) opts.ReadTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadTimeout) opts.ReadHeaderTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadHeaderTimeout) + if corsOpts != nil { + opts.CORS = corsOpts.ToOTELCorsConfig() + } return nil } @@ -235,7 +234,7 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) cOpts.SpanSizeMetricsEnabled = v.GetBool(flagSpanSizeMetricsEnabled) cOpts.Tenancy = tenancy.InitFromViper(v) - if err := initHTTPFromViper(v, logger, &cOpts.HTTP, httpServerFlagsCfg); err != nil { + if err := initHTTPFromViper(v, logger, &cOpts.HTTP, nil, httpServerFlagsCfg); err != nil { return cOpts, fmt.Errorf("failed to parse HTTP server options: %w", err) } @@ -244,22 +243,21 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) } cOpts.OTLP.Enabled = v.GetBool(flagCollectorOTLPEnabled) - if err := initHTTPFromViper(v, logger, &cOpts.OTLP.HTTP, otlpServerFlagsCfg.HTTP); err != nil { + corsOpts := corsOTLPFlags.InitFromViper(v) + + if err := initHTTPFromViper(v, logger, &cOpts.OTLP.HTTP, &corsOpts, otlpServerFlagsCfg.HTTP); err != nil { return cOpts, fmt.Errorf("failed to parse OTLP/HTTP server options: %w", err) } - corsOpts := corsOTLPFlags.InitFromViper(v) - cOpts.OTLP.HTTP.CORS = corsOpts.ToOTELCorsConfig() if err := initGRPCFromViper(v, logger, &cOpts.OTLP.GRPC, otlpServerFlagsCfg.GRPC); err != nil { return cOpts, fmt.Errorf("failed to parse OTLP/gRPC server options: %w", err) } cOpts.Zipkin.KeepAlive = v.GetBool(flagZipkinKeepAliveEnabled) + corsOpts = corsZipkinFlags.InitFromViper(v) - if err := initHTTPFromViper(v, logger, &cOpts.Zipkin.ServerConfig, zipkinServerFlagsCfg.HTTP); err != nil { + if err := initHTTPFromViper(v, logger, &cOpts.Zipkin.ServerConfig, &corsOpts, zipkinServerFlagsCfg); err != nil { return cOpts, fmt.Errorf("failed to parse Zipkin server options: %w", err) } - corsOpts = corsZipkinFlags.InitFromViper(v) - cOpts.Zipkin.CORS = corsOpts.ToOTELCorsConfig() return cOpts, nil } diff --git a/cmd/collector/app/handler/otlp_receiver.go b/cmd/collector/app/handler/otlp_receiver.go index afa7ff8b96d..ea31d5ce400 100644 --- a/cmd/collector/app/handler/otlp_receiver.go +++ b/cmd/collector/app/handler/otlp_receiver.go @@ -60,8 +60,6 @@ func startOTLPReceiver( otlpReceiverConfig := otlpFactory.CreateDefaultConfig().(*otlpreceiver.Config) otlpReceiverConfig.GRPC = &options.OTLP.GRPC otlpReceiverConfig.HTTP.ServerConfig = &options.OTLP.HTTP - fmt.Printf("OTLP HTTP Endpoint: %s\n", otlpReceiverConfig.HTTP.ServerConfig.Endpoint) - fmt.Printf("OTLP GRPC Endpoint: %s\n", otlpReceiverConfig.GRPC.NetAddr.Endpoint) statusReporter := func(ev *componentstatus.Event) { // TODO this could be wired into changing healthcheck.HealthCheck logger.Info("OTLP receiver status change", zap.Stringer("status", ev.Status())) diff --git a/cmd/collector/app/handler/otlp_receiver_test.go b/cmd/collector/app/handler/otlp_receiver_test.go index 8cb046ffba0..c0f80de60f2 100644 --- a/cmd/collector/app/handler/otlp_receiver_test.go +++ b/cmd/collector/app/handler/otlp_receiver_test.go @@ -26,16 +26,24 @@ import ( ) func optionsWithPorts(port string) *flags.CollectorOptions { - opts := &flags.CollectorOptions{} - opts.OTLP.HTTP = confighttp.ServerConfig{ - Endpoint: port, - } - opts.OTLP.GRPC = configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: port, + opts := &flags.CollectorOptions{ + OTLP: struct { + Enabled bool + GRPC configgrpc.ServerConfig + HTTP confighttp.ServerConfig + }{ + Enabled: true, + HTTP: confighttp.ServerConfig{ + Endpoint: port, + }, + GRPC: configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: port, + Transport: confignet.TransportTypeTCP, + }, + }, }, } - opts.OTLP.GRPC.NetAddr.Transport = confignet.TransportTypeTCP return opts } diff --git a/cmd/collector/app/handler/zipkin_receiver_tls_test.go b/cmd/collector/app/handler/zipkin_receiver_tls_test.go index f0f8e80a07e..6edc975d55f 100644 --- a/cmd/collector/app/handler/zipkin_receiver_tls_test.go +++ b/cmd/collector/app/handler/zipkin_receiver_tls_test.go @@ -194,8 +194,6 @@ func TestSpanCollectorZipkinTLS(t *testing.T) { } response, requestError := client.Post(fmt.Sprintf("https://localhost:%d", ports.CollectorZipkin), "", nil) - fmt.Print(response) - fmt.Print(requestError) if test.expectZipkinClientErr { require.Error(t, requestError) } else { diff --git a/cmd/collector/app/server/grpc.go b/cmd/collector/app/server/grpc.go index e79791c60d0..2f42c137342 100644 --- a/cmd/collector/app/server/grpc.go +++ b/cmd/collector/app/server/grpc.go @@ -41,7 +41,6 @@ type GRPCServerParams struct { func StartGRPCServer(params *GRPCServerParams) (*grpc.Server, error) { var server *grpc.Server var grpcOpts []configgrpc.ToServerOption - server, err := params.ToServer(context.Background(), nil, component.TelemetrySettings{ Logger: params.Logger, LeveledMeterProvider: func(_ configtelemetry.Level) metric.MeterProvider { diff --git a/cmd/collector/app/server/http.go b/cmd/collector/app/server/http.go index eaa22ca8b54..addf47aaa7b 100644 --- a/cmd/collector/app/server/http.go +++ b/cmd/collector/app/server/http.go @@ -80,12 +80,7 @@ func serveHTTP(server *http.Server, listener net.Listener, params *HTTPServerPar recoveryHandler := recoveryhandler.NewRecoveryHandler(params.Logger, true) server.Handler = httpmetrics.Wrap(recoveryHandler(r), params.MetricsFactory, params.Logger) go func() { - var err error - if params.TLSSetting != nil { - err = server.ServeTLS(listener, params.TLSSetting.CertFile, params.TLSSetting.KeyFile) - } else { - err = server.Serve(listener) - } + err := server.Serve(listener) if err != nil { if err != http.ErrServerClosed { params.Logger.Error("Could not start HTTP collector", zap.Error(err)) diff --git a/cmd/collector/app/server/http_test.go b/cmd/collector/app/server/http_test.go index b85f63d024c..24c00b94e54 100644 --- a/cmd/collector/app/server/http_test.go +++ b/cmd/collector/app/server/http_test.go @@ -17,6 +17,7 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/confighttp" + "go.opentelemetry.io/collector/config/configtls" "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/collector/app/handler" @@ -43,17 +44,17 @@ func TestFailToListenHTTP(t *testing.T) { func TestCreateTLSHTTPServerError(t *testing.T) { logger, _ := zap.NewDevelopment() - tlsCfg := tlscfg.Options{ - Enabled: true, - CertPath: "invalid/path", - KeyPath: "invalid/path", - ClientCAPath: "invalid/path", - } params := &HTTPServerParams{ ServerConfig: confighttp.ServerConfig{ - Endpoint: ":0", - TLSSetting: tlsCfg.ToOtelServerConfig(), + Endpoint: ":0", + TLSSetting: &configtls.ServerConfig{ + Config: configtls.Config{ + CertFile: "invalid/path", + KeyFile: "invalid/path", + CAFile: "invalid/path", + }, + }, }, HealthCheck: healthcheck.New(), Logger: logger, From 66d50532206b112b46d19a9acc7a3930dd910735 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Fri, 6 Dec 2024 08:31:30 +0530 Subject: [PATCH 10/18] unecessary loc Signed-off-by: chahatsagarmain --- cmd/collector/app/collector_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/cmd/collector/app/collector_test.go b/cmd/collector/app/collector_test.go index 2b86b7756af..923d4359ff2 100644 --- a/cmd/collector/app/collector_test.go +++ b/cmd/collector/app/collector_test.go @@ -80,12 +80,6 @@ func optionsForEphemeralPorts() *flags.CollectorOptions { }, Tenancy: tenancy.Options{}, } - collectorOpts.GRPC.NetAddr.Endpoint = ":0" - collectorOpts.GRPC.Keepalive = &configgrpc.KeepaliveServerConfig{ - ServerParameters: &configgrpc.KeepaliveServerParameters{ - MaxConnectionIdle: 10, - }, - } return collectorOpts } From 8b074ac2c0297a5c41be9d0278cdb9e57a659280 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Sat, 7 Dec 2024 16:59:50 +0530 Subject: [PATCH 11/18] add transport type Signed-off-by: chahatsagarmain --- cmd/collector/app/server/grpc.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/collector/app/server/grpc.go b/cmd/collector/app/server/grpc.go index 2f42c137342..0f5d8b56251 100644 --- a/cmd/collector/app/server/grpc.go +++ b/cmd/collector/app/server/grpc.go @@ -10,6 +10,7 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configgrpc" + "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/config/configtelemetry" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/noop" @@ -41,6 +42,8 @@ type GRPCServerParams struct { func StartGRPCServer(params *GRPCServerParams) (*grpc.Server, error) { var server *grpc.Server var grpcOpts []configgrpc.ToServerOption + // explicitly setting tranport type + params.NetAddr.Transport = confignet.TransportTypeTCP server, err := params.ToServer(context.Background(), nil, component.TelemetrySettings{ Logger: params.Logger, LeveledMeterProvider: func(_ configtelemetry.Level) metric.MeterProvider { From db9367efbf12e808b28f9f226751924698ce0333 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Sat, 7 Dec 2024 18:10:24 +0530 Subject: [PATCH 12/18] added transport type to otlp reciever Signed-off-by: chahatsagarmain --- cmd/collector/app/handler/otlp_receiver.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/collector/app/handler/otlp_receiver.go b/cmd/collector/app/handler/otlp_receiver.go index ea31d5ce400..1bb2b2a96d9 100644 --- a/cmd/collector/app/handler/otlp_receiver.go +++ b/cmd/collector/app/handler/otlp_receiver.go @@ -10,6 +10,7 @@ import ( otlp2jaeger "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/component/componentstatus" + "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/config/configtelemetry" "go.opentelemetry.io/collector/consumer" "go.opentelemetry.io/collector/extension" @@ -59,6 +60,7 @@ func startOTLPReceiver( ) (receiver.Traces, error) { otlpReceiverConfig := otlpFactory.CreateDefaultConfig().(*otlpreceiver.Config) otlpReceiverConfig.GRPC = &options.OTLP.GRPC + otlpReceiverConfig.GRPC.NetAddr.Transport = confignet.TransportTypeTCP otlpReceiverConfig.HTTP.ServerConfig = &options.OTLP.HTTP statusReporter := func(ev *componentstatus.Event) { // TODO this could be wired into changing healthcheck.HealthCheck From f36c84d3998a6f5b36d39f4b1209b669c4f2c771 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Sun, 8 Dec 2024 01:09:03 +0530 Subject: [PATCH 13/18] resolved comments Signed-off-by: chahatsagarmain --- cmd/collector/app/collector.go | 1 - cmd/collector/app/collector_test.go | 3 - cmd/collector/app/flags/flags.go | 8 +- .../app/handler/zipkin_receiver_tls_test.go | 144 ++++++++---------- cmd/collector/app/server/grpc.go | 1 - pkg/config/corscfg/flags.go | 7 +- pkg/config/corscfg/flags_test.go | 5 +- pkg/config/corscfg/options.go | 18 --- 8 files changed, 78 insertions(+), 109 deletions(-) delete mode 100644 pkg/config/corscfg/options.go diff --git a/cmd/collector/app/collector.go b/cmd/collector/app/collector.go index 4125ad790c3..c2c5b4a9272 100644 --- a/cmd/collector/app/collector.go +++ b/cmd/collector/app/collector.go @@ -123,7 +123,6 @@ func (c *Collector) Start(options *flags.CollectorOptions) error { if options.Zipkin.Endpoint == "" { c.logger.Info("Not listening for Zipkin HTTP traffic, port not configured") } else { - fmt.Printf("%v zipkin\n", options.Zipkin.Endpoint) zipkinReceiver, err := handler.StartZipkinReceiver(options, c.logger, c.spanProcessor, c.tenancyMgr) if err != nil { return fmt.Errorf("could not start Zipkin receiver: %w", err) diff --git a/cmd/collector/app/collector_test.go b/cmd/collector/app/collector_test.go index 923d4359ff2..4211f88cbbe 100644 --- a/cmd/collector/app/collector_test.go +++ b/cmd/collector/app/collector_test.go @@ -6,7 +6,6 @@ package app import ( "context" "expvar" - "fmt" "io" "sync/atomic" "testing" @@ -124,8 +123,6 @@ func TestNewCollector(t *testing.T) { }) collectorOpts := optionsForEphemeralPorts() - fmt.Printf("Collector Options: %+v\n", collectorOpts.OTLP.GRPC) - fmt.Print(collectorOpts.OTLP.HTTP.Endpoint) require.NoError(t, c.Start(collectorOpts)) assert.NotNil(t, c.SpanHandlers()) require.NoError(t, c.Close()) diff --git a/cmd/collector/app/flags/flags.go b/cmd/collector/app/flags/flags.go index 14efe28d60f..6f6ce454b34 100644 --- a/cmd/collector/app/flags/flags.go +++ b/cmd/collector/app/flags/flags.go @@ -190,7 +190,7 @@ func addGRPCFlags(flagSet *flag.FlagSet, cfg serverFlagsConfig, defaultHostPort cfg.tls.AddFlags(flagSet) } -func initHTTPFromViper(v *viper.Viper, _ *zap.Logger, opts *confighttp.ServerConfig, corsOpts *corscfg.Options, cfg serverFlagsConfig) error { +func initHTTPFromViper(v *viper.Viper, _ *zap.Logger, opts *confighttp.ServerConfig, corsOpts *confighttp.CORSConfig, cfg serverFlagsConfig) error { tlsOpts, err := cfg.tls.InitFromViper(v) if err != nil { return fmt.Errorf("failed to parse HTTP TLS options: %w", err) @@ -201,7 +201,7 @@ func initHTTPFromViper(v *viper.Viper, _ *zap.Logger, opts *confighttp.ServerCon opts.ReadTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadTimeout) opts.ReadHeaderTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadHeaderTimeout) if corsOpts != nil { - opts.CORS = corsOpts.ToOTELCorsConfig() + opts.CORS = corsOpts } return nil @@ -245,7 +245,7 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) cOpts.OTLP.Enabled = v.GetBool(flagCollectorOTLPEnabled) corsOpts := corsOTLPFlags.InitFromViper(v) - if err := initHTTPFromViper(v, logger, &cOpts.OTLP.HTTP, &corsOpts, otlpServerFlagsCfg.HTTP); err != nil { + if err := initHTTPFromViper(v, logger, &cOpts.OTLP.HTTP, corsOpts, otlpServerFlagsCfg.HTTP); err != nil { return cOpts, fmt.Errorf("failed to parse OTLP/HTTP server options: %w", err) } if err := initGRPCFromViper(v, logger, &cOpts.OTLP.GRPC, otlpServerFlagsCfg.GRPC); err != nil { @@ -255,7 +255,7 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) cOpts.Zipkin.KeepAlive = v.GetBool(flagZipkinKeepAliveEnabled) corsOpts = corsZipkinFlags.InitFromViper(v) - if err := initHTTPFromViper(v, logger, &cOpts.Zipkin.ServerConfig, &corsOpts, zipkinServerFlagsCfg); err != nil { + if err := initHTTPFromViper(v, logger, &cOpts.Zipkin.ServerConfig, corsOpts, zipkinServerFlagsCfg); err != nil { return cOpts, fmt.Errorf("failed to parse Zipkin server options: %w", err) } diff --git a/cmd/collector/app/handler/zipkin_receiver_tls_test.go b/cmd/collector/app/handler/zipkin_receiver_tls_test.go index 6edc975d55f..3c8252f273d 100644 --- a/cmd/collector/app/handler/zipkin_receiver_tls_test.go +++ b/cmd/collector/app/handler/zipkin_receiver_tls_test.go @@ -13,9 +13,9 @@ import ( "time" "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/config/configtls" "github.com/jaegertracing/jaeger/cmd/collector/app/flags" - "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/tenancy" "github.com/jaegertracing/jaeger/pkg/testutils" "github.com/jaegertracing/jaeger/ports" @@ -25,21 +25,21 @@ func TestSpanCollectorZipkinTLS(t *testing.T) { const testCertKeyLocation = "../../../../pkg/config/tlscfg/testdata" testCases := []struct { name string - serverTLS tlscfg.Options - clientTLS tlscfg.Options + serverTLS configtls.ServerConfig + clientTLS configtls.ClientConfig expectTLSClientErr bool expectZipkinClientErr bool expectServerFail bool }{ { name: "should fail with TLS client to untrusted TLS server", - serverTLS: tlscfg.Options{ - Enabled: true, - CertPath: testCertKeyLocation + "/example-server-cert.pem", - KeyPath: testCertKeyLocation + "/example-server-key.pem", + serverTLS: configtls.ServerConfig{ + Config: configtls.Config{ + CertFile: testCertKeyLocation + "/example-server-cert.pem", + KeyFile: testCertKeyLocation + "/example-server-key.pem", + }, }, - clientTLS: tlscfg.Options{ - Enabled: true, + clientTLS: configtls.ClientConfig{ ServerName: "example.com", }, expectTLSClientErr: true, @@ -48,14 +48,16 @@ func TestSpanCollectorZipkinTLS(t *testing.T) { }, { name: "should fail with TLS client to trusted TLS server with incorrect hostname", - serverTLS: tlscfg.Options{ - Enabled: true, - CertPath: testCertKeyLocation + "/example-server-cert.pem", - KeyPath: testCertKeyLocation + "/example-server-key.pem", + serverTLS: configtls.ServerConfig{ + Config: configtls.Config{ + CertFile: testCertKeyLocation + "/example-server-cert.pem", + KeyFile: testCertKeyLocation + "/example-server-key.pem", + }, }, - clientTLS: tlscfg.Options{ - Enabled: true, - CAPath: testCertKeyLocation + "/example-CA-cert.pem", + clientTLS: configtls.ClientConfig{ + Config: configtls.Config{ + CAFile: testCertKeyLocation + "/example-CA-cert.pem", + }, ServerName: "nonEmpty", }, expectTLSClientErr: true, @@ -64,14 +66,16 @@ func TestSpanCollectorZipkinTLS(t *testing.T) { }, { name: "should pass with TLS client to trusted TLS server with correct hostname", - serverTLS: tlscfg.Options{ - Enabled: true, - CertPath: testCertKeyLocation + "/example-server-cert.pem", - KeyPath: testCertKeyLocation + "/example-server-key.pem", + serverTLS: configtls.ServerConfig{ + Config: configtls.Config{ + CertFile: testCertKeyLocation + "/example-server-cert.pem", + KeyFile: testCertKeyLocation + "/example-server-key.pem", + }, }, - clientTLS: tlscfg.Options{ - Enabled: true, - CAPath: testCertKeyLocation + "/example-CA-cert.pem", + clientTLS: configtls.ClientConfig{ + Config: configtls.Config{ + CAFile: testCertKeyLocation + "/example-CA-cert.pem", + }, ServerName: "example.com", }, expectTLSClientErr: false, @@ -80,78 +84,64 @@ func TestSpanCollectorZipkinTLS(t *testing.T) { }, { name: "should fail with TLS client without cert to trusted TLS server requiring cert", - serverTLS: tlscfg.Options{ - Enabled: true, - CertPath: testCertKeyLocation + "/example-server-cert.pem", - KeyPath: testCertKeyLocation + "/example-server-key.pem", - ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", + serverTLS: configtls.ServerConfig{ + ClientCAFile: testCertKeyLocation + "/example-CA-cert.pem", + Config: configtls.Config{ + CertFile: testCertKeyLocation + "/example-server-cert.pem", + KeyFile: testCertKeyLocation + "/example-server-key.pem", + }, }, - clientTLS: tlscfg.Options{ - Enabled: true, - CAPath: testCertKeyLocation + "/example-CA-cert.pem", + clientTLS: configtls.ClientConfig{ + Config: configtls.Config{ + CAFile: testCertKeyLocation + "/example-CA-cert.pem", + }, ServerName: "example.com", }, expectTLSClientErr: false, - expectServerFail: false, expectZipkinClientErr: true, + expectServerFail: false, }, { name: "should pass with TLS client with cert to trusted TLS server requiring cert", - serverTLS: tlscfg.Options{ - Enabled: true, - CertPath: testCertKeyLocation + "/example-server-cert.pem", - KeyPath: testCertKeyLocation + "/example-server-key.pem", - ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", + serverTLS: configtls.ServerConfig{ + ClientCAFile: testCertKeyLocation + "/example-CA-cert.pem", + Config: configtls.Config{ + CertFile: testCertKeyLocation + "/example-server-cert.pem", + KeyFile: testCertKeyLocation + "/example-server-key.pem", + }, }, - clientTLS: tlscfg.Options{ - Enabled: true, - CAPath: testCertKeyLocation + "/example-CA-cert.pem", + clientTLS: configtls.ClientConfig{ + Config: configtls.Config{ + CAFile: testCertKeyLocation + "/example-CA-cert.pem", + CertFile: testCertKeyLocation + "/example-client-cert.pem", + KeyFile: testCertKeyLocation + "/example-client-key.pem", + }, ServerName: "example.com", - CertPath: testCertKeyLocation + "/example-client-cert.pem", - KeyPath: testCertKeyLocation + "/example-client-key.pem", }, expectTLSClientErr: false, - expectServerFail: false, expectZipkinClientErr: false, + expectServerFail: false, }, { - name: "should fail with TLS client without cert to trusted TLS server requiring cert from a different CA", - serverTLS: tlscfg.Options{ - Enabled: true, - CertPath: testCertKeyLocation + "/example-server-cert.pem", - KeyPath: testCertKeyLocation + "/example-server-key.pem", - ClientCAPath: testCertKeyLocation + "/wrong-CA-cert.pem", // NB: wrong CA + name: "should fail with TLS client without cert to trusted TLS server requiring cert from different CA", + serverTLS: configtls.ServerConfig{ + ClientCAFile: testCertKeyLocation + "/wrong-CA-cert.pem", + Config: configtls.Config{ + CertFile: testCertKeyLocation + "/example-server-cert.pem", + KeyFile: testCertKeyLocation + "/example-server-key.pem", + }, }, - clientTLS: tlscfg.Options{ - Enabled: true, - CAPath: testCertKeyLocation + "/example-CA-cert.pem", + clientTLS: configtls.ClientConfig{ + Config: configtls.Config{ + CAFile: testCertKeyLocation + "/example-CA-cert.pem", + CertFile: testCertKeyLocation + "/example-client-cert.pem", + KeyFile: testCertKeyLocation + "/example-client-key.pem", + }, ServerName: "example.com", - CertPath: testCertKeyLocation + "/example-client-cert.pem", - KeyPath: testCertKeyLocation + "/example-client-key.pem", }, expectTLSClientErr: false, - expectServerFail: false, expectZipkinClientErr: true, - }, - { - name: "should fail with TLS client with cert to trusted TLS server with incorrect TLS min", - serverTLS: tlscfg.Options{ - Enabled: true, - CertPath: testCertKeyLocation + "/example-server-cert.pem", - KeyPath: testCertKeyLocation + "/example-server-key.pem", - ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", - MinVersion: "1.5", - }, - clientTLS: tlscfg.Options{ - Enabled: true, - CAPath: testCertKeyLocation + "/example-CA-cert.pem", - ServerName: "example.com", - CertPath: testCertKeyLocation + "/example-client-cert.pem", - KeyPath: testCertKeyLocation + "/example-client-key.pem", - }, - expectTLSClientErr: true, - expectServerFail: true, - expectZipkinClientErr: false, + expectServerFail: false, }, } @@ -163,7 +153,7 @@ func TestSpanCollectorZipkinTLS(t *testing.T) { opts := &flags.CollectorOptions{} opts.Zipkin.Endpoint = ports.PortToHostPort(ports.CollectorZipkin) - opts.Zipkin.TLSSetting = test.serverTLS.ToOtelServerConfig() + opts.Zipkin.TLSSetting = &test.serverTLS server, err := StartZipkinReceiver(opts, logger, spanProcessor, tm) if test.expectServerFail { @@ -175,7 +165,7 @@ func TestSpanCollectorZipkinTLS(t *testing.T) { require.NoError(t, server.Shutdown(context.Background())) }() - clientTLSCfg, err0 := test.clientTLS.ToOtelClientConfig().LoadTLSConfig(context.Background()) + clientTLSCfg, err0 := test.clientTLS.LoadTLSConfig(context.Background()) require.NoError(t, err0) dialer := &net.Dialer{Timeout: 2 * time.Second} conn, clientError := tls.DialWithDialer(dialer, "tcp", fmt.Sprintf("localhost:%d", ports.CollectorZipkin), clientTLSCfg) diff --git a/cmd/collector/app/server/grpc.go b/cmd/collector/app/server/grpc.go index 0f5d8b56251..156d48fe3dd 100644 --- a/cmd/collector/app/server/grpc.go +++ b/cmd/collector/app/server/grpc.go @@ -42,7 +42,6 @@ type GRPCServerParams struct { func StartGRPCServer(params *GRPCServerParams) (*grpc.Server, error) { var server *grpc.Server var grpcOpts []configgrpc.ToServerOption - // explicitly setting tranport type params.NetAddr.Transport = confignet.TransportTypeTCP server, err := params.ToServer(context.Background(), nil, component.TelemetrySettings{ Logger: params.Logger, diff --git a/pkg/config/corscfg/flags.go b/pkg/config/corscfg/flags.go index c58b6b44bc4..892e177d7f7 100644 --- a/pkg/config/corscfg/flags.go +++ b/pkg/config/corscfg/flags.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/spf13/viper" + "go.opentelemetry.io/collector/config/confighttp" ) const ( @@ -25,8 +26,8 @@ func (c Flags) AddFlags(flags *flag.FlagSet) { flags.String(c.Prefix+corsAllowedOrigins, "", "Comma-separated CORS allowed origins. See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin") } -func (c Flags) InitFromViper(v *viper.Viper) Options { - var p Options +func (c Flags) InitFromViper(v *viper.Viper) *confighttp.CORSConfig { + var p confighttp.CORSConfig allowedHeaders := v.GetString(c.Prefix + corsAllowedHeaders) allowedOrigins := v.GetString(c.Prefix + corsAllowedOrigins) @@ -34,5 +35,5 @@ func (c Flags) InitFromViper(v *viper.Viper) Options { p.AllowedOrigins = strings.Split(strings.ReplaceAll(allowedOrigins, " ", ""), ",") p.AllowedHeaders = strings.Split(strings.ReplaceAll(allowedHeaders, " ", ""), ",") - return p + return &p } diff --git a/pkg/config/corscfg/flags_test.go b/pkg/config/corscfg/flags_test.go index 4ddb292e1fe..2ca108b6c2b 100644 --- a/pkg/config/corscfg/flags_test.go +++ b/pkg/config/corscfg/flags_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/config/confighttp" "github.com/jaegertracing/jaeger/pkg/config" "github.com/jaegertracing/jaeger/pkg/testutils" @@ -31,10 +32,10 @@ func TestCORSFlags(t *testing.T) { corsOpts := flagCfg.InitFromViper(v) fmt.Println(corsOpts) - assert.Equal(t, Options{ + assert.Equal(t, confighttp.CORSConfig{ AllowedHeaders: []string{"Content-Type", "Accept", "X-Requested-With"}, AllowedOrigins: []string{"http://example.domain.com", "http://*.domain.com"}, - }, corsOpts) + }, *corsOpts) }) } diff --git a/pkg/config/corscfg/options.go b/pkg/config/corscfg/options.go deleted file mode 100644 index 3f7979b07f8..00000000000 --- a/pkg/config/corscfg/options.go +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright (c) 2023 The Jaeger Authors. -// SPDX-License-Identifier: Apache-2.0 - -package corscfg - -import "go.opentelemetry.io/collector/config/confighttp" - -type Options struct { - AllowedOrigins []string - AllowedHeaders []string -} - -func (o *Options) ToOTELCorsConfig() *confighttp.CORSConfig { - return &confighttp.CORSConfig{ - AllowedOrigins: o.AllowedOrigins, - AllowedHeaders: o.AllowedHeaders, - } -} From 71194a5266396df0ab8bd930b8aac73e4e2d4528 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Sun, 8 Dec 2024 01:18:21 +0530 Subject: [PATCH 14/18] missing test case Signed-off-by: chahatsagarmain --- .../app/handler/zipkin_receiver_tls_test.go | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/cmd/collector/app/handler/zipkin_receiver_tls_test.go b/cmd/collector/app/handler/zipkin_receiver_tls_test.go index 3c8252f273d..108a1023150 100644 --- a/cmd/collector/app/handler/zipkin_receiver_tls_test.go +++ b/cmd/collector/app/handler/zipkin_receiver_tls_test.go @@ -143,6 +143,28 @@ func TestSpanCollectorZipkinTLS(t *testing.T) { expectZipkinClientErr: true, expectServerFail: false, }, + { + name: "should fail with TLS client with cert to trusted TLS server with incorrect TLS min", + serverTLS: configtls.ServerConfig{ + ClientCAFile: testCertKeyLocation + "/example-CA-cert.pem", + Config: configtls.Config{ + CertFile: testCertKeyLocation + "/example-server-cert.pem", + KeyFile: testCertKeyLocation + "/example-server-key.pem", + MinVersion: "1.5", + }, + }, + clientTLS: configtls.ClientConfig{ + Config: configtls.Config{ + CAFile: testCertKeyLocation + "/example-CA-cert.pem", + CertFile: testCertKeyLocation + "/example-client-cert.pem", + KeyFile: testCertKeyLocation + "/example-client-key.pem", + }, + ServerName: "example.com", + }, + expectTLSClientErr: true, + expectServerFail: true, + expectZipkinClientErr: false, + }, } for _, test := range testCases { From 06cc1d3f01ed4056da02fad583c5bf656b20791a Mon Sep 17 00:00:00 2001 From: chahat sagar <109112505+chahatsagarmain@users.noreply.github.com> Date: Mon, 9 Dec 2024 19:56:31 +0530 Subject: [PATCH 15/18] Update cmd/collector/app/server/http.go Co-authored-by: Yuri Shkuro Signed-off-by: chahat sagar <109112505+chahatsagarmain@users.noreply.github.com> --- cmd/collector/app/server/http.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/collector/app/server/http.go b/cmd/collector/app/server/http.go index addf47aaa7b..17424ecd77e 100644 --- a/cmd/collector/app/server/http.go +++ b/cmd/collector/app/server/http.go @@ -38,7 +38,7 @@ type HTTPServerParams struct { // StartHTTPServer based on the given parameters func StartHTTPServer(params *HTTPServerParams) (*http.Server, error) { - params.Logger.Info("Starting jaeger-collector HTTP server", zap.String("http host-port", params.Endpoint)) + params.Logger.Info("Starting jaeger-collector HTTP server", zap.String("host-port", params.Endpoint)) listener, err := params.ToListener(context.Background()) if err != nil { return nil, err From 6bfb708852268469b027abe4b11a61d1f7a8b7d2 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Thu, 12 Dec 2024 01:06:05 +0530 Subject: [PATCH 16/18] changes Signed-off-by: chahatsagarmain --- cmd/collector/app/server/grpc.go | 14 ++++---------- cmd/collector/app/server/http.go | 13 +++---------- pkg/telemetry/settings.go | 8 ++++++++ 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/cmd/collector/app/server/grpc.go b/cmd/collector/app/server/grpc.go index 156d48fe3dd..dbdedce6f26 100644 --- a/cmd/collector/app/server/grpc.go +++ b/cmd/collector/app/server/grpc.go @@ -8,12 +8,8 @@ import ( "fmt" "net" - "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configgrpc" "go.opentelemetry.io/collector/config/confignet" - "go.opentelemetry.io/collector/config/configtelemetry" - "go.opentelemetry.io/otel/metric" - "go.opentelemetry.io/otel/metric/noop" "go.uber.org/zap" "google.golang.org/grpc" "google.golang.org/grpc/health" @@ -23,6 +19,7 @@ import ( "github.com/jaegertracing/jaeger/cmd/collector/app/handler" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/samplingstrategy" + "github.com/jaegertracing/jaeger/pkg/telemetry" "github.com/jaegertracing/jaeger/proto-gen/api_v2" ) @@ -43,12 +40,9 @@ func StartGRPCServer(params *GRPCServerParams) (*grpc.Server, error) { var server *grpc.Server var grpcOpts []configgrpc.ToServerOption params.NetAddr.Transport = confignet.TransportTypeTCP - server, err := params.ToServer(context.Background(), nil, component.TelemetrySettings{ - Logger: params.Logger, - LeveledMeterProvider: func(_ configtelemetry.Level) metric.MeterProvider { - return noop.NewMeterProvider() - }, - }, grpcOpts...) + server, err := params.ToServer(context.Background(), nil, + telemetry.NoopSettings().ToOtelComponent(), + grpcOpts...) if err != nil { return nil, err } diff --git a/cmd/collector/app/server/http.go b/cmd/collector/app/server/http.go index addf47aaa7b..b79dc329f1c 100644 --- a/cmd/collector/app/server/http.go +++ b/cmd/collector/app/server/http.go @@ -9,11 +9,7 @@ import ( "net/http" "github.com/gorilla/mux" - "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/confighttp" - "go.opentelemetry.io/collector/config/configtelemetry" - "go.opentelemetry.io/otel/metric" - "go.opentelemetry.io/otel/metric/noop" "go.uber.org/zap" "go.uber.org/zap/zapcore" @@ -24,6 +20,7 @@ import ( "github.com/jaegertracing/jaeger/pkg/httpmetrics" "github.com/jaegertracing/jaeger/pkg/metrics" "github.com/jaegertracing/jaeger/pkg/recoveryhandler" + "github.com/jaegertracing/jaeger/pkg/telemetry" ) // HTTPServerParams to construct a new Jaeger Collector HTTP Server @@ -44,12 +41,8 @@ func StartHTTPServer(params *HTTPServerParams) (*http.Server, error) { return nil, err } errorLog, _ := zap.NewStdLogAt(params.Logger, zapcore.ErrorLevel) - server, err := params.ToServer(context.Background(), nil, component.TelemetrySettings{ - Logger: params.Logger, - LeveledMeterProvider: func(_ configtelemetry.Level) metric.MeterProvider { - return noop.NewMeterProvider() - }, - }, nil) + server, err := params.ToServer(context.Background(), nil, telemetry.NoopSettings().ToOtelComponent(), + nil) server.ErrorLog = errorLog if err != nil { diff --git a/pkg/telemetry/settings.go b/pkg/telemetry/settings.go index 1f76b38e107..5b58fd7583d 100644 --- a/pkg/telemetry/settings.go +++ b/pkg/telemetry/settings.go @@ -70,3 +70,11 @@ func FromOtelComponent(telset component.TelemetrySettings, host component.Host) Host: host, } } + +func (s Settings) ToOtelComponent() component.TelemetrySettings { + return component.TelemetrySettings{ + Logger: s.Logger, + MeterProvider: s.MeterProvider, + TracerProvider: s.TracerProvider, + } +} From 5e6901093c822c0f56c1e0ba43a843a7ef3d7311 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Thu, 12 Dec 2024 02:06:35 +0530 Subject: [PATCH 17/18] reviewed changes Signed-off-by: chahatsagarmain --- cmd/collector/app/flags/flags.go | 48 +++++++++++++++++--------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/cmd/collector/app/flags/flags.go b/cmd/collector/app/flags/flags.go index 6f6ce454b34..2d2362c15df 100644 --- a/cmd/collector/app/flags/flags.go +++ b/cmd/collector/app/flags/flags.go @@ -65,6 +65,9 @@ var httpServerFlagsCfg = serverFlagsConfig{ tls: tlscfg.ServerFlagsConfig{ Prefix: "collector.http", }, + corsCfg: corscfg.Flags{ + Prefix: "collector.otlp.http", + }, } var otlpServerFlagsCfg = struct { @@ -84,6 +87,9 @@ var otlpServerFlagsCfg = struct { Prefix: "collector.otlp.http", EnableCertReloadInterval: true, }, + corsCfg: corscfg.Flags{ + Prefix: "collector.otlp.http", + }, }, } @@ -92,14 +98,9 @@ var zipkinServerFlagsCfg = serverFlagsConfig{ tls: tlscfg.ServerFlagsConfig{ Prefix: "collector.zipkin", }, -} - -var corsZipkinFlags = corscfg.Flags{ - Prefix: "collector.zipkin", -} - -var corsOTLPFlags = corscfg.Flags{ - Prefix: "collector.otlp.http", + corsCfg: corscfg.Flags{ + Prefix: "collector.zipkin", + }, } // CollectorOptions holds configuration for collector @@ -134,8 +135,9 @@ type CollectorOptions struct { } type serverFlagsConfig struct { - prefix string - tls tlscfg.ServerFlagsConfig + prefix string + tls tlscfg.ServerFlagsConfig + corsCfg corscfg.Flags } // AddFlags adds flags for CollectorOptions @@ -151,13 +153,13 @@ func AddFlags(flagSet *flag.FlagSet) { flagSet.Bool(flagCollectorOTLPEnabled, true, "Enables OpenTelemetry OTLP receiver on dedicated HTTP and gRPC ports") addHTTPFlags(flagSet, otlpServerFlagsCfg.HTTP, ":4318") - corsOTLPFlags.AddFlags(flagSet) + otlpServerFlagsCfg.HTTP.corsCfg.AddFlags(flagSet) addGRPCFlags(flagSet, otlpServerFlagsCfg.GRPC, ":4317") flagSet.String(flagZipkinHTTPHostPort, "", "The host:port (e.g. 127.0.0.1:9411 or :9411) of the collector's Zipkin server (disabled by default)") flagSet.Bool(flagZipkinKeepAliveEnabled, true, "KeepAlive configures allow Keep-Alive for Zipkin HTTP server (enabled by default)") zipkinServerFlagsCfg.tls.AddFlags(flagSet) - corsZipkinFlags.AddFlags(flagSet) + zipkinServerFlagsCfg.corsCfg.AddFlags(flagSet) tenancy.AddFlags(flagSet) } @@ -190,7 +192,7 @@ func addGRPCFlags(flagSet *flag.FlagSet, cfg serverFlagsConfig, defaultHostPort cfg.tls.AddFlags(flagSet) } -func initHTTPFromViper(v *viper.Viper, _ *zap.Logger, opts *confighttp.ServerConfig, corsOpts *confighttp.CORSConfig, cfg serverFlagsConfig) error { +func initHTTPFromViper(v *viper.Viper, opts *confighttp.ServerConfig, corsOpts *confighttp.CORSConfig, cfg serverFlagsConfig) error { tlsOpts, err := cfg.tls.InitFromViper(v) if err != nil { return fmt.Errorf("failed to parse HTTP TLS options: %w", err) @@ -207,7 +209,7 @@ func initHTTPFromViper(v *viper.Viper, _ *zap.Logger, opts *confighttp.ServerCon return nil } -func initGRPCFromViper(v *viper.Viper, _ *zap.Logger, opts *configgrpc.ServerConfig, cfg serverFlagsConfig) error { +func initGRPCFromViper(v *viper.Viper, opts *configgrpc.ServerConfig, cfg serverFlagsConfig) error { tlsOpts, err := cfg.tls.InitFromViper(v) if err != nil { return fmt.Errorf("failed to parse GRPC TLS options: %w", err) @@ -226,7 +228,7 @@ func initGRPCFromViper(v *viper.Viper, _ *zap.Logger, opts *configgrpc.ServerCon } // InitFromViper initializes CollectorOptions with properties from viper -func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*CollectorOptions, error) { +func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, _ *zap.Logger) (*CollectorOptions, error) { cOpts.CollectorTags = flags.ParseJaegerTags(v.GetString(flagCollectorTags)) cOpts.NumWorkers = v.GetInt(flagNumWorkers) cOpts.QueueSize = v.GetUint(flagQueueSize) @@ -234,28 +236,30 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) cOpts.SpanSizeMetricsEnabled = v.GetBool(flagSpanSizeMetricsEnabled) cOpts.Tenancy = tenancy.InitFromViper(v) - if err := initHTTPFromViper(v, logger, &cOpts.HTTP, nil, httpServerFlagsCfg); err != nil { + httpCorsOpts := httpServerFlagsCfg.corsCfg.InitFromViper(v) + + if err := initHTTPFromViper(v, &cOpts.HTTP, httpCorsOpts, httpServerFlagsCfg); err != nil { return cOpts, fmt.Errorf("failed to parse HTTP server options: %w", err) } - if err := initGRPCFromViper(v, logger, &cOpts.GRPC, grpcServerFlagsCfg); err != nil { + if err := initGRPCFromViper(v, &cOpts.GRPC, grpcServerFlagsCfg); err != nil { return cOpts, fmt.Errorf("failed to parse gRPC server options: %w", err) } cOpts.OTLP.Enabled = v.GetBool(flagCollectorOTLPEnabled) - corsOpts := corsOTLPFlags.InitFromViper(v) + otlpCorsOpts := otlpServerFlagsCfg.HTTP.corsCfg.InitFromViper(v) - if err := initHTTPFromViper(v, logger, &cOpts.OTLP.HTTP, corsOpts, otlpServerFlagsCfg.HTTP); err != nil { + if err := initHTTPFromViper(v, &cOpts.OTLP.HTTP, otlpCorsOpts, otlpServerFlagsCfg.HTTP); err != nil { return cOpts, fmt.Errorf("failed to parse OTLP/HTTP server options: %w", err) } - if err := initGRPCFromViper(v, logger, &cOpts.OTLP.GRPC, otlpServerFlagsCfg.GRPC); err != nil { + if err := initGRPCFromViper(v, &cOpts.OTLP.GRPC, otlpServerFlagsCfg.GRPC); err != nil { return cOpts, fmt.Errorf("failed to parse OTLP/gRPC server options: %w", err) } cOpts.Zipkin.KeepAlive = v.GetBool(flagZipkinKeepAliveEnabled) - corsOpts = corsZipkinFlags.InitFromViper(v) + zipkinCorsOpts := zipkinServerFlagsCfg.corsCfg.InitFromViper(v) - if err := initHTTPFromViper(v, logger, &cOpts.Zipkin.ServerConfig, corsOpts, zipkinServerFlagsCfg); err != nil { + if err := initHTTPFromViper(v, &cOpts.Zipkin.ServerConfig, zipkinCorsOpts, zipkinServerFlagsCfg); err != nil { return cOpts, fmt.Errorf("failed to parse Zipkin server options: %w", err) } From 6943164661c80e92c9073922f0f91604639960e4 Mon Sep 17 00:00:00 2001 From: chahatsagarmain Date: Thu, 12 Dec 2024 02:29:18 +0530 Subject: [PATCH 18/18] simplified code Signed-off-by: chahatsagarmain --- cmd/collector/app/flags/flags.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/cmd/collector/app/flags/flags.go b/cmd/collector/app/flags/flags.go index 2d2362c15df..2cef1e5f585 100644 --- a/cmd/collector/app/flags/flags.go +++ b/cmd/collector/app/flags/flags.go @@ -192,7 +192,7 @@ func addGRPCFlags(flagSet *flag.FlagSet, cfg serverFlagsConfig, defaultHostPort cfg.tls.AddFlags(flagSet) } -func initHTTPFromViper(v *viper.Viper, opts *confighttp.ServerConfig, corsOpts *confighttp.CORSConfig, cfg serverFlagsConfig) error { +func initHTTPFromViper(v *viper.Viper, opts *confighttp.ServerConfig, cfg serverFlagsConfig) error { tlsOpts, err := cfg.tls.InitFromViper(v) if err != nil { return fmt.Errorf("failed to parse HTTP TLS options: %w", err) @@ -202,9 +202,7 @@ func initHTTPFromViper(v *viper.Viper, opts *confighttp.ServerConfig, corsOpts * opts.IdleTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPIdleTimeout) opts.ReadTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadTimeout) opts.ReadHeaderTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadHeaderTimeout) - if corsOpts != nil { - opts.CORS = corsOpts - } + opts.CORS = cfg.corsCfg.InitFromViper(v) return nil } @@ -236,9 +234,7 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, _ *zap.Logger) (*Co cOpts.SpanSizeMetricsEnabled = v.GetBool(flagSpanSizeMetricsEnabled) cOpts.Tenancy = tenancy.InitFromViper(v) - httpCorsOpts := httpServerFlagsCfg.corsCfg.InitFromViper(v) - - if err := initHTTPFromViper(v, &cOpts.HTTP, httpCorsOpts, httpServerFlagsCfg); err != nil { + if err := initHTTPFromViper(v, &cOpts.HTTP, httpServerFlagsCfg); err != nil { return cOpts, fmt.Errorf("failed to parse HTTP server options: %w", err) } @@ -247,9 +243,8 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, _ *zap.Logger) (*Co } cOpts.OTLP.Enabled = v.GetBool(flagCollectorOTLPEnabled) - otlpCorsOpts := otlpServerFlagsCfg.HTTP.corsCfg.InitFromViper(v) - if err := initHTTPFromViper(v, &cOpts.OTLP.HTTP, otlpCorsOpts, otlpServerFlagsCfg.HTTP); err != nil { + if err := initHTTPFromViper(v, &cOpts.OTLP.HTTP, otlpServerFlagsCfg.HTTP); err != nil { return cOpts, fmt.Errorf("failed to parse OTLP/HTTP server options: %w", err) } if err := initGRPCFromViper(v, &cOpts.OTLP.GRPC, otlpServerFlagsCfg.GRPC); err != nil { @@ -257,9 +252,8 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, _ *zap.Logger) (*Co } cOpts.Zipkin.KeepAlive = v.GetBool(flagZipkinKeepAliveEnabled) - zipkinCorsOpts := zipkinServerFlagsCfg.corsCfg.InitFromViper(v) - if err := initHTTPFromViper(v, &cOpts.Zipkin.ServerConfig, zipkinCorsOpts, zipkinServerFlagsCfg); err != nil { + if err := initHTTPFromViper(v, &cOpts.Zipkin.ServerConfig, zipkinServerFlagsCfg); err != nil { return cOpts, fmt.Errorf("failed to parse Zipkin server options: %w", err) }