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

admin: limit number of bytes an admin point can emit #31082

Open
jmarantz opened this issue Nov 28, 2023 · 2 comments
Open

admin: limit number of bytes an admin point can emit #31082

jmarantz opened this issue Nov 28, 2023 · 2 comments
Labels
area/admin enhancement Feature requests. Not bugs or questions. no stalebot Disables stalebot from closing an issue

Comments

@jmarantz
Copy link
Contributor

jmarantz commented Nov 28, 2023

Title: admin endpoints may yield more bytes than browsers can hold in memory, and it should be possible (and maybe default behavior) to limit that.

Description:
See related issue #16139 which is for /stats/prometheus in particular. But the same issue is present for other admin endpoints.

There are several approaches to this, which may need to be combined in some cases.

  1. don't buffer up data in the server but flush it out to the admin http stream periodically. this is done for /stats but not for other endpoints that need it. This addresses server memory for the most part. It is also not done yet for /stats/prometheus though. Prom stats perf improvements #24998 attempts to fix; it proved harder than expected.
  2. use high/low watermark hooks to pause generating and streaming more content until the client has read it. I implemented this but failed to write a test proving it worked: admin: apply flow control via high/low watermarks #31087
  3. use a paginated API to only provide explicitly asked for data. I implemented this in stats but did not fully develop it. (WiP admin: Add paging UI to admin stats page. #19413)
  4. use a hierarchical API to segment the data being asked for. I implemented this for stats also, including a UI with an outline-type view like a file-explorer, and ajax requests to fill in scopes as you expand them. This worked really nicely but I never got the PR ready for review. (RFC admin: hierarchical stats viewer #18670)
  5. Simply limit the default number of bytes any admin endpoint can emit, and provide a query-param to override the default. The risk is that if the output format is JSON then truncating it would break it. However, https://github.com/envoyproxy/envoy/blob/main/source/common/json/json_streamer.h could easily be enhanced to give it a maximum byte count, after which it will auto-close all hierarchy and suppress further streaming. That API is used only for /stats?format=json (it's significantly faster than populating a protobuf and serializing) but it could be used for other endpoints to safely limit output.

@nezdolik
@rulex123

@jmarantz jmarantz added enhancement Feature requests. Not bugs or questions. triage Issue requires triage labels Nov 28, 2023
@jmarantz jmarantz changed the title admin: limit number of bytes the any admin point can emit admin: limit number of bytes an admin point can emit Nov 28, 2023
@jmarantz jmarantz added area/admin and removed triage Issue requires triage labels Nov 28, 2023
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Dec 29, 2023
Copy link

github-actions bot commented Jan 5, 2024

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 5, 2024
@jmarantz jmarantz added no stalebot Disables stalebot from closing an issue and removed stale stalebot believes this issue/PR has not been touched recently labels Jan 5, 2024
@jmarantz jmarantz reopened this Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/admin enhancement Feature requests. Not bugs or questions. no stalebot Disables stalebot from closing an issue
Projects
None yet
Development

No branches or pull requests

1 participant