Skip to content

Commit

Permalink
[exporter/opensearch] chore: remove redundant config Validate call (#…
Browse files Browse the repository at this point in the history
…35233)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
Configuration validation is done during collector's startup, making it
redundant when being called inside component's logic. This PR removes
the Validate call done during exporter's constructor.

**Link to tracking Issue:** <Issue number if applicable>
#33498
(Last component, will close the issue)

**Testing:** <Describe what testing was performed and which tests were
added.> Added default config use case (validate error)

**Documentation:** <Describe the documentation added.>
  • Loading branch information
rogercoll authored Oct 18, 2024
1 parent 2c34918 commit d5c641a
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 46 deletions.
7 changes: 7 additions & 0 deletions exporter/opensearchexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ func TestLoadConfig(t *testing.T) {
expected: sampleCfg,
configValidateAssert: assert.NoError,
},
{
id: component.NewIDWithName(metadata.Type, "default"),
expected: withDefaultConfig(),
configValidateAssert: func(t assert.TestingT, err error, _ ...any) bool {
return assert.ErrorContains(t, err, "endpoint must be specified")
},
},
{
id: component.NewIDWithName(metadata.Type, "trace"),
expected: &Config{
Expand Down
16 changes: 6 additions & 10 deletions exporter/opensearchexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,10 @@ func newDefaultConfig() component.Config {

func createTracesExporter(ctx context.Context,
set exporter.Settings,
cfg component.Config) (exporter.Traces, error) {
cfg component.Config,
) (exporter.Traces, error) {
c := cfg.(*Config)
te, e := newSSOTracesExporter(c, set)
if e != nil {
return nil, e
}
te := newSSOTracesExporter(c, set)

return exporterhelper.NewTracesExporter(ctx, set, cfg,
te.pushTraceData,
Expand All @@ -58,12 +56,10 @@ func createTracesExporter(ctx context.Context,

func createLogsExporter(ctx context.Context,
set exporter.Settings,
cfg component.Config) (exporter.Logs, error) {
cfg component.Config,
) (exporter.Logs, error) {
c := cfg.(*Config)
le, e := newLogExporter(c, set)
if e != nil {
return nil, e
}
le := newLogExporter(c, set)

return exporterhelper.NewLogsExporter(ctx, set, cfg,
le.pushLogData,
Expand Down
24 changes: 0 additions & 24 deletions exporter/opensearchexporter/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,6 @@ func TestCreateDefaultConfig(t *testing.T) {
assert.NoError(t, componenttest.CheckConfigStruct(cfg))
}

func TestFactory_CreateMetricsExporter_Fail(t *testing.T) {
factory := NewFactory()
cfg := factory.CreateDefaultConfig()
params := exportertest.NewNopSettings()
_, err := factory.CreateMetricsExporter(context.Background(), params, cfg)
require.Error(t, err, "expected an error when creating a metrics exporter")
}

func TestFactory_CreateTracesExporter_Fail(t *testing.T) {
factory := NewFactory()
cfg := factory.CreateDefaultConfig()
params := exportertest.NewNopSettings()
_, err := factory.CreateTracesExporter(context.Background(), params, cfg)
require.Error(t, err, "expected an error when creating a traces exporter")
}

func TestFactory_CreateLogsExporter_Fail(t *testing.T) {
factory := NewFactory()
cfg := factory.CreateDefaultConfig()
params := exportertest.NewNopSettings()
_, err := factory.CreateLogsExporter(context.Background(), params, cfg)
require.Error(t, err, "expected an error when creating a logs exporter")
}

func TestFactory_CreateTracesExporter(t *testing.T) {
factory := NewFactory()
cfg := withDefaultConfig(func(cfg *Config) {
Expand Down
8 changes: 2 additions & 6 deletions exporter/opensearchexporter/sso_log_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,7 @@ type logExporter struct {
telemetry component.TelemetrySettings
}

func newLogExporter(cfg *Config, set exporter.Settings) (*logExporter, error) {
if err := cfg.Validate(); err != nil {
return nil, err
}

func newLogExporter(cfg *Config, set exporter.Settings) *logExporter {
model := &encodeModel{
dedup: cfg.Dedup,
dedot: cfg.Dedot,
Expand All @@ -45,7 +41,7 @@ func newLogExporter(cfg *Config, set exporter.Settings) (*logExporter, error) {
bulkAction: cfg.BulkAction,
httpSettings: cfg.ClientConfig,
model: model,
}, nil
}
}

func (l *logExporter) Start(ctx context.Context, host component.Host) error {
Expand Down
8 changes: 2 additions & 6 deletions exporter/opensearchexporter/sso_trace_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,7 @@ type ssoTracesExporter struct {
telemetry component.TelemetrySettings
}

func newSSOTracesExporter(cfg *Config, set exporter.Settings) (*ssoTracesExporter, error) {
if err := cfg.Validate(); err != nil {
return nil, err
}

func newSSOTracesExporter(cfg *Config, set exporter.Settings) *ssoTracesExporter {
model := &encodeModel{
dataset: cfg.Dataset,
namespace: cfg.Namespace,
Expand All @@ -42,7 +38,7 @@ func newSSOTracesExporter(cfg *Config, set exporter.Settings) (*ssoTracesExporte
bulkAction: cfg.BulkAction,
model: model,
httpSettings: cfg.ClientConfig,
}, nil
}
}

func (s *ssoTracesExporter) Start(ctx context.Context, host component.Host) error {
Expand Down
2 changes: 2 additions & 0 deletions exporter/opensearchexporter/testdata/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ opensearch:
http:
endpoint: https://opensearch.example.com:9200

opensearch/default:

opensearch/empty_namespace:
dataset: ngnix
namespace: ""
Expand Down

0 comments on commit d5c641a

Please sign in to comment.