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.

Sign up for GitHub

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

stats: json optimization of histograms #3280

Merged
merged 17 commits into from
May 10, 2018
Merged

stats: json optimization of histograms #3280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
5c3d11b
json optimization
ramaraochavali May 3, 2018
636e670
docs
ramaraochavali May 3, 2018
6243853
updated doc, introduced used check
ramaraochavali May 4, 2018
9c89ab4
formatted
ramaraochavali May 4, 2018
776d307
formatted the doc
ramaraochavali May 4, 2018
b829018
Merge branch 'master' into fix/json_optimization
ramaraochavali May 4, 2018
dbe7610
move ASSERT inside
ramaraochavali May 4, 2018
7c68d2a
corrected indentation
ramaraochavali May 4, 2018
81f8da7
add space in supported_quantlies
ramaraochavali May 4, 2018
e7ca172
correct indentation
ramaraochavali May 4, 2018
e86c4c8
fix indentation
ramaraochavali May 5, 2018
de78873
added test for histogram json
ramaraochavali May 8, 2018
6ca2748
formatted
ramaraochavali May 8, 2018
98dd266
added pretty_print optional
ramaraochavali May 10, 2018
10dc326
build formatted
ramaraochavali May 10, 2018
3e3bdaa
Merge branch 'master' into fix/json_optimization
ramaraochavali May 10, 2018
ae5dfbd
fixed minor comments
ramaraochavali May 10, 2018
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
51 changes: 49 additions & 2 deletions docs/root/operations/admin.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,56 @@ The fields are:

.. http:get:: /stats?format=json

Outputs /stats in JSON format. This can be used for programmatic access of stats.
Outputs /stats in JSON format. This can be used for programmatic access of stats. Counters and Gauges
will be in the form of a set of (name,value) pairs. Histograms will be under the element "histograms",
that contains "supported_quantiles" which lists the quantiles supported and an array of computed_quantiles
that has the computed quantile for each histogram. Only histograms with recorded values will be exported.

.. http:get:: /stats?format=prometheus
If a histogram is not updated during an interval, the ouput will have null for all the quantiles.

Example histogram output:

.. code-block:: json

{
"histograms": {
"supported_quantiles": [
0, 25, 50, 75, 90, 95, 99, 99.9, 100
],
"computed_quantiles": [
{
"name": "cluster.external_auth_cluster.upstream_cx_length_ms",
"values": [
{"interval": 0, "cumulative": 0},
{"interval": 0, "cumulative": 0},
{"interval": 1.0435787, "cumulative": 1.0435787},
{"interval": 1.0941565, "cumulative": 1.0941565},
{"interval": 2.0860023, "cumulative": 2.0860023},
{"interval": 3.0665233, "cumulative": 3.0665233},
{"interval": 6.046609, "cumulative": 6.046609},
{"interval": 229.57333,"cumulative": 229.57333},
{"interval": 260,"cumulative": 260}
]
},
{
"name": "http.admin.downstream_rq_time",
"values": [
{"interval": null, "cumulative": 0},
{"interval": null, "cumulative": 0},
{"interval": null, "cumulative": 1.0435787},
{"interval": null, "cumulative": 1.0941565},
{"interval": null, "cumulative": 2.0860023},
{"interval": null, "cumulative": 3.0665233},
{"interval": null, "cumulative": 6.046609},
{"interval": null, "cumulative": 229.57333},
{"interval": null, "cumulative": 260}
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks great now. Much easier to read. Thanks!

]
}
]
}
}

.. http:get:: /stats?format=prometheus

or alternatively,

