[GitHub] [bookkeeper] hangc0276 commented on a change in pull request #2718: fix prometheus metric provider bug and add test to cover label scope …

[GitBox <gi...@apache.org>]

hangc0276 commented on a change in pull request #2718:
URL: https://github.com/apache/bookkeeper/pull/2718#discussion_r638335331



##########
File path: bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusTextFormatUtil.java
##########
@@ -139,25 +139,34 @@ private static void writeQuantile(Writer w, DataSketchesOpStatsLogger opStat, St
         w.append(name)
                 .append("{success=\"").append(success.toString())
                 .append("\",quantile=\"").append(Double.toString(quantile))
-                .append("\", ");
-        writeLabelsNoBraces(w, opStat.getLabels());
+                .append("\"");
+        if (!opStat.getLabels().isEmpty()) {

Review comment:
       ```Java
   // PrometheusMetricsProvider.java
   public StatsLogger getStatsLogger(String scope) {
           return new PrometheusStatsLogger(PrometheusMetricsProvider.this, scope, Collections.emptyMap());
       }
   
   // PrometheusStatsLogger.java
   PrometheusStatsLogger(PrometheusMetricsProvider provider, String scope, Map<String, String> labels) {
           this.provider = provider;
           this.scope = scope;
           this.labels = labels;
       }
   
   @Override
   public OpStatsLogger getOpStatsLogger(String name) {
       return provider.opStats.computeIfAbsent(scopeContext(name), x -> new DataSketchesOpStatsLogger(labels));
   }
   ```
   Yes, it will use `Collections.emptyMap()` to init labels when there are no labels.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org