Add structured logging #19100
Draft
Add structured logging #19100
+631
−47
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds opt-in structured JSON logging with [zerolog](https://github.com/rs/zerolog). By default, Vitess will continue to use unstructured logging through `glog`. The approach taken here is two-fold: changing the external API (CLI flags used by users), and adding a new internal logging API (log functions called around the codebase). For the external API, the changes are simple. To opt-in to structured logging, use these new flags: - `--structured-logging`: Enables structured logging. - `--structured-logging-level`: Minimum log level: trace, debug, info, warn, error, fatal, panic, disabled (default: info) - `--structured-logging-pretty`: Enable pretty, human-readable output (default: false) - `--structured-logging-file`: Log to a file instead of stdout The existing `--log-rotate-max-size` flag controls the max size of the log file before rotation. Otherwise, the new structured logging flags are mutually exclusive with the old `glog` flags. For the internal API, we have to be able to support a few scenarios in the transition period: 1. The default and current setup: no structured logging enabled, and all logging around the codebase uses the existing global unstructured functions in `go/vt/log`. 2. Structured logging is enabled, but all the logging continues to use the existing global unstructured functions in `go/vt/log`. 3. Structured logging is not enabled, but some log sites around the codebase start migrating to the new structured API. These need to be logged with `glog`, even though they are meant to be structured. 4. Structured logging is enabled, and not all log sites around the codebase have been migrated to the new structured API. To support all these permutations, two things were done: 1. The existing global unstructured functions check if structured logging is enabled. If it isn't, log with `glog` as normal. If it is, log through zerolog, passing the entire message to zerolog's `Msg`. This helps with all cases above. A log using the old API will log to `glog` if structured logging isn't enabled, or to zerolog if it is. With structured logging disabled, normal `glog` output: ```console $ go run ./go/cmd/vttablet/ F0106 10:00:26.433396 86344 server.go:257] topo-global-server-address must be configured ``` With structured logging enabled, message is logged in the `message` key in the structured output: ```console $ go run ./go/cmd/vttablet/ {"level":"info","caller":"/Users/mhamza/dev/vitess/go/vt/servenv/servenv_unix.go:57","time":"2026-01-06T09:22:36-05:00","message":"Version: 24.0.0-SNAPSHOT (Git revision branch '') built on by @ using go1.25.5 darwin/arm64"} {"level":"fatal","caller":"/Users/mhamza/dev/vitess/go/vt/topo/server.go:257","time":"2026-01-06T09:22:36-05:00","message":"topo-global-server-address must be configured"} ``` 2. To support the cases where structured logging isn't enabled, but some log sites have started using the new API, I've added a custom zerolog writer called `glogWriter`. Instead of writing to `stdout` or a file as zerolog would normally, it instead takes zerolog's output buffer and logs it using `glog`. Example: ```diff - log.Exitf("topo-global-server-address must be configured") + log.SExit().Msg("topo-global-server-address must be configured") ``` ```console $ go run ./go/cmd/vttablet F0106 10:07:36.390862 92158 server.go:259] {"level":"fatal","caller":"/Users/mhamza/dev/vitess/go/vt/topo/server.go:259","time":"2026-01-06T10:07:36-05:00","message":"topo-global-server-address must be configured"} ``` As it is being logged through `glog`, all existing `glog` flags will continue to work, just that the normal unstructured message is now zerolog's JSON output. This supports any permutation of structured logging enabled/disabled + log site using old/new API. Note that I did not touch `glog`'s `V()` API. It can be easily mimicked in zerolog using a wrapper type, but since there are only a few places in the codebase that actually use it, it might be simpler just to go and change them all to use normal logging. Still mulling this over, will follow up with another PR. Signed-off-by: Mohamed Hamza <[email protected]> --- Example JSON output: ```console $ vttablet --structured-logging {"level":"info","caller":"/Users/mhamza/dev/vitess/go/vt/servenv/servenv_unix.go:57","time":"2026-01-06T09:22:36-05:00","message":"Version: 24.0.0-SNAPSHOT (Git revision branch '') built on by @ using go1.25.5 darwin/arm64"} {"level":"fatal","caller":"/Users/mhamza/dev/vitess/go/vt/topo/server.go:257","time":"2026-01-06T09:22:36-05:00","message":"topo-global-server-address must be configured"} ``` Example pretty output: ```console $ vttablet --structured-logging --structured-logging-pretty 2026-01-06T09:23:13-05:00 INF go/vt/servenv/servenv_unix.go:57 > Version: 24.0.0-SNAPSHOT (Git revision branch '') built on by @ using go1.25.5 darwin/arm64 2026-01-06T09:23:13-05:00 FTL go/vt/topo/server.go:257 > topo-global-server-address must be configured ``` In v25, structured logging will become the default and `glog` and its flags will be deprecated. In v26, `glog` will be removed. Note: I initially marked the old unstructured functions in `log` as deprecated. This made `staticcheck` complain about any usage of them. While this is nice so that we can be proactive in changing them to the structured API, it means that committing is blocked for each changed file unless the _entire_ package is migrated away from the deprecated functions. This is really aggressive, so I've instead left a note to mention to move to the structured API in each function's doc. When the time comes, we can switch the entire codebase to the new API in one fell swoop. Signed-off-by: Mohamed Hamza <[email protected]>
mhamza15
added
Type: Enhancement
Contributor
|
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. |
vitess-bot
bot
added
NeedsWebsiteDocsUpdate
Contributor
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
|
Signed-off-by: Mohamed Hamza <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
mhamza15
removed
NeedsDescriptionUpdate
mhamza15
requested review from
dbussink,
harshit-gangal,
mattlord,
rohit-nayak-ps and
timvaillancourt
as code owners
Contributor
|
📝 Documentation updates detected! New suggestion: Document structured logging feature |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19100 +/- ##
==========================================
- Coverage 69.90% 69.87% -0.04%
==========================================
Files 1612 1614 +2
Lines 215817 215960 +143
==========================================
+ Hits 150865 150897 +32
- Misses 64952 65063 +111 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Mohamed Hamza <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
Collaborator
Author
|
Moving back to draft to think this through a bit more |
Signed-off-by: Mohamed Hamza <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds opt-in structured JSON logging using zerolog. Vitess continues to use
glogby default.New flags:
--structured-logging- Enable structured logging--structured-logging-level- Set log level (default: info)--structured-logging-pretty- Human-readable output--structured-logging-file- Log to file instead of stdoutInternal changes: New structured logging API in
go/vt/log/zlog.gowithS-prefixed functions (e.g.,SInfo(),SExit()). Both old and new APIs route to the correct backend based on flags.Example JSON output:
Example pretty output:
Roadmap:
glogdeprecatedglogremovedChecklist