-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[elastic-agent] proxy requests to subprocesses to their metrics endpoints #28165
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
💚 Flaky test reportTests succeeded. 🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
@@ -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"] |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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.
Pinging @elastic/agent (Team:Agent) |
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
/test |
re-running the tests, not sure what the failure means:
|
/test |
There was a problem hiding this 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
* 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) ...
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 thebeat
module in metricbeat, as well.Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.How to test this PR locally