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 1 commit
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
43 changes: 42 additions & 1 deletion docs/root/operations/admin.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,48 @@ 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.

The JSON structure would as shown below.

.. code-block:: json

{
"stats": [
{
"name": "example.counter",
"value": 0
},
{
"name": "example.gauge",
"value": 0
},
{
"histograms": {
"supported_quantiles": [...],
Copy link
Member

Choose a reason for hiding this comment

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

Could you add real values here, so the user knows what the array will look like (as in, is it an array of doubles or subobjects)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will add.

"computed_quantiles": [
{
"name": "example.histogram",
"values": [
{
"interval": null,
Copy link
Member

Choose a reason for hiding this comment

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

As above, it would be nice to see an example of a histogram with non-null quantiles values since that will be the common case. But it'd also be nice to keep some sort of null example so the user knows what a no-data hist will look like.

Side note: will this ever print histograms that have no samples in both cumulative and internal since that would mean its used() method would return false?

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 will make those changes. and regarding used, it currently prints all histograms. And I think it is a good point, it would be good filter the unused ones. I will make that change as well.

"cumulative": null
},
{
"interval": null,
"cumulative": null
},
...
]
}
]
}
}
]
}

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

Expand Down
39 changes: 27 additions & 12 deletions source/server/http/admin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,39 +520,54 @@ 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);
rapidjson::Value computed_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);
quantile_obj.AddMember("interval", interval_value, allocator);
Value cumulative_value;
if (!std::isnan(histogram->cumulativeStatistics().computedQuantiles()[i])) {
cumulative_value.SetDouble(histogram->cumulativeStatistics().computedQuantiles()[i]);
}
quantile_obj.AddMember("cumulative_value", cumulative_value, allocator);
quantile_array.PushBack(quantile_obj, allocator);
quantile_obj.AddMember("cumulative", cumulative_value, allocator);
computed_quantile_array.PushBack(quantile_obj, allocator);
}
histogram_obj.AddMember("quantiles", quantile_array, allocator);
stats_array.PushBack(histogram_obj, allocator);
histogram_obj.AddMember("values", computed_quantile_array, allocator);
histogram_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);
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