You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by GitBox <gi...@apache.org> on 2022/11/11 05:45:50 UTC

[GitHub] [ratis] sumitagrawl opened a new pull request, #778: RATIS-1741 Add a removeReporterRegistration method

sumitagrawl opened a new pull request, #778:
URL: https://github.com/apache/ratis/pull/778

   ## What changes were proposed in this pull request?
   
   removeReporterRegistration method is provided
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/RATIS-1741
   
   ## How was this patch tested?
   
   Patch is tested adding Unit test with register and remove registration of reporter
   


-- 
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: issues-unsubscribe@ratis.apache.org

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

[GitHub] [ratis] szetszwo commented on a diff in pull request #778: RATIS-1741 Add a removeReporterRegistration method

Posted by GitBox <gi...@apache.org>.
szetszwo commented on code in PR #778:
URL: https://github.com/apache/ratis/pull/778#discussion_r1019862352


##########
ratis-metrics-api/src/main/java/org/apache/ratis/metrics/MetricRegistries.java:
##########
@@ -93,10 +93,20 @@ public static MetricRegistries global() {
    * Add hook to register reporter for the metricRegistry.
    *
    * @param reporterRegistration Consumer to create the reporter for the registry.
+   * @param stopReporter Consumer to stop the reporter for the registry.
    */
   public abstract void addReporterRegistration(Consumer<RatisMetricRegistry> reporterRegistration,
       Consumer<RatisMetricRegistry> stopReporter);
 
+  /**
+   * Remove hook of reporter for the metricRegistry.
+   *
+   * @param reporterRegistration Consumer to create the reporter for the registry.
+   * @param stopReporter Consumer to stop the reporter for the registry.
+   */
+  public abstract void removeReporterRegistration(Consumer<RatisMetricRegistry> reporterRegistration,
+                                               Consumer<RatisMetricRegistry> stopReporter);

Review Comment:
   Let's add a default implementation for compatibility.
   ```java
     public void removeReporterRegistration(Consumer<RatisMetricRegistry> reporterRegistration,
         Consumer<RatisMetricRegistry> stopReporter) {
     }
   ```



-- 
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: issues-unsubscribe@ratis.apache.org

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

[GitHub] [ratis] szetszwo merged pull request #778: RATIS-1741 Add a removeReporterRegistration method

Posted by GitBox <gi...@apache.org>.
szetszwo merged PR #778:
URL: https://github.com/apache/ratis/pull/778


-- 
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: issues-unsubscribe@ratis.apache.org

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

[GitHub] [ratis] sumitagrawl commented on pull request #778: RATIS-1741 Add a removeReporterRegistration method

Posted by GitBox <gi...@apache.org>.
sumitagrawl commented on PR #778:
URL: https://github.com/apache/ratis/pull/778#issuecomment-1311257527

   @szetszwo plz review


-- 
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: issues-unsubscribe@ratis.apache.org

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