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: Streaming /stats implementation #19693

Merged
merged 97 commits into from
Apr 6, 2022
Merged
Show file tree
Hide file tree
Changes from 95 commits
Commits
Show all changes
97 commits
Select commit Hold shift + click to select a range
acc5325
get basic streaming working for /stats
jmarantz Jan 21, 2022
34ea762
Merge branch 'main' into admin-chunk
jmarantz Jan 23, 2022
75861f1
chunk-by-scope checkpoint
jmarantz Jan 25, 2022
74fcba9
checkpoint with basic chunked text working, but not json.
jmarantz Jan 26, 2022
faccfd7
get chunking working on live server, 100k clusters.
jmarantz Jan 26, 2022
ca5aba0
Merge branch 'main' into admin-chunk-by-scope
jmarantz Jan 26, 2022
582951d
stats_handler_test working (except what had to be commented out).
jmarantz Jan 27, 2022
5878ecd
format
jmarantz Jan 27, 2022
87d89aa
typo
jmarantz Jan 27, 2022
29f80b4
fix tests
jmarantz Jan 27, 2022
99c8777
Merge branch 'main' into admin-chunk-by-scope
jmarantz Jan 27, 2022
e2fa8e2
fix clang-tidy issues
jmarantz Jan 28, 2022
7820e00
clang-tidy suppression.
jmarantz Jan 28, 2022
44d2d0a
Hacks to eliminate irrelevant overhead
jmarantz Feb 1, 2022
b2d5a8d
Merge branch 'main' into admin-chunk-by-scope
jmarantz Feb 2, 2022
b8ca697
compile fixes
jmarantz Feb 2, 2022
5afc02a
format
jmarantz Feb 2, 2022
1b7695a
remove ifdef-blocks for use of RefcountPtr for scopes.
jmarantz Feb 2, 2022
0f58de7
cleanup hacks used for perf testing
jmarantz Feb 2, 2022
0613f53
big rename
jmarantz Feb 2, 2022
cc6243f
Revert "big rename"
jmarantz Feb 2, 2022
82d32b9
back out accidental change.
jmarantz Feb 2, 2022
aa0d5a0
back out more accidental commits.
jmarantz Feb 2, 2022
ba7c63b
Merge branch 'main' into admin-chunk-by-scope
jmarantz Feb 2, 2022
b4c8141
remove superfluous allocater_impl change
jmarantz Feb 2, 2022
4b421af
revert un-needed test tweak.
jmarantz Feb 4, 2022
08d5475
Merge branch 'main' into admin-chunk-by-scope
jmarantz Feb 4, 2022
c006b04
Merge branch 'main' into admin-chunk-by-scope
jmarantz Feb 11, 2022
f2cbfc3
Connect chunk-friendly implementation of /stats into the admin chunk …
jmarantz Feb 12, 2022
766381a
Move StatsHandler::Context into .cc file.
jmarantz Feb 12, 2022
29e32ad
cleanup
jmarantz Feb 12, 2022
0eb9ffb
remove unused method
jmarantz Feb 12, 2022
059a32f
add more test coverage
jmarantz Feb 13, 2022
bda9297
clean up prometheus tests.
jmarantz Feb 13, 2022
0d79f4c
cleanup.
jmarantz Feb 14, 2022
ddcbcb0
more test tweaks
jmarantz Feb 14, 2022
c9473da
Merge branch 'main' into admin-chunk-by-scope
jmarantz Feb 16, 2022
80de8a8
post-merge cleanup
jmarantz Feb 16, 2022
71444c0
undo superfluous edit
jmarantz Feb 16, 2022
554c0cd
Merge branch 'main' into admin-chunk-by-scope
jmarantz Feb 17, 2022
4a941bd
Move superfluous headers out of the .h file and into the .cc if necee…
jmarantz Feb 17, 2022
c702167
use btree rather than std::map.
jmarantz Feb 17, 2022
7ea16f3
Merge branch 'main' into admin-chunk-by-scope
jmarantz Feb 17, 2022
37f9b9a
correct merge-related issues.
jmarantz Feb 17, 2022
aff664f
add some comments.
jmarantz Feb 19, 2022
e6de771
Merge branch 'main' into admin-chunk-by-scope
jmarantz Feb 19, 2022
d3742cd
name change and comments.
jmarantz Feb 21, 2022
c287a96
finish name-change edits.
jmarantz Feb 21, 2022
6e52482
cleanup
jmarantz Feb 21, 2022
d02ab70
more comments & tests.
jmarantz Feb 21, 2022
4f28d1e
add benchmark.
jmarantz Feb 21, 2022
920123a
format
jmarantz Feb 21, 2022
47ca61b
buffer up 64 stats before calling the json serialize function; that a…
jmarantz Feb 21, 2022
6609dbc
Merge branch 'main' into admin-chunk-by-scope
jmarantz Feb 22, 2022
234da74
add comments, const delcarations, improve var names
jmarantz Feb 22, 2022
59526dd
Merge branch 'main' into admin-chunk-by-scope
jmarantz Mar 1, 2022
17e9f73
Merge branch 'main' into admin-chunk-by-scope
jmarantz Mar 1, 2022
0170399
Merge branch 'main' into admin-chunk-by-scope
jmarantz Mar 2, 2022
fd76570
Merge branch 'main' into admin-chunk-by-scope
jmarantz Mar 2, 2022
7035035
compile works, need to fix json.
jmarantz Mar 2, 2022
ecc2708
tests pass
jmarantz Mar 3, 2022
4e62747
get tests working & add comments.
jmarantz Mar 3, 2022
49a632e
remove spurious declarations.
jmarantz Mar 3, 2022
f35e21b
remove spurious friend declaration
jmarantz Mar 3, 2022
805e7b5
Clean up map syntax with a nickname and refs.
jmarantz Mar 3, 2022
dee72d8
more cleanup
jmarantz Mar 3, 2022
ca81c8d
split out the Request and Render class implementations into their own…
jmarantz Mar 3, 2022
b8660f5
split up libraries
jmarantz Mar 3, 2022
ddeaba1
cleanup
jmarantz Mar 3, 2022
d2405e9
Reduce copying of vectors, which was needed because the disjoint vari…
jmarantz Mar 3, 2022
a1140e6
syntax cleanup
jmarantz Mar 3, 2022
27b748c
clarify ownership of returned value for computeDisjointBuckets()
jmarantz Mar 3, 2022
b9152a7
add unit test for the Render classes.
jmarantz Mar 8, 2022
256d681
Merge branch 'main' into admin-chunk-by-scope
jmarantz Mar 8, 2022
b5ed846
add override
jmarantz Mar 9, 2022
4e34dba
review comments
jmarantz Mar 10, 2022
91cd012
add comment suggeseting Prometheus strategy.
jmarantz Mar 10, 2022
cb935f3
add a test for StatsRequest that covers chunking behavior.
jmarantz Mar 10, 2022
2f4c4b9
Remove superfluous special-case for scopes with empty prefix, and avo…
jmarantz Mar 11, 2022
d8218ca
Merge branch 'main' into admin-chunk-by-scope
jmarantz Mar 11, 2022
8ffed3c
Merge branch 'main' into admin-chunk-by-scope
jmarantz Mar 16, 2022
126bddc
Address Pradeep's comments.
jmarantz Mar 16, 2022
cad4d12
Merge branch 'main' into admin-chunk-by-scope
jmarantz Mar 27, 2022
a099aa5
review comments round 1
jmarantz Mar 27, 2022
d8529f3
convert one list value.
jmarantz Mar 27, 2022
94be4b7
simplify direct population of ListValue.
jmarantz Mar 27, 2022
81e37cc
more listValue conversions
jmarantz Mar 27, 2022
0e0bf3d
commented-out changes which convert stats_array_ to a ListValue.
jmarantz Mar 27, 2022
7dc9a55
change commenting-out to ifdefs.
jmarantz Mar 27, 2022
72743e5
pass a protobuf-value by const ref.
jmarantz Mar 27, 2022
5e62221
Remove experiment of converting stats_rray_ to a ProtobufWkt::ListVal…
jmarantz Mar 27, 2022
9fc63d0
remove commented-out code and format.
jmarantz Mar 27, 2022
f594088
Merge branch 'main' into admin-chunk-by-scope
jmarantz Apr 1, 2022
9731b03
review comments
jmarantz Apr 1, 2022
5d628e4
Merge branch 'main' into admin-chunk-by-scope
jmarantz Apr 4, 2022
59c7a0f
Merge branch 'main' into admin-chunk-by-scope
jmarantz Apr 5, 2022
92e115e
remove accidental bazelversion commit
jmarantz Apr 5, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .bazelversion
Original file line number Diff line number Diff line change
@@ -1 +1 @@
5.0.0
5.1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be part of this PR

