feat(storage): GCS backend using thanos.io/objstore #11132

JoaoBraveCoding · 2023-11-03T17:46:35Z

What this PR does / why we need it:

This PR adds support to use the thanos.io/objstore backend for the GCS provider for all components ~~including the Ruler~~

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

We tried to the best of our efforts to preserve feature parity so the new backend still uses the ObjectClient interface
Thanos.io/objstore doesn't support all the CLI flags Loki supports nowadays (see snippet below)
Hedging support was added in GCS: Adds HTTP Config similar to S3 thanos-io/objstore#86 and 32a9e4c
This PR was inspired by the way Mimir uses thanos.io/objstore intending to make this part of the codebase as similar as possible
Support for rulers will be added in a follow-up pr feat(ruler): enables ruler store that uses clients from thanos-io/objstore pkg #11713
Congestion control support in a follow-up. pending on gcs: Add config to disable retries thanos-io/objstore#147
Named store support will be added separately

CLI Table

gcs.bucketname
gcs.service-account
gcs.chunk-buffer-size

NOT PRESENT AT THE MOMENT

gcs.enable-opencensus (Would need objstore patch)
gcs.enable-retries (we have an upstream patch)
gcs.request-timeout (will be added as a common option for all providers)
gcs.enable-http2 (not required for newer version of cristalhq/hedgedhttp)

Checklist

Reviewed the CONTRIBUTING.md guide (required)
Documentation added
Tests updated
CHANGELOG.md updated
- If the change is worth mentioning in the release notes, add add-to-release-notes label
Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

Signed-off-by: Joao Marcal <[email protected]>

github-actions · 2023-11-21T10:25:37Z

Trivy scan found the following vulnerabilities:

HIGH openssl: Incorrect cipher key and IV length processing in libcrypto3 v3.1.3-r0. Fixed in v3.1.4-r0
HIGH openssl: Incorrect cipher key and IV length processing in libssl3 v3.1.3-r0. Fixed in v3.1.4-r0

chaudum · 2023-11-23T13:49:40Z

This PR was inspired by the way Mimir uses thanos.io/objstore with the goal of making this part of the codebase as similar as possible

Thank you! ❤️

use a global thanos metric. remove ruler changes, will be added in a separate pr rename configs

DylanGuedes

I know that the change was introduced in a previous PR but anyway, now we're doing this everywhere:

		var objectClient client.ObjectClient
		var err error
		if t.Cfg.StorageConfig.UseThanosObjstore {
			objectClient, err = storage.NewObjectClientV2("compactor", periodConfig.ObjectType, t.Cfg.StorageConfig)
		} else {
			objectClient, err = storage.NewObjectClient(periodConfig.ObjectType, t.Cfg.StorageConfig, t.ClientMetrics)
		}
		if err != nil ...

WDYT of we instead moving the if conditional to the storage.NewObjectClientV2 instead, and adding a new parameter to it like legacy=true that dictates if internally it will instantiate V2 or V1?
the way we're doing here we're having to duplicate the condition everywhere (which has the risk of we missing something) and external libs will also have to add the same conditional.

…iod) Signed-off-by: Ashwanth Goli <[email protected]>

JoaoBraveCoding

Just some small comments but overall lgmt 🙌 thank you for the work to get this merged!

JoaoBraveCoding · 2024-10-15T10:49:22Z

pkg/storage/chunk/client/gcp/gcs_thanos_object_client.go

+	if err != nil {
+		size = -1
+		level.Warn(s.logger).Log("msg", "failed to get size of object", "err", err)
+	}


Here since we are always dealing with the gcs provider is this code path possible? IIUC the TryToGetSize will only produce an error if it gets an unsupported io.Reader

likely not for gcs, but it might also fail if the provider specific code fails to get the size for some reason. Seems very unlikely but I added this to not fail the Get request entirely if we cannot figure out the size. Size is only being used to reduce allocations, does not affect any functionality

