From 701c9a04859a6285fc793886cc2bb0a0f79687ed Mon Sep 17 00:00:00 2001 From: Antonio Santos Date: Thu, 25 Apr 2019 23:58:03 +0200 Subject: [PATCH 1/9] Increase the response header timeout --- pkg/objstore/s3/s3.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/objstore/s3/s3.go b/pkg/objstore/s3/s3.go index 87c0710cc6..73d0fa076a 100644 --- a/pkg/objstore/s3/s3.go +++ b/pkg/objstore/s3/s3.go @@ -143,7 +143,7 @@ func NewBucketWithConfig(logger log.Logger, config Config, component string) (*B // The ResponseHeaderTimeout here is the only change from the // default minio transport, it was introduced to cover cases // where the tcp connection works but the server never answers - ResponseHeaderTimeout: 15 * time.Second, + ResponseHeaderTimeout: 2 * time.Minute, // Set this value so that the underlying transport round-tripper // doesn't try to auto decode the body of objects with // content-encoding set to `gzip`. From 08fc6cb9d2e97c422141ed854bdc06c7f81f75a0 Mon Sep 17 00:00:00 2001 From: Antonio Santos Date: Tue, 14 May 2019 11:33:18 +0200 Subject: [PATCH 2/9] Format --- pkg/objstore/s3/s3.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/objstore/s3/s3.go b/pkg/objstore/s3/s3.go index 73d0fa076a..97d47511d5 100644 --- a/pkg/objstore/s3/s3.go +++ b/pkg/objstore/s3/s3.go @@ -37,7 +37,7 @@ const DirDelim = "/" type Config struct { Bucket string `yaml:"bucket"` Endpoint string `yaml:"endpoint"` - Region string `yaml:"region"` + Region string `yaml:"region"` AccessKey string `yaml:"access_key"` Insecure bool `yaml:"insecure"` SignatureV2 bool `yaml:"signature_version2"` From 48268e91d565cb8f6ce5dd85db89692aa4b46c8a Mon Sep 17 00:00:00 2001 From: Antonio Santos Date: Tue, 14 May 2019 11:33:39 +0200 Subject: [PATCH 3/9] Make response header timeout configurable --- pkg/objstore/s3/s3.go | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/pkg/objstore/s3/s3.go b/pkg/objstore/s3/s3.go index 97d47511d5..cd58767b20 100644 --- a/pkg/objstore/s3/s3.go +++ b/pkg/objstore/s3/s3.go @@ -54,8 +54,9 @@ type TraceConfig struct { // HTTPConfig stores the http.Transport configuration for the s3 minio client. type HTTPConfig struct { - IdleConnTimeout model.Duration `yaml:"idle_conn_timeout"` - InsecureSkipVerify bool `yaml:"insecure_skip_verify"` + IdleConnTimeout model.Duration `yaml:"idle_conn_timeout"` + ResponseHeaderTimeout model.Duration `yaml:"response_header_timeout"` + InsecureSkipVerify bool `yaml:"insecure_skip_verify"` } // Bucket implements the store.Bucket interface against s3-compatible APIs. @@ -69,7 +70,10 @@ type Bucket struct { // parseConfig unmarshals a buffer into a Config with default HTTPConfig values. func parseConfig(conf []byte) (Config, error) { - defaultHTTPConfig := HTTPConfig{IdleConnTimeout: model.Duration(90 * time.Second)} + defaultHTTPConfig := HTTPConfig{ + IdleConnTimeout: model.Duration(90 * time.Second), + ResponseHeaderTimeout: model.Duration(2 * time.Minute), + } config := Config{HTTPConfig: defaultHTTPConfig} if err := yaml.Unmarshal(conf, &config); err != nil { return Config{}, err @@ -140,10 +144,11 @@ func NewBucketWithConfig(logger log.Logger, config Config, component string) (*B IdleConnTimeout: time.Duration(config.HTTPConfig.IdleConnTimeout), TLSHandshakeTimeout: 10 * time.Second, ExpectContinueTimeout: 1 * time.Second, - // The ResponseHeaderTimeout here is the only change from the - // default minio transport, it was introduced to cover cases - // where the tcp connection works but the server never answers - ResponseHeaderTimeout: 2 * time.Minute, + // The ResponseHeaderTimeout here is the only change + // from the default minio transport, it was introduced + // to cover cases where the tcp connection works but + // the server never answers. Defaults to 2 minutes. + ResponseHeaderTimeout: time.Duration(config.HTTPConfig.ResponseHeaderTimeout), // Set this value so that the underlying transport round-tripper // doesn't try to auto decode the body of objects with // content-encoding set to `gzip`. From d6e1ab7294342a408ebfc8bad494cc7a02f04d4a Mon Sep 17 00:00:00 2001 From: Antonio Santos Date: Tue, 14 May 2019 11:43:08 +0200 Subject: [PATCH 4/9] Test default values for the HTTPConfig struct --- pkg/objstore/s3/s3_test.go | 49 ++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/pkg/objstore/s3/s3_test.go b/pkg/objstore/s3/s3_test.go index c8d8204a46..9f47f0c496 100644 --- a/pkg/objstore/s3/s3_test.go +++ b/pkg/objstore/s3/s3_test.go @@ -7,12 +7,48 @@ import ( "github.com/improbable-eng/thanos/pkg/testutil" ) -func TestParseConfig_DefaultHTTPOpts(t *testing.T) { +func TestParseConfig(t *testing.T) { + input := []byte(`bucket: abcd +insecure: false`) + cfg, err := parseConfig(input) + testutil.Ok(t, err) + + if cfg.Bucket != "abcd" { + t.Errorf("parsing of bucket failed: got %v, expected %v", cfg.Bucket, "abcd") + } + if cfg.Insecure { + t.Errorf("parsing of insecure failed: got %v, expected %v", cfg.Insecure, false) + } +} + +func TestParseConfig_DefaultHTTPConfig(t *testing.T) { + input := []byte(`bucket: abcd +insecure: false`) + cfg, err := parseConfig(input) + testutil.Ok(t, err) + + if time.Duration(cfg.HTTPConfig.IdleConnTimeout) != time.Duration(90*time.Second) { + t.Errorf("parsing of idle_conn_timeout failed: got %v, expected %v", + time.Duration(cfg.HTTPConfig.IdleConnTimeout), time.Duration(90*time.Second)) + } + + if time.Duration(cfg.HTTPConfig.ResponseHeaderTimeout) != time.Duration(2*time.Minute) { + t.Errorf("parsing of response_header_timeout failed: got %v, expected %v", + time.Duration(cfg.HTTPConfig.IdleConnTimeout), time.Duration(2*time.Minute)) + } + + if cfg.HTTPConfig.InsecureSkipVerify { + t.Errorf("parsing of insecure_skip_verify failed: got %v, expected %v", cfg.HTTPConfig.InsecureSkipVerify, false) + } +} + +func TestParseConfig_CustomHTTPConfig(t *testing.T) { input := []byte(`bucket: abcd insecure: false http_config: insecure_skip_verify: true - idle_conn_timeout: 50s`) + idle_conn_timeout: 50s + response_header_timeout: 1m`) cfg, err := parseConfig(input) testutil.Ok(t, err) @@ -21,12 +57,11 @@ http_config: time.Duration(cfg.HTTPConfig.IdleConnTimeout), time.Duration(50*time.Second)) } - if cfg.Bucket != "abcd" { - t.Errorf("parsing of bucket failed: got %v, expected %v", cfg.Bucket, "abcd") - } - if cfg.Insecure { - t.Errorf("parsing of insecure failed: got %v, expected %v", cfg.Insecure, false) + if time.Duration(cfg.HTTPConfig.ResponseHeaderTimeout) != time.Duration(1*time.Minute) { + t.Errorf("parsing of response_header_timeout failed: got %v, expected %v", + time.Duration(cfg.HTTPConfig.IdleConnTimeout), time.Duration(1*time.Minute)) } + if !cfg.HTTPConfig.InsecureSkipVerify { t.Errorf("parsing of insecure_skip_verify failed: got %v, expected %v", cfg.HTTPConfig.InsecureSkipVerify, false) } From cfab9d9402634d6931d6b8cd523a2f56c3ec146c Mon Sep 17 00:00:00 2001 From: Antonio Santos Date: Tue, 14 May 2019 11:45:58 +0200 Subject: [PATCH 5/9] Document new feature in the changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4696f39871..af54d0bd82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ We use *breaking* word for marking changes that are not backward compatible (rel ## Unreleased +### Added + +- [#1094](https://github.com/improbable-eng/thanos/pull/1094) Allow configuring the response header timeout for the S3 client. + ## [v0.4.0](https://github.com/improbable-eng/thanos/releases/tag/v0.4.0) - 2019.05.3 :warning: **IMPORTANT** :warning: This is the last release that supports gossip. From Thanos v0.5.0, gossip will be completely removed. From c90dbcc9213137bdeecbf16eebd481d5f3a6fe7b Mon Sep 17 00:00:00 2001 From: Antonio Santos Date: Tue, 14 May 2019 15:34:25 +0200 Subject: [PATCH 6/9] Add explanation for new option and refactor existing documentation --- docs/storage.md | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/docs/storage.md b/docs/storage.md index 8cd58a3dc7..4bc5906402 100644 --- a/docs/storage.md +++ b/docs/storage.md @@ -39,9 +39,9 @@ At that point, anyone can use your provider by spec ## AWS S3 configuration -Thanos uses minio client to upload Prometheus data into AWS S3. +Thanos uses the [minio client](https://github.com/minio/minio-go) library to upload Prometheus data into AWS S3. -To configure S3 bucket as an object store you need to set these mandatory S3 variables in yaml format stored in a file: +You can configure an S3 bucket as an object store with YAML, either by passing the configuration directly to the `--objstore.config` parameter, or (preferably) by passing the path to a configuration file to the `--objstore.config-file` option. [embedmd]:# (flags/config_s3.txt yaml) ```yaml @@ -49,29 +49,37 @@ type: S3 config: bucket: "" endpoint: "" - region: "" access_key: "" + secret_key: "" + region: "" insecure: false signature_version2: false encrypt_sse: false - secret_key: "" put_user_metadata: {} http_config: idle_conn_timeout: 0s + response_header_timeout: 0s insecure_skip_verify: false trace: enable: false ``` -The attribute `region` is optional. AWS region to endpoint mapping can be found in this [link](https://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region). +As a minimum, you will need to provide a value for the `bucket`, `endpoint`, `access_key`, and `secret_key` keys. The rest of the keys are optional. + +The AWS region to endpoint mapping can be found in this [link](https://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region). -Make sure you use a correct signature version. -Currently AWS require signature v4, so it needs `signature-version2: false`, otherwise, you will get Access Denied error, but several other S3 compatible use `signature-version2: true` +Make sure you use a correct signature version. Currently AWS requires signature v4, so it needs `signature-version2: false`. If you don't specify it, you will get an `Access Denied` error. On the other hand, several S3 compatible APIs use `signature-version2: true`. + +You can configure the timeout settings for the HTTP client by setting the `http_config.idle_conn_timeout` and `http_config.response_header_timeout` keys. As a rule of thumb, if you are seeing errors like `timeout awaiting response headers` in your logs, you may want to increase the value of `http_config.response_header_timeout`. + +Please refer to the documentation of [the Transport type](https://golang.org/pkg/net/http/#Transport) in the `net/http` package for detailed information on what each option does. For debug and testing purposes you can set * `insecure: true` to switch to plain insecure HTTP instead of HTTPS + * `http_config.insecure_skip_verify: true` to disable TLS certificate verification (if your S3 based storage is using a self-signed certificate, for example) + * `trace.enable: true` to enable the minio client's verbose logging. Each request and response will be logged into the debug logger, so debug level logging must be enabled for this functionality. ### Credentials From 0fe53e017a9487a84ba2f553a772b98403d0f98b Mon Sep 17 00:00:00 2001 From: Antonio Santos Date: Tue, 14 May 2019 15:57:21 +0200 Subject: [PATCH 7/9] Run make docs --- docs/storage.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/storage.md b/docs/storage.md index 4bc5906402..b27de11237 100644 --- a/docs/storage.md +++ b/docs/storage.md @@ -49,12 +49,12 @@ type: S3 config: bucket: "" endpoint: "" - access_key: "" - secret_key: "" region: "" + access_key: "" insecure: false signature_version2: false encrypt_sse: false + secret_key: "" put_user_metadata: {} http_config: idle_conn_timeout: 0s From 60f848360627733e5fa16242e81b42d3dd4e1764 Mon Sep 17 00:00:00 2001 From: Antonio Santos Date: Tue, 14 May 2019 17:36:21 +0200 Subject: [PATCH 8/9] Typo --- docs/storage.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/storage.md b/docs/storage.md index b27de11237..38d02150fa 100644 --- a/docs/storage.md +++ b/docs/storage.md @@ -64,7 +64,7 @@ config: enable: false ``` -As a minimum, you will need to provide a value for the `bucket`, `endpoint`, `access_key`, and `secret_key` keys. The rest of the keys are optional. +At a minimum, you will need to provide a value for the `bucket`, `endpoint`, `access_key`, and `secret_key` keys. The rest of the keys are optional. The AWS region to endpoint mapping can be found in this [link](https://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region). From 2eaff03f8ba02df22a90d85bceee2c2c0d603c1e Mon Sep 17 00:00:00 2001 From: Antonio Santos Date: Thu, 16 May 2019 09:50:07 +0200 Subject: [PATCH 9/9] Remove redundant blank line --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38d63b5302..6a2903b9d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,7 +34,6 @@ We use *breaking* word for marking changes that are not backward compatible (rel - [#1144](https://github.com/improbable-eng/thanos/pull/1144) Query/API: properly pass the downsampling parameter. Before this, wrong max resolution of the metrics data might have been selected. - ## [v0.4.0](https://github.com/improbable-eng/thanos/releases/tag/v0.4.0) - 2019.05.3 :warning: **IMPORTANT** :warning: This is the last release that supports gossip. From Thanos v0.5.0, gossip will be completely removed.