[confmap] Add error hint when invalid YAML was passed (#12180)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description <!-- Issue number if applicable --> Add error hint field to surface YAML parsing errors. This only surfaces the error for top-level URIs but it's a step in the right direction. Error when passing a file with contents `[invalid:,` as the config: Before: ``` retrieved value (type=string) cannot be used as a Conf ``` After: ``` retrieved value (type=string) cannot be used as a Conf: assuming string type since contents are not valid YAML: yaml: line 1: did not find expected node content ``` #### Link to tracking issue Updates #12000 <!--Describe what testing was performed and which tests were added.--> #### Testing <!--Describe the documentation added.--> #### Documentation <!--Please delete paragraphs that you did not use before submitting.-->
This commit is contained in:
parent
7c1d6945c1
commit
e7f071d43c
|
|
@ -0,0 +1,26 @@
|
||||||
|
# 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. otlpreceiver)
|
||||||
|
component: confmap
|
||||||
|
|
||||||
|
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
|
||||||
|
note: Surface YAML parsing errors when they happen at the top-level.
|
||||||
|
|
||||||
|
# One or more tracking issues or pull requests related to the change
|
||||||
|
issues: [12180]
|
||||||
|
|
||||||
|
# (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: |
|
||||||
|
This adds context to some instances of the error "retrieved value (type=string) cannot be used as a Conf", which typically happens because of invalid YAML documents
|
||||||
|
|
||||||
|
# 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: []
|
||||||
|
|
@ -100,6 +100,7 @@ type ChangeEvent struct {
|
||||||
// Retrieved holds the result of a call to the Retrieve method of a Provider object.
|
// Retrieved holds the result of a call to the Retrieve method of a Provider object.
|
||||||
type Retrieved struct {
|
type Retrieved struct {
|
||||||
rawConf any
|
rawConf any
|
||||||
|
errorHint error
|
||||||
closeFunc CloseFunc
|
closeFunc CloseFunc
|
||||||
|
|
||||||
stringRepresentation string
|
stringRepresentation string
|
||||||
|
|
@ -107,6 +108,7 @@ type Retrieved struct {
|
||||||
}
|
}
|
||||||
|
|
||||||
type retrievedSettings struct {
|
type retrievedSettings struct {
|
||||||
|
errorHint error
|
||||||
stringRepresentation string
|
stringRepresentation string
|
||||||
isSetString bool
|
isSetString bool
|
||||||
closeFunc CloseFunc
|
closeFunc CloseFunc
|
||||||
|
|
@ -138,6 +140,12 @@ func withStringRepresentation(stringRepresentation string) RetrievedOption {
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func withErrorHint(errorHint error) RetrievedOption {
|
||||||
|
return retrievedOptionFunc(func(settings *retrievedSettings) {
|
||||||
|
settings.errorHint = errorHint
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
// NewRetrievedFromYAML returns a new Retrieved instance that contains the deserialized data from the yaml bytes.
|
// NewRetrievedFromYAML returns a new Retrieved instance that contains the deserialized data from the yaml bytes.
|
||||||
// * yamlBytes the yaml bytes that will be deserialized.
|
// * yamlBytes the yaml bytes that will be deserialized.
|
||||||
// * opts specifies options associated with this Retrieved value, such as CloseFunc.
|
// * opts specifies options associated with this Retrieved value, such as CloseFunc.
|
||||||
|
|
@ -146,7 +154,10 @@ func NewRetrievedFromYAML(yamlBytes []byte, opts ...RetrievedOption) (*Retrieved
|
||||||
if err := yaml.Unmarshal(yamlBytes, &rawConf); err != nil {
|
if err := yaml.Unmarshal(yamlBytes, &rawConf); err != nil {
|
||||||
// If the string is not valid YAML, we try to use it verbatim as a string.
|
// If the string is not valid YAML, we try to use it verbatim as a string.
|
||||||
strRep := string(yamlBytes)
|
strRep := string(yamlBytes)
|
||||||
return NewRetrieved(strRep, append(opts, withStringRepresentation(strRep))...)
|
return NewRetrieved(strRep, append(opts,
|
||||||
|
withStringRepresentation(strRep),
|
||||||
|
withErrorHint(fmt.Errorf("assuming string type since contents are not valid YAML: %w", err)),
|
||||||
|
)...)
|
||||||
}
|
}
|
||||||
|
|
||||||
switch rawConf.(type) {
|
switch rawConf.(type) {
|
||||||
|
|
@ -175,6 +186,7 @@ func NewRetrieved(rawConf any, opts ...RetrievedOption) (*Retrieved, error) {
|
||||||
}
|
}
|
||||||
return &Retrieved{
|
return &Retrieved{
|
||||||
rawConf: rawConf,
|
rawConf: rawConf,
|
||||||
|
errorHint: set.errorHint,
|
||||||
closeFunc: set.closeFunc,
|
closeFunc: set.closeFunc,
|
||||||
stringRepresentation: set.stringRepresentation,
|
stringRepresentation: set.stringRepresentation,
|
||||||
isSetString: set.isSetString,
|
isSetString: set.isSetString,
|
||||||
|
|
@ -188,6 +200,9 @@ func (r *Retrieved) AsConf() (*Conf, error) {
|
||||||
}
|
}
|
||||||
val, ok := r.rawConf.(map[string]any)
|
val, ok := r.rawConf.(map[string]any)
|
||||||
if !ok {
|
if !ok {
|
||||||
|
if r.errorHint != nil {
|
||||||
|
return nil, fmt.Errorf("retrieved value (type=%T) cannot be used as a Conf: %w", r.rawConf, r.errorHint)
|
||||||
|
}
|
||||||
return nil, fmt.Errorf("retrieved value (type=%T) cannot be used as a Conf", r.rawConf)
|
return nil, fmt.Errorf("retrieved value (type=%T) cannot be used as a Conf", r.rawConf)
|
||||||
}
|
}
|
||||||
return NewFromStringMap(val), nil
|
return NewFromStringMap(val), nil
|
||||||
|
|
|
||||||
|
|
@ -134,7 +134,9 @@ func TestNewRetrievedFromYAMLInvalidYAMLBytes(t *testing.T) {
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|
||||||
_, err = ret.AsConf()
|
_, err = ret.AsConf()
|
||||||
require.Error(t, err)
|
require.EqualError(t, err,
|
||||||
|
"retrieved value (type=string) cannot be used as a Conf: assuming string type since contents are not valid YAML: yaml: line 1: did not find expected node content",
|
||||||
|
)
|
||||||
|
|
||||||
str, err := ret.AsString()
|
str, err := ret.AsString()
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|
@ -150,7 +152,7 @@ func TestNewRetrievedFromYAMLInvalidAsMap(t *testing.T) {
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|
||||||
_, err = ret.AsConf()
|
_, err = ret.AsConf()
|
||||||
require.Error(t, err)
|
require.EqualError(t, err, "retrieved value (type=string) cannot be used as a Conf")
|
||||||
|
|
||||||
str, err := ret.AsString()
|
str, err := ret.AsString()
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|
|
||||||
|
|
@ -312,7 +312,7 @@ func TestCollectorStartInvalidConfig(t *testing.T) {
|
||||||
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-invalid.yaml")}),
|
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-invalid.yaml")}),
|
||||||
})
|
})
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
assert.Error(t, col.Run(context.Background()))
|
assert.EqualError(t, col.Run(context.Background()), "invalid configuration: service::pipelines::traces: references processor \"invalid\" which is not configured")
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestNewCollectorInvalidConfigProviderSettings(t *testing.T) {
|
func TestNewCollectorInvalidConfigProviderSettings(t *testing.T) {
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue