You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "adithyachakilam (via GitHub)" <gi...@apache.org> on 2024/08/16 04:59:31 UTC
[PR] cgroup monitors: Add mem/disk/cpu usage metrics for V2 (druid)
adithyachakilam opened a new pull request, #16905:
URL: https://github.com/apache/druid/pull/16905
### Description
This PR add the support for cgroup monitors for v2. For now, only the usage of cpu/disk/memory are being supported.
#### Release note
New monitors for cgroups v2 are now available to track usage of cpu/disk/memory.
<hr>
##### Key changed/added classes in this PR
* `CgroupV2CpuMonitor`
* `CgroupV2DiskMonitor`
* `CgroupV2MemoryMonitor`
<hr>
This PR has:
- [x] been self-reviewed.
- [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
- [x] added documentation for new or modified features or behaviors.
- [x] a release note entry in the PR description.
- [x] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [ ] added integration tests.
- [x] been tested in a test Druid cluster.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] cgroup monitors: Add mem/disk/cpu usage metrics for V2 (druid)
Posted by "adithyachakilam (via GitHub)" <gi...@apache.org>.
adithyachakilam commented on code in PR #16905:
URL: https://github.com/apache/druid/pull/16905#discussion_r1770808839
##########
processing/src/main/java/org/apache/druid/java/util/metrics/cgroups/Cpu.java:
##########
@@ -34,8 +34,8 @@
*/
public class Cpu
{
+ public static final String CGROUP = "cpu";
Review Comment:
used is v2 too.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] cgroup monitors: Add mem/disk/cpu usage metrics for V2 (druid)
Posted by "adithyachakilam (via GitHub)" <gi...@apache.org>.
adithyachakilam commented on code in PR #16905:
URL: https://github.com/apache/druid/pull/16905#discussion_r1723951630
##########
docs/configuration/index.md:
##########
@@ -394,6 +394,9 @@ Metric monitoring is an essential part of Druid operations. The following monito
|`org.apache.druid.java.util.metrics.CgroupCpuSetMonitor`|Reports CPU core/HT and memory node allocations as per the `cpuset` cgroup.|
|`org.apache.druid.java.util.metrics.CgroupDiskMonitor`|Reports disk statistic as per the blkio cgroup.|
|`org.apache.druid.java.util.metrics.CgroupMemoryMonitor`|Reports memory statistic as per the memory cgroup.|
+|`org.apache.druid.java.util.metrics.CgroupV2CpuMonitor`|Reports CPU usage from `cpu.stat` file. Only applicable to `cgroupv2`.|
Review Comment:
I couldn't find any documented way of automatically identifying v1 vs v2, hence depending on user to specify.
I could take it up as a follow up if there is a documented way.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] cgroup monitors: Add mem/disk/cpu usage metrics for V2 (druid)
Posted by "adithyachakilam (via GitHub)" <gi...@apache.org>.
adithyachakilam commented on code in PR #16905:
URL: https://github.com/apache/druid/pull/16905#discussion_r1770813905
##########
processing/src/main/java/org/apache/druid/java/util/metrics/CgroupV2DiskMonitor.java:
##########
@@ -0,0 +1,130 @@
+/*
+ * 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.java.util.metrics;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+import org.apache.druid.java.util.common.logger.Logger;
+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.cgroups.CgroupDiscoverer;
+import org.apache.druid.java.util.metrics.cgroups.Disk;
+import org.apache.druid.java.util.metrics.cgroups.ProcCgroupV2Discoverer;
+import org.apache.druid.java.util.metrics.cgroups.ProcSelfCgroupDiscoverer;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Pattern;
+
+public class CgroupV2DiskMonitor extends FeedDefiningMonitor
+{
+ private static final Logger LOG = new Logger(CgroupV2DiskMonitor.class);
+ private static final String IO_STAT = "io.stat";
+ final CgroupDiscoverer cgroupDiscoverer;
+ final Map<String, String[]> dimensions;
+ private final KeyedDiff diff = new KeyedDiff();
+
+ public CgroupV2DiskMonitor(CgroupDiscoverer cgroupDiscoverer, final Map<String, String[]> dimensions, String feed)
+ {
+ super(feed);
+ this.cgroupDiscoverer = cgroupDiscoverer;
+ this.dimensions = dimensions;
+ }
+
+ @VisibleForTesting
+ CgroupV2DiskMonitor(CgroupDiscoverer cgroupDiscoverer)
+ {
+ this(cgroupDiscoverer, ImmutableMap.of(), DEFAULT_METRICS_FEED);
+ }
+
+ CgroupV2DiskMonitor()
+ {
+ this(new ProcSelfCgroupDiscoverer(ProcCgroupV2Discoverer.class));
+ }
+
+
+ @Override
+ public boolean doMonitor(ServiceEmitter emitter)
+ {
+ for (Disk.Metrics entry : snapshot()) {
+ final Map<String, Long> stats = diff.to(
+ entry.getDiskName(),
+ ImmutableMap.<String, Long>builder()
+ .put(CgroupUtil.DISK_READ_BYTES_METRIC, entry.getReadBytes())
+ .put(CgroupUtil.DISK_READ_COUNT_METRIC, entry.getReadCount())
+ .put(CgroupUtil.DISK_WRITE_BYTES_METRIC, entry.getWriteBytes())
+ .put(CgroupUtil.DISK_WRITE_COUNT_METRIC, entry.getWriteCount())
+ .build()
+ );
+
+ if (stats != null) {
+ final ServiceMetricEvent.Builder builder = builder()
+ .setDimension("diskName", entry.getDiskName());
+ MonitorUtils.addDimensionsToBuilder(builder, dimensions);
+ for (Map.Entry<String, Long> stat : stats.entrySet()) {
+ emitter.emit(builder.setMetric(stat.getKey(), stat.getValue()));
+ }
+ }
+ }
+ return true;
+ }
+
+ public List<Disk.Metrics> snapshot()
+ {
+ List<Disk.Metrics> diskStats = new ArrayList<>();
+ try (final BufferedReader reader = Files.newBufferedReader(
+ Paths.get(cgroupDiscoverer.discover("disk").toString(), IO_STAT))) {
+ for (String line = reader.readLine(); line != null; line = reader.readLine()) {
+ Disk.Metrics disk = getDiskMetrics(line);
+ diskStats.add(disk);
+ }
+ }
+ catch (IOException | RuntimeException ex) {
+ LOG.error(ex, "Unable to fetch memory snapshot");
+ }
+ return diskStats;
+ }
+
+ private static Disk.Metrics getDiskMetrics(String line)
+ {
+ final String[] parts = line.split(Pattern.quote(" "));
+
+ Disk.Metrics disk = new Disk.Metrics(parts[0]);
+ Map<String, Long> stats = new HashMap<>();
+ for (int i = 1; i < parts.length; i++) {
+ String[] keyValue = parts[i].split("=");
+ if (keyValue.length == 2) {
+ stats.put(keyValue[0], Long.parseLong(keyValue[1]));
Review Comment:
There shouldn't be any nulls, it should either be a valid long value that should be obtained here or a exception thrown, catched at line 113.
##########
processing/src/main/java/org/apache/druid/java/util/metrics/CgroupV2DiskMonitor.java:
##########
@@ -0,0 +1,130 @@
+/*
+ * 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.java.util.metrics;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+import org.apache.druid.java.util.common.logger.Logger;
+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.cgroups.CgroupDiscoverer;
+import org.apache.druid.java.util.metrics.cgroups.Disk;
+import org.apache.druid.java.util.metrics.cgroups.ProcCgroupV2Discoverer;
+import org.apache.druid.java.util.metrics.cgroups.ProcSelfCgroupDiscoverer;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Pattern;
+
+public class CgroupV2DiskMonitor extends FeedDefiningMonitor
+{
+ private static final Logger LOG = new Logger(CgroupV2DiskMonitor.class);
+ private static final String IO_STAT = "io.stat";
+ final CgroupDiscoverer cgroupDiscoverer;
+ final Map<String, String[]> dimensions;
+ private final KeyedDiff diff = new KeyedDiff();
+
+ public CgroupV2DiskMonitor(CgroupDiscoverer cgroupDiscoverer, final Map<String, String[]> dimensions, String feed)
+ {
+ super(feed);
+ this.cgroupDiscoverer = cgroupDiscoverer;
+ this.dimensions = dimensions;
+ }
+
+ @VisibleForTesting
+ CgroupV2DiskMonitor(CgroupDiscoverer cgroupDiscoverer)
+ {
+ this(cgroupDiscoverer, ImmutableMap.of(), DEFAULT_METRICS_FEED);
+ }
+
+ CgroupV2DiskMonitor()
+ {
+ this(new ProcSelfCgroupDiscoverer(ProcCgroupV2Discoverer.class));
+ }
+
+
+ @Override
+ public boolean doMonitor(ServiceEmitter emitter)
+ {
+ for (Disk.Metrics entry : snapshot()) {
+ final Map<String, Long> stats = diff.to(
+ entry.getDiskName(),
+ ImmutableMap.<String, Long>builder()
+ .put(CgroupUtil.DISK_READ_BYTES_METRIC, entry.getReadBytes())
+ .put(CgroupUtil.DISK_READ_COUNT_METRIC, entry.getReadCount())
+ .put(CgroupUtil.DISK_WRITE_BYTES_METRIC, entry.getWriteBytes())
+ .put(CgroupUtil.DISK_WRITE_COUNT_METRIC, entry.getWriteCount())
+ .build()
+ );
+
+ if (stats != null) {
+ final ServiceMetricEvent.Builder builder = builder()
+ .setDimension("diskName", entry.getDiskName());
+ MonitorUtils.addDimensionsToBuilder(builder, dimensions);
+ for (Map.Entry<String, Long> stat : stats.entrySet()) {
+ emitter.emit(builder.setMetric(stat.getKey(), stat.getValue()));
+ }
+ }
+ }
+ return true;
+ }
+
+ public List<Disk.Metrics> snapshot()
+ {
+ List<Disk.Metrics> diskStats = new ArrayList<>();
+ try (final BufferedReader reader = Files.newBufferedReader(
+ Paths.get(cgroupDiscoverer.discover("disk").toString(), IO_STAT))) {
+ for (String line = reader.readLine(); line != null; line = reader.readLine()) {
+ Disk.Metrics disk = getDiskMetrics(line);
+ diskStats.add(disk);
+ }
+ }
+ catch (IOException | RuntimeException ex) {
+ LOG.error(ex, "Unable to fetch memory snapshot");
+ }
+ return diskStats;
+ }
+
+ private static Disk.Metrics getDiskMetrics(String line)
+ {
+ final String[] parts = line.split(Pattern.quote(" "));
+
+ Disk.Metrics disk = new Disk.Metrics(parts[0]);
+ Map<String, Long> stats = new HashMap<>();
+ for (int i = 1; i < parts.length; i++) {
+ String[] keyValue = parts[i].split("=");
+ if (keyValue.length == 2) {
+ stats.put(keyValue[0], Long.parseLong(keyValue[1]));
Review Comment:
There shouldn't be any nulls, it should either be a valid long value that should be obtained here or a exception thrown, catched at line 113. Am I missing anything ?
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] cgroup monitors: Add mem/disk/cpu usage metrics for V2 (druid)
Posted by "suneet-s (via GitHub)" <gi...@apache.org>.
suneet-s merged PR #16905:
URL: https://github.com/apache/druid/pull/16905
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] cgroup monitors: Add mem/disk/cpu usage metrics for V2 (druid)
Posted by "suneet-s (via GitHub)" <gi...@apache.org>.
suneet-s commented on code in PR #16905:
URL: https://github.com/apache/druid/pull/16905#discussion_r1768850495
##########
docs/configuration/index.md:
##########
@@ -394,6 +394,9 @@ Metric monitoring is an essential part of Druid operations. The following monito
|`org.apache.druid.java.util.metrics.CgroupCpuSetMonitor`|Reports CPU core/HT and memory node allocations as per the `cpuset` cgroup.|
|`org.apache.druid.java.util.metrics.CgroupDiskMonitor`|Reports disk statistic as per the blkio cgroup.|
|`org.apache.druid.java.util.metrics.CgroupMemoryMonitor`|Reports memory statistic as per the memory cgroup.|
+|`org.apache.druid.java.util.metrics.CgroupV2CpuMonitor`|Reports CPU usage from `cpu.stat` file. Only applicable to `cgroupv2`.|
Review Comment:
Can we mark these monitors as `EXPERIMENTAL` please. Similar to `SegmentStatsMonitor`. That will allow us to remove this monitor when we figure out how to fit migrate this into the Cgroup monitors
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] cgroup monitors: Add mem/disk/cpu usage metrics for V2 (druid)
Posted by "adithyachakilam (via GitHub)" <gi...@apache.org>.
adithyachakilam commented on code in PR #16905:
URL: https://github.com/apache/druid/pull/16905#discussion_r1770814695
##########
processing/src/main/java/org/apache/druid/java/util/metrics/CgroupV2CpuMonitor.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * 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.java.util.metrics;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.primitives.Longs;
+import org.apache.druid.java.util.common.logger.Logger;
+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.cgroups.CgroupDiscoverer;
+import org.apache.druid.java.util.metrics.cgroups.Cpu;
+import org.apache.druid.java.util.metrics.cgroups.ProcCgroupV2Discoverer;
+import org.apache.druid.java.util.metrics.cgroups.ProcSelfCgroupDiscoverer;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.regex.Pattern;
+
+public class CgroupV2CpuMonitor extends FeedDefiningMonitor
+{
+ private static final Logger LOG = new Logger(CgroupV2CpuMonitor.class);
+ private static final String CPU_STAT_FILE = "cpu.stat";
+ private static final String SNAPSHOT = "snapshot";
+ final CgroupDiscoverer cgroupDiscoverer;
+ final Map<String, String[]> dimensions;
+ private final KeyedDiff diff = new KeyedDiff();
+
+ public CgroupV2CpuMonitor(CgroupDiscoverer cgroupDiscoverer, final Map<String, String[]> dimensions, String feed)
+ {
+ super(feed);
+ this.cgroupDiscoverer = cgroupDiscoverer;
+ this.dimensions = dimensions;
+ }
+
+ @VisibleForTesting
+ CgroupV2CpuMonitor(CgroupDiscoverer cgroupDiscoverer)
+ {
+ this(cgroupDiscoverer, ImmutableMap.of(), DEFAULT_METRICS_FEED);
+ }
+
+ CgroupV2CpuMonitor()
+ {
+ this(new ProcSelfCgroupDiscoverer(ProcCgroupV2Discoverer.class));
+ }
+
+ @Override
+ public boolean doMonitor(ServiceEmitter emitter)
+ {
+ final ServiceMetricEvent.Builder builder = builder();
+ MonitorUtils.addDimensionsToBuilder(builder, dimensions);
+ Snapshot snapshot = snapshot();
+ final Map<String, Long> elapsed = diff.to(
+ "usage",
+ ImmutableMap.<String, Long>builder()
+ .put(CgroupUtil.USER, snapshot.getUserUsec())
+ .put(CgroupUtil.SYSTEM, snapshot.getSystemUsec())
+ .put(CgroupUtil.TOTAL, snapshot.getUsageUsec())
+ .put(SNAPSHOT, ChronoUnit.MICROS.between(Instant.EPOCH, Instant.now()))
+ .build()
+ );
+
+ if (elapsed != null) {
+ long elapsedUsecs = elapsed.get(SNAPSHOT);
+ double totalUsagePct = 100.0 * elapsed.get(CgroupUtil.TOTAL) / elapsedUsecs;
+ double sysUsagePct = 100.0 * elapsed.get(CgroupUtil.SYSTEM) / elapsedUsecs;
+ double userUsagePct = 100.0 * elapsed.get(CgroupUtil.USER) / elapsedUsecs;
+ emitter.emit(builder.setMetric(CgroupUtil.CPU_TOTAL_USAGE_METRIC, totalUsagePct));
+ emitter.emit(builder.setMetric(CgroupUtil.CPU_SYS_USAGE_METRIC, sysUsagePct));
+ emitter.emit(builder.setMetric(CgroupUtil.CPU_USER_USAGE_METRIC, userUsagePct));
+ }
+ return true;
+ }
+
+ /*
+ file: cpu.stat
+
+ sample content:
+ usage_usec 2379951538
+ user_usec 1802023024
+ system_usec 577928513
+ nr_periods 1581231
+ nr_throttled 59
+ throttled_usec 3095133
+ */
+ public Snapshot snapshot()
+ {
+ Map<String, Long> entries = new HashMap<>();
+ try (final BufferedReader reader = Files.newBufferedReader(
+ Paths.get(cgroupDiscoverer.discover(Cpu.CGROUP).toString(), CPU_STAT_FILE)
+ )) {
+ for (String line = reader.readLine(); line != null; line = reader.readLine()) {
+ final String[] parts = line.split(Pattern.quote(" "));
+ if (parts.length != 2) {
+ // ignore
+ continue;
+ }
+ entries.put(parts[0], Longs.tryParse(parts[1]));
+ }
+ }
+ catch (IOException | RuntimeException ex) {
+ LOG.error(ex, "Unable to fetch cpu snapshot");
+ }
+
+ return new Snapshot(entries.get("usage_usec"), entries.get("user_usec"), entries.get("system_usec"));
+ }
+
+
+ public static class Snapshot
+ {
+ private final long usageUsec;
+ private final long userUsec;
+ private final long systemUsec;
+
+ public Snapshot(long usageUsec, long userUsec, long systemUsec)
Review Comment:
We basically skip emitting metrics on parse exceptions and would not end up creating this Snapshot object at all.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] cgroup monitors: Add mem/disk/cpu usage metrics for V2 (druid)
Posted by "suneet-s (via GitHub)" <gi...@apache.org>.
suneet-s commented on code in PR #16905:
URL: https://github.com/apache/druid/pull/16905#discussion_r1771884334
##########
processing/src/main/java/org/apache/druid/java/util/metrics/CgroupV2DiskMonitor.java:
##########
@@ -0,0 +1,130 @@
+/*
+ * 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.java.util.metrics;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+import org.apache.druid.java.util.common.logger.Logger;
+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.cgroups.CgroupDiscoverer;
+import org.apache.druid.java.util.metrics.cgroups.Disk;
+import org.apache.druid.java.util.metrics.cgroups.ProcCgroupV2Discoverer;
+import org.apache.druid.java.util.metrics.cgroups.ProcSelfCgroupDiscoverer;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Pattern;
+
+public class CgroupV2DiskMonitor extends FeedDefiningMonitor
+{
+ private static final Logger LOG = new Logger(CgroupV2DiskMonitor.class);
+ private static final String IO_STAT = "io.stat";
+ final CgroupDiscoverer cgroupDiscoverer;
+ final Map<String, String[]> dimensions;
+ private final KeyedDiff diff = new KeyedDiff();
+
+ public CgroupV2DiskMonitor(CgroupDiscoverer cgroupDiscoverer, final Map<String, String[]> dimensions, String feed)
+ {
+ super(feed);
+ this.cgroupDiscoverer = cgroupDiscoverer;
+ this.dimensions = dimensions;
+ }
+
+ @VisibleForTesting
+ CgroupV2DiskMonitor(CgroupDiscoverer cgroupDiscoverer)
+ {
+ this(cgroupDiscoverer, ImmutableMap.of(), DEFAULT_METRICS_FEED);
+ }
+
+ CgroupV2DiskMonitor()
+ {
+ this(new ProcSelfCgroupDiscoverer(ProcCgroupV2Discoverer.class));
+ }
+
+
+ @Override
+ public boolean doMonitor(ServiceEmitter emitter)
+ {
+ for (Disk.Metrics entry : snapshot()) {
+ final Map<String, Long> stats = diff.to(
+ entry.getDiskName(),
+ ImmutableMap.<String, Long>builder()
+ .put(CgroupUtil.DISK_READ_BYTES_METRIC, entry.getReadBytes())
+ .put(CgroupUtil.DISK_READ_COUNT_METRIC, entry.getReadCount())
+ .put(CgroupUtil.DISK_WRITE_BYTES_METRIC, entry.getWriteBytes())
+ .put(CgroupUtil.DISK_WRITE_COUNT_METRIC, entry.getWriteCount())
+ .build()
+ );
+
+ if (stats != null) {
+ final ServiceMetricEvent.Builder builder = builder()
+ .setDimension("diskName", entry.getDiskName());
+ MonitorUtils.addDimensionsToBuilder(builder, dimensions);
+ for (Map.Entry<String, Long> stat : stats.entrySet()) {
+ emitter.emit(builder.setMetric(stat.getKey(), stat.getValue()));
+ }
+ }
+ }
+ return true;
+ }
+
+ public List<Disk.Metrics> snapshot()
+ {
+ List<Disk.Metrics> diskStats = new ArrayList<>();
+ try (final BufferedReader reader = Files.newBufferedReader(
+ Paths.get(cgroupDiscoverer.discover("disk").toString(), IO_STAT))) {
+ for (String line = reader.readLine(); line != null; line = reader.readLine()) {
+ Disk.Metrics disk = getDiskMetrics(line);
+ diskStats.add(disk);
+ }
+ }
+ catch (IOException | RuntimeException ex) {
+ LOG.error(ex, "Unable to fetch memory snapshot");
+ }
+ return diskStats;
+ }
+
+ private static Disk.Metrics getDiskMetrics(String line)
+ {
+ final String[] parts = line.split(Pattern.quote(" "));
+
+ Disk.Metrics disk = new Disk.Metrics(parts[0]);
+ Map<String, Long> stats = new HashMap<>();
+ for (int i = 1; i < parts.length; i++) {
+ String[] keyValue = parts[i].split("=");
+ if (keyValue.length == 2) {
+ stats.put(keyValue[0], Long.parseLong(keyValue[1]));
Review Comment:
I think I got confused here because it looks like if there is an issue with parsing metrics for any disk, no disk metrics are reported and that is perhaps what hte code scanning bot is complaining about.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] cgroup monitors: Add mem/disk/cpu usage metrics for V2 (druid)
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #16905:
URL: https://github.com/apache/druid/pull/16905#discussion_r1719331803
##########
processing/src/main/java/org/apache/druid/java/util/metrics/CgroupV2DiskMonitor.java:
##########
@@ -0,0 +1,130 @@
+/*
+ * 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.java.util.metrics;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+import org.apache.druid.java.util.common.logger.Logger;
+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.cgroups.CgroupDiscoverer;
+import org.apache.druid.java.util.metrics.cgroups.Disk;
+import org.apache.druid.java.util.metrics.cgroups.ProcCgroupV2Discoverer;
+import org.apache.druid.java.util.metrics.cgroups.ProcSelfCgroupDiscoverer;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Pattern;
+
+public class CgroupV2DiskMonitor extends FeedDefiningMonitor
+{
+ private static final Logger LOG = new Logger(CgroupV2DiskMonitor.class);
+ private static final String IO_STAT = "io.stat";
+ final CgroupDiscoverer cgroupDiscoverer;
+ final Map<String, String[]> dimensions;
+ private final KeyedDiff diff = new KeyedDiff();
+
+ public CgroupV2DiskMonitor(CgroupDiscoverer cgroupDiscoverer, final Map<String, String[]> dimensions, String feed)
+ {
+ super(feed);
+ this.cgroupDiscoverer = cgroupDiscoverer;
+ this.dimensions = dimensions;
+ }
+
+ @VisibleForTesting
+ CgroupV2DiskMonitor(CgroupDiscoverer cgroupDiscoverer)
+ {
+ this(cgroupDiscoverer, ImmutableMap.of(), DEFAULT_METRICS_FEED);
+ }
+
+ CgroupV2DiskMonitor()
+ {
+ this(new ProcSelfCgroupDiscoverer(ProcCgroupV2Discoverer.class));
+ }
+
+
+ @Override
+ public boolean doMonitor(ServiceEmitter emitter)
+ {
+ for (Disk.Metrics entry : snapshot()) {
+ final Map<String, Long> stats = diff.to(
+ entry.getDiskName(),
+ ImmutableMap.<String, Long>builder()
+ .put(CgroupUtil.DISK_READ_BYTES_METRIC, entry.getReadBytes())
+ .put(CgroupUtil.DISK_READ_COUNT_METRIC, entry.getReadCount())
+ .put(CgroupUtil.DISK_WRITE_BYTES_METRIC, entry.getWriteBytes())
+ .put(CgroupUtil.DISK_WRITE_COUNT_METRIC, entry.getWriteCount())
+ .build()
+ );
+
+ if (stats != null) {
+ final ServiceMetricEvent.Builder builder = builder()
+ .setDimension("diskName", entry.getDiskName());
+ MonitorUtils.addDimensionsToBuilder(builder, dimensions);
+ for (Map.Entry<String, Long> stat : stats.entrySet()) {
+ emitter.emit(builder.setMetric(stat.getKey(), stat.getValue()));
+ }
+ }
+ }
+ return true;
+ }
+
+ public List<Disk.Metrics> snapshot()
+ {
+ List<Disk.Metrics> diskStats = new ArrayList<>();
+ try (final BufferedReader reader = Files.newBufferedReader(
+ Paths.get(cgroupDiscoverer.discover("disk").toString(), IO_STAT))) {
+ for (String line = reader.readLine(); line != null; line = reader.readLine()) {
+ Disk.Metrics disk = getDiskMetrics(line);
+ diskStats.add(disk);
+ }
+ }
+ catch (IOException | RuntimeException ex) {
+ LOG.error(ex, "Unable to fetch memory snapshot");
+ }
+ return diskStats;
+ }
+
+ private static Disk.Metrics getDiskMetrics(String line)
+ {
+ final String[] parts = line.split(Pattern.quote(" "));
+
+ Disk.Metrics disk = new Disk.Metrics(parts[0]);
+ Map<String, Long> stats = new HashMap<>();
+ for (int i = 1; i < parts.length; i++) {
+ String[] keyValue = parts[i].split("=");
+ if (keyValue.length == 2) {
+ stats.put(keyValue[0], Long.parseLong(keyValue[1]));
Review Comment:
## Missing catch of NumberFormatException
Potential uncaught 'java.lang.NumberFormatException'.
[Show more details](https://github.com/apache/druid/security/code-scanning/7721)
##########
processing/src/test/java/org/apache/druid/java/util/metrics/CgroupV2CpuMonitorTest.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.java.util.metrics;
+
+import com.google.common.collect.ImmutableSet;
+import org.apache.druid.java.util.emitter.core.Event;
+import org.apache.druid.java.util.metrics.cgroups.CgroupDiscoverer;
+import org.apache.druid.java.util.metrics.cgroups.ProcCgroupV2Discoverer;
+import org.apache.druid.java.util.metrics.cgroups.TestUtils;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.rules.TemporaryFolder;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.List;
+import java.util.stream.Collectors;
+
+public class CgroupV2CpuMonitorTest
+{
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
Review Comment:
## Deprecated method or constructor invocation
Invoking [ExpectedException.none](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/7718)
##########
processing/src/test/java/org/apache/druid/java/util/metrics/CgroupV2MemoryMonitorTest.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * 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.java.util.metrics;
+
+import org.apache.druid.java.util.emitter.core.Event;
+import org.apache.druid.java.util.metrics.cgroups.CgroupDiscoverer;
+import org.apache.druid.java.util.metrics.cgroups.ProcCgroupV2Discoverer;
+import org.apache.druid.java.util.metrics.cgroups.TestUtils;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.rules.TemporaryFolder;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.List;
+
+public class CgroupV2MemoryMonitorTest
+{
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
Review Comment:
## Deprecated method or constructor invocation
Invoking [ExpectedException.none](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/7720)
##########
processing/src/test/java/org/apache/druid/java/util/metrics/CgroupV2DiskMonitorTest.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.java.util.metrics;
+
+import org.apache.druid.java.util.metrics.cgroups.CgroupDiscoverer;
+import org.apache.druid.java.util.metrics.cgroups.ProcCgroupV2Discoverer;
+import org.apache.druid.java.util.metrics.cgroups.TestUtils;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.rules.TemporaryFolder;
+
+import java.io.File;
+import java.io.IOException;
+
+public class CgroupV2DiskMonitorTest
+{
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
Review Comment:
## Deprecated method or constructor invocation
Invoking [ExpectedException.none](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/7719)
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] cgroup monitors: Add mem/disk/cpu usage metrics for V2 (druid)
Posted by "arunramani (via GitHub)" <gi...@apache.org>.
arunramani commented on PR #16905:
URL: https://github.com/apache/druid/pull/16905#issuecomment-2299602770
The changes looks reasonable to me. My one ask would be to add some javadoc comments around what proc file is being parsed and what the expected format is supposed to be.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] cgroup monitors: Add mem/disk/cpu usage metrics for V2 (druid)
Posted by "suneet-s (via GitHub)" <gi...@apache.org>.
suneet-s commented on code in PR #16905:
URL: https://github.com/apache/druid/pull/16905#discussion_r1764977818
##########
processing/src/main/java/org/apache/druid/java/util/metrics/CgroupV2CpuMonitor.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * 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.java.util.metrics;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.primitives.Longs;
+import org.apache.druid.java.util.common.logger.Logger;
+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.cgroups.CgroupDiscoverer;
+import org.apache.druid.java.util.metrics.cgroups.Cpu;
+import org.apache.druid.java.util.metrics.cgroups.ProcCgroupV2Discoverer;
+import org.apache.druid.java.util.metrics.cgroups.ProcSelfCgroupDiscoverer;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.regex.Pattern;
+
+public class CgroupV2CpuMonitor extends FeedDefiningMonitor
+{
+ private static final Logger LOG = new Logger(CgroupV2CpuMonitor.class);
+ private static final String CPU_STAT_FILE = "cpu.stat";
+ private static final String SNAPSHOT = "snapshot";
+ final CgroupDiscoverer cgroupDiscoverer;
+ final Map<String, String[]> dimensions;
+ private final KeyedDiff diff = new KeyedDiff();
+
+ public CgroupV2CpuMonitor(CgroupDiscoverer cgroupDiscoverer, final Map<String, String[]> dimensions, String feed)
+ {
+ super(feed);
+ this.cgroupDiscoverer = cgroupDiscoverer;
+ this.dimensions = dimensions;
+ }
+
+ @VisibleForTesting
+ CgroupV2CpuMonitor(CgroupDiscoverer cgroupDiscoverer)
+ {
+ this(cgroupDiscoverer, ImmutableMap.of(), DEFAULT_METRICS_FEED);
+ }
+
+ CgroupV2CpuMonitor()
+ {
+ this(new ProcSelfCgroupDiscoverer(ProcCgroupV2Discoverer.class));
+ }
+
+ @Override
+ public boolean doMonitor(ServiceEmitter emitter)
+ {
+ final ServiceMetricEvent.Builder builder = builder();
+ MonitorUtils.addDimensionsToBuilder(builder, dimensions);
+ Snapshot snapshot = snapshot();
+ final Map<String, Long> elapsed = diff.to(
+ "usage",
+ ImmutableMap.<String, Long>builder()
+ .put(CgroupUtil.USER, snapshot.getUserUsec())
+ .put(CgroupUtil.SYSTEM, snapshot.getSystemUsec())
+ .put(CgroupUtil.TOTAL, snapshot.getUsageUsec())
+ .put(SNAPSHOT, ChronoUnit.MICROS.between(Instant.EPOCH, Instant.now()))
+ .build()
+ );
+
+ if (elapsed != null) {
+ long elapsedUsecs = elapsed.get(SNAPSHOT);
+ double totalUsagePct = 100.0 * elapsed.get(CgroupUtil.TOTAL) / elapsedUsecs;
+ double sysUsagePct = 100.0 * elapsed.get(CgroupUtil.SYSTEM) / elapsedUsecs;
+ double userUsagePct = 100.0 * elapsed.get(CgroupUtil.USER) / elapsedUsecs;
+ emitter.emit(builder.setMetric(CgroupUtil.CPU_TOTAL_USAGE_METRIC, totalUsagePct));
+ emitter.emit(builder.setMetric(CgroupUtil.CPU_SYS_USAGE_METRIC, sysUsagePct));
+ emitter.emit(builder.setMetric(CgroupUtil.CPU_USER_USAGE_METRIC, userUsagePct));
+ }
+ return true;
+ }
+
+ /*
+ file: cpu.stat
+
+ sample content:
+ usage_usec 2379951538
+ user_usec 1802023024
+ system_usec 577928513
+ nr_periods 1581231
+ nr_throttled 59
+ throttled_usec 3095133
+ */
+ public Snapshot snapshot()
+ {
+ Map<String, Long> entries = new HashMap<>();
+ try (final BufferedReader reader = Files.newBufferedReader(
+ Paths.get(cgroupDiscoverer.discover(Cpu.CGROUP).toString(), CPU_STAT_FILE)
+ )) {
+ for (String line = reader.readLine(); line != null; line = reader.readLine()) {
+ final String[] parts = line.split(Pattern.quote(" "));
+ if (parts.length != 2) {
+ // ignore
+ continue;
+ }
+ entries.put(parts[0], Longs.tryParse(parts[1]));
+ }
+ }
+ catch (IOException | RuntimeException ex) {
+ LOG.error(ex, "Unable to fetch cpu snapshot");
+ }
+
+ return new Snapshot(entries.get("usage_usec"), entries.get("user_usec"), entries.get("system_usec"));
+ }
+
+
+ public static class Snapshot
+ {
+ private final long usageUsec;
+ private final long userUsec;
+ private final long systemUsec;
+
+ public Snapshot(long usageUsec, long userUsec, long systemUsec)
Review Comment:
Should the constructor accept `@Nullable Long` objects instead so that we can distinguish between parse exceptions and 0s when reading the file?
##########
processing/src/main/java/org/apache/druid/java/util/metrics/CgroupV2CpuMonitor.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * 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.java.util.metrics;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.primitives.Longs;
+import org.apache.druid.java.util.common.logger.Logger;
+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.cgroups.CgroupDiscoverer;
+import org.apache.druid.java.util.metrics.cgroups.Cpu;
+import org.apache.druid.java.util.metrics.cgroups.ProcCgroupV2Discoverer;
+import org.apache.druid.java.util.metrics.cgroups.ProcSelfCgroupDiscoverer;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.regex.Pattern;
+
+public class CgroupV2CpuMonitor extends FeedDefiningMonitor
Review Comment:
nit: javadoc please. This could be a good place to describe what teh expected format of the file you're reading looks like. Similar comment for the other monitors
##########
processing/src/main/java/org/apache/druid/java/util/metrics/cgroups/Cpu.java:
##########
@@ -34,8 +34,8 @@
*/
public class Cpu
{
+ public static final String CGROUP = "cpu";
Review Comment:
nit unnecessary change?
##########
processing/src/main/java/org/apache/druid/java/util/metrics/CgroupV2DiskMonitor.java:
##########
@@ -0,0 +1,130 @@
+/*
+ * 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.java.util.metrics;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+import org.apache.druid.java.util.common.logger.Logger;
+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.cgroups.CgroupDiscoverer;
+import org.apache.druid.java.util.metrics.cgroups.Disk;
+import org.apache.druid.java.util.metrics.cgroups.ProcCgroupV2Discoverer;
+import org.apache.druid.java.util.metrics.cgroups.ProcSelfCgroupDiscoverer;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Pattern;
+
+public class CgroupV2DiskMonitor extends FeedDefiningMonitor
+{
+ private static final Logger LOG = new Logger(CgroupV2DiskMonitor.class);
+ private static final String IO_STAT = "io.stat";
+ final CgroupDiscoverer cgroupDiscoverer;
+ final Map<String, String[]> dimensions;
+ private final KeyedDiff diff = new KeyedDiff();
+
+ public CgroupV2DiskMonitor(CgroupDiscoverer cgroupDiscoverer, final Map<String, String[]> dimensions, String feed)
+ {
+ super(feed);
+ this.cgroupDiscoverer = cgroupDiscoverer;
+ this.dimensions = dimensions;
+ }
+
+ @VisibleForTesting
+ CgroupV2DiskMonitor(CgroupDiscoverer cgroupDiscoverer)
+ {
+ this(cgroupDiscoverer, ImmutableMap.of(), DEFAULT_METRICS_FEED);
+ }
+
+ CgroupV2DiskMonitor()
+ {
+ this(new ProcSelfCgroupDiscoverer(ProcCgroupV2Discoverer.class));
+ }
+
+
+ @Override
+ public boolean doMonitor(ServiceEmitter emitter)
+ {
+ for (Disk.Metrics entry : snapshot()) {
+ final Map<String, Long> stats = diff.to(
+ entry.getDiskName(),
+ ImmutableMap.<String, Long>builder()
+ .put(CgroupUtil.DISK_READ_BYTES_METRIC, entry.getReadBytes())
+ .put(CgroupUtil.DISK_READ_COUNT_METRIC, entry.getReadCount())
+ .put(CgroupUtil.DISK_WRITE_BYTES_METRIC, entry.getWriteBytes())
+ .put(CgroupUtil.DISK_WRITE_COUNT_METRIC, entry.getWriteCount())
+ .build()
+ );
+
+ if (stats != null) {
+ final ServiceMetricEvent.Builder builder = builder()
+ .setDimension("diskName", entry.getDiskName());
+ MonitorUtils.addDimensionsToBuilder(builder, dimensions);
+ for (Map.Entry<String, Long> stat : stats.entrySet()) {
+ emitter.emit(builder.setMetric(stat.getKey(), stat.getValue()));
+ }
+ }
+ }
+ return true;
+ }
+
+ public List<Disk.Metrics> snapshot()
+ {
+ List<Disk.Metrics> diskStats = new ArrayList<>();
+ try (final BufferedReader reader = Files.newBufferedReader(
+ Paths.get(cgroupDiscoverer.discover("disk").toString(), IO_STAT))) {
+ for (String line = reader.readLine(); line != null; line = reader.readLine()) {
+ Disk.Metrics disk = getDiskMetrics(line);
+ diskStats.add(disk);
+ }
+ }
+ catch (IOException | RuntimeException ex) {
+ LOG.error(ex, "Unable to fetch memory snapshot");
+ }
+ return diskStats;
+ }
+
+ private static Disk.Metrics getDiskMetrics(String line)
+ {
+ final String[] parts = line.split(Pattern.quote(" "));
+
+ Disk.Metrics disk = new Disk.Metrics(parts[0]);
+ Map<String, Long> stats = new HashMap<>();
+ for (int i = 1; i < parts.length; i++) {
+ String[] keyValue = parts[i].split("=");
+ if (keyValue.length == 2) {
+ stats.put(keyValue[0], Long.parseLong(keyValue[1]));
Review Comment:
Similar comment as the Cpu Monitor - if we use `Longs.parseLong` we will need to handle nulls when emitting the metrics
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] cgroup monitors: Add mem/disk/cpu usage metrics for V2 (druid)
Posted by "arunramani (via GitHub)" <gi...@apache.org>.
arunramani commented on code in PR #16905:
URL: https://github.com/apache/druid/pull/16905#discussion_r1723906616
##########
docs/configuration/index.md:
##########
@@ -394,6 +394,9 @@ Metric monitoring is an essential part of Druid operations. The following monito
|`org.apache.druid.java.util.metrics.CgroupCpuSetMonitor`|Reports CPU core/HT and memory node allocations as per the `cpuset` cgroup.|
|`org.apache.druid.java.util.metrics.CgroupDiskMonitor`|Reports disk statistic as per the blkio cgroup.|
|`org.apache.druid.java.util.metrics.CgroupMemoryMonitor`|Reports memory statistic as per the memory cgroup.|
+|`org.apache.druid.java.util.metrics.CgroupV2CpuMonitor`|Reports CPU usage from `cpu.stat` file. Only applicable to `cgroupv2`.|
Review Comment:
Now that I think about it, would it make sense to have just a single monitor and use the right cgroup discoverer instead? It might be too annoying for a user to figure out which one to use depending on how they setup their container.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] cgroup monitors: Add mem/disk/cpu usage metrics for V2 (druid)
Posted by "adithyachakilam (via GitHub)" <gi...@apache.org>.
adithyachakilam commented on PR #16905:
URL: https://github.com/apache/druid/pull/16905#issuecomment-2299645627
Added the comments for io.stat and cpu.stat, memory is same as v1. Also, test cases implies the format of the files
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org