[#461] Configure logger sampling policy #495
Labels
No labels
P0
P1
P2
P3
good first issue
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: TrueCloudLab/frostfs-s3-gw#495
Loading…
Reference in a new issue
No description provided.
Delete branch "pogpp/frostfs-s3-gw:feature/461_logger_sampling_pcy"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #461
@ -1067,0 +1077,4 @@
samplingCore := zapcore.NewSamplerWithOptions(
l.Core(),
time.Second,
What about configuring this value (I don't mind current behavior)?
Also we should mention all new params in
doc/configuration.md
@ -82,1 +81,3 @@
cfgLoggerDestination = "logger.destination"
cfgLoggerLevel = "logger.level"
cfgLoggerDestination = "logger.destination"
cfgLoggerSamplingInitial = "logger.sampling.initial"
We need flag that allow disable sampling at all
94f28ee961
to6b5c0c1478
6b5c0c1478
to93c7a37a3a
@ -787,6 +794,10 @@ func newSettings() *viper.Viper {
// logger:
v.SetDefault(cfgLoggerLevel, "debug")
v.SetDefault(cfgLoggerDestination, "stdout")
v.SetDefault(cfgLoggerSamplingEnabled, true)
Should we disable it by default?
@ -1067,0 +1082,4 @@
// are recorded in a one-second interval. Every other log entry will be dropped within the interval since
// cfgLoggerSamplingThereafter is specified here.
if v.GetBool(cfgLoggerSamplingEnabled) {
As I understand this won't work as expected because we have already built logger wit production config that has enabled sampling
@ -1033,3 +1044,3 @@
return newStdoutLogger(lvl)
return newStdoutLogger(v, lvl)
case destinationJournald:
return newJournaldLogger(lvl)
Why sampler configuration only affects stdout logger? I think both loggers can use it.
@ -1067,0 +1091,4 @@
)
l = zap.New(samplingCore)
}
Can we set this before calling Build? You can set sampling config in
c
as it is done inc.Encoding
andc.EncoderConfig
.https://github.com/uber-go/zap/blob/master/config.go#L37
We need rewrite initialization then (e.g. TrueCloudLab/frostfs-s3-lifecycler#21)
@ -58,0 +58,4 @@
sampling:
enabled: true
initial: 100
thereafter: 100
interval
is missing?@ -371,2 +371,4 @@
level: debug
destination: stdout
sampling:
initial: 100
enabled
is missing?still missing
93c7a37a3a
to864b5f26ab
864b5f26ab
to3d3765fe24
@ -1077,1 +1098,3 @@
c.Level = zap.NewAtomicLevelAt(lvl)
func newJournaldLogger(v *viper.Viper, lvl zapcore.Level) *Logger {
c := zap.NewProductionEncoderConfig()
c.EncodeTime = zapcore.ISO8601TimeEncoder
One last suggestion.
Both jouranld and stdout logger require:
Can we make separate function to get console encoder and reuse it in both constructors? It will make code similar and much easier to read.
For example:
4feb884105
toc71873aab8
@ -371,2 +371,4 @@
level: debug
destination: stdout
sampling:
` enabled: false
backtick symbol?
c71873aab8
tob9bc2f59ca
New commits pushed, approval review dismissed automatically according to repository settings
@ -1058,2 +1067,2 @@
c.Encoding = "console"
c.EncoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder
func newStdoutLogger(v *viper.Viper, lvl zapcore.Level) *Logger {
stdout := zapcore.AddSync(os.Stdout)
It seems before we use
stderr
(from zap production config). Is it ok that we change this?@ -1085,3 +1102,4 @@
zapjournald.SyslogPid(),
})
if v.GetBool(cfgLoggerSamplingEnabled) {
Can we hide this condition in separate function? Currently, we have exactly the same 8 lines here and in
newStdoutLogger
@ -1096,0 +1125,4 @@
encoder := zapcore.NewConsoleEncoder(c)
return encoder
Maybe just
b9bc2f59ca
to277520e19b
277520e19b
to0041203d8f
e26b1db759
to99f273f9af