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

Add additional querier metrics #7099

Merged
merged 6 commits into from
Jan 12, 2024
Merged

Conversation

jhalterman
Copy link
Member

@jhalterman jhalterman commented Jan 10, 2024

What this PR does

Adds metrics to:

  • count how many queries are executed in total for a particular source: ingesters vs store gateways
  • count the total chunks read from a store gateway
    • Counted in separate places for streaming and non-streaming fetches

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.

Adds metrics to:
- count how many queries are executed in total for a particular source: ingesters vs store gateways
- count the total chunks read from a store gateway
@@ -135,6 +137,11 @@ func newBlocksStoreQueryableMetrics(reg prometheus.Registerer) *blocksStoreQuery
Name: "cortex_querier_blocks_with_compactor_shard_but_incompatible_query_shard_total",
Help: "Blocks that couldn't be checked for query and compactor sharding optimization due to incompatible shard counts.",
}),
// Named to be consistent with distributor_query_ingester_chunks_total
Copy link
Member Author

Choose a reason for hiding this comment

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

This metric is very similar to the metrics in query_metrics.go, but those are mostly specific to ingesters/distributors and not as easy to share with a block store queryable, so I followed the precedent of keeping the ingester and store gateway metrics in queriers separate.

@jhalterman jhalterman marked this pull request as ready for review January 11, 2024 03:45
@jhalterman jhalterman requested a review from a team as a code owner January 11, 2024 03:45
@jhalterman jhalterman marked this pull request as draft January 11, 2024 04:23
CHANGELOG.md Outdated Show resolved Hide resolved
pkg/querier/stats/query_metrics.go Outdated Show resolved Hide resolved
jhalterman and others added 2 commits January 11, 2024 09:06
Co-authored-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
@jhalterman jhalterman marked this pull request as ready for review January 12, 2024 02:08
Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

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

LGTM, thanks. Only a nitpick on the changelog

CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
@jhalterman jhalterman merged commit 18c9df3 into grafana:main Jan 12, 2024
28 checks passed
@jhalterman jhalterman deleted the querier-metrics branch January 12, 2024 18:00
@@ -135,6 +137,11 @@ func newBlocksStoreQueryableMetrics(reg prometheus.Registerer) *blocksStoreQuery
Name: "cortex_querier_blocks_with_compactor_shard_but_incompatible_query_shard_total",
Help: "Blocks that couldn't be checked for query and compactor sharding optimization due to incompatible shard counts.",
}),
// Named to be consistent with distributor_query_ingester_chunks_total
chunksTotal: promauto.With(reg).NewCounter(prometheus.CounterOpts{
Name: "cortex_query_storegateway_chunks_total",
Copy link
Collaborator

@pracucci pracucci Jan 15, 2024

Choose a reason for hiding this comment

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

The querier metrics are prefixed by cortex_querier_. WDYT if we change the metric name to cortex_querier_query_storegateway_chunks_total?

Copy link
Member Author

Choose a reason for hiding this comment

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

That name sounds good. Pushed PR #7145

@@ -135,6 +137,11 @@ func newBlocksStoreQueryableMetrics(reg prometheus.Registerer) *blocksStoreQuery
Name: "cortex_querier_blocks_with_compactor_shard_but_incompatible_query_shard_total",
Help: "Blocks that couldn't be checked for query and compactor sharding optimization due to incompatible shard counts.",
}),
// Named to be consistent with distributor_query_ingester_chunks_total
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: the metric is named cortex_distributor_query_ingester_chunks_total (with cortex_ prefix).

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is really a nit, not worth a PR, but if you address the other comment then you could fix this too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants