-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
roachprod: don't pollute logs with verbose ssh logs #37594
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
@@ -116,6 +116,9 @@ func loadClusters() error { | |||
return err | |||
} | |||
|
|||
debugDir := os.ExpandEnv(config.DefaultDebugDir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: put this in initHostDir
and change the name of that function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something certainly needed to be done here, Ctrl-C on roachprod run was filling up my terminal buffer. Users may end up with a bunch of these over time but it’s probably fine. We could potentially add some logic that goes and gcs after some number of days. For another PR I suppose.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner and @tbg)
I added verbose logging in cockroachdb#37483 but made the poor decision to spew it to stderr when the command exited nonzero. As a consequence, many test failures are now obfuscated by high-verbosity ssh logs. Change the behavior so that the error message is wrapped with a path to the logs. We grab the contents of `~/.roachprod` in CI, so we'll be able to access the logs in the artifacts (`roachprod_state`). ``` $ ./bin/roachprod ssh tobias-test -- echo hi hi $ ls ~/.roachprod/debug/ $ ./bin/roachprod ssh tobias-test -- false Error: ssh verbose log retained in /Users/tschottdorf/.roachprod/debug/ssh_35.231.83.24_2019-05-20T12:53:49Z: exit status 1 $ ls ~/.roachprod/debug/ ssh_35.231.83.24_2019-05-20T12:53:49Z ``` Touches cockroachdb#36963. Release note: None
bors r=nvanbenschoten,ajwerner |
37594: roachprod: don't pollute logs with verbose ssh logs r=nvanbenschoten,ajwerner a=tbg I added verbose logging in #37483 but made the poor decision to spew it to stderr when the command exited nonzero. As a consequence, many test failures are now obfuscated by high-verbosity ssh logs. Change the behavior so that the error message is wrapped with a path to the logs. We grab the contents of `~/.roachprod` in CI, so we'll be able to access the logs in the artifacts (`roachprod_state`). ``` $ ./bin/roachprod ssh tobias-test -- echo hi hi $ ls ~/.roachprod/debug/ $ ./bin/roachprod ssh tobias-test -- false Error: ssh verbose log retained in /Users/tschottdorf/.roachprod/debug/ssh_35.231.83.24_2019-05-20T12:53:49Z: exit status 1 $ ls ~/.roachprod/debug/ ssh_35.231.83.24_2019-05-20T12:53:49Z ``` Touches #36963. Release note: None Co-authored-by: Tobias Schottdorf <[email protected]>
Build succeeded |
I added verbose logging in #37483 but made the poor decision to spew it
to stderr when the command exited nonzero. As a consequence, many test
failures are now obfuscated by high-verbosity ssh logs.
Change the behavior so that the error message is wrapped with a path to
the logs. We grab the contents of
~/.roachprod
in CI, so we'll be ableto access the logs in the artifacts (
roachprod_state
).Touches #36963.
Release note: None