Upgrade dskit and improve log.InitLogger function #2799
Conversation
duricanikolic
requested review from
joe-elliott,
annanay25,
mdisibio,
mapno,
yvrhdn,
zalegrala,
electron0zero,
ie-pham and
stoewer
as code owners
|
|
duricanikolic
force-pushed
the
yuri/dskit-log-update
branch
2 times, most recently
from
6a1a4f7 to
058cbff
Compare
Signed-off-by: Yuri Nikolic <[email protected]>
duricanikolic
force-pushed
the
yuri/dskit-log-update
branch
from
058cbff to
bae7aa7
Compare
| // add support for level based logging | ||
| logger = level.NewFilter(logger, LevelFilter(cfg.LogLevel.String())) | ||
| // use UTC timestamps and skip 5 stack frames. | ||
| logger = kitlog.With(logger, "ts", kitlog.DefaultTimestampUTC, "caller", kitlog.Caller(5)) |
There was a problem hiding this comment.
What does it mean to change the call stack depth here? Do we now report a different line is doing the logging?
There was a problem hiding this comment.
Hi @zalegrala,
The reason for that is the implementation of dslog.NewGoKitWithWriter(), that is now used to create the actual logger, and that replaces the “manual” creation that was used before.
I did a manual test to check the outcome and it prints the same source and line it used to print before my change.
I will prepare a Unit test that ensures the right thing is printed once I am back from PTO. I will also replace the usage of RateLimitedLogger with the one from dskit.
|
This PR has been automatically marked as stale because it has not had any activity in the past 60 days. |
What this PR does:
This PR upgrades
dskitand usesdskit'slogpackage for the creation of the global logger.For the main in the
dskitsee grafana/dskit#359.Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]