mirror of
https://github.com/mattermost/mattermost.git
synced 2025-02-25 18:55:24 -06:00
Prevent disabling or modifying l4g logging filters (#8628)
The underlying l4g library is not resilient to filter modifications in the presence of concurrent goroutines. In particular, it's not safe to call Close() on filters which might be actively held by a goroutine for logging. This change disables all modifications to existing filters once initialized by the App layer. In practice, we might be able to get away with some modifications to the existing filters (i.e. changing levels), but the [golang memory model](https://golang.org/ref/mem) makes no guarantees that it is safe to do so: > Programs that modify data being simultaneously accessed by multiple goroutines must serialize such access. We can solve this holistically by introducing the requisite locking within our fork of the l4g library. For now, we just disable all modifications.
This commit is contained in:
committed by
Derrick Anderson
parent
a7fd13384b
commit
8056dc33e3
@@ -125,7 +125,14 @@ func New(options ...Option) (outApp *App, outErr error) {
|
||||
}
|
||||
}
|
||||
model.AppErrorInit(utils.T)
|
||||
|
||||
// The first time we load config, clear any existing filters to allow the configuration
|
||||
// changes to take effect. This is safe only because no one else is logging at this point.
|
||||
l4g.Close()
|
||||
|
||||
if err := app.LoadConfig(app.configFile); err != nil {
|
||||
// Re-initialize the default logger as we bail out.
|
||||
l4g.Global = l4g.NewDefaultLogger(l4g.DEBUG)
|
||||
return nil, err
|
||||
}
|
||||
app.EnableConfigWatch()
|
||||
|
||||
@@ -83,13 +83,15 @@ func ConfigureCmdLineLog() {
|
||||
ConfigureLog(&ls)
|
||||
}
|
||||
|
||||
// ConfigureLog enables and configures logging.
|
||||
//
|
||||
// Note that it is not currently possible to disable filters nor to modify previously enabled
|
||||
// filters, given the lack of concurrency guarantees from the underlying l4g library.
|
||||
//
|
||||
// TODO: this code initializes console and file logging. It will eventually be replaced by JSON logging in logger/logger.go
|
||||
// See PLT-3893 for more information
|
||||
func ConfigureLog(s *model.LogSettings) {
|
||||
|
||||
l4g.Close()
|
||||
|
||||
if s.EnableConsole {
|
||||
if _, alreadySet := l4g.Global["stdout"]; !alreadySet && s.EnableConsole {
|
||||
level := l4g.DEBUG
|
||||
if s.ConsoleLevel == "INFO" {
|
||||
level = l4g.INFO
|
||||
@@ -104,8 +106,7 @@ func ConfigureLog(s *model.LogSettings) {
|
||||
l4g.AddFilter("stdout", level, lw)
|
||||
}
|
||||
|
||||
if s.EnableFile {
|
||||
|
||||
if _, alreadySet := l4g.Global["file"]; !alreadySet && s.EnableFile {
|
||||
var fileFormat = s.FileFormat
|
||||
|
||||
if fileFormat == "" {
|
||||
|
||||
Reference in New Issue
Block a user