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.

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

structured logs and logfile rollover features #2311

Merged
merged 7 commits into from
Dec 19, 2019
Merged
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
Apply configuration to audit log
sparrc committed Dec 18, 2019

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit 18bf7887d1816597ccae33514c6e06c2abe022a5
38 changes: 38 additions & 0 deletions agent/logger/audit/audit_log.go
Original file line number Diff line number Diff line change
@@ -15,11 +15,23 @@ package audit

import (
"fmt"
"strconv"

"github.com/aws/amazon-ecs-agent/agent/config"
"github.com/aws/amazon-ecs-agent/agent/logger"
"github.com/aws/amazon-ecs-agent/agent/logger/audit/request"
)

type AuditLogger interface {
Log(r request.LogRequest, httpResponseCode int, eventType string)
GetContainerInstanceArn() string
GetCluster() string
}

type InfoLogger interface {
Info(i ...interface{})
}

type auditLog struct {
containerInstanceArn string
cluster string
@@ -62,3 +74,29 @@ func (a *auditLog) GetCluster() string {
func (a *auditLog) GetContainerInstanceArn() string {
return a.containerInstanceArn
}

func AuditLoggerConfig(cfg *config.Config) string {
config := `
<seelog type="asyncloop" minlevel="info">
<outputs formatid="main">
<console />`
if cfg.CredentialsAuditLogFile != "" {
if logger.Config.RolloverType == "size" {
config += `
<rollingfile filename="` + cfg.CredentialsAuditLogFile + `" type="size"
maxsize="` + strconv.Itoa(int(logger.Config.MaxFileSizeMB*1000000)) + `" archivetype="none" maxrolls="` + strconv.Itoa(logger.Config.MaxRollCount) + `" />`
} else {
config += `
<rollingfile filename="` + cfg.CredentialsAuditLogFile + `" type="date"
datepattern="2006-01-02-15" archivetype="none" maxrolls="` + strconv.Itoa(logger.Config.MaxRollCount) + `" />`
}
}
config += `
</outputs>
<formats>
<format id="main" format="%Msg%n" />
</formats>
</seelog>
`
return config
}
26 changes: 0 additions & 26 deletions agent/logger/audit/interface.go

This file was deleted.

35 changes: 0 additions & 35 deletions agent/logger/audit/seelog_config.go

This file was deleted.

64 changes: 32 additions & 32 deletions agent/logger/log.go
Original file line number Diff line number Diff line change
@@ -40,16 +40,16 @@ const (
)

type logConfig struct {
RolloverType string
MaxRollCount int
MaxFileSizeMB float64
logfile string
level string
rolloverType string
outputFormat string
maxRollCount int
maxFileSizeMB float64
sync.Mutex
lock sync.Mutex
}

var config *logConfig
var Config *logConfig

func logfmtFormatter(params string) seelog.FormatterFunc {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a piece of example log file in the pr description? It will be more straightforward for us to know what the log look like now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure!

return func(message string, level seelog.LogLevel, context seelog.LogContextInterface) interface{} {
@@ -76,19 +76,19 @@ func reloadConfig() {

func seelogConfig() string {
c := `
<seelog type="asyncloop" minlevel="` + config.level + `">
<outputs formatid="` + config.outputFormat + `">
<seelog type="asyncloop" minlevel="` + Config.level + `">
<outputs formatid="` + Config.outputFormat + `">
<console />`
c += platformLogConfig()
if config.logfile != "" {
if config.rolloverType == "size" {
if Config.logfile != "" {
if Config.RolloverType == "size" {
c += `
<rollingfile filename="` + config.logfile + `" type="size"
maxsize="` + strconv.Itoa(int(config.maxFileSizeMB*1000000)) + `" archivetype="none" maxrolls="` + strconv.Itoa(config.maxRollCount) + `" />`
<rollingfile filename="` + Config.logfile + `" type="size"
maxsize="` + strconv.Itoa(int(Config.MaxFileSizeMB*1000000)) + `" archivetype="none" maxrolls="` + strconv.Itoa(Config.MaxRollCount) + `" />`
} else {
c += `
<rollingfile filename="` + config.logfile + `" type="date"
datepattern="2006-01-02-15" archivetype="none" maxrolls="` + strconv.Itoa(config.maxRollCount) + `" />`
<rollingfile filename="` + Config.logfile + `" type="date"
datepattern="2006-01-02-15" archivetype="none" maxrolls="` + strconv.Itoa(Config.MaxRollCount) + `" />`
}
}
c += `
@@ -114,50 +114,50 @@ func SetLevel(logLevel string) {
parsedLevel, ok := levels[strings.ToLower(logLevel)]

if ok {
config.Lock()
defer config.Unlock()
config.level = parsedLevel
Config.lock.Lock()
defer Config.lock.Unlock()
Config.level = parsedLevel
reloadConfig()
}
}

// GetLevel gets the log level
func GetLevel() string {
config.Lock()
defer config.Unlock()
Config.lock.Lock()
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't read lock good enough here since we are just reading here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes but I am using just a plain sync.Mutex....a RWMutex seemed like overkill given the low-throughput expectation here

defer Config.lock.Unlock()

return config.level
return Config.level
}

func init() {
fierlion marked this conversation as resolved.
Show resolved Hide resolved
config = &logConfig{
Config = &logConfig{
logfile: os.Getenv(LOGFILE_ENV_VAR),
level: DEFAULT_LOGLEVEL,
rolloverType: DEFAULT_ROLLOVER_TYPE,
RolloverType: DEFAULT_ROLLOVER_TYPE,
outputFormat: DEFAULT_OUTPUT_FORMAT,
maxFileSizeMB: DEFAULT_MAX_FILE_SIZE,
maxRollCount: DEFAULT_MAX_ROLL_COUNT,
MaxFileSizeMB: DEFAULT_MAX_FILE_SIZE,
MaxRollCount: DEFAULT_MAX_ROLL_COUNT,
}

fierlion marked this conversation as resolved.
Show resolved Hide resolved
SetLevel(os.Getenv(LOGLEVEL_ENV_VAR))
if rolloverType := os.Getenv(LOG_ROLLOVER_TYPE_ENV_VAR); rolloverType != "" {
config.rolloverType = rolloverType
if RolloverType := os.Getenv(LOG_ROLLOVER_TYPE_ENV_VAR); RolloverType != "" {
Config.RolloverType = RolloverType
Copy link
Contributor

Choose a reason for hiding this comment

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

If the rollover type and output format are not supported, what will happen? Will it fail some time? Or agent may just run with malformat log?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seelog should fail the create the logger, which means it would fallback to the default seelog logger and log an error

}
if outputFormat := os.Getenv(LOG_OUTPUT_FORMAT_ENV_VAR); outputFormat != "" {
config.outputFormat = outputFormat
Config.outputFormat = outputFormat
}
if maxRollCount := os.Getenv(LOG_MAX_ROLL_COUNT_ENV_VAR); maxRollCount != "" {
i, err := strconv.Atoi(maxRollCount)
if MaxRollCount := os.Getenv(LOG_MAX_ROLL_COUNT_ENV_VAR); MaxRollCount != "" {
i, err := strconv.Atoi(MaxRollCount)
if err == nil {
config.maxRollCount = i
Config.MaxRollCount = i
} else {
seelog.Error("Invalid value for "+LOG_MAX_ROLL_COUNT_ENV_VAR, err)
}
}
if maxFileSizeMB := os.Getenv(LOG_MAX_FILE_SIZE_ENV_VAR); maxFileSizeMB != "" {
f, err := strconv.ParseFloat(maxFileSizeMB, 64)
if MaxFileSizeMB := os.Getenv(LOG_MAX_FILE_SIZE_ENV_VAR); MaxFileSizeMB != "" {
f, err := strconv.ParseFloat(MaxFileSizeMB, 64)
if err == nil {
config.maxFileSizeMB = f
Config.MaxFileSizeMB = f
} else {
seelog.Error("Invalid value for "+LOG_MAX_FILE_SIZE_ENV_VAR, err)
}