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

Experiment with structured logger #5723

Closed

Conversation

andrewkroh
Copy link
Member

This is a proof of concept WIP to add structured logging support to libbeat. This is powered by uber-go/zap (https://github.com/uber-go/zap).

I'm opening this so we can discuss if we want to pursue this further.

Some new features that will be possible (on top of structured logging):

  • Test cases can access logs via an in-memory store
  • Much more configurability w.r.t. logging. We'll need to decide if we want to expose this.
  • We can make it possible to adjust the levels and selectors at runtime via HTTP.

This is a proof of concept WIP to add structured logging support to libbeat. This is powered by uber-go/zap (https://github.com/uber-go/zap).

I'm opening this so we can discuss if we want to pursue this further.

Some new features that will be possible (on top of structured logging):

- Test cases can access logs via an in-memory store
- Much more configurability w.r.t. logging. We'll need to decide if we want to expose this.
- We can make it possible to adjust the levels and selectors at runtime via HTTP.
@andrewkroh andrewkroh added discuss Issue needs further discussion. libbeat labels Nov 27, 2017
@@ -0,0 +1,243 @@
package file
Copy link
Member Author

Choose a reason for hiding this comment

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

I moved the FileRotator over to the common/file package because it's a reusable piece of code (file output and logging). And I refactored it. Plus it should be renamed b/c it stutters.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not even sure if log rotation should be a task of the beat itself or an external log rotation tool. We should definitively revisit how it works.

Copy link
Member Author

Choose a reason for hiding this comment

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

We should make it possible for beats to work with an external log rotator (logrotate). The beat needs to be able to accept a signal from logrotate that triggers the logger to write to a new file. We can make it a requirement to be logrotate compatible and make this happen.

Overall I like our current behavior because it just works (especially on Windows where there is no built it tool for rotating).

@@ -0,0 +1,157 @@
package monitoring
Copy link
Member Author

Choose a reason for hiding this comment

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

I moved this code the logs the metrics out of the logp package. Separation of concerns and all...

I just dropped it in the monitoring package and hacked it to make it work. I was wondering it it would make sense to have a libbeat/monitoring/report/log package that contains this code for reporting the metrics to the log?

err = out.rotator.CheckIfConfigSane()
if err != nil {
return err
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is an example of how the structured logger would be used.

@andrewkroh andrewkroh closed this Dec 15, 2017
@ruflin
Copy link
Contributor

ruflin commented Dec 17, 2017

@andrewkroh Seeing you closed this one, I wonder if you have any follow ups in mind?

@andrewkroh
Copy link
Member Author

andrewkroh commented Dec 18, 2017

Follow-up is #5901. I split this into a few PRs. These ones are already merged: #5882 #5885

@andrewkroh andrewkroh deleted the feature/structured-logging-zap branch January 17, 2018 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issue needs further discussion. libbeat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants