Skip to content

Introduce query-tee tool #2203

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

Merged
merged 17 commits into from
Mar 4, 2020
Merged

Introduce query-tee tool #2203

merged 17 commits into from
Mar 4, 2020

Conversation

pracucci
Copy link
Contributor

@pracucci pracucci commented Mar 3, 2020

What this PR does:
While working on the experimental blocks storage, we started having the need to compare chunks vs blocks storage in the real world. To do it, we setup a test cluster (with blocks storage) ingesting the same exact series of another cluster with chunks storage. We now have two parallel clusters with the same series and we have the need to query both clusters and compare performances and results.

Given this need, the idea we got is to add a new tool called query-tee that exposes the Prometheus API read endpoints and, for each request, send the same request to 2+ backends and make them compete.

In this PR:

  • Added a new tool query-tee
  • Track a metric with response time for each backend

What's out of the scope of this PR:

  • Compare results from backends

Which issue(s) this PR fixes:
N/A

Checklist

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

@pracucci pracucci requested a review from pstibrany March 3, 2020 17:30
Copy link
Contributor

@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.

I think it's a good first version. I have some comments about how to make it little bit more generic (by honoring backend paths). Also HTTP client configuration seems strange -- I think there are some conflicting options set. It would be good to clarify that.

Copy link
Contributor

@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.

Nice work and thanks for addressing my feedback.

@pull-request-size pull-request-size bot added size/XL and removed size/L labels Mar 4, 2020
@pracucci pracucci marked this pull request as ready for review March 4, 2020 13:52
@pracucci pracucci requested a review from pstibrany March 4, 2020 13:53
Copy link
Contributor

@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! I haven't noticed any test for different backend path prefixes (say one backend using /prefix1/api/v1/query and other one different prefix), would it be worth having one?

pracucci added 16 commits March 4, 2020 15:37
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
…custom path prefixes in the backend

Signed-off-by: Marco Pracucci <[email protected]>
…to check whether the response should be considered successful from the proxy perspective

Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
@pracucci
Copy link
Contributor Author

pracucci commented Mar 4, 2020

Thanks for your review @pstibrany!

I haven't noticed any test for different backend path prefixes (say one backend using /prefix1/api/v1/query and other one different prefix), would it be worth having one?

Makes sense. Done!

Signed-off-by: Marco Pracucci <[email protected]>
@pracucci pracucci merged commit 5d7b05c into cortexproject:master Mar 4, 2020
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.

2 participants