Continuation of: Adriansr fix mb windows procs #12301 #12475
Conversation
This updates vendored elastic/gosigar to v0.10.3, which addresses an issue when listing processes under a non-privileged Windows user. For the system/process metricset, only processes belonging to the user under which Metricbeat is running were reported. For the system/process_summary metricset, process belonging to other users were reported as unknown state. This fix will make Metricbeat able to report all processes, but information about which users owns those processes will be missing. Fixes elastic#12301
…nsr/beats into adriansr-fix_mb_windows_procs_12301
narph
added
Team:Integrations
| @@ -460,6 +460,35 @@ func (procStats *Stats) GetOne(pid int) (common.MapStr, error) { | |||
| return e, nil | |||
| } | |||
|
|
|||
| func (procStats *Stats) getSingleProcess1(pid int, newProcs ProcsMap) (*Process, error) { | |||
There was a problem hiding this comment.
Just curious, why not just add the return error on original function getSingleProcess? 😬
There was a problem hiding this comment.
@kaiyan-sheng , I have updated the description with more info on this, this PR was not meant to be reviewed, not sure why it was moved to inbox, I have set it back to in progress.
…sigar pr will follow), test only
CHANGELOG.next.asciidoc
Outdated
| @@ -145,6 +145,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d | |||
| - Validate that kibana/status metricset cannot be used when xpack is enabled. {pull}12264[12264] | |||
| - Ignore prometheus metrics when their values are NaN or Inf. {pull}12084[12084] {issue}10849[10849] | |||
| - In the kibana/stats metricset, only log error (don't also index it) if xpack is enabled. {pull}12265[12265] | |||
| - Fix an issue listing all processes when run under Windows as a non-privileged user. {issue}12301[12301] {pull}12305[12305] | |||
There was a problem hiding this comment.
This will need updating with this PR number
| @@ -483,7 +483,7 @@ func (procStats *Stats) getSingleProcess(pid int, newProcs ProcsMap) *Process { | |||
|
|
|||
| err = process.getDetails(procStats.isWhitelistedEnvVar) | |||
| if err != nil { | |||
| logp.Err("Error getting process details. pid=%d: %v", process.Pid, err) | |||
| logp.Debug("Error getting details for process %s with pid=%s: %v", process.Name, process.Pid, err) | |||
There was a problem hiding this comment.
logp.Debug will need a selector as first parameter, I'm also curious, I guess you changed this because it would be too noisy for non privileged users?
There was a problem hiding this comment.
@exekias, done, somehow, I missed it previously. As briefly mentioned in the description, the initial change (to report all processes , but information about which users owns those processes will be missing #12305) will result in a access denied errors in process.getDetails. This will break some of the tests in the CI as they did not expect any error type messages and indeed it will be noisy for the non privileged users. I have made sure the behavior is consistent in getsingleprocess function and return debug messages at all steps.
|
Thanks @narph I think this should be backported to 6.8.1 and 7.2.x |
…#12475) * [Metricbeat] Fix system/process* metricsets under Windows This updates vendored elastic/gosigar to v0.10.3, which addresses an issue when listing processes under a non-privileged Windows user. For the system/process metricset, only processes belonging to the user under which Metricbeat is running were reported. For the system/process_summary metricset, process belonging to other users were reported as unknown state. This fix will make Metricbeat able to report all processes, but information about which users owns those processes will be missing. Fixes elastic#12301 * Added test func in order to provide more information on the failing match * Fix build error * Removed test func, correcting access rights in sigar_windows file (gosigar pr will follow), test only * Revert test changes, return debug message for process.getDetails * Replaced the PR number in the changelog * Adding selector to log debug message * Wrong type for pid in debug message (cherry picked from commit 08b5c38)
Continuation of #12305, because of the initial change, getSingleProcess will not result in a debug message and exit but it will continue to execute the next func and by the looks of it, when running process.getDetails(procStats.isWhitelistedEnvVar) and fail the log message is not debug but error.
Temporary added a mock getSingleProcess1 func to test #12301 (comment) (more info on the process id failing) since getSingleProcess is being called by several functions.(removed now)
Changed the process.getDetails fail message from error to debug to be consistent to the previous steps in the getSingleProcess function.