Add a ring-buffer reporter to libbeat by michel-laterman · Pull Request #28750 · elastic/beats

michel-laterman · 2021-11-01T21:43:21Z

What does this PR do?

Add a ring buffer reporter that when enabled will store configured
namespaces in a buffer to allow operators to view recent metrics
history. Defaults are to gather the stats namespace every 10s for 10m.
Must be explicitly enabled along with the HTTP endpoints.
The buffer endpoint is intended to be used for diagnostics reporting.

Why is it important?

We wish to gather recent metrics data with the elastic-agent diagnostics command.
The buffer endpoint can be used to collect such data.

Also once this is implemented we may more easily disable the log metrics reporting functionality.

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.

How to test this PR locally

compile a beat, and run with the following options enabled in the config:

http.enabled: true
http.buffer.enabled: true

Related issues

Relates Beats diagnostic tooling - Phase II elastic-agent#94

Note that this functionality will not be backported, the number of cases metrics could help debug are minimal.

mergify · 2021-11-01T21:43:25Z

This pull request does not have a backport label. Could you fix it @michel-laterman? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

elasticmachine · 2021-11-01T23:49:53Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-02-11T22:44:39.843+0000
Duration: 17 min 40 sec

❕ Flaky test report

No test was executed to be analysed.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

mergify · 2021-11-15T21:32:41Z

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 monitoring-ring-buffer upstream/monitoring-ring-buffer
git merge upstream/master
git push upstream monitoring-ring-buffer

Add a ring buffer reporter that when enabled will store configured namespaces in a buffer to allow operators to view recent metrics history. Defaults are to gather the stats namespace every 10s for 10m. Must be explicitly enabled, along with monitoring, and the HTTP endpoint. The buffer endpoint is intended to be used for diagnostics reporting.

botelastic · 2021-12-20T00:47:04Z

Hi!
We just realized that we haven't looked into this PR in a while. We're sorry!

We're labeling this issue as Stale to make it hit our filters and make sure we get back to it in as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1.
Thank you for your contribution!

botelastic · 2022-01-20T02:47:15Z

Hi!
We just realized that we haven't looked into this PR in a while. We're sorry!

We're labeling this issue as Stale to make it hit our filters and make sure we get back to it in as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1.
Thank you for your contribution!

ph · 2022-01-20T14:20:18Z

This is interesting @michel-laterman let me know if it's ready to look at it.

michel-laterman · 2022-01-31T20:59:06Z

Finally got around to running memory/cpu usage tests:
Running metricbeat 8.1-SNAPSHOT on ubuntu 18.04 (left) and 20.04 (right).

When http + buffer is enabled on 20.04 we see:

the mb running with the buffer has an rss size of 162-163MB (same as the 18.04 instance without enabling the buffer).
The memory usage on 18.04 generally should be disregarded as I believe the OS difference injects too much noise into the result.

Just enabling the HTTP endpoint on 20.04 we can see the rss size drop to 159-160MB (share average also drops by around 1MB):

And disabling both the buffer and endpoint leads to an rss size of around 159MB:

CPU usage does not meaningfully change with the buffer enabled.
With default settings enabling the buffer does not lead to any major changes in memory usage. however if more namespaces are added to the buffer, or if extra process information is collected (through the system module), the numbers can increase

elasticmachine · 2022-02-08T17:23:30Z

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

blakerouse

Overall this looks really good.

It has great test coverage and it's easy to understand. I feel like I should have found something to comment on, but its really good, nothing I see!

ph · 2022-02-09T18:25:56Z

Going to restart the tests, it been a few months the PR was created.

/test

ph · 2022-02-09T19:06:42Z

/test

ph

Look good to me, adding only a small nit comment.

ph · 2022-02-09T19:36:56Z

libbeat/monitoring/report/buffer/buffer.go

+
+// ringBuffer is a buffer with a fixed number of items that can be tracked.
+//
+// we assume that the size of the buffer is greater than one.


Suggested change

// we assume that the size of the buffer is greater than one.

// We assume that the size of the buffer is greater than one.

``

ph · 2022-02-09T19:40:53Z

libbeat/monitoring/report/buffer/reporter.go

