FrankChen021 commented on code in PR #20200:
URL: https://github.com/apache/druid/pull/20200#discussion_r3904240976
##########
processing/src/main/java/org/apache/druid/java/util/metrics/JvmCpuMonitor.java:
##########
@@ -19,60 +19,74 @@
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.hyperic.sigar.ProcCpu;
-import org.hyperic.sigar.Sigar;
-import org.hyperic.sigar.SigarException;
+import oshi.SystemInfo;
+import oshi.software.os.OSProcess;
+import oshi.software.os.OperatingSystem;
import java.util.Map;
public class JvmCpuMonitor extends FeedDefiningMonitor
{
private static final Logger log = new Logger(JvmCpuMonitor.class);
- private final Sigar sigar = SigarUtil.getSigar();
- private final long currentProcessId = sigar.getPid();
-
+ private final OperatingSystem operatingSystem;
private final KeyedDiff diff = new KeyedDiff();
+ private OSProcess previousProcess;
public JvmCpuMonitor()
{
this(DEFAULT_METRICS_FEED);
}
public JvmCpuMonitor(String feed)
+ {
+ this(feed, new SystemInfo().getOperatingSystem());
+ }
+
+ @VisibleForTesting
+ JvmCpuMonitor(String feed, OperatingSystem operatingSystem)
{
super(feed);
+ this.operatingSystem = operatingSystem;
+ this.previousProcess = operatingSystem.getCurrentProcess();
}
@Override
public boolean doMonitor(ServiceEmitter emitter)
{
- // process CPU
- try {
- ProcCpu procCpu = sigar.getProcCpu(currentProcessId);
- final ServiceMetricEvent.Builder builder = builder();
- // delta for total, sys, user
- Map<String, Long> procDiff = diff.to(
- "proc/cpu", ImmutableMap.of(
- "jvm/cpu/total", procCpu.getTotal(),
- "jvm/cpu/sys", procCpu.getSys(),
- "jvm/cpu/user", procCpu.getUser()
- )
- );
- if (procDiff != null) {
- for (Map.Entry<String, Long> entry : procDiff.entrySet()) {
- emitter.emit(builder.setMetric(entry.getKey(), entry.getValue()));
- }
+ final OSProcess currentProcess = operatingSystem.getCurrentProcess();
+ if (currentProcess == null) {
+ log.error("Unable to get current process CPU metrics");
+ return true;
+ }
+
+ final ServiceMetricEvent.Builder builder = builder();
+ final long userTime = currentProcess.getUserTime();
+ final long sysTime = currentProcess.getKernelTime();
+ final Map<String, Long> procDiff = diff.to(
Review Comment:
Addressed in the latest commit. `JvmCpuMonitor` now uses the nullable
`OperatingSystem.getProcess(pid)` API, so a failed lookup is skipped before
either `KeyedDiff` or `previousProcess` is updated. This prevents failed
snapshots from producing bogus deltas or poisoning the next CPU percentage
calculation.
Added a regression test covering a valid snapshot, a failed lookup, and the
next valid snapshot; the original baselines are preserved.
Validation: 16 focused processing tests passed; Checkstyle reported 0
violations.
Reviewed 2 of 2 affected files.
<!-- mergelens:review -->
--
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]
