Enable pprof for elastic-agent and beats by michel-laterman · Pull Request #28983 · elastic/beats

michel-laterman · 2021-11-16T02:33:58Z

What does this PR do?

Enable the /debug/pprof/ endpoints for all beats that the elastic-agent
starts. Enable the pprof endpoints on elastic-agent if
agent.monitoring.pprof is true (default true). Agent endpoint can be
toggled in case it is located on a network and not localhost/unix
socket/windows N pipe.

Why is it important?

pprof endpoint data will be added to the diagnostics bundle

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 PR HERE
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.

Author's Checklist

[ ]

How to test this PR locally

Start agent with unaltered config, test /debug/pprof/ with curl

Related issues

Relates Beats diagnostics tooling #21965

Enable the /debug/pprof/ endpoints for all beats that the elastic-agent starts. Enable the pprof endpoints on elastic-agent if agent.monitoring.pprof is true (default true). Agent endpoint can be toggled in case it is located on a network and not localhost/unix socket/windows N pipe.

elasticmachine · 2021-11-16T02:34:00Z

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

elasticmachine · 2021-11-16T04:05:55Z

💚 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: 2021-11-18T17:24:38.366+0000
Duration: 88 min 45 sec
Commit: 6591a82

Test stats 🧪

Test	Results
Failed	0
Passed	7128
Skipped	16
Total	7144

💚 Flaky test report

Tests succeeded.

🤖 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!)

michel-laterman · 2021-11-16T04:56:16Z

@ruflin this is part 2 of splitting up #28798; i'll follow up with a 3rd that adds the grpc call and diagnostics commands.

michel-laterman · 2021-11-16T22:39:59Z

/test

ruflin · 2021-11-17T08:25:02Z

