review: judge lenses over the agent - #140
Merged
Merged
Conversation
CheckTCP dials its backends concurrently through one context-aware dialer and CheckHTTP drains the body before closing it, so the health sweep pays the slowest backend instead of the sum; HashBackends.Get loses the offset no caller varied; ReplaceNonUtf8 escapes through a strings.Builder (three log lines: 1427 ns, 1092 B, 23 allocs -> 416 ns, 240 B, 6 allocs); UseLabelAsFilter reads its environment once; GetDevicePath skips an entry whose Info fails instead of failing the lookup.
… the store The host-interface counters come from /proc/1/net/dev through the same reader as the netns ones (one read instead of eight per sweep); the metrics client drops the double-register memo that the collect task ordering already rules out, sorts labels in place and clears the gauge map; the journal encoder loses a mutex the writer's own lock covers; the keepalive uses a ticker; the heartbeat reports synchronously and logs once per report; Exit returns its error to the caller that logs it; the health sweep sends a core-only id straight to the die handler; the log subscriber's error channel is buffered so the pump cannot lose the error; the die handler keeps only its two load-bearing stops; IDFromFile checks the id itself; the systemd source keys its unit map by workload id and enters the netns only when a workload needs one; the core store hides its client, logs a dropped identifier error, and keys the status cache without the constant fields; Config.Print prints the yaml in one call.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A judgment-lens pass (reuse, simplification, efficiency, altitude, interface, over-design, dead) over every production file of the agent, read in full. Two commits, one per wave; every change is listed in the commit bodies. Highlights:
utils/check.go:CheckTCPdials its backends concurrently through one context-aware dialer, so a health sweep pays the slowest backend instead of the sum;CheckHTTPdrains the body before closing it.utils/utils.go:ReplaceNonUtf8escapes through astrings.Builder; the label filter reads its environment once.collector: host-interface counters come from/proc/1/net/devthrough the same reader as the netns counters (one file instead of eight per sweep); the metrics client drops a double-register memo the collect-task ordering already rules out.manager/workload: the health sweep sends a core-only id straight to the die handler; the die handler keeps only its two load-bearing stops (the third is pinned byTestHandleWorkloadDieStopsLocalTasksRestartedDuringStatusWrite); the log subscriber's error channel is buffered so the pump cannot lose the error.manager/node: the heartbeat reports synchronously and logs once per report;Exitreturns its error to the caller that already logs it.source/systemd: the unit map is keyed by workload id and the netns is entered only for workloads that need one.store/core: the client accessor is unexported, a dropped identifier error is logged, the status cache key leaves out the constant fields.HashBackends.Get's offset,JournalEncoder's mutex,netStatsFromIface, thereconcilewrapper.Hot-path cost
Health sweep: one
/procread per host-interface workload instead of eight sysfs reads; TCP checks in parallel. Log forward:ReplaceNonUtf8on three sample lines 1427 ns → 416 ns, 1092 B → 240 B, 23 → 6 allocs (go test -bench, both arms on the same host). Everything else is off the request path.Lines
Production −21 net, tests +13 net (a new fixture-backed test for the host-interface counters, a mixed-input case for the escaper, a cancelled-context case for the TCP check).
Evidence
Gate on the branch: build, vet, full tests,
make lint,make fmt-checkandaslon both GOOS green; comment delta +0 −0.