chore(deps): bump prometheus to v3.13.0-gmp.1-gke.0 - #2301
Conversation
There was a problem hiding this comment.
Code Review
This pull request upgrades the Prometheus dependency to v3.13.0 and updates various components, tests, and webhooks to align with the new Prometheus and controller-runtime APIs. Key changes include using structured label constructors, migrating to promslog, and updating webhook validators and defaulters. Feedback was provided to add nil checks for relabel configurations in common_types.go to prevent potential nil-pointer dereference panics.
ce4ddfb to
f5844e5
Compare
Signed-off-by: bwplotka <bwplotka@google.com>
691d126 to
ac42c18
Compare
…e protocol Configure legacy metric name validation and underscore escaping in the global collector config, and set PrometheusText0.0.4 as the fallback scrape protocol for Prometheus 3.x compatibility.
| return -1 | ||
| } | ||
| return 1 | ||
| if cmp := labels.Compare(a.Labels, b.Labels); cmp != 0 { |
There was a problem hiding this comment.
Rationale, for reviewers:
- In Prometheus 2.53 (
slicelabels),Labels.Hash()hashed\xff-separated strings (name\xffvalue\xff...). - In Prometheus 3.13 (
stringlabelsinlabels_stringlabels.go),Labelsis stored as a length-prefixed byte string (ls.data), andHash()runsxxhash.Sum64(yoloBytes(ls.data)).
Because the xxhash output values changed, sorting alerts by descending a.Labels.Hash() in alertsToAPIAlerts reversed the relative order of test-alert-1 and test-alert-2, causing TestAPI_HandleAlertsEndpoint/only_firing_alerts to fail (test-alert-2 sorted before test-alert-1).
Signed-off-by: bwplotka <bwplotka@google.com>
f6b4cdf to
9012006
Compare
| } | ||
| re := relabel.DefaultRelabelConfig.Regex |
There was a problem hiding this comment.
Generally I find those golden test odd.. we take PO objects, migrate to GMP and normalize GMP, so we can compare with different PO objects. We probably could store expected GMP objects as golden tests files? 🤔
Nevertheless rationale for reviewers for this change:
When we construct &relabel.Config{...} struct literals in Go without setting Regex, c.Regex.Regexp is nil:
-
c.Regex.IsZero()evaluatesnil == DefaultRelabelConfig.Regex.Regexp$\rightarrow$ false. - Because
IsZero()isfalse,yaml.Marshaldoes not omitregex(omitemptyis skipped) and callsMarshalYAML(), which returns(nil, nil)and serializesregex: nullinto every relabel rule in YAML. - When Prometheus unmarshals
regex: null,Regexp.UnmarshalYAMLunmarshalsnullas""(^(?s:)$— matching only empty strings instead of the default(.*)), which breaks relabeling.
Setting c.Regex = relabel.DefaultRelabelConfig.Regex when c.Regex.Regexp == nil makes IsZero() return true so yaml.Marshal omits regex as intended.
Signed-off-by: bwplotka <bwplotka@google.com>
|
This should be ready to go! cc @bernot-dev |
|
I think rule evaluator also needs to enforce |
bernot-dev
left a comment
There was a problem hiding this comment.
Thanks for working on this. I just found a few more things.
| Context: ctx, | ||
| Appendable: appendable, | ||
| // Ensure compatibility with 2.x Prometheus logic. See go/gmp:prom-3.13. | ||
| NameValidationScheme: model.LegacyValidation, |
There was a problem hiding this comment.
Actually we could allow it here - it's not a breaking change here. To discuss in separate change.
This effectively switches GMP to the 3.x Prometheus fork (
release-3.13.0-gmp).Changes
github.com/prometheus/prometheustogithub.com/GoogleCloudPlatform/prometheusv0.0.0-20260903164310-65985e7c948b(release-3.13.0-gmp) and remove obsolete v2.53replacedirectives.gke.gcr.io/prometheus-engine/prometheus:v3.13.0-gmp.1-gke.0@sha256:b8e81d2737a3ca7126b06cbd4bb495b4f9c694e9431e5a280d7d37f289ff2708(charts/values.global.yaml,examples/prometheus.yaml,manifests/operator.yaml).metric_name_validation_scheme: legacyandmetric_name_escaping_scheme: underscoresin collectorglobalconfig (pkg/operator/collection.go).fallback_scrape_protocol: PrometheusText0.0.4on generatedScrapeConfigs (pkg/operator/apis/monitoring/v1/common_types.go,pkg/operator/apis/monitoring/v1/operator_config.go).controller-runtimeAPI Updates:cmd/rule-evaluator: Migrate topromslog(*slog.Logger),notifier.Send,rules.NewManageroptions,labels.Compare, and updatedstorage.Select/labels.Labels(stringlabels) APIs.pkg/operator: Default unsetrelabel.Config.Regextorelabel.DefaultRelabelConfig.Regex(required byrelabel.Regexp.IsZero()), updaterulefmt.Parsecalls, and migrate webhooks to typedadmission.Validator[T]/admission.Defaulter[T].