Update vendored prometheus #8295

pracucci · 2024-06-06T14:18:15Z

What this PR does

In this PR I'm updating mimir-prometheus to get the changes synced in grafana/mimir-prometheus#639.

The main change affecting Mimir is the introduction of query_offset in rule group config, which deprecates evaluation_delay. In this PR I propose to do the safest change in Mimir, which is mapping query_offset and evaluation_delay independently in the rule group config we store in the object storage, so that when a tenant downloads a previously loaded config they get the same exact YAML. This is a property I would like to preserve for now, in case any customer runs a reconcile loop comparing the local YAML config with the one stored in Mimir (read via API): I prefer to make sure the two YAMLs always match (which wouldn't be true if we manipulate the YAML config to set query_offset to the value of evaluation_delay when it's not set).

Manual tests

I've run manual tests with mimirtool downloaded from Mimir 2.12.0 release and mimirtool built on top of this PR.

Test results against Mimir running from this branch:

Upload rule config
- mimirtool 2.12.0
  - Supports evaluation_delay
  - Fails parsing if config contains query_offset (expected)
- mimirtool this branch
  - Supports evaluation_delay and query_offset
Get rule config
- mimirtool 2.12.0
  - Supports evaluation_delay
  - Does not display query_offset if remote config contains it (expected), but doesn't fail
- mimirtool this branch
  - Supports evaluation_delay and query_offset

Test results against Mimir running from main:

