Add omitempty to all appropriate config struct fields
#12191
Labels
Comments
github-merge-queue bot
pushed a commit
that referenced
this issue
Jan 28, 2025
…#12190) #### Description Add wording that allows us to update struct field tags if changes to the tags doesn't change the values of fields in the struct during serialization/deserialization. This will allow us to add `omitempty` to fields on structs in stable packages. Adding this tag will cause fields which have zero values to not be written when marshaling into YAML, but there is no functional difference since missing fields during unmarshaling simply leaves a struct field with a zero value. Related to #12191.
sfc-gh-sili
pushed a commit
to sfc-gh-sili/opentelemetry-collector
that referenced
this issue
Feb 3, 2025
…open-telemetry#12190) #### Description Add wording that allows us to update struct field tags if changes to the tags doesn't change the values of fields in the struct during serialization/deserialization. This will allow us to add `omitempty` to fields on structs in stable packages. Adding this tag will cause fields which have zero values to not be written when marshaling into YAML, but there is no functional difference since missing fields during unmarshaling simply leaves a struct field with a zero value. Related to open-telemetry#12191.
github-merge-queue bot
pushed a commit
that referenced
this issue
Feb 5, 2025
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Adds `omitempty` to all struct fields that are not set with any of the `NewDefault[...]` functions. If we like the approach here, I'll open PRs for other config modules. <!-- Issue number if applicable --> #### Link to tracking issue Works toward #12191 <!--Describe what testing was performed and which tests were added.--> #### Testing I added tests to check that this works when structs are marshaled with confmap in a separate module so as not to create a direct dependency on confmap within confignet.
This was referenced Feb 6, 2025
github-merge-queue bot
pushed a commit
that referenced
this issue
Feb 7, 2025
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description I didn't add the tag to any fields that have defaults set, since setting these values to Go's zero value for that type would be a meaningful configuration option that should appear in the config. I think we could consider updates to the marshaler in the future to make it possible to omit fields that have values matching the ones produced by the `NewDefault[...]` functions, but that will be a more involved effort we can tackle later. <!-- Issue number if applicable --> #### Link to tracking issue Works toward #12191
github-merge-queue bot
pushed a commit
that referenced
this issue
Feb 10, 2025
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description This will dramatically reduce YAML output when marshaling config for many receivers and exporters since it's commonly used. <!-- Issue number if applicable --> #### Link to tracking issue Works toward #12191
This was referenced Feb 25, 2025
github-merge-queue bot
pushed a commit
that referenced
this issue
Feb 26, 2025
<!-- Issue number if applicable --> #### Link to tracking issue Works toward #12191
github-merge-queue bot
pushed a commit
that referenced
this issue
Feb 27, 2025
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Update config structs in the service module to include `omitempty` tags. <!-- Issue number if applicable --> #### Link to tracking issue Works toward #12191
github-merge-queue bot
pushed a commit
that referenced
this issue
Mar 3, 2025
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Depends on #12481. Note that I didn't add the tag to any fields where we set a default value. This is because the zero value is significant in these cases and omitting it from the output would cause it to be set to the default instead of the zero value. We could fix this with a custom interface implemented by our config structs, but that will have to be a separate effort, and simply using the `omitempty` tag is still an improvement for most fields. <!-- Issue number if applicable --> #### Link to tracking issue Works toward #12191
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Is your feature request related to a problem? Please describe.
Right now when marshaling our config structs into YAML, they produce a large number of keys that are unset. We should get as close to only emitting fields that have been actually set as possible.
Describe the solution you'd like
Use mapstructure's
omitemptysuffix to omit these values from serialization.Describe alternatives you've considered
We can add an option to
confmap.Marshalthat forces this regardless of whether it has been set on the struct. We may need this anyway, but it would be nice if our config structs handled this out-of-the-box.The text was updated successfully, but these errors were encountered: