Consistent logging #14232
Consistent logging #14232
Conversation
mkuratczyk
force-pushed
the
consistent-logging
branch
6 times, most recently
from
b77fb53 to
7e3ad60
Compare
mkuratczyk
force-pushed
the
consistent-logging
branch
3 times, most recently
from
e2b7ed3 to
4f3f4b0
Compare
mkuratczyk
force-pushed
the
consistent-logging
branch
3 times, most recently
from
02df90c to
108a39f
Compare
It's added automatically by logger
`?RMQLOG_DOMAIN_GLOBAL` used to be added by a now deleted logging module rabbit_log.
Some community plugins use rabbit_log. To simplify the transition, we can keep this module as a simple wrapper on logger macros.
mkuratczyk
force-pushed
the
consistent-logging
branch
from
8fed766 to
f6e718c
Compare
There was a problem hiding this comment.
There are many places where information such as module, function name, arity, line number are now passed into the format string in addition to the logger macros included by file logger.hrl providing this same information:
https://github.com/erlang/otp/blob/c8ee2093fce4766d52e4f246123818d2273bee92/lib/kernel/include/logger.hrl#L61-L63
Should this PR remove the now duplicated provided infos from the format strings?
EDIT: let's do these changes in a follow up PR
| rabbit_log:info("[~s:~s/~b] " Str, | ||
| ?LOG_INFO("[~s:~s/~b] " Str, |
There was a problem hiding this comment.
The INFO macro should be deleted given that the LOG_INFO macro already includes module, function name, and arity.
| ok_msg := OkMsg, | ||
| acting_user := ActingUser} = Spec, | ||
| rabbit_log:debug("~s:~s Local pid resolved ~0p", | ||
| ?LOG_DEBUG("~s:~s Local pid resolved ~0p", |
There was a problem hiding this comment.
The LOG_DEBUG already includes module, function name, and arity. There is no need to pass the module and function name again as parameter to the format string.
| -warning("AMQP tracing is enabled"). | ||
| -define(TRACE(Format, Args), | ||
| rabbit_log:debug( | ||
| ?LOG_DEBUG( |
Make our logging more consistent:
pid => self()metadata, since that's automatically added by loggerThe domain should be set on more processes, but we can do that over time. The fact they are missing shouldn't affect anything anyway, since currently we don't log the domain on the logger level, but rather many log strings contain the domain "informally" (eg. peer discovery sets the domain but also prefixes all logs with "Peer discovery:")