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

Proposal: Better logging infrastructure #6038

Open
zeripath opened this issue Feb 11, 2019 · 0 comments

Comments

@zeripath
Copy link
Contributor

commented Feb 11, 2019

Description

Although our logging infrastructure is quite configurable and better than the original Go logging, it could be better.

There are three main issues at present:

  1. Any use of go "log.Printf" (rather than "code.gitea.io/modules/log") will log to stdout by default - In the case of Macaron this can be ameliorated by REDIRECT_MACARON_LOG - however, there are likely other places
  • A sub-issue is that log.Fatal will kill Gitea...
  1. The Router logs are not in the NCSA Common Log format, and are not configurable to be so. This means that we cannot leverage external tools to use this.
  2. It is not possible to set different log levels for different gitea modules. Thus setting the log level to TRACE will lead to massive amounts of difficult to follow logging.

Proposals

Redirect the standard logger output to "gitea.log"

Through the use of log.SetOutput, log.SetPrefix and log.SetFlags with appropriate options - we could then reparse the output and re-log ourselves as appropriate.

Configurable NCSA Common Log Format for the Router

This is fixable through the use of a different logger, say log.NewAccessLogger, which has no flags set and with a potentially configurable template.

defaultTemplate := "{{.Ctx.RemoteAddr}} - {{.Identity}} {{.Start.Format \"[02/Jan/2006:15:04:05 -0700]\" }} \"{{.Ctx.Req.Method}} {{.Ctx.Req.RequestURI}} {{.Ctx.Req.Proto}}\" {{.ResponseWriter.Status}} {{.ResponseWriter.Size}} \"{{.Ctx.Req.Referer}}\" \"{{.Ctx.Req.UserAgent}}\""

type ncsaLoggerOptions struct {
	Ctx            *macaron.Context
	Identity       *string
	Start          *time.Time
	ResponseWriter *macaron.ResponseWriter
}

func ncsaLogger() macaron.Handler {
	log.NewAccessLogger(filepath.Join(setting.LogRootPath, "access.log"))
	l := log.NewLoggerAsWriter("INFO", log.AccessLogger)
	logTemplate, _ := template.New("log").Parse(defaultTemplate)
	return func(ctx *macaron.Context) {
		start := time.Now()
		ctx.Next()
		identity := "-"
		if val, ok := ctx.Data["SignedUserName"]; ok {
			if stringVal, ok := val.(string); ok && stringVal != "" {
				identity = stringVal
			}
		}
		rw := ctx.Resp.(macaron.ResponseWriter)

		buf := bytes.NewBuffer([]byte{})
		logTemplate.Execute(buf, ncsaLoggerOptions{
			Ctx:            ctx,
			Identity:       &identity,
			Start:          &start,
			ResponseWriter: &rw,
		})
		l.Log(buf.String())
	}
}

TRACE/DEBUG etc. for specific modules/packages/files

Through judicious use of runtime.Caller(), runtime.FuncForPC() and Func.Name() it should be possible to get the function name and go package for callers to log functions. Instead of making the log channel simply pass a level and a string message it could be changed to also pass the function or filename name - allowing individual loggers to check if it should log these.

I'm uncertain as to the cost of touching runtime.Caller() etc., however, it is unlikely to be a major cost unless we're logging too much. We could add IsDebug etc to the loggers to allow us to know check whether we would do any Debug/Tracing for any file to avoid having to look up the caller stack at all.

Configuration

Of course, configuration of this will require some thought.

It should be clear that at present that the XORM logger and the REDIRECT_MACARON_LOG options different beasts to those of the other log options. If we were to do this we would need a general log section dealing with these sort of special case loggers and specific gitea logger sections. (I think this can already be done?)

  • We would need to add configuration for the templated router logger - passing in a template, setting the output file, and switching it on.
  • In each logger section we would need configuration to match the chosen modules etc.

Summary

The proposed changes substantially complicate logging however, provide much more power. It should be possible to make these changes in a backwards compatible way.

zeripath added a commit that referenced this issue Apr 2, 2019

Better logging (#6038) (#6095)
* Panic don't fatal on create new logger

Fixes #5854

Signed-off-by: Andrew Thornton <art27@cantab.net>

* partial broken

* Update the logging infrastrcture

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Reset the skip levels for Fatal and Error

Signed-off-by: Andrew Thornton <art27@cantab.net>

* broken ncsa

* More log.Error fixes

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Remove nal

* set log-levels to lowercase

* Make console_test test all levels

* switch to lowercased levels

* OK now working

* Fix vetting issues

* Fix lint

* Fix tests

* change default logging to match current gitea

* Improve log testing

Signed-off-by: Andrew Thornton <art27@cantab.net>

* reset error skip levels to 0

* Update documentation and access logger configuration

* Redirect the router log back to gitea if redirect macaron log but also allow setting the log level - i.e. TRACE

* Fix broken level caching

* Refactor the router log

* Add Router logger

* Add colorizing options

* Adjust router colors

* Only create logger if they will be used

* update app.ini.sample

* rename Attribute ColorAttribute

* Change from white to green for function

* Set fatal/error levels

* Restore initial trace logger

* Fix Trace arguments in modules/auth/auth.go

* Properly handle XORMLogger

* Improve admin/config page

* fix fmt

* Add auto-compression of old logs

* Update error log levels

* Remove the unnecessary skip argument from Error, Fatal and Critical

* Add stacktrace support

* Fix tests

* Remove x/sync from vendors?

* Add stderr option to console logger

* Use filepath.ToSlash to protect against Windows in tests

* Remove prefixed underscores from names in colors.go

* Remove not implemented database logger

This was removed from Gogs on 4 Mar 2016 but left in the configuration
since then.

* Ensure that log paths are relative to ROOT_PATH

* use path.Join

* rename jsonConfig to logConfig

* Rename "config" to "jsonConfig" to make it clearer

* Requested changes

* Requested changes: XormLogger

* Try to color the windows terminal

If successful default to colorizing the console logs

* fixup

* Colorize initially too

* update vendor

* Colorize logs on default and remove if this is not a colorizing logger

* Fix documentation

* fix test

* Use go-isatty to detect if on windows we are on msys or cygwin

* Fix spelling mistake

* Add missing vendors

* More changes

* Rationalise the ANSI writer protection

* Adjust colors on advice from @0x5c

* Make Flags a comma separated list

* Move to use the windows constant for ENABLE_VIRTUAL_TERMINAL_PROCESSING

* Ensure matching is done on the non-colored message - to simpify EXPRESSION
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.