Skip to content

[admin] extract logs and profiler handlers to separate classes#11087

Merged
jmarantz merged 7 commits intoenvoyproxy:masterfrom
rulex123:log-profiling-handlers
May 13, 2020
Merged

[admin] extract logs and profiler handlers to separate classes#11087
jmarantz merged 7 commits intoenvoyproxy:masterfrom
rulex123:log-profiling-handlers

Conversation

@rulex123
Copy link
Copy Markdown
Contributor

@rulex123 rulex123 commented May 6, 2020

Description: extract logging and profiling-related handlers from admin.h|cc and into separate classes (part of #5505 )
Risk Level: low
Testing: pre-existing tests
Docs Changes: n/a
Release Notes: n/a

rulex123 added 7 commits May 5, 2020 17:28
Signed-off-by: Erica Manno <erica.manno@gmail.com>
Signed-off-by: Erica Manno <erica.manno@gmail.com>
Signed-off-by: Erica Manno <erica.manno@gmail.com>
Signed-off-by: Erica Manno <erica.manno@gmail.com>
Signed-off-by: Erica Manno <erica.manno@gmail.com>
Signed-off-by: Erica Manno <erica.manno@gmail.com>
Signed-off-by: Erica Manno <erica.manno@gmail.com>
Copy link
Copy Markdown
Contributor

@jmarantz jmarantz left a comment

Choose a reason for hiding this comment

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

This looks great. Thank you!

@mattklein123 I'm wondering, is there anything under source/server/http that is not for the admin endpoint? I wonder if we should rename that directory sourcer/server/admin. Also note that in the future the admin handling may occur via some gRPC mechanism in addition to via http, and in fact it's already available via direct call from outside the server, bypassing HTTP. WDYT?

That could be a follow-up to this PR of course.

@jmarantz
Copy link
Copy Markdown
Contributor

/wait

@mattklein123
Copy link
Copy Markdown
Member

@mattklein123 I'm wondering, is there anything under source/server/http that is not for the admin endpoint? I wonder if we should rename that directory sourcer/server/admin. Also note that in the future the admin handling may occur via some gRPC mechanism in addition to via http, and in fact it's already available via direct call from outside the server, bypassing HTTP. WDYT?

You read my mind, I was going to suggest renaming the directory when this was complete. Sounds great. Agreed we can do as a follow up so should we just merge this?

@jmarantz jmarantz merged commit 57d369a into envoyproxy:master May 13, 2020
@jmarantz
Copy link
Copy Markdown
Contributor

yup, follow-up would be great.

@rulex123
Copy link
Copy Markdown
Contributor Author

Cool, will do a follow-up PR to rename the directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants