-
Notifications
You must be signed in to change notification settings - Fork 453
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.
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
Remove unused ingester metrics #760
Conversation
@@ -272,7 +272,7 @@ local utils = import 'mixin-utils/utils.libsonnet'; | |||
.addPanel( | |||
$.panel('Corruptions / sec') + | |||
$.queryPanel([ | |||
'sum(rate(cortex_ingester_wal_corruptions_total{%s}[$__rate_interval]))' % $.jobMatcher($._config.job_names.ingester), | |||
'sum(rate(cortex_ingester_tsdb_wal_corruptions_total{%s}[$__rate_interval]))' % $.jobMatcher($._config.job_names.ingester), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a bug. It was querying the chunks storage WAL corruptions not the blocks storage one.
// A small number of chunks per series - 10*(8^(7-1)) = 2.6m. | ||
Buckets: prometheus.ExponentialBuckets(10, 8, 7), | ||
}), | ||
memSeries: promauto.With(r).NewGauge(prometheus.GaugeOpts{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not tracked in the CHANGELOG because this metric is still exported by the metric override we define in ingester.New()
.
31ef7a9
to
03b8f99
Compare
@@ -188,7 +188,9 @@ func TestRuler_alerts(t *testing.T) { | |||
cfg := defaultRulerConfig(t) | |||
|
|||
r := newTestRuler(t, cfg, newMockRuleStore(mockRules)) | |||
defer r.StopAsync() | |||
t.Cleanup(func() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got this test failing in CI (flaky). It failed to delete the temp dir created by defaultRulerConfig()
because not empty. I suspect the reason is that we don't wait for the ruler being stopped before we remove the dir. This change should fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
ada1537
to
0a8b2fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
What this PR does:
When we removed chunks storage support from ingester we forgot about removing metrics too. This PR does it.
Which issue(s) this PR fixes:
N/A
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]