Skip to content

Commit e7f071d

Browse files
authored
[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.-->
1 parent 7c1d694 commit e7f071d

File tree

4 files changed

+47
-4
lines changed

4 files changed

+47
-4
lines changed

.chloggen/mx-psi_error-map.yaml

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: confmap
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Surface YAML parsing errors when they happen at the top-level.
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [12180]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext: |
19+
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
20+
21+
# Optional: The change log or logs in which this entry should be included.
22+
# e.g. '[user]' or '[user, api]'
23+
# Include 'user' if the change is relevant to end users.
24+
# Include 'api' if there is a change to a library API.
25+
# Default: '[user]'
26+
change_logs: []

confmap/provider.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,15 @@ type ChangeEvent struct {
100100
// Retrieved holds the result of a call to the Retrieve method of a Provider object.
101101
type Retrieved struct {
102102
rawConf any
103+
errorHint error
103104
closeFunc CloseFunc
104105

105106
stringRepresentation string
106107
isSetString bool
107108
}
108109

109110
type retrievedSettings struct {
111+
errorHint error
110112
stringRepresentation string
111113
isSetString bool
112114
closeFunc CloseFunc
@@ -138,6 +140,12 @@ func withStringRepresentation(stringRepresentation string) RetrievedOption {
138140
})
139141
}
140142

143+
func withErrorHint(errorHint error) RetrievedOption {
144+
return retrievedOptionFunc(func(settings *retrievedSettings) {
145+
settings.errorHint = errorHint
146+
})
147+
}
148+
141149
// NewRetrievedFromYAML returns a new Retrieved instance that contains the deserialized data from the yaml bytes.
142150
// * yamlBytes the yaml bytes that will be deserialized.
143151
// * opts specifies options associated with this Retrieved value, such as CloseFunc.
@@ -146,7 +154,10 @@ func NewRetrievedFromYAML(yamlBytes []byte, opts ...RetrievedOption) (*Retrieved
146154
if err := yaml.Unmarshal(yamlBytes, &rawConf); err != nil {
147155
// If the string is not valid YAML, we try to use it verbatim as a string.
148156
strRep := string(yamlBytes)
149-
return NewRetrieved(strRep, append(opts, withStringRepresentation(strRep))...)
157+
return NewRetrieved(strRep, append(opts,
158+
withStringRepresentation(strRep),
159+
withErrorHint(fmt.Errorf("assuming string type since contents are not valid YAML: %w", err)),
160+
)...)
150161
}
151162

152163
switch rawConf.(type) {
@@ -175,6 +186,7 @@ func NewRetrieved(rawConf any, opts ...RetrievedOption) (*Retrieved, error) {
175186
}
176187
return &Retrieved{
177188
rawConf: rawConf,
189+
errorHint: set.errorHint,
178190
closeFunc: set.closeFunc,
179191
stringRepresentation: set.stringRepresentation,
180192
isSetString: set.isSetString,
@@ -188,6 +200,9 @@ func (r *Retrieved) AsConf() (*Conf, error) {
188200
}
189201
val, ok := r.rawConf.(map[string]any)
190202
if !ok {
203+
if r.errorHint != nil {
204+
return nil, fmt.Errorf("retrieved value (type=%T) cannot be used as a Conf: %w", r.rawConf, r.errorHint)
205+
}
191206
return nil, fmt.Errorf("retrieved value (type=%T) cannot be used as a Conf", r.rawConf)
192207
}
193208
return NewFromStringMap(val), nil

confmap/provider_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,9 @@ func TestNewRetrievedFromYAMLInvalidYAMLBytes(t *testing.T) {
134134
require.NoError(t, err)
135135

136136
_, err = ret.AsConf()
137-
require.Error(t, err)
137+
require.EqualError(t, err,
138+
"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",
139+
)
138140

139141
str, err := ret.AsString()
140142
require.NoError(t, err)
@@ -150,7 +152,7 @@ func TestNewRetrievedFromYAMLInvalidAsMap(t *testing.T) {
150152
require.NoError(t, err)
151153

152154
_, err = ret.AsConf()
153-
require.Error(t, err)
155+
require.EqualError(t, err, "retrieved value (type=string) cannot be used as a Conf")
154156

155157
str, err := ret.AsString()
156158
require.NoError(t, err)

otelcol/collector_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ func TestCollectorStartInvalidConfig(t *testing.T) {
312312
ConfigProviderSettings: newDefaultConfigProviderSettings(t, []string{filepath.Join("testdata", "otelcol-invalid.yaml")}),
313313
})
314314
require.NoError(t, err)
315-
assert.Error(t, col.Run(context.Background()))
315+
assert.EqualError(t, col.Run(context.Background()), "invalid configuration: service::pipelines::traces: references processor \"invalid\" which is not configured")
316316
}
317317

318318
func TestNewCollectorInvalidConfigProviderSettings(t *testing.T) {

0 commit comments

Comments
 (0)