Add plan: verbose V-level logging in the GCP provider
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
87
docs/plans/2026-08-11-1742-gcp-provider-verbose-logging.md
Normal file
87
docs/plans/2026-08-11-1742-gcp-provider-verbose-logging.md
Normal file
@@ -0,0 +1,87 @@
|
|||||||
|
# Plan: Verbose (V-level) logging in the GCP provider
|
||||||
|
|
||||||
|
**Created:** 2026-08-11 17:42
|
||||||
|
|
||||||
|
## Context
|
||||||
|
|
||||||
|
Debugging GCP provisioning is currently blind: the operator has zero `.V(n)` calls
|
||||||
|
anywhere, so `--zap-log-level=debug` (or any numeric level) reveals nothing about
|
||||||
|
what the GCP provider is doing — which API calls it makes, with what parameters,
|
||||||
|
and what came back. The goal: with debug/V-level logging enabled, see the details
|
||||||
|
of every GCP Compute API call (Insert/Get/Delete/AggregatedList) including a
|
||||||
|
summary of the response; with default `info` level, the provider stays as quiet
|
||||||
|
as today.
|
||||||
|
|
||||||
|
## Approach (the "how")
|
||||||
|
|
||||||
|
**Logger source — context-carried, not injected.** Provider methods all take
|
||||||
|
`ctx`, and the reconciler already builds a per-request logger
|
||||||
|
(`logf.FromContext(ctx)` in `internal/controller/proxy_controller.go:117`) that
|
||||||
|
carries the proxy's name/namespace. The GCP provider will do
|
||||||
|
`log := logf.FromContext(ctx).WithName("gcp").WithValues("provider", p.name)` at
|
||||||
|
the top of each public method. Zero wiring changes (no registry/constructor/struct
|
||||||
|
changes), and every provider log line automatically inherits the reconcile
|
||||||
|
context (which Proxy triggered it). Calls from the GC sweeper inherit its
|
||||||
|
`orphan-gc` logger name the same way.
|
||||||
|
|
||||||
|
**Verbosity scheme** (logr convention: `.Info()` = V(0), `debug` flag = V(1)):
|
||||||
|
|
||||||
|
- **V(1)** — one line per GCP API call, after it returns: operation, identifying
|
||||||
|
params, outcome. Examples:
|
||||||
|
- `Create`: `"GCP insert instance"` with `zone`, `name`, `machineType`,
|
||||||
|
`image`, `opName` (currently discarded at gcp.go:117 — capture it, it's the
|
||||||
|
only handle for correlating with GCP's operation log), plus a line for the
|
||||||
|
409-already-exists path.
|
||||||
|
- `Get`: `"GCP get instance"` with `zone`, `name`, `status`, mapped `state`, `ip`.
|
||||||
|
- `Delete`: `"GCP delete instance"` with `zone`, `name`, `opName`, and the
|
||||||
|
404-treated-as-success path.
|
||||||
|
- `ListByTag`: `"GCP aggregated list"` with `filter`, `count`.
|
||||||
|
- Error paths at V(1) too: log the raw classification (HTTP status / reason
|
||||||
|
from `googleapi.Error`) before it's wrapped, since the wrapped error the
|
||||||
|
reconciler sees is coarser.
|
||||||
|
- **V(2)** — request/response detail: full curated insert-request summary
|
||||||
|
(network, networkTag, diskSizeGB, port, labels, `cloudInitBytes` = `len`),
|
||||||
|
per-instance lines in `ListByTag` (id, state, uid, age).
|
||||||
|
|
||||||
|
**Curated fields, never raw proto dumps.** `CreateRequest.CloudInit` is resolved
|
||||||
|
user-data possibly sourced from a Secret, and it lands in the insert request's
|
||||||
|
metadata — so logging the request proto wholesale would leak it. Log named safe
|
||||||
|
fields only; for cloud-init, log only its byte length. This is a hard rule, and
|
||||||
|
a test asserts it.
|
||||||
|
|
||||||
|
**Where the calls live: the `Provider` methods in
|
||||||
|
`internal/provider/gcp/gcp.go`** (Create/Get/Delete/ListByTag), not in
|
||||||
|
`realInstances` (deliberately untested by design, gcp.go:86) and not an HTTP
|
||||||
|
round-tripper (would log auth headers/user-data, unredactable). The
|
||||||
|
`instancesAPI` fake seam (`newWithAPI`, gcp.go:96) keeps everything testable.
|
||||||
|
To surface `opName`, change `Provider.Create`/`Delete` to capture the string
|
||||||
|
their `instancesAPI` calls already return instead of discarding it.
|
||||||
|
|
||||||
|
## Files to change
|
||||||
|
|
||||||
|
- `internal/provider/gcp/gcp.go` — add `logf` import; V(1)/V(2) logging in
|
||||||
|
`Create`, `Get`, `Delete`, `ListByTag`; capture opNames. Only file with
|
||||||
|
production changes.
|
||||||
|
- `internal/provider/gcp/gcp_test.go` — new table-driven test
|
||||||
|
`TestLogging_verbosity` (name TBD per house `Test<Function>_<scenario>`
|
||||||
|
style): inject a capturing logger via `logf.IntoContext(ctx, funcr.New(...))`
|
||||||
|
(`github.com/go-logr/logr/funcr`, logr already a direct dep), assert:
|
||||||
|
- at V(1): expected message + keys per operation (incl. opName),
|
||||||
|
- at V(0): nothing logged,
|
||||||
|
- **cloud-init content never appears in any log output** (grep the captured
|
||||||
|
lines for a sentinel string placed in `CloudInit`).
|
||||||
|
- No changes to `provider.Provider` interface, registry, `cmd/main.go`,
|
||||||
|
manifests, or the kubernetes provider (it can copy this pattern later).
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
```bash
|
||||||
|
go test -race ./internal/provider/gcp/...
|
||||||
|
go build ./...
|
||||||
|
```
|
||||||
|
|
||||||
|
Optional live check: run the manager with `--zap-log-level=2` against the GCP
|
||||||
|
project and confirm insert/get lines appear during a Proxy reconcile, and that
|
||||||
|
`--zap-log-level=info` stays quiet.
|
||||||
|
|
||||||
|
Also append a CHANGELOG.md entry per house convention once confirmed working.
|
||||||
Reference in New Issue
Block a user