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.

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

mixin dashbaords: add support for ingest storage replication #8175

Conversation

dimitarvdimitrov
Copy link
Contributor

What this PR does

Some panels in the dashboards rely on cortex_distributor_replication_factor. This metric is not exported if running in ingest storage (#7809).

This PR updates all queries in the mixin that were relaying on cortex_distributor_replication_factor. If the replication factor metric isn't present now the queries fall back to the maximum of each partition for the metric in question (such as cortex_ingester_active_series).

Which issue(s) this PR fixes or relates to

Fixes #

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
@dimitarvdimitrov dimitarvdimitrov requested a review from a team as a code owner May 23, 2024 10:54
@dimitarvdimitrov dimitarvdimitrov changed the title mixin: add support for ingest storage replication mixin dashbaords: add support for ingest storage replication May 23, 2024
Copy link
Member

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

Very nice job!

Comment on lines +240 to +253
# Classic storage
sum by (%(groupByCluster)s, %(groupByLabels)s) (%(perIngesterQuery)s)
/ on (%(groupByCluster)s) group_left()
max by (%(groupByCluster)s) (cortex_distributor_replication_factor{%(distributor)s})
or on (%(groupByCluster)s)
# Ingest storage
sum by (%(groupByCluster)s, %(groupByLabels)s) (
max by (ingester_id, %(groupByCluster)s, %(groupByLabels)s) (
label_replace(
%(perIngesterQuery)s,
"ingester_id", "$1", "%(instance)s", ".*-([0-9]+)$"
)
)
)
Copy link
Member

Choose a reason for hiding this comment

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

🤯 Nice :)

operations/mimir-mixin/dashboards/writes.libsonnet Outdated Show resolved Hide resolved
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
@dimitarvdimitrov dimitarvdimitrov merged commit 487a5c9 into main May 24, 2024
31 checks passed
@dimitarvdimitrov dimitarvdimitrov deleted the dimitar/mixin/replace-usages-of-distributor_replication_factor branch May 24, 2024 08:41
sum by (%(groupByCluster)s, %(groupByLabels)s) (%(perIngesterQuery)s)
/ on (%(groupByCluster)s) group_left()
max by (%(groupByCluster)s) (cortex_distributor_replication_factor{%(distributor)s})
or on (%(groupByCluster)s)
Copy link
Collaborator

@pracucci pracucci May 28, 2024

Choose a reason for hiding this comment

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

I don't understand one thing: what is this on (%(groupByCluster)s) (in this line) used for?

Isn't the query we want something like:

(
  # Classic storage
) or (
  # Ingest storage
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah good point. I don't think the on clause does much. Both sides already have the same set of label names.

Opened #8211

narqo pushed a commit to narqo/grafana-mimir that referenced this pull request Jun 6, 2024
…#8175)

Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
@narqo narqo mentioned this pull request Sep 2, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants