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

querytee: Add support to log comparatively slow queries #7346

Merged
merged 13 commits into from
Mar 17, 2024

Conversation

jhesketh
Copy link
Contributor

@jhesketh jhesketh commented Feb 9, 2024

This adds proxy.log-slow-query-response-threshold which when enabled will log any queries that take longer than the configured threshold on one backend compared to the fastest backend.

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

This adds `proxy.log-slow-query-response-threshold` which when enabled
will log any queries that take longer than the configured threshold on
one backend compared to the fastest backend.
@jhesketh jhesketh marked this pull request as ready for review February 9, 2024 11:01
@jhesketh jhesketh requested review from a team as code owners February 9, 2024 11:02
tools/querytee/proxy_endpoint.go Show resolved Hide resolved
tools/querytee/proxy_metrics.go Outdated Show resolved Hide resolved
tools/querytee/proxy.go Outdated Show resolved Hide resolved
jhesketh and others added 2 commits February 19, 2024 22:39
Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com>
Copy link
Member

@jdbaldry jdbaldry left a comment

Choose a reason for hiding this comment

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

Docs LGTM.

Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

LGTM, but please see comments for a typo etc :)

docs/sources/mimir/manage/tools/query-tee.md 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
Copy link
Contributor

@charleskorn charleskorn left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

One other thing that would be handy to add (but could be a separate PR) is two histogram metrics of the relative and absolute latency difference between the two backends.

tools/querytee/proxy_endpoint.go Outdated Show resolved Hide resolved
tools/querytee/proxy_endpoint_test.go Outdated Show resolved Hide resolved
tools/querytee/proxy_endpoint_test.go Outdated Show resolved Hide resolved
jhesketh and others added 3 commits March 8, 2024 13:27
Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
Note: this changes the pre-existing `route-name` to `route_name`
Copy link
Contributor

@charleskorn charleskorn left a comment

Choose a reason for hiding this comment

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

LGTM modulo one suggestion for the tests

tools/querytee/proxy_endpoint_test.go Outdated Show resolved Hide resolved
@duricanikolic
Copy link
Contributor

The CHANGELOG has just been cut to prepare for the next Mimir release. Please rebase main and eventually move the CHANGELOG entry added / updated in this PR to the top of the CHANGELOG document. Thanks!

@jhesketh jhesketh merged commit 14d947d into grafana:main Mar 17, 2024
29 checks passed
@jhesketh jhesketh deleted the jhesketh/querytee branch March 17, 2024 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants