logging: reduce hot-path log noise, configurable stdout, harden two failure paths - #61
Merged
Merged
Conversation
Downgrade recurring, self-healing log statements to Debug so the
default (Info) level stays quiet on healthy and still-converging
meshes, while rare actionable failures keep their level:
- reconnect/bootstrap chatter that retries on its own loop
(bootstrap.go, host.go, clock.go), and the 20s CRDT ping publish
in the non-SWIM path
- unmarshalling of remote peer data on the CRDT hot paths (PutHook,
UpdateNodeTableHook, GetService): one peer publishing corrupt data
must not be able to spam every node's error log
- the RemoteGET helper: callers handle and log the returned error at
the severity appropriate to their context
- reannounce failures (fire per connection event, self-heal via CRDT
rebroadcast), and the repairDAG progress ticker ("DAG repair
finished" stays Info)
- proxy ErrorHandler response-write failures (the client is already
gone), aligned in both occurrences
- AnnounceLeave failures become Warn: nothing can act on them at
shutdown, but peers then only learn of the departure via liveness
eviction
Additionally:
- suppress the boxo provider/reprovider loggers, which busy-loop
when a node has no peers
- add a log.output config key (default "stderr", zap's default;
"stdout" routes all output to stdout for K8s-style log collection)
These levels have been running on the swiss-ai production deployment
(CSCS + cloud mesh) since March.
Co-authored-by: robmsmt <rob@robmsmt.com>
Co-authored-by: Rob <robmsmt@gmail.com>
- registerLLMService fetches /v1/models exactly once, right after the health check passes, and nothing re-fetches it later — so a transient blip (e.g. the server finishing warm-up) permanently registers the node without model identity groups. Retry up to 10 times over ~30s and log at Error on final failure. - WriteKeyToFile: a private-key marshal failure fell through and wrote an empty key file, silently poisoning the node's persisted identity. Exit instead, like the function's other failure branches. - LoadKeyFromFile: a corrupt key file means the node mints a fresh identity (new peer ID) and overwrites the file. Warn instead of logging at Debug, since peer IDs appear in bootstrap lists and access-control configuration.
xzyaoi
self-requested a review
August 25, 2026 15:50
xzyaoi
added a commit
that referenced
this pull request
Aug 25, 2026
PR #61 (opened from a fork) failed the "Go tests" job even though all 586 tests and the lint job passed. The only failing step was the best-effort "Comment Coverage" step: it calls github.rest.issues.createComment, but the GITHUB_TOKEN is read-only for pull_request events from forks, so the call 403s with "Resource not accessible by integration" and fails the whole job. - Skip the comment step on fork PRs (where posting is impossible). - Add continue-on-error so a token/permission/API error can never gate the build. Coverage is still generated to build/coverage.* and src/coverage.txt regardless (the Generate Coverage Report step runs unconditionally with if: always()).
xzyaoi
marked this pull request as ready for review
August 25, 2026 20:39
AryanAhadinia
added a commit
to swiss-ai/OpenTela
that referenced
this pull request
Aug 25, 2026
Swiss AI deployment residuals kept on top of upstream now that the bulk of the original logging work has been upstreamed (eth-easl#61): - default log.output to "stdout" (upstream defaults to "stderr") for our K8s/CSCS log collection; still overridable via config or OF_LOG_OUTPUT - rocm-smi / GPU detection Debug diagnostics in platform/gpu.go - ClearCRDTStore removal failure logged at Debug - a few explanatory comments upstream does not carry Co-authored-by: robmsmt <rob@robmsmt.com> Co-authored-by: Rob <robmsmt@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B6tjXbayVaAqJjNXtJZFC1
AryanAhadinia
added a commit
to swiss-ai/OpenTela
that referenced
this pull request
Aug 25, 2026
The single deliberate src divergence from upstream: our K8s/CSCS log collection expects everything on stdout, so the fork defaults log.output to "stdout" where upstream defaults to "stderr" (the mechanism itself was upstreamed in eth-easl#61). Still overridable via config or OF_LOG_OUTPUT. If the stdout routing turns out not to be load-bearing (or once OF_LOG_OUTPUT=stdout is set in the deployment manifests instead), this commit can be dropped entirely. Co-authored-by: robmsmt <rob@robmsmt.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B6tjXbayVaAqJjNXtJZFC1
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
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
Log-level cleanup plus two small robustness fixes, extracted from the swiss-ai production deployment (CSCS + cloud mesh), where these have been running since March. Two commits:
1.
logging: reduce hot-path log noise, add log.output configThe guiding rule: level by frequency and actionability, not by whether something is an error. Recurring statements whose condition is retried by their own loop and whose effect the system repairs itself go to Debug; rare failures someone must act on keep their level.
bootstrap.go,host.go,clock.go) and the 20s CRDT ping publish (non-SWIM path) → Debug. These fire continuously while a mesh is converging or a node is isolated; the one-shotServer started: bootstrap_connected=…line and metrics carry the steady-state signal.PutHook,UpdateNodeTableHook,GetService) → Debug at the call site. These run for every CRDT update, and the payload comes from remote peers — one peer publishing corrupt data shouldn't be able to spam every node's error log.RemoteGEThelper → Debug (callers handle the returned error and log at their own severity; the helper double-reports).repairDAGprogress ticker → Debug ("DAG repair finished" stays Info).ErrorHandlerresponse-write failures → Debug (the client is already gone), aligned in both occurrences.AnnounceLeavefailures → Warn (nothing can act on them at shutdown, but peers then only learn of the departure via liveness eviction — worth surfacing).provider/provider.simple/provider.queueloggers, which busy-loop when a node has no peers. We run FATAL in production; happy to soften to ERROR if you prefer.log.outputconfig key: default"stderr"(zap's default, unchanged);"stdout"routes all output to stdout for K8s-style log collection.2.
fix: retry model fetch on registration; fail hard on key marshal failureregisterLLMServicefetches/v1/modelsexactly once, right after the health check, and nothing re-fetches it later — a transient blip (e.g. vLLM finishing warm-up) permanently registers the node without model identity groups, so model-based routing never reaches it. Now retried up to 10× over ~30s, Error on final failure.WriteKeyToFile: a private-key marshal failure fell through and wrote an empty key file viaos.WriteFile(path, nil, …), silently poisoning the node's persisted identity. Now exits like the function's other failure branches.LoadKeyFromFile: a corrupt key file makes the node mint a fresh peer ID and overwrite the file — now a Warn, since peer IDs appear in bootstrap lists and access-control config.Notes
make test(all packages) andmake lint(golangci-lint 2.6.1, 0 issues) on top of currentmain.registrar.go/node_table.go— happy to rebase whichever lands second.Original noise-reduction work by @robmsmt on the swiss-ai fork; review, transient-vs-persistent triage, and the two hardening fixes were done with the help of Claude Code.
🤖 Generated with Claude Code
https://claude.ai/code/session_01B6tjXbayVaAqJjNXtJZFC1