From 6279b7e9eba86740c7a1722e8841f0f99bad7a91 Mon Sep 17 00:00:00 2001 From: Pablo Baeyens Date: Mon, 15 Jul 2024 13:07:01 +0200 Subject: [PATCH] [extension/observer/docker, receiver/dockerstats] Add hint about api_version (#34043) **Description:** 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:**
docker_stats config and output #### 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: https://github.com/open-telemetry/opentelemetry-collector/issues/10552 ```
docker_observer config and output ```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: https://github.com/open-telemetry/opentelemetry-collector/issues/10552 ```
--- ...i_api-version-explicit-error-observer.yaml | 27 ++++++++++++++++ .../mx-psi_api-version-explicit-error.yaml | 27 ++++++++++++++++ extension/observer/dockerobserver/config.go | 17 ++++++++++ .../observer/dockerobserver/config_test.go | 30 +++++++++++++++--- .../testdata/api_version_float.yaml | 2 ++ .../testdata/api_version_string.yaml | 2 ++ receiver/dockerstatsreceiver/config.go | 17 ++++++++++ receiver/dockerstatsreceiver/config_test.go | 31 +++++++++++++++---- .../testdata/api_version_float.yaml | 2 ++ .../testdata/api_version_string.yaml | 2 ++ 10 files changed, 146 insertions(+), 11 deletions(-) create mode 100644 .chloggen/mx-psi_api-version-explicit-error-observer.yaml create mode 100644 .chloggen/mx-psi_api-version-explicit-error.yaml create mode 100644 extension/observer/dockerobserver/testdata/api_version_float.yaml create mode 100644 extension/observer/dockerobserver/testdata/api_version_string.yaml create mode 100644 receiver/dockerstatsreceiver/testdata/api_version_float.yaml create mode 100644 receiver/dockerstatsreceiver/testdata/api_version_string.yaml diff --git a/.chloggen/mx-psi_api-version-explicit-error-observer.yaml b/.chloggen/mx-psi_api-version-explicit-error-observer.yaml new file mode 100644 index 000000000000..798b2ed869d4 --- /dev/null +++ b/.chloggen/mx-psi_api-version-explicit-error-observer.yaml @@ -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: [] diff --git a/.chloggen/mx-psi_api-version-explicit-error.yaml b/.chloggen/mx-psi_api-version-explicit-error.yaml new file mode 100644 index 000000000000..ac05a8cae544 --- /dev/null +++ b/.chloggen/mx-psi_api-version-explicit-error.yaml @@ -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: [] diff --git a/extension/observer/dockerobserver/config.go b/extension/observer/dockerobserver/config.go index 743a7cd9025a..4e3bb293beae 100644 --- a/extension/observer/dockerobserver/config.go +++ b/extension/observer/dockerobserver/config.go @@ -8,6 +8,8 @@ import ( "fmt" "time" + "go.opentelemetry.io/collector/confmap" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/docker" ) @@ -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 +} diff --git a/extension/observer/dockerobserver/config_test.go b/extension/observer/dockerobserver/config_test.go index 768cf8aa1a47..cab7d3342ede 100644 --- a/extension/observer/dockerobserver/config_test.go +++ b/extension/observer/dockerobserver/config_test.go @@ -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" @@ -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) +} diff --git a/extension/observer/dockerobserver/testdata/api_version_float.yaml b/extension/observer/dockerobserver/testdata/api_version_float.yaml new file mode 100644 index 000000000000..18f512e32e41 --- /dev/null +++ b/extension/observer/dockerobserver/testdata/api_version_float.yaml @@ -0,0 +1,2 @@ +docker_observer: + api_version: 1.40 diff --git a/extension/observer/dockerobserver/testdata/api_version_string.yaml b/extension/observer/dockerobserver/testdata/api_version_string.yaml new file mode 100644 index 000000000000..7f83b7ba30f7 --- /dev/null +++ b/extension/observer/dockerobserver/testdata/api_version_string.yaml @@ -0,0 +1,2 @@ +docker_observer: + api_version: "1.40" diff --git a/receiver/dockerstatsreceiver/config.go b/receiver/dockerstatsreceiver/config.go index 86825de8c65e..96085e513a9f 100644 --- a/receiver/dockerstatsreceiver/config.go +++ b/receiver/dockerstatsreceiver/config.go @@ -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" @@ -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 +} diff --git a/receiver/dockerstatsreceiver/config_test.go b/receiver/dockerstatsreceiver/config_test.go index 490f0d2f2eb7..4ce59bfea1b7 100644 --- a/receiver/dockerstatsreceiver/config_test.go +++ b/receiver/dockerstatsreceiver/config_test.go @@ -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() @@ -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)) @@ -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) +} diff --git a/receiver/dockerstatsreceiver/testdata/api_version_float.yaml b/receiver/dockerstatsreceiver/testdata/api_version_float.yaml new file mode 100644 index 000000000000..3c39b2aaaa89 --- /dev/null +++ b/receiver/dockerstatsreceiver/testdata/api_version_float.yaml @@ -0,0 +1,2 @@ +docker_stats: + api_version: 1.40 diff --git a/receiver/dockerstatsreceiver/testdata/api_version_string.yaml b/receiver/dockerstatsreceiver/testdata/api_version_string.yaml new file mode 100644 index 000000000000..f34ecb350e90 --- /dev/null +++ b/receiver/dockerstatsreceiver/testdata/api_version_string.yaml @@ -0,0 +1,2 @@ +docker_stats: + api_version: "1.40"