fleet icon indicating copy to clipboard operation
fleet copied to clipboard

logwriter: needs peer review

Open groob opened this issue 6 years ago • 1 comments

I created a new tag for these kinds of issues.

In the process of fixing #1617 I found the logwriter package a bit unclear. It would be great to pair on this and add documentation/refactor/simplify this package.

Current questions/concerns:

  1. is Close necessary? it's never called outside of tests.

  2. is the buffered writer appropriate here? the flush method is called every time a request calls the write method. So why not just call write on the file directly?

  3. Should the logWriter wrap the lumberjack initialization and expose a ...Option which allows configuring the options from: https://github.com/kolide/fleet/blob/7577c1e3b4f386fea1b0b923acdf1c5f2462b61f/server/service/service.go#L54-L60

  4. Should there be a debug logger added as a config param?

@zwass want to pair on this sometime?

groob avatar Nov 15 '17 16:11 groob

For (2) on top of #1693 one of the things I would like to do is to decouple of the writing of the log data from SubmitLogs handler, so perhaps having periodic flushing for remotes, and direct writes for local (file) would make sense ?

Exposing the config for (3) would be easy to add as well.

thorduri avatar Feb 01 '18 13:02 thorduri