Add metrics backfill support to mimirtool #1822

aknuds1 · 2022-05-05T14:27:35Z

What this PR does

Add support for metrics backfill to mimirtool.

TODOs:

Take individual blocks instead of a directory of blocks
Write integration tests that spin up Mimir and try to upload different blocks ~~(Prometheus/Thanos/Mimir)~~

Which issue(s) this PR fixes or relates to

Checklist

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

CLAassistant · 2022-06-02T22:21:25Z

All committers have signed the CLA.

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

pkg/mimirtool/client/backfill.go

pstibrany · 2022-06-24T15:31:35Z

pkg/mimirtool/client/backfill.go

+	}
+
+	// Upload each block file
+	if err := filepath.WalkDir(dpath, func(pth string, e fs.DirEntry, err error) error {


can we gather files to upload first, and then upload them concurrently to speed up the process?

pstibrany · 2022-06-24T15:35:34Z

pkg/mimirtool/client/backfill.go

+	}
+	for _, e := range es {
+		if err := c.backfillBlock(ctx, filepath.Join(source, e.Name()), logger); err != nil {
+			return err


We will need to react on various errors differently, and keep going even if some blocks fail to upload.

We should give user a summary at the end of how many blocks uploaded correctly, how many already existed, and how many failed to upload.

Final exit code should depend on this too. If all blocks uploaded correctly or already existed, we can report exit code 0 (all good!). If there were some non-recoverable upload errors, let's use exit code 1.

@pstibrany do you know how to make the Kingpin framework exit with a certain code (e.g. 1)? Should I just implement this by returning an error if one or more blocks failed to upload?

I don't know. I would try the same (returning error) and if that fails, check Kingpin framwork documentation :)

pstibrany · 2022-06-24T15:36:49Z

pkg/mimirtool/client/client.go

@@ -104,20 +104,19 @@ func New(cfg Config) (*MimirClient, error) {

 // Query executes a PromQL query against the Mimir cluster.
 func (r *MimirClient) Query(ctx context.Context, query string) (*http.Response, error) {
-
 	query = fmt.Sprintf("query=%s&time=%d", query, time.Now().Unix())
 	escapedQuery := url.PathEscape(query)


This escaping is wrong for query string. We should use "url.QueryEscape", and apply it on each query parameter individually. Escaping whole query string is wrong.

@pstibrany thanks, fixed. It's an old problem though, code isn't introduced by my PR :) PTAL.

You're right. Let's move that to separate PR.

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

pstibrany · 2022-07-18T16:06:23Z

I've fixed logging (#1822 (comment)), so now mimirtool backfill provides this output:

INFO[0000] Backfilling                                   blocks="/opt/homebrew/var/prometheus/01G7RCEZSF6H4J9BD8JFYQP7R7,/opt/homebrew/var/prometheus/01G7TAB1MA64QV9DSBVRP4WFNE,/opt/homebrew/var/prometheus/01G7W908TRFGK9KV37RQSBTBB8,/opt/homebrew/var/prometheus/01G7Y5VFKEHXRWAFZ4GXTTVCPQ" user=anonymous
INFO[0000] making request to start block upload          block=01G7RCEZSF6H4J9BD8JFYQP7R7 file=meta.json path=/opt/homebrew/var/prometheus/01G7RCEZSF6H4J9BD8JFYQP7R7
INFO[0000] uploading block file                          block=01G7RCEZSF6H4J9BD8JFYQP7R7 file=index path=/opt/homebrew/var/prometheus/01G7RCEZSF6H4J9BD8JFYQP7R7 size=283551
INFO[0000] uploading block file                          block=01G7RCEZSF6H4J9BD8JFYQP7R7 file=chunks/000001 path=/opt/homebrew/var/prometheus/01G7RCEZSF6H4J9BD8JFYQP7R7 size=6294804
INFO[0000] block uploaded successfully                   block=01G7RCEZSF6H4J9BD8JFYQP7R7 path=/opt/homebrew/var/prometheus/01G7RCEZSF6H4J9BD8JFYQP7R7
INFO[0000] making request to start block upload          block=01G7TAB1MA64QV9DSBVRP4WFNE file=meta.json path=/opt/homebrew/var/prometheus/01G7TAB1MA64QV9DSBVRP4WFNE
INFO[0000] uploading block file                          block=01G7TAB1MA64QV9DSBVRP4WFNE file=index path=/opt/homebrew/var/prometheus/01G7TAB1MA64QV9DSBVRP4WFNE size=274240
INFO[0000] uploading block file                          block=01G7TAB1MA64QV9DSBVRP4WFNE file=chunks/000001 path=/opt/homebrew/var/prometheus/01G7TAB1MA64QV9DSBVRP4WFNE size=6452725
INFO[0001] block uploaded successfully                   block=01G7TAB1MA64QV9DSBVRP4WFNE path=/opt/homebrew/var/prometheus/01G7TAB1MA64QV9DSBVRP4WFNE
INFO[0001] making request to start block upload          block=01G7W908TRFGK9KV37RQSBTBB8 file=meta.json path=/opt/homebrew/var/prometheus/01G7W908TRFGK9KV37RQSBTBB8
INFO[0001] uploading block file                          block=01G7W908TRFGK9KV37RQSBTBB8 file=index path=/opt/homebrew/var/prometheus/01G7W908TRFGK9KV37RQSBTBB8 size=276571
INFO[0001] uploading block file                          block=01G7W908TRFGK9KV37RQSBTBB8 file=chunks/000001 path=/opt/homebrew/var/prometheus/01G7W908TRFGK9KV37RQSBTBB8 size=5507840
INFO[0002] block uploaded successfully                   block=01G7W908TRFGK9KV37RQSBTBB8 path=/opt/homebrew/var/prometheus/01G7W908TRFGK9KV37RQSBTBB8
INFO[0002] making request to start block upload          block=01G7Y5VFKEHXRWAFZ4GXTTVCPQ file=meta.json path=/opt/homebrew/var/prometheus/01G7Y5VFKEHXRWAFZ4GXTTVCPQ
INFO[0002] uploading block file                          block=01G7Y5VFKEHXRWAFZ4GXTTVCPQ file=index path=/opt/homebrew/var/prometheus/01G7Y5VFKEHXRWAFZ4GXTTVCPQ size=244971
INFO[0002] uploading block file                          block=01G7Y5VFKEHXRWAFZ4GXTTVCPQ file=chunks/000001 path=/opt/homebrew/var/prometheus/01G7Y5VFKEHXRWAFZ4GXTTVCPQ size=4917324
INFO[0002] block uploaded successfully                   block=01G7Y5VFKEHXRWAFZ4GXTTVCPQ path=/opt/homebrew/var/prometheus/01G7Y5VFKEHXRWAFZ4GXTTVCPQ
INFO[0002] finished uploading blocks                     already_exists=0 failed=0 succeeded=4

In case of errors:

INFO[0000] Backfilling                                   blocks="/opt/homebrew/var/prometheus/01G7RCEZSF6H4J9BD8JFYQP7R7,/opt/homebrew/var/prometheus/01G7TAB1MA64QV9DSBVRP4WFNE,/opt/homebrew/var/prometheus/01G7W908TRFGK9KV37RQSBTBB8,/opt/homebrew/var/prometheus/01G7Y5VFKEHXRWAFZ4GXTTVCPQ" user=anonymous
INFO[0000] making request to start block upload          block=01G7RCEZSF6H4J9BD8JFYQP7R7 file=meta.json path=/opt/homebrew/var/prometheus/01G7RCEZSF6H4J9BD8JFYQP7R7
ERRO[0000] server returned HTTP status 400 Bad Request: block upload is disabled  status="400 Bad Request"
ERRO[0000] failed uploading block                        error="request to start block upload failed: POST request to http://localhost:8006/api/v1/upload/block/01G7RCEZSF6H4J9BD8JFYQP7R7 failed: server returned HTTP status 400 Bad Request: block upload is disabled" path=/opt/homebrew/var/prometheus/01G7RCEZSF6H4J9BD8JFYQP7R7
INFO[0000] making request to start block upload          block=01G7TAB1MA64QV9DSBVRP4WFNE file=meta.json path=/opt/homebrew/var/prometheus/01G7TAB1MA64QV9DSBVRP4WFNE
ERRO[0000] server returned HTTP status 400 Bad Request: block upload is disabled  status="400 Bad Request"
ERRO[0000] failed uploading block                        error="request to start block upload failed: POST request to http://localhost:8006/api/v1/upload/block/01G7TAB1MA64QV9DSBVRP4WFNE failed: server returned HTTP status 400 Bad Request: block upload is disabled" path=/opt/homebrew/var/prometheus/01G7TAB1MA64QV9DSBVRP4WFNE
INFO[0000] making request to start block upload          block=01G7W908TRFGK9KV37RQSBTBB8 file=meta.json path=/opt/homebrew/var/prometheus/01G7W908TRFGK9KV37RQSBTBB8
ERRO[0000] server returned HTTP status 400 Bad Request: block upload is disabled  status="400 Bad Request"
ERRO[0000] failed uploading block                        error="request to start block upload failed: POST request to http://localhost:8006/api/v1/upload/block/01G7W908TRFGK9KV37RQSBTBB8 failed: server returned HTTP status 400 Bad Request: block upload is disabled" path=/opt/homebrew/var/prometheus/01G7W908TRFGK9KV37RQSBTBB8
INFO[0000] making request to start block upload          block=01G7Y5VFKEHXRWAFZ4GXTTVCPQ file=meta.json path=/opt/homebrew/var/prometheus/01G7Y5VFKEHXRWAFZ4GXTTVCPQ
ERRO[0000] server returned HTTP status 400 Bad Request: block upload is disabled  status="400 Bad Request"
ERRO[0000] failed uploading block                        error="request to start block upload failed: POST request to http://localhost:8006/api/v1/upload/block/01G7Y5VFKEHXRWAFZ4GXTTVCPQ failed: server returned HTTP status 400 Bad Request: block upload is disabled" path=/opt/homebrew/var/prometheus/01G7Y5VFKEHXRWAFZ4GXTTVCPQ
INFO[0000] finished uploading blocks                     already_exists=0 failed=4 succeeded=0
mimirtool: error: blocks failed to upload 4 block(s), try --help

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

…gration test. Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

pracucci

Good job! I mostly focused on UX, and I left few comments I would be glad if you could take a look at. Other than comments:

We need a CHANGELOG entry.
We should update the doc at docs/sources/operators-guide/tools/mimirtool.md. Not required to be done in this PR, but I just want to make sure we'll update it.

pkg/mimirtool/commands/backfill.go

pracucci · 2022-07-19T11:32:16Z

pkg/mimirtool/commands/backfill.go

+	cmd := app.Command("backfill", "Upload metrics blocks to Grafana Mimir.")
+	cmd.Action(c.backfill)
+	cmd.Arg("block", "block to upload").Required().SetValue(&c.blocks)
+	cmd.Flag("address", "Address of the Grafana Mimir cluster").Required().StringVar(&c.clientConfig.Address)


Which component specifically? I would be more clear.

I don't think we should. The fact that the endpoint is handled by compactor is not relevant for mimirtool.

I don't feel strong, but an OSS user deploying in microservices mode (e.g. using Helm) would be facilitated if we just tell them to which microservice the request should be sent to.

Perhaps we should include the compactor endpoint in default nginx configuration instead?

I checked what we do in other commands ("rules", "alerts") and we do mention "ruler" or "Alertmanager" in those, although not in address option. What about we modify main backfill --help description:

Upload Prometheus TSDB blocks to Grafana Mimir compactor.

WDYT?

pkg/mimirtool/commands/backfill.go

pkg/mimirtool/client/backfill.go

integration/backfill_test.go

Co-authored-by: Marco Pracucci <marco@pracucci.com>

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

pracucci

Good job, LGTM! I would still reconsider the -user CLI flag (see dedicated comment).

pkg/mimirtool/commands/backfill.go

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

pstibrany · 2022-07-20T10:08:43Z

Documentation PR: #2481

aknuds1 added the enhancement New feature or request label May 5, 2022

aknuds1 force-pushed the feat/backfill-mimirtool branch from 2ff118e to f07fac1 Compare May 19, 2022 16:53

aknuds1 added the component/mimirtool label May 21, 2022

aknuds1 force-pushed the feat/backfill-mimirtool branch 2 times, most recently from 6a36db9 to eed40f0 Compare June 2, 2022 13:40

aknuds1 force-pushed the feat/backfill-mimirtool branch 4 times, most recently from 55984f3 to dd1d8f4 Compare June 24, 2022 06:22

mimirtool: Add backfill sub-command

b127814

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

aknuds1 force-pushed the feat/backfill-mimirtool branch from dd1d8f4 to b127814 Compare June 24, 2022 11:51

pstibrany reviewed Jun 24, 2022

View reviewed changes

aknuds1 added 12 commits June 27, 2022 13:41

Merge remote-tracking branch 'origin/main' into feat/backfill-mimirtool

b3aafec

Generate file index

e8bbd4e

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Escape URL path

4580218

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Merge remote-tracking branch 'origin/main' into feat/backfill-mimirtool

e90ba95

Merge remote-tracking branch 'origin/main' into feat/backfill-mimirtool

faa092d

Merge remote-tracking branch 'origin/main' into feat/backfill-mimirtool

139fb08

Drain response body before closing

4b6b15b

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

mimirtool: Make sure file path is Unix style

74b9908

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

mimirtool: block instead of block_id in log messages

dad4371

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

mimirtool: Improve error message

15f5e0e

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

mimirtool: Fix query escaping

37d6b68

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

mimirtool: Simplify logging

b8e35bb

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

aknuds1 force-pushed the feat/backfill-mimirtool branch from 5a37110 to b8e35bb Compare June 28, 2022 09:41

aknuds1 added 4 commits June 28, 2022 15:27

Print report at end of block upload

5c77b35

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Merge remote-tracking branch 'origin/main' into feat/backfill-mimirtool

9f0ebd2

Remove TODO

e45db6a

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

mimirtool: Report blocks that already exists

2eeca94

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

aknuds1 and others added 6 commits July 15, 2022 13:02

Refactor file upload, to expedite file closing

c0a8ef0

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Merge remote-tracking branch 'origin/main' into feat/backfill-mimirtool

947fad3

Add test skeleton

616c6a6

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

Add test for mimirtool backfill.

1187600

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

Use logrus library only, don't mix libraries.

2656960

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

msg is already included in errMsg, don't log it twice.

b3875a6

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

pstibrany added 3 commits July 18, 2022 18:08

Revert change in client.go, let's do that in separate PR.

83a8351

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

Remove some repeated code in getBlockMeta. Added comments. Fixed inte…

9b935a2

…gration test. Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

Revert unrelated code.

aab71e6

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

pstibrany marked this pull request as ready for review July 19, 2022 08:44

pstibrany requested a review from pracucci July 19, 2022 08:44

pstibrany changed the title ~~WIP: Add metrics backfill support to mimirtool~~ Add metrics backfill support to mimirtool Jul 19, 2022

pstibrany added 2 commits July 19, 2022 10:48

Upload block with external labels.

b306351

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

Merge branch 'main' into feat/backfill-mimirtool

a4f2fce

pracucci reviewed Jul 19, 2022

View reviewed changes

pstibrany and others added 7 commits July 19, 2022 15:13

Update pkg/mimirtool/commands/backfill.go

a67fbad

Co-authored-by: Marco Pracucci <marco@pracucci.com>

Extend parameters config.

9c63808

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

Remove unused context.

7e7a632

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

Remove unnecessary status code checks.

75c8ba1

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

Extract test_util.CapturedOutput.

d4b619d

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

Add license header.

7543424

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

Added CHANGELOG.md entry.

89dcd88

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

pracucci approved these changes Jul 20, 2022

View reviewed changes

pkg/mimirtool/commands/backfill.go Show resolved Hide resolved

pstibrany added 2 commits July 20, 2022 11:01

Add back user option.

30f970d

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

Use block-dir, mention compactor in main help.

4e7dc5b

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>

pstibrany merged commit 96a9824 into main Jul 20, 2022

pstibrany deleted the feat/backfill-mimirtool branch July 20, 2022 10:08

pracucci mentioned this pull request Aug 25, 2022

Fix mimirtool commands issued to API returning 404 #2834

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add metrics backfill support to mimirtool #1822

Add metrics backfill support to mimirtool #1822

aknuds1 commented May 5, 2022 •

edited by pstibrany

Loading

CLAassistant commented Jun 2, 2022 •

edited

Loading

pstibrany Jun 24, 2022

pstibrany Jun 24, 2022

aknuds1 Jun 28, 2022

pstibrany Jun 29, 2022

pstibrany Jun 24, 2022

aknuds1 Jun 28, 2022 •

edited

Loading

pstibrany Jul 19, 2022

pstibrany Jul 19, 2022

pstibrany commented Jul 18, 2022

pracucci left a comment

pracucci Jul 19, 2022

pstibrany Jul 19, 2022

pracucci Jul 20, 2022

pstibrany Jul 20, 2022

pracucci left a comment

pstibrany commented Jul 20, 2022

Add metrics backfill support to mimirtool #1822

Add metrics backfill support to mimirtool #1822

Conversation

aknuds1 commented May 5, 2022 • edited by pstibrany Loading

What this PR does

TODOs:

Which issue(s) this PR fixes or relates to

Checklist

CLAassistant commented Jun 2, 2022 • edited Loading

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

aknuds1 Jun 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pstibrany commented Jul 18, 2022

pracucci 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

pracucci left a comment

Choose a reason for hiding this comment

pstibrany commented Jul 20, 2022

aknuds1 commented May 5, 2022 •

edited by pstibrany

Loading

CLAassistant commented Jun 2, 2022 •

edited

Loading

aknuds1 Jun 28, 2022 •

edited

Loading