Skip to content

Commit

Permalink
[extension/observer/docker, receiver/dockerstats] Add hint about api_…
Browse files Browse the repository at this point in the history
…version (#34043)

**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.-->

Adds a hint like this to `api_version`:
```
Hint: You may want to wrap the 'api_version' value in quotes (api_version: "1.25")
```

**Testing:**
<details>
<summary>docker_stats config and output</summary>

#### Config

```yaml
receivers:
  docker_stats:
    collection_interval: 2s
    api_version: 1.25 # Floating value

exporters:
  nop:

service:
  pipelines:
    metrics:
      receivers: [docker_stats]
      exporters: [nop]
```

#### Output
```
Error: failed to get config: cannot unmarshal the configuration: 1 error(s) decoding:

* error decoding 'receivers': error reading configuration for "docker_stats": 1 error(s) decoding:

* 'api_version' expected type 'string', got unconvertible type 'float64', value: '1.25'.

Hint: You may want to wrap the 'api_version' value in quotes (api_version: "1.25")

Hint: Temporarily restore the previous behavior by disabling 
      the `confmap.strictlyTypedInput` feature gate. More details at:
      open-telemetry/opentelemetry-collector#10552
```


</details>


<details>
<summary>docker_observer config and output</summary>

```yaml
receivers:
  nop:

exporters:
  nop:

extensions:
  docker_observer:
    api_version: 1.25

service:
  extensions: [docker_observer]
  pipelines:
    metrics:
      receivers: [nop]
      exporters: [nop]
```

```
Error: failed to get config: cannot unmarshal the configuration: 1 error(s) decoding:

* error decoding 'extensions': error reading configuration for "docker_observer": 1 error(s) decoding:

* 'api_version' expected type 'string', got unconvertible type 'float64', value: '1.25'.

Hint: You may want to wrap the 'api_version' value in quotes (api_version: "1.25")

Hint: Temporarily restore the previous behavior by disabling 
      the `confmap.strictlyTypedInput` feature gate. More details at:
      open-telemetry/opentelemetry-collector#10552
```

</details>
  • Loading branch information
mx-psi committed Jul 15, 2024
1 parent d6b462b commit 6279b7e
Show file tree
Hide file tree
Showing 10 changed files with 146 additions and 11 deletions.
27 changes: 27 additions & 0 deletions .chloggen/mx-psi_api-version-explicit-error-observer.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

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

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

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Add hint to error when using float for `api_version` field

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

# (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:

# 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: []
27 changes: 27 additions & 0 deletions .chloggen/mx-psi_api-version-explicit-error.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

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

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

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Add hint to error when using float for `api_version` field

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

# (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:

# 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: []
17 changes: 17 additions & 0 deletions extension/observer/dockerobserver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"fmt"
"time"

"go.opentelemetry.io/collector/confmap"

"github.com/open-telemetry/opentelemetry-collector-contrib/internal/docker"
)

Expand Down Expand Up @@ -64,3 +66,18 @@ func (config Config) Validate() error {
}
return nil
}

func (config *Config) Unmarshal(conf *confmap.Conf) error {
err := conf.Unmarshal(config)
if err != nil {
if floatAPIVersion, ok := conf.Get("api_version").(float64); ok {
return fmt.Errorf(
"%w.\n\nHint: You may want to wrap the 'api_version' value in quotes (api_version: \"%1.2f\")",
err,
floatAPIVersion,
)
}
return err
}
return nil
}
30 changes: 25 additions & 5 deletions extension/observer/dockerobserver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/confmap"
"go.opentelemetry.io/collector/confmap/confmaptest"

"github.com/open-telemetry/opentelemetry-collector-contrib/extension/observer/dockerobserver/internal/metadata"
Expand Down Expand Up @@ -74,14 +75,33 @@ func TestValidateConfig(t *testing.T) {
assert.Nil(t, component.ValidateConfig(cfg))
}