x-pack/elastic-agent/pkg/core/monitoring/beats/beats_monitor.go

 		appendix = append(appendix,
 			"-E", "http.enabled=true",
 			"-E", "http.host="+endpoint,
+			"-E", "http.pprof.enabled=true",


If monitoring is enabled we always enable also pprof?

yes.
Also as far as my testing shows, even when the agent is not monitoring its beats the beats will bind to a socket (and pprof will be available)

I wonder if this might be unexpected from a user perspective. pprof exposes quite a bit more information than monitoring as it exposes some of the internals of the process. At the same time, whoever gets the monitoring data likely already knows about the internals.

@simitt @scunningham Any concerns with this?

Can you help me understand how the current config options play together:

agent.monitoring: enabled: false logs: false metrics: false http: enabled: true

Will metrics be exposed via the http endpoint?

We should not enable pprof unless customer explicitly turns it on. The HTTP interface is not defended and an attacker would be able to potentially steal secrets from the heap dump.

http.pprof.enabled is a libbeat setting that we are passing to the beats to expose the pprof options over the http endpoint.
When the agent passes it above, the http interface is bound to a local unix socket (or windows npipe)

@scunningham, when the elastic-agent starts a beat, the beat will bind the interface to a socket even if agent.monitoring.enabled: false, is your request to disable automatically enabling the pprof endpoints for these beats unless explicitly enabled in this case as well?

Are you saying we automatically enable pprof even if not explicitly enabled? That's not great.

lykkin

lgtm!

simitt · 2021-11-19T10:05:55Z

x-pack/elastic-agent/_meta/config/common.reference.p2.yml.tmpl

 #   metrics: false
+#   # exposes /debug/pprof/ endpoints
+#   # recommended that these endpoints are only enabled if the monitoring endpoint is set to localhost
+#   pprof: true


Why is pprof: true while logs and metrics are false by default?

I think that the reference here may be incorrect, the default MonitoringConfig has them set to true

simitt · 2021-11-19T10:10:02Z

x-pack/elastic-agent/pkg/core/monitoring/beats/beats_monitor.go

 		appendix = append(appendix,
 			"-E", "http.enabled=true",
 			"-E", "http.host="+endpoint,
+			"-E", "http.pprof.enabled=true",


Can you help me understand how the current config options play together:

agent.monitoring: enabled: false logs: false metrics: false http: enabled: true

Will metrics be exposed via the http endpoint?

scunningham

Spoke with Michel over Slack. He explained that this control will enable/disable pprof for agent. Different issue with existing beats.

Enable the /debug/pprof/ endpoints for all beats that the elastic-agent starts. Enable the pprof endpoints on elastic-agent if agent.monitoring.pprof is true (default true). Agent endpoint can be toggled in case it is located on a network and not localhost/unix socket/windows N pipe. (cherry picked from commit 6ad6bee)

Enable the /debug/pprof/ endpoints for all beats that the elastic-agent starts. Enable the pprof endpoints on elastic-agent if agent.monitoring.pprof is true (default true). Agent endpoint can be toggled in case it is located on a network and not localhost/unix socket/windows N pipe. (cherry picked from commit 6ad6bee) Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com>

ruflin · 2021-11-23T10:03:11Z

@michel-laterman For the pprof enabling in Beats by default, we likely should follow up in 8.0 to not have it enabled by default if monitoring is enabled or at least have a discussion. Could you file a Github issue?

simitt · 2021-11-23T13:09:52Z

On cloud we set elastic-agent.monitoring.http.enabled: true - does this mean that pprof will always be enabled then?

michel-laterman · 2021-11-23T20:22:23Z

@simitt, we should disable pprof for cloud

…ws-on-file-changes * upstream/master: override host on statsd metricset (elastic#29103) Skip config check in autodiscover for duplicated configurations (elastic#29048) Change "filebeat.config.modules.enabled" to "true" (elastic#28769) Remove deprecated spool queue from Beats (elastic#28869) Add `beat` field back to beat.stats (elastic#29094) Revert "Move labels and annotations under kubernetes.namespace. (elastic#27917)" (elastic#29069) heartbeat: remove w2008 in the CI (elastic#29093) Remove deprecated `--template` and `--index-policy` flags (elastic#28870) Fix parsing of apache trace log levels (elastic#28717) [Elastic-Agent] IUse itnernal port for local fleet server (elastic#28993) [Heartbeat] Log error on dupe monitor ID instead of strict req (elastic#29041) Enable pprof for elastic-agent and beats (elastic#28983)

michel-laterman added enhancement backport-v8.0.0 Automated backport with mergify backport-v7.16.0 Automated backport with mergify Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team labels Nov 16, 2021

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 Nov 16, 2021

ruflin reviewed Nov 17, 2021

View reviewed changes

Merge remote-tracking branch 'origin/master' into agent-enable-pprof

6591a82

lykkin approved these changes Nov 19, 2021

View reviewed changes

simitt reviewed Nov 19, 2021

View reviewed changes

scunningham approved these changes Nov 22, 2021

View reviewed changes

michel-laterman merged commit 6ad6bee into elastic:master Nov 22, 2021

michel-laterman deleted the agent-enable-pprof branch November 22, 2021 20:59

mergify bot mentioned this pull request Nov 22, 2021

[7.16](backport #28983) Enable pprof for elastic-agent and beats #29080

Merged

mergify bot mentioned this pull request Nov 22, 2021

[8.0](backport #28983) Enable pprof for elastic-agent and beats #29081

Merged

Conversation

michel-laterman commented Nov 16, 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

Author's Checklist

How to test this PR locally

Related issues

Uh oh!

elasticmachine commented Nov 16, 2021

Uh oh!

elasticmachine commented Nov 16, 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

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

Uh oh!

michel-laterman commented Nov 16, 2021

Uh oh!

michel-laterman commented Nov 16, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lykkin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scunningham left a comment

Choose a reason for hiding this comment

Uh oh!

ruflin commented Nov 23, 2021

Uh oh!

simitt commented Nov 23, 2021

Uh oh!

michel-laterman commented Nov 23, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

michel-laterman commented Nov 16, 2021 •

edited

Loading

elasticmachine commented Nov 16, 2021 •

edited by jenkins-beats-ci bot

Loading