abhishekrb19 commented on code in PR #18709:
URL: https://github.com/apache/druid/pull/18709#discussion_r2488405379
##########
server/src/main/java/org/apache/druid/server/metrics/GroupByStatsMonitor.java:
##########
@@ -41,21 +43,28 @@ public class GroupByStatsMonitor extends AbstractMonitor
{
private final GroupByStatsProvider groupByStatsProvider;
private final BlockingPool<ByteBuffer> mergeBufferPool;
+ private final Map<String, String[]> dimensions;
Review Comment:
I can rename, but I left this field as `dimensions` to be consistent with
the other monitors.
>I wonder if it wouldn't be cleaner to just bind the datasource and taskID
as @ExtraServiceDimensions so that they are used in EmitterModule and are
emitted for all metrics emitted by a peon.
This is a good idea! I will try to take a stab at it separately and put up a
PR as it affects all monitors - we'll need to
[rework](https://github.com/apache/druid/pull/18709/files#r2488395049) the
Guice bindings and likely need to reconcile how the extra service dimensions
will be bound to monitors in a backwards-compatible manner too (`id` vs
`taskId` is something to consider).
--
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]
