vmcp: accept injected *slog.Logger via Config (avoid sharing slog.Default with the embedder)
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 2.2k
- Forks
- 300
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 184
Description
Problem
pkg/vmcp/server library code uses package-level slog calls (slog.Info, slog.Warn, slog.Error, etc.) throughout. These read slog.Default() at call time. There is no public field on vmcpserver.Config for an embedder to inject a *slog.Logger.
For library consumers that embed pkg/vmcp/server inside a multi-component Go process with its own logger context, this means vmcp's logs share the embedder's process-wide slog.Default(). Useful when that's what the embedder wants; friction when the embedder wants per-component tagging, separate sinks, or different log levels per component.
The workarounds today:
- Mutate
slog.SetDefault()globally before vmcp lifecycle calls, then restore after. Fragile (any future internal vmcp callsite that reads the default at a different point breaks the save/restore window), and clobbers the embedder's own logger config for the duration. - Accept that vmcp logs land in the embedder's default sink with no per-component metadata.
The standalone cmd/vmcp binary doesn't run into this because it owns its process and sets slog.SetDefault() once at startup.
Requested fix
Add Logger *slog.Logger to vmcpserver.Config. Behavior:
- If
Config.Logger == nil: current behavior preserved. vmcp's package-levelslogcalls fall back toslog.Default(). - If
Config.Logger != nil: vmcp uses the injected logger for all internal logging. Internally this likely means routing the package-level helpers through a private function that prefersConfig.Loggerwhen set.
This is backward-compatible (additive Config field) and gives library consumers a clean opt-in injection point.
Suggested API
type Config struct {
// ... existing fields ...
// Logger, when non-nil, is used for all vmcp internal logging.
// When nil, vmcp's package-level slog calls fall back to
// slog.Default() (current behavior).
Logger *slog.Logger
}
The zap-side equivalent can land as a second additive field (ZapLogger *zap.Logger) or, preferably, vmcp uses an internal slog→zap adapter so a single Config.Logger covers both. Either is fine from a consumer perspective.
Acceptance
- Existing tests pass; standalone
cmd/vmcpbinary behavior unchanged whenConfig.Loggeris left nil. - A new test confirms that, with
Config.Loggerset to a fresh*slog.Logger, vmcp's emissions go to the injected handler rather than the package default. - A note in
pkg/vmcp/doc.go(or the server README) documents the library-consumer pattern.
Why this matters
The change is backward-compatible and unblocks a clean library-embedding story without forcing every consumer to wrap vmcp lifecycle calls in save/restore boilerplate or to accept vmcp's logs sharing the embedder's default sink unmodified.
Happy to send a PR if the API shape above is roughly the direction you'd accept; flag any deviations before I start.
Edited 2026-05-26: the original framing claimed vmcp itself mutates slog.SetDefault / zap.ReplaceGlobals at startup. On closer inspection that mutation is consumer-side (in a library-embedding consumer's own bootstrap), not in pkg/vmcp/*. The underlying request stands: an injectable Config.Logger is the clean fix.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in pkg/vmcp/server by locating Config, the package-level slog calls, and any zap-side logging paths; trace how Config reaches lifecycle code. Run the existing server tests before adding coverage for an injected logger and the nil fallback. Done means injected emissions use the configured handler, existing behavior and cmd/vmcp remain unchanged, and pkg/vmcp/doc.go or the server README documents the pattern.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 56/100