Skip to content

Commit

Permalink
admin: Streaming /stats implementation (#19693)
Browse files Browse the repository at this point in the history
Commit Message: Implements streaming /stats API for JSON and Text. Prometheus can be done in a follow-up.

Additional Description:

The speed up with this algorithm is dramatic for the "/stats" endpoint on 10k clusters, improving a 1.76 second CPU/memory burst into less than half a second, and speeding up JSON delivery from 5.2 seconds to 2.5 seconds. There is still a bottleneck is the JSON serialization library, which is needed to ensure we have proper escaping of stat-names with special characters, and a larger bottleneck whenever regex filers are used because std::regex has poor performance.

In the future, better mechanisms for filtering can be added, such as an HTML UI for navigating stats by name hierarchy (#18670) or prefix/suffix matching or using RE2.

This PR fully resolves #16981 but does not address #16139 at all. It might be possible to generalize the solution here to cover Prometheus by keeping our statss_map_ indexed by tagExtractedStatName, but that would be a whole separate effort, and we'd have to figure out if related-stats could span scopes.

This PR partially resolves #18675 but there's still a non-trivial burst of CPU especially when there's a regex filter. #18670 would (arguably) fully resolve #18675.

BEFORE:
```
------------------------------------------------------------------
Benchmark                        Time             CPU   Iterations
------------------------------------------------------------------
BM_AllCountersText            1760 ms         1760 ms            1
BM_UsedCountersText            118 ms          118 ms            6
BM_FilteredCountersText       4290 ms         4289 ms            1
BM_AllCountersJson            5221 ms         5218 ms            1
BM_UsedCountersJson            152 ms          152 ms            5
BM_FilteredCountersJson       4162 ms         4161 ms            1
```
AFTER
```
------------------------------------------------------------------
Benchmark                        Time             CPU   Iterations
------------------------------------------------------------------
BM_AllCountersText             454 ms          454 ms            2
BM_UsedCountersText           37.7 ms         37.7 ms           19
BM_FilteredCountersText       4056 ms         4055 ms            1
BM_AllCountersJson            2707 ms         2706 ms            1
BM_UsedCountersJson           63.4 ms         63.4 ms           11
BM_FilteredCountersJson       4008 ms         4008 ms            1
```

I also ran a manual test with 100k clusters sending `/stats` to the admin port with output to /dev/null. Before this change, this takes 35 seconds and after this change it takes 7 seconds, and consumes about 2G less memory. These were measured with `time wget ...` and observing the process with `top`. To run that I had to set up the clusters to reference static IP addresses and make the stats-sink interval 299 seconds, as others DNS resolution and stats sinks dominate the CPU and make other measurements difficult.

Risk Level: medium -- the stats generation code in admin is largely rewritten and is important in some flows.
Testing: //test/...
Docs Changes: n/a -- no functional changes in this PR -- just perf improvements
Release Notes: n/a
Platform Specific Features: n/a
Fixes: #16981

Signed-off-by: Joshua Marantz <[email protected]>
  • Loading branch information
jmarantz authored Apr 6, 2022
1 parent d8b6d3e commit 04aae9f
Show file tree
Hide file tree
Showing 25 changed files with 1,416 additions and 437 deletions.
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>;

/**
* 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) {
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
48 changes: 32 additions & 16 deletions source/server/admin/admin.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,9 @@ class AdminImpl : public Admin,
// The prefix must start with "/" and contain at least one additional character.
bool addHandler(const std::string& prefix, const std::string& help_text, HandlerCb callback,
bool removable, bool mutates_server_state) override;
bool addChunkedHandler(const std::string& prefix, const std::string& help_text,
GenHandlerCb callback, bool removable, bool mutates_server_state) override;
bool addStreamingHandler(const std::string& prefix, const std::string& help_text,
GenRequestFn callback, bool removable,
bool mutates_server_state) override;
bool removeHandler(const std::string& prefix) override;
ConfigTracker& getConfigTracker() override;

Expand Down Expand Up @@ -201,24 +202,16 @@ class AdminImpl : public Admin,
void closeSocket();
void addListenerToHandler(Network::ConnectionHandler* handler) override;

GenHandlerCb createHandlerFunction() {
return [this](absl::string_view path_and_query, AdminStream& admin_stream) -> HandlerPtr {
return findHandler(path_and_query, admin_stream);
GenRequestFn createRequestFunction() {
return [this](absl::string_view path_and_query, AdminStream& admin_stream) -> RequestPtr {
return makeRequest(path_and_query, admin_stream);
};
}
uint64_t maxRequestsPerConnection() const override { return 0; }
const HttpConnectionManagerProto::ProxyStatusConfig* proxyStatusConfig() const override {
return proxy_status_config_.get();
}

/**
* Makes a chunked handler for static text.
* @param resposne_text the text to populate response with
* @param code the Http::Code for the response
* @return the handler
*/
static HandlerPtr makeStaticTextHandler(absl::string_view response_text, Http::Code code);

private:
friend class AdminTestingPeer;

Expand All @@ -228,21 +221,44 @@ class AdminImpl : public Admin,
struct UrlHandler {
const std::string prefix_;
const std::string help_text_;
const GenHandlerCb handler_;
const GenRequestFn handler_;
const bool removable_;
const bool mutates_server_state_;
};

/**
* Creates a Handler instance given a request.
* Creates a Request from a url.
*/
HandlerPtr findHandler(absl::string_view path_and_query, AdminStream& admin_stream);
RequestPtr makeRequest(absl::string_view path_and_query, AdminStream& admin_stream);

/**
* Creates a UrlHandler structure from a non-chunked callback.
*/
UrlHandler makeHandler(const std::string& prefix, const std::string& help_text,
HandlerCb callback, bool removable, bool mutates_state);

/**
* Creates a URL prefix bound to chunked handler. Handler is expected to
* supply a method makeRequest(absl::string_view, AdminStream&).
*
* @param prefix the prefix to register
* @param help_text a help text ot display in a table in the admin home page
* @param handler the Handler object for the admin subsystem, supplying makeContext().
* @param removeable indicates whether the handler can be removed after being added
* @param mutates_state indicates whether the handler will mutate state and therefore
* must be accessed via HTTP POST rather than GET.
* @return the UrlHandler.
*/
template <class Handler>
UrlHandler makeStreamingHandler(const std::string& prefix, const std::string& help_text,
Handler& handler, bool removable, bool mutates_state) {
return {prefix, help_text,
[&handler](absl::string_view path, AdminStream& admin_stream) -> Admin::RequestPtr {
return handler.makeRequest(path, admin_stream);
},
removable, mutates_state};
}

/**
* Implementation of RouteConfigProvider that returns a static null route config.
*/
Expand Down
Loading

0 comments on commit 04aae9f

Please sign in to comment.