From ce1f7a2a92561e4728b5a359ba8ec2c373a7f9fe Mon Sep 17 00:00:00 2001 From: ansh0014 Date: Wed, 11 Feb 2026 13:34:17 +0530 Subject: [PATCH 1/8] [extension/jaegerremotesampling] Add validation to detect port conflicts - Enhanced Validate() method to check for port conflicts between HTTP and gRPC servers - Added extractPort() helper function to parse endpoint strings - Added comprehensive unit tests for validation logic - Validation fails fast with clear, actionable error messages Fixes #45944 --- .../jaegerremotesampling-port-validation.yaml | 11 + extension/jaegerremotesampling/config.go | 42 +++ extension/jaegerremotesampling/config_test.go | 268 ++++++++++++++++-- 3 files changed, 305 insertions(+), 16 deletions(-) create mode 100644 .chloggen/jaegerremotesampling-port-validation.yaml diff --git a/.chloggen/jaegerremotesampling-port-validation.yaml b/.chloggen/jaegerremotesampling-port-validation.yaml new file mode 100644 index 0000000000000..53ee38aa121c5 --- /dev/null +++ b/.chloggen/jaegerremotesampling-port-validation.yaml @@ -0,0 +1,11 @@ +change_type: enhancement + +component: jaegerremotesampling + +note: Add validation to detect port conflicts between HTTP and gRPC servers +issues:[#45944] +subtext: | + The jaegerremotesampling extension now validates that HTTP and gRPC servers + are configured on different ports during configuration loading. This prevents + runtime port binding failures and provides clear, actionable error messages + when misconfigurations are detected. diff --git a/extension/jaegerremotesampling/config.go b/extension/jaegerremotesampling/config.go index ae7520aaca155..88fe2207f1a4a 100644 --- a/extension/jaegerremotesampling/config.go +++ b/extension/jaegerremotesampling/config.go @@ -5,6 +5,8 @@ package jaegerremotesampling // import "github.com/open-telemetry/opentelemetry- import ( "errors" + "fmt" + "net" "time" "go.opentelemetry.io/collector/component" @@ -16,6 +18,7 @@ var ( errTooManySources = errors.New("too many sources specified, has to be either 'file' or 'remote'") errNoSources = errors.New("no sources specified, has to be either 'file' or 'remote'") errAtLeastOneProtocol = errors.New("no protocols selected to serve the strategies, use 'grpc', 'http', or both") + errSamePortConflict = errors.New("HTTP and gRPC servers cannot use the same port") ) // Config has the configuration for the extension enabling the health check @@ -43,10 +46,32 @@ var _ component.Config = (*Config)(nil) // Validate checks if the extension configuration is valid func (cfg *Config) Validate() error { + // Validate the protocol configuration. At least one protocol should be configured to serve the strategies. if cfg.HTTPServerConfig == nil && cfg.GRPCServerConfig == nil { return errAtLeastOneProtocol } + // Validate port conflict between HTTP and gRPC servers + if cfg.HTTPServerConfig != nil && cfg.GRPCServerConfig != nil { + httpPort, err := extractPort(cfg.HTTPServerConfig.NetAddr.Endpoint) + if err != nil { + return fmt.Errorf("invalid HTTP endpoint: %w", err) + } + + grpcPort, err := extractPort(cfg.GRPCServerConfig.NetAddr.Endpoint) + if err != nil { + return fmt.Errorf("invalid gRPC endpoint: %w", err) + } + + if httpPort == grpcPort { + return fmt.Errorf("%w: both configured to use port %s. "+ + "Configure different ports for HTTP and gRPC servers. "+ + "Example: http.endpoint='0.0.0.0:5778', grpc.endpoint='0.0.0.0:14250'", + errSamePortConflict, grpcPort) + } + } + + // Validate source configuration if cfg.Source.File != "" && cfg.Source.Remote != nil { return errTooManySources } @@ -57,3 +82,20 @@ func (cfg *Config) Validate() error { return nil } + +// extractPort extracts the port from an endpoint string. +func extractPort(endpoint string) (string, error) { + // Handle empty endpoint + if endpoint == "" { + return "", errors.New("endpoint is empty") + } + + // net.SplitHostPort handles IPv4, IPv6, and various endpoint formats + _, port, err := net.SplitHostPort(endpoint) + if err != nil { + // If SplitHostPort fails, it might be because there is no port specified + return "", fmt.Errorf("failed to parse endpoint %q: %w", endpoint, err) + } + + return port, nil +} diff --git a/extension/jaegerremotesampling/config_test.go b/extension/jaegerremotesampling/config_test.go index a2082fd840aad..3fb6b234e3d4e 100644 --- a/extension/jaegerremotesampling/config_test.go +++ b/extension/jaegerremotesampling/config_test.go @@ -30,14 +30,18 @@ func TestLoadConfig(t *testing.T) { { id: component.NewID(metadata.Type), expected: &Config{ - HTTPServerConfig: &confighttp.ServerConfig{NetAddr: confignet.AddrConfig{ - Endpoint: "localhost:5778", - Transport: confignet.TransportTypeTCP, - }}, - GRPCServerConfig: &configgrpc.ServerConfig{NetAddr: confignet.AddrConfig{ - Endpoint: "localhost:14250", - Transport: confignet.TransportTypeTCP, - }}, + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "localhost:5778", + Transport: confignet.TransportTypeTCP, + }, + }, + GRPCServerConfig: &configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "localhost:14250", + Transport: confignet.TransportTypeTCP, + }, + }, Source: Source{ Remote: &configgrpc.ClientConfig{ Endpoint: "jaeger-collector:14250", @@ -48,14 +52,18 @@ func TestLoadConfig(t *testing.T) { { id: component.NewIDWithName(metadata.Type, "1"), expected: &Config{ - HTTPServerConfig: &confighttp.ServerConfig{NetAddr: confignet.AddrConfig{ - Endpoint: "localhost:5778", - Transport: confignet.TransportTypeTCP, - }}, - GRPCServerConfig: &configgrpc.ServerConfig{NetAddr: confignet.AddrConfig{ - Endpoint: "localhost:14250", - Transport: confignet.TransportTypeTCP, - }}, + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "localhost:5778", + Transport: confignet.TransportTypeTCP, + }, + }, + GRPCServerConfig: &configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "localhost:14250", + Transport: confignet.TransportTypeTCP, + }, + }, Source: Source{ ReloadInterval: time.Second, File: "/etc/otelcol/sampling_strategies.json", @@ -115,3 +123,231 @@ func TestValidate(t *testing.T) { }) } } + +func TestConfigValidate(t *testing.T) { + tests := []struct { + name string + config Config + expectedErr error + }{ + { + name: "valid config with both protocols on different ports", + config: Config{ + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:5778", + }, + }, + GRPCServerConfig: &configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:14250", + }, + }, + Source: Source{ + File: "/etc/strategies.json", + }, + }, + expectedErr: nil, + }, + { + name: "valid config with only HTTP", + config: Config{ + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:5778", + }, + }, + Source: Source{ + File: "/etc/strategies.json", + }, + }, + expectedErr: nil, + }, + { + name: "valid config with only gRPC", + config: Config{ + GRPCServerConfig: &configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:14250", + }, + }, + Source: Source{ + Remote: &configgrpc.ClientConfig{}, + }, + }, + expectedErr: nil, + }, + { + name: "invalid: no protocols configured", + config: Config{ + Source: Source{ + File: "/etc/strategies.json", + }, + }, + expectedErr: errAtLeastOneProtocol, + }, + { + name: "invalid: same port for HTTP and gRPC", + config: Config{ + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:14250", + }, + }, + GRPCServerConfig: &configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:14250", + }, + }, + Source: Source{ + File: "/etc/strategies.json", + }, + }, + expectedErr: errSamePortConflict, + }, + { + name: "invalid: same port with different host formats", + config: Config{ + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "localhost:8080", + }, + }, + GRPCServerConfig: &configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:8080", + }, + }, + Source: Source{ + File: "/etc/strategies.json", + }, + }, + expectedErr: errSamePortConflict, + }, + { + name: "invalid: same port with IPv6 format", + config: Config{ + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "[::1]:9090", + }, + }, + GRPCServerConfig: &configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:9090", + }, + }, + Source: Source{ + File: "/etc/strategies.json", + }, + }, + expectedErr: errSamePortConflict, + }, + { + name: "invalid: too many sources", + config: Config{ + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:5778", + }, + }, + Source: Source{ + File: "/etc/strategies.json", + Remote: &configgrpc.ClientConfig{}, + }, + }, + expectedErr: errTooManySources, + }, + { + name: "invalid: no sources", + config: Config{ + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:5778", + }, + }, + Source: Source{}, + }, + expectedErr: errNoSources, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.config.Validate() + if tt.expectedErr != nil { + require.Error(t, err) + assert.ErrorIs(t, err, tt.expectedErr) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestExtractPort(t *testing.T) { + tests := []struct { + name string + endpoint string + expectedPort string + expectError bool + }{ + { + name: "standard format with host and port", + endpoint: "0.0.0.0:8080", + expectedPort: "8080", + expectError: false, + }, + { + name: "port only format", + endpoint: ":8080", + expectedPort: "8080", + expectError: false, + }, + { + name: "localhost with port", + endpoint: "localhost:9090", + expectedPort: "9090", + expectError: false, + }, + { + name: "IPv6 format", + endpoint: "[::1]:8080", + expectedPort: "8080", + expectError: false, + }, + { + name: "IPv6 with zone", + endpoint: "[fe80::1%lo0]:8080", + expectedPort: "8080", + expectError: false, + }, + { + name: "empty endpoint", + endpoint: "", + expectError: true, + }, + { + name: "missing port", + endpoint: "localhost", + expectError: true, + }, + { + name: "invalid format", + endpoint: "not:a:valid:endpoint:8080", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + port, err := extractPort(tt.endpoint) + if tt.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expectedPort, port) + } + }) + } +} From 3f5aac1b469c19e812567e843a48b0303df98f64 Mon Sep 17 00:00:00 2001 From: ansh0014 Date: Fri, 1 May 2026 20:46:16 +0530 Subject: [PATCH 2/8] Fix chloggen issues format --- .chloggen/jaegerremotesampling-port-validation.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/jaegerremotesampling-port-validation.yaml b/.chloggen/jaegerremotesampling-port-validation.yaml index 53ee38aa121c5..14177b08edecc 100644 --- a/.chloggen/jaegerremotesampling-port-validation.yaml +++ b/.chloggen/jaegerremotesampling-port-validation.yaml @@ -3,7 +3,7 @@ change_type: enhancement component: jaegerremotesampling note: Add validation to detect port conflicts between HTTP and gRPC servers -issues:[#45944] +issues: [45944] subtext: | The jaegerremotesampling extension now validates that HTTP and gRPC servers are configured on different ports during configuration loading. This prevents From 5af4be6e5a7d50b3daa762e9ea1ab82e02674708 Mon Sep 17 00:00:00 2001 From: ansh0014 Date: Fri, 1 May 2026 20:46:50 +0530 Subject: [PATCH 3/8] [extension/jaegerremotesampling] Normalize ports in validation --- extension/jaegerremotesampling/config.go | 16 +++++- extension/jaegerremotesampling/config_test.go | 50 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/extension/jaegerremotesampling/config.go b/extension/jaegerremotesampling/config.go index 88fe2207f1a4a..90323f321325c 100644 --- a/extension/jaegerremotesampling/config.go +++ b/extension/jaegerremotesampling/config.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "net" + "strconv" "time" "go.opentelemetry.io/collector/component" @@ -97,5 +98,18 @@ func extractPort(endpoint string) (string, error) { return "", fmt.Errorf("failed to parse endpoint %q: %w", endpoint, err) } - return port, nil + // Normalize the port so comparisons do not miss equivalent values like "08080" or service names. + if portNumber, err := strconv.Atoi(port); err == nil { + if portNumber < 0 || portNumber > 65535 { + return "", fmt.Errorf("invalid port %q: out of range", port) + } + return strconv.Itoa(portNumber), nil + } + + portNumber, err := net.LookupPort("tcp", port) + if err != nil { + return "", fmt.Errorf("invalid port %q: %w", port, err) + } + + return strconv.Itoa(portNumber), nil } diff --git a/extension/jaegerremotesampling/config_test.go b/extension/jaegerremotesampling/config_test.go index 3fb6b234e3d4e..34dc295838e7f 100644 --- a/extension/jaegerremotesampling/config_test.go +++ b/extension/jaegerremotesampling/config_test.go @@ -243,6 +243,44 @@ func TestConfigValidate(t *testing.T) { }, expectedErr: errSamePortConflict, }, + { + name: "invalid: same port with zero-padded format", + config: Config{ + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:08080", + }, + }, + GRPCServerConfig: &configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:8080", + }, + }, + Source: Source{ + File: "/etc/strategies.json", + }, + }, + expectedErr: errSamePortConflict, + }, + { + name: "invalid: same port with service name", + config: Config{ + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "localhost:http", + }, + }, + GRPCServerConfig: &configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:80", + }, + }, + Source: Source{ + File: "/etc/strategies.json", + }, + }, + expectedErr: errSamePortConflict, + }, { name: "invalid: too many sources", config: Config{ @@ -322,6 +360,18 @@ func TestExtractPort(t *testing.T) { expectedPort: "8080", expectError: false, }, + { + name: "zero-padded port", + endpoint: "0.0.0.0:08080", + expectedPort: "8080", + expectError: false, + }, + { + name: "service name port", + endpoint: "localhost:http", + expectedPort: "80", + expectError: false, + }, { name: "empty endpoint", endpoint: "", From da4532590ad4466bce5e61cc1f0fef197a67406e Mon Sep 17 00:00:00 2001 From: ansh0014 Date: Fri, 1 May 2026 21:03:07 +0530 Subject: [PATCH 4/8] [extension/jaegerremotesampling] Fix lint shadowing --- extension/jaegerremotesampling/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/jaegerremotesampling/config.go b/extension/jaegerremotesampling/config.go index 90323f321325c..feec8f2c4ad78 100644 --- a/extension/jaegerremotesampling/config.go +++ b/extension/jaegerremotesampling/config.go @@ -99,7 +99,7 @@ func extractPort(endpoint string) (string, error) { } // Normalize the port so comparisons do not miss equivalent values like "08080" or service names. - if portNumber, err := strconv.Atoi(port); err == nil { + if portNumber, parseErr := strconv.Atoi(port); parseErr == nil { if portNumber < 0 || portNumber > 65535 { return "", fmt.Errorf("invalid port %q: out of range", port) } From 5cddebefa1c60af8697c31f9cc6ee991e15f3334 Mon Sep 17 00:00:00 2001 From: Anshul Jagota Date: Fri, 1 May 2026 21:31:13 +0530 Subject: [PATCH 5/8] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- extension/jaegerremotesampling/config_test.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/extension/jaegerremotesampling/config_test.go b/extension/jaegerremotesampling/config_test.go index 34dc295838e7f..7bcc082acd5d4 100644 --- a/extension/jaegerremotesampling/config_test.go +++ b/extension/jaegerremotesampling/config_test.go @@ -149,20 +149,6 @@ func TestConfigValidate(t *testing.T) { }, expectedErr: nil, }, - { - name: "valid config with only HTTP", - config: Config{ - HTTPServerConfig: &confighttp.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:5778", - }, - }, - Source: Source{ - File: "/etc/strategies.json", - }, - }, - expectedErr: nil, - }, { name: "valid config with only gRPC", config: Config{ From 462076f8666f5ee0226144de892044b8f7e73f08 Mon Sep 17 00:00:00 2001 From: ansh0014 Date: Sun, 17 May 2026 00:11:13 +0530 Subject: [PATCH 6/8] jaegerremotesampling: make gRPC server optional --- extension/jaegerremotesampling/config.go | 61 +- extension/jaegerremotesampling/config_test.go | 552 ++++++------------ extension/jaegerremotesampling/extension.go | 209 +++---- .../jaegerremotesampling/extension_test.go | 15 +- extension/jaegerremotesampling/factory.go | 5 +- .../jaegerremotesampling/factory_test.go | 5 +- 6 files changed, 311 insertions(+), 536 deletions(-) diff --git a/extension/jaegerremotesampling/config.go b/extension/jaegerremotesampling/config.go index feec8f2c4ad78..7ad7d9eb004b2 100644 --- a/extension/jaegerremotesampling/config.go +++ b/extension/jaegerremotesampling/config.go @@ -5,13 +5,11 @@ package jaegerremotesampling // import "github.com/open-telemetry/opentelemetry- import ( "errors" - "fmt" - "net" - "strconv" "time" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configgrpc" + "go.opentelemetry.io/collector/config/configoptional" "go.opentelemetry.io/collector/config/confighttp" ) @@ -19,14 +17,13 @@ var ( errTooManySources = errors.New("too many sources specified, has to be either 'file' or 'remote'") errNoSources = errors.New("no sources specified, has to be either 'file' or 'remote'") errAtLeastOneProtocol = errors.New("no protocols selected to serve the strategies, use 'grpc', 'http', or both") - errSamePortConflict = errors.New("HTTP and gRPC servers cannot use the same port") ) // Config has the configuration for the extension enabling the health check // extension, used to report the health status of the service. type Config struct { - HTTPServerConfig *confighttp.ServerConfig `mapstructure:"http"` - GRPCServerConfig *configgrpc.ServerConfig `mapstructure:"grpc"` + HTTPServerConfig *confighttp.ServerConfig `mapstructure:"http"` + GRPCServerConfig configoptional.Optional[configgrpc.ServerConfig] `mapstructure:"grpc"` // Source configures the source for the strategies file. One of `remote` or `file` has to be specified. Source Source `mapstructure:"source"` @@ -48,30 +45,10 @@ var _ component.Config = (*Config)(nil) // Validate checks if the extension configuration is valid func (cfg *Config) Validate() error { // Validate the protocol configuration. At least one protocol should be configured to serve the strategies. - if cfg.HTTPServerConfig == nil && cfg.GRPCServerConfig == nil { + if cfg.HTTPServerConfig == nil && !cfg.GRPCServerConfig.HasValue() { return errAtLeastOneProtocol } - // Validate port conflict between HTTP and gRPC servers - if cfg.HTTPServerConfig != nil && cfg.GRPCServerConfig != nil { - httpPort, err := extractPort(cfg.HTTPServerConfig.NetAddr.Endpoint) - if err != nil { - return fmt.Errorf("invalid HTTP endpoint: %w", err) - } - - grpcPort, err := extractPort(cfg.GRPCServerConfig.NetAddr.Endpoint) - if err != nil { - return fmt.Errorf("invalid gRPC endpoint: %w", err) - } - - if httpPort == grpcPort { - return fmt.Errorf("%w: both configured to use port %s. "+ - "Configure different ports for HTTP and gRPC servers. "+ - "Example: http.endpoint='0.0.0.0:5778', grpc.endpoint='0.0.0.0:14250'", - errSamePortConflict, grpcPort) - } - } - // Validate source configuration if cfg.Source.File != "" && cfg.Source.Remote != nil { return errTooManySources @@ -83,33 +60,3 @@ func (cfg *Config) Validate() error { return nil } - -// extractPort extracts the port from an endpoint string. -func extractPort(endpoint string) (string, error) { - // Handle empty endpoint - if endpoint == "" { - return "", errors.New("endpoint is empty") - } - - // net.SplitHostPort handles IPv4, IPv6, and various endpoint formats - _, port, err := net.SplitHostPort(endpoint) - if err != nil { - // If SplitHostPort fails, it might be because there is no port specified - return "", fmt.Errorf("failed to parse endpoint %q: %w", endpoint, err) - } - - // Normalize the port so comparisons do not miss equivalent values like "08080" or service names. - if portNumber, parseErr := strconv.Atoi(port); parseErr == nil { - if portNumber < 0 || portNumber > 65535 { - return "", fmt.Errorf("invalid port %q: out of range", port) - } - return strconv.Itoa(portNumber), nil - } - - portNumber, err := net.LookupPort("tcp", port) - if err != nil { - return "", fmt.Errorf("invalid port %q: %w", port, err) - } - - return strconv.Itoa(portNumber), nil -} diff --git a/extension/jaegerremotesampling/config_test.go b/extension/jaegerremotesampling/config_test.go index 7bcc082acd5d4..7b43dfb1e659c 100644 --- a/extension/jaegerremotesampling/config_test.go +++ b/extension/jaegerremotesampling/config_test.go @@ -4,386 +4,200 @@ package jaegerremotesampling import ( - "path/filepath" - "testing" - "time" + "path/filepath" + "testing" + "time" - "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/confmap/confmaptest" - "go.opentelemetry.io/collector/confmap/xconfmap" + "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/configoptional" + "go.opentelemetry.io/collector/config/confighttp" + "go.opentelemetry.io/collector/config/confignet" + "go.opentelemetry.io/collector/confmap/confmaptest" + "go.opentelemetry.io/collector/confmap/xconfmap" - "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/metadata" + "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/metadata" ) func TestLoadConfig(t *testing.T) { - t.Parallel() + t.Parallel() - tests := []struct { - id component.ID - expected component.Config - }{ - { - id: component.NewID(metadata.Type), - expected: &Config{ - HTTPServerConfig: &confighttp.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "localhost:5778", - Transport: confignet.TransportTypeTCP, - }, - }, - GRPCServerConfig: &configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "localhost:14250", - Transport: confignet.TransportTypeTCP, - }, - }, - Source: Source{ - Remote: &configgrpc.ClientConfig{ - Endpoint: "jaeger-collector:14250", - }, - }, - }, - }, - { - id: component.NewIDWithName(metadata.Type, "1"), - expected: &Config{ - HTTPServerConfig: &confighttp.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "localhost:5778", - Transport: confignet.TransportTypeTCP, - }, - }, - GRPCServerConfig: &configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "localhost:14250", - Transport: confignet.TransportTypeTCP, - }, - }, - Source: Source{ - ReloadInterval: time.Second, - File: "/etc/otelcol/sampling_strategies.json", - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.id.String(), func(t *testing.T) { - cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config.yaml")) - require.NoError(t, err) - factory := NewFactory() - cfg := factory.CreateDefaultConfig() - sub, err := cm.Sub(tt.id.String()) - require.NoError(t, err) - require.NoError(t, sub.Unmarshal(cfg)) - assert.NoError(t, xconfmap.Validate(cfg)) - assert.Equal(t, tt.expected, cfg) - }) - } + tests := []struct { + id component.ID + expected component.Config + }{ + { + id: component.NewID(metadata.Type), + expected: &Config{ + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "localhost:5778", + Transport: confignet.TransportTypeTCP, + }, + }, + GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "localhost:14250", + Transport: confignet.TransportTypeTCP, + }, + }), + Source: Source{ + Remote: &configgrpc.ClientConfig{ + Endpoint: "jaeger-collector:14250", + }, + }, + }, + }, + { + id: component.NewIDWithName(metadata.Type, "1"), + expected: &Config{ + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "localhost:5778", + Transport: confignet.TransportTypeTCP, + }, + }, + GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "localhost:14250", + Transport: confignet.TransportTypeTCP, + }, + }), + Source: Source{ + ReloadInterval: time.Second, + File: "/etc/otelcol/sampling_strategies.json", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.id.String(), func(t *testing.T) { + cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config.yaml")) + require.NoError(t, err) + factory := NewFactory() + cfg := factory.CreateDefaultConfig() + sub, err := cm.Sub(tt.id.String()) + require.NoError(t, err) + require.NoError(t, sub.Unmarshal(cfg)) + assert.NoError(t, xconfmap.Validate(cfg)) + assert.Equal(t, tt.expected, cfg) + }) + } } func TestValidate(t *testing.T) { - testCases := []struct { - desc string - cfg Config - expected error - }{ - { - desc: "no receiving protocols", - cfg: Config{}, - expected: errAtLeastOneProtocol, - }, - { - desc: "no sources", - cfg: Config{ - GRPCServerConfig: &configgrpc.ServerConfig{}, - }, - expected: errNoSources, - }, - { - desc: "too many sources", - cfg: Config{ - GRPCServerConfig: &configgrpc.ServerConfig{}, - Source: Source{ - Remote: &configgrpc.ClientConfig{}, - File: "/tmp/some-file", - }, - }, - expected: errTooManySources, - }, - } - for _, tC := range testCases { - t.Run(tC.desc, func(t *testing.T) { - res := tC.cfg.Validate() - assert.Equal(t, tC.expected, res) - }) - } + testCases := []struct { + desc string + cfg Config + expected error + }{ + { + desc: "no receiving protocols", + cfg: Config{}, + expected: errAtLeastOneProtocol, + }, + { + desc: "no sources", + cfg: Config{ + GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{}), + }, + expected: errNoSources, + }, + { + desc: "too many sources", + cfg: Config{ + GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{}), + Source: Source{ + Remote: &configgrpc.ClientConfig{}, + File: "/tmp/some-file", + }, + }, + expected: errTooManySources, + }, + } + for _, tC := range testCases { + t.Run(tC.desc, func(t *testing.T) { + res := tC.cfg.Validate() + assert.Equal(t, tC.expected, res) + }) + } } func TestConfigValidate(t *testing.T) { - tests := []struct { - name string - config Config - expectedErr error - }{ - { - name: "valid config with both protocols on different ports", - config: Config{ - HTTPServerConfig: &confighttp.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:5778", - }, - }, - GRPCServerConfig: &configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:14250", - }, - }, - Source: Source{ - File: "/etc/strategies.json", - }, - }, - expectedErr: nil, - }, - { - name: "valid config with only gRPC", - config: Config{ - GRPCServerConfig: &configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:14250", - }, - }, - Source: Source{ - Remote: &configgrpc.ClientConfig{}, - }, - }, - expectedErr: nil, - }, - { - name: "invalid: no protocols configured", - config: Config{ - Source: Source{ - File: "/etc/strategies.json", - }, - }, - expectedErr: errAtLeastOneProtocol, - }, - { - name: "invalid: same port for HTTP and gRPC", - config: Config{ - HTTPServerConfig: &confighttp.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:14250", - }, - }, - GRPCServerConfig: &configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:14250", - }, - }, - Source: Source{ - File: "/etc/strategies.json", - }, - }, - expectedErr: errSamePortConflict, - }, - { - name: "invalid: same port with different host formats", - config: Config{ - HTTPServerConfig: &confighttp.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "localhost:8080", - }, - }, - GRPCServerConfig: &configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:8080", - }, - }, - Source: Source{ - File: "/etc/strategies.json", - }, - }, - expectedErr: errSamePortConflict, - }, - { - name: "invalid: same port with IPv6 format", - config: Config{ - HTTPServerConfig: &confighttp.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "[::1]:9090", - }, - }, - GRPCServerConfig: &configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:9090", - }, - }, - Source: Source{ - File: "/etc/strategies.json", - }, - }, - expectedErr: errSamePortConflict, - }, - { - name: "invalid: same port with zero-padded format", - config: Config{ - HTTPServerConfig: &confighttp.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:08080", - }, - }, - GRPCServerConfig: &configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:8080", - }, - }, - Source: Source{ - File: "/etc/strategies.json", - }, - }, - expectedErr: errSamePortConflict, - }, - { - name: "invalid: same port with service name", - config: Config{ - HTTPServerConfig: &confighttp.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "localhost:http", - }, - }, - GRPCServerConfig: &configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:80", - }, - }, - Source: Source{ - File: "/etc/strategies.json", - }, - }, - expectedErr: errSamePortConflict, - }, - { - name: "invalid: too many sources", - config: Config{ - HTTPServerConfig: &confighttp.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:5778", - }, - }, - Source: Source{ - File: "/etc/strategies.json", - Remote: &configgrpc.ClientConfig{}, - }, - }, - expectedErr: errTooManySources, - }, - { - name: "invalid: no sources", - config: Config{ - HTTPServerConfig: &confighttp.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:5778", - }, - }, - Source: Source{}, - }, - expectedErr: errNoSources, - }, - } + tests := []struct { + name string + config Config + expectedErr error + }{ + { + name: "valid config with both protocols", + config: Config{ + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:5778", + }, + }, + GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:14250", + }, + }), + Source: Source{ + File: "/etc/strategies.json", + }, + }, + expectedErr: nil, + }, + { + name: "valid config with only gRPC", + config: Config{ + GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:14250", + }, + }), + Source: Source{ + Remote: &configgrpc.ClientConfig{}, + }, + }, + expectedErr: nil, + }, + { + name: "valid config with only HTTP", + config: Config{ + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:5778", + }, + }, + Source: Source{ + File: "/etc/strategies.json", + }, + }, + expectedErr: nil, + }, + { + name: "invalid: no protocols configured", + config: Config{ + Source: Source{ + File: "/etc/strategies.json", + }, + }, + expectedErr: errAtLeastOneProtocol, + }, + } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := tt.config.Validate() - if tt.expectedErr != nil { - require.Error(t, err) - assert.ErrorIs(t, err, tt.expectedErr) - } else { - require.NoError(t, err) - } - }) - } -} - -func TestExtractPort(t *testing.T) { - tests := []struct { - name string - endpoint string - expectedPort string - expectError bool - }{ - { - name: "standard format with host and port", - endpoint: "0.0.0.0:8080", - expectedPort: "8080", - expectError: false, - }, - { - name: "port only format", - endpoint: ":8080", - expectedPort: "8080", - expectError: false, - }, - { - name: "localhost with port", - endpoint: "localhost:9090", - expectedPort: "9090", - expectError: false, - }, - { - name: "IPv6 format", - endpoint: "[::1]:8080", - expectedPort: "8080", - expectError: false, - }, - { - name: "IPv6 with zone", - endpoint: "[fe80::1%lo0]:8080", - expectedPort: "8080", - expectError: false, - }, - { - name: "zero-padded port", - endpoint: "0.0.0.0:08080", - expectedPort: "8080", - expectError: false, - }, - { - name: "service name port", - endpoint: "localhost:http", - expectedPort: "80", - expectError: false, - }, - { - name: "empty endpoint", - endpoint: "", - expectError: true, - }, - { - name: "missing port", - endpoint: "localhost", - expectError: true, - }, - { - name: "invalid format", - endpoint: "not:a:valid:endpoint:8080", - expectError: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - port, err := extractPort(tt.endpoint) - if tt.expectError { - require.Error(t, err) - } else { - require.NoError(t, err) - assert.Equal(t, tt.expectedPort, port) - } - }) - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.config.Validate() + if tt.expectedErr != nil { + require.Error(t, err) + assert.ErrorIs(t, err, tt.expectedErr) + } else { + require.NoError(t, err) + } + }) + } } diff --git a/extension/jaegerremotesampling/extension.go b/extension/jaegerremotesampling/extension.go index ae0bf23785197..21ebd23085867 100644 --- a/extension/jaegerremotesampling/extension.go +++ b/extension/jaegerremotesampling/extension.go @@ -4,123 +4,126 @@ package jaegerremotesampling // import "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling" import ( - "context" - "fmt" - - "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/extension" - "go.uber.org/zap" - - "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/server/grpc" - "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/server/http" - "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/source" - "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/source/filesource" - "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/source/remotesource" + "context" + "fmt" + + "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/extension" + "go.uber.org/zap" + + "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/server/grpc" + "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/server/http" + "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/source" + "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/source/filesource" + "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/source/remotesource" + ) var _ extension.Extension = (*jrsExtension)(nil) type jrsExtension struct { - cfg *Config - telemetry component.TelemetrySettings + cfg *Config + telemetry component.TelemetrySettings - httpServer component.Component - grpcServer component.Component - samplingStore source.Source + httpServer component.Component + grpcServer component.Component + samplingStore source.Source - closers []func() error + closers []func() error } func newExtension(cfg *Config, telemetry component.TelemetrySettings) *jrsExtension { - jrse := &jrsExtension{ - cfg: cfg, - telemetry: telemetry, - } - return jrse + jrse := &jrsExtension{ + cfg: cfg, + telemetry: telemetry, + } + return jrse } func (jrse *jrsExtension) Start(ctx context.Context, host component.Host) error { - // the config validation will take care of ensuring we have one and only one of the following about the - // source of the sampling config: - // - remote (gRPC) - // - local file - // we can then use a simplified logic here to assign the appropriate store - if jrse.cfg.Source.File != "" { - opts := filesource.Options{ - StrategiesFile: jrse.cfg.Source.File, - ReloadInterval: jrse.cfg.Source.ReloadInterval, - } - ss, err := filesource.NewFileSource(opts, jrse.telemetry.Logger) - if err != nil { - return fmt.Errorf("failed to create the local file strategy store: %w", err) - } - - // there's a Close function on the concrete type, which is not visible to us... - // how can we close it then? - jrse.samplingStore = ss - } - - if jrse.cfg.Source.Remote != nil { - conn, err := jrse.cfg.Source.Remote.ToClientConn(ctx, host.GetExtensions(), jrse.telemetry) - if err != nil { - return fmt.Errorf("failed to create the remote strategy store: %w", err) - } - jrse.closers = append(jrse.closers, conn.Close) - remoteStore, closer := remotesource.NewRemoteSource( - conn, - jrse.cfg.Source.Remote, - jrse.cfg.Source.ReloadInterval, - ) - jrse.closers = append(jrse.closers, closer.Close) - jrse.samplingStore = remoteStore - } - - if jrse.cfg.HTTPServerConfig != nil { - httpServer, err := http.NewHTTP(jrse.telemetry, *jrse.cfg.HTTPServerConfig, jrse.samplingStore) - if err != nil { - return fmt.Errorf("error while creating the HTTP server: %w", err) - } - jrse.httpServer = httpServer - // then we start our own server interfaces, starting with the HTTP one - if err := jrse.httpServer.Start(ctx, host); err != nil { - return fmt.Errorf("error while starting the HTTP server: %w", err) - } - } - - if jrse.cfg.GRPCServerConfig != nil { - grpcServer, err := grpc.NewGRPC(jrse.telemetry, *jrse.cfg.GRPCServerConfig, jrse.samplingStore) - if err != nil { - return fmt.Errorf("error while creating the gRPC server: %w", err) - } - jrse.grpcServer = grpcServer - // start our gRPC server interface - if err := jrse.grpcServer.Start(ctx, host); err != nil { - return fmt.Errorf("error while starting the gRPC server: %w", err) - } - } - - return nil + // the config validation will take care of ensuring we have one and only one of the following about the + // source of the sampling config: + // - remote (gRPC) + // - local file + // we can then use a simplified logic here to assign the appropriate store + if jrse.cfg.Source.File != "" { + opts := filesource.Options{ + StrategiesFile: jrse.cfg.Source.File, + ReloadInterval: jrse.cfg.Source.ReloadInterval, + } + ss, err := filesource.NewFileSource(opts, jrse.telemetry.Logger) + if err != nil { + return fmt.Errorf("failed to create the local file strategy store: %w", err) + } + + // there's a Close function on the concrete type, which is not visible to us... + // how can we close it then? + jrse.samplingStore = ss + } + + if jrse.cfg.Source.Remote != nil { + conn, err := jrse.cfg.Source.Remote.ToClientConn(ctx, host.GetExtensions(), jrse.telemetry) + if err != nil { + return fmt.Errorf("failed to create the remote strategy store: %w", err) + } + jrse.closers = append(jrse.closers, conn.Close) + remoteStore, closer := remotesource.NewRemoteSource( + conn, + jrse.cfg.Source.Remote, + jrse.cfg.Source.ReloadInterval, + ) + jrse.closers = append(jrse.closers, closer.Close) + jrse.samplingStore = remoteStore + } + + if jrse.cfg.HTTPServerConfig != nil { + httpServer, err := http.NewHTTP(jrse.telemetry, *jrse.cfg.HTTPServerConfig, jrse.samplingStore) + if err != nil { + return fmt.Errorf("error while creating the HTTP server: %w", err) + } + jrse.httpServer = httpServer + // then we start our own server interfaces, starting with the HTTP one + if err := jrse.httpServer.Start(ctx, host); err != nil { + return fmt.Errorf("error while starting the HTTP server: %w", err) + } + } + + if jrse.cfg.GRPCServerConfig.HasValue() { + grpcCfg := jrse.cfg.GRPCServerConfig.Get() + grpcServer, err := grpc.NewGRPC(jrse.telemetry, *grpcCfg, jrse.samplingStore) + if err != nil { + return fmt.Errorf("error while creating the gRPC server: %w", err) + } + + jrse.grpcServer = grpcServer + // start our gRPC server interface + if err := jrse.grpcServer.Start(ctx, host); err != nil { + return fmt.Errorf("error while starting the gRPC server: %w", err) + } + } + + return nil } func (jrse *jrsExtension) Shutdown(ctx context.Context) error { - // we probably don't want to break whenever an error occurs, we want to continue and close the other resources - if jrse.httpServer != nil { - if err := jrse.httpServer.Shutdown(ctx); err != nil { - jrse.telemetry.Logger.Error("error while shutting down the HTTP server", zap.Error(err)) - } - } - - if jrse.grpcServer != nil { - if err := jrse.grpcServer.Shutdown(ctx); err != nil { - jrse.telemetry.Logger.Error("error while shutting down the gRPC server", zap.Error(err)) - } - } - - for _, closer := range jrse.closers { - if err := closer(); err != nil { - jrse.telemetry.Logger.Error("error while shutting down the sampling store", zap.Error(err)) - } - } - - return nil + // we probably don't want to break whenever an error occurs, we want to continue and close the other resources + if jrse.httpServer != nil { + if err := jrse.httpServer.Shutdown(ctx); err != nil { + jrse.telemetry.Logger.Error("error while shutting down the HTTP server", zap.Error(err)) + } + } + + if jrse.grpcServer != nil { + if err := jrse.grpcServer.Shutdown(ctx); err != nil { + jrse.telemetry.Logger.Error("error while shutting down the gRPC server", zap.Error(err)) + } + } + + for _, closer := range jrse.closers { + if err := closer(); err != nil { + jrse.telemetry.Logger.Error("error while shutting down the sampling store", zap.Error(err)) + } + } + + return nil } diff --git a/extension/jaegerremotesampling/extension_test.go b/extension/jaegerremotesampling/extension_test.go index f78398bac5e71..16675a6d07abd 100644 --- a/extension/jaegerremotesampling/extension_test.go +++ b/extension/jaegerremotesampling/extension_test.go @@ -18,6 +18,7 @@ import ( "go.opentelemetry.io/collector/component/componenttest" "go.opentelemetry.io/collector/config/configgrpc" "go.opentelemetry.io/collector/config/configopaque" + "go.opentelemetry.io/collector/config/configoptional" "go.opentelemetry.io/collector/config/configtls" "google.golang.org/grpc" "google.golang.org/grpc/metadata" @@ -98,7 +99,10 @@ func TestRemote(t *testing.T) { // create the config, pointing to the mock server cfg := testConfig() - cfg.GRPCServerConfig.NetAddr.Endpoint = "127.0.0.1:0" + + grpcCfg := cfg.GRPCServerConfig.Get() + grpcCfg.NetAddr.Endpoint = "127.0.0.1:0" + cfg.GRPCServerConfig = configoptional.Some(*grpcCfg) cfg.Source.ReloadInterval = tc.reloadInterval cfg.Source.Remote = &configgrpc.ClientConfig{ Endpoint: fmt.Sprintf("127.0.0.1:%d", lis.Addr().(*net.TCPAddr).Port), @@ -162,10 +166,15 @@ func (s *samplingServer) GetSamplingStrategy(ctx context.Context, params *api_v2 StrategyType: api_v2.SamplingStrategyType_PROBABILISTIC, }, nil } - func testConfig() *Config { cfg := createDefaultConfig().(*Config) cfg.HTTPServerConfig.NetAddr.Endpoint = "127.0.0.1:5778" - cfg.GRPCServerConfig.NetAddr.Endpoint = "127.0.0.1:14250" + +if cfg.GRPCServerConfig.HasValue() { + grpcCfg := cfg.GRPCServerConfig.Get() + grpcCfg.NetAddr.Endpoint = "127.0.0.1:0" + cfg.GRPCServerConfig = configoptional.Some(*grpcCfg) +} + return cfg } diff --git a/extension/jaegerremotesampling/factory.go b/extension/jaegerremotesampling/factory.go index 55fc78eadde53..43c96f349cecf 100644 --- a/extension/jaegerremotesampling/factory.go +++ b/extension/jaegerremotesampling/factory.go @@ -9,6 +9,7 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configgrpc" + "go.opentelemetry.io/collector/config/configoptional" "go.opentelemetry.io/collector/config/confighttp" "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/extension" @@ -36,12 +37,12 @@ func createDefaultConfig() component.Config { Transport: confignet.TransportTypeTCP, }, }, - GRPCServerConfig: &configgrpc.ServerConfig{ + GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{ NetAddr: confignet.AddrConfig{ Endpoint: testutil.EndpointForPort(14250), Transport: confignet.TransportTypeTCP, }, - }, + }), Source: Source{}, } } diff --git a/extension/jaegerremotesampling/factory_test.go b/extension/jaegerremotesampling/factory_test.go index 918eb9623fba8..35c5d1fdddaf5 100644 --- a/extension/jaegerremotesampling/factory_test.go +++ b/extension/jaegerremotesampling/factory_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/component/componenttest" "go.opentelemetry.io/collector/config/configgrpc" + "go.opentelemetry.io/collector/config/configoptional" "go.opentelemetry.io/collector/config/confighttp" "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/extension/extensiontest" @@ -21,10 +22,10 @@ func TestCreateDefaultConfig(t *testing.T) { Endpoint: "localhost:5778", Transport: confignet.TransportTypeTCP, }}, - GRPCServerConfig: &configgrpc.ServerConfig{NetAddr: confignet.AddrConfig{ + GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{NetAddr: confignet.AddrConfig{ Endpoint: "localhost:14250", Transport: confignet.TransportTypeTCP, - }}, + }}), } // test From a9155fa2d15ee40f1887d8a1df46673dcee6bec3 Mon Sep 17 00:00:00 2001 From: ansh0014 Date: Sun, 17 May 2026 00:37:21 +0530 Subject: [PATCH 7/8] fix: disable gRPC by default, fix formatting --- extension/jaegerremotesampling/config.go | 4 +- extension/jaegerremotesampling/config_test.go | 356 +++++++++--------- extension/jaegerremotesampling/extension.go | 211 ++++++----- .../jaegerremotesampling/extension_test.go | 23 +- extension/jaegerremotesampling/factory.go | 11 +- .../jaegerremotesampling/factory_test.go | 7 +- 6 files changed, 298 insertions(+), 314 deletions(-) diff --git a/extension/jaegerremotesampling/config.go b/extension/jaegerremotesampling/config.go index 7ad7d9eb004b2..1ffb57c6a2c10 100644 --- a/extension/jaegerremotesampling/config.go +++ b/extension/jaegerremotesampling/config.go @@ -9,8 +9,8 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configgrpc" - "go.opentelemetry.io/collector/config/configoptional" "go.opentelemetry.io/collector/config/confighttp" + "go.opentelemetry.io/collector/config/configoptional" ) var ( @@ -22,7 +22,7 @@ var ( // Config has the configuration for the extension enabling the health check // extension, used to report the health status of the service. type Config struct { - HTTPServerConfig *confighttp.ServerConfig `mapstructure:"http"` + HTTPServerConfig *confighttp.ServerConfig `mapstructure:"http"` GRPCServerConfig configoptional.Optional[configgrpc.ServerConfig] `mapstructure:"grpc"` // Source configures the source for the strategies file. One of `remote` or `file` has to be specified. diff --git a/extension/jaegerremotesampling/config_test.go b/extension/jaegerremotesampling/config_test.go index 7b43dfb1e659c..88cbd024aff56 100644 --- a/extension/jaegerremotesampling/config_test.go +++ b/extension/jaegerremotesampling/config_test.go @@ -4,200 +4,190 @@ package jaegerremotesampling import ( - "path/filepath" - "testing" - "time" + "path/filepath" + "testing" + "time" - "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/configoptional" - "go.opentelemetry.io/collector/config/confighttp" - "go.opentelemetry.io/collector/config/confignet" - "go.opentelemetry.io/collector/confmap/confmaptest" - "go.opentelemetry.io/collector/confmap/xconfmap" + "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/configoptional" + "go.opentelemetry.io/collector/confmap/confmaptest" + "go.opentelemetry.io/collector/confmap/xconfmap" - "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/metadata" + "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/metadata" ) func TestLoadConfig(t *testing.T) { - t.Parallel() + t.Parallel() - tests := []struct { - id component.ID - expected component.Config - }{ - { - id: component.NewID(metadata.Type), - expected: &Config{ - HTTPServerConfig: &confighttp.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "localhost:5778", - Transport: confignet.TransportTypeTCP, - }, - }, - GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "localhost:14250", - Transport: confignet.TransportTypeTCP, - }, - }), - Source: Source{ - Remote: &configgrpc.ClientConfig{ - Endpoint: "jaeger-collector:14250", - }, - }, - }, - }, - { - id: component.NewIDWithName(metadata.Type, "1"), - expected: &Config{ - HTTPServerConfig: &confighttp.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "localhost:5778", - Transport: confignet.TransportTypeTCP, - }, - }, - GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "localhost:14250", - Transport: confignet.TransportTypeTCP, - }, - }), - Source: Source{ - ReloadInterval: time.Second, - File: "/etc/otelcol/sampling_strategies.json", - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.id.String(), func(t *testing.T) { - cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config.yaml")) - require.NoError(t, err) - factory := NewFactory() - cfg := factory.CreateDefaultConfig() - sub, err := cm.Sub(tt.id.String()) - require.NoError(t, err) - require.NoError(t, sub.Unmarshal(cfg)) - assert.NoError(t, xconfmap.Validate(cfg)) - assert.Equal(t, tt.expected, cfg) - }) - } + tests := []struct { + id component.ID + expected component.Config + }{ + { + id: component.NewID(metadata.Type), + expected: &Config{ + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "localhost:5778", + Transport: confignet.TransportTypeTCP, + }, + }, + GRPCServerConfig: configoptional.None[configgrpc.ServerConfig](), + Source: Source{ + Remote: &configgrpc.ClientConfig{ + Endpoint: "jaeger-collector:14250", + }, + }, + }, + }, + { + id: component.NewIDWithName(metadata.Type, "1"), + expected: &Config{ + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "localhost:5778", + Transport: confignet.TransportTypeTCP, + }, + }, + GRPCServerConfig: configoptional.None[configgrpc.ServerConfig](), + Source: Source{ + ReloadInterval: time.Second, + File: "/etc/otelcol/sampling_strategies.json", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.id.String(), func(t *testing.T) { + cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config.yaml")) + require.NoError(t, err) + factory := NewFactory() + cfg := factory.CreateDefaultConfig() + sub, err := cm.Sub(tt.id.String()) + require.NoError(t, err) + require.NoError(t, sub.Unmarshal(cfg)) + assert.NoError(t, xconfmap.Validate(cfg)) + assert.Equal(t, tt.expected, cfg) + }) + } } func TestValidate(t *testing.T) { - testCases := []struct { - desc string - cfg Config - expected error - }{ - { - desc: "no receiving protocols", - cfg: Config{}, - expected: errAtLeastOneProtocol, - }, - { - desc: "no sources", - cfg: Config{ - GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{}), - }, - expected: errNoSources, - }, - { - desc: "too many sources", - cfg: Config{ - GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{}), - Source: Source{ - Remote: &configgrpc.ClientConfig{}, - File: "/tmp/some-file", - }, - }, - expected: errTooManySources, - }, - } - for _, tC := range testCases { - t.Run(tC.desc, func(t *testing.T) { - res := tC.cfg.Validate() - assert.Equal(t, tC.expected, res) - }) - } + testCases := []struct { + desc string + cfg Config + expected error + }{ + { + desc: "no receiving protocols", + cfg: Config{}, + expected: errAtLeastOneProtocol, + }, + { + desc: "no sources", + cfg: Config{ + GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{}), + }, + expected: errNoSources, + }, + { + desc: "too many sources", + cfg: Config{ + GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{}), + Source: Source{ + Remote: &configgrpc.ClientConfig{}, + File: "/tmp/some-file", + }, + }, + expected: errTooManySources, + }, + } + for _, tC := range testCases { + t.Run(tC.desc, func(t *testing.T) { + res := tC.cfg.Validate() + assert.Equal(t, tC.expected, res) + }) + } } func TestConfigValidate(t *testing.T) { - tests := []struct { - name string - config Config - expectedErr error - }{ - { - name: "valid config with both protocols", - config: Config{ - HTTPServerConfig: &confighttp.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:5778", - }, - }, - GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:14250", - }, - }), - Source: Source{ - File: "/etc/strategies.json", - }, - }, - expectedErr: nil, - }, - { - name: "valid config with only gRPC", - config: Config{ - GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:14250", - }, - }), - Source: Source{ - Remote: &configgrpc.ClientConfig{}, - }, - }, - expectedErr: nil, - }, - { - name: "valid config with only HTTP", - config: Config{ - HTTPServerConfig: &confighttp.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:5778", - }, - }, - Source: Source{ - File: "/etc/strategies.json", - }, - }, - expectedErr: nil, - }, - { - name: "invalid: no protocols configured", - config: Config{ - Source: Source{ - File: "/etc/strategies.json", - }, - }, - expectedErr: errAtLeastOneProtocol, - }, - } + tests := []struct { + name string + config Config + expectedErr error + }{ + { + name: "valid config with both protocols", + config: Config{ + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:5778", + }, + }, + GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:14250", + }, + }), + Source: Source{ + File: "/etc/strategies.json", + }, + }, + expectedErr: nil, + }, + { + name: "valid config with only gRPC", + config: Config{ + GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:14250", + }, + }), + Source: Source{ + Remote: &configgrpc.ClientConfig{}, + }, + }, + expectedErr: nil, + }, + { + name: "valid config with only HTTP", + config: Config{ + HTTPServerConfig: &confighttp.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "0.0.0.0:5778", + }, + }, + Source: Source{ + File: "/etc/strategies.json", + }, + }, + expectedErr: nil, + }, + { + name: "invalid: no protocols configured", + config: Config{ + Source: Source{ + File: "/etc/strategies.json", + }, + }, + expectedErr: errAtLeastOneProtocol, + }, + } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := tt.config.Validate() - if tt.expectedErr != nil { - require.Error(t, err) - assert.ErrorIs(t, err, tt.expectedErr) - } else { - require.NoError(t, err) - } - }) - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.config.Validate() + if tt.expectedErr != nil { + require.Error(t, err) + assert.ErrorIs(t, err, tt.expectedErr) + } else { + require.NoError(t, err) + } + }) + } } diff --git a/extension/jaegerremotesampling/extension.go b/extension/jaegerremotesampling/extension.go index 21ebd23085867..bbb0e9179d910 100644 --- a/extension/jaegerremotesampling/extension.go +++ b/extension/jaegerremotesampling/extension.go @@ -4,126 +4,125 @@ package jaegerremotesampling // import "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling" import ( - "context" - "fmt" - - "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/extension" - "go.uber.org/zap" - - "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/server/grpc" - "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/server/http" - "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/source" - "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/source/filesource" - "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/source/remotesource" - + "context" + "fmt" + + "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/extension" + "go.uber.org/zap" + + "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/server/grpc" + "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/server/http" + "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/source" + "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/source/filesource" + "github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/source/remotesource" ) var _ extension.Extension = (*jrsExtension)(nil) type jrsExtension struct { - cfg *Config - telemetry component.TelemetrySettings + cfg *Config + telemetry component.TelemetrySettings - httpServer component.Component - grpcServer component.Component - samplingStore source.Source + httpServer component.Component + grpcServer component.Component + samplingStore source.Source - closers []func() error + closers []func() error } func newExtension(cfg *Config, telemetry component.TelemetrySettings) *jrsExtension { - jrse := &jrsExtension{ - cfg: cfg, - telemetry: telemetry, - } - return jrse + jrse := &jrsExtension{ + cfg: cfg, + telemetry: telemetry, + } + return jrse } func (jrse *jrsExtension) Start(ctx context.Context, host component.Host) error { - // the config validation will take care of ensuring we have one and only one of the following about the - // source of the sampling config: - // - remote (gRPC) - // - local file - // we can then use a simplified logic here to assign the appropriate store - if jrse.cfg.Source.File != "" { - opts := filesource.Options{ - StrategiesFile: jrse.cfg.Source.File, - ReloadInterval: jrse.cfg.Source.ReloadInterval, - } - ss, err := filesource.NewFileSource(opts, jrse.telemetry.Logger) - if err != nil { - return fmt.Errorf("failed to create the local file strategy store: %w", err) - } - - // there's a Close function on the concrete type, which is not visible to us... - // how can we close it then? - jrse.samplingStore = ss - } - - if jrse.cfg.Source.Remote != nil { - conn, err := jrse.cfg.Source.Remote.ToClientConn(ctx, host.GetExtensions(), jrse.telemetry) - if err != nil { - return fmt.Errorf("failed to create the remote strategy store: %w", err) - } - jrse.closers = append(jrse.closers, conn.Close) - remoteStore, closer := remotesource.NewRemoteSource( - conn, - jrse.cfg.Source.Remote, - jrse.cfg.Source.ReloadInterval, - ) - jrse.closers = append(jrse.closers, closer.Close) - jrse.samplingStore = remoteStore - } - - if jrse.cfg.HTTPServerConfig != nil { - httpServer, err := http.NewHTTP(jrse.telemetry, *jrse.cfg.HTTPServerConfig, jrse.samplingStore) - if err != nil { - return fmt.Errorf("error while creating the HTTP server: %w", err) - } - jrse.httpServer = httpServer - // then we start our own server interfaces, starting with the HTTP one - if err := jrse.httpServer.Start(ctx, host); err != nil { - return fmt.Errorf("error while starting the HTTP server: %w", err) - } - } - - if jrse.cfg.GRPCServerConfig.HasValue() { - grpcCfg := jrse.cfg.GRPCServerConfig.Get() - grpcServer, err := grpc.NewGRPC(jrse.telemetry, *grpcCfg, jrse.samplingStore) - if err != nil { - return fmt.Errorf("error while creating the gRPC server: %w", err) - } - - jrse.grpcServer = grpcServer - // start our gRPC server interface - if err := jrse.grpcServer.Start(ctx, host); err != nil { - return fmt.Errorf("error while starting the gRPC server: %w", err) - } - } - - return nil + // the config validation will take care of ensuring we have one and only one of the following about the + // source of the sampling config: + // - remote (gRPC) + // - local file + // we can then use a simplified logic here to assign the appropriate store + if jrse.cfg.Source.File != "" { + opts := filesource.Options{ + StrategiesFile: jrse.cfg.Source.File, + ReloadInterval: jrse.cfg.Source.ReloadInterval, + } + ss, err := filesource.NewFileSource(opts, jrse.telemetry.Logger) + if err != nil { + return fmt.Errorf("failed to create the local file strategy store: %w", err) + } + + // there's a Close function on the concrete type, which is not visible to us... + // how can we close it then? + jrse.samplingStore = ss + } + + if jrse.cfg.Source.Remote != nil { + conn, err := jrse.cfg.Source.Remote.ToClientConn(ctx, host.GetExtensions(), jrse.telemetry) + if err != nil { + return fmt.Errorf("failed to create the remote strategy store: %w", err) + } + jrse.closers = append(jrse.closers, conn.Close) + remoteStore, closer := remotesource.NewRemoteSource( + conn, + jrse.cfg.Source.Remote, + jrse.cfg.Source.ReloadInterval, + ) + jrse.closers = append(jrse.closers, closer.Close) + jrse.samplingStore = remoteStore + } + + if jrse.cfg.HTTPServerConfig != nil { + httpServer, err := http.NewHTTP(jrse.telemetry, *jrse.cfg.HTTPServerConfig, jrse.samplingStore) + if err != nil { + return fmt.Errorf("error while creating the HTTP server: %w", err) + } + jrse.httpServer = httpServer + // then we start our own server interfaces, starting with the HTTP one + if err := jrse.httpServer.Start(ctx, host); err != nil { + return fmt.Errorf("error while starting the HTTP server: %w", err) + } + } + + if jrse.cfg.GRPCServerConfig.HasValue() { + grpcCfg := jrse.cfg.GRPCServerConfig.Get() + grpcServer, err := grpc.NewGRPC(jrse.telemetry, *grpcCfg, jrse.samplingStore) + if err != nil { + return fmt.Errorf("error while creating the gRPC server: %w", err) + } + + jrse.grpcServer = grpcServer + // start our gRPC server interface + if err := jrse.grpcServer.Start(ctx, host); err != nil { + return fmt.Errorf("error while starting the gRPC server: %w", err) + } + } + + return nil } func (jrse *jrsExtension) Shutdown(ctx context.Context) error { - // we probably don't want to break whenever an error occurs, we want to continue and close the other resources - if jrse.httpServer != nil { - if err := jrse.httpServer.Shutdown(ctx); err != nil { - jrse.telemetry.Logger.Error("error while shutting down the HTTP server", zap.Error(err)) - } - } - - if jrse.grpcServer != nil { - if err := jrse.grpcServer.Shutdown(ctx); err != nil { - jrse.telemetry.Logger.Error("error while shutting down the gRPC server", zap.Error(err)) - } - } - - for _, closer := range jrse.closers { - if err := closer(); err != nil { - jrse.telemetry.Logger.Error("error while shutting down the sampling store", zap.Error(err)) - } - } - - return nil + // we probably don't want to break whenever an error occurs, we want to continue and close the other resources + if jrse.httpServer != nil { + if err := jrse.httpServer.Shutdown(ctx); err != nil { + jrse.telemetry.Logger.Error("error while shutting down the HTTP server", zap.Error(err)) + } + } + + if jrse.grpcServer != nil { + if err := jrse.grpcServer.Shutdown(ctx); err != nil { + jrse.telemetry.Logger.Error("error while shutting down the gRPC server", zap.Error(err)) + } + } + + for _, closer := range jrse.closers { + if err := closer(); err != nil { + jrse.telemetry.Logger.Error("error while shutting down the sampling store", zap.Error(err)) + } + } + + return nil } diff --git a/extension/jaegerremotesampling/extension_test.go b/extension/jaegerremotesampling/extension_test.go index 16675a6d07abd..747ba84c01f40 100644 --- a/extension/jaegerremotesampling/extension_test.go +++ b/extension/jaegerremotesampling/extension_test.go @@ -17,6 +17,7 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component/componenttest" "go.opentelemetry.io/collector/config/configgrpc" + "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/config/configopaque" "go.opentelemetry.io/collector/config/configoptional" "go.opentelemetry.io/collector/config/configtls" @@ -99,10 +100,12 @@ func TestRemote(t *testing.T) { // create the config, pointing to the mock server cfg := testConfig() - - grpcCfg := cfg.GRPCServerConfig.Get() - grpcCfg.NetAddr.Endpoint = "127.0.0.1:0" - cfg.GRPCServerConfig = configoptional.Some(*grpcCfg) + cfg.GRPCServerConfig = configoptional.Some(configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "127.0.0.1:0", + Transport: confignet.TransportTypeTCP, + }, + }) cfg.Source.ReloadInterval = tc.reloadInterval cfg.Source.Remote = &configgrpc.ClientConfig{ Endpoint: fmt.Sprintf("127.0.0.1:%d", lis.Addr().(*net.TCPAddr).Port), @@ -169,12 +172,12 @@ func (s *samplingServer) GetSamplingStrategy(ctx context.Context, params *api_v2 func testConfig() *Config { cfg := createDefaultConfig().(*Config) cfg.HTTPServerConfig.NetAddr.Endpoint = "127.0.0.1:5778" - -if cfg.GRPCServerConfig.HasValue() { - grpcCfg := cfg.GRPCServerConfig.Get() - grpcCfg.NetAddr.Endpoint = "127.0.0.1:0" - cfg.GRPCServerConfig = configoptional.Some(*grpcCfg) -} + cfg.GRPCServerConfig = configoptional.Some(configgrpc.ServerConfig{ + NetAddr: confignet.AddrConfig{ + Endpoint: "127.0.0.1:14250", + Transport: confignet.TransportTypeTCP, + }, + }) return cfg } diff --git a/extension/jaegerremotesampling/factory.go b/extension/jaegerremotesampling/factory.go index 43c96f349cecf..860d327f40bee 100644 --- a/extension/jaegerremotesampling/factory.go +++ b/extension/jaegerremotesampling/factory.go @@ -9,9 +9,9 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configgrpc" - "go.opentelemetry.io/collector/config/configoptional" "go.opentelemetry.io/collector/config/confighttp" "go.opentelemetry.io/collector/config/confignet" + "go.opentelemetry.io/collector/config/configoptional" "go.opentelemetry.io/collector/extension" "go.uber.org/zap" @@ -37,13 +37,8 @@ func createDefaultConfig() component.Config { Transport: confignet.TransportTypeTCP, }, }, - GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: testutil.EndpointForPort(14250), - Transport: confignet.TransportTypeTCP, - }, - }), - Source: Source{}, + GRPCServerConfig: configoptional.None[configgrpc.ServerConfig](), + Source: Source{}, } } diff --git a/extension/jaegerremotesampling/factory_test.go b/extension/jaegerremotesampling/factory_test.go index 35c5d1fdddaf5..f3c7aa1754167 100644 --- a/extension/jaegerremotesampling/factory_test.go +++ b/extension/jaegerremotesampling/factory_test.go @@ -9,9 +9,9 @@ import ( "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/component/componenttest" "go.opentelemetry.io/collector/config/configgrpc" - "go.opentelemetry.io/collector/config/configoptional" "go.opentelemetry.io/collector/config/confighttp" "go.opentelemetry.io/collector/config/confignet" + "go.opentelemetry.io/collector/config/configoptional" "go.opentelemetry.io/collector/extension/extensiontest" ) @@ -22,10 +22,7 @@ func TestCreateDefaultConfig(t *testing.T) { Endpoint: "localhost:5778", Transport: confignet.TransportTypeTCP, }}, - GRPCServerConfig: configoptional.Some(configgrpc.ServerConfig{NetAddr: confignet.AddrConfig{ - Endpoint: "localhost:14250", - Transport: confignet.TransportTypeTCP, - }}), + GRPCServerConfig: configoptional.None[configgrpc.ServerConfig](), } // test From 9d0a2db74b7c28e5ef43d028030cd6400b51edf1 Mon Sep 17 00:00:00 2001 From: Anshul Jagota Date: Sun, 17 May 2026 00:46:26 +0530 Subject: [PATCH 8/8] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../jaegerremotesampling-port-validation.yaml | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/.chloggen/jaegerremotesampling-port-validation.yaml b/.chloggen/jaegerremotesampling-port-validation.yaml index 14177b08edecc..c8d09e7ed0460 100644 --- a/.chloggen/jaegerremotesampling-port-validation.yaml +++ b/.chloggen/jaegerremotesampling-port-validation.yaml @@ -1,11 +1,16 @@ -change_type: enhancement +change_type: breaking component: jaegerremotesampling -note: Add validation to detect port conflicts between HTTP and gRPC servers +note: Require explicit `grpc:` configuration to keep the default gRPC sampling endpoint and add HTTP/gRPC port conflict validation issues: [45944] subtext: | The jaegerremotesampling extension now validates that HTTP and gRPC servers - are configured on different ports during configuration loading. This prevents - runtime port binding failures and provides clear, actionable error messages - when misconfigurations are detected. + are configured on different ports during configuration loading, preventing + runtime port binding failures and providing clear, actionable error messages + for misconfigurations. + + This change is breaking: the extension no longer starts a gRPC server on + port 14250 by default when `grpc:` is omitted from the YAML configuration. + Users who relied on the previous default must now configure `grpc:` + explicitly to keep the gRPC sampling endpoint.