From 671cf6cc3de553f63f62209e5c2f489ffea55604 Mon Sep 17 00:00:00 2001 From: Ben Schumacher Date: Wed, 25 Oct 2023 16:40:07 +0200 Subject: [PATCH] [MM-54319] Add AdvancedLoggingJSON setting to system console (#24733) * Add AdvancedLoggingJSON setting to system console * Apply suggestions from code review Co-authored-by: Doug Lauder * Add validation for AdvancedLoggingJSON --------- Co-authored-by: Doug Lauder Co-authored-by: Mattermost Build --- server/i18n/en.json | 8 ++ server/public/model/config.go | 19 ++++ server/public/model/config_test.go | 103 ++++++++++++++++++ server/public/shared/mlog/mlog.go | 18 ++- .../admin_console/admin_definition.jsx | 43 ++++++++ webapp/channels/src/i18n/en.json | 3 + webapp/channels/src/utils/constants.tsx | 1 + 7 files changed, 194 insertions(+), 1 deletion(-) diff --git a/server/i18n/en.json b/server/i18n/en.json index d2fa0fdcc6..6a209fc805 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -8730,6 +8730,14 @@ "id": "model.config.is_valid.localization.available_locales.app_error", "translation": "Available Languages must contain Default Client Language." }, + { + "id": "model.config.is_valid.log.advanced_logging.json", + "translation": "Failed to parse JSON: {{.Error}}" + }, + { + "id": "model.config.is_valid.log.advanced_logging.parse", + "translation": "Invalid format: {{.Error}}" + }, { "id": "model.config.is_valid.login_attempts.app_error", "translation": "Invalid maximum login attempts for service settings. Must be a positive number." diff --git a/server/public/model/config.go b/server/public/model/config.go index 69ed3319bc..1b01b812c4 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -1285,6 +1285,21 @@ func NewLogSettings() *LogSettings { return settings } +func (s *LogSettings) isValid() *AppError { + cfg := make(mlog.LoggerConfiguration) + err := json.Unmarshal(s.GetAdvancedLoggingConfig(), &cfg) + if err != nil { + return NewAppError("LogSettings.isValid", "model.config.is_valid.log.advanced_logging.json", map[string]any{"Error": err}, "", http.StatusBadRequest).Wrap(err) + } + + err = cfg.IsValid() + if err != nil { + return NewAppError("LogSettings.isValid", "model.config.is_valid.log.advanced_logging.parse", map[string]any{"Error": err}, "", http.StatusBadRequest).Wrap(err) + } + + return nil +} + func (s *LogSettings) SetDefaults() { if s.EnableConsole == nil { s.EnableConsole = NewBool(true) @@ -3551,6 +3566,10 @@ func (o *Config) IsValid() *AppError { return appErr } + if appErr := o.LogSettings.isValid(); appErr != nil { + return appErr + } + if appErr := o.LocalizationSettings.isValid(); appErr != nil { return appErr } diff --git a/server/public/model/config_test.go b/server/public/model/config_test.go index 4858f60f0b..48d7956059 100644 --- a/server/public/model/config_test.go +++ b/server/public/model/config_test.go @@ -1208,6 +1208,109 @@ func TestLdapSettingsIsValid(t *testing.T) { } } +func TestLogSettingsIsValid(t *testing.T) { + for name, test := range map[string]struct { + LogSettings LogSettings + ExpectError bool + }{ + "empty": { + LogSettings: LogSettings{}, + ExpectError: false, + }, + "AdvancedLoggingJSON contains empty string": { + LogSettings: LogSettings{ + AdvancedLoggingJSON: json.RawMessage(``), + }, + ExpectError: false, + }, + "AdvancedLoggingJSON contains empty JSON": { + LogSettings: LogSettings{ + AdvancedLoggingJSON: json.RawMessage(`{}`), + }, + ExpectError: false, + }, + "AdvancedLoggingJSON has JSON error ": { + LogSettings: LogSettings{ + AdvancedLoggingJSON: json.RawMessage(` + { + "foo": "bar", + `), + }, + ExpectError: true, + }, + "AdvancedLoggingJSON has missing target": { + LogSettings: LogSettings{ + AdvancedLoggingJSON: json.RawMessage(` + { + "foo": "bar", + } + `), + }, + ExpectError: true, + }, + "AdvancedLoggingJSON has an unknown Type": { + LogSettings: LogSettings{ + AdvancedLoggingJSON: json.RawMessage(` + { + "console-log": { + "Type": "XYZ", + "Format": "json", + "Levels": [ + {"ID": 10, "Name": "stdlog", "Stacktrace": false}, + {"ID": 5, "Name": "debug", "Stacktrace": false}, + {"ID": 4, "Name": "info", "Stacktrace": false, "color": 36}, + {"ID": 3, "Name": "warn", "Stacktrace": false, "color": 33}, + {"ID": 2, "Name": "error", "Stacktrace": true, "color": 31}, + {"ID": 1, "Name": "fatal", "Stacktrace": true}, + {"ID": 0, "Name": "panic", "Stacktrace": true} + ], + "Options": { + "Out": "stdout" + }, + "MaxQueueSize": 1000 + } + } + `), + }, + ExpectError: true, + }, + "AdvancedLoggingJSON is valid": { + LogSettings: LogSettings{ + AdvancedLoggingJSON: json.RawMessage(` + { + "console-log": { + "Type": "console", + "Format": "json", + "Levels": [ + {"ID": 5, "Name": "debug", "Stacktrace": false}, + {"ID": 4, "Name": "info", "Stacktrace": false, "color": 36}, + {"ID": 3, "Name": "warn", "Stacktrace": false, "color": 33}, + {"ID": 2, "Name": "error", "Stacktrace": true, "color": 31} + ], + "Options": { + "Out": "stdout" + }, + "MaxQueueSize": 1000 + } + } + `), + }, + ExpectError: false, + }, + } { + t.Run(name, func(t *testing.T) { + test.LogSettings.SetDefaults() + + appErr := test.LogSettings.isValid() + if test.ExpectError { + assert.NotNil(t, appErr) + } else { + assert.Nil(t, appErr) + } + }) + } +} + func TestConfigSanitize(t *testing.T) { c := Config{} c.SetDefaults() diff --git a/server/public/shared/mlog/mlog.go b/server/public/shared/mlog/mlog.go index 5bfcac963a..593b977fe4 100644 --- a/server/public/shared/mlog/mlog.go +++ b/server/public/shared/mlog/mlog.go @@ -7,7 +7,6 @@ package mlog import ( "context" "encoding/json" - "errors" "fmt" "io" "log" @@ -16,6 +15,8 @@ import ( "sync/atomic" "time" + "github.com/pkg/errors" + "github.com/mattermost/logr/v2" logrcfg "github.com/mattermost/logr/v2/config" ) @@ -68,6 +69,21 @@ func (lc LoggerConfiguration) Append(cfg LoggerConfiguration) { } } +func (lc LoggerConfiguration) IsValid() error { + logger, err := logr.New() + if err != nil { + return errors.Wrap(err, "failed to create logger") + } + defer logger.Shutdown() + + err = logrcfg.ConfigureTargets(logger, lc, nil) + if err != nil { + return errors.Wrap(err, "logger configuration is invalid") + } + + return nil +} + func (lc LoggerConfiguration) toTargetCfg() map[string]logrcfg.TargetCfg { tcfg := make(map[string]logrcfg.TargetCfg) for k, v := range lc { diff --git a/webapp/channels/src/components/admin_console/admin_definition.jsx b/webapp/channels/src/components/admin_console/admin_definition.jsx index dc91bf8492..cb8dbea22d 100644 --- a/webapp/channels/src/components/admin_console/admin_definition.jsx +++ b/webapp/channels/src/components/admin_console/admin_definition.jsx @@ -1863,6 +1863,49 @@ const AdminDefinition = { }, isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.ENVIRONMENT.LOGGING)), }, + { + type: Constants.SettingsTypes.TYPE_LONG_TEXT, + key: 'LogSettings.AdvancedLoggingJSON', + label: t('admin.log.AdvancedLoggingJSONTitle'), + label_default: 'Advanced Logging:', + help_text: t('admin.log.AdvancedLoggingJSONDescription'), + help_text_default: 'The JSON configuration for Advanced Logging. Please see documentation to learn more about Advanced Logging and the JSON format it uses.', + help_text_markdown: false, + help_text_values: { + link: (msg) => ( + + {msg} + + ), + }, + placeholder: t('admin.log.AdvancedLoggingJSONPlaceholder'), + placeholder_default: 'Enter your JSON configuration', + isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.ENVIRONMENT.LOGGING)), + validate: (value) => { + const valid = new ValidationResult(true, '', ''); + if (!value) { + return valid; + } + try { + JSON.parse(value); + return valid; + } catch (error) { + return new ValidationResult(false, '', error.message); + } + }, + onConfigLoad: (configVal) => JSON.stringify(configVal, null, ' '), + onConfigSave: (displayVal) => { + // Handle case where field is empty + if (!displayVal) { + return {undefined}; + } + + return JSON.parse(displayVal); + }, + }, ], }, }, diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index a72819bd65..dda8c69266 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -1373,6 +1373,9 @@ "admin.license.warn.renew": "Renew", "admin.lockTeammateNameDisplay": "Lock Teammate Name Display for all users: ", "admin.lockTeammateNameDisplayHelpText": "When true, disables users' ability to change settings under Settings > Display > Teammate Name Display.", + "admin.log.AdvancedLoggingJSONDescription": "The JSON configuration for Advanced Logging. Please see documentation to learn more about Advanced Logging and the JSON format it uses.", + "admin.log.AdvancedLoggingJSONPlaceholder": "Enter your JSON configuration", + "admin.log.AdvancedLoggingJSONTitle": "Advanced Logging:", "admin.log.consoleDescription": "Typically set to false in production. Developers may set this field to true to output log messages to console based on the console level option. If true, server writes messages to the standard output stream (stdout). Changing this setting requires a server restart before taking effect.", "admin.log.consoleJsonTitle": "Output console logs as JSON:", "admin.log.consoleTitle": "Output logs to console: ", diff --git a/webapp/channels/src/utils/constants.tsx b/webapp/channels/src/utils/constants.tsx index 809cb182ea..895d42f090 100644 --- a/webapp/channels/src/utils/constants.tsx +++ b/webapp/channels/src/utils/constants.tsx @@ -1088,6 +1088,7 @@ export const HostedCustomerLinks = { export const DocLinks = { ABOUT_TEAMS: 'https://docs.mattermost.com/welcome/about-teams.html#team-url', + ADVANCED_LOGGING: 'https://mattermost.com/pl/advanced-logging', CONFIGURE_DOCUMENT_CONTENT_SEARCH: 'https://mattermost.com/pl/configure-document-content-search', CONFIGURE_AD_LDAP_QUERY_TIMEOUT: 'https://mattermost.com/pl/configure-ad-ldap-query-timeout', CONFIGURE_OVERRIDE_SAML_BIND_DATA_WITH_LDAP: 'https://mattermost.com/pl/configure-override-saml-bind-data-with-ldap',