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

Feature/logfile #8

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Feature/logfile #8

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
736f772
server/server.go:
llogen Oct 11, 2021
a566ef7
server/server.go:
llogen Oct 11, 2021
c977da7
server/server.go:
llogen Oct 11, 2021
a7f74b1
Merge branch 'feature/logfile' of github.com:9elements/contest-1 into…
llogen Nov 29, 2021
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
14 changes: 14 additions & 0 deletions cmds/contest/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ package server
import (
"flag"
"fmt"
"io"
"os"
"sync"
"syscall"
"time"

"github.com/benbjohnson/clock"
"github.com/sirupsen/logrus"

"github.com/linuxboot/contest/pkg/api"
"github.com/linuxboot/contest/pkg/config"
Expand Down Expand Up @@ -47,6 +49,7 @@ var (
flagTargetLocker *string
flagInstanceTag *string
flagLogLevel *string
flagLogFile *string
flagPauseTimeout *time.Duration
flagResumeJobs *bool
flagTargetLockDuration *time.Duration
Expand All @@ -61,6 +64,7 @@ func initFlags(cmd string) {
flagTargetLocker = flagSet.String("targetLocker", "auto", "Target locker implementation to use, \"auto\" follows DBURI setting")
flagInstanceTag = flagSet.String("instanceTag", "", "A tag for this instance. Server will only operate on jobs with this tag and will add this tag to the jobs it creates.")
flagLogLevel = flagSet.String("logLevel", "debug", "A log level, possible values: debug, info, warning, error, panic, fatal")
flagLogFile = flagSet.String("logFile", "", "A path to a log file, where the logs will saved seperatly")
flagPauseTimeout = flagSet.Duration("pauseTimeout", 0, "SIGINT/SIGTERM shutdown timeout (seconds), after which pause will be escalated to cancellaton; -1 - no escalation, 0 - do not pause, cancel immediately")
flagResumeJobs = flagSet.Bool("resumeJobs", false, "Attempt to resume paused jobs")
flagTargetLockDuration = flagSet.Duration("targetLockDuration", config.DefaultTargetLockDuration,
Expand Down Expand Up @@ -153,6 +157,16 @@ func Main(pluginConfig *PluginConfig, cmd string, args []string, sigs <-chan os.
log := ctx.Logger()
defer cancel()

// set the log output to file
if *flagLogFile != "" {
f, err := os.OpenFile(*flagLogFile, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0600)
if err != nil {
return err
}
defer f.Close()
log.OriginalLogger().(*logrus.Entry).Logger.SetOutput(io.MultiWriter(os.Stderr, f))
Copy link
Member

Choose a reason for hiding this comment

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

@xaionaro does this make sense here? i dont know if we actually use logrus anymore

Copy link
Member

@xaionaro xaionaro May 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

i dont know if we actually use logrus anymore

At least the upstream version of contest uses logrus:
https://github.com/linuxboot/contest/blob/develop/pkg/logging/default_options.go#L25

does this make sense here?

It kinda does. But a cleaner solution would be to feed an option to https://github.com/linuxboot/contest/blob/develop/pkg/logging/default_options.go#L21-L24 to add a multiwriter there if needed (to avoid type assertions; or at least to keep them local to where we make sure it is logrus). If one wants to avoid a type assertion at all then they can manually initialize a logrus instance and wrap it using this function: https://github.com/facebookincubator/go-belt/blob/main/tool/logger/implementation/logrus/logger.go#L484

Or if people are lazy, they can just move this line into the function WithBelt (to keep the assertion local to where we build a logrus logger).

Copy link
Member

Choose a reason for hiding this comment

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

yeah, that's what I was thinking. that type assertion looks kinda off to me. Not sure what we can do here, because this PR comes from a 9elements repo so we can't fix anything on this branch without the original author

}

pluginRegistry := pluginregistry.NewPluginRegistry(ctx)
if err := registerPlugins(pluginRegistry, pluginConfig); err != nil {
return fmt.Errorf("failed to register plugins: %w", err)
Expand Down