-
Notifications
You must be signed in to change notification settings - Fork 109
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
Emit runtime logs in oasis-node logs #4709
Conversation
518d990
to
ceec4f0
Compare
prefixes := []interface{}{ | ||
"ts", log.DefaultTimestampUTC, | ||
"caller", log.Caller(defaultUnwind + extraUnwind), | ||
"module", module, |
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.
Slight simplification: We only used to set this for module!=""
. Grepping shows this is currently always the case. I don't think that usage of module==""
is encouraged, so if it ever does happen, it should be fine to explicitly draw attention to it with an empty string in the logs.
// except for the module name. Its level will be set in accordance | ||
// with the global config, which can be per-module. | ||
// | ||
// This may be called from any point, including before Initialize is |
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.
I implemented this (and copy-pasted the doc paragraph) for consistency with GetLogger(), but I'm happy to simplify the code and panic if logging is not initialized and the method is called. It would work for the current use case, and no other use cases are foreseen.
222e5e9
to
018d4bd
Compare
Seems like some lints and unit tests are failing. |
991ab35
to
52582f8
Compare
This exposes the previously private base logger. It also slightly changes the semantics of base logger in that it no longer contains _any_ prefixes, whereas it used to contain the timestamp. This is because logs from the runtime already come timestamped, and we want to expose them via the base logger.
Co-authored-by: Jernej Kos <jernej@kos.mx>
- extract wrapper from helpers.go to a new logger.go - extract max buffer size constant - log lines with no specified level at INFO
52582f8
to
ec8878b
Compare
Codecov Report
@@ Coverage Diff @@
## master #4709 +/- ##
==========================================
+ Coverage 66.86% 66.97% +0.11%
==========================================
Files 440 441 +1
Lines 49254 49338 +84
==========================================
+ Hits 32934 33045 +111
+ Misses 12280 12237 -43
- Partials 4040 4056 +16
Continue to review full report at Codecov.
|
Resolves #1819
Testing (besides unit tests): Ran an e2e scenario involving a nontrivial runtime, manually inspected logs (
less $(ls -dt --color=none /tmp/oasis-test-runner* | head -1)/e2e/runtime/runtime/network/compute-0/node.log
) to verify that runtime logs look as expected.