[Auditbeat] Update process metricset #9139
Conversation
|
Pinging @elastic/secops |
| } | ||
|
|
||
| func (ms *MetricSet) getProcessInfos() ([]*ProcessInfo, error) { | ||
| // TODO: Implement Processes() in go-sysinfo |
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".
| // e.g. https://github.com/elastic/go-sysinfo/blob/master/providers/darwin/process_darwin_amd64.go#L41 | ||
| pids, err := process.Pids() | ||
| if err != nil { | ||
| return nil, errors.Wrap(err, "Failed to fetch the list of PIDs") |
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, |
cwurm
force-pushed
the
processes_to_process
branch
from
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: