Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[otlpexporter] Validate the configuration explicitly #9523

Merged
merged 1 commit into from
Feb 9, 2024
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
9 changes: 9 additions & 0 deletions exporter/otlpexporter/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package otlpexporter // import "go.opentelemetry.io/collector/exporter/otlpexporter"

import (
"errors"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/configretry"
Expand All @@ -19,4 +21,11 @@ type Config struct {
configgrpc.ClientConfig `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct.
}

func (c *Config) Validate() error {
if c.SanitizedEndpoint() == "" {
atoulme marked this conversation as resolved.
Show resolved Hide resolved
return errors.New(`requires a non-empty "endpoint"`)
}
return nil
}

var _ component.Config = (*Config)(nil)
36 changes: 36 additions & 0 deletions exporter/otlpexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,39 @@ func TestUnmarshalConfig(t *testing.T) {
},
}, cfg)
}

func TestUnmarshalInvalidConfig(t *testing.T) {
cm, err := confmaptest.LoadConf(filepath.Join("testdata", "invalid_configs.yaml"))
require.NoError(t, err)
factory := NewFactory()
for _, test := range []struct {
name string
errorMsg string
}{
{
name: "no_endpoint",
errorMsg: `requires a non-empty "endpoint"`,
},
{
name: "http_endpoint",
errorMsg: `requires a non-empty "endpoint"`,
},
{
name: "invalid_timeout",
errorMsg: `'timeout' must be non-negative`,
},
{
name: "invalid_retry",
errorMsg: `'randomization_factor' must be within [0, 1]`,
},
} {
t.Run(test.name, func(t *testing.T) {
cfg := factory.CreateDefaultConfig()
sub, err := cm.Sub(test.name)
require.NoError(t, err)
assert.NoError(t, component.UnmarshalConfig(sub, cfg))
assert.ErrorContains(t, component.ValidateConfig(cfg), test.errorMsg)
})
}

}
15 changes: 3 additions & 12 deletions exporter/otlpexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,7 @@ func createTracesExporter(
set exporter.CreateSettings,
cfg component.Config,
) (exporter.Traces, error) {
oce, err := newExporter(cfg, set)
if err != nil {
return nil, err
}
oce := newExporter(cfg, set)
oCfg := cfg.(*Config)
return exporterhelper.NewTracesExporter(ctx, set, cfg,
oce.pushTraces,
Expand All @@ -68,10 +65,7 @@ func createMetricsExporter(
set exporter.CreateSettings,
cfg component.Config,
) (exporter.Metrics, error) {
oce, err := newExporter(cfg, set)
if err != nil {
return nil, err
}
oce := newExporter(cfg, set)
oCfg := cfg.(*Config)
return exporterhelper.NewMetricsExporter(ctx, set, cfg,
oce.pushMetrics,
Expand All @@ -89,10 +83,7 @@ func createLogsExporter(
set exporter.CreateSettings,
cfg component.Config,
) (exporter.Logs, error) {
oce, err := newExporter(cfg, set)
if err != nil {
return nil, err
}
oce := newExporter(cfg, set)
oCfg := cfg.(*Config)
return exporterhelper.NewLogsExporter(ctx, set, cfg,
oce.pushLogs,
Expand Down
20 changes: 3 additions & 17 deletions exporter/otlpexporter/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,10 @@ func TestCreateMetricsExporter(t *testing.T) {
func TestCreateTracesExporter(t *testing.T) {
endpoint := testutil.GetAvailableLocalAddress(t)
tests := []struct {
name string
config *Config
mustFailOnCreate bool
mustFailOnStart bool
name string
config *Config
mustFailOnStart bool
}{
{
name: "NoEndpoint",
config: &Config{
ClientConfig: configgrpc.ClientConfig{
Endpoint: "",
},
},
mustFailOnCreate: true,
},
{
name: "UseSecure",
config: &Config{
Expand Down Expand Up @@ -178,10 +168,6 @@ func TestCreateTracesExporter(t *testing.T) {
factory := NewFactory()
set := exportertest.NewNopCreateSettings()
consumer, err := factory.CreateTracesExporter(context.Background(), set, tt.config)
if tt.mustFailOnCreate {
assert.NotNil(t, err)
return
}
assert.NoError(t, err)
assert.NotNil(t, consumer)
err = consumer.Start(context.Background(), componenttest.NewNopHost())
Expand Down
9 changes: 2 additions & 7 deletions exporter/otlpexporter/otlp.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package otlpexporter // import "go.opentelemetry.io/collector/exporter/otlpexpor

import (
"context"
"errors"
"fmt"
"runtime"
"time"
Expand Down Expand Up @@ -47,17 +46,13 @@ type baseExporter struct {
userAgent string
}

func newExporter(cfg component.Config, set exporter.CreateSettings) (*baseExporter, error) {
func newExporter(cfg component.Config, set exporter.CreateSettings) *baseExporter {
oCfg := cfg.(*Config)

if oCfg.Endpoint == "" {
return nil, errors.New("OTLP exporter config requires an Endpoint")
}

userAgent := fmt.Sprintf("%s/%s (%s/%s)",
set.BuildInfo.Description, set.BuildInfo.Version, runtime.GOOS, runtime.GOARCH)

return &baseExporter{config: oCfg, settings: set.TelemetrySettings, userAgent: userAgent}, nil
return &baseExporter{config: oCfg, settings: set.TelemetrySettings, userAgent: userAgent}
}

// start actually creates the gRPC connection. The client construction is deferred till this point as this
Expand Down
55 changes: 55 additions & 0 deletions exporter/otlpexporter/testdata/invalid_configs.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
no_endpoint:
timeout: 10s
sending_queue:
enabled: true
num_consumers: 2
queue_size: 10
retry_on_failure:
enabled: true
initial_interval: 10s
randomization_factor: 0.7
multiplier: 1.3
max_interval: 60s
max_elapsed_time: 10m
http_endpoint:
endpoint: http://
timeout: 10s
sending_queue:
enabled: true
num_consumers: 2
queue_size: 10
retry_on_failure:
enabled: true
initial_interval: 10s
randomization_factor: 0.7
multiplier: 1.3
max_interval: 60s
max_elapsed_time: 10m
invalid_timeout:
endpoint: example.com:443
timeout: -5s
sending_queue:
enabled: true
num_consumers: 2
queue_size: 10
retry_on_failure:
enabled: true
initial_interval: 10s
randomization_factor: 0.7
multiplier: 1.3
max_interval: 60s
max_elapsed_time: 10m
invalid_retry:
endpoint: example.com:443
timeout: 30s
sending_queue:
enabled: true
num_consumers: 2
queue_size: 10
retry_on_failure:
enabled: true
initial_interval: 10s
randomization_factor: -5
multiplier: 1.3
max_interval: 60s
max_elapsed_time: 10m
Loading