FrankChen021 commented on code in PR #20200:
URL: https://github.com/apache/druid/pull/20200#discussion_r3904061078
##########
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:
[P2] Do not advance the diff on failed OSHI snapshots
`OperatingSystem.getCurrentProcess()` is non-null by contract: when the
native lookup fails it returns a `CurrentProcessStub` with zero or unknown
fields. Because this code only checks for null, that stub is fed into
`KeyedDiff` and then stored as `previousProcess`, emitting bogus zero deltas
and making the next successful sample compare against the zero snapshot. Detect
an invalid snapshot and leave the diff and previous-process state unchanged
when OSHI cannot read the process.
--
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]
