Skip to content

Commit

Permalink
Merge branch 'main' into charleskorn/jaeger-queue-size
Browse files Browse the repository at this point in the history
  • Loading branch information
charleskorn committed Jan 9, 2024
2 parents 1471182 + b533458 commit 67fae19
Show file tree
Hide file tree
Showing 61 changed files with 1,607 additions and 1,467 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@
* [CHANGE] Validate tenant IDs according to [documented behavior](https://grafana.com/docs/mimir/latest/configure/about-tenant-ids/) even when tenant federation is not enabled. Note that this will cause some previously accepted tenant IDs to be rejected such as those longer than 150 bytes or containing `|` characters. #6959
* [CHANGE] Ruler: don't use backoff retry on remote evaluation in case of `4xx` errors. #7004
* [CHANGE] Server: responses with HTTP 4xx status codes are now treated as errors and used in `status_code` label of request duration metric. #7045
* [CHANGE] Memberlist: change default for `-memberlist.stream-timeout` from `10s` to `2s`. #7076
* [CHANGE] Memcached: remove legacy `thanos_cache_memcached_*` and `thanos_memcached_*` prefixed metrics. Instead, Memcached and Redis cache clients now emit `thanos_cache_*` prefixed metrics with a `backend` label. #7076
* [ENHANCEMENT] Store-gateway: add no-compact details column on store-gateway tenants admin UI. #6848
* [ENHANCEMENT] PromQL: ignore small errors for bucketQuantile #6766
* [ENHANCEMENT] Distributor: improve efficiency of some errors #6785
* [ENHANCEMENT] Ruler: exclude vector queries from being tracked in `cortex_ruler_queries_zero_fetched_series_total`. #6544
* [ENHANCEMENT] Query-Frontend and Query-Scheduler: split tenant query request queues by query component with `query-frontend.additional-query-queue-dimensions-enabled` and `query-scheduler.additional-query-queue-dimensions-enabled`. #6772
* [ENHANCEMENT] Distributor: support disabling metric relabel rules per-tenant via the flag `-distributor.metric-relabeling-enabled` or associated YAML. #6970
* [ENHANCEMENT] Distributor: `-distributor.remote-timeout` is now accounted from the first ingester push request being sent. #6972
* [ENHANCEMENT] Storage Provider: allow aws sts support for s3 storage provider #6172
* [FEATURE] Introduce `-tenant-federation.max-tenants` option to limit the max number of tenants allowed for requests when federation is enabled. #6959
* [ENHANCEMENT] Query-frontend: add experimental support for sharding active series queries via `-query-frontend.shard-active-series-queries`. #6784
* [BUGFIX] Ingester: don't ignore errors encountered while iterating through chunks or samples in response to a query request. #6451
Expand Down
42 changes: 41 additions & 1 deletion cmd/mimir/config-descriptor.json
Original file line number Diff line number Diff line change
Expand Up @@ -5824,6 +5824,16 @@
"fieldType": "boolean",
"fieldCategory": "experimental"
},
{
"kind": "field",
"name": "sts_endpoint",
"required": false,
"desc": "Accessing S3 resources using temporary, secure credentials provided by AWS Security Token Service.",
"fieldValue": null,
"fieldDefaultValue": "",
"fieldFlag": "blocks-storage.s3.sts-endpoint",
"fieldType": "string"
},
{
"kind": "block",
"name": "sse",
Expand Down Expand Up @@ -11688,6 +11698,16 @@
"fieldType": "boolean",
"fieldCategory": "experimental"
},
{
"kind": "field",
"name": "sts_endpoint",
"required": false,
"desc": "Accessing S3 resources using temporary, secure credentials provided by AWS Security Token Service.",
"fieldValue": null,
"fieldDefaultValue": "",
"fieldFlag": "ruler-storage.s3.sts-endpoint",
"fieldType": "string"
},
{
"kind": "block",
"name": "sse",
Expand Down Expand Up @@ -13730,6 +13750,16 @@
"fieldType": "boolean",
"fieldCategory": "experimental"
},
{
"kind": "field",
"name": "sts_endpoint",
"required": false,
"desc": "Accessing S3 resources using temporary, secure credentials provided by AWS Security Token Service.",
"fieldValue": null,
"fieldDefaultValue": "",
"fieldFlag": "alertmanager-storage.s3.sts-endpoint",
"fieldType": "string"
},
{
"kind": "block",
"name": "sse",
Expand Down Expand Up @@ -14296,7 +14326,7 @@
"required": false,
"desc": "The timeout for establishing a connection with a remote node, and for read/write operations.",
"fieldValue": null,
"fieldDefaultValue": 10000000000,
"fieldDefaultValue": 2000000000,
"fieldFlag": "memberlist.stream-timeout",
"fieldType": "duration",
"fieldCategory": "advanced"
Expand Down Expand Up @@ -16027,6 +16057,16 @@
"fieldType": "boolean",
"fieldCategory": "experimental"
},
{
"kind": "field",
"name": "sts_endpoint",
"required": false,
"desc": "Accessing S3 resources using temporary, secure credentials provided by AWS Security Token Service.",
"fieldValue": null,
"fieldDefaultValue": "",
"fieldFlag": "common.storage.s3.sts-endpoint",
"fieldType": "string"
},
{
"kind": "block",
"name": "sse",
Expand Down
10 changes: 9 additions & 1 deletion cmd/mimir/help-all.txt.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ Usage of ./cmd/mimir/mimir:
Enable AWS Server Side Encryption. Supported values: SSE-KMS, SSE-S3.
-alertmanager-storage.s3.storage-class string
[experimental] The S3 storage class to use, not set by default. Details can be found at https://aws.amazon.com/s3/storage-classes/. Supported values are: STANDARD, REDUCED_REDUNDANCY, GLACIER, STANDARD_IA, ONEZONE_IA, INTELLIGENT_TIERING, DEEP_ARCHIVE, OUTPOSTS, GLACIER_IR, SNOW, EXPRESS_ONEZONE
-alertmanager-storage.s3.sts-endpoint string
Accessing S3 resources using temporary, secure credentials provided by AWS Security Token Service.
-alertmanager-storage.s3.tls-handshake-timeout duration
Maximum time to wait for a TLS handshake. 0 means no limit. (default 10s)
-alertmanager-storage.storage-prefix string
Expand Down Expand Up @@ -709,6 +711,8 @@ Usage of ./cmd/mimir/mimir:
Enable AWS Server Side Encryption. Supported values: SSE-KMS, SSE-S3.
-blocks-storage.s3.storage-class string
[experimental] The S3 storage class to use, not set by default. Details can be found at https://aws.amazon.com/s3/storage-classes/. Supported values are: STANDARD, REDUCED_REDUNDANCY, GLACIER, STANDARD_IA, ONEZONE_IA, INTELLIGENT_TIERING, DEEP_ARCHIVE, OUTPOSTS, GLACIER_IR, SNOW, EXPRESS_ONEZONE
-blocks-storage.s3.sts-endpoint string
Accessing S3 resources using temporary, secure credentials provided by AWS Security Token Service.
-blocks-storage.s3.tls-handshake-timeout duration
Maximum time to wait for a TLS handshake. 0 means no limit. (default 10s)
-blocks-storage.storage-prefix string
Expand Down Expand Up @@ -873,6 +877,8 @@ Usage of ./cmd/mimir/mimir:
Enable AWS Server Side Encryption. Supported values: SSE-KMS, SSE-S3.
-common.storage.s3.storage-class string
[experimental] The S3 storage class to use, not set by default. Details can be found at https://aws.amazon.com/s3/storage-classes/. Supported values are: STANDARD, REDUCED_REDUNDANCY, GLACIER, STANDARD_IA, ONEZONE_IA, INTELLIGENT_TIERING, DEEP_ARCHIVE, OUTPOSTS, GLACIER_IR, SNOW, EXPRESS_ONEZONE
-common.storage.s3.sts-endpoint string
Accessing S3 resources using temporary, secure credentials provided by AWS Security Token Service.
-common.storage.s3.tls-handshake-timeout duration
Maximum time to wait for a TLS handshake. 0 means no limit. (default 10s)
-common.storage.swift.auth-url string
Expand Down Expand Up @@ -1526,7 +1532,7 @@ Usage of ./cmd/mimir/mimir:
-memberlist.retransmit-factor int
Multiplication factor used when sending out messages (factor * log(N+1)). (default 4)
-memberlist.stream-timeout duration
The timeout for establishing a connection with a remote node, and for read/write operations. (default 10s)
The timeout for establishing a connection with a remote node, and for read/write operations. (default 2s)
-memberlist.tls-ca-path string
Path to the CA certificates to validate server certificate against. If not set, the host's root CA certificates are used.
-memberlist.tls-cert-path string
Expand Down Expand Up @@ -2283,6 +2289,8 @@ Usage of ./cmd/mimir/mimir:
Enable AWS Server Side Encryption. Supported values: SSE-KMS, SSE-S3.
-ruler-storage.s3.storage-class string
[experimental] The S3 storage class to use, not set by default. Details can be found at https://aws.amazon.com/s3/storage-classes/. Supported values are: STANDARD, REDUCED_REDUNDANCY, GLACIER, STANDARD_IA, ONEZONE_IA, INTELLIGENT_TIERING, DEEP_ARCHIVE, OUTPOSTS, GLACIER_IR, SNOW, EXPRESS_ONEZONE
-ruler-storage.s3.sts-endpoint string
Accessing S3 resources using temporary, secure credentials provided by AWS Security Token Service.
-ruler-storage.s3.tls-handshake-timeout duration
Maximum time to wait for a TLS handshake. 0 means no limit. (default 10s)
-ruler-storage.storage-prefix string
Expand Down
8 changes: 8 additions & 0 deletions cmd/mimir/help.txt.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ Usage of ./cmd/mimir/mimir:
KMS Key ID used to encrypt objects in S3
-alertmanager-storage.s3.sse.type string
Enable AWS Server Side Encryption. Supported values: SSE-KMS, SSE-S3.
-alertmanager-storage.s3.sts-endpoint string
Accessing S3 resources using temporary, secure credentials provided by AWS Security Token Service.
-alertmanager-storage.storage-prefix string
Prefix for all objects stored in the backend storage. For simplicity, it may only contain digits and English alphabet letters.
-alertmanager-storage.swift.auth-url string
Expand Down Expand Up @@ -195,6 +197,8 @@ Usage of ./cmd/mimir/mimir:
KMS Key ID used to encrypt objects in S3
-blocks-storage.s3.sse.type string
Enable AWS Server Side Encryption. Supported values: SSE-KMS, SSE-S3.
-blocks-storage.s3.sts-endpoint string
Accessing S3 resources using temporary, secure credentials provided by AWS Security Token Service.
-blocks-storage.storage-prefix string
Prefix for all objects stored in the backend storage. For simplicity, it may only contain digits and English alphabet letters.
-blocks-storage.swift.auth-url string
Expand Down Expand Up @@ -265,6 +269,8 @@ Usage of ./cmd/mimir/mimir:
KMS Key ID used to encrypt objects in S3
-common.storage.s3.sse.type string
Enable AWS Server Side Encryption. Supported values: SSE-KMS, SSE-S3.
-common.storage.s3.sts-endpoint string
Accessing S3 resources using temporary, secure credentials provided by AWS Security Token Service.
-common.storage.swift.auth-url string
OpenStack Swift authentication URL
-common.storage.swift.auth-version int
Expand Down Expand Up @@ -585,6 +591,8 @@ Usage of ./cmd/mimir/mimir:
KMS Key ID used to encrypt objects in S3
-ruler-storage.s3.sse.type string
Enable AWS Server Side Encryption. Supported values: SSE-KMS, SSE-S3.
-ruler-storage.s3.sts-endpoint string
Accessing S3 resources using temporary, secure credentials provided by AWS Security Token Service.
-ruler-storage.storage-prefix string
Prefix for all objects stored in the backend storage. For simplicity, it may only contain digits and English alphabet letters.
-ruler-storage.swift.auth-url string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2700,7 +2700,7 @@ The `memberlist` block configures the Gossip memberlist.
# (advanced) The timeout for establishing a connection with a remote node, and
# for read/write operations.
# CLI flag: -memberlist.stream-timeout
[stream_timeout: <duration> | default = 10s]
[stream_timeout: <duration> | default = 2s]
# (advanced) Multiplication factor used when sending out messages (factor *
# log(N+1)).
Expand Down Expand Up @@ -4610,6 +4610,11 @@ The s3_backend block configures the connection to Amazon S3 object storage backe
# CLI flag: -<prefix>.s3.send-content-md5
[send_content_md5: <boolean> | default = false]
# Accessing S3 resources using temporary, secure credentials provided by AWS
# Security Token Service.
# CLI flag: -<prefix>.s3.sts-endpoint
[sts_endpoint: <string> | default = ""]
sse:
# Enable AWS Server Side Encryption. Supported values: SSE-KMS, SSE-S3.
# CLI flag: -<prefix>.s3.sse.type
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ require (
github.com/golang/snappy v0.0.4
github.com/google/gopacket v1.1.19
github.com/gorilla/mux v1.8.1
github.com/grafana/dskit v0.0.0-20240104111617-ea101a3b86eb
github.com/grafana/dskit v0.0.0-20240108174153-a150e79e4581
github.com/grafana/e2e v0.1.1
github.com/hashicorp/golang-lru v1.0.2 // indirect
github.com/json-iterator/go v1.1.12
Expand All @@ -47,7 +47,7 @@ require (
golang.org/x/net v0.19.0
golang.org/x/sync v0.6.0
golang.org/x/time v0.5.0
google.golang.org/grpc v1.59.0
google.golang.org/grpc v1.60.1
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1
)
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -541,8 +541,8 @@ github.com/gosimple/slug v1.1.1 h1:fRu/digW+NMwBIP+RmviTK97Ho/bEj/C9swrCspN3D4=
github.com/gosimple/slug v1.1.1/go.mod h1:ER78kgg1Mv0NQGlXiDe57DpCyfbNywXXZ9mIorhxAf0=
github.com/grafana-tools/sdk v0.0.0-20220919052116-6562121319fc h1:PXZQA2WCxe85Tnn+WEvr8fDpfwibmEPgfgFEaC87G24=
github.com/grafana-tools/sdk v0.0.0-20220919052116-6562121319fc/go.mod h1:AHHlOEv1+GGQ3ktHMlhuTUwo3zljV3QJbC0+8o2kn+4=
github.com/grafana/dskit v0.0.0-20240104111617-ea101a3b86eb h1:AWE6+kvtE18HP+lRWNUCyvymyrFSXs6TcS2vXIXGIuw=
github.com/grafana/dskit v0.0.0-20240104111617-ea101a3b86eb/go.mod h1:kkWM4WUV230bNG3urVRWPBnSJHs64y/0RmWjftnnn0c=
github.com/grafana/dskit v0.0.0-20240108174153-a150e79e4581 h1:m8XnBonHhjenIYLI4cJmTxTAuVlmt0xeiKz6p39sl6Q=
github.com/grafana/dskit v0.0.0-20240108174153-a150e79e4581/go.mod h1:kkWM4WUV230bNG3urVRWPBnSJHs64y/0RmWjftnnn0c=
github.com/grafana/e2e v0.1.1 h1:/b6xcv5BtoBnx8cZnCiey9DbjEc8z7gXHO5edoeRYxc=
github.com/grafana/e2e v0.1.1/go.mod h1:RpNLgae5VT+BUHvPE+/zSypmOXKwEu4t+tnEMS1ATaE=
github.com/grafana/goautoneg v0.0.0-20231010094147-47ce5e72a9ae h1:Yxbw9jKGJVC6qAK5Ubzzb/qZwM6rRMMqaDc/d4Vp3pM=
Expand Down Expand Up @@ -1550,8 +1550,8 @@ google.golang.org/grpc v1.44.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ5
google.golang.org/grpc v1.45.0/go.mod h1:lN7owxKUQEqMfSyQikvvk5tf/6zMPsrK+ONuO11+0rQ=
google.golang.org/grpc v1.46.0/go.mod h1:vN9eftEi1UMyUsIF80+uQXhHjbXYbm0uXoFCACuMGWk=
google.golang.org/grpc v1.46.2/go.mod h1:vN9eftEi1UMyUsIF80+uQXhHjbXYbm0uXoFCACuMGWk=
google.golang.org/grpc v1.59.0 h1:Z5Iec2pjwb+LEOqzpB2MR12/eKFhDPhuqW91O+4bwUk=
google.golang.org/grpc v1.59.0/go.mod h1:aUPDwccQo6OTjy7Hct4AfBPD1GptF4fyUjIkQ9YtF98=
google.golang.org/grpc v1.60.1 h1:26+wFr+cNqSGFcOXcabYC0lUVJVRa2Sb2ortSK7VrEU=
google.golang.org/grpc v1.60.1/go.mod h1:OlCHIeLYqSSsLi6i49B5QGdzaMZK9+M7LXN2FKz4eGM=
google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0/go.mod h1:6Kw0yEErY5E/yWrBtf03jp27GLLJujG4z/JK95pnjjw=
google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8=
google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0=
Expand Down
8 changes: 4 additions & 4 deletions integration/querier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ func testQuerierWithBlocksStorageRunningInMicroservicesMode(t *testing.T, stream
require.NoError(t, storeGateways.WaitSumMetrics(e2e.Equals(2*2+2+3), "thanos_store_index_cache_items")) // 2 series both for postings and series cache, 2 expanded postings on one block, 3 on another one
require.NoError(t, storeGateways.WaitSumMetrics(e2e.Equals(2*2+2+3), "thanos_store_index_cache_items_added_total")) // 2 series both for postings and series cache, 2 expanded postings on one block, 3 on another one
} else if testCfg.indexCacheBackend == tsdb.IndexCacheBackendMemcached {
require.NoError(t, storeGateways.WaitSumMetrics(comparingFunction(9*2), "thanos_memcached_operations_total")) // one set for each get
require.NoError(t, storeGateways.WaitSumMetrics(comparingFunction(9*2), "thanos_cache_operations_total")) // one set for each get
}

// Query back again the 1st series from storage. This time it should use the index cache.
Expand All @@ -265,7 +265,7 @@ func testQuerierWithBlocksStorageRunningInMicroservicesMode(t *testing.T, stream
require.NoError(t, storeGateways.WaitSumMetrics(e2e.Equals(2*2+2+3), "thanos_store_index_cache_items")) // as before
require.NoError(t, storeGateways.WaitSumMetrics(e2e.Equals(2*2+2+3), "thanos_store_index_cache_items_added_total")) // as before
} else if testCfg.indexCacheBackend == tsdb.IndexCacheBackendMemcached {
require.NoError(t, storeGateways.WaitSumMetrics(comparingFunction(9*2+2), "thanos_memcached_operations_total")) // as before + 2 gets (expanded postings and series)
require.NoError(t, storeGateways.WaitSumMetrics(comparingFunction(9*2+2), "thanos_cache_operations_total")) // as before + 2 gets (expanded postings and series)
}

// Query range. We expect 1 data point with a value of 3 (number of series).
Expand Down Expand Up @@ -484,7 +484,7 @@ func TestQuerierWithBlocksStorageRunningInSingleBinaryMode(t *testing.T) {
require.NoError(t, cluster.WaitSumMetrics(e2e.Equals(float64((2*2+2+3)*seriesReplicationFactor)), "thanos_store_index_cache_items")) // 2 series both for postings and series cache, 2 expanded postings on one block, 3 on another one
require.NoError(t, cluster.WaitSumMetrics(e2e.Equals(float64((2*2+2+3)*seriesReplicationFactor)), "thanos_store_index_cache_items_added_total")) // 2 series both for postings and series cache, 2 expanded postings on one block, 3 on another one
} else if testCfg.indexCacheBackend == tsdb.IndexCacheBackendMemcached {
require.NoError(t, cluster.WaitSumMetrics(e2e.Equals(float64(expectedMemcachedOps)), "thanos_memcached_operations_total"))
require.NoError(t, cluster.WaitSumMetrics(e2e.Equals(float64(expectedMemcachedOps)), "thanos_cache_operations_total"))
}

// Query back again the 1st series from storage. This time it should use the index cache.
Expand All @@ -505,7 +505,7 @@ func TestQuerierWithBlocksStorageRunningInSingleBinaryMode(t *testing.T) {
require.NoError(t, cluster.WaitSumMetrics(e2e.Equals(float64((2*2+2+3)*seriesReplicationFactor)), "thanos_store_index_cache_items")) // as before
require.NoError(t, cluster.WaitSumMetrics(e2e.Equals(float64((2*2+2+3)*seriesReplicationFactor)), "thanos_store_index_cache_items_added_total")) // as before
} else if testCfg.indexCacheBackend == tsdb.IndexCacheBackendMemcached {
require.NoError(t, cluster.WaitSumMetrics(e2e.Equals(float64(expectedMemcachedOps)), "thanos_memcached_operations_total"))
require.NoError(t, cluster.WaitSumMetrics(e2e.Equals(float64(expectedMemcachedOps)), "thanos_cache_operations_total"))
}

// Query metadata.
Expand Down
8 changes: 4 additions & 4 deletions integration/query_frontend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,8 @@ func runQueryFrontendTest(t *testing.T, cfg queryFrontendTestConfig) {
require.NoError(t, s.WaitReady(queryFrontend))

// Check if we're discovering memcache or not.
require.NoError(t, queryFrontend.WaitSumMetrics(e2e.Equals(1), "thanos_memcached_dns_provider_results"))
require.NoError(t, queryFrontend.WaitSumMetrics(e2e.Greater(0), "thanos_memcached_dns_lookups_total"))
require.NoError(t, queryFrontend.WaitSumMetrics(e2e.Equals(1), "thanos_cache_dns_provider_results"))
require.NoError(t, queryFrontend.WaitSumMetrics(e2e.Greater(0), "thanos_cache_dns_lookups_total"))

// Wait until distributor and querier have updated the ingesters ring.
require.NoError(t, distributor.WaitSumMetricsWithOptions(e2e.Equals(1), []string{"cortex_ring_members"}, e2e.WithLabelMatchers(
Expand Down Expand Up @@ -759,8 +759,8 @@ func runQueryFrontendWithQueryShardingHTTPTest(t *testing.T, cfg queryFrontendTe
require.NoError(t, s.WaitReady(queryFrontend))

// Check if we're discovering memcache or not.
require.NoError(t, queryFrontend.WaitSumMetrics(e2e.Equals(1), "thanos_memcached_dns_provider_results"))
require.NoError(t, queryFrontend.WaitSumMetrics(e2e.Greater(0), "thanos_memcached_dns_lookups_total"))
require.NoError(t, queryFrontend.WaitSumMetrics(e2e.Equals(1), "thanos_cache_dns_provider_results"))
require.NoError(t, queryFrontend.WaitSumMetrics(e2e.Greater(0), "thanos_cache_dns_lookups_total"))

// Wait until distributor and querier have updated the ingesters ring.
require.NoError(t, distributor.WaitSumMetricsWithOptions(e2e.Equals(1), []string{"cortex_ring_members"}, e2e.WithLabelMatchers(
Expand Down
Loading

0 comments on commit 67fae19

Please sign in to comment.