set up windows event logger first in main #1672
Conversation
| fmt.Fprintf(os.Stderr, "error creating system logger: %v\n", err) | ||
| os.Exit(1) | ||
| } | ||
| defer logCloser.Close() |
There was a problem hiding this comment.
should we close this just before starting the windows svc? it have two windows event writers does not seem to be an issue 🤷
There was a problem hiding this comment.
Maybe, yeah? We might not want to do the defer anyway since defers won't execute when we exit with os.Exit, which we do a lot in this file.
I think it'd be ideal if it were easy to pass this system slogger into runWindowsSvc, but that looks like it would be kind of an annoying refactor to have to do...
There was a problem hiding this comment.
yea, it would be an annoying refactor, also weird passing around a closer
There was a problem hiding this comment.
I'm sure we want the refactor. I'm less sure we want it today (I'm not sure how hard it would be, it's just adding slogger as a parameter to the subcommands, right?)
There was a problem hiding this comment.
seems like we would also need to pass around the closer within main.go if we don't want to use defer .... I'm torn between wanting clean code and wanting to make sure we close the handler before we exit
There was a problem hiding this comment.
I bet we could refactor out the os.Exit calls and use defer as expected. Or at least consolidate them and be clever. I wouldn't want to pass closers around.
There was a problem hiding this comment.
we could do something like
func main() {
if err := runMain(); err != nil {
os.Exit(1)
}
}
func runMain() error {
// do stuff
// return an err or nil
}There was a problem hiding this comment.
Yeah, that's not bad. I was thinking of something much more complicated:
var exitDeferred []func
func osExit(ret int) {
for _, fn := range exitDeferred {
fn()
}
os.Exit(ret)
}
func main() {
exitDeferred = append(exitDeferred, ...)
osExit(0)
}
There was a problem hiding this comment.
I think both of these are good options 👍 since we already have 2 other defer funcs in here that we'd probably want to move around too, I'm fine w punting this refactor to a subsequent PR in order to merge this one
There was a problem hiding this comment.
Adding system multislogger to subcommands: #1674
| fmt.Fprintf(os.Stderr, "error creating system logger: %v\n", err) | ||
| os.Exit(1) | ||
| } | ||
| defer logCloser.Close() |
There was a problem hiding this comment.
Maybe, yeah? We might not want to do the defer anyway since defers won't execute when we exit with os.Exit, which we do a lot in this file.
I think it'd be ideal if it were easy to pass this system slogger into runWindowsSvc, but that looks like it would be kind of an annoying refactor to have to do...
| if !windows.GetCurrentProcessToken().IsElevated() { | ||
| syslogger := defaultSystemSlogger() |
There was a problem hiding this comment.
This is an interesting idea. Is the intent to try to capture people running in a terminal? Im game for trying this. I don't know if we have better rope to know what our caller is
| fmt.Fprintf(os.Stderr, "error creating system logger: %v\n", err) | ||
| os.Exit(1) | ||
| } | ||
| defer logCloser.Close() |
There was a problem hiding this comment.
Yeah, that's not bad. I was thinking of something much more complicated:
var exitDeferred []func
func osExit(ret int) {
for _, fn := range exitDeferred {
fn()
}
os.Exit(ret)
}
func main() {
exitDeferred = append(exitDeferred, ...)
osExit(0)
}
Set's up system slogger to log to windows events if running as windows elevated user, first thing in main.
You can see the logs with powershell via
They will now contain the "started kolide launcher" msg