Skip to content

chore(deps): bump prometheus to v3.13.0-gmp.1-gke.0 - #2301

Merged
bwplotka merged 12 commits into
GoogleCloudPlatform:mainfrom
bwplotka:bump_prometheus_version_3_13
Sep 23, 2026
Merged

bwplotka merged 12 commits into
GoogleCloudPlatform:mainfrom
bwplotka:bump_prometheus_version_3_13

Conversation

@bwplotka

@bwplotka bwplotka commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

This effectively switches GMP to the 3.x Prometheus fork (release-3.13.0-gmp).

Changes

  • Dependencies & Images:
    • Bump github.com/prometheus/prometheus to github.com/GoogleCloudPlatform/prometheus v0.0.0-20260903164310-65985e7c948b (release-3.13.0-gmp) and remove obsolete v2.53 replace directives.
    • Switch Prometheus image to 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).
  • Prometheus 2.x Compatibility Defaults:
    • Configure metric_name_validation_scheme: legacy and metric_name_escaping_scheme: underscores in collector global config (pkg/operator/collection.go).
    • Configure fallback_scrape_protocol: PrometheusText0.0.4 on generated ScrapeConfigs (pkg/operator/apis/monitoring/v1/common_types.go, pkg/operator/apis/monitoring/v1/operator_config.go).
  • Prometheus 3.x & controller-runtime API Updates:
    • cmd/rule-evaluator: Migrate to promslog (*slog.Logger), notifier.Send, rules.NewManager options, labels.Compare, and updated storage.Select / labels.Labels (stringlabels) APIs.
    • pkg/operator: Default unset relabel.Config.Regex to relabel.DefaultRelabelConfig.Regex (required by relabel.Regexp.IsZero()), update rulefmt.Parse calls, and migrate webhooks to typed admission.Validator[T] / admission.Defaulter[T].

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread pkg/operator/apis/monitoring/v1/common_types.go
@bwplotka
bwplotka force-pushed the bump_prometheus_version_3_13 branch from ce4ddfb to f5844e5 Compare September 21, 2026 13:06
@bwplotka bwplotka changed the title chore: bump Prometheus to v3.13.0-gmp.1-gke.0 chore(deps): bump prometheus to v3.13.0-gmp.1-gke.0 Sep 21, 2026

@bwplotka bwplotka left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self review.

Comment thread pkg/operator/apis/monitoring/v1/common_types.go
Comment thread pkg/operator/operator_config_test.go Outdated
Comment thread pkg/operator/operator_config_test.go
Comment thread pkg/operator/apis/monitoring/v1/common_types.go
Comment thread pkg/operator/operator_config_test.go Outdated
Signed-off-by: bwplotka <bwplotka@google.com>
@bwplotka
bwplotka force-pushed the bump_prometheus_version_3_13 branch from 691d126 to ac42c18 Compare September 21, 2026 15:15
…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 {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rationale, for reviewers:

  • In Prometheus 2.53 (slicelabels), Labels.Hash() hashed \xff-separated strings (name\xffvalue\xff...).
  • In Prometheus 3.13 (stringlabels in labels_stringlabels.go), Labels is stored as a length-prefixed byte string (ls.data), and Hash() runs xxhash.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).

@bwplotka
bwplotka force-pushed the bump_prometheus_version_3_13 branch from f6b4cdf to 9012006 Compare September 22, 2026 07:17
Comment on lines 57 to +58
}
re := relabel.DefaultRelabelConfig.Regex

@bwplotka bwplotka Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. c.Regex.IsZero() evaluates nil == DefaultRelabelConfig.Regex.Regexp $\rightarrow$ false.
  2. Because IsZero() is false, yaml.Marshal does not omit regex (omitempty is skipped) and calls MarshalYAML(), which returns (nil, nil) and serializes regex: null into every relabel rule in YAML.
  3. When Prometheus unmarshals regex: null, Regexp.UnmarshalYAML unmarshals null as "" (^(?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>
@bwplotka
bwplotka marked this pull request as ready for review September 22, 2026 07:47
@bwplotka

Copy link
Copy Markdown
Collaborator Author

This should be ready to go! cc @bernot-dev

@bernot-dev

Copy link
Copy Markdown
Collaborator

I think rule evaluator also needs to enforce NameValidationScheme: model.LegacyValidation.

https://github.com/bwplotka/prometheus-engine/blob/bump_prometheus_version_3_13/cmd/rule-evaluator/main.go#L953

Comment thread pkg/operator/apis/monitoring/v1/rules_config.go Outdated
Comment thread cmd/rule-evaluator/main.go

@bernot-dev bernot-dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this. I just found a few more things.

@bernot-dev bernot-dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bwplotka
bwplotka merged commit c0a56f6 into GoogleCloudPlatform:main Sep 23, 2026
100 of 102 checks passed
Context: ctx,
Appendable: appendable,
// Ensure compatibility with 2.x Prometheus logic. See go/gmp:prom-3.13.
NameValidationScheme: model.LegacyValidation,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we could allow it here - it's not a breaking change here. To discuss in separate change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants