let the system/process metricset monitor a single PID #39620

fearful-symmetry · 2024-05-17T17:12:44Z

Proposed commit message

This adds a process.pid config flag to system/process to let the metricset monitor a single PID.

This is needed for elastic/elastic-agent#4083, as we need to let metricbeat report metrics on a single process, which this metricset currently can't do.

Right now this has no documentation, as I'm not sure if we want to make this a public feature? It's a bit odd outside of our use case.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

elasticmachine · 2024-05-17T17:12:46Z

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

mergify · 2024-05-17T17:13:26Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @fearful-symmetry? 🙏.
For such, you'll need to label your PR with:

The upcoming major version of the Elastic Stack
The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

fearful-symmetry · 2024-05-17T18:03:06Z

Sigh, looks like I introduced a bug in elastic/elastic-agent-system-metrics#150, lemme deal with that...

mergify · 2024-05-17T18:25:42Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b system-process-pid upstream/system-process-pid
git merge upstream/main
git push upstream system-process-pid

## What does this PR do? This fixes a small bug introduced in #150, where we weren't setting the `ProcState` object while creating the root event. This is needed for elastic/beats#39620 ## Why is it important? The previous PR adds fields that weren't in events before, this fixes that. ## Checklist - [x] My code follows the style guidelines of this project - [ ] I have commented my code, particularly in hard-to-understand areas - [x] I have added tests that prove my fix is effective or that my feature works - [ ] I have added an entry in `CHANGELOG.md`

elasticmachine · 2024-05-17T21:02:02Z

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

faec

Looks good, one naming nit

faec · 2024-05-21T12:40:13Z

metricbeat/module/system/process/config.go

@@ -32,6 +32,8 @@ type Config struct {
 	IncludeCPUTicks bool                     `config:"process.include_cpu_ticks"`
 	IncludePerCPU   bool                     `config:"process.include_per_cpu"`
 	CPUTicks        *bool                    `config:"cpu_ticks"` // Deprecated
+	// Pid, if set, will override the `processes` config, and only monitor a single process.
+	Pid int `config:"process.pid"`


Not a blocker, but what if the config field was just pid instead of process.pid? It seems a little confusing having two mutually exclusive top-level fields to specify the target, but one of them lives in a subobject for other behavior settings.

metricbeat/module/system/process/process.go

cmacknz · 2024-05-21T16:43:57Z

Right now this has no documentation, as I'm not sure if we want to make this a public feature? It's a bit odd outside of our use case.

Leave it this way for now, if someone asks us specifically for this we can re-evaluate. We do not want to be limited in our ability to change or remove this in the future unnecessarily.

mergify · 2024-05-21T17:39:15Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b system-process-pid upstream/system-process-pid
git merge upstream/main
git push upstream system-process-pid

…c#39620)" This reverts commit 43d5dd4.

…" (#39714) This reverts commit 43d5dd4.

pchila · 2024-05-24T11:08:15Z

@fearful-symmetry I reverted this PR merge on main due to a linking problem, could you please have a look and check it again?

ycombinator · 2024-05-24T14:27:07Z

@fearful-symmetry I reverted this PR merge on main due to a linking problem, could you please have a look and check it again?

@fearful-symmetry this might be helpful: #39714 (comment)

fearful-symmetry · 2024-05-24T14:28:24Z

@ycombinator yup, looking at things now

fearful-symmetry · 2024-05-24T14:29:32Z

a thought for later: why wasn't this caught in PR? Do we not have any tests that are building agentbeat?

…ic#39620)" (elastic#39714) This reverts commit 05c5957.

…update to `go-systemd` (#39730) * Reapply "let the system/process metricset monitor a single PID (#39620)" (#39714) This reverts commit 05c5957. * reapply single-pid changes, tinker with go-systemd dep * fix deps

fearful-symmetry added 4 commits May 13, 2024 09:59

init commit

aaea360

Merge remote-tracking branch 'upstream/main' into system-process-pid

d88574d

fixing versions

ca1ea56

Merge remote-tracking branch 'upstream/main' into system-process-pid

560fcad

fearful-symmetry added the Team:Elastic-Agent Label for the Agent team label May 17, 2024

fearful-symmetry self-assigned this May 17, 2024

fearful-symmetry requested review from a team as code owners May 17, 2024 17:12

fearful-symmetry requested review from AndersonQ and leehinman May 17, 2024 17:12

botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels May 17, 2024

add comments

b420df6

fearful-symmetry added 2 commits May 17, 2024 10:16

add notice

6e5f1ad

linter

96e0bc7

fearful-symmetry mentioned this pull request May 17, 2024

Use pointer for processRootevent elastic/elastic-agent-system-metrics#153

Merged

4 tasks

fearful-symmetry added 2 commits May 17, 2024 13:33

update deps

f2951a7

Merge remote-tracking branch 'upstream/main' into system-process-pid

8825fcf

ycombinator added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label May 17, 2024

pierrehilbert requested review from faec and removed request for AndersonQ May 21, 2024 07:50

faec approved these changes May 21, 2024

View reviewed changes

change log line

16ea249

Merge remote-tracking branch 'upstream/main' into system-process-pid

32a22f8

fearful-symmetry added 2 commits May 21, 2024 10:41

Merge remote-tracking branch 'upstream/main' into system-process-pid

b1f1a18

fix mod

cd52564

fearful-symmetry mentioned this pull request May 21, 2024

Allow agent to monitor endpoint elastic/elastic-agent#4789

Merged

7 tasks

leehinman approved these changes May 22, 2024

View reviewed changes

Merge branch 'main' into system-process-pid

0dc3e83

jlind23 approved these changes May 23, 2024

View reviewed changes

fearful-symmetry merged commit 43d5dd4 into elastic:main May 23, 2024
105 checks passed

pchila added a commit to pchila/beats that referenced this pull request May 24, 2024

Revert "let the system/process metricset monitor a single PID (elasti…

f6a5931

…c#39620)" This reverts commit 43d5dd4.

pchila mentioned this pull request May 24, 2024

Revert "let the system/process metricset monitor a single PID (#39620)" #39714

Merged

6 tasks

pchila added a commit that referenced this pull request May 24, 2024

Revert "let the system/process metricset monitor a single PID (#39620)…

05c5957

…" (#39714) This reverts commit 43d5dd4.

fearful-symmetry added a commit to fearful-symmetry/beats that referenced this pull request May 24, 2024

Reapply "let the system/process metricset monitor a single PID (elast…

085435b

…ic#39620)" (elastic#39714) This reverts commit 05c5957.

fearful-symmetry mentioned this pull request May 24, 2024

Reapply single-PID adapation for metricbeat's system/process, revert update to go-systemd #39730

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

let the system/process metricset monitor a single PID #39620

let the system/process metricset monitor a single PID #39620

fearful-symmetry commented May 17, 2024

elasticmachine commented May 17, 2024

mergify bot commented May 17, 2024

fearful-symmetry commented May 17, 2024

mergify bot commented May 17, 2024

elasticmachine commented May 17, 2024

faec left a comment

faec May 21, 2024

cmacknz commented May 21, 2024

mergify bot commented May 21, 2024

pchila commented May 24, 2024

ycombinator commented May 24, 2024

fearful-symmetry commented May 24, 2024

fearful-symmetry commented May 24, 2024

let the system/process metricset monitor a single PID #39620

let the system/process metricset monitor a single PID #39620

Conversation

fearful-symmetry commented May 17, 2024

Proposed commit message

Checklist

elasticmachine commented May 17, 2024

mergify bot commented May 17, 2024

fearful-symmetry commented May 17, 2024

mergify bot commented May 17, 2024

elasticmachine commented May 17, 2024

faec left a comment

Choose a reason for hiding this comment

faec May 21, 2024

Choose a reason for hiding this comment

cmacknz commented May 21, 2024

mergify bot commented May 21, 2024

pchila commented May 24, 2024

ycombinator commented May 24, 2024

fearful-symmetry commented May 24, 2024

fearful-symmetry commented May 24, 2024