stats: Remember recent lookups and display them in an admin endpoint#8116
stats: Remember recent lookups and display them in an admin endpoint#8116jmarantz merged 69 commits intoenvoyproxy:masterfrom
Conversation
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
|
This is ready for a round of review, though I realize I may need to work a little more on the stats.md doc. |
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
|
I have an un-pushed change on top of this that adds logging. The extra code is small (~ 300 lines) but the implication is risk of spamming logs. Currently I have this set to dump the remembered (10) recent lookups for the last 5 minutes, at most once every 5 minutes. I am wondering if that should be flag-controlled and if therefore we should review this as is, and then let the logging addition be a follow-up. I'm also thinking that a config API to add builtin stat-names would make any reports in those logs actionable by Envoy users without having to make code changes. Maybe that should all be integrated into this one PR? WDTY? |
|
This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
…r to gauge. Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
…k for this! Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
|
/retest |
|
🔨 rebuilding |
mattklein123
left a comment
There was a problem hiding this comment.
Thanks this looks great, just some small nits.
/wait
docs/root/operations/admin.rst
Outdated
| but in response to user requests on high core-count machines, this | ||
| can cause performance issues due to mutex contention. | ||
|
|
||
| This option requires Envoy to be started with `use-fake-symbol-table 0`. |
There was a problem hiding this comment.
nit: I think you can use an :option ref link here.
There was a problem hiding this comment.
Actually I backed this out, as use-fake-symbol-table is not docced, as it was meant to be temporary.
docs/root/operations/admin.rst
Outdated
| .. http:post:: /stats/recentlookups/clear | ||
|
|
||
| Clears all outstanding lookups and counts. If called when recent lookup | ||
| collection is enabled, this clears all the, but collection |
There was a problem hiding this comment.
fixed, and I simplified this, as I don't think it adds anything to the paragraph to discuss the two different cases.
commit coming.
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
…nvoyproxy#8116) * stats: Remember recent lookups and display them in an admin endpoint Signed-off-by: Joshua Marantz <jmarantz@google.com>
Description: Adds an admin endpoint to show recent lookups. This blocks #4980 .
Risk Level: medium -- this does add overhead when creating new StatNames.
Testing: //test/...
Docs Changes: yes, in the PR.
Release Notes: in the PR.
Fixes: #8035