[metricbeat] - Allow metricsets to report their status via v2 protocol by VihasMakwana · Pull Request #40025 · elastic/beats

VihasMakwana · 2024-06-26T12:49:29Z

Proposed commit message

Add StatusReporter for letting individual metricsets to report their statuses.

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.

Related issues

Use cases

Screenshots

Logs

mergify · 2024-06-26T12:50:08Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @VihasMakwana? 🙏.
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

elasticmachine · 2024-06-26T16:03:45Z

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

faec

This looks good, but I don't see any actual invocations of SetStatusReporter, which seems to be required for the other changes to have an effect. Am I missing something, or is that coming in a later PR?

metricbeat/mb/mb.go

VihasMakwana · 2024-07-15T19:42:35Z

This looks good, but I don't see any actual invocations of SetStatusReporter, which seems to be required for the other changes to have an effect. Am I missing something, or is that coming in a later PR?

@Fae, This part was introduced by #39209 and it's done at

beats/libbeat/cfgfile/list.go

Lines 157 to 161 in 1a6318d

    
           if config.StatusReporter != nil { 
        
           	if runnerWithStatus, ok := runner.(status.WithStatusReporter); ok { 
        
           		runnerWithStatus.SetStatusReporter(config.StatusReporter) 
        
           	} 
        
           }

in a generic way for all the runners.

faec

Thanks for clarifying, looks good to me!

VihasMakwana · 2024-07-16T17:26:20Z

Thanks I wasn't following that. I think it would be a more obvious if we set state based on the return value of Fetch (certainly it would be for me). I think it also has more obvious behavior with respect to when the status will be cleared - it will be cleared on the first call to Fetch that doesn't return an error.

@cmacknz Let me know how it looks to you now and if you have any more comments regarding this.

@faec Thanks for reviewing!

cmacknz · 2024-07-16T21:19:37Z

The changes themselves LGTM, but I am not getting the result I expected when I test this manually. What I did:

Create an ubuntu VM with multipass and an 8.15.0-SNAPSHOT ESS deployment on staging.elastic.co.
Build agentbeat from this branch with SNAPSHOT=true PLATFORMS=linux/arm64 PACKAGES=targz mage package
Transfer agentbeat into the VM: multipass transfer ./build/distributions/agentbeat-8.16.0-SNAPSHOT-linux-arm64.tar.gz $VM_NAME:/home/ubuntu
Shell into the VM with multipass shell
Download latest 8.15.0 BC agent: curl -LO https://staging.elastic.co/8.15.0-a47a2107/downloads/beats/elastic-agent/elastic-agent-8.15.0-linux-arm64.tar.gz
Extract the agent: tar xvfz elastic-agent-8.15.0-linux-arm64.tar.gz
Overwrite the packaged agentbeat with the one from this branch: cp ./agentbeat-8.16.0-SNAPSHOT-linux-arm64/agentbeat ./elastic-agent-8.15.0-linux-arm64/data/elastic-agent-475d24/components/
Install the agent as unprivileged using the default Fleet policy to collect system metrics: ./elastic-agent install --url=X --enrollment-token=Y --unprivileged -f
Create a simple script and run it as root to guarantee there is some PID running under root that we should fail to get metrics from.

~$ cat script.sh
while true;
do
        echo "Hello!";
        sleep 1;
done
~$ sudo ./script.sh

Observe agent remains healthy but the logs show privilege related errors, many of which are only visible at debug level.

