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

[elastic-agent] proxy requests to subprocesses to their metrics endpoints #28165

Merged

Conversation

stuartnelson3
Copy link
Contributor

@stuartnelson3 stuartnelson3 commented Sep 28, 2021

What does this PR do?

proxy requests to the exposed subprocess for gathering metrics.

Why is it important?

in order for libbeat services, such as apm-server, to be easily scraped and displayed in the stack monitoring ui, both the /stats and /state endpoints are necessary. exposing both of these makes it easy to use the beat module in metricbeat, as well.

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

  • start the elastic-agent
  • verify that requests to a subprocess are proxied to its metrics endpoints:
# curl -s http://localhost:6791/processes/metricbeat-default | jq '.|keys'
[
  "beat",
  "hostname",
  "name",
  "uuid",
  "version"
]

# curl -s http://localhost:6791/processes/metricbeat-default/state | jq '.|keys'
[
  "beat",
  "host",
  "management",
  "module",
  "output",
  "outputs",
  "queue",
  "service"
]

# curl -s http://localhost:6791/processes/metricbeat-default/stats | jq '.|keys'
[
  "beat",
  "libbeat",
  "metricbeat",
  "system"
]

@stuartnelson3 stuartnelson3 added enhancement Agent backport-v7.16.0 Automated backport with mergify labels Sep 28, 2021
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Sep 28, 2021
@stuartnelson3 stuartnelson3 added the Team:Elastic-Agent Label for the Agent team label Sep 28, 2021
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Sep 28, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Sep 28, 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-10-06T11:47:53.586+0000

  • Duration: 81 min 35 sec

  • Commit: c3c8348

Test stats 🧪

Test Results
Failed 0
Passed 7109
Skipped 16
Total 7125

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

@@ -67,8 +67,14 @@ func processHandler(statsHandler func(http.ResponseWriter, *http.Request) error)
// proxy stats for elastic agent process
return statsHandler(w, r)
}
// TODO: allowlist of accepted endpoints to proxy to?
beatsEndpoint := vars["beatsEndpoint"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if subprocesses will mostly be beats, so we can limit the allowed paths to /, /state, and /stats, or how you want to handle this.

Copy link
Contributor

Choose a reason for hiding this comment

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

definitely add allowlist here, including some proper escaping

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what escaping are you looking for? I added just a direct match check for /, /state, and /stats, so I'm not sure what to escape


metricsBytes, statusCode, metricsErr := processMetrics(r.Context(), id)
endpoint, err := generateEndpoint(id)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pulled this out so I could inject my own endpoint into processMetrics, to make testing easier.

@stuartnelson3 stuartnelson3 marked this pull request as ready for review October 1, 2021 13:53
@elasticmachine
Copy link
Collaborator

Pinging @elastic/agent (Team:Agent)

@stuartnelson3 stuartnelson3 requested a review from a team October 1, 2021 13:53
@stuartnelson3
Copy link
Contributor Author

Not sure if/how you want to handle testing this on windows, so I added a build tag for now

limit proxying paths to just /, /state, and /stats
@stuartnelson3
Copy link
Contributor Author

/test

@stuartnelson3
Copy link
Contributor Author

re-running the tests, not sure what the failure means:

14:44:57 org.jenkinsci.plugins.workflow.steps.MissingContextVariableException: Required context class hudson.Launcher is missing
14:44:57 Perhaps you forgot to surround the code with a step that provides this, such as: node
14:44:57 at org.jenkinsci.plugins.workflow.steps.StepDescriptor.checkContextAvailability(StepDescriptor.java:266)
14:44:57 at org.jenkinsci.plugins.workflow.cps.DSL.invokeStep(DSL.java:296)

@stuartnelson3
Copy link
Contributor Author

/test

Copy link
Contributor

@michalpristas michalpristas left a comment

Choose a reason for hiding this comment

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

looks good. one small nit there

@stuartnelson3 stuartnelson3 merged commit 8ce047e into elastic:master Oct 6, 2021
@stuartnelson3 stuartnelson3 deleted the agent-proxy-subprocess-metrics branch October 6, 2021 13:16
mergify bot pushed a commit that referenced this pull request Oct 6, 2021
stuartnelson3 added a commit that referenced this pull request Oct 6, 2021
…ints (#28165) (#28281)

(cherry picked from commit 8ce047e)

Co-authored-by: stuart nelson <stuartnelson3@gmail.com>
v1v added a commit to v1v/beats that referenced this pull request Oct 11, 2021
* upstream/master: (73 commits)
  Remove GCP support from Functionbeat (elastic#28253)
  Move labels and annotations under kubernetes.namespace. (elastic#27917)
  Update go release version 1.17.1 (elastic#27543)
  Osquerybeat: Runner and Fetcher unit tests (elastic#28290)
  Osquerybeat: Improve handling of osquery.autoload file, allow customizations (elastic#28289)
  seccomp: allow clone3 syscall for x86 (elastic#28117)
  packetbeat/protos/dns: don't render missing A and AAAA addresses from truncated records (elastic#28297)
  [7.x] [DOCS] Update api_key example on elasticsearch output (elastic#28288)
  [cloud][docker] use the private docker namespace (elastic#28286)
  Update aws-lambda-go library version to 1.13.3 (elastic#28236)
  Deprecate common.Float (elastic#28280)
  Filebeat: Change compatibility test stage to test against previous minor instead of 7.11 (elastic#28274)
  x-pack/filebeat/module/threatintel/misp: add support for secondary object attribute handling (elastic#28124)
  Explicitly pass http config to doppler consumer (elastic#28277)
  processors/actions/add_fields: Do not panic if event.Fields is nil map (elastic#28219)
  Resolved timestamp for defender atp (elastic#28272)
  [Winlogbeat] Tolerate faults when Windows Event Log session is interrupted (elastic#28191)
  [elastic-agent] proxy requests to subprocesses to their metrics endpoints (elastic#28165)
  Build cloud docker images for elastic-agent (elastic#28134)
  Upgrade k8s go-client library (elastic#28228)
  ...
Icedroid pushed a commit to Icedroid/beats that referenced this pull request Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agent backport-v7.16.0 Automated backport with mergify enhancement Team:Elastic-Agent Label for the Agent team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants