gianm commented on code in PR #13331:
URL: https://github.com/apache/druid/pull/13331#discussion_r1017439580


##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisor.java:
##########
@@ -4095,6 +4096,17 @@ protected void emitLag()
         }
 
         LagStats lagStats = computeLags(partitionLags);
+        for (Map.Entry<PartitionIdType, Long> entry : 
partitionLags.entrySet()) {
+          emitter.emit(
+              ServiceMetricEvent.builder()
+                                .setDimension(DruidMetrics.DATASOURCE, 
dataSource)
+                                .setDimension(DruidMetrics.PARTITION, 
entry.getKey())

Review Comment:
   Shall we add `stream` too? (And to all the existing metrics too?)



##########
indexing-service/src/main/java/org/apache/druid/indexing/common/actions/LocalTaskActionClient.java:
##########
@@ -72,14 +73,15 @@ public <RetType> RetType submit(TaskAction<RetType> 
taskAction)
 
     final long performStartTime = System.currentTimeMillis();
     final RetType result = taskAction.perform(task, toolbox);
-    emitTimerMetric("task/action/run/time", System.currentTimeMillis() - 
performStartTime);
+    emitTimerMetric("task/action/run/time", System.currentTimeMillis() - 
performStartTime, taskAction);
     return result;
   }
 
-  private void emitTimerMetric(final String metric, final long time)
+  private void emitTimerMetric(final String metric, final long time, 
TaskAction taskAction)
   {
     final ServiceMetricEvent.Builder metricBuilder = 
ServiceMetricEvent.builder();
     IndexTaskUtils.setTaskDimensions(metricBuilder, task);
+    metricBuilder.setDimension(DruidMetrics.TASK_ACTION_TYPE, 
taskAction.getClass().getName());

Review Comment:
   I raised a PR here with an alternate approach, using ObjectMapper to get the 
JSON type instead of using the Java class name: 
https://github.com/apache/druid/pull/13333. IMO, the JSON type is better since 
it's more stable (it's part of the API).



##########
indexing-service/src/main/java/org/apache/druid/indexing/common/actions/LocalTaskActionClient.java:
##########
@@ -72,14 +73,15 @@ public <RetType> RetType submit(TaskAction<RetType> 
taskAction)
 
     final long performStartTime = System.currentTimeMillis();
     final RetType result = taskAction.perform(task, toolbox);
-    emitTimerMetric("task/action/run/time", System.currentTimeMillis() - 
performStartTime);
+    emitTimerMetric("task/action/run/time", System.currentTimeMillis() - 
performStartTime, taskAction);
     return result;
   }
 
-  private void emitTimerMetric(final String metric, final long time)
+  private void emitTimerMetric(final String metric, final long time, 
TaskAction taskAction)
   {
     final ServiceMetricEvent.Builder metricBuilder = 
ServiceMetricEvent.builder();
     IndexTaskUtils.setTaskDimensions(metricBuilder, task);
+    metricBuilder.setDimension(DruidMetrics.TASK_ACTION_TYPE, 
taskAction.getClass().getName());

Review Comment:
   I raised a PR here with an alternate approach, using ObjectMapper to get the 
JSON type instead of using the Java class name: 
https://github.com/apache/druid/pull/13333. IMO, the JSON type is better since 
it's more stable (it's part of the API). Thoughts?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to