Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ behavior_changes:

minor_behavior_changes:
# *Changes that may cause incompatibilities for some users, but should not for most*
- area: stats
change: |
Optimized prometheus stats endpoint. Users should see a roughly 30-40% latency improvement in calls to the endpoint
for cases where the scrape results in lots of cluster stats.
There should be no visible changes to users, or incompatibilities.
bug_fixes:
# *Changes expected to improve the state of the world and are unlikely to have negative effects*
Expand Down
3 changes: 3 additions & 0 deletions envoy/stats/primitive_stats.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ class PrimitiveMetricMetadata {
const std::string& tagExtractedName() const { return tag_extracted_name_; }
const std::string& name() const { return name_; }
const Stats::TagVector& tags() const { return tags_; }

// Returns the current tags, leaves tags_ in an unknown state.
Stats::TagVector getAndClearTags() { return std::move(tags_); }
bool used() const { return true; }
bool hidden() const { return false; }

Expand Down
199 changes: 154 additions & 45 deletions source/server/admin/prometheus_stats.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ namespace Server {

namespace {

constexpr absl::string_view kCounter = "counter";
constexpr absl::string_view kGauge = "gauge";

const Regex::CompiledGoogleReMatcher& promRegex() {
CONSTRUCT_ON_FIRST_USE(Regex::CompiledGoogleReMatcherNoSafetyChecks, "[^a-zA-Z0-9_]");
}
Expand All @@ -34,6 +37,15 @@ std::string sanitizeName(const absl::string_view name) {
return promRegex().replaceAll(name, "_");
}

// same logic as above, but does it in place (no allocations)
void sanitizeNameInPlace(std::string& name) {
for (char& c : name) {
if (!((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '_')) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

would it be faster to use a constexpr bool array with the valid/invalid state? THis looks like a lot of conditionals. Your benchmark can tell us.

Similar pattern:

static constexpr uint32_t needs_slow_sanitizer[256] = {

Though apparently when I wrote that I measured that it was faster to use a uint32_t array rather than bool.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Let me try it out, good suggestion!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I tried out having this as a lookup table, and, interestingly, it was a pretty significant slowdown slowdown:

constexpr std::array<bool, 256> createLookupTable() {
    std::array<bool, 256> table{0};
    for (size_t i = 0; i < 256; i++) {
        char c = static_cast<char>(i);
        table[i] = !((c >= 'a' && c <= 'z') || 
                    (c >= 'A' && c <= 'Z') ||
                    (c >= '0' && c <= '9') ||
                     c == '_');
    }
    return table;
}

constexpr std::array<bool, 256> lookupTable = createLookupTable();

I may take a look at the assembly after work, but it seems like the current loop gets turned into SIMD instructions by LLVM at the very least. For example: https://godbolt.org/z/75Meb1d9x

c = '_';
}
}
}

/**
* Take tag values and sanitize it for text serialization, according to
* Prometheus conventions.
Expand Down Expand Up @@ -76,9 +88,9 @@ class TextFormat : public PrometheusStatsFormatter::OutputFormat {
}

void generateOutput(Buffer::Instance& output,
const std::vector<const Stats::PrimitiveCounterSnapshot*>& counters,
std::vector<Stats::PrimitiveCounterSnapshot*>&& counters,
const std::string& prefixed_tag_extracted_name) const override {
generateNumericOutput(output, counters, prefixed_tag_extracted_name);
generateNumericOutput(output, std::move(counters), prefixed_tag_extracted_name);
}

void generateOutput(Buffer::Instance& output, const std::vector<const Stats::Gauge*>& gauges,
Expand All @@ -87,9 +99,9 @@ class TextFormat : public PrometheusStatsFormatter::OutputFormat {
}

void generateOutput(Buffer::Instance& output,
const std::vector<const Stats::PrimitiveGaugeSnapshot*>& gauges,
std::vector<Stats::PrimitiveGaugeSnapshot*>&& gauges,
const std::string& prefixed_tag_extracted_name) const override {
generateNumericOutput(output, gauges, prefixed_tag_extracted_name);
generateNumericOutput(output, std::move(gauges), prefixed_tag_extracted_name);
}

void generateOutput(Buffer::Instance& output,
Expand Down Expand Up @@ -124,7 +136,7 @@ class TextFormat : public PrometheusStatsFormatter::OutputFormat {
for (const auto* text_readout : text_readouts) {
auto tags = text_readout->tags();
tags.push_back(Stats::Tag{"text_value", text_readout->value()});
const std::string formattedTags = PrometheusStatsFormatter::formattedTags(tags);
const std::string formattedTags = PrometheusStatsFormatter::formattedTags(std::move(tags));
output.add(fmt::format("{0}{{{1}}} 0\n", prefixed_tag_extracted_name, formattedTags));
}
}
Expand All @@ -139,12 +151,10 @@ class TextFormat : public PrometheusStatsFormatter::OutputFormat {
void generateNumericOutput(Buffer::Instance& output, const std::vector<const StatType*>& metrics,
const std::string& prefixed_tag_extracted_name) const {
absl::string_view type;
if constexpr (std::is_same_v<Stats::Counter, StatType> ||
std::is_same_v<Stats::PrimitiveCounterSnapshot, StatType>) {
type = "counter";
} else if constexpr (std::is_same_v<Stats::Gauge, StatType> ||
std::is_same_v<Stats::PrimitiveGaugeSnapshot, StatType>) {
type = "gauge";
if constexpr (std::is_same_v<Stats::Counter, StatType>) {
Comment thread
nshipilov marked this conversation as resolved.
type = kCounter;
} else if constexpr (std::is_same_v<Stats::Gauge, StatType>) {
type = kGauge;
} else {
static_assert(false, "Unexpected StatsType");
}
Expand All @@ -157,6 +167,27 @@ class TextFormat : public PrometheusStatsFormatter::OutputFormat {
}
}

template <class StatType>
void generateNumericOutput(Buffer::Instance& output, std::vector<StatType*>&& metrics,
const std::string& prefixed_tag_extracted_name) const {
absl::string_view type;
if constexpr (std::is_same_v<Stats::PrimitiveCounterSnapshot, StatType>) {
Comment thread
nshipilov marked this conversation as resolved.
type = kCounter;
} else if constexpr (std::is_same_v<Stats::PrimitiveGaugeSnapshot, StatType>) {
type = kGauge;
} else {
static_assert(false, "Unexpected StatsType");
}

generateTypeOutput(output, type, prefixed_tag_extracted_name);
for (auto* metric : metrics) {
const std::string formatted_tags =
PrometheusStatsFormatter::formattedTags(metric->getAndClearTags());
output.add(fmt::format("{0}{{{1}}} {2}\n", prefixed_tag_extracted_name, formatted_tags,
metric->value()));
}
}

/*
* Returns the prometheus output for a histogram. The output is a multi-line string (with embedded
* newlines) that contains all the individual bucket counts and sum/count for a single histogram
Expand All @@ -168,8 +199,11 @@ class TextFormat : public PrometheusStatsFormatter::OutputFormat {
generateTypeOutput(output, "histogram", prefixed_tag_extracted_name);

for (const auto* histogram : histograms) {
const std::string tags = PrometheusStatsFormatter::formattedTags(histogram->tags());
const std::string hist_tags = histogram->tags().empty() ? EMPTY_STRING : (tags + ",");
auto histogram_tags = histogram->tags();
const bool empty_tags = histogram_tags.empty();

const std::string tags = PrometheusStatsFormatter::formattedTags(std::move(histogram_tags));
const std::string hist_tags = empty_tags ? EMPTY_STRING : (tags + ",");

const Stats::HistogramStatistics& stats = histogram->cumulativeStatistics();
Stats::ConstSupportedBuckets& supported_buckets = stats.supportedBuckets();
Expand Down Expand Up @@ -243,9 +277,9 @@ class ProtobufFormat : public PrometheusStatsFormatter::OutputFormat {

// Return the prometheus output for a group of PrimitiveCounters.
void generateOutput(Buffer::Instance& output,
const std::vector<const Stats::PrimitiveCounterSnapshot*>& counters,
std::vector<Stats::PrimitiveCounterSnapshot*>&& counters,
const std::string& prefixed_tag_extracted_name) const override {
generateNumericOutput(output, counters, prefixed_tag_extracted_name,
generateNumericOutput(output, std::move(counters), prefixed_tag_extracted_name,
io::prometheus::client::MetricType::COUNTER);
}

Expand Down Expand Up @@ -286,9 +320,9 @@ class ProtobufFormat : public PrometheusStatsFormatter::OutputFormat {

// Return the prometheus output for a group of PrimitiveGauges.
void generateOutput(Buffer::Instance& output,
const std::vector<const Stats::PrimitiveGaugeSnapshot*>& gauges,
std::vector<Stats::PrimitiveGaugeSnapshot*>&& gauges,
const std::string& prefixed_tag_extracted_name) const override {
generateNumericOutput(output, gauges, prefixed_tag_extracted_name,
generateNumericOutput(output, std::move(gauges), prefixed_tag_extracted_name,
io::prometheus::client::MetricType::GAUGE);
}

Expand Down Expand Up @@ -328,6 +362,17 @@ class ProtobufFormat : public PrometheusStatsFormatter::OutputFormat {
}
}

void addLabelsToMetric(io::prometheus::client::Metric* metric,
std::vector<Stats::Tag>&& tags) const {
metric->mutable_label()->Reserve(tags.size());
for (auto& tag : tags) {
auto* label = metric->add_label();
sanitizeNameInPlace(tag.name_);
label->set_name(std::move(tag.name_));
label->set_value(sanitizeValue(tag.value_));
}
}

template <class StatType>
void generateNumericOutput(Buffer::Instance& output, const std::vector<const StatType*>& metrics,
const std::string& prefixed_tag_extracted_name,
Expand Down Expand Up @@ -356,6 +401,38 @@ class ProtobufFormat : public PrometheusStatsFormatter::OutputFormat {
writeDelimitedMessage(metric_family, output);
}

template <class StatType>
void generateNumericOutput(Buffer::Instance& output, std::vector<StatType*>&& metrics,
const std::string& prefixed_tag_extracted_name,
io::prometheus::client::MetricType type) const {
ASSERT(!metrics.empty());

io::prometheus::client::MetricFamily metric_family;
metric_family.set_name(prefixed_tag_extracted_name);
metric_family.set_type(type);
metric_family.mutable_metric()->Reserve(metrics.size());

for (auto* metric : metrics) {
auto* prom_metric = metric_family.add_metric();

uint64_t value = metric->value();
auto tags = metric->getAndClearTags();

addLabelsToMetric(prom_metric, std::move(tags));

// Set value based on type
if (type == io::prometheus::client::MetricType::COUNTER) {
auto* counter = prom_metric->mutable_counter();
counter->set_value(value);
} else {
auto* gauge = prom_metric->mutable_gauge();
gauge->set_value(value);
}
}

writeDelimitedMessage(metric_family, output);
}

void generateHistogramOutput(io::prometheus::client::MetricFamily& metric_family,
const std::vector<const Stats::ParentHistogram*>& histograms) const {
metric_family.set_type(io::prometheus::client::MetricType::HISTOGRAM);
Expand Down Expand Up @@ -681,23 +758,39 @@ uint64_t outputStatType(Buffer::Instance& response, const StatsParams& params,
// comparison.
const Stats::SymbolTable& global_symbol_table = metrics.front()->constSymbolTable();

// Sorted collection of metrics sorted by their tagExtractedName, to satisfy the requirements
// of the exposition format.
std::map<Stats::StatName, StatTypeUnsortedCollection, Stats::StatNameLessThan> groups(
global_symbol_table);
// Collection of metrics by their tagExtractedName.
// Sorting will be done on the names separately.
absl::flat_hash_map<Stats::StatName, StatTypeUnsortedCollection> groups;
absl::flat_hash_set<Stats::StatName> unique_stat_names;

for (const auto& metric : metrics) {
ASSERT(&global_symbol_table == &metric->constSymbolTable());
if (!params.shouldShowMetric(*metric)) {
continue;
}
groups[metric->tagExtractedStatName()].push_back(metric.get());
Stats::StatName tag_extracted_stat_name = metric->tagExtractedStatName();
unique_stat_names.insert(tag_extracted_stat_name);
groups[tag_extracted_stat_name].push_back(metric.get());
}

std::vector<Stats::StatName> sorted_stat_names;
sorted_stat_names.reserve(unique_stat_names.size());
for (auto it = unique_stat_names.begin(); it != unique_stat_names.end();) {
auto extract_it = it;
it++;
absl::flat_hash_set<Stats::StatName>::node_type nh = unique_stat_names.extract(extract_it);
ASSERT(nh);
sorted_stat_names.push_back(std::move(nh.value()));
}

Stats::StatNameLessThan comp(global_symbol_table);
std::sort(sorted_stat_names.begin(), sorted_stat_names.end(), comp);

auto result = groups.size();
for (auto& group : groups) {
for (auto& group_name : sorted_stat_names) {
auto& group = groups[group_name];
const absl::optional<std::string> prefixed_tag_extracted_name =
PrometheusStatsFormatter::metricName(global_symbol_table.toString(group.first),
PrometheusStatsFormatter::metricName(global_symbol_table.toString(group_name),
custom_namespaces);
if (!prefixed_tag_extracted_name.has_value()) {
--result;
Expand All @@ -707,17 +800,16 @@ uint64_t outputStatType(Buffer::Instance& response, const StatsParams& params,
// Sort before producing the final output to satisfy the "preferred" ordering from the
// prometheus spec: metrics will be sorted by their tags' textual representation, which will
// be consistent across calls.
std::sort(group.second.begin(), group.second.end(), MetricLessThan());
std::sort(group.begin(), group.end(), MetricLessThan());

output_format.generateOutput(response, group.second, prefixed_tag_extracted_name.value());
output_format.generateOutput(response, group, prefixed_tag_extracted_name.value());
}
return result;
}

template <class StatType, class OutputFormat>
uint64_t outputPrimitiveStatType(Buffer::Instance& response, const StatsParams& params,
const std::vector<StatType>& metrics,
const OutputFormat& output_format,
std::vector<StatType>&& metrics, const OutputFormat& output_format,
const Stats::CustomStatNamespaces& custom_namespaces) {

/*
Expand All @@ -735,7 +827,7 @@ uint64_t outputPrimitiveStatType(Buffer::Instance& response, const StatsParams&
// be sorted before producing the final output to satisfy the "preferred" ordering from the
// prometheus spec: metrics will be sorted by their tags' textual representation, which will be
// consistent across calls.
using StatTypeUnsortedCollection = std::vector<const StatType*>;
using StatTypeUnsortedCollection = std::vector<StatType*>;

// Return early to avoid crashing when getting the symbol table from the first metric.
if (metrics.empty()) {
Expand All @@ -744,19 +836,34 @@ uint64_t outputPrimitiveStatType(Buffer::Instance& response, const StatsParams&

// Sorted collection of metrics sorted by their tagExtractedName, to satisfy the requirements
// of the exposition format.
std::map<std::string, StatTypeUnsortedCollection> groups;
absl::flat_hash_map<std::string, StatTypeUnsortedCollection> groups;
absl::flat_hash_set<std::string> unique_group_names;

for (const auto& metric : metrics) {
for (auto& metric : metrics) {
if (!params.shouldShowMetric(metric)) {
continue;
}
groups[metric.tagExtractedName()].push_back(&metric);
std::string tag_extracted_name = metric.tagExtractedName();
unique_group_names.insert(tag_extracted_name);
groups[std::move(tag_extracted_name)].push_back(&metric);
}

std::vector<std::string> sorted_group_names;
sorted_group_names.reserve(unique_group_names.size());
for (auto it = unique_group_names.begin(); it != unique_group_names.end();) {
auto extract_it = it;
it++;
absl::flat_hash_set<std::string>::node_type nh = unique_group_names.extract(extract_it);
ASSERT(nh);
sorted_group_names.push_back(std::move(nh.value()));
}
std::sort(sorted_group_names.begin(), sorted_group_names.end());

auto result = groups.size();
for (auto& group : groups) {
for (auto& group_name : sorted_group_names) {
auto& group = groups[group_name];
const absl::optional<std::string> prefixed_tag_extracted_name =
PrometheusStatsFormatter::metricName(group.first, custom_namespaces);
PrometheusStatsFormatter::metricName(std::move(group_name), custom_namespaces);
if (!prefixed_tag_extracted_name.has_value()) {
--result;
continue;
Expand All @@ -765,9 +872,9 @@ uint64_t outputPrimitiveStatType(Buffer::Instance& response, const StatsParams&
// Sort before producing the final output to satisfy the "preferred" ordering from the
// prometheus spec: metrics will be sorted by their tags' textual representation, which will
// be consistent across calls.
std::sort(group.second.begin(), group.second.end(), PrimitiveMetricSnapshotLessThan());
std::sort(group.begin(), group.end(), PrimitiveMetricSnapshotLessThan());

output_format.generateOutput(response, group.second, prefixed_tag_extracted_name.value());
output_format.generateOutput(response, std::move(group), prefixed_tag_extracted_name.value());
}
return result;
}
Expand Down Expand Up @@ -819,11 +926,12 @@ bool useProtobufFormat(const StatsParams& params, const Http::RequestHeaderMap&

} // namespace

std::string PrometheusStatsFormatter::formattedTags(const std::vector<Stats::Tag>& tags) {
std::string PrometheusStatsFormatter::formattedTags(std::vector<Stats::Tag>&& tags) {
std::vector<std::string> buf;
buf.reserve(tags.size());
for (const Stats::Tag& tag : tags) {
buf.push_back(fmt::format("{}=\"{}\"", sanitizeName(tag.name_), sanitizeValue(tag.value_)));
for (Stats::Tag& tag : tags) {
sanitizeNameInPlace(tag.name_);
buf.push_back(fmt::format("{}=\"{}\"", tag.name_, sanitizeValue(tag.value_)));
}
return absl::StrJoin(buf, ",");
}
Expand Down Expand Up @@ -854,7 +962,7 @@ absl::Status PrometheusStatsFormatter::validateParams(const StatsParams& params,
}

absl::optional<std::string>
PrometheusStatsFormatter::metricName(const std::string& extracted_name,
PrometheusStatsFormatter::metricName(std::string&& extracted_name,
const Stats::CustomStatNamespaces& custom_namespaces) {
const absl::optional<absl::string_view> custom_namespace_stripped =
custom_namespaces.stripRegisteredPrefix(extracted_name);
Expand All @@ -876,7 +984,8 @@ PrometheusStatsFormatter::metricName(const std::string& extracted_name,
// If it does not have a custom namespace, add namespacing prefix to avoid conflicts, as per best
// practice: https://prometheus.io/docs/practices/naming/#metric-names Also, naming conventions on
// https://prometheus.io/docs/concepts/data_model/
return absl::StrCat("envoy_", sanitizeName(extracted_name));
sanitizeNameInPlace(extracted_name);
return absl::StrCat("envoy_", extracted_name);
}

uint64_t PrometheusStatsFormatter::generateWithOutputFormat(
Expand Down Expand Up @@ -938,10 +1047,10 @@ uint64_t PrometheusStatsFormatter::generateWithOutputFormat(
},
[&](Stats::PrimitiveGaugeSnapshot&& metric) { host_gauges.emplace_back(std::move(metric)); });

metric_name_count +=
outputPrimitiveStatType(response, params, host_counters, output_format, custom_namespaces);
metric_name_count +=
outputPrimitiveStatType(response, params, host_gauges, output_format, custom_namespaces);
metric_name_count += outputPrimitiveStatType(response, params, std::move(host_counters),
output_format, custom_namespaces);
metric_name_count += outputPrimitiveStatType(response, params, std::move(host_gauges),
output_format, custom_namespaces);

return metric_name_count;
}
Expand Down
Loading