MM-25731 Advanced Logging (#14888)

Adds Advanced Logging to server. Advanced Logging is an optional logging capability that allows customers to send log records to any number of destinations.

Supported destinations:
- file
- syslog (with out without TLS)
- raw TCP socket (with out without TLS)

Allows developers to specify discrete log levels as well as the standard trace, debug, info, ... panic. Existing code and logging API usage is unchanged.

Log records are emitted asynchronously to reduce latency to the caller. Supports hot-reloading of logger config, including adding removing targets.

Advanced Logging is configured within config.json via "LogSettings.AdvancedLoggingConfig" which can contain a filespec to another config file, a database DSN, or JSON.

config/emitter.go

@@ -6,6 +6,7 @@ package config
 import (
 	"sync"
 
+	"github.com/mattermost/mattermost-server/v5/mlog"
 	"github.com/mattermost/mattermost-server/v5/model"
 )
 
@@ -37,3 +38,29 @@ func (e *emitter) invokeConfigListeners(oldCfg, newCfg *model.Config) {
 		return true
 	})
 }
+
+// srcEmitter enables threadsafe registration and broadcasting to configuration listeners
+type logSrcEmitter struct {
+	listeners sync.Map
+}
+
+// AddListener adds a callback function to invoke when the configuration is modified.
+func (e *logSrcEmitter) AddListener(listener LogSrcListener) string {
+	id := model.NewId()
+	e.listeners.Store(id, listener)
+	return id
+}
+
+// RemoveListener removes a callback function using an id returned from AddListener.
+func (e *logSrcEmitter) RemoveListener(id string) {
+	e.listeners.Delete(id)
+}
+
+// invokeConfigListeners synchronously notifies all listeners about the configuration change.
+func (e *logSrcEmitter) invokeConfigListeners(oldCfg, newCfg mlog.LogTargetCfg) {
+	e.listeners.Range(func(key, value interface{}) bool {
+		listener := value.(LogSrcListener)
+		listener(oldCfg, newCfg)
+		return true
+	})
+}

config/emitter_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 
+	"github.com/mattermost/mattermost-server/v5/mlog"
 	"github.com/mattermost/mattermost-server/v5/model"
 )
 
@@ -51,3 +52,44 @@ func TestEmitter(t *testing.T) {
 	assert.False(t, listener1, "listener 1 should not have been called")
 	assert.False(t, listener2, "listener 2 should not have been called")
 }
+
+func TestLogSrcEmitter(t *testing.T) {
+	var e logSrcEmitter
+
+	expectedOldCfg := make(mlog.LogTargetCfg)
+	expectedNewCfg := make(mlog.LogTargetCfg)
+
+	listener1 := false
+	id1 := e.AddListener(func(oldCfg, newCfg mlog.LogTargetCfg) {
+		assert.Equal(t, expectedOldCfg, oldCfg)
+		assert.Equal(t, expectedNewCfg, newCfg)
+		listener1 = true
+	})
+
+	listener2 := false
+	id2 := e.AddListener(func(oldCfg, newCfg mlog.LogTargetCfg) {
+		assert.Equal(t, expectedOldCfg, oldCfg)
+		assert.Equal(t, expectedNewCfg, newCfg)
+		listener2 = true
+	})
+
+	e.invokeConfigListeners(expectedOldCfg, expectedNewCfg)
+	assert.True(t, listener1, "listener 1 not called")
+	assert.True(t, listener2, "listener 2 not called")
+
+	e.RemoveListener(id2)
+
+	listener1 = false
+	listener2 = false
+	e.invokeConfigListeners(expectedOldCfg, expectedNewCfg)
+	assert.True(t, listener1, "listener 1 not called")
+	assert.False(t, listener2, "listener 2 should not have been called")
+
+	e.RemoveListener(id1)
+
+	listener1 = false
+	listener2 = false
+	e.invokeConfigListeners(expectedOldCfg, expectedNewCfg)
+	assert.False(t, listener1, "listener 1 should not have been called")
+	assert.False(t, listener2, "listener 2 should not have been called")
+}

config/file.go

@@ -181,6 +181,12 @@ func (fs *FileStore) GetFile(name string) ([]byte, error) {
 	return data, nil
 }
 
