[Auditbeat] Update process metricset#9139
[Auditbeat] Update process metricset#9139cwurm merged 7 commits intoelastic:feature-auditbeat-hostfrom
Conversation
|
Pinging @elastic/secops |
andrewkroh
left a comment
There was a problem hiding this comment.
Will finish reviewing tomorrow.
webmat
left a comment
There was a problem hiding this comment.
Please leverage process. as much as possible, and I'll be good with the PR 👍🏼
Note that I haven't looked at the code closely nor tried the module yet. I'll do that next week, when a few of the modules are available in one build.
andrewkroh
left a comment
There was a problem hiding this comment.
Overall LGTM. We need to move some of the process fields up to the process namespace before merging.
There was a problem hiding this comment.
When we merge to master we should collect a list of the open TODOs in the module and make sure we don't forget to address them. This one would be caught be CI once we have it running on darwin.
There was a problem hiding this comment.
You mean the CI will fail when there are TODO comments in the code? I've been using them quite liberally, also for minor things that we can address anytime (maybe never, to be honest).
Kind of as breadcrumbs meaning "If somebody ever touches this code again and has some spare cycles, maybe do this".
There was a problem hiding this comment.
Error strings should not be capitalized. This occurs in a few places. https://github.com/golang/go/wiki/CodeReviewComments#declaring-empty-slices
There was a problem hiding this comment.
Thanks - done. I seem to be doing this a lot, so I built myself a patched version of golint that works with errors.Wrap(f). Hopefully I caught all of them this time.
|
Fields are moved to top-level As a follow-up, |
dc62e3a to
d5c5692
Compare
Updates the `process` metricset to follow newest conventions: - Rename from `processes` to `process` - Change to single documents instead of arrays - Scheduled state reporting - Use top-level ECS fields
(Sorry for the vague name - maybe I should have done this in multiple PRs, but since this is still going into the features branch I didn't want to make it too complicated.)
Following all our discussions and decisions the initial implementation of the
processesmetricset was rather outdated (same withhostandpackages). This PR basically updates it to follow the newest conventions (esp. those laid out in the now mergedusermetricset).High-level changes:
processestoprocessEven though I used
git mvit seems Github is showingprocess.goas completely new in the PR view. Since these are just two commits it might be easier to look at them directly: