Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generalize seelog config and add setters #4048

Merged
merged 1 commit into from
Dec 4, 2023
Merged

Generalize seelog config and add setters #4048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
80 changes: 63 additions & 17 deletions agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/logger/log.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

80 changes: 63 additions & 17 deletions ecs-agent/logger/log.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,21 @@ const (
DEFAULT_LOGLEVEL_WHEN_DRIVER_SET = "off"
DEFAULT_ROLLOVER_TYPE = "date"
DEFAULT_OUTPUT_FORMAT = logFmt
DEFAULT_TIMESTAMP_FORMAT = time.RFC3339
DEFAULT_MAX_FILE_SIZE float64 = 10
DEFAULT_MAX_ROLL_COUNT int = 24
)

type logConfig struct {
RolloverType string
MaxRollCount int
MaxFileSizeMB float64
logfile string
driverLevel string
instanceLevel string
outputFormat string
lock sync.Mutex
RolloverType string
MaxRollCount int
MaxFileSizeMB float64
logfile string
driverLevel string
instanceLevel string
outputFormat string
timestampFormat string
lock sync.Mutex
}

var Config *logConfig
Expand Down Expand Up @@ -87,7 +89,7 @@ func logfmtFormatter(params string) seelog.FormatterFunc {
buf.WriteString(level.String())
buf.WriteByte(' ')
buf.WriteString("time=")
buf.WriteString(context.CallTime().UTC().Format(time.RFC3339))
buf.WriteString(context.CallTime().UTC().Format(Config.timestampFormat))
buf.WriteByte(' ')
// temporary measure to make this change backwards compatible as we update to structured logs
if strings.HasPrefix(message, structuredTxtFormatPrefix) {
Expand All @@ -112,7 +114,7 @@ func jsonFormatter(params string) seelog.FormatterFunc {
buf.WriteString(`{"level":"`)
buf.WriteString(level.String())
buf.WriteString(`","time":"`)
buf.WriteString(context.CallTime().UTC().Format(time.RFC3339))
buf.WriteString(context.CallTime().UTC().Format(Config.timestampFormat))
buf.WriteString(`",`)
// temporary measure to make this change backwards compatible as we update to structured logs
if strings.HasPrefix(message, structuredJsonFormatPrefix) {
Expand Down Expand Up @@ -234,15 +236,59 @@ func setInstanceLevelDefault() string {
return DEFAULT_LOGLEVEL
}

// SetConfigLogFile sets the default output file of the logger.
func SetConfigLogFile(logFile string) {
Config.lock.Lock()
defer Config.lock.Unlock()

Config.logfile = logFile
}

// SetConfigLogFormat sets the output format of the logger.
// e.g. json, xml, etc.
func SetConfigLogFormat(logFormat string) {
Config.lock.Lock()
defer Config.lock.Unlock()

os.Setenv(LOG_OUTPUT_FORMAT_ENV_VAR, logFormat)
Config.outputFormat = logFormat
}

// SetConfigMaxFileSizeMB sets the max file size of a log file
// in Megabytes before the logger rotates to a new file.
func SetConfigMaxFileSizeMB(maxSizeInMB float64) {
Copy link
Contributor

@amogh09 amogh09 Dec 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The existing SetLevel function takes care of reloading the seelog config but the new setter functions do not. I understand that now there are multiple setter functions, and a user might want to call the setter functions and then trigger a (re)load manually by calling InitSeeLog once but this breaks the current pattern that setter functions take care of reloading the configuration.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not make the new setter functions call reloadConfig?

Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed offline that reloadConfig doesn't help with reloading log format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Synced offline, Some setters will require a reload of the custom formatters, which is not something reloadConfig does. If we are always calling InitSeeLog after each setter anyways, the access pattern can be abstracted to call InitSeeLog manually after modifying desired config attributes

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's update the descriptions of the new setter functions to say that InitSeeLog needs to be called for the change to take effect.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can be done in a new PR.

Config.lock.Lock()
defer Config.lock.Unlock()

// Parse unit as string to set as environment variable.
strsize := strconv.FormatFloat(maxSizeInMB, 'f', -1, 64)
os.Setenv(LOG_MAX_FILE_SIZE_ENV_VAR, strsize)
Config.MaxFileSizeMB = maxSizeInMB
}

// SetTimestampFormat sets the time formatting
// for custom seelog formatters. It will expect
// a valid time format such as time.RFC3339
// or "2006-01-02T15:04:05.000".
func SetTimestampFormat(format string) {
Config.lock.Lock()
defer Config.lock.Unlock()

if format != "" {
Config.timestampFormat = format
}
}

func init() {
Config = &logConfig{
logfile: os.Getenv(LOGFILE_ENV_VAR),
driverLevel: DEFAULT_LOGLEVEL,
instanceLevel: setInstanceLevelDefault(),
RolloverType: DEFAULT_ROLLOVER_TYPE,
outputFormat: DEFAULT_OUTPUT_FORMAT,
MaxFileSizeMB: DEFAULT_MAX_FILE_SIZE,
MaxRollCount: DEFAULT_MAX_ROLL_COUNT,
logfile: os.Getenv(LOGFILE_ENV_VAR),
driverLevel: DEFAULT_LOGLEVEL,
instanceLevel: setInstanceLevelDefault(),
RolloverType: DEFAULT_ROLLOVER_TYPE,
outputFormat: DEFAULT_OUTPUT_FORMAT,
MaxFileSizeMB: DEFAULT_MAX_FILE_SIZE,
MaxRollCount: DEFAULT_MAX_ROLL_COUNT,
timestampFormat: DEFAULT_TIMESTAMP_FORMAT,
}
}

Expand Down
23 changes: 23 additions & 0 deletions ecs-agent/logger/log_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,18 @@ func TestLogfmtFormat_Structured_debug(t *testing.T) {
`, s)
}

func TestLogfmtFormat_Structured_Timestamp(t *testing.T) {
SetTimestampFormat("2006-01-02T15:04:05.000")
defer SetTimestampFormat(DEFAULT_TIMESTAMP_FORMAT)
logfmt := logfmtFormatter("")
fm := defaultStructuredTextFormatter.Format("This is my log message")
out := logfmt(fm, seelog.DebugLvl, &LogContextMock{})
s, ok := out.(string)
require.True(t, ok)
require.Equal(t, `level=debug time=2018-10-01T01:02:03.000 msg="This is my log message"
`, s)
}

func TestJSONFormat_debug(t *testing.T) {
jsonF := jsonFormatter("")
out := jsonF("This is my log message", seelog.DebugLvl, &LogContextMock{})
Expand All @@ -119,6 +131,17 @@ func TestJSONFormat_Structured_debug(t *testing.T) {
require.JSONEq(t, `{"level": "debug", "time": "2018-10-01T01:02:03Z", "msg": "This is my log message"}`, s)
}

func TestJSONFormat_Structured_Timestamp(t *testing.T) {
SetTimestampFormat("2006-01-02T15:04:05.000")
defer SetTimestampFormat(DEFAULT_TIMESTAMP_FORMAT)
jsonF := jsonFormatter("")
fm := defaultStructuredJsonFormatter.Format("This is my log message")
out := jsonF(fm, seelog.DebugLvl, &LogContextMock{})
s, ok := out.(string)
require.True(t, ok)
require.JSONEq(t, `{"level": "debug", "time": "2018-10-01T01:02:03.000", "msg": "This is my log message"}`, s)
}

func TestSetLevel(t *testing.T) {
resetEnv := func() {
os.Unsetenv(LOGLEVEL_ENV_VAR)
Expand Down