+// GetFilePath returns the resolved path of a configuration file.
+// The file may not necessarily exist.
+func (fs *FileStore) GetFilePath(name string) string {
+	return fs.resolveFilePath(name)
+}
+
 // SetFile sets or replaces the contents of a configuration file.
 func (fs *FileStore) SetFile(name string, data []byte) error {
 	resolvedPath := fs.resolveFilePath(name)

config/logging.go

@@ -0,0 +1,188 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package config
+
+import (
+	"os"
+	"strings"
+	"sync"
+
+	"github.com/mattermost/mattermost-server/v5/mlog"
+)
+
+type LogSrcListener func(old, new mlog.LogTargetCfg)
+
+type FileGetter interface {
+	GetFile(name string) ([]byte, error)
+}
+
+// LogConfigSrc abstracts the Advanced Logging configuration so that implementations can
+// fetch from file, database, etc.
+type LogConfigSrc interface {
+	// Get fetches the current, cached configuration.
+	Get() mlog.LogTargetCfg
+
+	// Set updates the dsn specifying the source and reloads
+	Set(dsn string, fget FileGetter) (err error)
+
+	// AddListener adds a callback function to invoke when the configuration is modified.
+	AddListener(listener LogSrcListener) string
+
+	// RemoveListener removes a callback function using an id returned from AddListener.
+	RemoveListener(id string)
+
+	// Close cleans up resources.
+	Close() error
+}
+
+// NewLogConfigSrc creates an advanced logging configuration source, backed by a
+// file, JSON string, or database.
+func NewLogConfigSrc(dsn string, isJSON bool, fget FileGetter) (LogConfigSrc, error) {
+	dsn = strings.TrimSpace(dsn)
+
+	if isJSON {
+		return newJSONSrc(dsn)
+	}
+	return newFileSrc(dsn, fget)
+}
+
+// jsonSrc
+
+type jsonSrc struct {
+	logSrcEmitter
+	mutex sync.RWMutex
+	cfg   mlog.LogTargetCfg
+}
+
+func newJSONSrc(data string) (*jsonSrc, error) {
+	src := &jsonSrc{}
+	return src, src.Set(data, nil)
+}
+
+// Get fetches the current, cached configuration
+func (src *jsonSrc) Get() mlog.LogTargetCfg {
+	src.mutex.RLock()
+	defer src.mutex.RUnlock()
+	return src.cfg
+}
+
+// Set updates the JSON specifying the source and reloads
+func (src *jsonSrc) Set(data string, _ FileGetter) error {
+	cfg, err := JSONToLogTargetCfg([]byte(data))
+	if err != nil {
+		return err
+	}
+
+	src.set(cfg)
+	return nil
+}
+
+func (src *jsonSrc) set(cfg mlog.LogTargetCfg) {
+	src.mutex.Lock()
+	defer src.mutex.Unlock()
+
+	old := src.cfg
+	src.cfg = cfg
+	src.invokeConfigListeners(old, cfg)
+}
+
+// Close cleans up resources.
+func (src *jsonSrc) Close() error {
+	return nil
+}
+
+// fileSrc
+
+type fileSrc struct {
+	logSrcEmitter
+	mutex sync.RWMutex
+	cfg   mlog.LogTargetCfg
+
+	path    string
+	watcher *watcher
+}
+
+func newFileSrc(path string, fget FileGetter) (*fileSrc, error) {
+	src := &fileSrc{
+		path: path,
+	}
+	if err := src.Set(path, fget); err != nil {
+		return nil, err
+	}
+	return src, nil
+}
+
+// Get fetches the current, cached configuration
+func (src *fileSrc) Get() mlog.LogTargetCfg {
+	src.mutex.RLock()
+	defer src.mutex.RUnlock()
+	return src.cfg
+}
+
+// Set updates the dsn specifying the file source and reloads.
+// The file will be watched for changes and reloaded as needed,
+// and all listeners notified.
+func (src *fileSrc) Set(path string, fget FileGetter) error {
+	data, err := fget.GetFile(path)
+	if err != nil {
+		return err
+	}
+
+	cfg, err := JSONToLogTargetCfg(data)
+	if err != nil {
+		return err
+	}
+
+	src.set(cfg)
+
+	// If path is a real file and not just the name of a database resource then watch it for changes.
+	// Absolute paths are explicit and require no resolution.
+	if _, err = os.Stat(path); os.IsNotExist(err) {
+		return nil
+	}
+
+	src.mutex.Lock()
+	defer src.mutex.Unlock()
+
+	if src.watcher != nil {
+		if err = src.watcher.Close(); err != nil {
+			mlog.Error("Failed to close watcher", mlog.Err(err))
+		}
+		src.watcher = nil
+	}
+
+	watcher, err := newWatcher(path, func() {
+		if serr := src.Set(path, fget); serr != nil {
+			mlog.Error("Failed to reload file on change", mlog.String("path", path), mlog.Err(serr))
+		}
+	})
+	if err != nil {
+		return err
+	}
+
+	src.watcher = watcher
+
+	return nil
+}
+
+func (src *fileSrc) set(cfg mlog.LogTargetCfg) {
+	src.mutex.Lock()
+	defer src.mutex.Unlock()
+
+	old := src.cfg
+	src.cfg = cfg
+	src.invokeConfigListeners(old, cfg)
+}
+
+// Close cleans up resources.
+func (src *fileSrc) Close() error {
+	var err error
+	src.mutex.Lock()
+	defer src.mutex.Unlock()
+	if src.watcher != nil {
+		err = src.watcher.Close()
+		src.watcher = nil
+	}
+	return err
+}

config/logging_test.go

@@ -0,0 +1,58 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package config
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+const (
+	validJSON = `{"file":{ "Type":"file"}}`
+	badJSON   = `{"file":{ Type="file"}}`
+)
+
+type fgetFunc func(string) ([]byte, error)
+
+func (f fgetFunc) GetFile(path string) ([]byte, error) {
+	return f(path)
+}
+
+func getValidFile(path string) ([]byte, error) {
+	return []byte(validJSON), nil
+}
+
+func getInvalidFile(path string) ([]byte, error) {
+	return nil, os.ErrNotExist
+}
+
+func TestNewLogConfigSrc(t *testing.T) {
+	tests := []struct {
+		name     string
+		dsn      string
+		fget     FileGetter
+		wantErr  bool
+		wantType LogConfigSrc
+	}{
+		{name: "empty dsn", dsn: "", fget: fgetFunc(getInvalidFile), wantErr: true, wantType: nil},
+		{name: "garbage dsn", dsn: "!@wfejwcevioj", fget: fgetFunc(getInvalidFile), wantErr: true, wantType: nil},
+		{name: "valid json dsn", dsn: validJSON, fget: fgetFunc(getInvalidFile), wantErr: false, wantType: &jsonSrc{}},
+		{name: "invalid json dsn", dsn: badJSON, fget: fgetFunc(getInvalidFile), wantErr: true, wantType: nil},
+		{name: "valid filespec dsn", dsn: "advancedlogging.conf", fget: fgetFunc(getValidFile), wantErr: false, wantType: &fileSrc{}},
+		{name: "invalid filespec dsn", dsn: "/nobody/here.conf", fget: fgetFunc(getInvalidFile), wantErr: true, wantType: nil},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := NewLogConfigSrc(tt.dsn, IsJsonMap(tt.dsn), tt.fget)
+			if tt.wantErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				assert.IsType(t, tt.wantType, got)
+			}
+		})
+	}
+}

