Remove dependencies on thanos-io/thanos #3288

bboreham · 2022-10-22T13:05:56Z

What this PR does

Remove all dependencies on upstream project thanos-io/thanos, either by forking the necessary code into Mimir or by adding to upstream project dskit.

This makes it easier to adopt changes, for instance from Prometheus, that otherwise require synchronisation, local forks of Thanos, etc.

The PR is split into a number of commits which go package by package, to make it easier to follow the changes.

Builds on #2682 and #3222; needs upstream change grafana/dskit#228.

Notes

The function setupThanosTracing has been temporarily removed; as far as I can see the required hook is absent from thanos-io/objstore and have reported a bug there.
I excluded shipper_e2e_test.go and its associated test utils; this brought in large amounts of new dependencies via thanos/objstore to use different cloud providers that Mimir does not currently support.

Some packages already had some indirection in Mimir, and would benefit from further rationalisation:

cache
pool

Checklist

Tests updated
NA Documentation added
NA CHANGELOG.md updated - not a user-visible change.

pracucci

Massive work! LGTM! I just have a couple of comments I would be glad you to look at before merging:

Could you also doing the package import replace in the new tools/copyblocks/main.go?
Could you update the lint Makefile target, removing all Thanos faillint rules and adding a single rule like `faillint -paths "github.com/thanos-io/thanos/pkg" ./pkg/... ./cmd/... ./tools/... ./integration/...

pkg/cache/memcache_config.go

pkg/shipper/shipper_test.go

pkg/util/extprom/extprom.go

pracucci · 2022-10-25T12:14:57Z

I also want to take a look at the tracing thing, cause looks like an issue we already have in main. If so, not a blocker for this PR, but we need to fix it asap.

pkg/mimir/tracing.go

Part of breaking Mimir dependency on Thanos

Part of breaking Mimir dependency on Thanos. Needs a couple of extra metadata consts.

Part of breaking Mimir dependency on Thanos

bytes and dns packages are copied from Thanos; also the `DeleteAll` function in package runutil. Part of breaking Mimir dependency on Thanos.

Part of breaking Mimir dependency on Thanos.

Part of breaking Mimir dependency on Thanos

Add a `New()` function in `gate` package to simplify calling. Part of breaking Mimir dependency on Thanos.

Part of breaking Mimir dependency on Thanos.

Note there is no way to set up the tracing context now, hence tracing from the objstore package probably doesn't work. Part of breaking Mimir dependency on thanos-io/thanos

Part of breaking Mimir dependency on thanos-io/thanos.

Part of breaking Mimir dependency on Thanos.

Comments say never to call it.

Part of breaking Mimir dependency on Thanos.

Missed from original port. Part of breaking Mimir dependency on Thanos.

Note the previous rule had a typo. Part of breaking Mimir dependency on Thanos.

Prometheus client_go upstream has had this behaviour built in for 2 years. Part of breaking Mimir dependency on Thanos.

Should be less confusing in future. Part of breaking Mimir dependency on Thanos.

For consistency, and to reduce dependencies.

bboreham · 2022-10-27T12:13:11Z

Believe I have done everything you suggested.
~~Note this depends on upstream change grafana/dskit#228, so we should merge that first and update the dskit dependency back to main.~~
UPDATE: dskit dependency now merged and updated.

Missed from earlier import. Needs a couple of changes elsewhere. Some tests removed, either because they test code that was not brought into Mimir, or because they add dependencies on multiple cloud SDKs.

For consistency, and to reduce dependencies.

Instead of the PR branch.

pracucci

Sorry for the late review, LGTM! 🚀

* Fork Thanos metadata package Part of breaking Mimir dependency on Thanos * Use Mimir copy of metadata package Part of breaking Mimir dependency on Thanos * Fix typo mimit->mimir * Fork Thanos block package Part of breaking Mimir dependency on Thanos. Needs a couple of extra metadata consts. * Use Mimir copy of block package Part of breaking Mimir dependency on Thanos * Remove unused code from block package * Fork Thanos extprom package Part of breaking Mimir dependency on Thanos * Use Mimir copy of extprom package Part of breaking Mimir dependency on Thanos * Import dskit with bytes and dns packages bytes and dns packages are copied from Thanos; also the `DeleteAll` function in package runutil. Part of breaking Mimir dependency on Thanos. * Use dskit/runutil instead of thanos/pkg/runutil Part of breaking Mimir dependency on Thanos. * Use MultiError from dskit instead of Thanos Part of breaking Mimir dependency on Thanos. * Simplify test values in bucket e2e test * Fork Thanos cache package Part of breaking Mimir dependency on Thanos * Fork Thanos cacheutil package Part of breaking Mimir dependency on Thanos * Make cacheutil package use local packages Add a `New()` function in `gate` package to simplify calling. Part of breaking Mimir dependency on Thanos. * Use Mimir fork of cacheutil package Part of breaking Mimir dependency on Thanos. * Use thanos-io/objstore/tracing Note there is no way to set up the tracing context now, hence tracing from the objstore package probably doesn't work. Part of breaking Mimir dependency on thanos-io/thanos * Use Bytes type from dskit/flagext Part of breaking Mimir dependency on thanos-io/thanos. * Fork Thanos strutil package Part of breaking Mimir dependency on Thanos. * Add a test for util.MergeSlices * Stop calling strings.Compare Comments say never to call it. * Use Mimir version of MergeSlices Part of breaking Mimir dependency on Thanos. * Use dns package from dnskit instead of Thanos Part of breaking Mimir dependency on Thanos. * Remove unused Memcached AutoDiscovery Part of breaking Mimir dependency on Thanos. * Fork Thanos pool package Part of breaking Mimir dependency on Thanos. * Use Mimir copy of pool package Part of breaking Mimir dependency on Thanos. * Fork Thanos shipper package Did not include shipper_e2e_test.go, because it drags in dependencies on lots of new cloud providers. Part of breaking Mimir dependency on Thanos. * Use Mimir copy of shipper package Part of breaking Mimir dependency on Thanos. * go mod tidy, update vendor * Use Mimir copy of packages in copyblocks tool Missed from original port. Part of breaking Mimir dependency on Thanos. * Check that thanos-io/thanos is not used as import Note the previous rule had a typo. Part of breaking Mimir dependency on Thanos. * go mod tidy, update vendor * Update thanos-io/thanos for tracing fix * Re-add tracing for thanos-io/objstore * Remove unnecessary extprom.WrapRegistererWith Prometheus client_go upstream has had this behaviour built in for 2 years. Part of breaking Mimir dependency on Thanos. * Rename model.Bytes to flagext.Bytes Should be less confusing in future. Part of breaking Mimir dependency on Thanos. * Fix lint warnings from thanos code * Use testify instead of Thanos testutil For consistency, and to reduce dependencies. * Bring in tests for Thanos block package Missed from earlier import. Needs a couple of changes elsewhere. Some tests removed, either because they test code that was not brought into Mimir, or because they add dependencies on multiple cloud SDKs. * tsdb/block: use testify instead of Thanos testutil For consistency, and to reduce dependencies. * Update grafana/dskit to main branch Instead of the PR branch.

…t PUT on s3-compatible Object Storage. The grafana#3288 introduce `thanos-io/objstore` instead of `thanos-io/thanos` and brings along `github.com/minio/minio-go/v7 v7.0.37`. This version contains a bug who breaks the Multipart PUT on s3-compatible Object Storage (like the one at OVHcloud, Scaleway and Cloudflare). The error can be seen in the log as such: ``` upload chunks: upload file /data/mimir/tsdb/[redacted].mimir-continuous-test-100.eu/01GMV0ZR7740VNFM63DVMTQ25W/chunks/000001 as 01GMV0ZR7740VNFM63DVMTQ25W/chunks/000001: upload s3 object: The XML you provided was not well-formed or did not validate against our published schema ``` Related issues: * thanos-io/thanos#5857 * minio/minio-go#1702 Signed-off-by: Wilfried Roset <wilfriedroset@users.noreply.github.com>

…t PUT on s3-compatible Object Storage. (#3802) The #3288 introduce `thanos-io/objstore` instead of `thanos-io/thanos` and brings along `github.com/minio/minio-go/v7 v7.0.37`. This version contains a bug who breaks the Multipart PUT on s3-compatible Object Storage (like the one at OVHcloud, Scaleway and Cloudflare). The error can be seen in the log as such: ``` upload chunks: upload file /data/mimir/tsdb/[redacted].mimir-continuous-test-100.eu/01GMV0ZR7740VNFM63DVMTQ25W/chunks/000001 as 01GMV0ZR7740VNFM63DVMTQ25W/chunks/000001: upload s3 object: The XML you provided was not well-formed or did not validate against our published schema ``` Related issues: * thanos-io/thanos#5857 * minio/minio-go#1702 Signed-off-by: Wilfried Roset <wilfriedroset@users.noreply.github.com> Signed-off-by: Wilfried Roset <wilfriedroset@users.noreply.github.com>

bboreham requested a review from a team as a code owner October 22, 2022 13:05

osg-grafana mentioned this pull request Oct 24, 2022

Updated detailed steps for migrating blocks from Thanos to Mimir. #3290

Merged

3 tasks

pracucci self-requested a review October 25, 2022 11:40

pracucci reviewed Oct 25, 2022

View reviewed changes

pkg/cache/memcache_config.go Outdated Show resolved Hide resolved

pkg/shipper/shipper_test.go Outdated Show resolved Hide resolved

pkg/util/extprom/extprom.go Outdated Show resolved Hide resolved

pracucci reviewed Oct 25, 2022

View reviewed changes

pkg/mimir/tracing.go Outdated Show resolved Hide resolved

bboreham added 24 commits October 27, 2022 12:06

Fork Thanos metadata package

5cf8ea1

Part of breaking Mimir dependency on Thanos

Use Mimir copy of metadata package

43cdf6f

Part of breaking Mimir dependency on Thanos

Fix typo mimit->mimir

b919e45

Fork Thanos block package

87c0446

Part of breaking Mimir dependency on Thanos. Needs a couple of extra metadata consts.

Use Mimir copy of block package

2366cba

Part of breaking Mimir dependency on Thanos

Remove unused code from block package

231bb91

Fork Thanos extprom package

99d065c

Part of breaking Mimir dependency on Thanos

Use Mimir copy of extprom package

a88e55f

Part of breaking Mimir dependency on Thanos

Import dskit with bytes and dns packages

64d7413

bytes and dns packages are copied from Thanos; also the `DeleteAll` function in package runutil. Part of breaking Mimir dependency on Thanos.

Use dskit/runutil instead of thanos/pkg/runutil

62ea399

Part of breaking Mimir dependency on Thanos.

Use MultiError from dskit instead of Thanos

ebc6a2c

Part of breaking Mimir dependency on Thanos.

Simplify test values in bucket e2e test

8e6f7a3

Fork Thanos cache package

dc9fc77

Part of breaking Mimir dependency on Thanos

Fork Thanos cacheutil package

8702e63

Part of breaking Mimir dependency on Thanos

Make cacheutil package use local packages

8583399

Add a `New()` function in `gate` package to simplify calling. Part of breaking Mimir dependency on Thanos.

Use Mimir fork of cacheutil package

69f6a88

Part of breaking Mimir dependency on Thanos.

Use thanos-io/objstore/tracing

5cccc9d

Note there is no way to set up the tracing context now, hence tracing from the objstore package probably doesn't work. Part of breaking Mimir dependency on thanos-io/thanos

Use Bytes type from dskit/flagext

c86a582

Part of breaking Mimir dependency on thanos-io/thanos.

Fork Thanos strutil package

8971ed8

Part of breaking Mimir dependency on Thanos.

Add a test for util.MergeSlices

2414906

Stop calling strings.Compare

0b45264

Comments say never to call it.

Use Mimir version of MergeSlices

480ac16

Part of breaking Mimir dependency on Thanos.

Use dns package from dnskit instead of Thanos

0c72e48

Part of breaking Mimir dependency on Thanos.

Remove unused Memcached AutoDiscovery

00060de

Part of breaking Mimir dependency on Thanos.

bboreham added 11 commits October 27, 2022 12:08

Use Mimir copy of shipper package

06d3801

Part of breaking Mimir dependency on Thanos.

go mod tidy, update vendor

766d657

Use Mimir copy of packages in copyblocks tool

fdbe63f

Missed from original port. Part of breaking Mimir dependency on Thanos.

Check that thanos-io/thanos is not used as import

add6fad

Note the previous rule had a typo. Part of breaking Mimir dependency on Thanos.

go mod tidy, update vendor

69f084e

Update thanos-io/thanos for tracing fix

38526bc

Re-add tracing for thanos-io/objstore

18dc4cd

Remove unnecessary extprom.WrapRegistererWith

10c25f1

Prometheus client_go upstream has had this behaviour built in for 2 years. Part of breaking Mimir dependency on Thanos.

Rename model.Bytes to flagext.Bytes

cc58811

Should be less confusing in future. Part of breaking Mimir dependency on Thanos.

Fix lint warnings from thanos code

2cc3ea4

Use testify instead of Thanos testutil

2d7e149

For consistency, and to reduce dependencies.

bboreham force-pushed the no-thanos branch from 436bf3d to 2d7e149 Compare October 27, 2022 12:11

bboreham added 3 commits October 28, 2022 18:57

Bring in tests for Thanos block package

3f743ea

Missed from earlier import. Needs a couple of changes elsewhere. Some tests removed, either because they test code that was not brought into Mimir, or because they add dependencies on multiple cloud SDKs.

tsdb/block: use testify instead of Thanos testutil

942264b

For consistency, and to reduce dependencies.

Update grafana/dskit to main branch

a006daf

Instead of the PR branch.

pracucci approved these changes Nov 3, 2022

View reviewed changes

pracucci merged commit ef1a5bf into main Nov 3, 2022

pracucci deleted the no-thanos branch November 3, 2022 10:14

pracucci mentioned this pull request Nov 3, 2022

Remove Thanos ZLabel type in favour of identical LabelAdapter type #3345

Merged

1 task

pstibrany mentioned this pull request Nov 29, 2022

cleanup: Move used parts of pkg/shipper/shipper.go into pkg/ingester/shipper.go. #3566

Merged

bboreham mentioned this pull request Nov 30, 2022

Remove unused GroupKey function #3594

Merged

wilfriedroset mentioned this pull request Dec 22, 2022

Update github.com/thanos-io/objstore to address issue with Multipar… #3802

Merged

3 tasks

pracucci mentioned this pull request Jan 25, 2024

Chore: remove superfluous errors.Cause() usage in pkg/storage #7212

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove dependencies on thanos-io/thanos #3288

Remove dependencies on thanos-io/thanos #3288

bboreham commented Oct 22, 2022 •

edited

Loading

pracucci left a comment

pracucci commented Oct 25, 2022

bboreham commented Oct 27, 2022 •

edited

Loading

pracucci left a comment

Remove dependencies on thanos-io/thanos #3288

Remove dependencies on thanos-io/thanos #3288

Conversation

bboreham commented Oct 22, 2022 • edited Loading

What this PR does

Notes

Checklist

pracucci left a comment

Choose a reason for hiding this comment

pracucci commented Oct 25, 2022

bboreham commented Oct 27, 2022 • edited Loading

pracucci left a comment

Choose a reason for hiding this comment

bboreham commented Oct 22, 2022 •

edited

Loading

bboreham commented Oct 27, 2022 •

edited

Loading