50 changes: 39 additions & 11 deletions envoy/server/admin.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,33 @@ class AdminStream {
}

/**
* Global admin HTTP endpoint for the server.
* Global admin HTTP endpoint for the server, holding a map from URL prefixes to
* handlers. When an HTTP request arrives at the admin port, the URL is linearly
* prefixed-matched against an ordered list of handlers. When a match is found,
* the handler is used to generate a Request.
*
* Requests are capable of streaming out content to the client, however, most
* requests are delivered all at once. The implementation supplies adapters for
* simplifying the creation of streaming requests based on a simple callback
* that takes a URL and generates response headers and response body.
*
* A Taxonomy of the major types involved may help clarify:
* Request a class holding state for streaming admin content to clients.
* These are re-created for each request.
* Handler a class that holds context for a family of admin requests,
* supplying one-shot callbacks for non-streamed responses, and
* for generating Request objects directly for streamed responses.
* These have the same lifetime as Admin objects.
* Admin Holds the ordered list of handlers to be prefix-matched.
*/
class Admin {
public:
virtual ~Admin() = default;

// Represents a handler for admin endpoints, allowing for chunked responses.
class Handler {
// Represents a request for admin endpoints, enabling streamed responses.
class Request {
public:
virtual ~Handler() = default;
virtual ~Request() = default;

/**
* Initiates a handler. The URL can be supplied to the constructor if needed.
Expand All @@ -102,7 +119,7 @@ class Admin {
*/
virtual bool nextChunk(Buffer::Instance& response) PURE;
};
using HandlerPtr = std::unique_ptr<Handler>;
using RequestPtr = std::unique_ptr<Request>;
jmarantz marked this conversation as resolved.
Show resolved Hide resolved

/**
* Callback for admin URL handlers.
Expand All @@ -119,9 +136,9 @@ class Admin {
Buffer::Instance& response, AdminStream& admin_stream)>;

/**
* Lambda to generate a Handler.
* Lambda to generate a Request.
*/
using GenHandlerCb = std::function<HandlerPtr(absl::string_view path, AdminStream&)>;
using GenRequestFn = std::function<RequestPtr(absl::string_view path, AdminStream&)>;

/**
* Add a legacy admin handler where the entire response is written in
Expand All @@ -142,14 +159,14 @@ class Admin {
*
* @param prefix supplies the URL prefix to handle.
* @param help_text supplies the help text for the handler.
* @param callback supplies the callback to generate a Handler.
* @param gen_request supplies the callback to generate a Request.
* @param removable if true allows the handler to be removed via removeHandler.
* @param mutates_server_state indicates whether callback will mutate server state.
* @return bool true if the handler was added, false if it was not added.
*/
virtual bool addChunkedHandler(const std::string& prefix, const std::string& help_text,
GenHandlerCb callback, bool removable,
bool mutates_server_state) PURE;
virtual bool addStreamingHandler(const std::string& prefix, const std::string& help_text,
GenRequestFn gen_request, bool removable,
bool mutates_server_state) PURE;

/**
* Remove an admin handler if it is removable.
Expand Down Expand Up @@ -207,6 +224,17 @@ class Admin {
* @return the number of worker threads to run in the server.
*/
virtual uint32_t concurrency() const PURE;

/**
* Makes a request for streamed static text. The version that takes the
* Buffer::Instance& transfers the content from the passed-in buffer.
*
* @param response_text the text to populate response with
* @param code the Http::Code for the response
* @return the request
*/
static RequestPtr makeStaticTextRequest(absl::string_view response_text, Http::Code code);
static RequestPtr makeStaticTextRequest(Buffer::Instance& response_text, Http::Code code);
};

} // namespace Server
Expand Down
34 changes: 32 additions & 2 deletions source/server/admin/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -91,22 +91,52 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "stats_request_lib",
srcs = ["stats_request.cc"],
hdrs = ["stats_request.h"],
deps = [
":handler_ctx_lib",
":prometheus_stats_lib",
":stats_render_lib",
":utils_lib",
"//envoy/http:codes_interface",
"//envoy/server:admin_interface",
"//source/common/http:codes_lib",
"//source/common/http:header_map_lib",
"//source/common/stats:histogram_lib",
"@com_google_absl//absl/container:btree",
],
)

