TrueCloudLab/frostfs-node
20
1
Fork 28
Code Issues 114 Pull requests 4 Projects Releases 2 Activity Actions

logger: Add journald support #971

Merged
fyrchik merged 1 commit from dstepanov-yadro/frostfs-node:feat/zapjournald into master 2024-02-12 06:17:24 +00:00
Conversation 0 Commits 1 Files changed 11 +108 -6
11 changed files with 108 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
cmd/frostfs-ir/main.go
View file

@ -68,6 +68,10 @@ func main() {
cfg.GetString("logger.level"),
)
exitErr(err)
err = logPrm.SetDestination(
cfg.GetString("logger.destination"),
)
exitErr(err)
logPrm.SamplingHook = metrics.LogMetrics().GetSamplingHook()
log, err = logger.NewLogger(logPrm)
exitErr(err)

9
cmd/frostfs-node/config.go
View file

@ -96,7 +96,8 @@ type applicationConfiguration struct {
_read bool
LoggerCfg struct {
level string
level string
destination string
}
EngineCfg struct {
@ -209,6 +210,7 @@ func (a *applicationConfiguration) readConfig(c *config.Config) error {
// Logger
a.LoggerCfg.level = loggerconfig.Level(c)
a.LoggerCfg.destination = loggerconfig.Destination(c)
// Storage Engine
@ -1010,6 +1012,11 @@ func (c *cfg) loggerPrm() (*logger.Prm, error) {
// not expected since validation should be performed before
panic(fmt.Sprintf("incorrect log level format: %s", c.LoggerCfg.level))
}
err = c.dynamicConfiguration.logger.SetDestination(c.LoggerCfg.destination)
if err != nil {
// not expected since validation should be performed before
fyrchik marked this conversation as resolved
fyrchik commented 2024-02-08 17:30:07 +00:00
Review
Copy link

Where is validation performed?
I see a comment in the previous if but it could be misleading too.

Where is validation performed? I see a comment in the previous `if` but it could be misleading too.
fyrchik commented 2024-02-08 17:34:56 +00:00
Review
Copy link

Also, do we change this on SIGHUP? Not saying that we need to (it could be even harmful), just to know

Also, do we change this on SIGHUP? Not saying that we need to (it could be even harmful), just to know
dstepanov-yadro commented 2024-02-09 07:00:13 +00:00
Review
Copy link

Where is validation performed?

Here: cmd/frostfs-node/validate.go

do we change this on SIGHUP

No.

> Where is validation performed? Here: `cmd/frostfs-node/validate.go` > do we change this on SIGHUP No.
panic(fmt.Sprintf("incorrect log destination format: %s", c.LoggerCfg.destination))
}
return c.dynamicConfiguration.logger, nil
}

18
cmd/frostfs-node/config/logger/config.go
View file

@ -5,12 +5,14 @@ import (
"time"
"git.frostfs.info/TrueCloudLab/frostfs-node/cmd/frostfs-node/config"
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/logger"
"git.frostfs.info/TrueCloudLab/frostfs-observability/logging/lokicore/loki"
)
const (
// LevelDefault is a default logger level.
LevelDefault = "info"
DestinationDefault = logger.DestinationStdout
subsection = "logger"
lokiSubsection = "loki"
AddressDefault = "localhost:3100"
@ -34,6 +36,22 @@ func Level(c *config.Config) string {
return LevelDefault
}
// Destination returns the value of "destination" config parameter
// from "logger" section.
//
// Returns DestinationDefault if the value is not a non-empty string.
func Destination(c *config.Config) string {
v := config.StringSafe(
c.Sub(subsection),
"destination",
)
if v != "" {
return v
}
return DestinationDefault
}
// ToLokiConfig extracts loki config.
func ToLokiConfig(c *config.Config) loki.Config {
hostname, _ := os.Hostname()

8
cmd/frostfs-node/config/logger/config_test.go
View file

@ -11,15 +11,15 @@ import (
func TestLoggerSection_Level(t *testing.T) {
t.Run("defaults", func(t *testing.T) {
v := loggerconfig.Level(configtest.EmptyConfig())
require.Equal(t, loggerconfig.LevelDefault, v)
require.Equal(t, loggerconfig.LevelDefault, loggerconfig.Level(configtest.EmptyConfig()))
require.Equal(t, loggerconfig.DestinationDefault, loggerconfig.Destination(configtest.EmptyConfig()))
})
const path = "../../../../config/example/node"
fileConfigTest := func(c *config.Config) {
v := loggerconfig.Level(c)
require.Equal(t, "debug", v)
require.Equal(t, "debug", loggerconfig.Level(c))
require.Equal(t, "journald", loggerconfig.Destination(c))
}
configtest.ForEachFileType(path, fileConfigTest)

5
cmd/frostfs-node/validate.go
View file

@ -25,6 +25,11 @@ func validateConfig(c *config.Config) error {
return fmt.Errorf("invalid logger level: %w", err)
}
err = loggerPrm.SetDestination(loggerconfig.Destination(c))
if err != nil {
return fmt.Errorf("invalid logger destination: %w", err)
}
// shard configuration validation
shardNum := 0

1
config/example/node.env
View file

@ -1,4 +1,5 @@
FROSTFS_LOGGER_LEVEL=debug
FROSTFS_LOGGER_DESTINATION=journald
FROSTFS_PPROF_ENABLED=true
FROSTFS_PPROF_ADDRESS=localhost:6060

3
config/example/node.json
View file

@ -1,6 +1,7 @@
{
"logger": {
"level": "debug"
"level": "debug",
"destination": "journald"
},
"pprof": {
"enabled": true,

1
config/example/node.yaml
View file

@ -1,5 +1,6 @@
logger:
level: debug # logger level: one of "debug", "info" (default), "warn", "error", "dpanic", "panic", "fatal"
destination: journald # logger destination: one of "stdout" (default), "journald"
systemdnotify:
enabled: true

2
go.mod
View file

@ -10,6 +10,7 @@ require (
git.frostfs.info/TrueCloudLab/hrw v1.2.1
git.frostfs.info/TrueCloudLab/policy-engine v0.0.0-20240129064140-8d21ab2d99d9
git.frostfs.info/TrueCloudLab/tzhash v1.8.0
git.frostfs.info/TrueCloudLab/zapjournald v0.0.0-20240124114243-cb2e66427d02
github.com/cheggaaa/pb v1.0.29
github.com/chzyer/readline v1.5.1
github.com/flynn-archive/go-shlex v0.0.0-20150515145356-3f9db97f8568
@ -29,6 +30,7 @@ require (
github.com/spf13/cobra v1.8.0
github.com/spf13/pflag v1.0.5
github.com/spf13/viper v1.18.2
github.com/ssgreg/journald v1.0.0
github.com/stretchr/testify v1.8.4
go.etcd.io/bbolt v1.3.8
go.opentelemetry.io/otel v1.22.0

4
go.sum
View file

@ -16,6 +16,8 @@ git.frostfs.info/TrueCloudLab/rfc6979 v0.4.0 h1:M2KR3iBj7WpY3hP10IevfIB9MURr4O9m
git.frostfs.info/TrueCloudLab/rfc6979 v0.4.0/go.mod h1:okpbKfVYf/BpejtfFTfhZqFP+sZ8rsHrP8Rr/jYPNRc=
git.frostfs.info/TrueCloudLab/tzhash v1.8.0 h1:UFMnUIk0Zh17m8rjGHJMqku2hCgaXDqjqZzS4gsb4UA=
git.frostfs.info/TrueCloudLab/tzhash v1.8.0/go.mod h1:dhY+oy274hV8wGvGL4MwwMpdL3GYvaX1a8GQZQHvlF8=
git.frostfs.info/TrueCloudLab/zapjournald v0.0.0-20240124114243-cb2e66427d02 h1:HeY8n27VyPRQe49l/fzyVMkWEB2fsLJYKp64pwA7tz4=
git.frostfs.info/TrueCloudLab/zapjournald v0.0.0-20240124114243-cb2e66427d02/go.mod h1:rQFJJdEOV7KbbMtQYR2lNfiZk+ONRDJSbMCTWxKt8Fw=
github.com/BurntSushi/toml v1.3.2/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ=
github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI=
github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g=
@ -239,6 +241,8 @@ github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/spf13/viper v1.18.2 h1:LUXCnvUvSM6FXAsj6nnfc8Q2tp1dIgUfY9Kc8GsSOiQ=
github.com/spf13/viper v1.18.2/go.mod h1:EKmWIqdnk5lOcmR72yw6hS+8OPYcwD0jteitLMVB+yk=
github.com/ssgreg/journald v1.0.0 h1:0YmTDPJXxcWDPba12qNMdO6TxvfkFSYpFIJ31CwmLcU=
github.com/ssgreg/journald v1.0.0/go.mod h1:RUckwmTM8ghGWPslq2+ZBZzbb9/2KgjzYZ4JEP+oRt0=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=

59
pkg/util/logger/logger.go
View file

@ -1,6 +1,10 @@
package logger
import (
"fmt"
"git.frostfs.info/TrueCloudLab/zapjournald"
"github.com/ssgreg/journald"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
)
@ -35,8 +39,15 @@ type Prm struct {
SamplingHook func(e zapcore.Entry, sd zapcore.SamplingDecision)
// do not support runtime rereading
dest string
}
const (
DestinationUndefined = ""
fyrchik commented 2024-02-08 17:33:20 +00:00
Review
Copy link

We also have Loki which doesn't use this field. What about unifying the approach? (destination: loki)

We also have `Loki` which doesn't use this field. What about unifying the approach? (`destination: loki`)
dstepanov-yadro commented 2024-02-09 07:03:16 +00:00
Review
Copy link

loki works with both of journald and stdout, it is not a destination.

`loki` works with both of `journald` and `stdout`, it is not a destination.
fyrchik commented 2024-02-09 07:27:02 +00:00
Review
Copy link

Journald core can work with stdout too (it does by default I think)

Journald core can work with stdout too (it does by default I think)
dstepanov-yadro commented 2024-02-09 08:04:49 +00:00
Review
Copy link

So then destination possible values will be stdout, journald, stdout+loki, journald+loki? Looks ... strange.

So then `destination` possible values will be `stdout`, `journald`, `stdout+loki`, `journald+loki`? Looks ... strange.
DestinationStdout = "stdout"
DestinationJournald = "journald"
)
// SetLevelString sets the minimum logging level. Default is
// "info".
//
@ -48,6 +59,16 @@ func (p *Prm) SetLevelString(s string) error {
return p.level.UnmarshalText([]byte(s))
}
func (p *Prm) SetDestination(d string) error {
if d != DestinationStdout && d != DestinationJournald {
return fmt.Errorf("invalid logger destination %s", d)
}
if p != nil {
p.dest = d
}
return nil
}
// Reload reloads configuration of a connected instance of the Logger.
// Returns ErrLoggerNotConnected if no connection has been performed.
// Returns any reconfiguration error from the Logger directly.
@ -79,7 +100,17 @@ func NewLogger(prm *Prm) (*Logger, error) {
if prm == nil {
prm = defaultPrm()
}
switch prm.dest {
case DestinationUndefined, DestinationStdout:
return newConsoleLogger(prm)
case DestinationJournald:
return newJournaldLogger(prm)
default:
return nil, fmt.Errorf("unknown destination %s", prm.dest)
}
}
func newConsoleLogger(prm *Prm) (*Logger, error) {
lvl := zap.NewAtomicLevelAt(prm.level)
c := zap.NewProductionConfig()
@ -103,6 +134,34 @@ func NewLogger(prm *Prm) (*Logger, error) {
return l, nil
}
func newJournaldLogger(prm *Prm) (*Logger, error) {
lvl := zap.NewAtomicLevelAt(prm.level)
c := zap.NewProductionConfig()
c.Level = lvl
c.Encoding = "console"
c.EncoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder
if prm.SamplingHook != nil {
c.Sampling.Hook = prm.SamplingHook
}
encoder := zapjournald.NewPartialEncoder(zapcore.NewConsoleEncoder(c.EncoderConfig), zapjournald.SyslogFields)
core := zapjournald.NewCore(zap.NewAtomicLevelAt(prm.level), encoder, &journald.Journal{}, zapjournald.SyslogFields)
coreWithContext := core.With([]zapcore.Field{
zapjournald.SyslogFacility(zapjournald.LogDaemon),
zapjournald.SyslogIdentifier(),
zapjournald.SyslogPid(),
})
lZap := zap.New(coreWithContext, zap.AddStacktrace(zap.NewAtomicLevelAt(zap.FatalLevel)))
l := &Logger{Logger: lZap, lvl: lvl}
prm._log = l
return l, nil
}
func (l *Logger) reload(prm Prm) error {
l.lvl.SetLevel(prm.level)
return nil