Azure / Azure/unbounded

Unify repository logging on slog (retire controller-runtime zap backend)

Open
#534 1 comment 0 reactions 0 assignees View on GitHub
enhancement go
Dominant language
Go
Stars
28
Forks
11
Avg merge
1d 8h
Merged PRs (30d)
55

Description

### Summary

The repository standardizes application logging on `log/slog` (via `internal/logger.NewPrettyFieldHandler` and `slog.Default()`), but the controller-runtime binaries are split across two logr backends. controller-runtime's logging facade is `logr`, so each manager must call `ctrl.SetLogger()`; some binaries bridge slog into logr, while others use the kubebuilder-default zap backend. This issue tracks unifying them all onto slog and retiring the zap backend.

### Current state

**slog-bridged (desired pattern):**
- `metalman` - `cmd/metalman/main.go`: `ctrl.SetLogger(logr.FromSlogHandler(slog.Default().Handler()))`
- `agent` - `cmd/agent/internal/cmd/context.go`: builds an slog logger via `internal/logger.NewPrettyFieldHandler` (with level control) then `ctrl.SetLogger(logr.FromSlogHandler(c.Logger.Handler()))`
- `unbounded-operator` - switched in #535 (`ctrl.SetLogger(logr.FromSlogHandler(slog.Default().Handler()))`)

**still on zap (`sigs.k8s.io/controller-runtime/pkg/log/zap`):**
- `machina` - `cmd/machina/machina/controller/manager.go`: `ctrl.SetLogger(zap.New(zap.UseDevMode(true)))`
- `machine-ops-controller` - `cmd/machine-ops-controller/main.go`
- `playpen-operator` - `cmd/playpen-operator/main.go`

### Rationale

- **Consistency.** One logging stack across the repo (slog + `internal/logger`), rather than per-binary console formats.
- **Working verbosity.** `zap.New(zap.UseDevMode(true))` pins the effective level to Debug, so logr `V(1)` still prints. With `logr.FromSlogHandler` over an Info-level slog handler, logr `V(n)` maps to slog level `-n`, so `V(1)+` is suppressed by default and `V`-levels become a real quiet lever. This is the root cause behind #533 (BootstrapMaintainer log churn); unifying on slog is a prerequisite for that fix to work as intended.

### Proposed change

For each remaining zap binary, replace:
```go
ctrl.SetLogger(zap.New(zap.UseDevMode(true)))
```
with either the minimal metalman form:
```go
ctrl.SetLogger(logr.FromSlogHandler(slog.Default().Handler()))
```
or the fuller agent form (explicit level + `internal/logger.NewPrettyFieldHandler`, optional `--log-level`/JSON-in-prod) where a level knob is wanted. Prefer the agent-style setup for long-running controllers so operators get a configurable level.

Once no binary imports `sigs.k8s.io/controller-runtime/pkg/log/zap`, drop the dependency (`go mod tidy`).

### Acceptance criteria

- [ ] No `cmd/` binary imports `sigs.k8s.io/controller-runtime/pkg/log/zap` (`machina`, `machine-ops-controller`, `playpen-operator` migrated).
- [ ] controller-runtime logs for these binaries flow through slog.
- [ ] `zap` removed from `go.mod`/`go.sum` if fully unused across the module.
- [ ] `make build`, `make lint`, `make test` pass.

### References

- #535 - switches `unbounded-operator` to slog (minimal form).
- #533 - BootstrapMaintainer log churn; depends on working `V`-level semantics.
- Precedent: `cmd/metalman/main.go`, `cmd/agent/internal/cmd/context.go`.

Contributor guide

Open the contributing guide

Research direction

Start with the existing slog setup in cmd/metalman/main.go and cmd/agent/internal/cmd/context.go, then inspect the zap logger usage in cmd/machina/machina/controller/manager.go, cmd/machine-ops-controller/main.go, and cmd/playpen-operator/main.go. Compare the minimal and agent-style precedents before checking module references. Done means the listed binaries use slog, zap is removed if unused, and make build, make lint, and make test pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, observability
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.