Add macOS logs, battery, thermal, storage, smart collectors - #22475
Merged
Conversation
ktsaou
requested review from
a team,
Ancairon,
Ferroin,
ilyam8,
kanelatechnical,
thiagoftsm and
vkalintiris
as code owners
May 13, 2026 05:57
|
|
ktsaou
marked this pull request as draft
May 13, 2026 08:09
|
ktsaou
marked this pull request as ready for review
May 17, 2026 19:39
Contributor
There was a problem hiding this comment.
6 issues found and verified against the latest diff
Confidence score: 3/5
src/collectors/macos-logs.plugin/macos-logs.chas a shutdown race whereused_hashes_registrymay be freed before worker threads fully unwind, which can trigger a use-after-free crash during plugin exit; addfunctions_evloop_join_threads(wg)after cancellation before merging.src/collectors/macos.plugin/macos_powermetrics.ccan stop reading stdout too early whenPOLLHUPappears alongside unread data, causing intermittent truncated plist reads and missed samples; only break on HUP whenPOLLINis not set so buffered bytes are drained.src/collectors/macos.plugin/macos_nvme.cdelays rediscovery after transient read outages becauselast_discovery_utstill gates probes, so NVMe charts may remain absent for a full interval; reset discovery timing when calls fail to restore charts promptly.src/collectors/macos.plugin/macos_power.candsrc/collectors/macos.plugin/plugin_macos.chave power-source lifecycle risks (sanitized ID collisions can merge device metrics, and shutdown misses power-source cleanup), leading to overwritten charts and lingering resources; keep a raw unique key for lookup and add power cleanup inmacos_main_cleanupbefore merge.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/collectors/macos.plugin/macos_power.c">
<violation number="1" location="src/collectors/macos.plugin/macos_power.c:342">
P2: Different power sources can collapse into one chart when their original names differ but sanitize to the same chart ID, so one device’s metrics can overwrite the other. Keeping a raw unique identity for lookup and using a separate sanitized value only for chart IDs avoids this collision path.</violation>
</file>
<file name="src/collectors/macos.plugin/plugin_macos.c">
<violation number="1" location="src/collectors/macos.plugin/plugin_macos.c:38">
P3: Power sources cleanup is missing from the plugin shutdown path. The `macos_main_cleanup` function calls `macos_powermetrics_cleanup()` and `macos_nvme_smart_cleanup()` to free background threads, mark charts obsolete, and release IOKit service references, but there is no equivalent `macos_power_sources_cleanup()` for the `power_sources_root` linked list. On plugin shutdown or dynamic module disable, every `callocz` and `strdupz` call in `macos_power_source_get_or_create` leaks the entire list. The diff adds cleanup calls at this exact location for the other two modules; consider adding a power-sources cleanup that frees the list and marks its charts obsolete before the worker unregisters.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
Member
Author
|
@cubic-dev-ai please review again |
Contributor
@ktsaou I have started the AI code review. It will take a few minutes to complete. |
|
| if(!cmd || cmd[0] != '/' || !dst || !dst_size) | ||
| return false; | ||
|
|
||
| size_t len = strnlen(cmd, dst_size); |
stelfrag
added a commit
to stelfrag/netdata
that referenced
this pull request
Jul 12, 2026
…#23085) Backport the macos.plugin sensor collectors (gpu, sensors/SMC, power, powermetrics, nvme, iohid) and the shared hw-sensors histogram/function framework to the 2.10.x line. macOS-only paths taken from master tip; the functions_evloop/logs/network-viewer/status-file changes bundled in those PRs are excluded (not needed by the in-process macOS collector). Adds the ndsudo powermetrics whitelist (with <errno.h>) and CMake wiring.
Merged
Ferroin
pushed a commit
that referenced
this pull request
Jul 15, 2026
Backport the macos.plugin sensor collectors (gpu, sensors/SMC, power, powermetrics, nvme, iohid) and the shared hw-sensors histogram/function framework to the 2.10.x line. macOS-only paths taken from master tip; the functions_evloop/logs/network-viewer/status-file changes bundled in those PRs are excluded (not needed by the in-process macOS collector). Adds the ndsudo powermetrics whitelist (with <errno.h>) and CMake wiring.
Contributor
|
@ktsaou Apparently everything was added without proper version checks, and now builds break down on macos plugin: |
1 task
Collaborator
|
@barracuda156 #23421 may fix this. If you would be kind enough to check |
Contributor
|
@stelfrag Thank you very much, I will try that. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds native macOS endpoint monitoring coverage and the supporting build, install, documentation, and review-fix work needed to ship it.
macos-logs.plugin --testpath for reproducible local queries.smartctl, including Apple Silicon Homebrew paths and generated integration documentation.network-viewer.pluginon macOS with a Darwinlibprocsocket backend, macOS install enablement, permission/TCC warnings,network-connections,topology:network-connections, andnetwork-protocolssupport.macos-logs.plugin,ndsudopowermetrics commands, setuid ownership/mode handling, and build-info/install metadata updates.Validation
git diff --checkpython3 integrations/gen_integrations.pypython3 integrations/gen_docs_integrations.pyfor affected collectorspython3 -m py_compile integrations/gen_docs_integrations.pycmake --build build --target netdata -j2cmake --build build --target ndsudo -j2cmake --build build --target macos-logs.plugin -j2cmake --build build --target cups.plugin -j2cmake --build .local/build-network-viewer-macos --target network-viewer.plugin -j 6macos-logs.plugin --testFunction-protocol query returned HTTP 200 and unified-log rows/opt/netdataruntime check: Agent running,/api/v1/inforeturns macOSmacos-logs.pluginandndsudoare root-owned setuid helpers with mode4750ndsudo powermetrics-thermal-smc --sampleWindowMs 1000as the Netdata user produced a plist without superuser-permission failurenetwork-protocols,network-connections sockets:aggregated,topology:network-connections, and explicit UDP PID/port visibilityNotes
powermetricsexposed thermal pressure but returned an empty SMC dictionary, so fan/SMC chart validation is limited by the data available on the test host.