perf(sandbox): keep envd logging out of journald#2675
Merged
Conversation
PR SummaryMedium Risk Overview Reviewed by Cursor Bugbot for commit 0b94771. Bugbot is set up for automated code reviews on this repo. Configure here. |
❌ 9 Tests Failed:
View the full list of 15 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
The implementation in NewLogger disables stdout logging by default when running in local development mode (isNotFC), requiring the verbose flag to be explicitly set. This contradicts the expected behavior where the -isnotfc flag should automatically enable stdout printing. The condition should be updated to enable stdout if either isNotFC or verbose is true.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Verbose guard added inside unreachable dead code branch
- Removed the unreachable isNotFC branch in HTTPExporter.start() since the HTTP exporter is only created when !isNotFC, making w.isNotFC always false.
- ✅ Fixed: Local dev mode silently discards all log output
- Added stdout as a default exporter when isNotFC=true so local development mode always logs to stdout for visibility.
Or push these changes by commenting:
@cursor push 963a6a035c
Preview (963a6a035c)
diff --git a/packages/envd/internal/logs/exporter/exporter.go b/packages/envd/internal/logs/exporter/exporter.go
--- a/packages/envd/internal/logs/exporter/exporter.go
+++ b/packages/envd/internal/logs/exporter/exporter.go
@@ -109,16 +109,6 @@
continue
}
- if w.isNotFC {
- if w.verbose {
- for _, log := range logs {
- fmt.Fprintf(os.Stdout, "%v", string(log))
- }
- }
-
- continue
- }
-
for _, logLine := range logs {
w.mmdsLock.RLock()
logLineWithOpts, err := w.mmdsOpts.AddOptsToJSON(logLine)
diff --git a/packages/envd/internal/logs/logger.go b/packages/envd/internal/logs/logger.go
--- a/packages/envd/internal/logs/logger.go
+++ b/packages/envd/internal/logs/logger.go
@@ -24,7 +24,8 @@
// Stdout is opt-in via -verbose. Inside FC stdout flows into journald and
// dirties guest pages on every snapshot, so we keep it off by default and
// rely on the HTTP exporter to ship debug logs to the orchestrator.
- if verbose {
+ // In local dev mode (isNotFC), always log to stdout for visibility.
+ if verbose || isNotFC {
exporters = append(exporters, os.Stdout)
}You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue. You can view the agent here.
Reviewed by Cursor Bugbot for commit d50a5ecdb06dc5881254c0e7bac1faea7e5e8f3f. Configure here.
ValentaTomas
force-pushed
the
perf/envd-journal-suppression
branch
3 times, most recently
from
149f960 to
f0e4ad3
Compare
Member
Author
|
Fixed in latest push: stdout is now added when |
ValentaTomas
force-pushed
the
perf/envd-journal-suppression
branch
from
f0e4ad3 to
ee3303b
Compare
envd's zerolog stdout writer is now gated behind a new -verbose flag (default off), so production envd inside FC no longer writes anything to stdout — journald stays clean of per-request debug events. The HTTP exporter still ships full debug to the orchestrator regardless. A journald drop-in caps the rest of the in-VM journal at Storage=persistent / SystemMaxUse=8M / MaxLevelStore=warning so other systemd services can't grow it without bound either. handler.go's "error reading from pty/stdout/stderr" messages were on raw stderr but are about envd-handled user processes, not envd internals; they now flow through the zerolog logger.
ValentaTomas
force-pushed
the
perf/envd-journal-suppression
branch
from
ee3303b to
0b94771
Compare
dobrac
approved these changes
May 16, 2026
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
3 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.
envd's zerolog stdout writer is now gated behind a new `-verbose` flag (default off), so production envd inside FC no longer writes anything to stdout — journald stays clean of per-request debug events. The HTTP exporter still ships full debug to the orchestrator regardless.
A journald drop-in caps the rest of the in-VM journal at `Storage=persistent` / `SystemMaxUse=8M` / `MaxLevelStore=warning` so other systemd services can't grow it without bound either.
`handler.go`'s "error reading from pty/stdout/stderr" messages were on raw stderr but are about envd-handled user processes, not envd internals; they now flow through the zerolog logger.
Split out of #2674 (journal-side half).