Skip to content

Commit 4647660

Browse files
wxiaoguangwolfogredelvhGiteaBot
authored
Rewrite logger system (#24726)
## ⚠️ Breaking The `log.<mode>.<logger>` style config has been dropped. If you used it, please check the new config manual & app.example.ini to make your instance output logs as expected. Although many legacy options still work, it's encouraged to upgrade to the new options. The SMTP logger is deleted because SMTP is not suitable to collect logs. If you have manually configured Gitea log options, please confirm the logger system works as expected after upgrading. ## Description Close #12082 and maybe more log-related issues, resolve some related FIXMEs in old code (which seems unfixable before) Just like rewriting queue #24505 : make code maintainable, clear legacy bugs, and add the ability to support more writers (eg: JSON, structured log) There is a new document (with examples): `logging-config.en-us.md` This PR is safer than the queue rewriting, because it's just for logging, it won't break other logic. ## The old problems The logging system is quite old and difficult to maintain: * Unclear concepts: Logger, NamedLogger, MultiChannelledLogger, SubLogger, EventLogger, WriterLogger etc * Some code is diffuclt to konw whether it is right: `log.DelNamedLogger("console")` vs `log.DelNamedLogger(log.DEFAULT)` vs `log.DelLogger("console")` * The old system heavily depends on ini config system, it's difficult to create new logger for different purpose, and it's very fragile. * The "color" trick is difficult to use and read, many colors are unnecessary, and in the future structured log could help * It's difficult to add other log formats, eg: JSON format * The log outputer doesn't have full control of its goroutine, it's difficult to make outputer have advanced behaviors * The logs could be lost in some cases: eg: no Fatal error when using CLI. * Config options are passed by JSON, which is quite fragile. * INI package makes the KEY in `[log]` section visible in `[log.sub1]` and `[log.sub1.subA]`, this behavior is quite fragile and would cause more unclear problems, and there is no strong requirement to support `log.<mode>.<logger>` syntax. ## The new design See `logger.go` for documents. ## Screenshot <details> ![image](https://github.com/go-gitea/gitea/assets/2114189/4462d713-ba39-41f5-bb08-de912e67e1ff) ![image](https://github.com/go-gitea/gitea/assets/2114189/b188035e-f691-428b-8b2d-ff7b2199b2f9) ![image](https://github.com/go-gitea/gitea/assets/2114189/132e9745-1c3b-4e00-9e0d-15eaea495dee) </details> ## TODO * [x] add some new tests * [x] fix some tests * [x] test some sub-commands (manually ....) --------- Co-authored-by: Jason Song <i@wolfogre.com> Co-authored-by: delvh <dev.lh@web.de> Co-authored-by: Giteabot <teabot@gitea.io>
1 parent 65dff8e commit 4647660

File tree

109 files changed

+3772
-5303
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

109 files changed

+3772
-5303
lines changed

cmd/cmd.go

+16-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"context"
1010
"errors"
1111
"fmt"
12+
"io"
1213
"os"
1314
"os/signal"
1415
"strings"
@@ -59,7 +60,7 @@ func confirm() (bool, error) {
5960
func initDB(ctx context.Context) error {
6061
setting.Init(&setting.Options{})
6162
setting.LoadDBSetting()
62-
setting.InitSQLLog(false)
63+
setting.InitSQLLoggersForCli(log.INFO)
6364

6465
if setting.Database.Type == "" {
6566
log.Fatal(`Database settings are missing from the configuration file: %q.
@@ -93,3 +94,17 @@ func installSignals() (context.Context, context.CancelFunc) {
9394

9495
return ctx, cancel
9596
}
97+
98+
func setupConsoleLogger(level log.Level, colorize bool, out io.Writer) {
99+
if out != os.Stdout && out != os.Stderr {
100+
panic("setupConsoleLogger can only be used with os.Stdout or os.Stderr")
101+
}
102+
103+
writeMode := log.WriterMode{
104+
Level: level,
105+
Colorize: colorize,
106+
WriterOption: log.WriterConsoleOption{Stderr: out == os.Stderr},
107+
}
108+
writer := log.NewEventWriterConsole("console-default", writeMode)
109+
log.GetManager().GetLogger(log.DEFAULT).RemoveAllWriters().AddWriters(writer)
110+
}

cmd/doctor.go

+31-59
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44
package cmd
55

66
import (
7-
"errors"
87
"fmt"
98
golog "log"
109
"os"
10+
"path/filepath"
1111
"strings"
1212
"text/tabwriter"
1313

@@ -82,23 +82,25 @@ You should back-up your database before doing this and ensure that your database
8282
}
8383

8484
func runRecreateTable(ctx *cli.Context) error {
85+
stdCtx, cancel := installSignals()
86+
defer cancel()
87+
8588
// Redirect the default golog to here
8689
golog.SetFlags(0)
8790
golog.SetPrefix("")
88-
golog.SetOutput(log.NewLoggerAsWriter("INFO", log.GetLogger(log.DEFAULT)))
91+
golog.SetOutput(log.LoggerToWriter(log.GetLogger(log.DEFAULT).Info))
8992

93+
debug := ctx.Bool("debug")
9094
setting.Init(&setting.Options{})
9195
setting.LoadDBSetting()
9296

93-
setting.Log.EnableXORMLog = ctx.Bool("debug")
94-
setting.Database.LogSQL = ctx.Bool("debug")
95-
// FIXME: don't use CfgProvider directly
96-
setting.CfgProvider.Section("log").Key("XORM").SetValue(",")
97-
98-
setting.InitSQLLog(!ctx.Bool("debug"))
99-
stdCtx, cancel := installSignals()
100-
defer cancel()
97+
if debug {
98+
setting.InitSQLLoggersForCli(log.DEBUG)
99+
} else {
100+
setting.InitSQLLoggersForCli(log.INFO)
101+
}
101102

103+
setting.Database.LogSQL = debug
102104
if err := db.InitEngine(stdCtx); err != nil {
103105
fmt.Println(err)
104106
fmt.Println("Check if you are using the right config file. You can use a --config directive to specify one.")
@@ -125,67 +127,49 @@ func runRecreateTable(ctx *cli.Context) error {
125127
})
126128
}
127129

128-
func setDoctorLogger(ctx *cli.Context) {
130+
func setupDoctorDefaultLogger(ctx *cli.Context, colorize bool) {
131+
// Silence the default loggers
132+
setupConsoleLogger(log.FATAL, log.CanColorStderr, os.Stderr)
133+
129134
logFile := ctx.String("log-file")
130135
if !ctx.IsSet("log-file") {
131136
logFile = "doctor.log"
132137
}
133-
colorize := log.CanColorStdout
134-
if ctx.IsSet("color") {
135-
colorize = ctx.Bool("color")
136-
}
137138

138139
if len(logFile) == 0 {
139-
log.NewLogger(1000, "doctor", "console", fmt.Sprintf(`{"level":"NONE","stacktracelevel":"NONE","colorize":%t}`, colorize))
140+
// if no doctor log-file is set, do not show any log from default logger
140141
return
141142
}
142143

143-
defer func() {
144-
recovered := recover()
145-
if recovered == nil {
146-
return
147-
}
148-
149-
err, ok := recovered.(error)
150-
if !ok {
151-
panic(recovered)
152-
}
153-
if errors.Is(err, os.ErrPermission) {
154-
fmt.Fprintf(os.Stderr, "ERROR: Unable to write logs to provided file due to permissions error: %s\n %v\n", logFile, err)
155-
} else {
156-
fmt.Fprintf(os.Stderr, "ERROR: Unable to write logs to provided file: %s\n %v\n", logFile, err)
157-
}
158-
fmt.Fprintf(os.Stderr, "WARN: Logging will be disabled\n Use `--log-file` to configure log file location\n")
159-
log.NewLogger(1000, "doctor", "console", fmt.Sprintf(`{"level":"NONE","stacktracelevel":"NONE","colorize":%t}`, colorize))
160-
}()
161-
162144
if logFile == "-" {
163-
log.NewLogger(1000, "doctor", "console", fmt.Sprintf(`{"level":"trace","stacktracelevel":"NONE","colorize":%t}`, colorize))
145+
setupConsoleLogger(log.TRACE, colorize, os.Stdout)
164146
} else {
165-
log.NewLogger(1000, "doctor", "file", fmt.Sprintf(`{"filename":%q,"level":"trace","stacktracelevel":"NONE"}`, logFile))
147+
logFile, _ = filepath.Abs(logFile)
148+
writeMode := log.WriterMode{Level: log.TRACE, WriterOption: log.WriterFileOption{FileName: logFile}}
149+
writer, err := log.NewEventWriter("console-to-file", "file", writeMode)
150+
if err != nil {
151+
log.FallbackErrorf("unable to create file log writer: %v", err)
152+
return
153+
}
154+
log.GetManager().GetLogger(log.DEFAULT).RemoveAllWriters().AddWriters(writer)
166155
}
167156
}
168157

169158
func runDoctor(ctx *cli.Context) error {
170159
stdCtx, cancel := installSignals()
171160
defer cancel()
172161

173-
// Silence the default loggers
174-
log.DelNamedLogger("console")
175-
log.DelNamedLogger(log.DEFAULT)
176-
177-
// Now setup our own
178-
setDoctorLogger(ctx)
179-
180162
colorize := log.CanColorStdout
181163
if ctx.IsSet("color") {
182164
colorize = ctx.Bool("color")
183165
}
184166

185-
// Finally redirect the default golog to here
167+
setupDoctorDefaultLogger(ctx, colorize)
168+
169+
// Finally redirect the default golang's log to here
186170
golog.SetFlags(0)
187171
golog.SetPrefix("")
188-
golog.SetOutput(log.NewLoggerAsWriter("INFO", log.GetLogger(log.DEFAULT)))
172+
golog.SetOutput(log.LoggerToWriter(log.GetLogger(log.DEFAULT).Info))
189173

190174
if ctx.IsSet("list") {
191175
w := tabwriter.NewWriter(os.Stdout, 0, 8, 1, '\t', 0)
@@ -233,17 +217,5 @@ func runDoctor(ctx *cli.Context) error {
233217
}
234218
}
235219

236-
// Now we can set up our own logger to return information about what the doctor is doing
237-
if err := log.NewNamedLogger("doctorouter",
238-
0,
239-
"console",
240-
"console",
241-
fmt.Sprintf(`{"level":"INFO","stacktracelevel":"NONE","colorize":%t,"flags":-1}`, colorize)); err != nil {
242-
fmt.Println(err)
243-
return err
244-
}
245-
246-
logger := log.GetLogger("doctorouter")
247-
defer logger.Close()
248-
return doctor.RunChecks(stdCtx, logger, ctx.Bool("fix"), checks)
220+
return doctor.RunChecks(stdCtx, colorize, ctx.Bool("fix"), checks)
249221
}

cmd/dump.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -172,10 +172,7 @@ func runDump(ctx *cli.Context) error {
172172
outType := ctx.String("type")
173173
if fileName == "-" {
174174
file = os.Stdout
175-
err := log.DelLogger("console")
176-
if err != nil {
177-
fatal("Deleting default logger failed. Can not write to stdout: %v", err)
178-
}
175+
setupConsoleLogger(log.FATAL, log.CanColorStderr, os.Stderr)
179176
} else {
180177
for _, suffix := range outputTypeEnum.Enum {
181178
if strings.HasSuffix(fileName, "."+suffix) {

cmd/embedded.go

+1-7
Original file line numberDiff line numberDiff line change
@@ -97,13 +97,7 @@ type assetFile struct {
9797
}
9898

9999
func initEmbeddedExtractor(c *cli.Context) error {
100-
// FIXME: there is a bug, if the user runs `gitea embedded` with a different user or root,
101-
// The setting.Init (loadRunModeFrom) will fail and do log.Fatal
102-
// But the console logger has been deleted, so nothing is printed, the user sees nothing and Gitea just exits.
103-
104-
// Silence the console logger
105-
log.DelNamedLogger("console")
106-
log.DelNamedLogger(log.DEFAULT)
100+
setupConsoleLogger(log.ERROR, log.CanColorStderr, os.Stderr)
107101

108102
// Read configuration file
109103
setting.Init(&setting.Options{

cmd/manager_logging.go

+21-99
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@ import (
1616
var (
1717
defaultLoggingFlags = []cli.Flag{
1818
cli.StringFlag{
19-
Name: "group, g",
20-
Usage: "Group to add logger to - will default to \"default\"",
19+
Name: "logger",
20+
Usage: `Logger name - will default to "default"`,
2121
}, cli.StringFlag{
22-
Name: "name, n",
23-
Usage: "Name of the new logger - will default to mode",
22+
Name: "writer",
23+
Usage: "Name of the log writer - will default to mode",
2424
}, cli.StringFlag{
25-
Name: "level, l",
25+
Name: "level",
2626
Usage: "Logging level for the new logger",
2727
}, cli.StringFlag{
2828
Name: "stacktrace-level, L",
@@ -83,8 +83,8 @@ var (
8383
cli.BoolFlag{
8484
Name: "debug",
8585
}, cli.StringFlag{
86-
Name: "group, g",
87-
Usage: "Group to add logger to - will default to \"default\"",
86+
Name: "logger",
87+
Usage: `Logger name - will default to "default"`,
8888
},
8989
},
9090
Action: runRemoveLogger,
@@ -93,15 +93,6 @@ var (
9393
Usage: "Add a logger",
9494
Subcommands: []cli.Command{
9595
{
96-
Name: "console",
97-
Usage: "Add a console logger",
98-
Flags: append(defaultLoggingFlags,
99-
cli.BoolFlag{
100-
Name: "stderr",
101-
Usage: "Output console logs to stderr - only relevant for console",
102-
}),
103-
Action: runAddConsoleLogger,
104-
}, {
10596
Name: "file",
10697
Usage: "Add a file logger",
10798
Flags: append(defaultLoggingFlags, []cli.Flag{
@@ -148,28 +139,6 @@ var (
148139
},
149140
}...),
150141
Action: runAddConnLogger,
151-
}, {
152-
Name: "smtp",
153-
Usage: "Add an SMTP logger",
154-
Flags: append(defaultLoggingFlags, []cli.Flag{
155-
cli.StringFlag{
156-
Name: "username, u",
157-
Usage: "Mail server username",
158-
}, cli.StringFlag{
159-
Name: "password, P",
160-
Usage: "Mail server password",
161-
}, cli.StringFlag{
162-
Name: "host, H",
163-
Usage: "Mail server host (defaults to: 127.0.0.1:25)",
164-
}, cli.StringSliceFlag{
165-
Name: "send-to, s",
166-
Usage: "Email address(es) to send to",
167-
}, cli.StringFlag{
168-
Name: "subject, S",
169-
Usage: "Subject header of sent emails",
170-
},
171-
}...),
172-
Action: runAddSMTPLogger,
173142
},
174143
},
175144
}, {
@@ -194,50 +163,16 @@ func runRemoveLogger(c *cli.Context) error {
194163
defer cancel()
195164

196165
setup(ctx, c.Bool("debug"))
197-
group := c.String("group")
198-
if len(group) == 0 {
199-
group = log.DEFAULT
166+
logger := c.String("logger")
167+
if len(logger) == 0 {
168+
logger = log.DEFAULT
200169
}
201-
name := c.Args().First()
170+
writer := c.Args().First()
202171

203-
extra := private.RemoveLogger(ctx, group, name)
172+
extra := private.RemoveLogger(ctx, logger, writer)
204173
return handleCliResponseExtra(extra)
205174
}
206175

207-
func runAddSMTPLogger(c *cli.Context) error {
208-
ctx, cancel := installSignals()
209-
defer cancel()
210-
211-
setup(ctx, c.Bool("debug"))
212-
vals := map[string]interface{}{}
213-
mode := "smtp"
214-
if c.IsSet("host") {
215-
vals["host"] = c.String("host")
216-
} else {
217-
vals["host"] = "127.0.0.1:25"
218-
}
219-
220-
if c.IsSet("username") {
221-
vals["username"] = c.String("username")
222-
}
223-
if c.IsSet("password") {
224-
vals["password"] = c.String("password")
225-
}
226-
227-
if !c.IsSet("send-to") {
228-
return fmt.Errorf("Some recipients must be provided")
229-
}
230-
vals["sendTos"] = c.StringSlice("send-to")
231-
232-
if c.IsSet("subject") {
233-
vals["subject"] = c.String("subject")
234-
} else {
235-
vals["subject"] = "Diagnostic message from Gitea"
236-
}
237-
238-
return commonAddLogger(c, mode, vals)
239-
}
240-
241176
func runAddConnLogger(c *cli.Context) error {
242177
ctx, cancel := installSignals()
243178
defer cancel()
@@ -301,25 +236,12 @@ func runAddFileLogger(c *cli.Context) error {
301236
return commonAddLogger(c, mode, vals)
302237
}
303238

304-
func runAddConsoleLogger(c *cli.Context) error {
305-
ctx, cancel := installSignals()
306-
defer cancel()
307-
308-
setup(ctx, c.Bool("debug"))
309-
vals := map[string]interface{}{}
310-
mode := "console"
311-
if c.IsSet("stderr") && c.Bool("stderr") {
312-
vals["stderr"] = c.Bool("stderr")
313-
}
314-
return commonAddLogger(c, mode, vals)
315-
}
316-
317239
func commonAddLogger(c *cli.Context, mode string, vals map[string]interface{}) error {
318240
if len(c.String("level")) > 0 {
319-
vals["level"] = log.FromString(c.String("level")).String()
241+
vals["level"] = log.LevelFromString(c.String("level")).String()
320242
}
321243
if len(c.String("stacktrace-level")) > 0 {
322-
vals["stacktraceLevel"] = log.FromString(c.String("stacktrace-level")).String()
244+
vals["stacktraceLevel"] = log.LevelFromString(c.String("stacktrace-level")).String()
323245
}
324246
if len(c.String("expression")) > 0 {
325247
vals["expression"] = c.String("expression")
@@ -333,18 +255,18 @@ func commonAddLogger(c *cli.Context, mode string, vals map[string]interface{}) e
333255
if c.IsSet("color") {
334256
vals["colorize"] = c.Bool("color")
335257
}
336-
group := "default"
337-
if c.IsSet("group") {
338-
group = c.String("group")
258+
logger := log.DEFAULT
259+
if c.IsSet("logger") {
260+
logger = c.String("logger")
339261
}
340-
name := mode
341-
if c.IsSet("name") {
342-
name = c.String("name")
262+
writer := mode
263+
if c.IsSet("writer") {
264+
writer = c.String("writer")
343265
}
344266
ctx, cancel := installSignals()
345267
defer cancel()
346268

347-
extra := private.AddLogger(ctx, group, name, mode, vals)
269+
extra := private.AddLogger(ctx, logger, writer, mode, vals)
348270
return handleCliResponseExtra(extra)
349271
}
350272

0 commit comments

Comments
 (0)