{"log.level":"debug","@timestamp":"2024-07-16T21:04:30.148Z","message":"Non-fatal error fetching PID metrics for 30358, metrics are valid, but partial: Not enough privileges to fetch information: /io unavailable; if running inside a container, use SYS_PTRACE: error fetching IO metrics: open /proc/30358/io: permission denied","component":{"binary":"metricbeat","dataset":"elastic_agent.metricbeat","id":"system/metrics-default","type":"system/metrics"},"log":{"source":"system/metrics-default"},"log.logger":"processes","log.origin":{"file.line":268,"file.name":"process/process.go","function":"github.com/elastic/elastic-agent-system-metrics/metric/system/process.(*Stats).pidFill"},"service.name":"metricbeat","ecs.version":"1.6.0","ecs.version":"1.6.0"}
{"log.level":"debug","@timestamp":"2024-07-16T21:04:40.143Z","message":"Non-fatal error fetching PID metrics for 30357, metrics are valid, but partial: Not enough privileges to fetch information: /io unavailable; if running inside a container, use SYS_PTRACE: error fetching IO metrics: open /proc/30357/io: permission denied","component":{"binary":"metricbeat","dataset":"elastic_agent.metricbeat","id":"system/metrics-default","type":"system/metrics"},"log":{"source":"system/metrics-default"},"log.logger":"processes","log.origin":{"file.line":268,"file.name":"process/process.go","function":"github.com/elastic/elastic-agent-system-metrics/metric/system/process.(*Stats).pidFill"},"service.name":"metricbeat","ecs.version":"1.6.0","ecs.version":"1.6.0"}
{"log.level":"debug","@timestamp":"2024-07-16T21:05:50.143Z","message":"Non-fatal error fetching PID metrics for 30358, metrics are valid, but partial: Not enough privileges to fetch information: /io unavailable; if running inside a container, use SYS_PTRACE: error fetching IO metrics: open /proc/30358/io: permission denied","component":{"binary":"metricbeat","dataset":"elastic_agent.metricbeat","id":"system/metrics-default","type":"system/metrics"},"log":{"source":"system/metrics-default"},"service.name":"metricbeat","ecs.version":"1.6.0","log.logger":"processes","log.origin":{"file.line":268,"file.name":"process/process.go","function":"github.com/elastic/elastic-agent-system-metrics/metric/system/process.(*Stats).pidFill"},"ecs.version":"1.6.0"}
{"log.level":"debug","@timestamp":"2024-07-16T21:05:50.143Z","message":"Error fetching PID info for 30642, skipping: GetInfoForPid: no such process","component":{"binary":"metricbeat","dataset":"elastic_agent.metricbeat","id":"system/metrics-default","type":"system/metrics"},"log":{"source":"system/metrics-default"},"service.name":"metricbeat","ecs.version":"1.6.0","log.logger":"processes","log.origin":{"file.line":198,"file.name":"process/process.go","function":"github.com/elastic/elastic-agent-system-metrics/metric/system/process.(*Stats).pidIter"},"ecs.version":"1.6.0"}
{"log.level":"debug","@timestamp":"2024-07-16T21:06:10.055Z","message":"Non-fatal error fetching PID metrics for 30356, metrics are valid, but partial: Not enough privileges to fetch information: /io unavailable; if running inside a container, use SYS_PTRACE: error fetching IO metrics: open /proc/30356/io: permission denied","component":{"binary":"metricbeat","dataset":"elastic_agent.metricbeat","id":"system/metrics-default","type":"system/metrics"},"log":{"source":"system/metrics-default"},"log.logger":"processes","log.origin":{"file.line":268,"file.name":"process/process.go","function":"github.com/elastic/elastic-agent-system-metrics/metric/system/process.(*Stats).pidFill"},"service.name":"metricbeat","ecs.version":"1.6.0","ecs.version":"1.6.0"}
{"log.level":"debug","@timestamp":"2024-07-16T21:06:10.056Z","message":"Non-fatal error fetching PID metrics for 30357, metrics are valid, but partial: Not enough privileges to fetch information: /io unavailable; if running inside a container, use SYS_PTRACE: error fetching IO metrics: open /proc/30357/io: permission denied","component":{"binary":"metricbeat","dataset":"elastic_agent.metricbeat","id":"system/metrics-default","type":"system/metrics"},"log":{"source":"system/metrics-default"},"service.name":"metricbeat","ecs.version":"1.6.0","log.logger":"processes","log.origin":{"file.line":268,"file.name":"process/process.go","function":"github.com/elastic/elastic-agent-system-metrics/metric/system/process.(*Stats).pidFill"},"ecs.version":"1.6.0"}
{"log.level":"debug","@timestamp":"2024-07-16T21:06:10.056Z","message":"Non-fatal error fetching PID metrics for 30358, metrics are valid, but partial: Not enough privileges to fetch information: /io unavailable; if running inside a container, use SYS_PTRACE: error fetching IO metrics: open /proc/30358/io: permission denied","component":{"binary":"metricbeat","dataset":"elastic_agent.metricbeat","id":"system/metrics-default","type":"system/metrics"},"log":{"source":"system/metrics-default"},"ecs.version":"1.6.0","log.logger":"processes","log.origin":{"file.line":268,"file.name":"process/process.go","function":"github.com/elastic/elastic-agent-system-metrics/metric/system/process.(*Stats).pidFill"},"service.name":"metricbeat","ecs.version":"1.6.0"}
{"log.level":"debug","@timestamp":"2024-07-16T21:06:10.056Z","message":"Non-fatal error fetching PID metrics for 30649, metrics are valid, but partial: Not enough privileges to fetch information: /io unavailable; if running inside a container, use SYS_PTRACE: error fetching IO metrics: open /proc/30649/io: permission denied","component":{"binary":"metricbeat","dataset":"elastic_agent.metricbeat","id":"system/metrics-default","type":"system/metrics"},"log":{"source":"system/metrics-default"},"log.logger":"processes","log.origin":{"file.line":268,"file.name":"process/process.go","function":"github.com/elastic/elastic-agent-system-metrics/metric/system/process.(*Stats).pidFill"},"service.name":"metricbeat","ecs.version":"1.6.0","ecs.version":"1.6.0"}
{"log.level":"debug","@timestamp":"2024-07-16T21:06:10.056Z","message":"Non-fatal error fetching PID metrics for 30650, metrics are valid, but partial: Not enough privileges to fetch information: /io unavailable; if running inside a container, use SYS_PTRACE: error fetching IO metrics: open /proc/30650/io: permission denied","component":{"binary":"metricbeat","dataset":"elastic_agent.metricbeat","id":"system/metrics-default","type":"system/metrics"},"log":{"source":"system/metrics-default"},"service.name":"metricbeat","ecs.version":"1.6.0","log.logger":"processes","log.origin":{"file.line":268,"file.name":"process/process.go","function":"github.com/elastic/elastic-agent-system-metrics/metric/system/process.(*Stats).pidFill"},"ecs.version":"1.6.0"}
{"log.level":"debug","@timestamp":"2024-07-16T21:06:10.056Z","message":"Non-fatal error fetching PID metrics for 30651, metrics are valid, but partial: Not enough privileges to fetch information: /io unavailable; if running inside a container, use SYS_PTRACE: error fetching IO metrics: open /proc/30651/io: permission denied","component":{"binary":"metricbeat","dataset":"elastic_agent.metricbeat","id":"system/metrics-default","type":"system/metrics"},"log":{"source":"system/metrics-default"},"log.logger":"processes","log.origin":{"file.line":268,"file.name":"process/process.go","function":"github.com/elastic/elastic-agent-system-metrics/metric/system/process.(*Stats).pidFill"},"service.name":"metricbeat","ecs.version":"1.6.0","ecs.version":"1.6.0"}
{"log.level":"debug","@timestamp":"2024-07-16T21:06:10.056Z","message":"Non-fatal error fetching PID metrics for 30652, metrics are valid, but partial: Not enough privileges to fetch information: /io unavailable; if running inside a container, use SYS_PTRACE: error fetching IO metrics: open /proc/30652/io: permission denied","component":{"binary":"metricbeat","dataset":"elastic_agent.metricbeat","id":"system/metrics-default","type":"system/metrics"},"log":{"source":"system/metrics-default"},"log.logger":"processes","log.origin":{"file.line":268,"file.name":"process/process.go","function":"github.com/elastic/elastic-agent-system-metrics/metric/system/process.(*Stats).pidFill"},"service.name":"metricbeat","ecs.version":"1.6.0","ecs.version":"1.6.0"}
{"log.level":"debug","@timestamp":"2024-07-16T21:06:10.056Z","message":"Non-fatal error fetching PID metrics for 30670, metrics are valid, but partial: Not enough privileges to fetch information: /io unavailable; if running inside a container, use SYS_PTRACE: error fetching IO metrics: open /proc/30670/io: permission denied","component":{"binary":"metricbeat","dataset":"elastic_agent.metricbeat","id":"system/metrics-default","type":"system/metrics"},"log":{"source":"system/metrics-default"},"log.origin":{"file.line":268,"file.name":"process/process.go","function":"github.com/elastic/elastic-agent-system-metrics/metric/system/process.(*Stats).pidFill"},"service.name":"metricbeat","ecs.version":"1.6.0","log.logger":"processes","ecs.version":"1.6.0"}

