chenBright commented on code in PR #3549:
URL: https://github.com/apache/brpc/pull/3549#discussion_r4033467936
##########
src/bvar/multi_dimension_inl.h:
##########
@@ -269,32 +269,51 @@ MultiDimension<T, KeyType, Shared>::dump_impl(Dumper*
dumper, const DumpOptions*
continue;
}
- // latency
- std::ostringstream oss_latency_key;
- make_dump_key(oss_latency_key, label_name, "_latency");
- if (dumper->dump_mvar(oss_latency_key.str(),
std::to_string(bvar->latency()))) {
- n++;
- }
// latency_percentiles
// p1/p2/p3
- int latency_percentiles[3] {FLAGS_bvar_latency_p1,
FLAGS_bvar_latency_p2, FLAGS_bvar_latency_p3};
+ int latency_percentiles[3] {
+ FLAGS_bvar_latency_p1,
+ FLAGS_bvar_latency_p2,
+ FLAGS_bvar_latency_p3
+ };
for (auto lp : latency_percentiles) {
+ // Quantile must be a fraction, e.g. 0.99 for p99.
std::ostringstream oss_lp_key;
- make_dump_key(oss_lp_key, label_name, "_latency", lp);
- if (dumper->dump_mvar(oss_lp_key.str(),
std::to_string(bvar->latency_percentile(lp / 100.0)))) {
+ make_dump_key(oss_lp_key, label_name, "_latency", lp / 100.0);
+ int64_t latency_percentile = bvar->latency_percentile(lp / 100.0);
+ if (dumper->dump_mvar(oss_lp_key.str(),
std::to_string(latency_percentile))) {
n++;
}
}
// 999
std::ostringstream oss_p999_key;
- make_dump_key(oss_p999_key, label_name, "_latency", 999);
- if (dumper->dump_mvar(oss_p999_key.str(),
std::to_string(bvar->latency_percentile(0.999)))) {
+ make_dump_key(oss_p999_key, label_name, "_latency", 0.999);
+ int64_t latency_percentile = bvar->latency_percentile(0.999);
+ if (dumper->dump_mvar(oss_p999_key.str(),
std::to_string(latency_percentile))) {
n++;
}
// 9999
std::ostringstream oss_p9999_key;
- make_dump_key(oss_p9999_key, label_name, "_latency", 9999);
- if (dumper->dump_mvar(oss_p9999_key.str(),
std::to_string(bvar->latency_percentile(0.9999)))) {
+ make_dump_key(oss_p9999_key, label_name, "_latency", 0.9999);
+ latency_percentile = bvar->latency_percentile(0.9999);
+ if (dumper->dump_mvar(oss_p9999_key.str(),
std::to_string(latency_percentile))) {
+ n++;
+ }
+ }
+
+ // latency_average comment
+ // The average latency has to be a separate metric rather than a series of
+ // `_latency` without a quantile label, otherwise an aggregation over
+ // `_latency` would silently mix the average into the percentiles.
+ dumper->dump_comment(this->name() + "_avg_latency", METRIC_TYPE_GAUGE);
+ for (auto &label_name : label_names) {
+ LatencyRecorder* bvar = get_stats_impl(label_name);
+ if (nullptr == bvar) {
+ continue;
+ }
Review Comment:
Opt in a7e6236a9bc70ccea361d2049b0b444cbaf3c7b3 .
##########
test/brpc_prometheus_metrics_unittest.cpp:
##########
@@ -88,6 +92,32 @@ TEST(PrometheusMetrics, sanity) {
ASSERT_FALSE(cntl.Failed());
std::string res = cntl.response_attachment().to_string();
LOG(INFO) << "output:\n" << res;
+
+ // The average latency is a separate metric rather than a quantile series,
+ // because the quantile label must be parsable as a float.
+ ASSERT_EQ(std::string::npos, res.find("quantile=\"avg\""));
+ ASSERT_NE(std::string::npos, res.find("# TYPE mlat_avg_latency gauge\n"));
+ ASSERT_NE(std::string::npos, res.find("mlat_avg_latency{label1=\"val1\","
+ "label2=\"val2\"}"));
+ // The single dimension LatencyRecorder uses the same suffix.
+ ASSERT_NE(std::string::npos, res.find("_service_echo_avg_latency "));
+ // Quantile is a fraction rather than an integer.
+ ASSERT_NE(std::string::npos, res.find("quantile=\"0.99\""));
+ ASSERT_NE(std::string::npos, res.find("quantile=\"0.999\""));
+ ASSERT_NE(std::string::npos, res.find("quantile=\"0.9999\""));
+ ASSERT_EQ(std::string::npos, res.find("quantile=\"99\""));
+ ASSERT_EQ(std::string::npos, res.find("quantile=\"999\""));
+ ASSERT_EQ(std::string::npos, res.find("quantile=\"9999\""));
+ ASSERT_NE(std::string::npos,
res.find("mlat_latency{label1=\"val1\",label2=\"val2\","
+ "quantile=\"0.99\"}"));
Review Comment:
fixed in a7e6236a9bc70ccea361d2049b0b444cbaf3c7b3.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
