[GitHub] [kafka] jlprat commented on a change in pull request #11302: KAFKA-13243: KIP-773 Differentiate metric latency measured in ms and ns

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

jlprat commented on a change in pull request #11302:
URL: https://github.com/apache/kafka/pull/11302#discussion_r703047638



##########
File path: clients/src/main/java/org/apache/kafka/common/network/Selector.java
##########
@@ -1272,12 +1274,27 @@ private Meter createMeter(Metrics metrics, String groupName,  Map<String, String
             return createMeter(metrics, groupName, metricTags, null, baseName, descriptiveName);
         }
 
-        private Meter createIOThreadRatioMeter(Metrics metrics, String groupName,  Map<String, String> metricTags,
+        /**
+         * This method generates `time-total` metrics with a couple of errors, no `-ns` suffix and no dash between basename
+         * and `time-toal` suffix.
+         * @deprecated use {{@link #createIOThreadRatioMeter(Metrics, String, Map, String, String)}} for new metrics instead
+         */
+        @Deprecated
+        private Meter createIOThreadRatioMeterLegacy(Metrics metrics, String groupName,  Map<String, String> metricTags,
                 String baseName, String action) {
             MetricName rateMetricName = metrics.metricName(baseName + "-ratio", groupName,
-                    String.format("The fraction of time the I/O thread spent %s", action), metricTags);
+                    String.format("*Deprecated* The fraction of time the I/O thread spent %s", action), metricTags);
             MetricName totalMetricName = metrics.metricName(baseName + "time-total", groupName,

Review comment:
       This line was causing the bug where there was no hyphen between `io` and `time` and `io-wait` and `time`.

##########
File path: docs/upgrade.html
##########
@@ -22,9 +22,12 @@
 <h5><a id="upgrade_310_notable" href="#upgrade_310_notable">Notable changes in 3.1.0</a></h5>
 <ul>
     <li>Apache Kafka supports Java 17.</li>
+    <li>The following metrics have been deprecated: <code>bufferpool-wait-time-total</code>, <code>io-waittime-total</code>,
+        and <code>iotime-total</code>. Please use <code>bufferpool-wait-time-ns-total</code>, <code>io-wait-time-ns-total</code>,
+        and <code>io-time-ns-total</code> instead.</li>
 </ul>
 
-<h5><a id="upgrade_310_notable" href="#upgrade_300_notable">Notable changes in 3.0.0</a></h5>
+<h5><a id="upgrade_300_notable" href="#upgrade_300_notable">Notable changes in 3.0.0</a></h5>

Review comment:
       This was a small mistake, it should be 300 not 310

##########
File path: docs/ops.html
##########
@@ -1890,6 +1890,16 @@ <h4><a id="selector_monitoring" href="#selector_monitoring">Common monitoring me
         <td>The average length of time the I/O thread spent waiting for a socket ready for reads or writes in nanoseconds.</td>
         <td>kafka.[producer|consumer|connect]:type=[producer|consumer|connect]-metrics,client-id=([-.\w]+)</td>
       </tr>
+      <tr>
+        <td>io-wait-time-ns-total</td>
+        <td>The total time the I/O thread spent waiting in nanoseconds</td>
+        <td>kafka.[producer|consumer|connect]:type=[producer|consumer|connect]-metrics,client-id=([-.\w]+)</td>
+      </tr>
+      <tr>
+        <td>io-waittime-total</td>
+        <td><b>*Deprecated*</b> The total time the I/O thread spent waiting in nanoseconds</td>
+        <td>kafka.[producer|consumer|connect]:type=[producer|consumer|connect]-metrics,client-id=([-.\w]+)</td>
+      </tr>

Review comment:
       Added the pairs of metrics to the documentation site. The metric with the wrong name wasn't present in the docs before.

##########
File path: clients/src/main/java/org/apache/kafka/common/network/Selector.java
##########
@@ -1272,12 +1274,27 @@ private Meter createMeter(Metrics metrics, String groupName,  Map<String, String
             return createMeter(metrics, groupName, metricTags, null, baseName, descriptiveName);
         }
 
-        private Meter createIOThreadRatioMeter(Metrics metrics, String groupName,  Map<String, String> metricTags,
+        /**
+         * This method generates `time-total` metrics with a couple of errors, no `-ns` suffix and no dash between basename
+         * and `time-toal` suffix.
+         * @deprecated use {{@link #createIOThreadRatioMeter(Metrics, String, Map, String, String)}} for new metrics instead
+         */
+        @Deprecated
+        private Meter createIOThreadRatioMeterLegacy(Metrics metrics, String groupName,  Map<String, String> metricTags,

Review comment:
       Keeping the old method for compatibility reasons, but added a javadoc and deprecation to it




-- 
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: jira-unsubscribe@kafka.apache.org

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