cgroup logging with slog #203
Conversation
…onfig. Updated log file cleaning.
src/common/logger.go
Outdated
| func LoadLoggers() error { | ||
| if Conf.Trace.Enable_JSON == true { | ||
| level := new(slog.LevelVar) | ||
| if Conf.Trace.Cgroups == "INFO" { |
There was a problem hiding this comment.
Please create a level string parsing function. It should not be case sensitive.
"" and "off" should mean it's off. Level is just an int, so we can use LevelError+1 for off, since no logs will be high enough threshold.
src/worker/sandbox/cgroups/cgroup.go
Outdated
| msg := fmt.Sprintf(format, args...) | ||
| log.Printf("%s [CGROUP %s: %s]", strings.TrimRight(msg, "\n"), cg.pool.Name, cg.name) | ||
| } | ||
| logger slog.Logger |
There was a problem hiding this comment.
We'll be typing this a lot, so lets just have "log" instead of "logger"
src/worker/sandbox/cgroups/pool.go
Outdated
| pool := &CgroupPool{ | ||
| Name: path.Base(path.Dir(common.Conf.Worker_dir)) + "-" + name, | ||
| ready: make(chan *CgroupImpl, CGROUP_RESERVE), | ||
| recycled: make(chan *CgroupImpl, CGROUP_RESERVE), | ||
| quit: make(chan chan bool), | ||
| nextID: 0, | ||
| logger: common.CgTopLogger, |
There was a problem hiding this comment.
why not just set it to what you want instead of immediately updating it after creating the struct?
src/worker/sandbox/cgroups/pool.go
Outdated
| } | ||
| cg.logger = *cg.logger.With("cgroup pool", cg.pool.Name, "cgroup", cg.name) |
There was a problem hiding this comment.
can't you create if from the pools logger so you don't need to specify the pool name again?
src/worker/sandbox/sock.go
Outdated
| "sync/atomic" | ||
| "net/http" | ||
| "os" | ||
| "os/exec" | ||
| "path/filepath" | ||
| "strconv" | ||
| "strings" | ||
| //"strings" |
There was a problem hiding this comment.
just delete, don't comment stuff out
|
I left a few comments in logger.go right next to the code to make it more clear. Could you take a look at them and let me know your decision for those parts, please? Also, I am currently working on the CGroup subsystem only. Other subsystems such as Sock are not being dealt with yet for now. As soon as you approve all the changes, I will go ahead and apply them to the rest of the OLs, which can then wrap up the project. |
No description provided.