Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

added logging filters for IL #595

Merged
merged 1 commit into from
Jul 18, 2024
Merged

added logging filters for IL #595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions cmd/o2-aliecs-core/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,18 @@ func init() {
ForceFormatting: true,
})
log.SetOutput(os.Stdout)
ilHook, err := infologger.NewDirectHook("ECS", "core")
if err == nil {
log.AddHook(ilHook)
}
}

func main() {
if err := core.NewConfig(); err != nil {
log.Fatal(err)
}

ilHook, err := infologger.NewDirectHook("ECS", "core", nil)
if err == nil {
log.AddHook(ilHook)
}

if err := core.Run(); err != nil {
log.Fatal(err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/o2-aliecs-executor/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import (

func init() {
logrus.SetOutput(os.Stdout)
ilHook, err := infologger.NewDirectHook("ECS", "executor")
ilHook, err := infologger.NewDirectHook("ECS", "executor", logrus.AllLevels)
if err == nil {
logrus.AddHook(ilHook)
}
Expand Down
15 changes: 8 additions & 7 deletions cmd/o2-apricot/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,26 +35,27 @@ import (

func init() {
log.SetFormatter(&prefixed.TextFormatter{
FullTimestamp: true,
SpacePadding: 20,
PrefixPadding: 12,
FullTimestamp: true,
SpacePadding: 20,
PrefixPadding: 12,

// Needed for colored stdout/stderr in GoLand, IntelliJ, etc.
ForceColors: true,
ForceFormatting: true,
})
log.SetOutput(os.Stdout)
ilHook, err := infologger.NewDirectHook("ECS", "apricot")
if err == nil {
log.AddHook(ilHook)
}
}

func main() {
if err := apricot.NewConfig(); err != nil {
log.Fatal(err)
}

ilHook, err := infologger.NewDirectHook("ECS", "apricot", nil)
if err == nil {
log.AddHook(ilHook)
}

if err := apricot.Run(); err != nil {
log.Fatal(err)
}
Expand Down
43 changes: 30 additions & 13 deletions common/logger/infologger/directhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (

"github.com/AliceO2Group/Control/common/utils"
"github.com/sirupsen/logrus"
"github.com/spf13/viper"
Copy link
Member

@teo teo Jul 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inconvenience here is that you've just pulled viper as a dependency of the executor by way of DirectHook, and the executor doesn't even use Viper. I don't know how fat Viper is in absolute terms, but it's a fairly comprehensive library with over a dozen of its own dependencies. So far I've mostly tried to keep the executor small and light as much as possible to ensure fast copying to the Mesos sandbox and startup (and even with that, it's up to 28MB now). Most likely it's not a big deal, but it would be good if you could check the effect of this change on binary size, and if significant, try to find a way to avoid the added dependency.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, looks like I did that recently by using utils.TimeTrack in the executor (and only this so far, AFAICT), so a Viper dependency is already there, unfortunately. I think it would still be good to keep Viper out of the executor, but it's probably out of scope of this logging effort.

)

const INFOLOGGER_MAX_MESSAGE_SIZE = 1024
Expand All @@ -46,8 +47,26 @@ var (
hostname string
Pid string
username string

logILInfoLevel = []logrus.Level{
logrus.PanicLevel,
logrus.FatalLevel,
logrus.ErrorLevel,
logrus.WarnLevel,
logrus.InfoLevel,
}
logILAllLevel = logrus.AllLevels
currentIlLevel = logILInfoLevel
)

func setCurrentILLevelFromViper() {
if viper.GetBool("logAllIL") {
currentIlLevel = logILAllLevel
} else {
currentIlLevel = logILInfoLevel
}
}

var lineBreaksRe = regexp.MustCompile(`\r?\n`)

func init() {
Expand Down Expand Up @@ -140,7 +159,14 @@ func guessSocketPath() string {
}
}

func NewDirectHook(defaultSystem string, defaultFacility string) (*DirectHook, error) {
func NewDirectHook(defaultSystem string, defaultFacility string, levelsToLog []logrus.Level) (*DirectHook, error) {

if levelsToLog == nil {
setCurrentILLevelFromViper()
} else {
currentIlLevel = levelsToLog
}

socketPath := guessSocketPath()
sender := newSender(socketPath)
if sender == nil {
Expand All @@ -154,25 +180,16 @@ func NewDirectHook(defaultSystem string, defaultFacility string) (*DirectHook, e
}, nil
}

func NewDirectHookWithRole(defaultSystem string, defaultFacility string, defaultRole string) (*DirectHook, error) {
dh, err := NewDirectHook(defaultSystem, defaultFacility)
func NewDirectHookWithRole(defaultSystem string, defaultFacility string, defaultRole string, levelsToLog []logrus.Level) (*DirectHook, error) {
dh, err := NewDirectHook(defaultSystem, defaultFacility, levelsToLog)
if dh != nil {
dh.role = defaultRole
}
return dh, err
}

func (h *DirectHook) Levels() []logrus.Level {
// Everything except Trace
return []logrus.Level{
logrus.PanicLevel,
logrus.FatalLevel,
logrus.ErrorLevel,
logrus.WarnLevel,
logrus.InfoLevel,
logrus.DebugLevel,
logrus.TraceLevel,
}
return currentIlLevel
}

func (h *DirectHook) Fire(e *logrus.Entry) error {
Expand Down
2 changes: 2 additions & 0 deletions core/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ func setDefaults() error {
viper.SetDefault("taskClassCacheTTL", 7*24*time.Hour)
viper.SetDefault("kafkaEndpoints", []string{"localhost:9092"})
viper.SetDefault("enableKafka", true)
viper.SetDefault("logAllIL", false)
return nil
}

Expand Down Expand Up @@ -190,6 +191,7 @@ func setFlags() error {
pflag.Duration("taskClassCacheTTL", viper.GetDuration("taskClassCacheTTL"), "TTL for task class cache entries")
pflag.StringSlice("kafkaEndpoints", viper.GetStringSlice("kafkaEndpoints"), "List of Kafka endpoints to connect to (default: localhost:9092)")
pflag.Bool("enableKafka", viper.GetBool("enableKafka"), "Turn on the kafka messaging")
pflag.Bool("logAllIL", viper.GetBool("logAllIL"), "Send all the logs into IL, including Debug and Trace messages")

pflag.Parse()
return viper.BindPFlags(pflag.CommandLine)
Expand Down
Loading