You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2022/07/15 06:22:31 UTC

[GitHub] [bookkeeper] Shoothzj opened a new pull request, #3406: Rename bookkeeper metrics to specify OpenMetrics

Shoothzj opened a new pull request, #3406:
URL: https://github.com/apache/bookkeeper/pull/3406

   ### Motivation
   See https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md
   A COUNTER needs `metrics_name_total` or `metrics_name_created`
   
   This PR contains metric name broken changes.
   
   ### Modifications
   Rename counter's `_count` to `_total`
   
   ### metrics name broken changes
   - rename `FAILED_TO_RESOLVE_NETWORK_LOCATION_COUNTER` to `FAILED_TO_RESOLVE_NETWORK_LOCATION_TOTAL`
   - rename `ENSEMBLE_NOT_ADHERING_TO_PLACEMENT_POLICY_COUNT` to `ENSEMBLE_NOT_ADHERING_TO_PLACEMENT_POLICY_TOTAL`
   - rename `ACTIVE_ENTRY_LOG_COUNT` to `ACTIVE_ENTRY_LOG_TOTAL`
   - rename `MAJOR_COMPACTION_COUNT` to `MAJOR_COMPACTION_TOTAL`
   - rename `MINOR_COMPACTION_COUNT` to `MINOR_COMPACTION_TOTAL`
   - rename `ACTIVE_LEDGER_COUNT` to `ACTIVE_LEDGER_TOTAL`
   - rename `DELETED_LEDGER_COUNT` to `DELETED_LEDGER_TOTAL`
   - rename `JOURNAL_FORCE_WRITE_GROUPING_COUNT` to `JOURNAL_FORCE_WRITE_GROUPING_TOTAL`
   
   ### Also see
   - https://github.com/apache/pulsar/pull/16591
   - https://github.com/apache/pulsar/pull/16610
   - https://github.com/apache/pulsar/pull/16611


-- 
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@bookkeeper.apache.org

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

[GitHub] [bookkeeper] Shoothzj merged pull request #3406: Rename bookkeeper metrics to specify OpenMetrics

Posted by GitBox <gi...@apache.org>.
Shoothzj merged PR #3406:
URL: https://github.com/apache/bookkeeper/pull/3406


-- 
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@bookkeeper.apache.org

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

[GitHub] [bookkeeper] Shoothzj commented on pull request #3406: Rename bookkeeper metrics to specify OpenMetrics

Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #3406:
URL: https://github.com/apache/bookkeeper/pull/3406#issuecomment-1185386339

   @dlg99 @nicoloboschi @hangc0276 PTAL


-- 
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@bookkeeper.apache.org

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

[GitHub] [bookkeeper] hangc0276 commented on a diff in pull request #3406: Rename bookkeeper metrics to specify OpenMetrics

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on code in PR #3406:
URL: https://github.com/apache/bookkeeper/pull/3406#discussion_r922078647


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookieWatcherImpl.java:
##########
@@ -17,7 +17,7 @@
  */
 package org.apache.bookkeeper.client;
 
-import static org.apache.bookkeeper.bookie.BookKeeperServerStats.ENSEMBLE_NOT_ADHERING_TO_PLACEMENT_POLICY_COUNTER;
+import static org.apache.bookkeeper.bookie.BookKeeperServerStats.ENSEMBLE_NOT_ADHERING_TO_PLACEMENT_POLICY_COUNT;

Review Comment:
   Do we need to use `TOTAL` suffix instead of `COUNT` according to the new rules?



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookKeeperServerStats.java:
##########
@@ -148,15 +148,15 @@ public interface BookKeeperServerStats {
     String LEDGER_CACHE_MISS = "LEDGER_CACHE_MISS";
 
     // Compaction/Garbage Collection Related Counters
-    String ACTIVE_ENTRY_LOG_COUNT = "ACTIVE_ENTRY_LOG_COUNT";
+    String ACTIVE_ENTRY_LOG_COUNT = "ACTIVE_ENTRY_LOG_TOTAL";

Review Comment:
   Do we need to keep the variable name sync with the metric name?



-- 
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@bookkeeper.apache.org

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

[GitHub] [bookkeeper] Shoothzj commented on pull request #3406: Rename bookkeeper metrics to specify OpenMetrics

Posted by GitBox <gi...@apache.org>.
Shoothzj commented on PR #3406:
URL: https://github.com/apache/bookkeeper/pull/3406#issuecomment-1185459387

   @hangc0276 I don't think it's necessary to modify these variable names. Let's keep the `_count` suffix consistent.


-- 
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@bookkeeper.apache.org

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