[GitHub] [kafka] cadonna commented on a change in pull request #9094: KAFKA-10054: KIP-613, add TRACE-level e2e latency metrics

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

cadonna commented on a change in pull request #9094:
URL: https://github.com/apache/kafka/pull/9094#discussion_r469781319



##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/internals/metrics/ProcessorNodeMetrics.java
##########
@@ -289,6 +294,25 @@ public static Sensor processorAtSourceSensorOrForwardSensor(final String threadI
         return processAtSourceSensor(threadId, taskId, processorNodeId, streamsMetrics);
     }
 
+    public static Sensor e2ELatencySensor(final String threadId,
+                                          final String taskId,
+                                          final String processorNodeId,
+                                          final StreamsMetricsImpl streamsMetrics) {
+        final String sensorName = processorNodeId + "-" + RECORD_E2E_LATENCY;
+        final Sensor sensor = streamsMetrics.taskLevelSensor(threadId, taskId, sensorName, RecordingLevel.INFO);

Review comment:
       I do not think that using `StreamsMetrics#nodeLevelSensor()` is a breaking change. Sensors are not exposed publicly. They are merely containers for metrics. Metrics are exposed publicy. The full sensor name is just the key to retrieve sensors in the metrics map. It is also stored in `StreamsMetricsImpl` in the `*LevelSensor` data structures to know what sensors need to be removed from the metrics map when `removeAll*LevelSensors()` is called. So, the full sensor name is used for internal house keeping and hidden from the users. I think it is fine to change the `fullSensorName`.




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