-
Notifications
You must be signed in to change notification settings - Fork 678
Open
Labels
area: file-configRelated to file-based configurationRelated to file-based configurationenhancementNew feature or requestNew feature or request
Description
null
attribute should be considered as an invalid input for ParseYAML
(and when parsing JSON as well) as we already have test cases like invalid nil value
.
Originally posted by @pellared in #6603
CC @codeboten
Metadata
Metadata
Assignees
Labels
area: file-configRelated to file-based configurationRelated to file-based configurationenhancementNew feature or requestNew feature or request
Type
Projects
Milestone
Relationships
Development
Select code repository
Activity
mikeblum commentedon Jan 16, 2025
Can I pick this up? @pellared
[open-telemetryGH-6629] config: Handle null attributes as invalid
mikeblum commentedon Mar 13, 2025
After digging pretty deeply into the YAML spec its unclear to me if we want to be writing custom parsers to error on different cases such as:
attr: null
attr: []
attr: 0
attr:
imo the current behaviour of
parseYAML
ignoring these attributes entirely is correct but I defer to @pellared on what the correct behavious should be.pellared commentedon Mar 13, 2025
We should consistently "ignore" invalid values or return an error. I think that if something is not compliant with the scheme then it should return an error.
Moreover, from https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk.md#parse:
This is my expectation.
MrAlias commentedon Mar 13, 2025
Wouldn't this mean we would completely rewrite a yaml parsing library? That does not seem like something we want to maintain.
pellared commentedon Mar 13, 2025
I mean add additional validation which is unfortunately not handled by
github.com/atombender/go-jsonschema
. It should not be a rewrite. It seems we already doing it inconfig_json.go
andconfig_yaml.go
.However, probably it would be a lot better to contribute to https://github.com/omissis/go-jsonschema (or at least create issues).