refactor!: remove deprecated SysMonitor - #20200
Conversation
2061c65 to
41336f8
Compare
FrankChen021
left a comment
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
Reviewed 19 of 19 changed files.
This is an automated review by Codex GPT-5.6-Luna(max)
| final ServiceMetricEvent.Builder builder = builder(); | ||
| final long userTime = currentProcess.getUserTime(); | ||
| final long sysTime = currentProcess.getKernelTime(); | ||
| final Map<String, Long> procDiff = diff.to( |
There was a problem hiding this 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.
There was a problem hiding this 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.
Fixes #10615.
Description
The deprecated
org.apache.druid.java.util.metrics.SysMonitormonitor is removed. Its no-op variant, unused Druid subclass, Guice provider, and obsolete tests are removed as well.JvmCpuMonitornow uses OSHI process snapshots to report the existing total, user, system, and percentage CPU metrics.JvmPidDiscovereruses JavaProcessHandleto discover the current JVM PID. The Sigar PID discoverer and utility, native library packaging, Maven dependencies, and associated tests have been removed.Existing configurations that list
SysMonitorindruid.monitoring.monitorsmust be updated to useorg.apache.druid.java.util.metrics.OshiSysMonitor.The metrics documentation now documents only
OshiSysMonitor.Release note
SysMonitorhas been removed after being deprecated in Druid 27. UseOshiSysMonitorinstead.Validation
mvn -ntp test -pl processing,server -am -Dtest="org.apache.druid.java.util.metrics.JvmCpuMonitorTest,org.apache.druid.java.util.metrics.JvmPidDiscovererTest,org.apache.druid.java.util.metrics.OshiSysMonitorTest,org.apache.druid.java.util.metrics.NoopOshiSysMonitorTest,org.apache.druid.server.metrics.MetricsModuleTest" -Dsurefire.failIfNoSpecifiedTests=false -Pskip-static-checks -Dweb.console.skip=true -T1Cnpm run spellcheck