Add structured logging with slog#19327
Conversation
Adds support for JSON structured logging with `slog`. To enable, pass `--log-structured` and optionally a `--log-level debug|info|warn|error` (default info). `glog` is still the default unless `slog` is explicitly enabled with the flags.
We still use a global logging package `go/vt/log` to make the migration easier. In the future, each component should be passed in a logger.
The global functions in the log package now check if structured logging is enabled. If so, passes the message to `slog`. If not, passes it to `glog` as normal.
Call sites were migrated using ast-grep as so:
- `Info(...)` => `Info(fmt.Sprint(...))`
- `Infof(...)` => `Info(fmt.Sprintf(...))`
- ...and so on for other levels and `Depth` variants.
Call sites are now free to incrementally add additional attributes to the message.
Example JSON output:
```console
$ ./bin/vttablet --log-structured
{"time":"2026-01-22T16:13:49.136782-05:00","level":"INFO","source":{"function":"vitess.io/vitess/go/vt/servenv.Init","file":"vitess.io/vitess/go/vt/servenv/servenv_unix.go","line":58},"msg":"Version: 24.0.0-SNAPSHOT (Git revision bba5510 branch 'structured-logging-slog') built on Thu Jan 22 15:59:46 EST 2026 by mhamza@mhamza-psc.local using go1.25.6 darwin/arm64"}
{"time":"2026-01-22T16:13:49.136913-05:00","level":"ERROR","source":{"function":"vitess.io/vitess/go/vt/topo.Open","file":"vitess.io/vitess/go/vt/topo/server.go","line":261},"msg":"topo-global-server-address must be configured"}
```
Signed-off-by: Mohamed Hamza <mhamza@fastmail.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
|
Signed-off-by: Mohamed Hamza <mhamza@fastmail.com>
There was a problem hiding this comment.
Pretty much the only real change. The API is changed to match slog's, and internal helpers decide whether to log with slog or glog depending on the configured flags.
There was a problem hiding this comment.
This is the original prefix logger from log.go, extracted to its own file and changed to the new slog API. This should eventually be removed and consumers (VTOrc) should use structured attributes instead.
|
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. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19327 +/- ##
==========================================
- Coverage 69.95% 69.91% -0.05%
==========================================
Files 1610 1612 +2
Lines 216056 216191 +135
==========================================
+ Hits 151133 151140 +7
- Misses 64923 65051 +128 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
| utils.SetFlagVar(fs, &flagVal, "log-rotate-max-size", "size in bytes at which logs are rotated (glog.MaxSize)") | ||
|
|
||
| fs.BoolVar(&logStructured, "log-structured", true, "enable structured logging") |
There was a problem hiding this comment.
@mhamza15 I suggest we make it clear to the user the structure is JSON
timvaillancourt
left a comment
There was a problem hiding this comment.
LGTM, one help-text suggestion
Signed-off-by: Mohamed Hamza <mhamza@fastmail.com>
Signed-off-by: Mohamed Hamza <mhamza@fastmail.com>
Signed-off-by: Mohamed Hamza <mhamza@fastmail.com>
Signed-off-by: Mohamed Hamza <mhamza@fastmail.com>
Signed-off-by: Mohamed Hamza <mhamza@fastmail.com>
|
@mhamza15 how does this work with file based logging? One thing glog provides users is logging to disk directly, as well as file-rotation, etc. I believe that was built-in to glog Do we plan to support an equivalent feature for users that enable structured logging, but were used to logging to a file? Sorry if this is in the diff, GitHub and I are having problems reading the change |
I intentionally left out that functionality. My reasoning is that Vitess should log to Open to pushback though if anyone feels strongly, but my suggestion is to go this route 🙇♂️ . |
|
@mhamza15 makes sense I'd lean towards we should support logging to disk for structured logging, because I know some bare-metal users that rely (or did rely) on direct logging to disk. So I expect retooling for logging to be a surprise to some. But it's not a must I suppose |
Signed-off-by: Mohamed Hamza <mhamza@fastmail.com>
Signed-off-by: Mohamed Hamza <mhamza@fastmail.com>
Signed-off-by: Mohamed Hamza <mhamza@fastmail.com>
|
📝 Documentation updates detected! Updated existing suggestion: Changelog entry for structured logging with Tip: Set up a GitHub Issues trigger in Projects to enable @Promptless mentions in issues 🐙 |
|
📝 Documentation updates detected! Updated existing suggestion: Document structured logging with slog Tip: Set up a GitHub Issues trigger in Projects to enable @Promptless mentions in issues 🐙 |
Signed-off-by: Mohamed Hamza <mhamza@fastmail.com>
Description
Adds support for JSON structured logging with
slog. To enable, pass--log-structuredand optionally a--log-level debug|info|warn|error(default info).glogis still the default unlessslogis explicitly enabled with the flags.We still use a global logging package
go/vt/logto make the migration easier. In the future, each component should be passed in a logger.The global functions in the log package now check if structured logging is enabled. If so, passes the message to
slog. If not, passes it toglogas normal.The API changes from
Info(args ...any)andInfof(args ...any)toInfo(msg string, attrs ...slog.Attr). Call sites were migrated using ast-grep as so:Info(...)=>Info(fmt.Sprint(...))Infof(...)=>Info(fmt.Sprintf(...))Depthvariants.Call sites are now free to incrementally add additional attributes to the message.
Example JSON output:
If structured attributes are passed, but structured logging is not enabled, the attributes are outputted through glog as
key=valuepairs:The plan is for structured logging to be the default in v24, and glog to be deprecated. In v25, glog will be removed.
Benchmark quick links:
Related Issue(s)
Checklist
Deployment Notes
AI Disclosure