func loadConfig(t testing.TB, id component.ID) *Config {
cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config.yaml"))
func loadConf(t testing.TB, path string, id component.ID) *confmap.Conf {
cm, err := confmaptest.LoadConf(filepath.Join("testdata", path))
require.NoError(t, err)
factory := NewFactory()
cfg := factory.CreateDefaultConfig()
sub, err := cm.Sub(id.String())
require.NoError(t, err)
require.NoError(t, sub.Unmarshal(cfg))
return sub
}

func loadConfig(t testing.TB, id component.ID) *Config {
factory := NewFactory()
cfg := factory.CreateDefaultConfig()
sub := loadConf(t, "config.yaml", id)
require.NoError(t, sub.Unmarshal(cfg))
return cfg.(*Config)
}

func TestApiVersionCustomError(t *testing.T) {
sub := loadConf(t, "api_version_float.yaml", component.NewID(metadata.Type))
factory := NewFactory()
cfg := factory.CreateDefaultConfig()
err := sub.Unmarshal(cfg)
require.Error(t, err)
assert.Contains(t, err.Error(),
`Hint: You may want to wrap the 'api_version' value in quotes (api_version: "1.40")`,
)

sub = loadConf(t, "api_version_string.yaml", component.NewID(metadata.Type))
err = sub.Unmarshal(cfg)
require.NoError(t, err)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
docker_observer:
api_version: 1.40
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
docker_observer:
api_version: "1.40"
17 changes: 17 additions & 0 deletions receiver/dockerstatsreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ package dockerstatsreceiver // import "github.com/open-telemetry/opentelemetry-c

import (
"errors"
"fmt"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/confmap"
"go.opentelemetry.io/collector/receiver/scraperhelper"

"github.com/open-telemetry/opentelemetry-collector-contrib/internal/docker"
Expand Down Expand Up @@ -54,3 +56,18 @@ func (config Config) Validate() error {
}
return nil
}

func (config *Config) Unmarshal(conf *confmap.Conf) error {
err := conf.Unmarshal(config)
if err != nil {
if floatAPIVersion, ok := conf.Get("api_version").(float64); ok {
return fmt.Errorf(
"%w.\n\nHint: You may want to wrap the 'api_version' value in quotes (api_version: \"%1.2f\")",
err,
floatAPIVersion,
)
}
return err
}
return nil
}
31 changes: 25 additions & 6 deletions receiver/dockerstatsreceiver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,21 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/confmap"
"go.opentelemetry.io/collector/confmap/confmaptest"
"go.opentelemetry.io/collector/receiver/scraperhelper"

"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/dockerstatsreceiver/internal/metadata"
)

func loadConf(t testing.TB, path string, id component.ID) *confmap.Conf {
cm, err := confmaptest.LoadConf(filepath.Join("testdata", path))
require.NoError(t, err)
sub, err := cm.Sub(id.String())
require.NoError(t, err)
return sub
}

func TestLoadConfig(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -72,14 +81,9 @@ func TestLoadConfig(t *testing.T) {

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)

sub := loadConf(t, "config.yaml", tt.id)
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, component.ValidateConfig(cfg))
Expand Down Expand Up @@ -108,3 +112,18 @@ func TestValidateErrors(t *testing.T) {
}
assert.Equal(t, `"collection_interval": requires positive value`, component.ValidateConfig(cfg).Error())
}

func TestApiVersionCustomError(t *testing.T) {
sub := loadConf(t, "api_version_float.yaml", component.NewID(metadata.Type))
factory := NewFactory()
cfg := factory.CreateDefaultConfig()
err := sub.Unmarshal(cfg)
require.Error(t, err)
assert.Contains(t, err.Error(),
`Hint: You may want to wrap the 'api_version' value in quotes (api_version: "1.40")`,
)

sub = loadConf(t, "api_version_string.yaml", component.NewID(metadata.Type))
err = sub.Unmarshal(cfg)
require.NoError(t, err)
}
2 changes: 2 additions & 0 deletions receiver/dockerstatsreceiver/testdata/api_version_float.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
docker_stats:
api_version: 1.40
2 changes: 2 additions & 0 deletions receiver/dockerstatsreceiver/testdata/api_version_string.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
docker_stats:
api_version: "1.40"

0 comments on commit 6279b7e

Please sign in to comment.