Expand Down
113 changes: 69 additions & 44 deletions source/server/http/admin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,17 +403,19 @@ Http::Code AdminImpl::handlerStats(absl::string_view url, Http::HeaderMap& respo
// implemented this can be switched back to a normal map.
std::multimap<std::string, std::string> all_histograms;
for (const Stats::ParentHistogramSharedPtr& histogram : server_.stats().histograms()) {
std::vector<std::string> summary;
const std::vector<double>& supported_quantiles_ref =
histogram->intervalStatistics().supportedQuantiles();
summary.reserve(supported_quantiles_ref.size());
for (size_t i = 0; i < supported_quantiles_ref.size(); ++i) {
summary.push_back(fmt::format("P{}({},{})", 100 * supported_quantiles_ref[i],
histogram->intervalStatistics().computedQuantiles()[i],
histogram->cumulativeStatistics().computedQuantiles()[i]));
}
if (histogram->used()) {
std::vector<std::string> summary;
const std::vector<double>& supported_quantiles_ref =
histogram->intervalStatistics().supportedQuantiles();
summary.reserve(supported_quantiles_ref.size());
for (size_t i = 0; i < supported_quantiles_ref.size(); ++i) {
summary.push_back(fmt::format("P{}({},{})", 100 * supported_quantiles_ref[i],
histogram->intervalStatistics().computedQuantiles()[i],
histogram->cumulativeStatistics().computedQuantiles()[i]));
}

all_histograms.emplace(histogram->name(), absl::StrJoin(summary, " "));
all_histograms.emplace(histogram->name(), absl::StrJoin(summary, " "));
}
}

if (params.size() == 0) {
Expand Down Expand Up @@ -499,9 +501,9 @@ PrometheusStatsFormatter::statsAsPrometheus(const std::list<Stats::CounterShared
return metric_type_tracker.size();
}

std::string
AdminImpl::statsAsJson(const std::map<std::string, uint64_t>& all_stats,
const std::list<Stats::ParentHistogramSharedPtr>& all_histograms) {
std::string AdminImpl::statsAsJson(const std::map<std::string, uint64_t>& all_stats,
const std::list<Stats::ParentHistogramSharedPtr>& all_histograms,
const bool pretty_print) {
rapidjson::Document document;
document.SetObject();
rapidjson::Value stats_array(rapidjson::kArrayType);
Expand All @@ -518,43 +520,66 @@ AdminImpl::statsAsJson(const std::map<std::string, uint64_t>& all_stats,
stats_array.PushBack(stat_obj, allocator);
}

Value histograms_container_obj;
histograms_container_obj.SetObject();

Value histograms_obj;
histograms_obj.SetObject();

// It is not possible for the supported quantiles to differ across histograms, so it is ok to
// send them once.
Stats::HistogramStatisticsImpl empty_statistics;
rapidjson::Value supported_quantile_array(rapidjson::kArrayType);
for (double quantile : empty_statistics.supportedQuantiles()) {
Value quantile_type;
quantile_type.SetDouble(quantile * 100);
supported_quantile_array.PushBack(quantile_type, allocator);
}
histograms_obj.AddMember("supported_quantiles", supported_quantile_array, allocator);
rapidjson::Value histogram_array(rapidjson::kArrayType);

for (const Stats::ParentHistogramSharedPtr& histogram : all_histograms) {
Value histogram_obj;
histogram_obj.SetObject();
Value histogram_name;
histogram_name.SetString(histogram->name().c_str(), allocator);
histogram_obj.AddMember("name", histogram_name, allocator);

rapidjson::Value quantile_array(rapidjson::kArrayType);

// TODO(ramaraochavali): consider optimizing the model here. Quantiles can be added once,
// followed by two arrays interval and cumulative.
for (size_t i = 0; i < histogram->intervalStatistics().supportedQuantiles().size(); ++i) {
Value quantile_obj;
quantile_obj.SetObject();
Value quantile_type;
quantile_type.SetDouble(histogram->intervalStatistics().supportedQuantiles()[i] * 100);
quantile_obj.AddMember("quantile", quantile_type, allocator);
Value interval_value;
if (!std::isnan(histogram->intervalStatistics().computedQuantiles()[i])) {
interval_value.SetDouble(histogram->intervalStatistics().computedQuantiles()[i]);
}
quantile_obj.AddMember("interval_value", interval_value, allocator);
Value cumulative_value;
if (!std::isnan(histogram->cumulativeStatistics().computedQuantiles()[i])) {
cumulative_value.SetDouble(histogram->cumulativeStatistics().computedQuantiles()[i]);
if (histogram->used()) {
Value histogram_obj;
histogram_obj.SetObject();
Value histogram_name;
histogram_name.SetString(histogram->name().c_str(), allocator);
histogram_obj.AddMember("name", histogram_name, allocator);

rapidjson::Value computed_quantile_array(rapidjson::kArrayType);

for (size_t i = 0; i < histogram->intervalStatistics().supportedQuantiles().size(); ++i) {
Value quantile_obj;
quantile_obj.SetObject();
Value interval_value;
if (!std::isnan(histogram->intervalStatistics().computedQuantiles()[i])) {
interval_value.SetDouble(histogram->intervalStatistics().computedQuantiles()[i]);
}
quantile_obj.AddMember("interval", interval_value, allocator);
Value cumulative_value;
// We skip nan entries to put in the {null, null} entry to keep other data aligned.
if (!std::isnan(histogram->cumulativeStatistics().computedQuantiles()[i])) {
cumulative_value.SetDouble(histogram->cumulativeStatistics().computedQuantiles()[i]);
}
quantile_obj.AddMember("cumulative", cumulative_value, allocator);
computed_quantile_array.PushBack(quantile_obj, allocator);
}
quantile_obj.AddMember("cumulative_value", cumulative_value, allocator);
quantile_array.PushBack(quantile_obj, allocator);
histogram_obj.AddMember("values", computed_quantile_array, allocator);
histogram_array.PushBack(histogram_obj, allocator);
}
histogram_obj.AddMember("quantiles", quantile_array, allocator);
stats_array.PushBack(histogram_obj, allocator);
}

histograms_obj.AddMember("computed_quantiles", histogram_array, allocator);
histograms_container_obj.AddMember("histograms", histograms_obj, allocator);
stats_array.PushBack(histograms_container_obj, allocator);
document.AddMember("stats", stats_array, allocator);
rapidjson::StringBuffer strbuf;
rapidjson::PrettyWriter<StringBuffer> writer(strbuf);
document.Accept(writer);
if (pretty_print) {
rapidjson::PrettyWriter<StringBuffer> writer(strbuf);
document.Accept(writer);
} else {
rapidjson::Writer<StringBuffer> writer(strbuf);
document.Accept(writer);
}
return strbuf.GetString();
}

Expand Down
5 changes: 4 additions & 1 deletion source/server/http/admin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ class AdminImpl : public Admin,
Router::ConfigConstSharedPtr config_;
};

friend class AdminStatsTest;

/**
* Attempt to change the log level of a logger or all loggers
* @param params supplies the incoming endpoint query params.
Expand All @@ -132,7 +134,8 @@ class AdminImpl : public Admin,
const Upstream::Outlier::Detector* outlier_detector,
Buffer::Instance& response);
static std::string statsAsJson(const std::map<std::string, uint64_t>& all_stats,
const std::list<Stats::ParentHistogramSharedPtr>& all_histograms);
const std::list<Stats::ParentHistogramSharedPtr>& all_histograms,
bool pretty_print = false);
static std::string
runtimeAsJson(const std::vector<std::pair<std::string, Runtime::Snapshot::Entry>>& entries);
std::vector<const UrlHandler*> sortedHandlers() const;
Expand Down
40 changes: 0 additions & 40 deletions test/common/stats/thread_local_store_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,46 +25,6 @@ using testing::_;
namespace Envoy {
namespace Stats {

/**
* This is a heap test allocator that works similar to how the shared memory allocator works in
* terms of reference counting, etc.
*/
class TestAllocator : public RawStatDataAllocator {
public:
~TestAllocator() { EXPECT_TRUE(stats_.empty()); }

RawStatData* alloc(const std::string& name) override {
CSmartPtr<RawStatData, freeAdapter>& stat_ref = stats_[name];
if (!stat_ref) {
stat_ref.reset(static_cast<RawStatData*>(::calloc(RawStatData::size(), 1)));
stat_ref->initialize(name);
} else {
stat_ref->ref_count_++;
}

return stat_ref.get();
}

void free(RawStatData& data) override {
if (--data.ref_count_ > 0) {
return;
}

for (auto i = stats_.begin(); i != stats_.end(); i++) {
if (i->second.get() == &data) {
stats_.erase(i);
return;
}
}

FAIL();
}

private:
static void freeAdapter(RawStatData* data) { ::free(data); }
std::unordered_map<std::string, CSmartPtr<RawStatData, freeAdapter>> stats_;
};

class StatsThreadLocalStoreTest : public testing::Test, public RawStatDataAllocator {
public:
StatsThreadLocalStoreTest() {
Expand Down
29 changes: 28 additions & 1 deletion test/integration/integration_admin_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,37 @@ TEST_P(IntegrationAdminTest, Admin) {
response = IntegrationUtil::makeSingleRequest(lookupPort("admin"), "GET", "/stats?format=json",
"", downstreamProtocol(), version_);
EXPECT_TRUE(response->complete());
EXPECT_STREQ("200", response->headers().Status()->value().c_str());

Json::ObjectSharedPtr statsjson = Json::Factory::loadFromString(response->body());
EXPECT_TRUE(statsjson->hasObject("stats"));
EXPECT_STREQ("application/json", ContentType(response));
EXPECT_STREQ("200", response->headers().Status()->value().c_str());

uint64_t histogram_count = 0;
for (Json::ObjectSharedPtr obj_ptr : statsjson->getObjectArray("stats")) {
if (obj_ptr->hasObject("histograms")) {
histogram_count++;
const Json::ObjectSharedPtr& histograms_ptr = obj_ptr->getObject("histograms");
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused here: you are looking for 'histograms' plural so I was expecting another loop over all the histograms. Can you explain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a toplevel "histograms" element that has supported_quantiles array and computed_quantiels, which is again an array of actual histogram computed values in the form of "name" and "values" (interval and cumulative) - the last array is repeated as many times as there are histograms"

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still not entirely clear on the cardinality of variouis things from looking at the code. If we actually add a second histogram and make the test expect 2?

Copy link
Contributor Author

@ramaraochavali ramaraochavali May 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

no. it is still 1. The computed_quantiles array size would be equivalent to the number of histograms.
Cardinality of histograms : 1
Cardinality of supported_quantile array : 1
Cardinality if computed_quantiles array = number of histograms (with each element having histogram name and values array which has interval and cumulative values)

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please add a second histogram to this test, so computed_quantiles array has length 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not adding any histograms in the test. I just grab whatever the output is from /stats?format=json and validate that it has histograms, it would definitely some (but non predictable). I am not setting up any thing for this test.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, very cool. Let me rephrase my request. Can you make a new test where you add some specific histogram data with more than one histogram, and check the JSON output?

// Validate that both supported_quantiles and computed_quantiles are present in JSON.
EXPECT_TRUE(histograms_ptr->hasObject("supported_quantiles"));
EXPECT_TRUE(histograms_ptr->hasObject("computed_quantiles"));

const std::vector<Json::ObjectSharedPtr>& computed_quantiles =
histograms_ptr->getObjectArray("computed_quantiles");
EXPECT_GT(computed_quantiles.size(), 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

can we make this EXPECT_EQ something specific here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this count would be equivalent to number of histograms. I do not know exactly how many will come that is why I have added this GT zero condition.

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the number of histograms predictable in this test?


// Validate that each computed_quantile has name and value objects.
EXPECT_TRUE(computed_quantiles[0]->hasObject("name"));
EXPECT_TRUE(computed_quantiles[0]->hasObject("values"));

// Validate that supported and computed quantiles are of the same size.
EXPECT_EQ(histograms_ptr->getObjectArray("supported_quantiles").size(),
computed_quantiles[0]->getObjectArray("values").size());
}
}

// Validate that the stats JSON has exactly one histograms element.
EXPECT_EQ(1, histogram_count);

response = IntegrationUtil::makeSingleRequest(
lookupPort("admin"), "GET", "/stats?format=prometheus", "", downstreamProtocol(), version_);
Expand Down
1 change: 1 addition & 0 deletions test/server/http/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ envoy_cc_test(
"//source/common/http:message_lib",
"//source/common/json:json_loader_lib",
"//source/common/profiler:profiler_lib",
"//source/common/stats:thread_local_store_lib",
"//source/server/http:admin_lib",
"//test/mocks/runtime:runtime_mocks",
"//test/mocks/server:server_mocks",
Expand Down
Loading