JoaoBraveCoding · 2024-10-15T10:55:51Z

pkg/storage/bucket/client.go

+	ErrUnsupportedStorageBackend        = errors.New("unsupported storage backend")
+	ErrInvalidCharactersInStoragePrefix = errors.New("storage prefix contains invalid characters, it may only contain digits and English alphabet letters")
+
+	// TODO: Remove this global if we can tag all clients with component name


Even if we do this aren't we still going to hit duplicated metric registration for multiple schema_config?

yeah, i think the comment can be more descriptive. i'll fix it :)

docs/sources/shared/configuration.md

DylanGuedes · 2024-10-17T11:36:03Z

pkg/storage/chunk/client/object_client.go

+	// reset if GetObject fails to return a valid size
+	if size < 0 {
+		size = 0
+	}


hey would that be possible a bug on our side? If so, i think we should at least log something. If that's a known problem with the GCP API please ignore this comment.

we log it in the called method here

I added some context here about the check, but this is unlikely to fail. I'll monitor for errors as i test this out

DylanGuedes

lgtm just a quick question on #11132 (comment)

pull-request-size bot added the size/L label Nov 3, 2023

JoaoBraveCoding mentioned this pull request Nov 9, 2023

GCS: Adds HTTP Config similar to S3 thanos-io/objstore#86

Merged

2 tasks

JoaoBraveCoding changed the title ~~WIP: GCP implementation using thanos.io/objstore~~ GCP implementation using thanos.io/objstore Nov 14, 2023

JoaoBraveCoding added 4 commits November 14, 2023 10:22

GCP implementation using thanos.io/objstore

326d994

Implemented all ObjectClient methods

5960741

Cleaned up code

9878c97

Change Iter options to be more standard

f6ea86d

JoaoBraveCoding force-pushed the log-4550-gcp branch from a2f70a5 to daf3d62 Compare November 14, 2023 10:23

Allow support for ruler to use thanos/obj storage

0c88216

Signed-off-by: Joao Marcal <[email protected]>

JoaoBraveCoding force-pushed the log-4550-gcp branch from daf3d62 to 0c88216 Compare November 14, 2023 10:28

JoaoBraveCoding marked this pull request as ready for review November 14, 2023 10:30

JoaoBraveCoding requested a review from a team as a code owner November 14, 2023 10:30

JoaoBraveCoding changed the title ~~GCP implementation using thanos.io/objstore~~ storage: GCS backend using thanos.io/objstore Nov 14, 2023

JoaoBraveCoding mentioned this pull request Nov 14, 2023

feat(storage): AWS backend using thanos.io/objstore #11221

Open

8 tasks

Add thanos/objstore to common configuration

184ea74

pull-request-size bot added size/XL and removed size/L labels Nov 21, 2023

Fix metric registration by solving the "component" label conflict

e816a3c

JoaoBraveCoding force-pushed the log-4550-gcp branch from 6b6b613 to e816a3c Compare November 23, 2023 10:47

Merge branch 'main' into log-4550-gcp

1af852e

JoaoBraveCoding mentioned this pull request Nov 24, 2023

feat(storage): Azure backend using thanos.io/objstore #11315

Open

8 tasks

JoaoBraveCoding changed the title ~~storage: GCS backend using thanos.io/objstore~~ feat(storage): GCS backend using thanos.io/objstore Mar 14, 2024

Merge branch 'main' into log-4550-gcp

19f610c

JoaoBraveCoding force-pushed the log-4550-gcp branch from bc19a01 to 19f610c Compare March 14, 2024 11:56

pull-request-size bot added size/XXL and removed size/XL labels Mar 14, 2024

github-actions bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Mar 14, 2024

Merge branch 'main' into log-4550-gcp

80333a4

