Skip to content

[receiver/jaeger] Removing remote sampling configuration code #24186

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Aug 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions .chloggen/disable_jaeger_receiver_remote_sampling.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: jaegerreceiver

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Deprecate remote_sampling config in the jaeger receiver

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [24186]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: >
The jaeger receiver will fail to start if remote_sampling config is specified in it.
The `receiver.jaeger.DisableRemoteSampling` feature gate can be set to let the receiver start and treat
remote_sampling config as no-op. In a future version this feature gate will be removed and the receiver will always
fail when remote_sampling config is specified.

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [user, api]
2 changes: 1 addition & 1 deletion extension/jaegerremotesampling/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"github.com/open-telemetry/opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/metadata"
)

// NewFactory creates a factory for the OIDC Authenticator extension.
// NewFactory creates a factory for the jaeger remote sampling extension.
func NewFactory() extension.Factory {
return extension.NewFactory(
metadata.Type,
Expand Down
2 changes: 1 addition & 1 deletion extension/oauth2clientauthextension/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
"github.com/open-telemetry/opentelemetry-collector-contrib/extension/oauth2clientauthextension/internal/metadata"
)

// NewFactory creates a factory for the OIDC Authenticator extension.
// NewFactory creates a factory for the oauth2 client Authenticator extension.
func NewFactory() extension.Factory {
return extension.NewFactory(
metadata.Type,
Expand Down
12 changes: 2 additions & 10 deletions receiver/jaegerreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,16 +109,8 @@ func (cfg *Config) Validate() error {
}

if cfg.RemoteSampling != nil {
if err := checkPortFromEndpoint(cfg.RemoteSampling.HostEndpoint); err != nil {
return fmt.Errorf("invalid port number for the Remote Sampling endpoint: %w", err)
}

if len(cfg.RemoteSampling.StrategyFile) != 0 && cfg.GRPC == nil {
return fmt.Errorf("strategy file requires the gRPC protocol to be enabled")
}

if cfg.RemoteSampling.StrategyFileReloadInterval < 0 {
return fmt.Errorf("strategy file reload interval should be great or equal zero")
if disableJaegerReceiverRemoteSampling.IsEnabled() {
return fmt.Errorf("remote sampling config detected in the Jaeger receiver; use the `jaegerremotesampling` extension instead")
}
}

Expand Down
49 changes: 0 additions & 49 deletions receiver/jaegerreceiver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package jaegerreceiver
import (
"path/filepath"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -62,14 +61,6 @@ func TestLoadConfig(t *testing.T) {
},
},
},
RemoteSampling: &RemoteSamplingConfig{
HostEndpoint: "0.0.0.0:5778",
GRPCClientSettings: configgrpc.GRPCClientSettings{
Endpoint: "jaeger-collector:1234",
},
StrategyFile: "/etc/strategies.json",
StrategyFileReloadInterval: time.Second * 10,
},
},
},
{
Expand Down Expand Up @@ -207,15 +198,6 @@ func TestInvalidConfig(t *testing.T) {
},
err: "receiver creation with no port number for Thrift UDP - Binary must fail",
},
{
desc: "remote-sampling-http-no-port",
apply: func(cfg *Config) {
cfg.RemoteSampling = &RemoteSamplingConfig{
HostEndpoint: "localhost:",
}
},
err: "receiver creation with no port number for the remote sampling HTTP endpoint must fail",
},
{
desc: "grpc-invalid-host",
apply: func(cfg *Config) {
Expand Down Expand Up @@ -244,37 +226,6 @@ func TestInvalidConfig(t *testing.T) {
},
err: "receiver creation with too large port number must fail",
},
{
desc: "port-outside-of-range",
apply: func(cfg *Config) {
cfg.Protocols = Protocols{}
cfg.ThriftCompact = &ProtocolUDP{
Endpoint: defaultThriftCompactBindEndpoint,
}
cfg.RemoteSampling = &RemoteSamplingConfig{
HostEndpoint: "localhost:5778",
StrategyFile: "strategies.json",
}
},
err: "receiver creation without gRPC and with remote sampling config",
},
{
desc: "reload-interval-outside-of-range",
apply: func(cfg *Config) {
cfg.Protocols.GRPC = &configgrpc.GRPCServerSettings{
NetAddr: confignet.NetAddr{
Endpoint: "1234",
Transport: "tcp",
},
}
cfg.RemoteSampling = &RemoteSamplingConfig{
HostEndpoint: "localhost:5778",
StrategyFile: "strategies.json",
StrategyFileReloadInterval: -time.Second,
}
},
err: "strategy file reload interval should be great zero",
},
}
for _, tC := range testCases {
t.Run(tC.desc, func(t *testing.T) {
Expand Down
11 changes: 11 additions & 0 deletions receiver/jaegerreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/confignet"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/featuregate"
"go.opentelemetry.io/collector/receiver"

"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/jaegerreceiver/internal/metadata"
Expand All @@ -32,6 +33,12 @@ const (
defaultThriftBinaryBindEndpoint = "0.0.0.0:6832"
)

var disableJaegerReceiverRemoteSampling = featuregate.GlobalRegistry().MustRegister(
"receiver.jaeger.DisableRemoteSampling",
featuregate.StageBeta,
featuregate.WithRegisterDescription("When enabled, the Jaeger Receiver will fail to start when it is configured with remote_sampling config. When disabled, the receiver will start and the remote_sampling config will be no-op."),
)

// NewFactory creates a new Jaeger receiver factory.
func NewFactory() receiver.Factory {
return receiver.NewFactory(
Expand Down Expand Up @@ -97,6 +104,10 @@ func createTracesReceiver(
config.AgentCompactThrift = *rCfg.ThriftCompact
}

if rCfg.RemoteSampling != nil {
set.Logger.Warn("You are using a deprecated no-op `remote_sampling` option which will be removed soon; use a `jaegerremotesampling` extension instead")
}

// Create the receiver.
return newJaegerReceiver(set.ID, &config, nextConsumer, set)
}
2 changes: 1 addition & 1 deletion receiver/jaegerreceiver/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ require (
go.opentelemetry.io/collector/config/configtls v0.83.0
go.opentelemetry.io/collector/confmap v0.83.0
go.opentelemetry.io/collector/consumer v0.83.0
go.opentelemetry.io/collector/featuregate v1.0.0-rcv0014
go.opentelemetry.io/collector/pdata v1.0.0-rcv0014
go.opentelemetry.io/collector/receiver v0.83.0
go.opentelemetry.io/collector/semconv v0.83.0
Expand Down Expand Up @@ -60,7 +61,6 @@ require (
go.opentelemetry.io/collector/exporter v0.83.0 // indirect
go.opentelemetry.io/collector/extension v0.83.0 // indirect
go.opentelemetry.io/collector/extension/auth v0.83.0 // indirect
go.opentelemetry.io/collector/featuregate v1.0.0-rcv0014 // indirect
go.opentelemetry.io/collector/processor v0.83.0 // indirect
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.42.1-0.20230612162650-64be7e574a17 // indirect
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.42.0 // indirect
Expand Down
5 changes: 0 additions & 5 deletions receiver/jaegerreceiver/testdata/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ jaeger/customname:
max_packet_size: 65_536
workers: 5
socket_buffer_size: 0
remote_sampling:
host_endpoint: "0.0.0.0:5778"
endpoint: "jaeger-collector:1234"
strategy_file: "/etc/strategies.json"
strategy_file_reload_interval: 10s
# The following demonstrates how to enable protocols with defaults.
jaeger/defaults:
protocols:
Expand Down