Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
26 changes: 20 additions & 6 deletions custom/conf/app.example.ini
Original file line number Diff line number Diff line change
Expand Up @@ -466,14 +466,28 @@ LEVEL = Info
;; Set the log "modes" for the router log (if file is set the log file will default to router.log)
ROUTER = console
;;
;; The log level that the router should log at. (If you are setting the access log, it's recommended to place this at Debug.)
;ROUTER_LOG_LEVEL = Info
;; This router will log different things at different levels.
;;
;; * started messages will be logged at DEBUG level
;; * slow routers will be logged at WARN
;; * polling/completed routers will be logged at INFO
;; * failed routers will be logged at WARN
;;
;; The routing level will default to that of the system but individual router level can be set in
;; [log.<mode>.router] LEVEL
;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Router V1 Logger
;;
;; Enable the legacy router
;ENABLE_ROUTER_V1 = false
;;
;; The handler for router logs, it controls the router log format, mainly for debug purpose, two log handlers:
;; router_logger_v1: default for before 1.16
;; router_logger_v2: default for 1.16 and later. Developers could set both `LEVEL=Debug` to get the "started" router messages
;ROUTER_LOG_HANDLER = router_logger_v2
;;Set the log "modes" for the router log (if file is set the log file will default to router.log)
;ROUTER_V1 = console
;;
;; The log level that the router should log at. (If you are setting the access log, it's recommended to place this at Debug.)
;ROUTER_V1_LOG_LEVEL = Info
Copy link
Owner

@wxiaoguang wxiaoguang Dec 26, 2021

Choose a reason for hiding this comment

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

I can not understand why should we introduce so many setting options to keep the router_v1.

What's the Pros/Cons of using these options instead of ROUTER_LOG_LEVEL + ROUTER_LOG_HANDLER ? Is there any real case I can learn from?

And in my mind we might want to deprecate (remove) router v1 in future, but these options (code/document) make it more complex.

Copy link
Author

Choose a reason for hiding this comment

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

I think it's clearer to separate out the old logger instead of overloading the meaning of the configuration parameters.

For example, your PR proposed changing the meaning of ROUTER_LOG_LEVEL to do something very different from what it does on the original router.

Then we simply deprecate these and remove them in 1.17 leaving in code that checks for the existence of the configuration - which causes a warning that this has been removed and finally in 1.18 remove even the check.

If we use the ROUTER_LOG_HANDLER=v2 then we're gonna end up having to keep checking that version and there's no way for a user to have both routers running for a while so they can migrate more easily.

Copy link
Owner

@wxiaoguang wxiaoguang Jan 2, 2022

Choose a reason for hiding this comment

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

IMO, we should unify and simplify the logger system, instead of introducing new specialized settings to make it more complex. Maybe we can narrow down the changes to:

  1. Keep router v1 in 1.16
  2. Deprecate v1 and ROUTER_LOG_LEVEL (the remove them in 1.17 or 1.18)
  3. Only introduce one setting option ROUTER_LOG_HANDLER
  4. In v2, we do not use ROUTER_LOG_LEVEL anymore, instead, we only show logs at Debug/Info/Warning levels.

I can not imagine a real use case for users to use v1 and v2 and the same time.

Copy link
Owner

Choose a reason for hiding this comment

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

@zeripath bump ~~

Copy link
Author

Choose a reason for hiding this comment

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

Adding suffices of V1 to the configuration of the ROUTER_V1 is the right thing to do here. It means that if you want the old router behaviour - or need it whilst you're migrating you configure it as ROUTER_V1 and ROUTER_V1_LOG_LEVEL.

  • In version 1 almost all users of the ROUTER logger will not have set ROUTER_LOG_LEVEL so the change of name won't negatively affect them.
  • In version 2 there is no use of ROUTER_LOG_LEVEL so we can easily deprecate it and ROUTER_V1_*
  • We immediately deprecate ROUTER_V1_* ROUTER_V1_LOG_LEVEL

I don't see any benefit to ROUTER_LOG_HANDLER.

I can easily imagine a use case for v1 and v2 - temporarily - I wouldn't have written this code if I couldn't see it.

Copy link
Owner

@wxiaoguang wxiaoguang Jan 10, 2022

Choose a reason for hiding this comment

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

I don't see any benefit to ROUTER_LOG_HANDLER.

ROUTER_LOG_HANDLER is much easier and simpler than introducing many new setting options like ENABLE_ROUTER_V1 / ROUTER_V1 / ROUTER_V1_LOG_LEVEL.

I can easily imagine a use case for v1 and v2 - temporarily - I wouldn't have written this code if I couldn't see it.

To be honest, I still can not get the point why v1 and v2 should co-exist.

Even if the v1 and v2 co-exist temporarily, that should be for development and refactoring purpose. After this PR gets merged (and we finish the refactoring), are there still users who need to see v1 and v2 logs at the same time? What could they do after reading v1 and v2 logs? If they want to see the differences, they can simply first use v1 to collect some logs and then use v2 to collect logs. If they decide to stay with v1, just set ROUTER_LOG_HANDLER=v1, if they decide to migrate to v2, just set ROUTER_LOG_HANDLER=v2, no need to make v1 and v2 co-exist.


;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;
;; Access Logger (Creates log in NCSA common log format)
Expand Down
7 changes: 5 additions & 2 deletions docs/content/doc/advanced/config-cheat-sheet.en-us.md
Original file line number Diff line number Diff line change
Expand Up @@ -757,8 +757,11 @@ Default templates for project boards:
- `DISABLE_ROUTER_LOG`: **false**: Mute printing of the router log.
- `ROUTER`: **console**: The mode or name of the log the router should log to. (If you set this to `,` it will log to default Gitea logger.)
NB: You must have `DISABLE_ROUTER_LOG` set to `false` for this option to take effect. Configure each mode in per mode log subsections `\[log.modename.router\]`.
- `ROUTER_LOG_LEVEL`: **Info**: The log level that the router should log at. (If you are setting the access log, it's recommended to place this at Debug.)
- `ROUTER_LOG_HANDLER`: **router_logger_v2**: The log handler that controls the log output format. Before 1.16 the router logs are outputted by handler `router_logger_v1`. From 1.16, the default handler is `router_logger_v2` which is more meaningful and friendly.
### Router V1 Log (`log`)
- `ENABLE_ROUTER_V1`: **false**: Mute printing of the router log.
- `ROUTER_V1`: **console**: The mode or name of the log the router should log to. (If you set this to `,` it will log to default Gitea logger.)
NB: You must have `ENABLE_ROUTER_V1` set to `true` for this option to take effect. Configure each mode in per mode log subsections `\[log.modename.router_v1\]`.
- `ROUTER_V1_LOG_LEVEL`: **Info**: The log level that the router should log at. (If you are setting the access log, it's recommended to place this at Debug.)

### Access Log (`log`)
- `ENABLE_ACCESS_LOG`: **false**: Creates an access.log in NCSA common log format, or as per the following template
Expand Down
52 changes: 41 additions & 11 deletions docs/content/doc/advanced/logging-documentation.en-us.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,23 +68,25 @@ multiple subloggers that will log to files.

### The "Router" logger

You can disable Router log by setting `DISABLE_ROUTER_LOG`.
The Router logger has been substantially changed in v1.16. The old router logger is still available as `router_v1`
(see below) but it is deprecated and will be removed at some point.

You can disable Router log by setting `DISABLE_ROUTER_LOG` or setting all of its sublogger configurations to `none`.

You can configure the outputs of this
router log by setting the `ROUTER` value in the `[log]` section of the
configuration. `ROUTER` will default to `console` if unset. The Gitea
Router logs at the `Info` level by default, but this can be
changed if desired by setting the `ROUTER_LOG_LEVEL` value.
configuration. `ROUTER` will default to `console` if unset and will default to same level as main logger.

Please note, setting the `LEVEL` of this logger to a level above
`ROUTER_LOG_LEVEL` will result in no router logs.
The router logger will log various things at different levels:

You can control the output format by setting a log handler to `ROUTER_LOG_HANDLER`.
Now Gitea has two log handlers:
* `router_logger_v1` is the default handler for Gitea before 1.16
* `router_logger_v2` is the default handler for Gitea from 1.16, it's more meaningful and friendly.
- `started` messages will be logged at DEBUG level
- `slow` routers will be logged at WARN
- `polling`/`completed` routers will be logged at INFO
- `failed` routers will be logged at WARN

If you have applications depending on the log format (eg: fail2ban), please make sure you use the correct log handler and log format.
The logging level for the router will default to that of the main configuration. Set `[log.<mode>.router]` `LEVEL` to change this.

If you have applications depending on the log format (eg: fail2ban), please update your configuration or use the old logger.

Each output sublogger for this logger is configured in
`[log.sublogger.router]` sections. There are certain default values
Expand All @@ -99,6 +101,34 @@ which will not be inherited from the `[log]` or relevant
NB: You can redirect the router logger to send its events to the Gitea
log using the value: `ROUTER = ,`

### The "Router_V1" logger

The original router logger is still available by setting `ENABLE_ROUTER_V1`.

You can configure the outputs of this
router log by setting the `ROUTER_V1` value in the `[log]` section of the
configuration. `ROUTER_V1` will default to `console` if unset. The Gitea
Router logs at the `Info` level by default, but this can be
changed if desired by setting the `ROUTER_V1_LOG_LEVEL` value.

Please note, setting the `LEVEL` of this logger to a level above
`ROUTER_V1_LOG_LEVEL` will result in no router logs.

If you have applications depending on the log format (eg: fail2ban), please make sure you use the correct log handler and log format.

Each output sublogger for this logger is configured in
`[log.sublogger.router_v1]` sections. There are certain default values
which will not be inherited from the `[log]` or relevant
`[log.sublogger]` sections:

- `FILE_NAME` will default to `%(ROOT_PATH)/router.log`
- `FLAGS` defaults to `date,time`
- `EXPRESSION` will default to `""`
- `PREFIX` will default to `""`

NB: You can redirect the router logger to send its events to the Gitea
log using the value: `ROUTER_V1 = ,` (but remember to enable it first using `ENABLE_ROUTER_V1`.)

### The "Access" logger

The Access logger is a new logger for version 1.9. It provides a NCSA
Expand Down
12 changes: 12 additions & 0 deletions modules/setting/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,18 @@ func newRouterLogService() {
options.bufferLength = Cfg.Section("log").Key("BUFFER_LEN").MustInt64(10000)
generateNamedLogger("router", options)
}

Cfg.Section("log").Key("ROUTER_V1").MustString("console")

EnableRouterV1 = Cfg.Section("log").Key("ENABLE_ROUTER_V1").MustBool(false)
if EnableRouterV1 {
options := newDefaultLogOptions()
options.filename = filepath.Join(LogRootPath, "router.log")
options.flags = "date,time" // For the router we don't want any prefixed flags
options.bufferLength = Cfg.Section("log").Key("BUFFER_LEN").MustInt64(10000)
generateNamedLogger("router_v1", options)
}

}

func newLogService() {
Expand Down
7 changes: 3 additions & 4 deletions modules/setting/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -338,8 +338,8 @@ var (
EnableXORMLog bool

DisableRouterLog bool
RouterLogLevel log.Level
RouterLogHandler string
EnableRouterV1 bool
RouterV1LogLevel log.Level

EnableAccessLog bool
AccessLogTemplate string
Expand Down Expand Up @@ -606,8 +606,7 @@ func loadFromConf(allowEmpty bool, extraConfig string) {
StacktraceLogLevel = getStacktraceLogLevel(Cfg.Section("log"), "STACKTRACE_LEVEL", "None")
LogRootPath = Cfg.Section("log").Key("ROOT_PATH").MustString(path.Join(AppWorkPath, "log"))
forcePathSeparator(LogRootPath)
RouterLogLevel = log.FromString(Cfg.Section("log").Key("ROUTER_LOG_LEVEL").MustString("Info"))
RouterLogHandler = Cfg.Section("log").Key("ROUTER_LOG_HANDLER").MustString("router_logger_v2")
RouterV1LogLevel = log.FromString(Cfg.Section("log").Key("ROUTER_V1_LOG_LEVEL").MustString("Info"))

sec := Cfg.Section("server")
AppName = Cfg.Section("").Key("APP_NAME").MustString("Gitea: Git with a cup of tea")
Expand Down
4 changes: 2 additions & 2 deletions modules/web/routing/logger_v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func NewLoggerHandlerV1(level log.Level) func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
start := time.Now()

_ = log.GetLogger("router").Log(0, level, "Started %s %s for %s", log.ColoredMethod(req.Method), req.URL.RequestURI(), req.RemoteAddr)
_ = log.GetLogger("router_v1").Log(0, level, "Started %s %s for %s", log.ColoredMethod(req.Method), req.URL.RequestURI(), req.RemoteAddr)

next.ServeHTTP(w, req)

Expand All @@ -27,7 +27,7 @@ func NewLoggerHandlerV1(level log.Level) func(next http.Handler) http.Handler {
status = v.Status()
}

_ = log.GetLogger("router").Log(0, level, "Completed %s %s %v %s in %v", log.ColoredMethod(req.Method), req.URL.RequestURI(), log.ColoredStatus(status), log.ColoredStatus(status, http.StatusText(status)), log.ColoredTime(time.Since(start)))
_ = log.GetLogger("router_v1").Log(0, level, "Completed %s %s %v %s in %v", log.ColoredMethod(req.Method), req.URL.RequestURI(), log.ColoredStatus(status), log.ColoredStatus(status, http.StatusText(status)), log.ColoredTime(time.Since(start)))
})
}
}
26 changes: 17 additions & 9 deletions modules/web/routing/logger_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (

"code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
)

// NewLoggerHandlerV2 is a handler that will log routing to the router log taking account of
Expand All @@ -29,15 +28,16 @@ func NewLoggerHandlerV2() func(next http.Handler) http.Handler {

func logPrinter(logger log.Logger) func(trigger Event, record *requestRecord) {
return func(trigger Event, record *requestRecord) {
if trigger == StartEvent && setting.LogLevel > log.DEBUG {
if trigger == StartEvent && !logger.IsDebug() {
// for performance, if the "started" message shouldn't be logged, we just return as early as possible
// developers could set both `log.LEVEL=Debug` to get the "started" request messages.
// developers can set the router log level to DEBUG to get the "started" request messages.
return
}

shortFilename := ""
line := 0
shortName := ""
isError := false

record.lock.RLock()
isLongPolling := record.isLongPolling
Expand All @@ -47,34 +47,36 @@ func logPrinter(logger log.Logger) func(trigger Event, record *requestRecord) {
// we might not find all handlers, so if a handler has not called `UpdateFuncInfo`, we won't know its information
// in such case, we should debug to find what handler it is and use `UpdateFuncInfo` to report its information
shortFilename = "unknown-handler"
isError = true
}
record.lock.RUnlock()

req := record.request

if trigger == StartEvent {
// when a request starts, we have no information about the handler function information, we only have the request path
logger.Debug("router: started %v %s for %s", log.ColoredMethod(req.Method), req.RequestURI, req.RemoteAddr)
logger.Debug("router: %s %v %s for %s", log.NewColoredValueBytes("started ", log.DEBUG.Color()), log.ColoredMethod(req.Method), req.RequestURI, req.RemoteAddr)
return
}

handlerFuncInfo := fmt.Sprintf("%s:%d(%s)", shortFilename, line, shortName)
if trigger == StillExecutingEvent {
message := "still-executing"
message := "slow "
level := log.WARN
if isLongPolling {
level = log.INFO
message = "long-polling"
message = "polling "
}
_ = logger.Log(0, level, "router: %s %v %s for %s, elapsed %v @ %s",
message,
log.NewColoredValueBytes(message, level.Color()),
log.ColoredMethod(req.Method), req.RequestURI, req.RemoteAddr,
log.ColoredTime(time.Since(record.startTime)),
handlerFuncInfo,
)
} else {
if record.panicError != nil {
_ = logger.Log(0, log.WARN, "router: failed %v %s for %s, panic in %v @ %s, err=%v",
_ = logger.Log(0, log.WARN, "router: %s %v %s for %s, panic in %v @ %s, err=%v",
log.NewColoredValueBytes("failed ", log.WARN.Color()),
shortFilename, line, shortName,
log.ColoredMethod(req.Method), req.RequestURI, req.RemoteAddr,
log.ColoredTime(time.Since(record.startTime)),
Expand All @@ -86,7 +88,13 @@ func logPrinter(logger log.Logger) func(trigger Event, record *requestRecord) {
if v, ok := record.responseWriter.(context.ResponseWriter); ok {
status = v.Status()
}
_ = logger.Log(0, setting.RouterLogLevel, "router: completed %v %s for %s, %v %v in %v @ %s",
level := log.INFO
if isError {
level = log.ERROR
}

_ = logger.Log(0, level, "router: %s %v %s for %s, %v %v in %v @ %s",
log.NewColoredValueBytes("completed", level.Color()),
log.ColoredMethod(req.Method), req.RequestURI, req.RemoteAddr,
log.ColoredStatus(status), log.ColoredStatus(status, http.StatusText(status)), log.ColoredTime(time.Since(record.startTime)),
handlerFuncInfo,
Expand Down
17 changes: 6 additions & 11 deletions routers/common/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,13 @@ func Middlewares() []func(http.Handler) http.Handler {
handlers = append(handlers, middleware.StripSlashes)

if !setting.DisableRouterLog {
var routerLogHandler func(http.Handler) http.Handler
if setting.RouterLogHandler == "router_logger_v1" {
routerLogHandler = routing.NewLoggerHandlerV1(setting.RouterLogLevel)
} else if setting.RouterLogHandler == "router_logger_v2" {
routerLogHandler = routing.NewLoggerHandlerV2()
}
if routerLogHandler == nil {
log.Warn("unknown router log handler '%s', fall back to router_logger_v2")
routerLogHandler = routing.NewLoggerHandlerV2()
}
handlers = append(handlers, routerLogHandler)
handlers = append(handlers, routing.NewLoggerHandlerV2())
}

if setting.EnableRouterV1 {
handlers = append(handlers, routing.NewLoggerHandlerV1(setting.RouterV1LogLevel))
}

if setting.EnableAccessLog {
handlers = append(handlers, context.AccessLogger())
}
Expand Down