I've attached diagnostics from the VM. The logs indicating the error is non-fatal might be what is catching us here, I think this has changed since the issue to implement this was created. Perhaps you were initially right hooking into the error in the event, which might catch these since they are non-fatal, but I still think looking at events makes it harder to see when we will go back to healthy. Regardless, this isn't actually catching the degraded state we want.

unpriv-metricbeat-diagnostics.zip

I also wonder if we could get a test to catch this, assuming I verified this correctly.

VihasMakwana · 2024-07-17T10:15:41Z

@cmacknz I see.

I think this is due to the fact that elastic-agent-system-metrics only logs the error and doesn't return them to the caller when monitoring set of processes. Example https://github.com/elastic/elastic-agent-system-metrics/blob/a9ecc37f3de53d18a85ad182718010650aad774b/metric/system/process/process.go#L263-L269

We can make use of multierr to capture the errors encountered for set of processes and pass it to the caller.

mergify · 2024-07-17T15:06:22Z

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 add-metricbeat-status-reporter upstream/add-metricbeat-status-reporter
git merge upstream/main
git push upstream add-metricbeat-status-reporter

mergify · 2024-07-23T19:46:26Z

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 add-metricbeat-status-reporter upstream/add-metricbeat-status-reporter
git merge upstream/main
git push upstream add-metricbeat-status-reporter

