-
Notifications
You must be signed in to change notification settings - Fork 2k
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 support for sampling rate in streamlog
#15919
base: main
Are you sure you want to change the base?
Add support for sampling rate in streamlog
#15919
Conversation
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15919 +/- ##
==========================================
+ Coverage 68.40% 68.45% +0.04%
==========================================
Files 1556 1559 +3
Lines 195121 196836 +1715
==========================================
+ Hits 133479 134749 +1270
- Misses 61642 62087 +445 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Hello! 👋 This Pull Request is now handled by arewefastyet. The current HEAD and future commits will be benchmarked. You can find the performance comparison on the arewefastyet website. |
Flag docs are auto-generated, so we don't need any manual changes on the website for this. @frouioui can you confirm that I'm getting this right? |
@deepthi according https://github.com/vitessio/vitess-bot/blob/main/README.md#vitess-bot it should generate a PR upon merge to |
After talking with @mattlord, I realized that actually the automation does not work. We should update the website docs manually at the same time as this PR. |
Right, I forgot this is automatic now, thanks 👍 Also, based on the fact |
@frouioui can you provide the instructions for that? Or should we simply plan to do that manually for v20 before release? I suspect there may be other PRs which are missing website documentation as well. One example: #16021 EDIT: There's a Makefile target in the website repo to generate the docs. Do the following in a website branch, and make sure you have the main vitess repo checked out and accessible from there.
It becomes easier if doing after merge into main, because we can use |
Yeah exactly running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
This PR implements the RFC #15909 by adding a flag
--querylog-sample-rate float
tovtcombo
,vtgate
andvttablet
in order for queries to be sampled randomly without the need for the client to trigger query logging via a query directive/comment ==--querylog-filter-tag string
This flag supports values between
0.0
(no logging) and1.0
(log all queries) to match other0.0-1.0
float "sample" flagsmath/rand/v2
was used because it sounds betterRelated Issue(s)
Resolves #15909
Checklist
Deployment Notes