Implement stateless mode for Thanos Ruler (continue #4250) #4731

yeya24 · 2021-10-03T09:38:04Z

I added CHANGELOG entry for this change.
Change is not relevant to the end user.

Changes

This pr is based on top of the awesome pr #4250 by @idoqo and fixes E2E tests.

Verification

E2E tests should pass.

yeya24 · 2021-10-03T10:34:24Z

prometheus/prometheus#9109 requires some modification on the vendored grafana agent wal package.

/home/runner/work/thanos/thanos/pkg/rules/remotewrite/wal_test.go:75:29: cannot use &collector (value of type *walDataCollector) as wal.WriteTo value in struct literal: missing method UpdateSeriesSegment

Maybe let's wait for the upgrade from the agent side? @rfratto

rfratto · 2021-10-04T13:56:33Z

Maybe let's wait for the upgrade from the agent side? @rfratto

I'm going to be bumping the Prometheus dependency in grafana/agent ideally this week and expect to also run into this. I'll tag you @yeya24 once we have the PR to update the code to resolve the error.

rfratto · 2021-10-08T16:38:16Z

I'm going to be bumping the Prometheus dependency in grafana/agent ideally this week and expect to also run into this. I'll tag you @yeya24 once we have the PR to update the code to resolve the error.

Hey y'all, I wanted to give a quick update on this. I've put effort into updating the Grafana Agent's Prometheus dependency this week, but it's going to take some more time. The bump also requires a bump to github.com/prometheus/common, which is currently blocked by a few of the exporters we embed still using the (since removed) github.com/prometheus/common/log package. Such is the life of a project that has a lot of dependencies :)

We've definitely made progress here, but it might take another week before I can update the WAL code. I'll still tag y'all when we have something.

yeya24 · 2021-10-28T20:07:40Z

This pr is ready for review!

GiedriusS

Just some nits and questions. Good work! Do we still need pkg/rules/remotewrite/{wal,series}.go with prometheus/prometheus#8785 merged? ;P It says:

// TODO(idoqo): Migrate to prometheus package when https://github.com/prometheus/prometheus/pull/8785 is ready.

CHANGELOG.md

pkg/rules/remotewrite/wal_test.go

GiedriusS · 2021-11-01T21:58:13Z

test/e2e/rule_test.go

+// TestRule_CanPersistWALData checks that in stateless mode, Thanos Ruler can persist rule evaluations
+// which couldn't be sent to the remote write endpoint (e.g because receiver isn't available).
+func TestRule_CanPersistWALData(t *testing.T) {
+	//TODO: Implement test with unavailable remote-write endpoint(receiver)


Do we need to implement this before merging?

I guess this is not a blocker as we are using the upstream prom agent code already so data should be persisted. Not sure how to implement this right now though.

yeya24 · 2021-11-02T05:55:29Z

Just some nits and questions. Good work! Do we still need pkg/rules/remotewrite/{wal,series}.go with prometheus/prometheus#8785 merged? ;P It says:

It seems that we can totally remove the pkg/rules/remotewrite package after using the upstream Prometheus agent code 😄

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

This removes the need to pass a separate `remote-write` flag to ruler to enable stateless mode. Instead, we now check if a remote-write config is provided and automatically enables stateless mode based off that. Ruler test is also cleaned up to remove unnecessary tests (i.e those that have been performed by other e2e suites). Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

Signed-off-by: Ben Ye <ben.ye@bytedance.com>

Signed-off-by: yeya24 <yb532204897@gmail.com>

Signed-off-by: Ben Ye <ben.ye@bytedance.com>

yeya24 · 2021-11-08T08:40:33Z

@GiedriusS Let me know if there is anything left in this pr. If it is small nits then I guess we can clean up later. 😄

GiedriusS

Just a few very small nits but after that, we can merge this IMO 👍

GiedriusS · 2021-11-08T16:02:38Z

cmd/thanos/rule.go

-		}, func(error) {
-			close(done)
-		})
+		db, err := agent.Open(logger, reg, remoteStore, walDir, agentOpts)


Could we please call this something other than db to avoid shadowing the outer variable? This is confusing and I had to double check this place 😂 I suggest agentDB or something like that, and to rename the outer db variable into tsdbDB.

GiedriusS · 2021-11-08T16:05:06Z

cmd/thanos/rule.go

+		grpcserver.WithTLSConfig(tlsCfg),
+	}
+	if db != nil {
+		tsdbStore := store.NewTSDBStore(logger, db, component.Rule, conf.lset)


In the future maybe we could also answer StoreAPI queries by using the data in the WAL? Perhaps worth adding a TODO here 😄

I am not sure about this. If the agent is able to query head data, then what's the difference between the agent and a prometheus with very low retention? I am not sure whether this is in their roadmap.

Fair point 👍

cmd/thanos/rule.go

test/e2e/rule_test.go

GiedriusS · 2021-11-08T16:09:32Z

test/e2e/rule_test.go

+// TestRule_CanPersistWALData checks that in stateless mode, Thanos Ruler can persist rule evaluations
+// which couldn't be sent to the remote write endpoint (e.g because receiver isn't available).
+func TestRule_CanPersistWALData(t *testing.T) {
+	//TODO: Implement test with unavailable remote-write endpoint(receiver)


Signed-off-by: Ben Ye <ben.ye@bytedance.com>

yeya24 · 2021-11-08T17:54:21Z

Hi @GiedriusS, I have addressed the comments in the latest commit.

GiedriusS

In the end, it became a very small pull request with all of the agent code in upstream Prometheus 💪 awesome!

onprem · 2021-11-09T05:28:50Z

Great work! 🎉

bwplotka

Amazing work @yeya24 !

yeya24 force-pushed the remote-write-rule branch from 3a79087 to 2390e96 Compare October 28, 2021 20:06

GiedriusS reviewed Nov 1, 2021

View reviewed changes

yeya24 force-pushed the remote-write-rule branch 2 times, most recently from 5979b10 to 23988c5 Compare November 2, 2021 05:40

yeya24 force-pushed the remote-write-rule branch 2 times, most recently from be3ab7e to ae51c39 Compare November 2, 2021 06:10

idoqo and others added 19 commits November 7, 2021 00:37

import wal package from grafana/agent (with string interning disabled)

a25e730

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

Set up remote-write config and test skeleton

6a14fc5

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

Setup fanout and related storages for stateless ruler

021cb36

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

Optionally run ruler in stateless mode

fd19217

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

Set up tests and implementations for configuring remote-write for ruler

5814523

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

Implement stub querier for WAL storage to fix nil pointer error

7acfa2e

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

Setup e2e test for stateless ruler

2d7d6d9

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

Add copied code commentary to remotewrite packages

c2f999c

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

Use static addresses for am and querier

31a75f2

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

Generate docs for stateless ruler flags and fix tests

d567faf

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

Use promauto for prometheus primitives

6fb7d3b

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

Group imports and satisfy go-lint

62ef47c

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

Always return empty series set from WAL storage

2054971

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

re-generate rule documentation

3d5413d

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

copyright headers to satisfy golint

27f4984

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

Rename wal storage metrics

1c82a78

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

Use Prometheus' remote write config instead of rolling another

760be92

Signed-off-by: Michael Okoko <okokomichaels@outlook.com>

Fix E2E tests

45e9f99

Signed-off-by: Ben Ye <ben.ye@bytedance.com>

yeya24 and others added 6 commits November 7, 2021 00:37

add changelog

1eb0fe3

Signed-off-by: yeya24 <yb532204897@gmail.com>

remove wal related tests

3b65c42

Signed-off-by: Ben Ye <ben.ye@bytedance.com>

fix e2e

69bb9b8

Signed-off-by: Ben Ye <ben.ye@bytedance.com>

remove unused structs in tests

9ec5690

Signed-off-by: Ben Ye <ben.ye@bytedance.com>

add licence header

d0bdcd5

Signed-off-by: Ben Ye <ben.ye@bytedance.com>

use upstream agent package

fb0dfba

Signed-off-by: Ben Ye <ben.ye@bytedance.com>

yeya24 force-pushed the remote-write-rule branch from ae51c39 to fb0dfba Compare November 7, 2021 07:38

GiedriusS previously approved these changes Nov 8, 2021

View reviewed changes

address comments

46acaee

Signed-off-by: Ben Ye <ben.ye@bytedance.com>

yeya24 dismissed GiedriusS’s stale review via 46acaee November 8, 2021 17:53

GiedriusS approved these changes Nov 8, 2021

View reviewed changes

GiedriusS enabled auto-merge (squash) November 8, 2021 18:08

GiedriusS merged commit ff286f1 into main Nov 8, 2021

yeya24 mentioned this pull request Nov 8, 2021

Implement stateless mode for Thanos Ruler #4250

Closed

2 tasks

GiedriusS deleted the remote-write-rule branch November 8, 2021 18:49

mousavian mentioned this pull request Nov 8, 2021

Upgrading prometheus/exporter-toolkit to add support for security-related HTTP headers #4840

Closed

2 tasks

bwplotka reviewed Nov 9, 2021

View reviewed changes

GiedriusS mentioned this pull request Nov 12, 2021

Implement Stateless Ruler Proposal #3761

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement stateless mode for Thanos Ruler (continue #4250) #4731

Implement stateless mode for Thanos Ruler (continue #4250) #4731

yeya24 commented Oct 3, 2021 •

edited

Loading

yeya24 commented Oct 3, 2021

rfratto commented Oct 4, 2021

rfratto commented Oct 8, 2021

yeya24 commented Oct 28, 2021

GiedriusS left a comment

GiedriusS Nov 1, 2021

yeya24 Nov 2, 2021

GiedriusS Nov 8, 2021

yeya24 commented Nov 2, 2021

yeya24 commented Nov 8, 2021

GiedriusS left a comment

GiedriusS Nov 8, 2021

GiedriusS Nov 8, 2021

yeya24 Nov 8, 2021

GiedriusS Nov 8, 2021

GiedriusS Nov 8, 2021

yeya24 commented Nov 8, 2021

GiedriusS left a comment

onprem commented Nov 9, 2021

bwplotka left a comment

Implement stateless mode for Thanos Ruler (continue #4250) #4731

Implement stateless mode for Thanos Ruler (continue #4250) #4731

Conversation

yeya24 commented Oct 3, 2021 • edited Loading

Changes

Verification

yeya24 commented Oct 3, 2021

rfratto commented Oct 4, 2021

rfratto commented Oct 8, 2021

yeya24 commented Oct 28, 2021

GiedriusS left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeya24 commented Nov 2, 2021

yeya24 commented Nov 8, 2021

GiedriusS left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeya24 commented Nov 8, 2021

GiedriusS left a comment

Choose a reason for hiding this comment

onprem commented Nov 9, 2021

bwplotka left a comment

Choose a reason for hiding this comment

yeya24 commented Oct 3, 2021 •

edited

Loading