-
Notifications
You must be signed in to change notification settings - Fork 820
TSDB: fix user metrics when using blocks storage #1990
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
Conversation
… metrics. This affects cortex_ingester_memory_series_created_total and cortex_ingester_memory_series_removed_total. This change also renames previous shipperMetrics to tsdbMetrics, and registers the same registry to TSDB and TSDB shipper component. Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
This fixes problem with collection occuring when TSDB already registered metrics, but wasn't otherwise fully initialized yet, and it crashed. Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
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.
Excellent job @pstibrany! I left few minor comments.
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Thanks for review @pracucci, good points. I've addressed all your feedback. |
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.
Thanks @pstibrany! LGTM
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!
When using TSDB, we get per-user created and removed series from TSDB metrics.
This affects
cortex_ingester_memory_series_created_total
andcortex_ingester_memory_series_removed_total
.This PR also renames
shipperMetrics
totsdbMetrics
, and registers the same registry to TSDB and TSDB shipper component.Registration of prometheus registry now happens only after TSDB initialization is finished, in order to fix a panic where metrics were collected while TSDB was still initialized.
This PR builds on PR #1983, so "Files changed" also shows changes from there. I'll rebase once (if) #1983 is merged. Update: now rebased.