config/migrate.go

@@ -5,7 +5,7 @@ package config
 
 import "github.com/pkg/errors"
 
-// Migrate migrates SAML keys and certificates from one store to another given their data source names.
+// Migrate migrates SAML keys, certificates, and other config files from one store to another given their data source names.
 func Migrate(from, to string) error {
 	source, err := NewStore(from, false)
 	if err != nil {
@@ -30,6 +30,11 @@ func Migrate(from, to string) error {
 		*sourceConfig.SamlSettings.PrivateKeyFile,
 	}
 
+	// Only migrate advanced logging config if it is not embedded JSON.
+	if !IsJsonMap(*sourceConfig.LogSettings.AdvancedLoggingConfig) {
+		files = append(files, *sourceConfig.LogSettings.AdvancedLoggingConfig)
+	}
+
 	files = append(files, sourceConfig.PluginSettings.SignaturePublicKeyFiles...)
 
 	for _, file := range files {

config/utils.go

@@ -4,6 +4,7 @@
 package config
 
 import (
+	"encoding/json"
 	"strings"
 
 	"github.com/mattermost/mattermost-server/v5/mlog"
@@ -161,3 +162,17 @@ func stripPassword(dsn, schema string) string {
 
 	return prefix + dsn[:i+1] + dsn[j:]
 }
+
+func IsJsonMap(data string) bool {
+	var m map[string]interface{}
+	return json.Unmarshal([]byte(data), &m) == nil
+}
+
+func JSONToLogTargetCfg(data []byte) (mlog.LogTargetCfg, error) {
+	cfg := make(mlog.LogTargetCfg)
+	err := json.Unmarshal(data, &cfg)
+	if err != nil {
+		return nil, err
+	}
+	return cfg, nil
+}

config/utils_test.go

@@ -6,10 +6,9 @@ package config
 import (
 	"testing"
 
-	"github.com/stretchr/testify/assert"
-
 	"github.com/mattermost/mattermost-server/v5/model"
 	"github.com/mattermost/mattermost-server/v5/utils"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestDesanitize(t *testing.T) {
@@ -204,3 +203,34 @@ func sToP(s string) *string {
 func bToP(b bool) *bool {
 	return &b
 }
+
+func TestIsJsonMap(t *testing.T) {
+	tests := []struct {
+		name string
+		data string
+		want bool
+	}{
+		{name: "good json", data: `{"local_tcp": {
+			"Type": "tcp","Format": "json","Levels": [
+				{"ID": 5,"Name": "debug","Stacktrace": false}
+			],
+			"Options": {"ip": "localhost","port": 18065},
+			"MaxQueueSize": 1000}}
+			`, want: true,
+		},
+		{name: "empty json", data: "{}", want: true},
+		{name: "string json", data: `"test"`, want: false},
+		{name: "array json", data: `["test1", "test2"]`, want: false},
+		{name: "bad json", data: `{huh?}`, want: false},
+		{name: "filename", data: "/tmp/logger.conf", want: false},
+		{name: "mysql dsn", data: "mysql://mmuser:@tcp(localhost:3306)/mattermost?charset=utf8mb4,utf8&readTimeout=30s", want: false},
+		{name: "postgres dsn", data: "postgres://mmuser:passwordlocalhost:5432/mattermost?sslmode=disable&connect_timeout=10", want: false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := IsJsonMap(tt.data); got != tt.want {
+				t.Errorf("IsJsonMap() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}