From 93d35b012058f365dfbb38863529ec5f9f5bfb8e Mon Sep 17 00:00:00 2001 From: amitsetty Date: Fri, 2 Sep 2022 11:32:18 +0300 Subject: [PATCH 1/8] Add support for storage class configuration --- tempodb/backend/s3/config.go | 2 ++ tempodb/backend/s3/s3.go | 18 ++++++++------ tempodb/backend/s3/s3_test.go | 47 ++++++++++++++++++++++++++++++++--- 3 files changed, 56 insertions(+), 11 deletions(-) diff --git a/tempodb/backend/s3/config.go b/tempodb/backend/s3/config.go index f1ada21696e..1de2efbb64c 100644 --- a/tempodb/backend/s3/config.go +++ b/tempodb/backend/s3/config.go @@ -21,4 +21,6 @@ type Config struct { SignatureV2 bool `yaml:"signature_v2"` ForcePathStyle bool `yaml:"forcepathstyle"` Tags map[string]string `yaml:"tags"` + StorageClass string `yaml:"storage_class"` + UserMetadata map[string]string `yaml:"user_metadata"` } diff --git a/tempodb/backend/s3/s3.go b/tempodb/backend/s3/s3.go index 47c95ec129c..d1dc6b59c63 100644 --- a/tempodb/backend/s3/s3.go +++ b/tempodb/backend/s3/s3.go @@ -103,14 +103,20 @@ func internalNew(cfg *Config, confirm bool) (backend.RawReader, backend.RawWrite return rw, rw, rw, nil } +func getTempoObjectOptions(rw *readerWriter) minio.PutObjectOptions { + return minio.PutObjectOptions{ + PartSize: rw.cfg.PartSize, + UserTags: rw.cfg.Tags, + StorageClass: rw.cfg.StorageClass, + UserMetadata: rw.cfg.UserMetadata, + } +} + // Write implements backend.Writer func (rw *readerWriter) Write(ctx context.Context, name string, keypath backend.KeyPath, data io.Reader, size int64, _ bool) error { objName := backend.ObjectFileName(keypath, name) - putObjectOptions := minio.PutObjectOptions{ - PartSize: rw.cfg.PartSize, - UserTags: rw.cfg.Tags, - } + putObjectOptions := getTempoObjectOptions(rw) info, err := rw.core.Client.PutObject( ctx, @@ -138,9 +144,7 @@ func (rw *readerWriter) Append(ctx context.Context, name string, keypath backend var a appendTracker objectName := backend.ObjectFileName(keypath, name) - options := minio.PutObjectOptions{ - PartSize: rw.cfg.PartSize, - } + options := getTempoObjectOptions(rw) if tracker != nil { a = tracker.(appendTracker) } else { diff --git a/tempodb/backend/s3/s3_test.go b/tempodb/backend/s3/s3_test.go index e751b5c204e..7a6d8d18598 100644 --- a/tempodb/backend/s3/s3_test.go +++ b/tempodb/backend/s3/s3_test.go @@ -19,6 +19,9 @@ import ( "github.com/stretchr/testify/require" ) +const TAGHEADER = "X-Amz-Tagging" +const STORAGECLASSHEADER = "X-Amz-Storage-Class" + func TestHedge(t *testing.T) { tests := []struct { name string @@ -118,17 +121,17 @@ func TestReadError(t *testing.T) { assert.Equal(t, wups, errB) } -func fakeServerWithTags(t *testing.T, obj *url.Values) *httptest.Server { +func fakeServerWithHeader(testedHeaderName string, t *testing.T, obj *url.Values) *httptest.Server { require.NotNil(t, obj) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch method := r.Method; method { case "PUT": // https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObject.html - switch tagHeaders := r.Header.Get("X-Amz-Tagging"); tagHeaders { + switch testedHeaderValue := r.Header.Get(testedHeaderName); testedHeaderValue { case "": default: - value, err := url.ParseQuery(tagHeaders) + value, err := url.ParseQuery(testedHeaderValue) require.NoError(t, err) *obj = value } @@ -162,7 +165,7 @@ func TestObjectBlockTags(t *testing.T) { // rawObject := raw.Object{} var obj url.Values - server := fakeServerWithTags(t, &obj) + server := fakeServerWithHeader(TAGHEADER, t, &obj) _, w, _, err := New(&Config{ Region: "blerg", AccessKey: "test", @@ -185,3 +188,39 @@ func TestObjectBlockTags(t *testing.T) { }) } } + +func TestObjectStorageClass(t *testing.T) { + + tests := []struct { + name string + StorageClass string + // expectedObject raw.Object + }{ + { + "Standard", "STANDARD", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // rawObject := raw.Object{} + var obj url.Values + + server := fakeServerWithHeader(STORAGECLASSHEADER, t, &obj) + _, w, _, err := New(&Config{ + Region: "blerg", + AccessKey: "test", + SecretKey: flagext.SecretWithValue("test"), + Bucket: "blerg", + Insecure: true, + Endpoint: server.URL[7:], // [7:] -> strip http:// + StorageClass: tc.StorageClass, + }) + require.NoError(t, err) + + ctx := context.Background() + _ = w.Write(ctx, "object", backend.KeyPath{"test"}, bytes.NewReader([]byte{}), 0, false) + + }) + } +} From 669cde16dc8d02e970642803a3e9324e1f68f79d Mon Sep 17 00:00:00 2001 From: amitsetty Date: Fri, 2 Sep 2022 11:39:03 +0300 Subject: [PATCH 2/8] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8475f4ca723..563abb734f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * [CHANGE] Increase default values for `server.grpc_server_max_recv_msg_size` and `server.grpc_server_max_send_msg_size` from 4MB to 16MB [#1688](https://github.com/grafana/tempo/pull/1688) (@mapno) * [CHANGE] **BREAKING CHANGE** Use storage.trace.block.row_group_size_bytes to cut rows during compaction instead of compactor.compaction.flush_size_bytes. [#1696](https://github.com/grafana/tempo/pull/1696) (@joe-elliott) +* [FEATURE] Add capability to configure the used S3 Storage Class [#1697](https://github.com/grafana/tempo/pull/1714) (@amitsetty) * [ENHANCEMENT] metrics-generator: expose span size as a metric [#1662](https://github.com/grafana/tempo/pull/1662) (@ie-pham) * [ENHANCEMENT] Set Max Idle connections to 100 for Azure, should reduce DNS errors in Azure [#1632](https://github.com/grafana/tempo/pull/1632) (@electron0zero) * [ENHANCEMENT] Add PodDisruptionBudget to ingesters in jsonnet [#1691](https://github.com/grafana/tempo/pull/1691) (@joe-elliott) From bf026b9a3d8d11589bd63da50ea29c6e0554f174 Mon Sep 17 00:00:00 2001 From: amitsetty Date: Fri, 2 Sep 2022 18:20:20 +0300 Subject: [PATCH 3/8] Get rid of irrelevant character from changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88e562c313a..081fb39a727 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ * [CHANGE] **BREAKING CHANGE** Use storage.trace.block.row_group_size_bytes to cut rows during compaction instead of compactor.compaction.flush_size_bytes. [#1696](https://github.com/grafana/tempo/pull/1696) (@joe-elliott) * [FEATURE] Add capability to configure the used S3 Storage Class [#1697](https://github.com/grafana/tempo/pull/1714) (@amitsetty) -* [ENHANCEMENT] cache: expose username and sentinel_username redis configuration options for ACL-based Redis Auth support [#1708](https://github.com/grafana/tempo/pull/1708) (@jsievenpiper)9 +* [ENHANCEMENT] cache: expose username and sentinel_username redis configuration options for ACL-based Redis Auth support [#1708](https://github.com/grafana/tempo/pull/1708) (@jsievenpiper) * [ENHANCEMENT] metrics-generator: expose span size as a metric [#1662](https://github.com/grafana/tempo/pull/1662) (@ie-pham) * [ENHANCEMENT] Set Max Idle connections to 100 for Azure, should reduce DNS errors in Azure [#1632](https://github.com/grafana/tempo/pull/1632) (@electron0zero) * [ENHANCEMENT] Add PodDisruptionBudget to ingesters in jsonnet [#1691](https://github.com/grafana/tempo/pull/1691) (@joe-elliott) From 51cce886cdb689227a8013a0ef22de5eb7fcc360 Mon Sep 17 00:00:00 2001 From: amitsetty Date: Fri, 2 Sep 2022 18:35:37 +0300 Subject: [PATCH 4/8] CR Changes --- tempodb/backend/s3/s3.go | 6 +++--- tempodb/backend/s3/s3_test.go | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tempodb/backend/s3/s3.go b/tempodb/backend/s3/s3.go index d1dc6b59c63..85a17533901 100644 --- a/tempodb/backend/s3/s3.go +++ b/tempodb/backend/s3/s3.go @@ -103,7 +103,7 @@ func internalNew(cfg *Config, confirm bool) (backend.RawReader, backend.RawWrite return rw, rw, rw, nil } -func getTempoObjectOptions(rw *readerWriter) minio.PutObjectOptions { +func getPutObjectOptions(rw *readerWriter) minio.PutObjectOptions { return minio.PutObjectOptions{ PartSize: rw.cfg.PartSize, UserTags: rw.cfg.Tags, @@ -116,7 +116,7 @@ func getTempoObjectOptions(rw *readerWriter) minio.PutObjectOptions { func (rw *readerWriter) Write(ctx context.Context, name string, keypath backend.KeyPath, data io.Reader, size int64, _ bool) error { objName := backend.ObjectFileName(keypath, name) - putObjectOptions := getTempoObjectOptions(rw) + putObjectOptions := getPutObjectOptions(rw) info, err := rw.core.Client.PutObject( ctx, @@ -144,7 +144,7 @@ func (rw *readerWriter) Append(ctx context.Context, name string, keypath backend var a appendTracker objectName := backend.ObjectFileName(keypath, name) - options := getTempoObjectOptions(rw) + options := getPutObjectOptions(rw) if tracker != nil { a = tracker.(appendTracker) } else { diff --git a/tempodb/backend/s3/s3_test.go b/tempodb/backend/s3/s3_test.go index 7a6d8d18598..983dba14041 100644 --- a/tempodb/backend/s3/s3_test.go +++ b/tempodb/backend/s3/s3_test.go @@ -19,8 +19,8 @@ import ( "github.com/stretchr/testify/require" ) -const TAGHEADER = "X-Amz-Tagging" -const STORAGECLASSHEADER = "X-Amz-Storage-Class" +const tagHeader = "X-Amz-Tagging" +const storageClassHeader = "X-Amz-Storage-Class" func TestHedge(t *testing.T) { tests := []struct { @@ -131,7 +131,7 @@ func fakeServerWithHeader(testedHeaderName string, t *testing.T, obj *url.Values switch testedHeaderValue := r.Header.Get(testedHeaderName); testedHeaderValue { case "": default: - value, err := url.ParseQuery(testedHeaderValue) + value, err := url.ParseQuery(testedHeaderValueZ) require.NoError(t, err) *obj = value } @@ -165,7 +165,7 @@ func TestObjectBlockTags(t *testing.T) { // rawObject := raw.Object{} var obj url.Values - server := fakeServerWithHeader(TAGHEADER, t, &obj) + server := fakeServerWithHeader(tagHeader, t, &obj) _, w, _, err := New(&Config{ Region: "blerg", AccessKey: "test", @@ -206,7 +206,7 @@ func TestObjectStorageClass(t *testing.T) { // rawObject := raw.Object{} var obj url.Values - server := fakeServerWithHeader(STORAGECLASSHEADER, t, &obj) + server := fakeServerWithHeader(storageClassHeader, t, &obj) _, w, _, err := New(&Config{ Region: "blerg", AccessKey: "test", From 5560374784a48ac4ccdbe04f2b98fe183a325a17 Mon Sep 17 00:00:00 2001 From: amitsetty Date: Fri, 2 Sep 2022 18:37:16 +0300 Subject: [PATCH 5/8] Change order of parameters to fake server with header --- tempodb/backend/s3/s3_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tempodb/backend/s3/s3_test.go b/tempodb/backend/s3/s3_test.go index 983dba14041..f0e754a16d6 100644 --- a/tempodb/backend/s3/s3_test.go +++ b/tempodb/backend/s3/s3_test.go @@ -121,7 +121,7 @@ func TestReadError(t *testing.T) { assert.Equal(t, wups, errB) } -func fakeServerWithHeader(testedHeaderName string, t *testing.T, obj *url.Values) *httptest.Server { +func fakeServerWithHeader(t *testing.T, obj *url.Values, testedHeaderName string) *httptest.Server { require.NotNil(t, obj) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -165,7 +165,7 @@ func TestObjectBlockTags(t *testing.T) { // rawObject := raw.Object{} var obj url.Values - server := fakeServerWithHeader(tagHeader, t, &obj) + server := fakeServerWithHeader(t, &obj, tagHeader) _, w, _, err := New(&Config{ Region: "blerg", AccessKey: "test", @@ -206,7 +206,7 @@ func TestObjectStorageClass(t *testing.T) { // rawObject := raw.Object{} var obj url.Values - server := fakeServerWithHeader(storageClassHeader, t, &obj) + server := fakeServerWithHeader(t, &obj, storageClassHeader) _, w, _, err := New(&Config{ Region: "blerg", AccessKey: "test", From b458017b65894ab15f38cc51f7d172240a2f11a8 Mon Sep 17 00:00:00 2001 From: amitsetty Date: Fri, 2 Sep 2022 19:16:32 +0300 Subject: [PATCH 6/8] Add test to check that storage class value is added to header --- tempodb/backend/s3/config.go | 2 +- tempodb/backend/s3/s3.go | 2 +- tempodb/backend/s3/s3_test.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tempodb/backend/s3/config.go b/tempodb/backend/s3/config.go index 1de2efbb64c..bfec5efe5c2 100644 --- a/tempodb/backend/s3/config.go +++ b/tempodb/backend/s3/config.go @@ -22,5 +22,5 @@ type Config struct { ForcePathStyle bool `yaml:"forcepathstyle"` Tags map[string]string `yaml:"tags"` StorageClass string `yaml:"storage_class"` - UserMetadata map[string]string `yaml:"user_metadata"` + Metadata map[string]string `yaml:"metadata"` } diff --git a/tempodb/backend/s3/s3.go b/tempodb/backend/s3/s3.go index 85a17533901..1477d4d7cd3 100644 --- a/tempodb/backend/s3/s3.go +++ b/tempodb/backend/s3/s3.go @@ -108,7 +108,7 @@ func getPutObjectOptions(rw *readerWriter) minio.PutObjectOptions { PartSize: rw.cfg.PartSize, UserTags: rw.cfg.Tags, StorageClass: rw.cfg.StorageClass, - UserMetadata: rw.cfg.UserMetadata, + UserMetadata: rw.cfg.Metadata, } } diff --git a/tempodb/backend/s3/s3_test.go b/tempodb/backend/s3/s3_test.go index f0e754a16d6..ba3fbf4c86e 100644 --- a/tempodb/backend/s3/s3_test.go +++ b/tempodb/backend/s3/s3_test.go @@ -131,7 +131,7 @@ func fakeServerWithHeader(t *testing.T, obj *url.Values, testedHeaderName string switch testedHeaderValue := r.Header.Get(testedHeaderName); testedHeaderValue { case "": default: - value, err := url.ParseQuery(testedHeaderValueZ) + value, err := url.ParseQuery(testedHeaderValue) require.NoError(t, err) *obj = value } @@ -220,7 +220,7 @@ func TestObjectStorageClass(t *testing.T) { ctx := context.Background() _ = w.Write(ctx, "object", backend.KeyPath{"test"}, bytes.NewReader([]byte{}), 0, false) - + require.Equal(t, obj.Has(tc.StorageClass), true) }) } } From 75d3d9682dee49c49dbd2194443a7f40eca370b5 Mon Sep 17 00:00:00 2001 From: amitsetty <32575407+amitsetty@users.noreply.github.com> Date: Fri, 9 Sep 2022 12:50:24 +0300 Subject: [PATCH 7/8] Fix typo in changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58f32e9b7f0..9fe4fa18c6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ * [CHANGE] Improve parquet compaction memory profile when dropping spans [#1692](https://github.com/grafana/tempo/pull/1692) (@joe-elliott) * [CHANGE] Increase default values for `server.grpc_server_max_recv_msg_size` and `server.grpc_server_max_send_msg_size` from 4MB to 16MB [#1688](https://github.com/grafana/tempo/pull/1688) (@mapno) * [CHANGE] **BREAKING CHANGE** Use storage.trace.block.row_group_size_bytes to cut rows during compaction instead of - compactor.compaction.flush_size_bytes. [#1696](https://github.com/grafana/tempo/pull/1696) (@joe- + compactor.compaction.flush_size_bytes. [#1696](https://github.com/grafana/tempo/pull/1696) (@joe-elliott) * [CHANGE] Return 200 instead of 206 when blocks failed is < tolerate_failed_blocks. [#1725](https://github.com/grafana/tempo/pull/1725) (@joe-elliott) * [CHANGE] Update Go to 1.19 [#1665](https://github.com/grafana/tempo/pull/1665) (@ie-pham) elliott) From b55c4a5f6520123c09f26744eea86400c6308e46 Mon Sep 17 00:00:00 2001 From: amitsetty <32575407+amitsetty@users.noreply.github.com> Date: Fri, 9 Sep 2022 12:57:23 +0300 Subject: [PATCH 8/8] Fix changelog --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fe4fa18c6a..72cd3897ae7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,6 @@ compactor.compaction.flush_size_bytes. [#1696](https://github.com/grafana/tempo/pull/1696) (@joe-elliott) * [CHANGE] Return 200 instead of 206 when blocks failed is < tolerate_failed_blocks. [#1725](https://github.com/grafana/tempo/pull/1725) (@joe-elliott) * [CHANGE] Update Go to 1.19 [#1665](https://github.com/grafana/tempo/pull/1665) (@ie-pham) -elliott) * [FEATURE] Add capability to configure the used S3 Storage Class [#1697](https://github.com/grafana/tempo/pull/1714) (@amitsetty) * [ENHANCEMENT] cache: expose username and sentinel_username redis configuration options for ACL-based Redis Auth support [#1708](https://github.com/grafana/tempo/pull/1708) (@jsievenpiper) * [ENHANCEMENT] metrics-generator: expose span size as a metric [#1662](https://github.com/grafana/tempo/pull/1662) (@ie-pham)