envoy_cc_library(
name = "stats_render_lib",
srcs = ["stats_render.cc"],
hdrs = ["stats_render.h"],
deps = [
":utils_lib",
"//source/common/buffer:buffer_lib",
"//source/common/html:utility_lib",
"//source/common/stats:histogram_lib",
],
)

envoy_cc_library(
name = "stats_handler_lib",
srcs = ["stats_handler.cc"],
hdrs = ["stats_handler.h"],
deps = [
":handler_ctx_lib",
":prometheus_stats_lib",
":stats_render_lib",
":stats_request_lib",
":utils_lib",
"//envoy/http:codes_interface",
"//envoy/server:admin_interface",
"//envoy/server:instance_interface",
"//source/common/buffer:buffer_lib",
"//source/common/html:utility_lib",
"//source/common/http:codes_lib",
"//source/common/http:header_map_lib",
"//source/common/stats:histogram_lib",
"@envoy_api//envoy/admin/v3:pkg_cc_proto",
],
)
Expand Down
70 changes: 39 additions & 31 deletions source/server/admin/admin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,7 @@ AdminImpl::AdminImpl(const std::string& profile_path, Server::Instance& server,
MAKE_ADMIN_HANDLER(server_info_handler_.handlerServerInfo), false, false),
makeHandler("/ready", "print server state, return 200 if LIVE, otherwise return 503",
MAKE_ADMIN_HANDLER(server_info_handler_.handlerReady), false, false),
makeHandler("/stats", "print server stats",
MAKE_ADMIN_HANDLER(stats_handler_.handlerStats), false, false),
makeStreamingHandler("/stats", "print server stats", stats_handler_, false, false),
makeHandler("/stats/prometheus", "print server stats in prometheus format",
MAKE_ADMIN_HANDLER(stats_handler_.handlerPrometheusStats), false, false),
makeHandler("/stats/recentlookups", "Show recent stat-name lookups",
Expand Down Expand Up @@ -251,41 +250,45 @@ bool AdminImpl::createNetworkFilterChain(Network::Connection& connection,
}

void AdminImpl::createFilterChain(Http::FilterChainFactoryCallbacks& callbacks) {
callbacks.addStreamFilter(std::make_shared<AdminFilter>(createHandlerFunction()));
callbacks.addStreamFilter(std::make_shared<AdminFilter>(createRequestFunction()));
}

namespace {

// Implements a chunked handler for static text.
class StaticTextHandler : public Admin::Handler {
// Implements a chunked request for static text.
class StaticTextRequest : public Admin::Request {
public:
StaticTextHandler(absl::string_view response_text, Http::Code code)
: response_text_(std::string(response_text)), code_(code) {}
StaticTextRequest(absl::string_view response_text, Http::Code code) : code_(code) {
response_text_.add(response_text);
}
StaticTextRequest(Buffer::Instance& response_text, Http::Code code) : code_(code) {
response_text_.move(response_text);
}

Http::Code start(Http::ResponseHeaderMap&) override { return code_; }
bool nextChunk(Buffer::Instance& response) override {
response.add(response_text_);
response.move(response_text_);
return false;
}

private:
const std::string response_text_;
Buffer::OwnedImpl response_text_;
const Http::Code code_;
};

// Implements a Chunked Handler implementation based on a non-chunked callback
// that generates the entire admin output in one shot.
class HandlerGasket : public Admin::Handler {
// Implements a streaming Request based on a non-streaming callback that
// generates the entire admin output in one shot.
class RequestGasket : public Admin::Request {
public:
HandlerGasket(Admin::HandlerCb handler_cb, absl::string_view path_and_query,
RequestGasket(Admin::HandlerCb handler_cb, absl::string_view path_and_query,
AdminStream& admin_stream)
: path_and_query_(std::string(path_and_query)), handler_cb_(handler_cb),
admin_stream_(admin_stream) {}

static Admin::GenHandlerCb makeGen(Admin::HandlerCb callback) {
static Admin::GenRequestFn makeGen(Admin::HandlerCb callback) {
return [callback](absl::string_view path_and_query,
AdminStream& admin_stream) -> Server::Admin::HandlerPtr {
return std::make_unique<HandlerGasket>(callback, path_and_query, admin_stream);
AdminStream& admin_stream) -> Server::Admin::RequestPtr {
return std::make_unique<RequestGasket>(callback, path_and_query, admin_stream);
};
}

Expand All @@ -307,24 +310,28 @@ class HandlerGasket : public Admin::Handler {

} // namespace

Admin::HandlerPtr AdminImpl::makeStaticTextHandler(absl::string_view response, Http::Code code) {
return std::make_unique<StaticTextHandler>(response, code);
Admin::RequestPtr Admin::makeStaticTextRequest(absl::string_view response, Http::Code code) {
return std::make_unique<StaticTextRequest>(response, code);
}

Admin::RequestPtr Admin::makeStaticTextRequest(Buffer::Instance& response, Http::Code code) {
return std::make_unique<StaticTextRequest>(response, code);
}

Http::Code AdminImpl::runCallback(absl::string_view path_and_query,
Http::ResponseHeaderMap& response_headers,
Buffer::Instance& response, AdminStream& admin_stream) {
HandlerPtr handler = findHandler(path_and_query, admin_stream);
Http::Code code = handler->start(response_headers);
RequestPtr request = makeRequest(path_and_query, admin_stream);
Http::Code code = request->start(response_headers);
bool more_data;
do {
more_data = handler->nextChunk(response);
more_data = request->nextChunk(response);
} while (more_data);
Memory::Utils::tryShrinkHeap();
return code;
}

Admin::HandlerPtr AdminImpl::findHandler(absl::string_view path_and_query,
Admin::RequestPtr AdminImpl::makeRequest(absl::string_view path_and_query,
AdminStream& admin_stream) {
std::string::size_type query_index = path_and_query.find('?');
if (query_index == std::string::npos) {
Expand All @@ -338,8 +345,9 @@ Admin::HandlerPtr AdminImpl::findHandler(absl::string_view path_and_query,
if (method != Http::Headers::get().MethodValues.Post) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense making handlers_ a flat_hash_set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wouldn't work as a flat_hash_set because it does prefix matching. It could be an rb_tree or std::map though. This iterative loop already exists though and I don't want to change it in this PR. I will leave a TODO

ENVOY_LOG(error, "admin path \"{}\" mutates state, method={} rather than POST",
handler.prefix_, method);
return makeStaticTextHandler(fmt::format("Method {} not allowed, POST required.", method),
Http::Code::MethodNotAllowed);
return Admin::makeStaticTextRequest(
fmt::format("Method {} not allowed, POST required.", method),
Http::Code::MethodNotAllowed);
}
}

Expand All @@ -352,7 +360,7 @@ Admin::HandlerPtr AdminImpl::findHandler(absl::string_view path_and_query,
Buffer::OwnedImpl error_response;
error_response.add("invalid path. ");
getHelp(error_response);
return makeStaticTextHandler(error_response.toString(), Http::Code::NotFound);
return Admin::makeStaticTextRequest(error_response, Http::Code::NotFound);
}

std::vector<const AdminImpl::UrlHandler*> AdminImpl::sortedHandlers() const {
Expand Down Expand Up @@ -435,11 +443,11 @@ const Network::Address::Instance& AdminImpl::localAddress() {
AdminImpl::UrlHandler AdminImpl::makeHandler(const std::string& prefix,
const std::string& help_text, HandlerCb callback,
bool removable, bool mutates_state) {
return UrlHandler{prefix, help_text, HandlerGasket::makeGen(callback), removable, mutates_state};
return UrlHandler{prefix, help_text, RequestGasket::makeGen(callback), removable, mutates_state};
}

bool AdminImpl::addChunkedHandler(const std::string& prefix, const std::string& help_text,
GenHandlerCb callback, bool removable, bool mutates_state) {
bool AdminImpl::addStreamingHandler(const std::string& prefix, const std::string& help_text,
GenRequestFn callback, bool removable, bool mutates_state) {
ASSERT(prefix.size() > 1);
ASSERT(prefix[0] == '/');

Expand All @@ -464,8 +472,8 @@ bool AdminImpl::addChunkedHandler(const std::string& prefix, const std::string&

bool AdminImpl::addHandler(const std::string& prefix, const std::string& help_text,
HandlerCb callback, bool removable, bool mutates_state) {
return addChunkedHandler(prefix, help_text, HandlerGasket::makeGen(callback), removable,
mutates_state);
return addStreamingHandler(prefix, help_text, RequestGasket::makeGen(callback), removable,
mutates_state);
}

bool AdminImpl::removeHandler(const std::string& prefix) {
Expand All @@ -480,7 +488,7 @@ bool AdminImpl::removeHandler(const std::string& prefix) {

Http::Code AdminImpl::request(absl::string_view path_and_query, absl::string_view method,
Http::ResponseHeaderMap& response_headers, std::string& body) {
AdminFilter filter(createHandlerFunction());
AdminFilter filter(createRequestFunction());

auto request_headers = Http::RequestHeaderMapImpl::create();
request_headers->setMethod(method);
Expand Down
Loading