Remove our yaml hacks by switching to goccy/go-yaml lib.
Recently we have trouble customizing marshalling of 3rd party structs like:
- Prometheus omitzero problem that cause us to copy structs which is hard to sustain https://github.com/GoogleCloudPlatform/prometheus-engine/pull/1555#issue-2955046159
- Alertmanager secret redaction hacks https://github.com/GoogleCloudPlatform/prometheus-engine/pull/1562
I think we coud replace those frictions by replacing gopkg.in/yaml with https://github.com/goccy/go-yaml?tab=readme-ov-file#features
Another reason for this work is that gopkg.in/yaml.* project seems to be deprecated.
We might want to try https://github.com/GoogleCloudPlatform/prometheus-engine/issues/1629 long term
goccy/go-yaml doesn't seem to support omitzero, either.
The latest version of alertmanager supports unmasking secrets. So does common.
That said, it's probably a good idea to switch to a maintained library at some point, but I don't know of any current problems it would solve for us.
Thanks for a quick look!
Note: I think you mean omitempty not omitzero? AFAIK omitempty is what we want and it's supported.. Maybe in other libs it's called omitzero.
Anyway, I understand this problem it's not trivial.
However, the unmasking problem is one thing, but we still generate "garbage" for all empty fields, which impacts debugging, we hit scalability limits for k8s resource sizes sooner. Plus we still have challenge with Prometheus struct compatibility -- one could say it's tmp, but next month we will do another upgrade and hit the same issue.
As a result I think it's worth to properly fix all our yaml pain once and for all, even if it involves getting our hands dirty in the world of yaml unmarshalling.
I tried today something and I think we could do a bit of magic like below, thanks to marshal customizations:
package yamlutil
import (
"github.com/goccy/go-yaml"
)
func MarshalOmitEmpty(v any) ([]byte, error) {
return yaml.MarshalWithOptions(v, yaml.CustomMarshaler(customOmitEmptyMarshaler()))
}
func customOmitEmptyMarshaler() func(t any) ([]byte, error) {
return func(t any) ([]byte, error) {
// Option1: Copy t into our own struct and edit struct tag to add omitempty.
// Option2: Copy non-empty fields into our own struct.
// ...
return yaml.Marshal(t)
}
}
BUT!
However, there might be much better solution that community ask for -- direct support for global omitempty override (https://github.com/goccy/go-yaml/issues/306) 💪🏽
I actually went ahead and proposed a PR for it: https://github.com/goccy/go-yaml/pull/691
We can use forked lib in the mean time too.
Lol it got merged in like 20m from PR creation: https://github.com/goccy/go-yaml/pull/691
I started an effort to use it on top of your PR @bernot-dev https://github.com/GoogleCloudPlatform/prometheus-engine/pull/1555 -- https://github.com/GoogleCloudPlatform/prometheus-engine/tree/omitempty - works well.
But it might more clean to do a separate PR for switch. Pausing for 1h gym for now!
Good point about omitzero noticed by @bernot-dev https://github.com/goccy/go-yaml/pull/691/files#r2039813449
Just to update on the rationales and specifics:
We do want to move to this YAML library because of the following reasons:
- For more compatibility around unused fields that are missing omitempty.
- For possible future solutions around better compatibility of encoding configs of AM and Prometheus while avoiding defaulting behaviours.
- To replace deprecated yaml.v2 and yaml.v3 modules.