From c137028364316dae6cb8c8508febcad305ab80e0 Mon Sep 17 00:00:00 2001 From: Jan Novak Date: Tue, 11 Aug 2026 17:43:51 +0200 Subject: [PATCH] Add plan: verbose V-level logging in the GCP provider Co-Authored-By: Claude --- ...08-11-1742-gcp-provider-verbose-logging.md | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 docs/plans/2026-08-11-1742-gcp-provider-verbose-logging.md diff --git a/docs/plans/2026-08-11-1742-gcp-provider-verbose-logging.md b/docs/plans/2026-08-11-1742-gcp-provider-verbose-logging.md new file mode 100644 index 0000000..88d4b0c --- /dev/null +++ b/docs/plans/2026-08-11-1742-gcp-provider-verbose-logging.md @@ -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_` + 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.