-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Update logging from auditd module #6018
Update logging from auditd module #6018
Conversation
Use logp.Logger for all logging from the auditd module. I also increased the logging level for two statements because they will be useful for troubleshooting (without having to ask users to re-run with debug enabled).
return | ||
} | ||
|
||
out, err := ms.receiveEvents(reporter.Done()) | ||
if err != nil { | ||
reporter.Error(err) | ||
logp.Err("%v %v", logPrefix, err) | ||
ms.log.Errorw("Failure receiving audit events", "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.
Would be nice if this would be error.message
the same as we have in our fields.yml
.
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 would. The error is written as an object, but it doesn’t match our normal structure. See https://github.com/uber-go/zap/blob/master/zapcore/error.go#L28
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 should be possible to intercept (but with some expense) the Error types and wrap them in another type that implements https://godoc.org/go.uber.org/zap/zapcore#ObjectMarshaler to customize the marshaling.
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.
The part I'm worried if we don't do it is that someone is going to read filebeat json logs which will conflict with our own filebeat template.
@@ -269,7 +269,10 @@ func RunPushMetricSetV2(timeout time.Duration, waitEvents int, metricSet mb.Push | |||
select { | |||
case <-timer.C: | |||
return | |||
case e := <-r.eventsC: | |||
case e, ok := <-r.eventsC: |
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.
@andrewkroh Is this change related? LGTM but curious if that is related to an other issue.
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.
While testing some failure conditions I noticed the test case didn’t stop immediately and this was the fix.
Use
logp.Logger
for all logging from the auditd module. I also increased the logging level for two statements because they will be useful for troubleshooting (without having to ask users to re-run with debug enabled).