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.

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

Fix sub-command log level #25537

Merged
merged 2 commits into from
Jun 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 17 additions & 1 deletion cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,5 +106,21 @@ func setupConsoleLogger(level log.Level, colorize bool, out io.Writer) {
WriterOption: log.WriterConsoleOption{Stderr: out == os.Stderr},
}
writer := log.NewEventWriterConsole("console-default", writeMode)
log.GetManager().GetLogger(log.DEFAULT).RemoveAllWriters().AddWriters(writer)
log.GetManager().GetLogger(log.DEFAULT).ReplaceAllWriters(writer)
}

// PrepareConsoleLoggerLevel by default, use INFO level for console logger, but some sub-commands (for git/ssh protocol) shouldn't output any log to stdout.
// Any log appears in git stdout pipe will break the git protocol, eg: client can't push and hangs forever.
func PrepareConsoleLoggerLevel(defaultLevel log.Level) func(*cli.Context) error {
return func(c *cli.Context) error {
level := defaultLevel
if c.Bool("quiet") || c.GlobalBoolT("quiet") {
level = log.FATAL
}
if c.Bool("debug") || c.GlobalBool("debug") || c.Bool("verbose") || c.GlobalBool("verbose") {
level = log.TRACE
}
log.SetConsoleLogger(log.DEFAULT, "console-default", level)
return nil
}
}
2 changes: 1 addition & 1 deletion cmd/doctor.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func setupDoctorDefaultLogger(ctx *cli.Context, colorize bool) {
log.FallbackErrorf("unable to create file log writer: %v", err)
return
}
log.GetManager().GetLogger(log.DEFAULT).RemoveAllWriters().AddWriters(writer)
log.GetManager().GetLogger(log.DEFAULT).ReplaceAllWriters(writer)
}
}

Expand Down
2 changes: 2 additions & 0 deletions cmd/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"time"

"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/private"
repo_module "code.gitea.io/gitea/modules/repository"
"code.gitea.io/gitea/modules/setting"
Expand All @@ -32,6 +33,7 @@ var (
Name: "hook",
Usage: "Delegate commands to corresponding Git hooks",
Description: "This should only be called by Git",
Before: PrepareConsoleLoggerLevel(log.FATAL),
Subcommands: []cli.Command{
subcmdHookPreReceive,
subcmdHookUpdate,
Expand Down
2 changes: 2 additions & 0 deletions cmd/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"strings"

"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/private"

"github.com/urfave/cli"
Expand All @@ -17,6 +18,7 @@ import (
var CmdKeys = cli.Command{
Name: "keys",
Usage: "This command queries the Gitea database to get the authorized command for a given ssh key fingerprint",
Before: PrepareConsoleLoggerLevel(log.FATAL),
Action: runKeys,
Flags: []cli.Flag{
cli.StringFlag{
Expand Down
1 change: 1 addition & 0 deletions cmd/serv.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ var CmdServ = cli.Command{
Name: "serv",
Usage: "This command should only be called by SSH shell",
Description: "Serv provides access auth for repositories",
Before: PrepareConsoleLoggerLevel(log.FATAL),
Action: runServ,
Flags: []cli.Flag{
cli.BoolFlag{
Expand Down
6 changes: 1 addition & 5 deletions cmd/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ var CmdWeb = cli.Command{
Usage: "Start Gitea web server",
Description: `Gitea web server is the only thing you need to run,
and it takes care of all the other things for you`,
Before: PrepareConsoleLoggerLevel(log.INFO),
Action: runWeb,
Flags: []cli.Flag{
cli.StringFlag{
Expand Down Expand Up @@ -206,11 +207,6 @@ func servePprof() {
}

func runWeb(ctx *cli.Context) error {
if ctx.Bool("verbose") {
setupConsoleLogger(log.TRACE, log.CanColorStdout, os.Stdout)
} else if ctx.Bool("quiet") {
setupConsoleLogger(log.FATAL, log.CanColorStdout, os.Stdout)
}
defer func() {
if panicked := recover(); panicked != nil {
log.Fatal("PANIC: %v\n%s", panicked, log.Stack(2))
Expand Down
7 changes: 5 additions & 2 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,9 @@ func main() {
cmd.CmdActions,
}

// default configuration flags
// shared configuration flags, they are for global and for each sub-command at the same time
// eg: such command is valid: "./gitea --config /tmp/app.ini web --config /tmp/app.ini", while it's discouraged indeed
// keep in mind that the short flags like "-C", "-c" and "-w" are globally polluted, they can't be used for sub-commands anymore.
globalFlags := []cli.Flag{
cli.HelpFlag,
cli.StringFlag{
Expand All @@ -128,9 +130,10 @@ func main() {

// Set the default to be equivalent to cmdWeb and add the default flags
app.Flags = append(app.Flags, globalFlags...)
app.Flags = append(app.Flags, cmd.CmdWeb.Flags...)
app.Flags = append(app.Flags, cmd.CmdWeb.Flags...) // TODO: the web flags polluted the global flags, they are not really global flags
app.Action = prepareWorkPathAndCustomConf(cmd.CmdWeb.Action)
app.HideHelp = true // use our own help action to show helps (with more information like default config)
app.Before = cmd.PrepareConsoleLoggerLevel(log.INFO)
app.Commands = append(app.Commands, cmdHelp)
for i := range app.Commands {
prepareSubcommands(&app.Commands[i], globalFlags)
Expand Down
2 changes: 1 addition & 1 deletion modules/log/logger_global.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,5 +79,5 @@ func SetConsoleLogger(loggerName, writerName string, level Level) {
Colorize: CanColorStdout,
WriterOption: WriterConsoleOption{},
})
GetManager().GetLogger(loggerName).RemoveAllWriters().AddWriters(writer)
GetManager().GetLogger(loggerName).ReplaceAllWriters(writer)
}
13 changes: 7 additions & 6 deletions modules/log/logger_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,10 @@ func (l *LoggerImpl) removeWriterInternal(w EventWriter) {
func (l *LoggerImpl) AddWriters(writer ...EventWriter) {
l.eventWriterMu.Lock()
defer l.eventWriterMu.Unlock()
l.addWritersInternal(writer...)
}

func (l *LoggerImpl) addWritersInternal(writer ...EventWriter) {
for _, w := range writer {
if old, ok := l.eventWriters[w.GetWriterName()]; ok {
l.removeWriterInternal(old)
Expand Down Expand Up @@ -126,17 +129,16 @@ func (l *LoggerImpl) RemoveWriter(modeName string) error {
return nil
}

// RemoveAllWriters removes all writers from the logger, non-shared writers are closed and flushed
func (l *LoggerImpl) RemoveAllWriters() *LoggerImpl {
// ReplaceAllWriters replaces all writers from the logger, non-shared writers are closed and flushed
func (l *LoggerImpl) ReplaceAllWriters(writer ...EventWriter) {
l.eventWriterMu.Lock()
defer l.eventWriterMu.Unlock()

for _, w := range l.eventWriters {
l.removeWriterInternal(w)
}
l.eventWriters = map[string]EventWriter{}
l.syncLevelInternal()
return l
l.addWritersInternal(writer...)
}

// DumpWriters dumps the writers as a JSON map, it's used for debugging and display purposes.
Expand All @@ -161,7 +163,7 @@ func (l *LoggerImpl) DumpWriters() map[string]any {

// Close closes the logger, non-shared writers are closed and flushed
func (l *LoggerImpl) Close() {
l.RemoveAllWriters()
l.ReplaceAllWriters()
l.ctxCancel()
}

Expand Down Expand Up @@ -233,7 +235,6 @@ func NewLoggerWithWriters(ctx context.Context, name string, writer ...EventWrite
l.ctx, l.ctxCancel = newProcessTypedContext(ctx, "Logger: "+name)
l.LevelLogger = BaseLoggerToGeneralLogger(l)
l.eventWriters = map[string]EventWriter{}
l.syncLevelInternal()
l.AddWriters(writer...)
return l
}
2 changes: 1 addition & 1 deletion modules/log/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func TestSharedWorker(t *testing.T) {
loggerTest := m.GetLogger("test")
loggerTest.AddWriters(w)
loggerTest.Info("msg-1")
loggerTest.RemoveAllWriters() // the shared writer is not closed here
loggerTest.ReplaceAllWriters() // the shared writer is not closed here
loggerTest.Info("never seen")

// the shared writer can still be used later
Expand Down
2 changes: 1 addition & 1 deletion modules/setting/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ func initLoggerByName(manager *log.LoggerManager, rootCfg ConfigProvider, logger
eventWriters = append(eventWriters, eventWriter)
}

manager.GetLogger(loggerName).RemoveAllWriters().AddWriters(eventWriters...)
manager.GetLogger(loggerName).ReplaceAllWriters(eventWriters...)
}

func InitSQLLoggersForCli(level log.Level) {
Expand Down