Fix journald logger #1674

Merged
dstepanov-yadro merged 2 commits from dstepanov-yadro/frostfs-node:fix/fix_logger_unused into master 2025-03-13 12:14:24 +00:00
  1. lvl and encoding are redundant: lvl used directly for zap.Core creation and console encoder created directly by calling zapcore.NewConsoleEncoder
  2. journald logger did not use sampling as console logger. Console logger sampling creates by config.Build call as follows: 6d482535bd/config.go (L283)

Fix #1673

1. lvl and encoding are redundant: lvl used directly for zap.Core creation and console encoder created directly by calling `zapcore.NewConsoleEncoder` 2. journald logger did not use sampling as console logger. Console logger sampling creates by `config.Build` call as follows: https://github.com/uber-go/zap/blob/6d482535bdd97f4d97b2f9573ac308f1cf9b574e/config.go#L283 Fix #1673
dstepanov-yadro added 2 commits 2025-03-13 07:45:05 +00:00
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
[#1673] logger: Add sampling for journald logger
All checks were successful
DCO action / DCO (pull_request) Successful in 30s
Vulncheck / Vulncheck (pull_request) Successful in 1m5s
Build / Build Components (pull_request) Successful in 1m26s
Pre-commit hooks / Pre-commit (pull_request) Successful in 1m29s
Tests and linters / Run gofumpt (pull_request) Successful in 2m23s
Tests and linters / Lint (pull_request) Successful in 2m46s
Tests and linters / Staticcheck (pull_request) Successful in 2m47s
Tests and linters / Tests (pull_request) Successful in 3m1s
Tests and linters / Tests with -race (pull_request) Successful in 3m40s
Tests and linters / gopls check (pull_request) Successful in 3m47s
e59b426d39
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
fyrchik approved these changes 2025-03-13 07:59:00 +00:00
@ -172,0 +171,4 @@
if c.Sampling.Hook != nil {
samplerOpts = append(samplerOpts, zapcore.SamplerHook(c.Sampling.Hook))
}
samplingCore := zapcore.NewSamplerWithOptions(
Owner

What if c.Sampling.Hook == nil, do we need this wrapper?

What if `c.Sampling.Hook == nil`, do we need this wrapper?
Author
Member

Yes.
Console logger uses zap's production config: 6d482535bd/config.go (L161) . Production config defines, that zap will log only first 100 log entries for each level for 1 second. If there are more than 100 log entries in 1 second for level, then zap will log only 100-th log entries. For example, if there are 300 log entries for level warning, then only 0...99, 100, 200, 300 log entries will be logged.
Sampling hook only notifies if log entry was sampled or dropped.

Yes. Console logger uses `zap`'s production config: https://github.com/uber-go/zap/blob/6d482535bdd97f4d97b2f9573ac308f1cf9b574e/config.go#L161 . Production config defines, that zap will log only first 100 log entries for each level for 1 second. If there are more than 100 log entries in 1 second for level, then zap will log only 100-th log entries. For example, if there are 300 log entries for level `warning`, then only 0...99, 100, 200, 300 log entries will be logged. Sampling hook only notifies if log entry was sampled or dropped.
Owner

Hm, it was unexpected to me that we have this enabled by default.

Hm, it was unexpected to me that we have this enabled by default.
fyrchik marked this conversation as resolved
fyrchik added this to the v0.45.0 milestone 2025-03-13 08:42:39 +00:00
Owner

Please, mention Fix #1673 in the OP.

Please, mention `Fix #1673` in the OP.
dstepanov-yadro force-pushed fix/fix_logger_unused from e59b426d39 to 3f149a0a0a 2025-03-13 08:48:40 +00:00 Compare
dstepanov-yadro changed title from WIP: Fix journald logger to Fix journald logger 2025-03-13 11:50:27 +00:00
requested reviews from storage-core-developers, storage-core-committers 2025-03-13 11:50:28 +00:00
dstepanov-yadro force-pushed fix/fix_logger_unused from 3f149a0a0a to 7893d763d1 2025-03-13 11:52:20 +00:00 Compare
acid-ant approved these changes 2025-03-13 11:56:09 +00:00
aarifullin approved these changes 2025-03-13 12:00:01 +00:00
dstepanov-yadro merged commit 7893d763d1 into master 2025-03-13 12:14:24 +00:00
dstepanov-yadro deleted branch fix/fix_logger_unused 2025-03-13 12:14:31 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#1674
No description provided.