log: structured logging #9083
log: structured logging #9083
Conversation
|
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ellemouton
changed the title
ellemouton
force-pushed
the
slog
branch
2 times, most recently
from
61e241c to
9ca6dac
Compare
ellemouton
force-pushed
the
slog
branch
2 times, most recently
from
3bdef45 to
4a09edb
Compare
ellemouton
force-pushed
the
slog
branch
3 times, most recently
from
f259e66 to
9587f54
Compare
ellemouton
force-pushed
the
slog
branch
2 times, most recently
from
81f301e to
fb0489c
Compare
ellemouton
force-pushed
the
slog
branch
2 times, most recently
from
845fa01 to
cde7702
Compare
ellemouton
force-pushed
the
slog
branch
5 times, most recently
from
5ce706e to
03c2689
Compare
|
added a bonus commit which adds call-site to the log lines (if user wants) :)
Nice for devs so they can click through to the log :) |
| @@ -98,13 +111,55 @@ func (p *PrefixLog) Critical(v ...interface{}) { | |||
| p.log.Critical(p.addArgsPrefix(v)...) | |||
| } | |||
|
|
|||
| // TraceS writes a structured log with the given message and key-value pair | |||
| // attributes with LevelTrace to the log. | |||
| func (p *PrefixLog) TraceS(ctx context.Context, msg string, attrs ...any) { | |||
There was a problem hiding this comment.
Can we create the context inside this method so the caller doesn't need to create it? Think this way we won't need more code lines when creating logs
There was a problem hiding this comment.
Passing in a context is part of what's cool about this feature cause we can add kv pair attributes to a context and that will get logged without us needing to remember to include it in the log every time
ellemouton
force-pushed
the
slog
branch
3 times, most recently
from
7dd1b56 to
0dbd683
Compare
build/handlers.go
Outdated
| "github.com/btcsuite/btclog/v2" | ||
| ) | ||
|
|
||
| // NewDefaultLoggers returns the standard console logger and rotating log |
There was a problem hiding this comment.
I find this name a bit misleading, because it returns a handler not a logger or was it intentional. Moreover I think we separated the rotating log from the logger so this comment seems to be outdated ?
There was a problem hiding this comment.
changed the name.
Moreover I think we separated the rotating log from the logger so this comment seems to be outdated ?
looks correct to me? it returns a console log handler and a rotator file handler
There was a problem hiding this comment.
Upps, you are right sorry
To make the upcoming commits easier to review and for general code organisation.
These are two separate concerns. So this commit splits them up and just passes a LogWriter from the one to the other. This will become cleaner in an upcoming commit where the Rotator will implement io.Writer and there will no longer be a need for LogWriter.
This commit then also adds an implementation of the btclog.Handler interface called `handlerSets` which basically lets us have a Handler backed by many Handlers (we need one for the stdout and one for our log file).
|
Thanks @ziggie1984 - PTAL 🙏 |
Start using the new slog handlers. With this commit we also remove the need for the LogWriter since we let our LogRotator implement io.Writer and pass that in to our log file handler.
This commit adds config options so that users can for both the console logger and the file logger set the following things: - disable the logger - omit timestamps from log lines.
Since most of our projects will use the same handler duo and apply the config options the same way, let's make our lives easier and add a default handler constructor.
This will only be available when the dev build tag is set
Such that it implements the new expanded interface of btclog v2.
Ensure that the ShutdownLogger correctly calls shutdown for the new CriticalS method added to the btclog.Logger.
Update LND logger(s) to make use of structured logging (the
Loggerinterface now has structured logging messages (DebugS,InfoSetc))See btclog update here: btcsuite/btclog#17
Tracing
The big bonus of this update is that you only need to add your identifier(s) once to the context when the request comes in and then as long as you pass that context through & use it when you log, you dont need to remember to specify the identifiers again. This can take the form of a
request_id=??for incoming user requests orchannel_id=xyzfor anything channel related.Searchable logs
With the tracing above, each log will have a
request_id=abcdin it and each log will use this same structure. So log searches will now become way easier.The other thing is that the way we generally write logs today is using formatted strings like:
❌ Out with the old
Which appears in the logs as:
This is not ideal for a few reasons:
%d``%v) makes the log unique... And you cant necessarily just grep onFinalizing pending_idbecause what if there is another log line somewhere that starts the same way?Enter structured logging:
✅ In with the new
The above log line could now be converted into something like this:
Which will result in the following log line:
A few things to note:
request_id) at log time.request_ideach time you add a new log line! It will just automagically appear 🪄Details
slog.Handlers(which underneath could be a json formatter, a logfmt formatter or any anything else like a charm HandlerUsage
The idea is that the
msgpassed to anSmethod should be a constant. Theattr...params are then used for key-value pairs.Each new method also takes a
context.Context. TheWithCtxhelper can be used to attach any key-value pairs to acontext and then when the logger is called, the pairs will be extracted from the context and added as attributes in the log.
For example:
Other additions:
Config options
This also introduces a new
--logging.console.disableoption to disable logs being writtento stdout and a new
--logging.file.disableoption to disable writing logs to the standard log file.It also has
--logging.console.no-timestamps&--logging.file.no-timestampsto omit timestamps from the logsIt also adds new
--logging.console.call-siteand--logging.console.call-siteoptions which can be used to include log line source call-site in the log lines. The options are "off", "short" (source file name and line number) and "long" (full path to source file and line number)finally, there is a new
--logging.console.styleoption which can be set to use color formatting - this is only available if thedevbuild flag is set.TODO:
Fixes #8713
Fixes #6828
Fixes #6163