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<String> getSupervisorIds()
return supervisors.keySet();
}
+ @Override
+ public Collection<SupervisorStatsProvider.SupervisorStats>
getSupervisorStats()
+ {
+ List<SupervisorStatsProvider.SupervisorStats> stats = new ArrayList<>();
+ for (Map.Entry<String, Pair<Supervisor, SupervisorSpec>> 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.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<SupervisorStatsProvider.SupervisorStats> 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:
I think we can leave this as `state` to align with the field name
[documented](https://druid.apache.org/docs/latest/ingestion/supervisor/#status-report)
and included in the supervisor report
--
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]
