Logging improvements #399
Merged
Logging improvements #399
+69
−104
Conversation
This file contains 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
Unified logging is a fetaure that enables writing logs emitted by all server instances instances and the luatest runner itself to a single file. It's implemented by duplicating logs to stdout (using the tee Linux command) and piping stdout to a log file in OutputBeautifier. The OutputBeautifier still writes the logs to stdout, even if unified logging is enabled. As a result, on any failure the runner spams stdout with captured logs of started server instances. There's no need for that because all logs can be read from the unified log file. It only clutters the output making it difficult to figure out what went wrong. So let's disable writing to stdout in OutputBeautifier if unified logging is enabled. No changelog because the feature was added by commit f8a1c10 ("Add logging to unified file"), which hasn't been released yet. Closes tarantool#382
This is ugly. Instead, let's redirect logs directly to the pipe created by the OutputBeautifier. No changelog because the feature was added by commit f8a1c10 ("Add logging to unified file"), which hasn't been released yet.
In the scope of unified logging, new options -vv and -vvv were introduced. They set the luatest log level to VERBOSE and DEBUG, respectively. Actually, there's no need to suppress any logging because it goes to a file and doesn't spam the output. Besides, it's ugly that we mix luatest logging with luatest output verbosity. Let's drop the levels and use log.info instead of log.verbose or log.debug everywhere in luatest. No changelog because the feature was added by commit f8a1c10 ("Add logging to unified file"), which hasn't been released yet.
* Log all assertions, not just a few select ones. * Don't log assertion arguments because that would be too verbose. Instead, log only the file and the line where the assertion is checked. It should be enough for debugging. * Log where helpers.retrying is called. Also, fix formatting of fractional seconds. * Log test errors and failures so that one can figure out where the test failed by just looking at the unified log, without the luatest output. No changelog because the feature was added by commit d985997 ("Add more logs"), which hasn't been released yet.
This is bad because it quotes all strings although most of the times we don't need it. This also makes `luatest.log` work differently from the built-in logger while we want them to be interchangeable. We don't need pretty-printing anymore because we don't log assertion arguments. If the caller needs pretty-printing, they can do it manually. This effectively reverts commit 15dbf75 ("Improve luatest.log function if a nil value is passed") and a good part of commit a005329 ("Add syntax sugar for more convenient logging").
Now, that auto-quoting is disabled in the log, let's quote server names and conditions because without quoting they look ugly. No changelog as this is a minor change related to the previous commit.
locker
force-pushed
the
logging-improvements
branch
from
32fe345
to
0dfcc0d
Compare
Totktonada
approved these changes
Oct 31, 2024
There was a problem hiding this comment.
If I understood the changes right, after the patch we have the following situation: we have -c to see extra logs (child processes output, invoked assertions) and -l to write them to the file. The default is to drop these extra logs. It should be convenient, I think.
I'm not in context of the work that was done for unified logging before, so my review is not deep. However, the proposed patches look meaningful for me.
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.
The goal of this PR is to fix spamming stdout on test failure with unified logging is enabled. Apart from that, it also brings a few improvements to unified logging.
Close #382