use global thanos metrics and other changes (#1)

ca835ad

use a global thanos metric. remove ruler changes, will be added in a separate pr rename configs

DylanGuedes reviewed Oct 9, 2024

View reviewed changes

ashwanthgoli added 7 commits October 9, 2024 15:56

use existing NewObjectClient method

36de56f

fixup! use existing NewObjectClient method

b057471

fix tests

f120df2

wire http conf + transport for gcs correctly

225866f

remove backend field, loki supports multiple backends (per period)

7ba4c3b

fixup! remove backend field, loki supports multiple backends (per per…

9977d5b

…iod) Signed-off-by: Ashwanth Goli <[email protected]>

update docs + config cleanup

0baea5a

pull-request-size bot added size/XL and removed size/XXL labels Oct 15, 2024

ashwanthgoli added 6 commits October 15, 2024 12:31

move storage backend to root block

df50926

fixup! move storage backend to root block

0aa6c3c

metric reg cleanup

5a1d2e5

remove sse limits, add back in s3 pr

a0478d0

use objstore.TryToGetSize

529f1d6

nits

5788eec

JoaoBraveCoding commented Oct 15, 2024

View reviewed changes

ashwanthgoli added 6 commits October 15, 2024 16:41

fix compile + update docs

f473928

Merge branch 'main' into log-4550-gcp

3da88e5

go mod tidy & vendor

1e6c079

fixup! go mod tidy & vendor

66dc091

fix lint

db57fca

add tests

6050e0b

DylanGuedes reviewed Oct 17, 2024

View reviewed changes

DylanGuedes approved these changes Oct 17, 2024

View reviewed changes

ashwanthgoli added 3 commits October 18, 2024 12:22

update docs and comments

45d33c3

Merge branch 'main' into log-4550-gcp

d7d0243

add back after fixing doc-gen

ef26a40

ashwanthgoli merged commit c059ace into grafana:main Oct 18, 2024
63 of 64 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(storage): GCS backend using thanos.io/objstore #11132

feat(storage): GCS backend using thanos.io/objstore #11132

JoaoBraveCoding commented Nov 3, 2023 •

edited by ashwanthgoli

Loading

github-actions bot commented Nov 21, 2023 •

edited

Loading

chaudum commented Nov 23, 2023

DylanGuedes left a comment •

edited

Loading

JoaoBraveCoding left a comment

JoaoBraveCoding Oct 15, 2024

ashwanthgoli Oct 15, 2024

JoaoBraveCoding Oct 15, 2024

ashwanthgoli Oct 15, 2024

DylanGuedes Oct 17, 2024

ashwanthgoli Oct 17, 2024

DylanGuedes left a comment •

edited

Loading

feat(storage): GCS backend using thanos.io/objstore #11132

feat(storage): GCS backend using thanos.io/objstore #11132

Conversation

JoaoBraveCoding commented Nov 3, 2023 • edited by ashwanthgoli Loading

github-actions bot commented Nov 21, 2023 • edited Loading

chaudum commented Nov 23, 2023

DylanGuedes left a comment • edited Loading

Choose a reason for hiding this comment

JoaoBraveCoding left a comment

Choose a reason for hiding this comment

JoaoBraveCoding Oct 15, 2024

Choose a reason for hiding this comment

ashwanthgoli Oct 15, 2024

Choose a reason for hiding this comment

JoaoBraveCoding Oct 15, 2024

Choose a reason for hiding this comment

ashwanthgoli Oct 15, 2024

Choose a reason for hiding this comment

DylanGuedes Oct 17, 2024

Choose a reason for hiding this comment

ashwanthgoli Oct 17, 2024

Choose a reason for hiding this comment

DylanGuedes left a comment • edited Loading

Choose a reason for hiding this comment

JoaoBraveCoding commented Nov 3, 2023 •

edited by ashwanthgoli

Loading

github-actions bot commented Nov 21, 2023 •

edited

Loading

DylanGuedes left a comment •

edited

Loading

DylanGuedes left a comment •

edited

Loading