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

query-tee: Add an option to shift the query times for comparison #9319

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

codesome
Copy link
Member

@codesome codesome commented Sep 17, 2024

What this PR does

Adds an option proxy.shift-comparison-queries-by to shift the query times for query-tee comparisons.

Sometimes proxy.compare-skip-recent-samples option may not be enough because if you want to set it to a higher duration (say few hours), it will completely skip comparing the queries for recent times, which in practice forms a big % of all queries (like recording rules, many most instant queries, etc).

With this change, the query-tee still does the query with original times for the preferred backend, but for comparison, it shifts the query times, and does another query to the preferred backend with the shifted times and only one shifted query to the secondary backend and uses those results for comparison.

Added proxy.shift-comparison-sampling-ratio to samples the queries for shifting the query times.

Draft PR because there are still some TODOs and need to write tests.

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.

@codesome codesome requested a review from narqo September 17, 2024 20:48
Signed-off-by: Ganesh Vernekar <[email protected]>
Signed-off-by: Ganesh Vernekar <[email protected]>
Signed-off-by: Ganesh Vernekar <[email protected]>
Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

The overall design LGTM. I left a couple of comments. Would be glad to see a unit test.

tools/querytee/proxy.go Outdated Show resolved Hide resolved
tools/querytee/proxy_endpoint.go Outdated Show resolved Hide resolved
tools/querytee/proxy_endpoint.go Outdated Show resolved Hide resolved
tools/querytee/proxy_endpoint.go Outdated Show resolved Hide resolved
Signed-off-by: Ganesh Vernekar <[email protected]>
Signed-off-by: Ganesh Vernekar <[email protected]>
@codesome
Copy link
Member Author

Added a unit test

@codesome codesome marked this pull request as ready for review September 18, 2024 17:47
@codesome codesome requested a review from a team as a code owner September 18, 2024 17:47
@codesome
Copy link
Member Author

codesome commented Sep 21, 2024

Tried it in a dev environment, weird that all shifted queries are being skipped for comparison because the response format is text/plain and not application/json. (I increased the shift ratio multiple times, hence the jumps)

Screenshot 2024-09-20 at 10 14 17 PM

@codesome codesome marked this pull request as draft September 24, 2024 13:18
@codesome
Copy link
Member Author

I converted this to draft since there is the above bug that I need to fix.

@codesome
Copy link
Member Author

It is working locally with the docker-compose setup, so I have no clue why it gave issues in the dev environment. I will try running it again in dev next week and try to debug it there.

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.

3 participants