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 rule information to query paramters sent to the QFE #6539

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SungJin1212
Copy link
Contributor

When the -ruler.frontend-address is configured, the Ruler evaluates rules via QFE. But, all of the values of the query stats logs and metrics in the Ruler are 0 because the QFE doesn't send query stat.

In Ruler, for example:
the query stats logs are:

cortex_ruler_query_seconds_total=0.003627083 query_wall_time_seconds=0s query_storage_wall_time_seconds=0s fetched_series_count=0 fetched_chunks_count=0 fetched_samples_count=0 fetched_chunks_bytes=0 fetched_data_bytes=0

Also, the values of these metrics are all 0

cortex_ruler_fetched_series_total, cortex_ruler_samples_total, cortex_ruler_fetched_chunks_bytes_total ...

To address it, this PR adds rule information to query parameters sent to the QFE to leave rule information logs on query stats.
For the metric, #6470 add the source label to categorize query stat metric.

Which issue(s) this PR fixes:
Fixes #

Checklist

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

@dosubot dosubot bot added the component/rules Bits & bobs todo with rules and alerts: the ruler, config service etc. label Jan 23, 2025
@SungJin1212 SungJin1212 requested a review from yeya24 January 23, 2025 06:51
@alanprot
Copy link
Member

Maybe we can return the stats at least on the proto response?

@SungJin1212
Copy link
Contributor Author

@alanprot
Yeah, we could return query stats in the protobuf from QFE to Ruler. But Idk if it is worth it more. We can leave query stats logs by adding query parameters at the Ruler. (https://github.com/cortexproject/cortex/blob/master/pkg/frontend/transport/handler.go#L514)

@SungJin1212 SungJin1212 force-pushed the Add-rule-info-to-ruler-request branch from 8ea9f15 to cdda1c8 Compare January 29, 2025 13:13
@alanprot
Copy link
Member

But the rulers logs would still be "empty" right? so should we at least stop logging it?

@SungJin1212
Copy link
Contributor Author

SungJin1212 commented Jan 31, 2025

But the rulers logs would still be "empty" right? so should we at least stop logging it?

Yeah, the evaluation logs are still empty.
Stopping the logging at the Ruler is fine for me.
@yeya24
WDYT?

Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

This looks good.

Stopping the logging at the Ruler is fine for me.

Do we want to do this? Or it is already doable by just turning off query stats log in Ruler

Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
@SungJin1212 SungJin1212 force-pushed the Add-rule-info-to-ruler-request branch from cdda1c8 to dee8817 Compare February 19, 2025 05:47
@SungJin1212
Copy link
Contributor Author

@yeya24
I changed the behavior, the Ruler tracks metrics and logs only if -ruler.frontend-address enabled is not configured. When -ruler.frontend-address enabled flag is set, the QFE tracks logs and metrics.

@SungJin1212 SungJin1212 requested a review from yeya24 February 19, 2025 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/rules Bits & bobs todo with rules and alerts: the ruler, config service etc. size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants