- Notifications
You must be signed in to change notification settings - Fork18
feat: write agent output to a temporary file#422
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This pull request has been linked toClubhouse Story #16783: Modify the coder-cli agent to save to a log as well as stderr. |
coveralls commentedSep 2, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Pull Request Test Coverage Report forBuild 1194765666
💛 -Coveralls |
| sloghuman.Sink(os.Stderr), | ||
| } | ||
| file, err := os.OpenFile(filepath.Join(os.TempDir(), "coder-agent.log"), os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
it didn't seem like the file we write needs to be hidden, so I made it a not-dotfile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Should be consistent for the other log files in that case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Which other log files do we have? I think just a code-server one right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
code-server and envagent, they're in environment_stages.go
internal/cmd/agent.go Outdated
| file, err := os.OpenFile(filepath.Join(os.TempDir(), "coder-agent.log"), os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600) | ||
| if err == nil && file != nil { | ||
| sinks = append(sinks, slogjson.Sink(file)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
slogjson is very hard to read, and we don't have automated ingestion of this log file into stackdriver so I'd rather stick with sloghuman for now. We can change this later with no consequences
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We don't have tools for consuming slogjson output and converting to sloghuman output? The slogjson output seems to have more info (e.g. full paths to files) and being machine-readable does seem potentially useful.
I don't have strong feelings about it though, I'll change to sloghuman 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeah that's the only difference between them, they have the same details otherwise. coder-cli doesn't have many files so I'm not too worried about not being able to find the files due to conflicts
| log := slog.Make(sinks...).Leveled(slog.LevelDebug) | ||
| if err != nil { | ||
| log.Info(ctx, "failed to open agent log file", slog.Error(err)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Should this be anerror log?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
maybe, but I don't expect it to happen for coder workspaces and everything would be logged to our console anyways
(cherry picked from commit6389faf)
Uh oh!
There was an error while loading.Please reload this page.
How I tested this
Running the agent, the log data is written to a file as well (and truncated/re-created on subsequent starts):
If there's a permission error preventing the file frmo being truncated, a log message will be output to stderr (simulated by chowning the file to root):