chenBright commented on code in PR #3557:
URL: https://github.com/apache/brpc/pull/3557#discussion_r4069132530
##########
src/bvar/latency_recorder.cpp:
##########
@@ -235,6 +240,16 @@ int LatencyRecorder::expose(const butil::StringPiece&
prefix1,
_latency.set_debug_name(prefix);
_latency_percentile.set_debug_name(prefix);
+ // Exposing a LatencyRecorder is all-or-nothing: the prometheus exporter
+ // turns the sub-bvars below back into one summary and would happily write
+ // that summary out with empty values if only some of them made it.
+ bool expose_succeeded = false;
+ BUTIL_SCOPE_EXIT {
+ if (!expose_succeeded) {
+ hide();
+ }
+ };
Review Comment:
In d01fd3e16267686a38680fe20454085c8371d41e , Moved the deduplication into
`PrometheusMetricsDumper`, which is the only place that knows about the
synthesized names. It records every metric name it writes during a scrape,
including the ones it makes up and the ones the mbvar pass writes, and skips a
family whose name is already taken after logging an error, all or nothing so
that half a summary never reaches the scrape.
The name reservation registry is gone with it, so `expose()` keeps its old
behaviour and nothing that used to expose successfully starts failing. A
collision is now reported at scrape time instead, and prometheus still gets a
response it can parse.
--
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]
