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: envoy prometheus endpoint fails promlint #2597 #2757

Merged
merged 2 commits into from
Mar 16, 2018
Merged

stats: envoy prometheus endpoint fails promlint #2597 #2757

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc8d067
stats: envoy prometheus endpoint fails promlint fixes #2597
pitiwari Mar 8, 2018
eaf30b7
more changes based on review
pitiwari Mar 13, 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
19 changes: 14 additions & 5 deletions source/server/http/admin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,22 +427,31 @@ std::string PrometheusStatsFormatter::metricName(const std::string& extractedNam
return fmt::format("envoy_{0}", sanitizeName(extractedName));
}

void PrometheusStatsFormatter::statsAsPrometheus(const std::list<Stats::CounterSharedPtr>& counters,
const std::list<Stats::GaugeSharedPtr>& gauges,
Buffer::Instance& response) {
uint64_t
PrometheusStatsFormatter::statsAsPrometheus(const std::list<Stats::CounterSharedPtr>& counters,
const std::list<Stats::GaugeSharedPtr>& gauges,
Buffer::Instance& response) {
std::unordered_set<std::string> metric_type_tracker;
for (const auto& counter : counters) {
const std::string tags = formattedTags(counter->tags());
const std::string metric_name = metricName(counter->tagExtractedName());
response.add(fmt::format("# TYPE {0} counter\n", metric_name));
if (metric_type_tracker.find(metric_name) == metric_type_tracker.end()) {
metric_type_tracker.insert(metric_name);
response.add(fmt::format("# TYPE {0} counter\n", metric_name));
}
response.add(fmt::format("{0}{{{1}}} {2}\n", metric_name, tags, counter->value()));
}

for (const auto& gauge : gauges) {
const std::string tags = formattedTags(gauge->tags());
const std::string metric_name = metricName(gauge->tagExtractedName());
response.add(fmt::format("# TYPE {0} gauge\n", metric_name));
if (metric_type_tracker.find(metric_name) == metric_type_tracker.end()) {
metric_type_tracker.insert(metric_name);
response.add(fmt::format("# TYPE {0} gauge\n", metric_name));
}
response.add(fmt::format("{0}{{{1}}} {2}\n", metric_name, tags, gauge->value()));
}
return metric_type_tracker.size();
}

std::string AdminImpl::statsAsJson(const std::map<std::string, uint64_t>& all_stats) {
Expand Down
7 changes: 4 additions & 3 deletions source/server/http/admin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,11 @@ class PrometheusStatsFormatter {
/**
* Extracts counters and gauges and relevant tags, appending them to
* the response buffer after sanitizing the metric / label names.
* @return uint64_t total number of metric types inserted in response.
*/
static void statsAsPrometheus(const std::list<Stats::CounterSharedPtr>& counters,
const std::list<Stats::GaugeSharedPtr>& gauges,
Buffer::Instance& response);
static uint64_t statsAsPrometheus(const std::list<Stats::CounterSharedPtr>& counters,
Copy link
Member

Choose a reason for hiding this comment

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

nit: please update the method documentation with @return uint64_t explanation of return value.

const std::list<Stats::GaugeSharedPtr>& gauges,
Buffer::Instance& response);
/**
* Format the given tags, returning a string as a comma-separated list
* of <tag_name>="<tag_value>" pairs.
Expand Down
117 changes: 117 additions & 0 deletions test/server/http/admin_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "common/http/message_impl.h"
#include "common/json/json_loader.h"
#include "common/profiler/profiler.h"
#include "common/stats/stats_impl.h"

#include "server/http/admin.h"

Expand Down Expand Up @@ -293,5 +294,121 @@ TEST(PrometheusStatsFormatter, FormattedTags) {
EXPECT_EQ(expected, actual);
}

TEST(PrometheusStatsFormatter, MetricNameCollison) {

// Create two counters and two gauges with each pair having the same name,
// but having different tag names and values.
//`statsAsPrometheus()` should return two implying it found two unique stat names
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, one more nit that I didn't notice the first few times through: please remove the backticks from statsAsPrometheus().


Stats::HeapRawStatDataAllocator alloc;
std::list<Stats::CounterSharedPtr> counters;
std::list<Stats::GaugeSharedPtr> gauges;

{
std::string name = "cluster.test_cluster_1.upstream_cx_total";
std::vector<Stats::Tag> cluster_tags;
Stats::Tag tag = {"a.tag-name", "a.tag-value"};
cluster_tags.push_back(tag);
Stats::RawStatData* data = alloc.alloc(name);
Stats::CounterSharedPtr c = std::make_shared<Stats::CounterImpl>(*data, alloc, std::move(name),
std::move(cluster_tags));
counters.push_back(c);
}

{
std::string name = "cluster.test_cluster_1.upstream_cx_total";
std::vector<Stats::Tag> cluster_tags;
Stats::Tag tag = {"another_tag_name", "another_tag-value"};
cluster_tags.push_back(tag);
Stats::RawStatData* data = alloc.alloc(name);
Stats::CounterSharedPtr c = std::make_shared<Stats::CounterImpl>(*data, alloc, std::move(name),
std::move(cluster_tags));
counters.push_back(c);
}

{
std::vector<Stats::Tag> cluster_tags;
std::string name = "cluster.test_cluster_2.upstream_cx_total";
Stats::Tag tag = {"another_tag_name_3", "another_tag_3-value"};
cluster_tags.push_back(tag);
Stats::RawStatData* data = alloc.alloc(name);
Stats::GaugeSharedPtr g =
std::make_shared<Stats::GaugeImpl>(*data, alloc, std::move(name), std::move(cluster_tags));
gauges.push_back(g);
}

{
std::vector<Stats::Tag> cluster_tags;
std::string name = "cluster.test_cluster_2.upstream_cx_total";
Stats::Tag tag = {"another_tag_name_4", "another_tag_4-value"};
cluster_tags.push_back(tag);
Stats::RawStatData* data = alloc.alloc(name);
Stats::GaugeSharedPtr g =
std::make_shared<Stats::GaugeImpl>(*data, alloc, std::move(name), std::move(cluster_tags));
gauges.push_back(g);
}

Buffer::OwnedImpl response;
EXPECT_EQ(2UL, PrometheusStatsFormatter::statsAsPrometheus(counters, gauges, response));
}

TEST(PrometheusStatsFormatter, UniqueMetricName) {

// Create two counters and two gauges, all with unique names.
// statsAsPrometheus() should return four implying it found
// four unique stat names.

Stats::HeapRawStatDataAllocator alloc;
std::list<Stats::CounterSharedPtr> counters;
std::list<Stats::GaugeSharedPtr> gauges;

{
std::string name = "cluster.test_cluster_1.upstream_cx_total";
std::vector<Stats::Tag> cluster_tags;
Stats::Tag tag = {"a.tag-name", "a.tag-value"};
cluster_tags.push_back(tag);
Stats::RawStatData* data = alloc.alloc(name);
Stats::CounterSharedPtr c = std::make_shared<Stats::CounterImpl>(*data, alloc, std::move(name),
std::move(cluster_tags));
counters.push_back(c);
}

{
std::string name = "cluster.test_cluster_2.upstream_cx_total";
std::vector<Stats::Tag> cluster_tags;
Stats::Tag tag = {"another_tag_name", "another_tag-value"};
cluster_tags.push_back(tag);
Stats::RawStatData* data = alloc.alloc(name);
Stats::CounterSharedPtr c = std::make_shared<Stats::CounterImpl>(*data, alloc, std::move(name),
std::move(cluster_tags));
counters.push_back(c);
}

{
std::vector<Stats::Tag> cluster_tags;
std::string name = "cluster.test_cluster_3.upstream_cx_total";
Stats::Tag tag = {"another_tag_name_3", "another_tag_3-value"};
cluster_tags.push_back(tag);
Stats::RawStatData* data = alloc.alloc(name);
Stats::GaugeSharedPtr g =
std::make_shared<Stats::GaugeImpl>(*data, alloc, std::move(name), std::move(cluster_tags));
gauges.push_back(g);
}

{
std::vector<Stats::Tag> cluster_tags;
std::string name = "cluster.test_cluster_4.upstream_cx_total";
Stats::Tag tag = {"another_tag_name_4", "another_tag_4-value"};
cluster_tags.push_back(tag);
Stats::RawStatData* data = alloc.alloc(name);
Stats::GaugeSharedPtr g =
std::make_shared<Stats::GaugeImpl>(*data, alloc, std::move(name), std::move(cluster_tags));
gauges.push_back(g);
}

Buffer::OwnedImpl response;
EXPECT_EQ(4UL, PrometheusStatsFormatter::statsAsPrometheus(counters, gauges, response));
}

} // namespace Server
} // namespace Envoy