[GitHub] [pulsar] yyj8 commented on a diff in pull request #18116: [PIP-214][broker]Add broker level metrics statistics and expose to prometheus

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

yyj8 commented on code in PR #18116:
URL: https://github.com/apache/pulsar/pull/18116#discussion_r1007035859


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/prometheus/NamespaceStatsAggregator.java:
##########
@@ -70,7 +71,19 @@ public static void generate(PulsarService pulsar, boolean includeTopicMetrics, b
         LongAdder topicsCount = new LongAdder();
         Map<String, Long> localNamespaceTopicCount = new HashMap<>();
 
-        printDefaultBrokerStats(stream, cluster);
+        LongAdder brokerTopicsCount = new LongAdder();

Review Comment:
   Thank you for your suggestion. This is a very good idea. I will make the following adjustments:
   
   First ,create a new class `AggregatedBrokerStats` to record broker metrics, which can be consistent with the namespace metrics design;
   For detailed code changes, please refer to：
   [pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/prometheus/AggregatedBrokerStats.java](https://github.com/apache/pulsar/pull/18116/commits/f2b851979ddb067264e6970e96a29ddc6689838c#diff-133ea36ffb45e70b4f381511fa3552e44b29db184f8d6d9ad4a7097ec4687d3a)
   
   Second, through `brokerStats.updateStats (topicStats);`  to update the broker's metrics;
   ```
   public class NamespaceStatsAggregator {
   
       private static final FastThreadLocal<AggregatedBrokerStats> localBrokerStats =
               new FastThreadLocal<>() {
                   @Override
                   protected AggregatedBrokerStats initialValue() {
                       return new AggregatedBrokerStats();
                   }
               };
   ```
   ```
   String cluster = pulsar.getConfiguration().getClusterName();
   AggregatedBrokerStats brokerStats = localBrokerStats.get();
   brokerStats.reset();
   AggregatedNamespaceStats namespaceStats = localNamespaceStats.get();
   ```
   
   ```
   //Record broker level metrics
   brokerStats.updateStats(topicStats);
   
   if (includeTopicMetrics) {
       topicsCount.add(1);
       TopicStats.printTopicStats(stream, topicStats, compactorMXBean, cluster, namespace, name,
               splitTopicAndPartitionIndexLabel);
   } else {
       namespaceStats.updateStats(topicStats);
   }              
   ```
   
   Third, through `printBrokerStats (stream, cluster, brokerStats); ` to print broker metrics;
   ```
   if (includeTopicMetrics) {
          printTopicsCountStats(stream, localNamespaceTopicCount, cluster);
   }
   
    //Print broker level metrics to prometheus
   printBrokerStats(stream, cluster, brokerStats);
   ```
   For detailed code changes, please refer to：
   [pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/prometheus/NamespaceStatsAggregator.java](https://github.com/apache/pulsar/pull/18116/commits/f2b851979ddb067264e6970e96a29ddc6689838c#diff-b3f64ff76fdabb9e471435147298c3c707cbecae09023f0d86cf06f6ad78cbda)



-- 
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: commits-unsubscribe@pulsar.apache.org

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