Upload rule config
- mimirtool this branch
  - Supports evaluation_delay
  - Upload succeed if config contains query_offset but server ignores it (a subsequent rules get doesn't return it)
Get rule config
- mimirtool this branch
  - Supports evaluation_delay

Which issue(s) this PR fixes or relates to

N/A

Checklist

Tests updated.
Documentation added.
CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
about-versioning.md updated with experimental features.

…roup config and deprecating evaluation_delay Signed-off-by: Marco Pracucci <marco@pracucci.com>

Signed-off-by: Marco Pracucci <marco@pracucci.com>

pracucci · 2024-06-06T14:37:32Z

CHANGELOG.md

@@ -82,6 +83,7 @@
 * [BUGFIX] Store-gateway: Allow long-running index scans to be interrupted. #8154
 * [BUGFIX] Query-frontend: fix splitting of queries using `@ start()` and `@end()` modifiers on a subquery. Previously the `start()` and `end()` would be evaluated using the start end end of the split query instead of the original query. #8162
 * [BUGFIX] Distributor: Don't discard time series with invalid exemplars, just drop affected exemplars. #8224
+* [BUGFIX] Ingester: fixed in-memory series count when replaying a corrupted WAL. #8295


Note to reviewers: this is related to prometheus/prometheus#14079.

pracucci · 2024-06-06T14:37:56Z

docs/sources/mimir/configure/about-versioning.md

@@ -211,3 +211,8 @@ The following features or configuration parameters are currently deprecated and
  - `-ingester.client.report-grpc-codes-in-instrumentation-label-enabled`
 - Mimirtool
  - the flag `--rule-files`
+
+The following features or configuration parameters are currently deprecated and will be **removed in a future release (to be announced)**:


Note to reviewers: I want to keep evaluation_delay support for longer.

pracucci · 2024-06-06T14:38:24Z

pkg/ruler/api_test.go

@@ -1071,7 +1124,7 @@ rules:

 				router.ServeHTTP(w, req)
 				require.Equal(t, 200, w.Code)
-				require.YAMLEq(t, tt.output, w.Body.String())
+				require.YAMLEq(t, tt.input, w.Body.String())


Note to reviewers: we can simplify the test. We expect to receive the same YAML we uploaded.

agree, although isn't that because we don't test 0 value here?

Correct. If we would test the 0 value the output would be different

pkg/ruler/rulespb/compat_test.go

krajorama

LGTM, small typo nit

Signed-off-by: Marco Pracucci <marco@pracucci.com>

Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com>

gotjosh

LGTM, one comment, non-blocking, I don't need to look at this again.

Thanks again for bringing these changes in.

gotjosh · 2024-06-06T17:43:27Z

pkg/ruler/compat.go

@@ -330,7 +330,7 @@ func DefaultTenantManagerFactory(
 			ForGracePeriod:             cfg.ForGracePeriod,
 			ResendDelay:                cfg.ResendDelay,
 			AlwaysRestoreAlertState:    true,
-			DefaultEvaluationDelay: func() time.Duration {
+			DefaultRuleQueryOffset: func() time.Duration {
 				// Delay the evaluation of all rules by a set interval to give a buffer
 				// to metric that haven't been forwarded to Mimir yet.
 				return overrides.EvaluationDelay(userID)


nit [non-blocking]: At some point, we probably want to create a new limit so that it matches the description a bit more accurately. Happy to create an issue for it if you think it's worth it.

I agree we should but looks very low priority. I'm fine creating an issue for now.

pracucci added 3 commits June 6, 2024 16:16

Update vendored prometheus, adding support for query_offset in rule g…

ad7dc1a

…roup config and deprecating evaluation_delay Signed-off-by: Marco Pracucci <marco@pracucci.com>

Fix linter issues

0da89da

Signed-off-by: Marco Pracucci <marco@pracucci.com>

Add CHANGELOG entry

8dd3498

Signed-off-by: Marco Pracucci <marco@pracucci.com>

pracucci commented Jun 6, 2024

View reviewed changes

krajorama reviewed Jun 6, 2024

View reviewed changes

pkg/ruler/rulespb/compat_test.go Outdated Show resolved Hide resolved

pracucci mentioned this pull request Jun 6, 2024

mimirtool rules sync: fix sync when query_offset or evaluation_delay change #8297

Merged

4 tasks

krajorama approved these changes Jun 6, 2024

View reviewed changes

pracucci and others added 2 commits June 6, 2024 18:51

Updated mimir-prometheus again

da84a8d

Signed-off-by: Marco Pracucci <marco@pracucci.com>

Update pkg/ruler/rulespb/compat_test.go

9f715be

Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com>

pracucci marked this pull request as ready for review June 6, 2024 16:51

pracucci requested review from grafanabot, a team and jdbaldry as code owners June 6, 2024 16:51

pracucci enabled auto-merge (squash) June 6, 2024 16:52

gotjosh approved these changes Jun 6, 2024

View reviewed changes

pracucci merged commit 1fcecee into main Jun 7, 2024
29 checks passed

pracucci deleted the update-mimir-prometheus branch June 7, 2024 06:36

davidkohr mentioned this pull request Jun 28, 2024

feat: add evaluation_period for recording rules fgouteroux/terraform-provider-mimir#39

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update vendored prometheus #8295

Update vendored prometheus #8295

pracucci commented Jun 6, 2024 •

edited

Loading

pracucci Jun 6, 2024

pracucci Jun 6, 2024

pracucci Jun 6, 2024

krajorama Jun 6, 2024

pracucci Jun 6, 2024

krajorama left a comment

gotjosh left a comment

gotjosh Jun 6, 2024

pracucci Jun 7, 2024

Update vendored prometheus #8295

Update vendored prometheus #8295

Conversation

pracucci commented Jun 6, 2024 • edited Loading

What this PR does

Manual tests

Which issue(s) this PR fixes or relates to

Checklist

pracucci Jun 6, 2024

Choose a reason for hiding this comment

pracucci Jun 6, 2024

Choose a reason for hiding this comment

pracucci Jun 6, 2024

Choose a reason for hiding this comment

krajorama Jun 6, 2024

Choose a reason for hiding this comment

pracucci Jun 6, 2024

Choose a reason for hiding this comment

krajorama left a comment

Choose a reason for hiding this comment

gotjosh left a comment

Choose a reason for hiding this comment

gotjosh Jun 6, 2024

Choose a reason for hiding this comment

pracucci Jun 7, 2024

Choose a reason for hiding this comment

pracucci commented Jun 6, 2024 •

edited

Loading