Revert "Allow inline JSON in config.json for advanced logging config (#20954)" (#20983)

This reverts commit 5211d5de15.

app/audit.go

@@ -109,14 +109,14 @@ func (s *Server) configureAudit(adt *audit.Audit, bAllowAdvancedLogging bool) er
 	adt.OnError = s.onAuditError
 
 	var logConfigSrc config.LogConfigSrc
-	auditSettings := s.platform.Config().ExperimentalAuditSettings
-	if bAllowAdvancedLogging && !config.IsEmptyDSN(auditSettings.AdvancedLoggingConfig) {
+	dsn := *s.platform.Config().ExperimentalAuditSettings.AdvancedLoggingConfig
+	if bAllowAdvancedLogging && dsn != "" {
 		var err error
-		logConfigSrc, err = config.NewLogConfigSrc(auditSettings.AdvancedLoggingConfig, s.platform.GetConfigStore())
+		logConfigSrc, err = config.NewLogConfigSrc(dsn, s.platform.GetConfigStore())
 		if err != nil {
 			return fmt.Errorf("invalid config source for audit, %w", err)
 		}
-		mlog.Debug("Loaded audit configuration", mlog.String("source", string(auditSettings.AdvancedLoggingConfig)))
+		mlog.Debug("Loaded audit configuration", mlog.String("source", dsn))
 	}
 
 	// ExperimentalAuditSettings provides basic file audit (E0, E10); logConfigSrc provides advanced config (E20).

app/platform/config.go

@@ -122,14 +122,14 @@ func (ps *PlatformService) ConfigureLogger(name string, logger *mlog.Logger, log
 	// file is loaded.  If no valid E20 license exists then advanced logging will be
 	// shutdown once license is loaded/checked.
 	var err error
+	dsn := *logSettings.AdvancedLoggingConfig
 	var logConfigSrc config.LogConfigSrc
-
-	if !config.IsEmptyDSN(logSettings.AdvancedLoggingConfig) {
-		logConfigSrc, err = config.NewLogConfigSrc(logSettings.AdvancedLoggingConfig, ps.configStore)
+	if dsn != "" {
+		logConfigSrc, err = config.NewLogConfigSrc(dsn, ps.configStore)
 		if err != nil {
 			return fmt.Errorf("invalid config source for %s, %w", name, err)
 		}
-		ps.logger.Info("Loaded configuration for "+name, mlog.String("source", string(logSettings.AdvancedLoggingConfig)))
+		ps.logger.Info("Loaded configuration for "+name, mlog.String("source", dsn))
 	}
 
 	cfg, err := config.MloggerConfigFromLoggerConfig(logSettings, logConfigSrc, getPath)

config/logconfigsrc.go

@@ -4,11 +4,9 @@
 package config
 
 import (
-	"bytes"
 	"encoding/json"
 	"errors"
 	"path/filepath"
-	"strconv"
 	"strings"
 	"sync"
 
@@ -24,23 +22,16 @@ type LogConfigSrc interface {
 	Get() mlog.LoggerConfiguration
 
 	// Set updates the dsn specifying the source and reloads
-	Set(dsn []byte, configStore *Store) (err error)
+	Set(dsn string, configStore *Store) (err error)
 
 	// Close cleans up resources.
 	Close() error
 }
 
-func IsEmptyDSN(dsn json.RawMessage) bool {
-	if len(dsn) == 0 || bytes.Equal(dsn, []byte("{}")) || bytes.Equal(dsn, []byte("\"\"")) {
-		return true
-	}
-	return false
-}
-
 // NewLogConfigSrc creates an advanced logging configuration source, backed by a
-// file, JSON, or database.
-func NewLogConfigSrc(dsn json.RawMessage, configStore *Store) (LogConfigSrc, error) {
-	if len(dsn) == 0 {
+// file, JSON string, or database.
+func NewLogConfigSrc(dsn string, configStore *Store) (LogConfigSrc, error) {
+	if dsn == "" {
 		return nil, errors.New("dsn should not be empty")
 	}
 
@@ -48,28 +39,17 @@ func NewLogConfigSrc(dsn json.RawMessage, configStore *Store) (LogConfigSrc, err
 		return nil, errors.New("configStore should not be nil")
 	}
 
-	// check if embedded JSON
+	dsn = strings.TrimSpace(dsn)
+
 	if isJSONMap(dsn) {
 		return newJSONSrc(dsn)
 	}
 
-	// Now we're treating the DSN as a string which may contain escaped JSON or be a filespec.
-	str := strings.TrimSpace(string(dsn))
-	if s, err := strconv.Unquote(str); err == nil {
-		str = s
-	}
-
-	// check if escaped JSON
-	strBytes := []byte(str)
-	if isJSONMap(strBytes) {
-		return newJSONSrc(strBytes)
-	}
-
+	path := dsn
 	// If this is a file based config we need the full path so it can be watched.
-	path := str
-	if strings.HasPrefix(configStore.String(), "file://") && !filepath.IsAbs(path) {
+	if strings.HasPrefix(configStore.String(), "file://") && !filepath.IsAbs(dsn) {
 		configPath := strings.TrimPrefix(configStore.String(), "file://")
-		path = filepath.Join(filepath.Dir(configPath), path)
+		path = filepath.Join(filepath.Dir(configPath), dsn)
 	}
 
 	return newFileSrc(path, configStore)
@@ -83,7 +63,7 @@ type jsonSrc struct {
 	cfg   mlog.LoggerConfiguration
 }
 
-func newJSONSrc(data json.RawMessage) (*jsonSrc, error) {
+func newJSONSrc(data string) (*jsonSrc, error) {
 	src := &jsonSrc{}
 	return src, src.Set(data, nil)
 }
@@ -96,8 +76,8 @@ func (src *jsonSrc) Get() mlog.LoggerConfiguration {
 }
 
 // Set updates the JSON specifying the source and reloads
-func (src *jsonSrc) Set(data []byte, _ *Store) error {
-	cfg, err := logTargetCfgFromJSON(data)
+func (src *jsonSrc) Set(data string, _ *Store) error {
+	cfg, err := logTargetCfgFromJSON([]byte(data))
 	if err != nil {
 		return err
 	}
@@ -132,7 +112,7 @@ func newFileSrc(path string, configStore *Store) (*fileSrc, error) {
 	src := &fileSrc{
 		path: path,
 	}
-	if err := src.Set([]byte(path), configStore); err != nil {
+	if err := src.Set(path, configStore); err != nil {
 		return nil, err
 	}
 	return src, nil
@@ -148,8 +128,8 @@ func (src *fileSrc) Get() mlog.LoggerConfiguration {
 // 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 []byte, configStore *Store) error {
-	data, err := configStore.GetFile(string(path))
+func (src *fileSrc) Set(path string, configStore *Store) error {
+	data, err := configStore.GetFile(path)
 	if err != nil {
 		return err
 	}

config/logconfigsrc_test.go

@@ -4,41 +4,36 @@
 package config
 
 import (
-	"encoding/json"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-var (
-	validJSON        = []byte(`{"file":{ "Type":"file"}}`)
-	badJSON          = []byte(`{"file":{ Type="file"}}`)
-	validEscapedJSON = []byte(`"{\"file\":{ \"Type\":\"file\"}}"`)
-	badEscapedJSON   = []byte(`"{\"file\":{ Type:\"file\"}}"`)
+const (
+	validJSON = `{"file":{ "Type":"file"}}`
+	badJSON   = `{"file":{ Type="file"}}`
 )
 
 func TestNewLogConfigSrc(t *testing.T) {
 	store := NewTestMemoryStore()
 	require.NotNil(t, store)
-	err := store.SetFile("advancedlogging.conf", validJSON)
+	err := store.SetFile("advancedlogging.conf", []byte(validJSON))
 	require.NoError(t, err)
 
 	tests := []struct {
 		name        string
-		dsn         json.RawMessage
+		dsn         string
 		configStore *Store
 		wantErr     bool
 		wantType    LogConfigSrc
 	}{
-		{name: "empty dsn", dsn: []byte(""), configStore: store, wantErr: true, wantType: nil},
-		{name: "garbage dsn", dsn: []byte("!@wfejwcevioj"), configStore: store, wantErr: true, wantType: nil},
+		{name: "empty dsn", dsn: "", configStore: store, wantErr: true, wantType: nil},
+		{name: "garbage dsn", dsn: "!@wfejwcevioj", configStore: store, wantErr: true, wantType: nil},
 		{name: "valid json dsn", dsn: validJSON, configStore: store, wantErr: false, wantType: &jsonSrc{}},
 		{name: "invalid json dsn", dsn: badJSON, configStore: store, wantErr: true, wantType: nil},
-		{name: "valid escaped json dsn", dsn: validEscapedJSON, configStore: store, wantErr: false, wantType: &jsonSrc{}},
-		{name: "invalid escaped json dsn", dsn: badEscapedJSON, configStore: store, wantErr: true, wantType: nil},
-		{name: "valid filespec dsn", dsn: []byte("advancedlogging.conf"), configStore: store, wantErr: false, wantType: &fileSrc{}},
-		{name: "invalid filespec dsn", dsn: []byte("/nobody/here.conf"), configStore: store, wantErr: true, wantType: nil},
+		{name: "valid filespec dsn", dsn: "advancedlogging.conf", configStore: store, wantErr: false, wantType: &fileSrc{}},
+		{name: "invalid filespec dsn", dsn: "/nobody/here.conf", configStore: store, wantErr: true, wantType: nil},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

config/migrate.go

@@ -33,8 +33,8 @@ func Migrate(from, to string) error {
 	}
 
 	// Only migrate advanced logging config if it is not embedded JSON.
-	if !isJSONMap(sourceConfig.LogSettings.AdvancedLoggingConfig) {
-		files = append(files, string(sourceConfig.LogSettings.AdvancedLoggingConfig))
+	if !isJSONMap(*sourceConfig.LogSettings.AdvancedLoggingConfig) {
+		files = append(files, *sourceConfig.LogSettings.AdvancedLoggingConfig)
 	}
 
 	files = append(files, sourceConfig.PluginSettings.SignaturePublicKeyFiles...)

config/utils.go

@@ -200,10 +200,9 @@ func stripPassword(dsn, schema string) string {
 	return prefix + dsn[:i+1] + dsn[j:]
 }
 
-func isJSONMap(data []byte) bool {
+func isJSONMap(data string) bool {
 	var m map[string]any
-	err := json.Unmarshal(data, &m)
-	return err == nil
+	return json.Unmarshal([]byte(data), &m) == nil
 }
 
 func GetValueByPath(path []string, obj any) (any, bool) {

config/utils_test.go

@@ -276,7 +276,7 @@ func TestIsJSONMap(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := isJSONMap([]byte(tt.data)); got != tt.want {
+			if got := isJSONMap(tt.data); got != tt.want {
 				t.Errorf("isJSONMap() = %v, want %v", got, tt.want)
 			}
 		})

model/config.go

@@ -1210,18 +1210,18 @@ func (s *SqlSettings) SetDefaults(isUpdate bool) {
 }
 
 type LogSettings struct {
-	EnableConsole          *bool           `access:"environment_logging,write_restrictable,cloud_restrictable"`
-	ConsoleLevel           *string         `access:"environment_logging,write_restrictable,cloud_restrictable"`
-	ConsoleJson            *bool           `access:"environment_logging,write_restrictable,cloud_restrictable"`
-	EnableColor            *bool           `access:"environment_logging,write_restrictable,cloud_restrictable"` // telemetry: none
-	EnableFile             *bool           `access:"environment_logging,write_restrictable,cloud_restrictable"`
-	FileLevel              *string         `access:"environment_logging,write_restrictable,cloud_restrictable"`
-	FileJson               *bool           `access:"environment_logging,write_restrictable,cloud_restrictable"`
-	FileLocation           *string         `access:"environment_logging,write_restrictable,cloud_restrictable"`
-	EnableWebhookDebugging *bool           `access:"environment_logging,write_restrictable,cloud_restrictable"`
-	EnableDiagnostics      *bool           `access:"environment_logging,write_restrictable,cloud_restrictable"` // telemetry: none
-	EnableSentry           *bool           `access:"environment_logging,write_restrictable,cloud_restrictable"` // telemetry: none
-	AdvancedLoggingConfig  json.RawMessage `access:"environment_logging,write_restrictable,cloud_restrictable"`
+	EnableConsole          *bool   `access:"environment_logging,write_restrictable,cloud_restrictable"`
+	ConsoleLevel           *string `access:"environment_logging,write_restrictable,cloud_restrictable"`
+	ConsoleJson            *bool   `access:"environment_logging,write_restrictable,cloud_restrictable"`
+	EnableColor            *bool   `access:"environment_logging,write_restrictable,cloud_restrictable"` // telemetry: none
+	EnableFile             *bool   `access:"environment_logging,write_restrictable,cloud_restrictable"`
+	FileLevel              *string `access:"environment_logging,write_restrictable,cloud_restrictable"`
+	FileJson               *bool   `access:"environment_logging,write_restrictable,cloud_restrictable"`
+	FileLocation           *string `access:"environment_logging,write_restrictable,cloud_restrictable"`
+	EnableWebhookDebugging *bool   `access:"environment_logging,write_restrictable,cloud_restrictable"`
+	EnableDiagnostics      *bool   `access:"environment_logging,write_restrictable,cloud_restrictable"` // telemetry: none
+	EnableSentry           *bool   `access:"environment_logging,write_restrictable,cloud_restrictable"` // telemetry: none
+	AdvancedLoggingConfig  *string `access:"environment_logging,write_restrictable,cloud_restrictable"`
 }
 
 func NewLogSettings() *LogSettings {
@@ -1276,19 +1276,19 @@ func (s *LogSettings) SetDefaults() {
 	}
 
 	if s.AdvancedLoggingConfig == nil {
-		s.AdvancedLoggingConfig = []byte("{}")
+		s.AdvancedLoggingConfig = NewString("")
 	}
 }
 
 type ExperimentalAuditSettings struct {
-	FileEnabled           *bool           `access:"experimental_features,write_restrictable,cloud_restrictable"`
-	FileName              *string         `access:"experimental_features,write_restrictable,cloud_restrictable"` // telemetry: none
-	FileMaxSizeMB         *int            `access:"experimental_features,write_restrictable,cloud_restrictable"`
-	FileMaxAgeDays        *int            `access:"experimental_features,write_restrictable,cloud_restrictable"`
-	FileMaxBackups        *int            `access:"experimental_features,write_restrictable,cloud_restrictable"`
-	FileCompress          *bool           `access:"experimental_features,write_restrictable,cloud_restrictable"`
-	FileMaxQueueSize      *int            `access:"experimental_features,write_restrictable,cloud_restrictable"`
-	AdvancedLoggingConfig json.RawMessage `access:"experimental_features,write_restrictable,cloud_restrictable"`
+	FileEnabled           *bool   `access:"experimental_features,write_restrictable,cloud_restrictable"`
+	FileName              *string `access:"experimental_features,write_restrictable,cloud_restrictable"` // telemetry: none
+	FileMaxSizeMB         *int    `access:"experimental_features,write_restrictable,cloud_restrictable"`
+	FileMaxAgeDays        *int    `access:"experimental_features,write_restrictable,cloud_restrictable"`
+	FileMaxBackups        *int    `access:"experimental_features,write_restrictable,cloud_restrictable"`
+	FileCompress          *bool   `access:"experimental_features,write_restrictable,cloud_restrictable"`
+	FileMaxQueueSize      *int    `access:"experimental_features,write_restrictable,cloud_restrictable"`
+	AdvancedLoggingConfig *string `access:"experimental_features,write_restrictable,cloud_restrictable"`
 }
 
 func (s *ExperimentalAuditSettings) SetDefaults() {
@@ -1321,20 +1321,20 @@ func (s *ExperimentalAuditSettings) SetDefaults() {
 	}
 
 	if s.AdvancedLoggingConfig == nil {
-		s.AdvancedLoggingConfig = []byte("{}")
+		s.AdvancedLoggingConfig = NewString("")
 	}
 }
 
 type NotificationLogSettings struct {
-	EnableConsole         *bool           `access:"write_restrictable,cloud_restrictable"`
-	ConsoleLevel          *string         `access:"write_restrictable,cloud_restrictable"`
-	ConsoleJson           *bool           `access:"write_restrictable,cloud_restrictable"`
-	EnableColor           *bool           `access:"write_restrictable,cloud_restrictable"` // telemetry: none
-	EnableFile            *bool           `access:"write_restrictable,cloud_restrictable"`
-	FileLevel             *string         `access:"write_restrictable,cloud_restrictable"`
-	FileJson              *bool           `access:"write_restrictable,cloud_restrictable"`
-	FileLocation          *string         `access:"write_restrictable,cloud_restrictable"`
-	AdvancedLoggingConfig json.RawMessage `access:"write_restrictable,cloud_restrictable"`
+	EnableConsole         *bool   `access:"write_restrictable,cloud_restrictable"`
+	ConsoleLevel          *string `access:"write_restrictable,cloud_restrictable"`
+	ConsoleJson           *bool   `access:"write_restrictable,cloud_restrictable"`
+	EnableColor           *bool   `access:"write_restrictable,cloud_restrictable"` // telemetry: none
+	EnableFile            *bool   `access:"write_restrictable,cloud_restrictable"`
+	FileLevel             *string `access:"write_restrictable,cloud_restrictable"`
+	FileJson              *bool   `access:"write_restrictable,cloud_restrictable"`
+	FileLocation          *string `access:"write_restrictable,cloud_restrictable"`
+	AdvancedLoggingConfig *string `access:"write_restrictable,cloud_restrictable"`
 }
 
 func (s *NotificationLogSettings) SetDefaults() {
@@ -1371,7 +1371,7 @@ func (s *NotificationLogSettings) SetDefaults() {
 	}
 
 	if s.AdvancedLoggingConfig == nil {
-		s.AdvancedLoggingConfig = []byte("{}")
+		s.AdvancedLoggingConfig = NewString("")
 	}
 }
 

services/telemetry/telemetry.go

@@ -503,7 +503,7 @@ func (ts *TelemetryService) trackConfig() {
 		"file_json":                cfg.LogSettings.FileJson,
 		"enable_webhook_debugging": cfg.LogSettings.EnableWebhookDebugging,
 		"isdefault_file_location":  isDefault(cfg.LogSettings.FileLocation, ""),
-		"advanced_logging_config":  len(cfg.LogSettings.AdvancedLoggingConfig) != 0,
+		"advanced_logging_config":  *cfg.LogSettings.AdvancedLoggingConfig != "",
 	})
 
 	ts.SendTelemetry(TrackConfigAudit, map[string]any{
@@ -513,7 +513,7 @@ func (ts *TelemetryService) trackConfig() {
 		"file_max_backups":        *cfg.ExperimentalAuditSettings.FileMaxBackups,
 		"file_compress":           *cfg.ExperimentalAuditSettings.FileCompress,
 		"file_max_queue_size":     *cfg.ExperimentalAuditSettings.FileMaxQueueSize,
-		"advanced_logging_config": len(cfg.ExperimentalAuditSettings.AdvancedLoggingConfig) != 0,
+		"advanced_logging_config": *cfg.ExperimentalAuditSettings.AdvancedLoggingConfig != "",
 	})
 
 	ts.SendTelemetry(TrackConfigNotificationLog, map[string]any{
@@ -524,7 +524,7 @@ func (ts *TelemetryService) trackConfig() {
 		"file_level":              *cfg.NotificationLogSettings.FileLevel,
 		"file_json":               *cfg.NotificationLogSettings.FileJson,
 		"isdefault_file_location": isDefault(*cfg.NotificationLogSettings.FileLocation, ""),
-		"advanced_logging_config": len(cfg.NotificationLogSettings.AdvancedLoggingConfig) != 0,
+		"advanced_logging_config": *cfg.NotificationLogSettings.AdvancedLoggingConfig != "",
 	})
 
 	ts.SendTelemetry(TrackConfigPassword, map[string]any{