-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 structure logging (Zap) to Vitess #13061
Conversation
Signed-off-by: Emad Habib <[email protected]>
Signed-off-by: Emad Habib <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
If a new flag is being introduced:
If a workflow is added or modified:
Bug fixes
Non-trivial changes
New/Existing features
Backward compatibility
|
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.
I don't see the need to prefix the new files with vts_
, they can just be logger.go and logger_test.go.
We don't do that anywhere else.
This PR is being marked as stale because it has been open for 30 days with no activity. To rectify, you may do any of the following:
If no action is taken within 7 days, this PR will be closed. |
This PR was closed because it has been stale for 7 days with no activity. |
There are files with the same name. I intend to keep the structure logging separated in a different file. |
This PR is being marked as stale because it has been open for 30 days with no activity. To rectify, you may do any of the following:
If no action is taken within 7 days, this PR will be closed. |
This PR was closed because it has been stale for 7 days with no activity. |
Signed-off-by: Tim Vaillancourt <[email protected]>
* Install zap log and noglog Signed-off-by: Emad Habib <[email protected]> * Implement the Vitess Structure Logger VTSLoger Signed-off-by: Emad Habib <[email protected]> * Move the structure logging code to logger.go file * make PR suggestions from vitessio#13061 Signed-off-by: Tim Vaillancourt <[email protected]> * Fix bad merge conflict Signed-off-by: Tim Vaillancourt <[email protected]> * Fix bad merge conflict again Signed-off-by: Tim Vaillancourt <[email protected]> * fix test Signed-off-by: Tim Vaillancourt <[email protected]> * add flags e2e test for vtgateclienttest Signed-off-by: Tim Vaillancourt <[email protected]> * update error msg Signed-off-by: Tim Vaillancourt <[email protected]> --------- Signed-off-by: Emad Habib <[email protected]> Signed-off-by: Tim Vaillancourt <[email protected]> Co-authored-by: Emad Habib <[email protected]> Co-authored-by: Emad Mokhtar <[email protected]>
Description
The goal of this PR is to introduce the structure logging to Vitess. Vitess is using
glog
as logger. So I decided to usenoglog
and create drop-in replacement. I used Zap log to replaceglog
.The PR is also introducing a flag
structure_logging
where the user can choose to use structure logging or the default logging. The default value isFalse
so the default is to use the current loggingglog
.Related Issue(s)
#11613
Checklist
Deployment Notes