pkoutsovasilis · 2024-07-24T09:31:19Z

metricbeat/mb/module/wrapper.go

 		err := fetcher.Fetch(reporter.V2())
 		if err != nil {
 			reporter.V2().Error(err)
+			msw.module.UpdateStatus(status.Degraded, fmt.Sprintf("Error fetching data for metricset %s.%s: %s", msw.module.Name(), msw.MetricSet.Name(), err))


nit: use %v for the err

pkoutsovasilis · 2024-07-24T09:31:26Z

metricbeat/mb/module/wrapper.go

 		err := fetcher.Fetch(ctx, reporter.V2())
 		if err != nil {
 			reporter.V2().Error(err)
+			msw.module.UpdateStatus(status.Degraded, fmt.Sprintf("Error fetching data for metricset %s.%s: %s", msw.module.Name(), msw.MetricSet.Name(), err))


nit: use %v for the err

@pkoutsovasilis I didn't see this.
I'll raise another PR for this.
Sorry from my end!

pkoutsovasilis

Left some nits, but looks good to me

VihasMakwana · 2024-07-24T09:33:40Z

@cmacknz Screenshot of error event on Kibana:

VihasMakwana · 2024-07-24T09:34:07Z

Merging this as of now.
#40298 will take care of the non-fatal errors.

#40025) * fix: initial commit * tests: add integration test cases * fix: expand testing scenarios * fix: add comments * fix: move integration tests to system/process * cleanup * fix: ci * fix: ci and typos * chore: update changelog * fix: add helper * fix: remove extra space * fix: ci * fix: move integration tests to x-pack * fix: add null check * fix: ci * fix: remove unused code * fix: lint * fix: lint and imports * fix: ci windows * inting for windows * fix lint linux * fix: go imports * fix: switch to the generic way * chore: make error descriptive * fix: move status report after fetch * fix: typo * fix: remove nolint --------- Co-authored-by: Pierre HILBERT <pierre.hilbert@elastic.co> (cherry picked from commit c86330a)

mergify · 2024-07-31T12:09:22Z

⚠️ The sha of the head commit of this PR conflicts with #40400. Mergify cannot evaluate rules on this PR. ⚠️

fix: initial commit

f87e6bb

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jun 26, 2024

VihasMakwana added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Jun 26, 2024

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jun 26, 2024

mergify bot assigned VihasMakwana Jun 26, 2024

VihasMakwana added 2 commits June 26, 2024 18:20

tests: add integration test cases

98108a1

fix: expand testing scenarios

e37ef86

VihasMakwana force-pushed the add-metricbeat-status-reporter branch from 6c10a9f to e37ef86 Compare June 26, 2024 13:42

VihasMakwana added 3 commits June 26, 2024 19:13

fix: add comments

258c786

fix: move integration tests to system/process

d7cecfe

cleanup

feaa0fa

VihasMakwana changed the title ~~[DRAFT] - Allow metrcisets to report their status via v2 protocol~~ [metricbeat] - Allow metricsets to report their status via v2 protocol Jun 26, 2024

VihasMakwana and others added 6 commits June 26, 2024 20:02

fix: ci

5f35ae5

fix: ci and typos

ecdfa1e

chore: update changelog

6c877bb

fix: add helper

19b82c7

fix: remove extra space

52cd0ed

Merge branch 'main' into add-metricbeat-status-reporter

735c611

VihasMakwana marked this pull request as ready for review June 26, 2024 16:03

VihasMakwana requested a review from a team as a code owner June 26, 2024 16:03

VihasMakwana requested review from faec and fearful-symmetry June 26, 2024 16:03

VihasMakwana requested a review from cmacknz June 26, 2024 16:10

fix: ci

2fa72c9

VihasMakwana requested a review from a team as a code owner June 26, 2024 16:14

fix: move integration tests to x-pack

06c98f4

miltonhultgren approved these changes Jun 26, 2024

View reviewed changes

fix: add null check

bdd1464

faec reviewed Jul 15, 2024

View reviewed changes

metricbeat/mb/mb.go Outdated Show resolved Hide resolved

fix: typo

2f06c14

VihasMakwana requested a review from faec July 15, 2024 20:50

faec approved these changes Jul 16, 2024

View reviewed changes

jlind23 mentioned this pull request Jul 17, 2024

[metric/system/process] - return errors encountered while monitoring set of processes elastic/elastic-agent-system-metrics#164

Closed

Merge branch 'main' into add-metricbeat-status-reporter

abcb26a

lucabelluccini mentioned this pull request Jul 19, 2024

[Elastic Agent] The system/metrics input should report itself as degraded when it encounters a permissions error #39737

Closed

VihasMakwana requested review from a team as code owners July 23, 2024 19:45

VihasMakwana force-pushed the add-metricbeat-status-reporter branch from 210c929 to abcb26a Compare July 23, 2024 19:46

VihasMakwana mentioned this pull request Jul 23, 2024

DO NOT MERGE [metricbeat][system/process] - Handle non-fatal errors #40298

Draft

6 tasks

fix: remove nolint

6d5f22f

pkoutsovasilis reviewed Jul 24, 2024

View reviewed changes

pkoutsovasilis approved these changes Jul 24, 2024

View reviewed changes

VihasMakwana merged commit c86330a into elastic:main Jul 24, 2024

mergify bot mentioned this pull request Jul 24, 2024

[8.15](backport #40025) [metricbeat] - Allow metricsets to report their status via v2 protocol #40327

Closed

6 tasks

VihasMakwana mentioned this pull request Jul 25, 2024

Revert "[metricbeat] - Allow metricsets to report their status via v2 protocol" #40352

Merged

Conversation

VihasMakwana commented Jun 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed commit message

Checklist

Related issues

Use cases

Screenshots

Logs

Uh oh!

mergify bot commented Jun 26, 2024

Uh oh!

elasticmachine commented Jun 26, 2024

Uh oh!

faec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

VihasMakwana commented Jul 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

faec left a comment

Choose a reason for hiding this comment

Uh oh!

VihasMakwana commented Jul 16, 2024

Uh oh!

cmacknz commented Jul 16, 2024

Uh oh!

VihasMakwana commented Jul 17, 2024

Uh oh!

mergify bot commented Jul 17, 2024

Uh oh!

mergify bot commented Jul 23, 2024

Uh oh!

pkoutsovasilis Jul 24, 2024

Choose a reason for hiding this comment

Uh oh!

pkoutsovasilis Jul 24, 2024

Choose a reason for hiding this comment

Uh oh!

VihasMakwana Jul 24, 2024

Choose a reason for hiding this comment

Uh oh!

pkoutsovasilis left a comment

Choose a reason for hiding this comment

Uh oh!

VihasMakwana commented Jul 24, 2024

Uh oh!

VihasMakwana commented Jul 24, 2024

Uh oh!

mergify bot commented Jul 31, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

VihasMakwana commented Jun 26, 2024 •

edited

Loading

VihasMakwana commented Jul 15, 2024 •

edited

Loading