Re: [PR] Add supervisor/count metric for health and state monitoring (druid)
abhishekrb19 merged PR #19114: URL: https://github.com/apache/druid/pull/19114 -- 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]
Re: [PR] Add supervisor/count metric for health and state monitoring (druid)
abhishekrb19 commented on code in PR #19114:
URL: https://github.com/apache/druid/pull/19114#discussion_r2915134134
##
indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorManager.java:
##
@@ -88,6 +89,26 @@ public Set getSupervisorIds()
return supervisors.keySet();
}
+ @Override
+ public Collection
getSupervisorStats()
+ {
+List stats = new ArrayList<>();
+for (Map.Entry> entry :
supervisors.entrySet()) {
+ final SupervisorSpec spec = entry.getValue().rhs;
+ final SupervisorStateManager.State state =
entry.getValue().lhs.getState();
+
+ stats.add(new SupervisorStatsProvider.SupervisorStats(
+ spec.getId(),
+ spec.getType(),
+ state == null ? "UNKNOWN" : state.getBasicState().toString(),
+ DefaultQueryMetrics.getTableNamesAsString(new
HashSet<>(spec.getDataSources())),
+ spec.getSource(),
Review Comment:
It looks like `jacoco-coverage` is failing due to lack of code coverage for
this new method, which seems legitimate. Could you please add a few unit tests
in `SupervisorManagerTest`?
--
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]
Re: [PR] Add supervisor/count metric for health and state monitoring (druid)
abhishekrb19 commented on code in PR #19114:
URL: https://github.com/apache/druid/pull/19114#discussion_r2915134134
##
indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorManager.java:
##
@@ -88,6 +89,26 @@ public Set getSupervisorIds()
return supervisors.keySet();
}
+ @Override
+ public Collection
getSupervisorStats()
+ {
+List stats = new ArrayList<>();
+for (Map.Entry> entry :
supervisors.entrySet()) {
+ final SupervisorSpec spec = entry.getValue().rhs;
+ final SupervisorStateManager.State state =
entry.getValue().lhs.getState();
+
+ stats.add(new SupervisorStatsProvider.SupervisorStats(
+ spec.getId(),
+ spec.getType(),
+ state == null ? "UNKNOWN" : state.getBasicState().toString(),
+ DefaultQueryMetrics.getTableNamesAsString(new
HashSet<>(spec.getDataSources())),
+ spec.getSource(),
Review Comment:
It looks like `jacoco-coverage` is failing due to code coverage checks for
this new method, which seems legitimate. Could you please add a few unit tests
in `SupervisorManagerTest`?
--
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]
Re: [PR] Add supervisor/count metric for health and state monitoring (druid)
aruraghuwanshi commented on code in PR #19114:
URL: https://github.com/apache/druid/pull/19114#discussion_r2914678686
##
indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorManager.java:
##
@@ -88,6 +90,37 @@ public Set getSupervisorIds()
return supervisors.keySet();
}
+ @Override
+ public Collection
getSupervisorStats()
+ {
+List stats = new ArrayList<>();
+for (Map.Entry> entry :
supervisors.entrySet()) {
+ final String supervisorId = entry.getKey();
+ final Supervisor supervisor = entry.getValue().lhs;
+ final SupervisorSpec spec = entry.getValue().rhs;
+
+ SupervisorStateManager.State state = supervisor.getState();
+ String stateStr = state != null && state.getBasicState() != null
+? state.getBasicState().toString()
+: "UNKNOWN";
+ String dataSource = DefaultQueryMetrics.getTableNamesAsString(
+ new HashSet<>(spec.getDataSources() != null ? spec.getDataSources()
: Collections.emptyList())
+ );
+ String stream = spec.getSource();
+ String detailedState = state != null ? state.toString() : null;
+
+ stats.add(new SupervisorStatsProvider.SupervisorStats(
+ supervisorId,
+ spec.getType(),
+ stateStr,
+ dataSource,
+ stream,
+ detailedState
+ ));
+}
Review Comment:
Added necessary changes to cleanup those temporary variables in the latest
commit - didn't pull in the above due to a typo on "UNKOWN". But yeah, the
changes are in
--
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]
Re: [PR] Add supervisor/count metric for health and state monitoring (druid)
abhishekrb19 commented on code in PR #19114:
URL: https://github.com/apache/druid/pull/19114#discussion_r2914403425
##
indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorManager.java:
##
@@ -88,6 +90,37 @@ public Set getSupervisorIds()
return supervisors.keySet();
}
+ @Override
+ public Collection
getSupervisorStats()
+ {
+List stats = new ArrayList<>();
+for (Map.Entry> entry :
supervisors.entrySet()) {
+ final String supervisorId = entry.getKey();
+ final Supervisor supervisor = entry.getValue().lhs;
+ final SupervisorSpec spec = entry.getValue().rhs;
+
+ SupervisorStateManager.State state = supervisor.getState();
+ String stateStr = state != null && state.getBasicState() != null
+? state.getBasicState().toString()
+: "UNKNOWN";
+ String dataSource = DefaultQueryMetrics.getTableNamesAsString(
+ new HashSet<>(spec.getDataSources() != null ? spec.getDataSources()
: Collections.emptyList())
+ );
+ String stream = spec.getSource();
+ String detailedState = state != null ? state.toString() : null;
+
+ stats.add(new SupervisorStatsProvider.SupervisorStats(
+ supervisorId,
+ spec.getType(),
+ stateStr,
+ dataSource,
+ stream,
+ detailedState
+ ));
+}
Review Comment:
nit:
1. we can inline these and cleanup the variables above since they're not
reused
2. `SupervisorSpec.getDatasources()` doesn't return
[null](https://github.com/apache/druid/blob/master/server/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorSpec.java#L55)
(validations exist at the time of create/update of supervisors)
```suggestion
stats.add(new SupervisorStatsProvider.SupervisorStats(
spec.getId(),
spec.getType(),
state == null ? "UNKOWN" : state.getBasicState().toString(),
DefaultQueryMetrics.getTableNamesAsString(spec.getDataSources()),
spec.getStream(),
state == null ? "UNKNOWN" : state.toString()
));
```
##
extensions-contrib/prometheus-emitter/src/main/resources/defaultMetrics.json:
##
@@ -149,6 +149,7 @@
"task/running/count" : { "dimensions" : ["dataSource"], "type" : "count",
"help": "Number of current running tasks."},
"task/pending/count" : { "dimensions" : ["dataSource"], "type" : "count",
"help": "Number of current pending tasks."},
"task/waiting/count" : { "dimensions" : ["dataSource"], "type" : "count",
"help": "Number of current waiting tasks."},
+ "supervisor/count" : { "dimensions" : ["supervisorId", "type", "state"],
"type" : "gauge", "help": "Count of active supervisors. Each supervisor emits
1, tagged with its state. Available only if the SupervisorStatsMonitor module
is included."},
Review Comment:
Perhaps it's useful to add `detailedState` by default:
```suggestion
"supervisor/count" : { "dimensions" : ["supervisorId", "type", "state",
"detailedState"], "type" : "gauge", "help": "Count of active supervisors. Each
supervisor emits 1, tagged with its state. Available only if the
SupervisorStatsMonitor module is included."},
```
--
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]
Re: [PR] Add supervisor/count metric for health and state monitoring (druid)
aruraghuwanshi commented on code in PR #19114:
URL: https://github.com/apache/druid/pull/19114#discussion_r2914222509
##
server/src/main/java/org/apache/druid/server/metrics/SupervisorStatsMonitor.java:
##
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.server.metrics;
+
+import com.google.inject.Inject;
+import org.apache.druid.discovery.NodeRole;
+import org.apache.druid.guice.annotations.LoadScope;
+import org.apache.druid.java.util.emitter.service.ServiceEmitter;
+import org.apache.druid.java.util.emitter.service.ServiceMetricEvent;
+import org.apache.druid.java.util.metrics.AbstractMonitor;
+import org.apache.druid.query.DruidMetrics;
+
+import java.util.Collection;
+
+@LoadScope(roles = NodeRole.OVERLORD_JSON_NAME)
+public class SupervisorStatsMonitor extends AbstractMonitor
+{
+ private final SupervisorStatsProvider statsProvider;
+
+ @Inject
+ public SupervisorStatsMonitor(
+ SupervisorStatsProvider statsProvider
+ )
+ {
+this.statsProvider = statsProvider;
+ }
+
+ @Override
+ public boolean doMonitor(ServiceEmitter emitter)
+ {
+Collection stats =
statsProvider.getSupervisorStats();
+if (stats == null) {
+ return true;
+}
+
+for (SupervisorStatsProvider.SupervisorStats stat : stats) {
+ emitter.emit(
+ ServiceMetricEvent.builder()
+.setDimension(DruidMetrics.SUPERVISOR_ID,
stat.getSupervisorId())
Review Comment:
Done. I've added dataSource (fetched via
[DefaultQueryMetrics.getTableNamesAsString()](https://github.com/aruraghuwanshi/druid/blob/f98c1c2c6ac9696a33c893a9f8e0eeb3f717b6b2/indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorManager.java#L106))
and stream (emitted only when non-null)
--
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]
Re: [PR] Add supervisor/count metric for health and state monitoring (druid)
aruraghuwanshi commented on code in PR #19114:
URL: https://github.com/apache/druid/pull/19114#discussion_r2914118199
##
indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorManager.java:
##
@@ -88,6 +87,29 @@ public Set getSupervisorIds()
return supervisors.keySet();
}
+ @Override
+ public Collection
getSupervisorStats()
+ {
+List stats = new ArrayList<>();
+for (Map.Entry> entry :
supervisors.entrySet()) {
+ final String supervisorId = entry.getKey();
+ final Supervisor supervisor = entry.getValue().lhs;
+ final SupervisorSpec spec = entry.getValue().rhs;
+
+ SupervisorStateManager.State state = supervisor.getState();
+ String stateStr = state != null && state.getBasicState() != null
+? state.getBasicState().toString()
+: "UNKNOWN";
+
Review Comment:
Adding `detailedState` makes sense. For most supervisors it will match
state, but for unhealthy SeekableStreamSupervisors it gives more granular
values.
--
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]
Re: [PR] Add supervisor/count metric for health and state monitoring (druid)
aruraghuwanshi commented on code in PR #19114:
URL: https://github.com/apache/druid/pull/19114#discussion_r2914024984
##
server/src/main/java/org/apache/druid/server/metrics/SupervisorStatsMonitor.java:
##
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.server.metrics;
+
+import com.google.inject.Inject;
+import org.apache.druid.discovery.NodeRole;
+import org.apache.druid.guice.annotations.LoadScope;
+import org.apache.druid.java.util.emitter.service.ServiceEmitter;
+import org.apache.druid.java.util.emitter.service.ServiceMetricEvent;
+import org.apache.druid.java.util.metrics.AbstractMonitor;
+import org.apache.druid.query.DruidMetrics;
+
+import java.util.Collection;
+
+@LoadScope(roles = NodeRole.OVERLORD_JSON_NAME)
+public class SupervisorStatsMonitor extends AbstractMonitor
+{
+ private final SupervisorStatsProvider statsProvider;
+
+ @Inject
+ public SupervisorStatsMonitor(
+ SupervisorStatsProvider statsProvider
+ )
+ {
+this.statsProvider = statsProvider;
+ }
+
+ @Override
+ public boolean doMonitor(ServiceEmitter emitter)
+ {
+Collection stats =
statsProvider.getSupervisorStats();
+if (stats == null) {
+ return true;
+}
+
+for (SupervisorStatsProvider.SupervisorStats stat : stats) {
+ emitter.emit(
+ ServiceMetricEvent.builder()
+.setDimension(DruidMetrics.SUPERVISOR_ID,
stat.getSupervisorId())
+.setDimension("type", stat.getType())
+.setDimension("state", stat.getState())
Review Comment:
Yeah, I was of the opinion on sticking to what's mentioned in the
[Supervisor
doc](https://druid.apache.org/docs/latest/ingestion/supervisor/#status-report:~:text=The%20status%20report%20contains%20two%20properties%20that%20correspond%20to%20the%20state%20of%20the%20supervisor%3A%20state%20and%20detailedState)
which calls out both `state` and `detailedState`. Lmk if that works.
--
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]
Re: [PR] Add supervisor/count metric for health and state monitoring (druid)
abhishekrb19 commented on code in PR #19114:
URL: https://github.com/apache/druid/pull/19114#discussion_r2912815264
##
indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorManager.java:
##
@@ -88,6 +87,29 @@ public Set getSupervisorIds()
return supervisors.keySet();
}
+ @Override
+ public Collection
getSupervisorStats()
+ {
+List stats = new ArrayList<>();
+for (Map.Entry> entry :
supervisors.entrySet()) {
+ final String supervisorId = entry.getKey();
+ final Supervisor supervisor = entry.getValue().lhs;
+ final SupervisorSpec spec = entry.getValue().rhs;
+
+ SupervisorStateManager.State state = supervisor.getState();
+ String stateStr = state != null && state.getBasicState() != null
+? state.getBasicState().toString()
+: "UNKNOWN";
+
Review Comment:
What do you think about including `detailedState` as a dimension as well?
For unhealthy `SeekableStreamSupervisor`s, it can capture additional useful
[information](https://druid.apache.org/docs/latest/ingestion/supervisor/#status-report).
We could emit this dimension when it’s not null
##
docs/operations/metrics.md:
##
@@ -342,6 +342,7 @@ If the JVM does not support CPU time measurement for the
current thread, `ingest
|`task/running/count`|Number of current running tasks. This metric is
available only if the `TaskCountStatsMonitor` module is
included.|`dataSource`,`taskType`, `supervisorId`|Varies|
|`task/pending/count`|Number of current pending tasks. This metric is
available only if the `TaskCountStatsMonitor` module is
included.|`dataSource`,`taskType`, `supervisorId`|Varies|
|`task/waiting/count`|Number of current waiting tasks. This metric is
available only if the `TaskCountStatsMonitor` module is
included.|`dataSource`,`taskType`, `supervisorId`|Varies|
+|`supervisor/count`|Count of active supervisors. Each supervisor emits 1,
tagged with its state (`RUNNING`, `SUSPENDED`, `UNHEALTHY_SUPERVISOR`,
`UNHEALTHY_TASKS`, etc.). This metric is available only if the
`SupervisorStatsMonitor` module is included.|`supervisorId`, `type`, `state`|1
per supervisor; aggregate by state for totals|
Review Comment:
For state, we could link it to this section in the
[docs](https://druid.apache.org/docs/latest/ingestion/supervisor/#status-report),
which has this info:
>Possible state values are PENDING, RUNNING, SUSPENDED, STOPPING,
UNHEALTHY_SUPERVISOR, and UNHEALTHY_TASKS.
##
docs/configuration/index.md:
##
@@ -1983,6 +1978,7 @@ The following table lists available monitors and the
respective services where t
|`org.apache.druid.server.emitter.HttpEmittingMonitor`|Reports internal
metrics of `http` or `parametrized` emitter (see below). Must not be used with
another emitter type. See the description of the metrics here:
https://github.com/apache/druid/pull/4973.|Any|
|`org.apache.druid.server.metrics.TaskCountStatsMonitor`|Reports how many
ingestion tasks are currently running/pending/waiting and also the number of
successful/failed tasks per emission period.|Overlord|
|`org.apache.druid.server.metrics.TaskSlotCountStatsMonitor`|Reports metrics
about task slot usage per emission period.|Overlord|
+|`org.apache.druid.server.metrics.SupervisorStatsMonitor`|Reports supervisor
count and state (RUNNING, SUSPENDED, UNHEALTHY_SUPERVISOR, UNHEALTHY_TASKS,
etc.) per supervisor.|Overlord|
Review Comment:
nit: maybe leave the specifics to the individual metric dimension docs, as
this may go stale?
```suggestion
|`org.apache.druid.server.metrics.SupervisorStatsMonitor`|Reports supervisor
count and state, per supervisor per emission period.|Overlord|
```
##
server/src/main/java/org/apache/druid/server/metrics/SupervisorStatsMonitor.java:
##
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.server.metrics;
+
+import com.google.inject.Inject;
+import org.apache.druid.discovery.NodeRole;
+import org.apache.druid.guice.annotations.LoadScope;
+import org.apache.druid.java.util.emitter.service.ServiceEmi
Re: [PR] Add supervisor/count metric for health and state monitoring (druid)
gianm commented on code in PR #19114:
URL: https://github.com/apache/druid/pull/19114#discussion_r2912527097
##
server/src/main/java/org/apache/druid/server/metrics/SupervisorStatsMonitor.java:
##
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.server.metrics;
+
+import com.google.inject.Inject;
+import org.apache.druid.discovery.NodeRole;
+import org.apache.druid.guice.annotations.LoadScope;
+import org.apache.druid.java.util.emitter.service.ServiceEmitter;
+import org.apache.druid.java.util.emitter.service.ServiceMetricEvent;
+import org.apache.druid.java.util.metrics.AbstractMonitor;
+import org.apache.druid.query.DruidMetrics;
+
+import java.util.Collection;
+
+@LoadScope(roles = NodeRole.OVERLORD_JSON_NAME)
+public class SupervisorStatsMonitor extends AbstractMonitor
+{
+ private final SupervisorStatsProvider statsProvider;
+
+ @Inject
+ public SupervisorStatsMonitor(
+ SupervisorStatsProvider statsProvider
+ )
+ {
+this.statsProvider = statsProvider;
+ }
+
+ @Override
+ public boolean doMonitor(ServiceEmitter emitter)
+ {
+Collection stats =
statsProvider.getSupervisorStats();
+if (stats == null) {
+ return true;
+}
+
+for (SupervisorStatsProvider.SupervisorStats stat : stats) {
+ emitter.emit(
+ ServiceMetricEvent.builder()
+.setDimension(DruidMetrics.SUPERVISOR_ID,
stat.getSupervisorId())
Review Comment:
For datasource, since there can be multiple, try using
`DefaultQueryMetrics.getTableNamesAsString`.
--
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]
Re: [PR] Add supervisor/count metric for health and state monitoring (druid)
kfaraz commented on code in PR #19114:
URL: https://github.com/apache/druid/pull/19114#discussion_r2911296697
##
server/src/main/java/org/apache/druid/server/metrics/SupervisorStatsMonitor.java:
##
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.server.metrics;
+
+import com.google.inject.Inject;
+import org.apache.druid.discovery.NodeRole;
+import org.apache.druid.guice.annotations.LoadScope;
+import org.apache.druid.java.util.emitter.service.ServiceEmitter;
+import org.apache.druid.java.util.emitter.service.ServiceMetricEvent;
+import org.apache.druid.java.util.metrics.AbstractMonitor;
+import org.apache.druid.query.DruidMetrics;
+
+import java.util.Collection;
+
+@LoadScope(roles = NodeRole.OVERLORD_JSON_NAME)
+public class SupervisorStatsMonitor extends AbstractMonitor
+{
+ private final SupervisorStatsProvider statsProvider;
+
+ @Inject
+ public SupervisorStatsMonitor(
+ SupervisorStatsProvider statsProvider
+ )
+ {
+this.statsProvider = statsProvider;
+ }
+
+ @Override
+ public boolean doMonitor(ServiceEmitter emitter)
+ {
+Collection stats =
statsProvider.getSupervisorStats();
+if (stats == null) {
+ return true;
+}
+
+for (SupervisorStatsProvider.SupervisorStats stat : stats) {
+ emitter.emit(
+ ServiceMetricEvent.builder()
+.setDimension(DruidMetrics.SUPERVISOR_ID,
stat.getSupervisorId())
Review Comment:
Dimensions should also include datasource and stream, if not null.
##
server/src/main/java/org/apache/druid/server/metrics/SupervisorStatsMonitor.java:
##
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.server.metrics;
+
+import com.google.inject.Inject;
+import org.apache.druid.discovery.NodeRole;
+import org.apache.druid.guice.annotations.LoadScope;
+import org.apache.druid.java.util.emitter.service.ServiceEmitter;
+import org.apache.druid.java.util.emitter.service.ServiceMetricEvent;
+import org.apache.druid.java.util.metrics.AbstractMonitor;
+import org.apache.druid.query.DruidMetrics;
+
+import java.util.Collection;
+
+@LoadScope(roles = NodeRole.OVERLORD_JSON_NAME)
+public class SupervisorStatsMonitor extends AbstractMonitor
+{
+ private final SupervisorStatsProvider statsProvider;
+
+ @Inject
+ public SupervisorStatsMonitor(
+ SupervisorStatsProvider statsProvider
+ )
+ {
+this.statsProvider = statsProvider;
+ }
+
+ @Override
+ public boolean doMonitor(ServiceEmitter emitter)
+ {
+Collection stats =
statsProvider.getSupervisorStats();
+if (stats == null) {
+ return true;
+}
+
+for (SupervisorStatsProvider.SupervisorStats stat : stats) {
+ emitter.emit(
+ ServiceMetricEvent.builder()
+.setDimension(DruidMetrics.SUPERVISOR_ID,
stat.getSupervisorId())
+.setDimension("type", stat.getType())
Review Comment:
```suggestion
.setDimension(DruidMetrics.TYPE, stat.getType())
```
##
server/src/main/java/org/apache/druid/server/metrics/SupervisorStatsMonitor.java:
##
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownersh
