Copilot commented on code in PR #3557:
URL: https://github.com/apache/brpc/pull/3557#discussion_r4061056478
##########
src/brpc/builtin/prometheus_metrics_service.cpp:
##########
@@ -86,14 +86,96 @@ class PrometheusMetricsDumper : public bvar::Dumper {
butil::StringPiece GetMetricsName(const std::string& name) {
auto pos = name.find_first_of('{');
- int size = (pos == std::string::npos) ? name.size() : pos;
+ int size = pos == std::string::npos ? name.size() : pos;
return butil::StringPiece(name.data(), size);
}
+// Case-insensitive match of [p, end) against the NUL-terminated, lower-case
+// ASCII `word`. The whole rest of the input must be consumed, so "inf" matches
+// but "infx" does not.
+static bool MatchWordIgnoreCase(const char* p, const char* end, const char*
word) {
+ for (; p != end && *word != '\0'; ++p, ++word) {
+ char c = *p;
+ if (c >= 'A' && c <= 'Z') {
+ c = static_cast<char>(c - 'A' + 'a');
+ }
+ if (c != *word) {
+ return false;
+ }
+ }
+ return p == end && *word == '\0';
+}
+
+// Whether `s` is a number prometheus would accept as a sample value: a float64
+// in the decimal, or one of the specials it spells as "+Inf"/"-Inf"/"NaN"
+// (case-insensitive, "Infinity" allowed).
+//
+// Everything else is rejected: a quoted string, the json of a
Window<Histogram>
+// or a compound PassiveStatus, and the bare `true`/'false` of a bool gflag,
+// which sniffing only the first char let through. Skipping such a variable
+// is not cosmetic: one malformed line makes prometheus reject the whole
scrape,
+// not just that one metric.
+//
+// Scans the StringPiece in place, no copy and no allocation. Hexadecimal
+// floats are deliberately not accepted.
+bool IsDumpableToPrometheus(butil::StringPiece s) {
+ const char* p = s.data();
+ const char* const end = p + s.size();
+ if (p == end) {
+ return false;
+ }
+ if (*p == '+' || *p == '-') {
+ ++p;
+ if (p == end) {
+ return false; // a lone sign
+ }
+ }
+ const char c = *p;
+ if (c >= '0' && c <= '9') {
+ // "123", "123." or "123.45".
+ while (p != end && *p >= '0' && *p <= '9') {
+ ++p;
+ }
+ if (p != end && *p == '.') {
+ ++p;
+ while (p != end && *p >= '0' && *p <= '9') {
+ ++p;
+ }
+ }
+ } else if (c == '.') {
+ // ".5": the digits before the point may be omitted, those after may
not.
+ ++p;
+ if (p == end || *p < '0' || *p > '9') {
+ return false;
+ }
+ while (p != end && *p >= '0' && *p <= '9') {
+ ++p;
+ }
+ } else {
+ return MatchWordIgnoreCase(p, end, "inf") ||
+ MatchWordIgnoreCase(p, end, "infinity") ||
+ MatchWordIgnoreCase(p, end, "nan");
Review Comment:
`Infinity` is not a valid Prometheus text-format sample literal; the grammar
accepts `NaN`, `+Inf`, and `-Inf` (with the usual numeric forms), not the
spelled-out `Infinity`. Returning true here causes a value such as `Infinity`
to be emitted and can make the entire `/brpc_metrics` scrape invalid. Please
remove the `infinity` alternative and its corresponding positive tests.
--
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]
