Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add config option to attach pprof endpoints to http socket #28902

Merged
merged 4 commits into from
Nov 15, 2021

Conversation

michel-laterman
Copy link
Contributor

What does this PR do?

Add a config option to attach /debug/pprof/ endpoints to the stats handler enabled via http.enabled.

Why is it important?

Currently pprof endpoints are only enabled when the -httpprof flag is used; these endpoints are bound to a different socket then the stats endpoint. We would like to serve these endpoints on the same socket in order to unify calls from external processes.

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

Enable http and pprof when running a beat via -E http.enabled=true -E http.pprof.enabled=true.
Check the /debug/pprof/ endpoint via curl

Related issues

@michel-laterman michel-laterman added enhancement backport-v7.16.0 Automated backport with mergify Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team labels Nov 10, 2021
@michel-laterman michel-laterman requested review from a team as code owners November 10, 2021 00:39
@elasticmachine
Copy link
Collaborator

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

@botelastic 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 10, 2021
@michel-laterman
Copy link
Contributor Author

I'm splitting apart and improving on some aspects of #28798 so its easier to review

@elasticmachine
Copy link
Collaborator

elasticmachine commented Nov 10, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-11-10T23:12:31.623+0000

  • Duration: 133 min 38 sec

  • Commit: ed16bf8

Test stats 🧪

Test Results
Failed 0
Passed 54230
Skipped 5345
Total 59575

💚 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.

libbeat/_meta/config/http.reference.yml.tmpl Outdated Show resolved Hide resolved
libbeat/api/routes.go Show resolved Hide resolved
@michel-laterman michel-laterman added the backport-v8.0.0 Automated backport with mergify label Nov 10, 2021
Copy link
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I'm wondering if we should even have this documented / supported in the context of Beats standalone as we only need this for Elastic Agent.

@@ -105,6 +105,7 @@ type beatConfig struct {

// beat internal components configurations
HTTP *common.Config `config:"http"`
HTTPPprof *common.Config `config:"http.pprof"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I was thinking this would conflict with the HTTP config one line before but it seems we have the same pattern on line 110/111?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, i followed the same pattern

@michel-laterman michel-laterman merged commit 3d54291 into elastic:master Nov 15, 2021
@michel-laterman michel-laterman deleted the libbeat-http-pprof branch November 15, 2021 20:22
mergify bot pushed a commit that referenced this pull request Nov 15, 2021
* Add config option to attach pprof endpoints to http socket

* Fix linting issues

* add security note

* Add pprof security docs

(cherry picked from commit 3d54291)
mergify bot pushed a commit that referenced this pull request Nov 15, 2021
* Add config option to attach pprof endpoints to http socket

* Fix linting issues

* add security note

* Add pprof security docs

(cherry picked from commit 3d54291)
michel-laterman added a commit that referenced this pull request Nov 16, 2021
…o http socket (#28978)

* Add config option to attach pprof endpoints to http socket (#28902)

* Add config option to attach pprof endpoints to http socket

* Fix linting issues

* add security note

* Add pprof security docs

(cherry picked from commit 3d54291)

* Fix CHANGELOG

Co-authored-by: Michel Laterman <[email protected]>
Co-authored-by: michel-laterman <[email protected]>
michel-laterman added a commit that referenced this pull request Nov 16, 2021
…28979)

* Add config option to attach pprof endpoints to http socket

* Fix linting issues

* add security note

* Add pprof security docs

(cherry picked from commit 3d54291)

Co-authored-by: Michel Laterman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.16.0 Automated backport with mergify backport-v8.0.0 Automated backport with mergify enhancement Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants