[confmap] **Discuss** Identify string field type and set the expanded value accordingly #12860
+58
−83
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Note: This PR is for discussion as I might be missing something obvious in how the type identification works in confmap. If code owner agreed with the Issue identified and the PR is accepted, I will need to push more changes to cleanup unused code.
Issue: #12793
If you have a config that contains a slice/map of type struct and one of the fields in that struct is an env variable which is set to a numerical value, the collector will fail to start with
The existing
isStringyStructure
does not handle structs which causes the first call made touseExpandValue
to sanitize "ALL" data input and use the parsed value.After looking into mapstructure code, I have noticed that the
useExpandValue
is called on every sub config with the associated data https://github.com/go-viper/mapstructure/blame/main/mapstructure.go#L514-L546This made me wonder why the extra logic to check the whole structure is needed and that the first block of
useExpandValue
if exp, ok := data.(expandedValue); ok {
should do the job.Link to tracking issue
I have removed the extra structure type check and some and fixed the string pointer edge case.
Added an extra unit test that exposes the issue at hand.
Testing
Added a unit test and ran a bunch of manual tests