Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: follow-up to #11151 and #11025 #11225

Merged
merged 4 commits into from
Nov 14, 2023
Merged

chore: follow-up to #11151 and #11025 #11225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
030db9a
chore: follow-up to #11151
ashwanthgoli Nov 14, 2023
68200b7
fixup! chore: follow-up to #11151
ashwanthgoli Nov 14, 2023
f8e4c5d
make format
ashwanthgoli Nov 14, 2023
63ba95a
make documentation-helm-reference-check
ashwanthgoli Nov 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion docs/sources/setup/install/helm/reference.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2027,7 +2027,6 @@ null
<td>Limits config</td>
<td><pre lang="json">
{
"enforce_metric_name": false,
"max_cache_freshness_per_query": "10m",
"reject_old_samples": true,
"reject_old_samples_max_age": "168h",
Expand Down
4 changes: 0 additions & 4 deletions docs/sources/setup/upgrade/_index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,10 +241,6 @@ Some Loki metrics started with the prefix `cortex_`. In this release they will b
- `cortex_query_scheduler_queue_duration_seconds_sum`
- `cortex_query_scheduler_queue_length`
- `cortex_query_scheduler_running`
- `cortex_quota_cgroup_cpu_max`
- `cortex_quota_cgroup_cpu_period`
- `cortex_quota_cpu_count`
- `cortex_quota_gomaxprocs`
- `cortex_ring_member_heartbeats_total`
- `cortex_ring_member_tokens_owned`
- `cortex_ring_member_tokens_to_own`
Expand Down
20 changes: 11 additions & 9 deletions pkg/ruler/base/client_pool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,27 +48,29 @@ func newRulerClientPool(clientCfg grpcclient.Config, logger log.Logger, reg prom
})

return &rulerClientsPool{
client.NewPool("ruler", poolCfg, nil, newRulerClientFactory(clientCfg, reg), clientsCount, logger),
client.NewPool("ruler", poolCfg, nil, newRulerClientFactory(clientCfg, reg, metricsNamespace), clientsCount, logger),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels weird to me that we're passing in the metrics namespace all over the codebase. Feels like we should be able to solve this with prometheus.WrapRegistererWithPrefix("loki_", reg) or defining the namespace in one central location as a singleton.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember @MichelHollands tried to make WrapRegistererWithPrefix work. I'll check with him and do this change in a follow-up

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, works for me

}
}

func newRulerClientFactory(clientCfg grpcclient.Config, reg prometheus.Registerer) client.PoolFactory {
func newRulerClientFactory(clientCfg grpcclient.Config, reg prometheus.Registerer, metricsNamespace string) client.PoolFactory {
requestDuration := promauto.With(reg).NewHistogramVec(prometheus.HistogramOpts{
Name: "cortex_ruler_client_request_duration_seconds",
Help: "Time spent executing requests to the ruler.",
Buckets: prometheus.ExponentialBuckets(0.008, 4, 7),
Namespace: metricsNamespace,
Name: "ruler_client_request_duration_seconds",
Help: "Time spent executing requests to the ruler.",
Buckets: prometheus.ExponentialBuckets(0.008, 4, 7),
}, []string{"operation", "status_code"})

return client.PoolAddrFunc(func(addr string) (client.PoolClient, error) {
return dialRulerClient(clientCfg, addr, requestDuration)
})
}

func newRulerPoolClient(clientCfg grpcclient.Config, reg prometheus.Registerer) func(addr string) (client.PoolClient, error) {
func newRulerPoolClient(clientCfg grpcclient.Config, reg prometheus.Registerer, metricsNamespace string) func(addr string) (client.PoolClient, error) {
requestDuration := promauto.With(reg).NewHistogramVec(prometheus.HistogramOpts{
Name: "cortex_ruler_client_request_duration_seconds",
Help: "Time spent executing requests to the ruler.",
Buckets: prometheus.ExponentialBuckets(0.008, 4, 7),
Namespace: metricsNamespace,
Name: "ruler_client_request_duration_seconds",
Help: "Time spent executing requests to the ruler.",
Buckets: prometheus.ExponentialBuckets(0.008, 4, 7),
}, []string{"operation", "status_code"})

return func(addr string) (client.PoolClient, error) {
Expand Down
6 changes: 4 additions & 2 deletions pkg/ruler/base/client_pool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/grpc"

"github.com/grafana/loki/pkg/util/constants"
)

func Test_newRulerClientFactory(t *testing.T) {
Expand All @@ -36,7 +38,7 @@ func Test_newRulerClientFactory(t *testing.T) {
flagext.DefaultValues(&cfg)

reg := prometheus.NewPedanticRegistry()
factory := newRulerPoolClient(cfg, reg)
factory := newRulerPoolClient(cfg, reg, constants.Loki)

for i := 0; i < 2; i++ {
client, err := factory(listener.Addr().String())
Expand All @@ -54,7 +56,7 @@ func Test_newRulerClientFactory(t *testing.T) {
require.NoError(t, err)

assert.Len(t, metrics, 1)
assert.Equal(t, "cortex_ruler_client_request_duration_seconds", metrics[0].GetName())
assert.Equal(t, "loki_ruler_client_request_duration_seconds", metrics[0].GetName())
assert.Equal(t, dto.MetricType_HISTOGRAM, metrics[0].GetType())
assert.Len(t, metrics[0].GetMetric(), 1)
assert.Equal(t, uint64(2), metrics[0].GetMetric()[0].GetHistogram().GetSampleCount())
Expand Down
1 change: 0 additions & 1 deletion production/docker/config/loki.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ schema_config:

limits_config:
max_cache_freshness_per_query: '10m'
enforce_metric_name: false
reject_old_samples: true
reject_old_samples_max_age: 30m
ingestion_rate_mb: 10
Expand Down
1 change: 0 additions & 1 deletion production/helm/loki/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ loki:
grpc_listen_port: 9095
# -- Limits config
limits_config:
enforce_metric_name: false
reject_old_samples: true
reject_old_samples_max_age: 168h
max_cache_freshness_per_query: 10m
Expand Down
1 change: 0 additions & 1 deletion production/ksonnet/loki-simple-scalable/example/main.jsonnet
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ loki {
},
},
limits_config: {
enforce_metric_name: false,
reject_old_samples_max_age: '168h', //1 week
max_global_streams_per_user: 60000,
ingestion_rate_mb: 75,
Expand Down
1 change: 0 additions & 1 deletion production/ksonnet/loki/config.libsonnet
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@
query_ingesters_within: '2h', // twice the max-chunk age (1h default) for safety buffer
},
limits_config: {
enforce_metric_name: false,
// align middleware parallelism with shard factor to optimize one-legged sharded queries.
max_query_parallelism: if $._config.queryFrontend.sharded_queries_enabled then
// For a sharding factor of 16 (default), this is 256, or enough for 16 sharded queries.
Expand Down
1 change: 0 additions & 1 deletion production/nomad/loki-distributed/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,5 @@ ruler:
dir: {{ env "NOMAD_ALLOC_DIR" }}/data/ruler

limits_config:
enforce_metric_name: false
reject_old_samples: true
reject_old_samples_max_age: 168h
1 change: 0 additions & 1 deletion production/nomad/loki-simple/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ storage_config:
s3forcepathstyle: true

limits_config:
enforce_metric_name: false
reject_old_samples: true
reject_old_samples_max_age: 168h

Expand Down
1 change: 0 additions & 1 deletion production/nomad/loki/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ storage_config:
s3forcepathstyle: true

limits_config:
enforce_metric_name: false
reject_old_samples: true
reject_old_samples_max_age: 168h

Expand Down
Loading