codelipenghui commented on code in PR #19440:
URL: https://github.com/apache/pulsar/pull/19440#discussion_r1102222860
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/data/BrokerLoadData.java:
##########
@@ -187,4 +191,35 @@ public String toString(ServiceConfiguration conf) {
);
}
+ public List<Metrics> toMetrics(String advertisedBrokerAddress) {
+ var metrics = new ArrayList<Metrics>();
+ var dimensions = new HashMap<String, String>();
+ dimensions.put("metric", "loadBalancing");
+ dimensions.put("broker", advertisedBrokerAddress);
Review Comment:
@heesung-sn
> However, as I said above, some existing pulsar metrics explicitly emit
this broker label, and I kept them here for the backward compatibility.
Could you please give more context about this one? I haven't got the key
point of what is the compatibility issue that we need to resolve. We don't have
the `broker` label for all the metrics that exposed by broker. And it will be
duplicated with `instance` label for most cases. So we need a strong reason to
add the `broker` label to show it is necessary and the `instance` label will
not be a good substitute here.
--
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]
