Print errors once, not per span level#3592
Open
kichristensen wants to merge 2 commits into
Open
Conversation
Remove console logging from TraceLogger.Error() so errors are only recorded to the OTel span. main.go (porter and exec) now prints the final error to stderr exactly once after command execution fails, eliminating the repeated/growing error chain visible in the CLI output. Re-enable TestCLI integration test. Fixes getporter#3355, getporter#2278 Signed-off-by: Kim Christensen <kimworking@gmail.com>
kichristensen
changed the title
Errors are now written to the zap logger (file sink) but suppressed from the console sink; main.go still prints the final error once to stderr. Plugin logger also excludes error level since plugins communicate errors via return values. Update golden files and test expectations accordingly. Signed-off-by: Kim Christensen <kimworking@gmail.com>
schristoff
reviewed
May 18, 2026
schristoff
reviewed
May 18, 2026
| Writing logs to /.porter/logs/0.json | ||
| a thing happened | ||
| a weird thing happened | ||
| a bad thing happened |
Member
There was a problem hiding this comment.
I still believe these should be capture-able, are they no longer so?
Contributor
Author
There was a problem hiding this comment.
Errors are still captured to the log file, but not by the console per-span. This file only tracks console output, the error is shown once, printed by main.go
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
2 participants
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 does this change
Errors in Porter were printed multiple times to the console — once per call stack layer — because
TraceLogger.Error()wrote to both the OTel span and the Zap console logger, and error wrapping caused each layer to re-log the growing chain:This PR removes the Zap logger call from
TraceLogger.Error()so it only records to the OTel span. Bothcmd/porter/main.goandcmd/exec/main.gonow print the final error to stderr exactly once after the command fails.The rule is now clear:
log.Error(err)— records to telemetry/tracing only, no console outputlog.Debug/Info/Warn— prints to console and records to spanmain.go— sole place that prints the final error to the userThe long-disabled
TestCLIintegration test is re-enabled.The new error output will look like this:
instead of the the current:
What issue does it fix
Closes #3355
Closes #2278
Notes for the reviewer
Issue #2284 proposed adding a new span-without-logger API. This PR takes a simpler approach: remove console output from
Error()entirely and letmainown the user-facing error print. No new API surface, no call-site changes needed.Checklist