Files
egress-proxies-operator/docs/plans-executions/2026-09-10-0805-kvm-libvirt-tracing.md
T
kacerrandClaude Fable 5.1 5a6c889531 Make the libvirt trace compact: libvirt API names, one error field, terse test output
The first live run showed the trace working but noisy: testr printed
ts/level/msg and the per-run constants on every line, the error text
appeared twice, host appeared twice on image-cache lines, and the Go
method name was less useful than the libvirt function it maps to.

The tracer now logs rpc=virDomainLookupByName (go-libvirt's names are the
public API names without the vir prefix), error plus libvirtError (code
name) and libvirtCode, and image.go stops re-adding host. The integration
test replaces testr with a compact logr sink: time, logger name, the rpc
as headline, then fields, with provider/host/name omitted since the first
line of the run states them. Docs and execution log updated.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-10 19:13:51 +00:00

5.3 KiB
Raw Blame History

Execution: Trace every libvirt call in the KVM provider

Plan: 2026-09-10-0805-kvm-libvirt-tracing.md

  • Step 0 — Save plan
  • Step 1 — errors.go: shared error fields
  • Step 2 — trace.go: the tracingAPI decorator
  • Step 3 — conn.go: pool logging and wrapping
  • Step 4 — Call sites
  • Step 5 — Integration test output
  • Step 6 — Unit tests
  • Step 7 — Docs, execution log, CHANGELOG (docs and this log done; CHANGELOG waits for the live run)

Steps 1–6 — Tracer, pool logging, tests, integration wiring

Done as one commit on feat/kvm-provider; the plan's assumption that this belongs on the KVM branch rather than its own feature branch was accepted at plan approval.

The code follows the plan closely. Things that came out differently or are worth knowing:

  • Logger names join with / in funcr, . in zap. The trace lines are kvm/libvirt in the unit-test capture and in testr output, and kvm.libvirt under the operator's zap logger. The docs use the zap spelling; the unit test asserts the funcr one. First run of the tier test failed on exactly this.
  • Cancel is not reported as a transport error. The original call dropped the connection twice on cancellation (once from the abort closure, once from the "not a libvirt error" branch, harmlessly, since drop is idempotent). With a log line on each drop that would have printed both context cancelled and transport error for one event, so the transport branch now skips ctx.Err(). Covered by TestHostPool_call_logsDropReason.
  • ConnectListAllDomains in the fake includes every defined domain, so the trace test's fixture (one running managed domain plus one stopped domain for DomainCreate to start) lists two; the V(2) assertion checks for the names key rather than an exact list.
  • libvirtErrorFields also feeds logAPIError, which therefore gains libvirtCodeName (e.g. ErrNoDomain) next to the numeric code.
  • The seam-coverage guard (reflect count of libvirtAPI methods vs the test table) is the only thing standing between a future 18th RPC and a silently untraced call; keep it.

Verification run before committing:

go build ./... && go vet ./... && go vet -tags=integration ./internal/provider/kvm/
go test -short ./...
go test -race -count=1 -cover ./internal/provider/kvm/   # 90.2 %, up from 88.2
go test -tags=integration -count=1 -run TestKVM ./internal/provider/kvm/   # self-skips without env

Worth noting: the live run (make test-integration-kvm against beelink) had not happened when this was written; it is what Step 7 of the KVM provider plan is waiting on, and its output is the acceptance test for this feature. Expected shape is in the plan's Verification section.

Revision after the first live run — less noise, libvirt names

The first live run on beelink showed the trace working but hard to read: testr printed "ts", "level", "msg" and the three per-run constants on every line, the error text appeared twice (error and libvirtMessage), host appeared twice on image-cache lines (the logger already carried it), and method=DomainLookupByName was go-libvirt's name rather than something to look up in libvirt's docs. The user asked for "less verbose but more informative". Changes:

  • The tracer's headline field is now rpc=virDomainLookupByName: the public libvirt API function, which go-libvirt's method names are exactly minus the vir prefix (they come from the same remote_protocol.x). Not gRPC, for the record: libvirt's own XDR-based RPC over the forwarded unix socket.
  • Errors are one error field plus libvirtError=ErrNoDomain and libvirtCode=42; libvirtMessage is gone from both the tracer and logAPIError, since it is the error text.
  • image.go no longer adds host to lines whose logger has it.
  • The integration test replaces testr with a small compactSink (logr.LogSink plus GetCallStackHelper, so t.Log still attributes lines to trace.go): HH:MM:SS.mmm libvirt virDomainCreate domain=… duration=… error=… libvirtError=…. The rpc value becomes the headline, provider/host/name are dropped (the run's first t.Log line states them), and values with spaces are quoted. Production zap output is unchanged apart from the field renames; structured logs are meant to repeat context.

Previewed by driving the sink through the fake provider with an injected DomainCreate failure (a throwaway integration-tagged test, not committed):

trace.go:55: 09:04:33.165 libvirt virDomainLookupByName domain=proxy-abc123def456ghij duration=4µs error="Domain not found: …" libvirtError=ErrNoDomain libvirtCode=42
trace.go:55: 09:04:33.165 libvirt virStorageVolUpload pool=default volume=proxy-abc123def456ghij-seed.iso offset=0 length=0 duration=71µs
kvm.go:262:  09:04:33.165 kvm seed volume uploaded volume=proxy-abc123def456ghij-seed.iso bytes=45056
trace.go:55: 09:04:33.165 libvirt virDomainCreate domain=proxy-abc123def456ghij duration=0s error="internal error: process exited…" libvirtError=ErrResourceBusy libvirtCode=87

Worth noting: length=0 on virStorageVolUpload is what the provider passes (libvirt reads to EOF); the following seed volume uploaded line has the byte count.