+func (r *reporter) Stop() {
+	close(r.done)
+	r.wg.Wait()
+	// Clear entries?


Remove comment so there is no confusion on the godoc of the function, no need to clear the ring buffer there.

efd6 · 2022-02-09T21:48:16Z

libbeat/api/server.go

+			switch r.(type) {
+			case error:
+				err = r.(error)
+			case string:
+				err = fmt.Errorf(r.(string))
+			default:
+				err = fmt.Errorf("handle attempted to panic with %v", r)
+			}


Suggested change

switch r.(type) {

case error:

err = r.(error)

case string:

err = fmt.Errorf(r.(string))

default:

err = fmt.Errorf("handle attempted to panic with %v", r)

}

switch r := r.(type) {

case error:

err = r

case string:

err = errors.New(r)

default:

err = fmt.Errorf("handle attempted to panic with %v", r)

}

Needs "errors" added to imports.

efd6 · 2022-02-09T21:48:56Z

libbeat/api/server_test.go

+	require.NoError(t, err)
+	defer r.Body.Close()
+
+	body, err := ioutil.ReadAll(r.Body)


s/ioutil/io/

efd6 · 2022-02-09T21:51:08Z

libbeat/monitoring/report/buffer/buffer.go

+	entries []interface{}
+	i       int
+	full    bool
+	mu      sync.Mutex


Conventionally mutexes are placed above the field they protect.

michel-laterman · 2022-02-10T20:55:34Z

/test

kuisathaverat · 2022-02-11T22:43:52Z

/test

…into feature/use-with-kind-k8s-env * 'feature/use-with-kind-k8s-env' of github.com:v1v/beats: (52 commits) ci: home is declared within withBeatsEnv ci: use withKindEnv step ci: use getBranchesFromAliases and support next-patch-8 (elastic#30400) Update fields.yml (elastic#29609) Heartbeat: fix browser metrics and trace mappings (elastic#30258) Apply light edits to 8.0 changelog (elastic#30351) packetbeat/beater: make sure Npcap installation runs before interfaces are needed (elastic#30396) Add a ring-buffer reporter to libbeat (elastic#28750) Osquerybeat: Add install verification for osquerybeat (elastic#30388) update windows matrix support (elastic#30373) Refactor of metricbeat process-gathering metrics and system/process (elastic#30076) adjust next changelog wording (elastic#30371) [Metricbeat] azure: move event report into loop validDim loop (elastic#29945) fix: report GitHub Check before the cache (elastic#30372) Add support for non-unique keys in Kafka output headers (elastic#30369) ci: 6 major branch reached EOL (elastic#30357) reduce Elastic Agent shut down time by stopping processes concurrently (elastic#29650) [Filebeat] Add message to register encode/decode debug logs (elastic#30271) [libbeat] kafka message header support (elastic#29940) Heartbeat: set duration to zero for syntax errors (elastic#30227) ...

michel-laterman added enhancement Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team labels Nov 1, 2021

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Nov 1, 2021

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Nov 1, 2021

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Nov 1, 2021

michel-laterman force-pushed the monitoring-ring-buffer branch from 47c2b95 to 2a7f38e Compare November 19, 2021 21:30

Add basic benchmarks

a3ec1d8

botelastic bot added the Stalled label Dec 20, 2021

Merge remote-tracking branch 'origin/master' into monitoring-ring-buffer

6006ffc

botelastic bot removed the Stalled label Dec 21, 2021

botelastic bot added the Stalled label Jan 20, 2022

botelastic bot removed the Stalled label Jan 20, 2022

Merge remote-tracking branch 'origin/master' into monitoring-ring-buffer

6c8797f

michel-laterman added backport-7.17 Automated backport to the 7.17 branch with mergify backport-v8.2.0 Automated backport with mergify labels Feb 8, 2022

mergify bot removed the backport-skip Skip notification from the automated backport with mergify label Feb 8, 2022

michel-laterman removed the backport-7.17 Automated backport to the 7.17 branch with mergify label Feb 8, 2022

michel-laterman marked this pull request as ready for review February 8, 2022 17:23

mergify bot assigned michel-laterman Feb 8, 2022

blakerouse approved these changes Feb 9, 2022

View reviewed changes

ph approved these changes Feb 9, 2022

View reviewed changes

efd6 reviewed Feb 9, 2022

View reviewed changes

Review feddback

4d74030

michel-laterman requested review from a team as code owners February 10, 2022 17:11

botelastic bot added the Team:Automation Label for the Observability productivity team label Feb 10, 2022

Merge branch 'main' into monitoring-ring-buffer

b0947cc

ph approved these changes Feb 10, 2022

View reviewed changes

efd6 approved these changes Feb 10, 2022

View reviewed changes

michel-laterman added the v8.2.0 label Feb 15, 2022

michel-laterman merged commit 6769d47 into elastic:main Feb 15, 2022

michel-laterman deleted the monitoring-ring-buffer branch February 15, 2022 16:59

	// we assume that the size of the buffer is greater than one.
	// We assume that the size of the buffer is greater than one.
	``

Conversation

michel-laterman commented Nov 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Why is it important?

Checklist

How to test this PR locally

Related issues

Uh oh!

mergify bot commented Nov 1, 2021

Uh oh!

elasticmachine commented Nov 1, 2021 • edited by jenkins-beats-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

❕ Flaky test report

🤖 GitHub comments

Uh oh!

mergify bot commented Nov 15, 2021

Uh oh!

botelastic bot commented Dec 20, 2021

Uh oh!

botelastic bot commented Jan 20, 2022

Uh oh!

ph commented Jan 20, 2022

Uh oh!

michel-laterman commented Jan 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Feb 8, 2022

Uh oh!

blakerouse left a comment

Choose a reason for hiding this comment

Uh oh!

ph commented Feb 9, 2022

Uh oh!

ph commented Feb 9, 2022

Uh oh!

ph left a comment

Choose a reason for hiding this comment

Uh oh!

ph Feb 9, 2022

Choose a reason for hiding this comment

Uh oh!

ph Feb 9, 2022

Choose a reason for hiding this comment

Uh oh!

efd6 Feb 9, 2022

Choose a reason for hiding this comment

Uh oh!

efd6 Feb 9, 2022

Choose a reason for hiding this comment

Uh oh!

efd6 Feb 9, 2022

Choose a reason for hiding this comment

Uh oh!

michel-laterman commented Feb 10, 2022

Uh oh!

kuisathaverat commented Feb 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

michel-laterman commented Nov 1, 2021 •

edited

Loading

elasticmachine commented Nov 1, 2021 •

edited by jenkins-beats-ci bot

Loading

michel-laterman commented Jan 31, 2022 •

edited

Loading