Skip to content

Commit

Permalink
[receiver/podman] remove config Validate call on start (open-telemetr…
Browse files Browse the repository at this point in the history
…y#33555)

**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 Podman's receiver start function.

**Link to tracking Issue:** <Issue number if applicable>

open-telemetry#33498

Context:
https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/32981/files#r1621849257

**Testing:** <Describe what testing was performed and which tests were
added.>
Tests moved to the `config_test.go` file following the testdata
strategy.

**Documentation:** <Describe the documentation added.>

---------

Co-authored-by: Andrzej Stencel <[email protected]>
  • Loading branch information
rogercoll and andrzej-stencel authored Jul 11, 2024
1 parent 16a7b29 commit 02d5636
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 15 deletions.
19 changes: 16 additions & 3 deletions receiver/podmanreceiver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ func TestLoadConfig(t *testing.T) {
require.NoError(t, err)

tests := []struct {
id component.ID
expected component.Config
expectedErr error
id component.ID
expected component.Config
expectedErrMsg string
}{
{
id: component.NewIDWithName(metadata.Type, ""),
Expand Down Expand Up @@ -54,6 +54,14 @@ func TestLoadConfig(t *testing.T) {
MetricsBuilderConfig: metadata.DefaultMetricsBuilderConfig(),
},
},
{
id: component.NewIDWithName(metadata.Type, "empty_endpoint"),
expectedErrMsg: "config.Endpoint must be specified",
},
{
id: component.NewIDWithName(metadata.Type, "invalid_collection_interval"),
expectedErrMsg: `config.CollectionInterval must be specified; "collection_interval": requires positive value`,
},
}

for _, tt := range tests {
Expand All @@ -65,6 +73,11 @@ func TestLoadConfig(t *testing.T) {
require.NoError(t, err)
require.NoError(t, sub.Unmarshal(cfg))

if tt.expectedErrMsg != "" {
assert.EqualError(t, component.ValidateConfig(cfg), tt.expectedErrMsg)
return
}

assert.NoError(t, component.ValidateConfig(cfg))
assert.Equal(t, tt.expected, cfg)
})
Expand Down
5 changes: 0 additions & 5 deletions receiver/podmanreceiver/receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,6 @@ func createMetricsReceiver(
}

func (r *metricsReceiver) start(ctx context.Context, _ component.Host) error {
err := r.config.Validate()
if err != nil {
return err
}

podmanClient, err := r.clientFactory(r.set.Logger, r.config)
if err != nil {
return err
Expand Down
8 changes: 1 addition & 7 deletions receiver/podmanreceiver/receiver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,7 @@ func TestErrorsInStart(t *testing.T) {
assert.NotNil(t, recv)
err := recv.start(context.Background(), componenttest.NewNopHost())
require.Error(t, err)
assert.Equal(t, "config.Endpoint must be specified", err.Error())

recv = newMetricsReceiver(receivertest.NewNopSettings(), &Config{Endpoint: "someEndpoint"}, nil)
assert.NotNil(t, recv)
err = recv.start(context.Background(), componenttest.NewNopHost())
require.Error(t, err)
assert.Equal(t, "config.CollectionInterval must be specified", err.Error())
assert.Equal(t, `unable to create connection. "" is not a supported schema`, err.Error())
}

func TestScraperLoop(t *testing.T) {
Expand Down
4 changes: 4 additions & 0 deletions receiver/podmanreceiver/testdata/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,7 @@ podman_stats/all:
endpoint: http://example.com/
collection_interval: 2s
timeout: 20s
podman_stats/empty_endpoint:
endpoint: ""
podman_stats/invalid_collection_interval:
collection_interval: 0s

0 comments on commit 02d5636

Please sign in to comment.