Copilot commented on code in PR #3557:
URL: https://github.com/apache/brpc/pull/3557#discussion_r4069150559
##########
src/bvar/mvariable.cpp:
##########
@@ -189,6 +198,7 @@ int MVariableBase::expose_impl(const butil::StringPiece&
prefix,
if (entry == nullptr) {
entry = &m[_name];
entry->ref = _ref;
Review Comment:
This still checks collisions only in the mvariable map;
`Variable::expose_impl` performs the equivalent lookup in a separate bvar map,
so a bvar and an mvar such as `foo.bar` and `foo_bar` can both expose
successfully and emit the same normalized Prometheus name with potentially
different types/labels. The PR's cross-bvar/mbvar collision guarantee is
therefore not implemented; both expose paths need to claim the same
normalized-name registry (and release it on hide).
##########
src/bvar/variable.cpp:
##########
@@ -179,7 +201,14 @@ int Variable::expose_impl(const butil::StringPiece& prefix,
}
}
to_underscored_name(&_name, name);
-
+
+ bool expose_succeeded = false;
+ BUTIL_SCOPE_EXIT {
+ if (!expose_succeeded) {
+ _name.clear();
+ }
+ };
+
VarMapWithLock& m = get_var_map(_name);
Review Comment:
The new rollback only clears `_name`; it does not reserve the normalized
name in a registry shared with `MVariableBase`. `Variable::expose_impl()` still
inserts only into `get_var_map()`, while mbvars insert into a separate map, so
a bvar and an mbvar can both successfully expose the same normalized Prometheus
name (for example `foo.bar` and `foo::bar`). This leaves the stated cross
bvar/mbvar collision problem unresolved outside the scrape-time exporter
workaround; coordinate both expose/hide paths through one shared reservation
mechanism.
--
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]
