feat(collectors): add Windows support for hardware monitoring collectors#21635
Merged
ilyam8 merged 3 commits intonetdata:masterfrom Feb 13, 2026
Merged
feat(collectors): add Windows support for hardware monitoring collectors#21635ilyam8 merged 3 commits intonetdata:masterfrom
ilyam8 merged 3 commits intonetdata:masterfrom
Conversation
github-actions
bot
added
area/collectors
Member
Author
|
@ilyam8 people are searching for megacli on windows - this is how I figured this out. The changes mirror the pattern found on |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 34 files
Confidence score: 3/5
- Some risk due to path handling across multiple collectors; several issues could cause unintended binaries to be resolved when ProgramFiles variables are empty or inaccessible, which is user-impacting on Windows.
- Most severe:
src/go/plugin/go.d/collector/nvidia_smi/init.gotreats anyos.Staterror as missing, so an inaccessible configuredbinary_pathcan silently fall back to PATH/defaults and execute a different binary. - Debuggability concern:
src/go/plugin/go.d/collector/megacli/exec.godrops stderr fromcmd.Output(), making execution failures harder to troubleshoot. - Pay close attention to
src/go/plugin/go.d/collector/nvidia_smi/init.go,src/go/plugin/go.d/collector/hpssa/init.go,src/go/plugin/go.d/collector/storcli/init.go,src/go/plugin/go.d/collector/nvme/init.go,src/go/plugin/go.d/collector/megacli/exec.go- path fallback and error-reporting behaviors.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/go/plugin/go.d/collector/nvidia_smi/init.go">
<violation number="1" location="src/go/plugin/go.d/collector/nvidia_smi/init.go:49">
P2: `fileExists` treats any `os.Stat` error as “missing”, so a configured but inaccessible `binary_path` will be ignored and the collector will fall back to PATH/defaults, potentially executing a different binary. Preserve the original behavior by only falling back on `os.IsNotExist`.</violation>
</file>
<file name="src/go/plugin/go.d/collector/megacli/exec.go">
<violation number="1" location="src/go/plugin/go.d/collector/megacli/exec.go:73">
P3: cmd.Output() discards stderr, so Windows execution failures lose the CLI’s diagnostic output and are hard to troubleshoot. Capture stderr (like ndexec does) and include it in the error message.</violation>
</file>
<file name="src/go/plugin/go.d/collector/hpssa/init.go">
<violation number="1" location="src/go/plugin/go.d/collector/hpssa/init.go:35">
P2: Guard ProgramFiles env lookups to avoid generating relative default paths when the variables are unset.</violation>
</file>
<file name="src/go/plugin/go.d/collector/storcli/init.go">
<violation number="1" location="src/go/plugin/go.d/collector/storcli/init.go:35">
P2: Guard against empty ProgramFiles environment variables so default paths remain absolute; otherwise a relative path can be resolved from the working directory and the collector could execute an unintended binary.</violation>
</file>
<file name="src/go/plugin/go.d/collector/nvme/init.go">
<violation number="1" location="src/go/plugin/go.d/collector/nvme/init.go:39">
P2: Guard against empty ProgramFiles variables before building fallback paths to avoid resolving relative `nvme-cli\nvme.exe` from the working directory.</violation>
</file>
Architecture diagram
sequenceDiagram
participant C as Collector (Go)
participant FS as FileSystem
participant OS as OS Process Manager
participant NDS as ndsudo (Linux/BSD)
participant CLI as Vendor Tool (Win)
Note over C: Scope: nvme, megacli, storcli,<br/>adaptecraid, hpssa, nvidia_smi
%% Initialization Phase
Note over C,FS: Initialization Phase
alt NEW: Runtime is Windows
C->>C: Check runtime.GOOS == "windows"
loop Search for Binary
C->>FS: LookPath() (PATH env)
opt Not found in PATH
C->>FS: os.Stat() (ProgramFiles defaults)
end
end
alt Binary Found
FS-->>C: Valid Path (e.g., "C:\...\nvme.exe")
C->>C: Init DirectExecutor (stores path)
else Binary Missing
C->>C: Error: Init failed
end
else UNCHANGED: Runtime is Linux/BSD
C->>C: Init NdsudoExecutor
end
%% Collection Phase
Note over C,CLI: Collection Loop (Tick)
C->>C: Interface Call (e.g., controllersInfo)
alt NEW: Windows (Direct Execution)
C->>OS: exec.Command(path, specific_args)
Note right of C: Args: -output-format=json, etc.<br/>No sudo wrapper needed
OS->>CLI: Spawn Process
CLI-->>OS: Stdout/Stderr
OS-->>C: Raw Bytes
else UNCHANGED: Linux/BSD (Privileged Wrapper)
C->>NDS: RunNDSudo(script_name)
Note right of C: Relies on external wrapper<br/>for privilege escalation
NDS-->>C: Raw Bytes
end
C->>C: Parse Output (JSON/Text)
C->>C: Update Metrics
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Member
|
I know that nvidia_smi works on Windows without your changes. I will need to understand them. |
Enable Windows support for the following Go collectors by implementing direct command execution alongside the existing ndsudo-based execution for Linux/BSD: - megacli: LSI MegaRAID controller monitoring - storcli: Broadcom StorCLI RAID controller monitoring - nvme: NVMe drive SMART monitoring - adaptecraid: Adaptec RAID controller monitoring - hpssa: HP Smart Storage Administrator monitoring - nvidia_smi: NVIDIA GPU monitoring Changes: - Remove Unix-only build constraints from collector files - Add platform detection using runtime.GOOS in init.go - Implement direct CLI execution for Windows (bypassing ndsudo) - Add Windows default installation path detection for each tool This follows the same pattern established by the smartctl collector for cross-platform support. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ilyam8
force-pushed
the
windows-collectors-support
branch
from
5b2666d to
23a8b32
Compare
Add RunDirect() and FindBinary() to pkg/ndexec to eliminate ~250 lines of identical code duplicated across 7 hardware monitoring collectors. RunDirect() runs a binary directly with timeout and stderr capture, replacing the copy-pasted execute() method in each collector's direct exec implementation. FindBinary() searches PATH names then default filesystem paths, replacing the duplicated binary lookup logic in each collector's init. Refactored collectors: adaptecraid, megacli, storcli, hpssa, nvme, nvidia_smi, smartctl.
Member
|
@cubic-dev-ai review this PR |
Contributor
@ilyam8 I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
1 issue found across 15 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/go/plugin/go.d/pkg/ndexec/ndexec.go">
<violation number="1" location="src/go/plugin/go.d/pkg/ndexec/ndexec.go:127">
P3: The log statement uses `%s` with an *exec.Cmd, which results in a formatting error (`%!s(*exec.Cmd=...)`) instead of the command. Use `%v` or `cmd.String()` to log the command cleanly.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
1 issue found across 37 files
Confidence score: 4/5
- Overall risk looks low; only a minor robustness issue is noted, and it’s low severity (4/10) with moderate confidence.
- In
src/go/plugin/go.d/collector/nvidia_smi/init.go,fileExiststreats all stat errors as missing, so permission/IO errors could be masked by falling back to PATH lookup, potentially hiding real access problems. - Pay close attention to
src/go/plugin/go.d/collector/nvidia_smi/init.go- error handling aroundfileExistsmay mask permission/IO failures.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/go/plugin/go.d/collector/nvidia_smi/init.go">
<violation number="1" location="src/go/plugin/go.d/collector/nvidia_smi/init.go:45">
P2: `fileExists` treats any stat error as "missing," so permission/IO errors will silently fall back to PATH lookup instead of surfacing the real issue. Consider treating non-"not exists" errors as existing so the later validation returns the proper error.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Coll as Hardware Collector (e.g., NVMe, MegaCLI)
participant Lib as ndexec Utility
participant Env as OS Environment (FS/Vars)
participant Sudo as ndsudo (Unix Wrapper)
participant CLI as Vendor CLI (Binary)
Note over Coll,CLI: Initialization Flow
Coll->>Coll: Check runtime.GOOS
alt NEW: Windows Path
Coll->>Lib: NEW: FindBinary(names, winPaths)
Lib->>Env: Check %PATH%
Lib->>Env: NEW: Check common Vendor install paths
Env-->>Lib: Return absolute path (e.g., C:\Program Files\...)
Lib-->>Coll: Binary path
else Linux / BSD Path
Coll->>Coll: Use standard ndsudo command alias
end
Note over Coll,CLI: Runtime Collection Flow
alt NEW: Windows (Direct Execution)
Coll->>Lib: NEW: RunDirect(binPath, args)
Lib->>CLI: NEW: exec.CommandContext() (direct call)
CLI-->>Lib: Standard Output (JSON/XML/Text)
Lib-->>Coll: Raw data bytes
else Linux / BSD (Privileged Execution)
Coll->>Lib: RunNDSudo(alias, args)
Lib->>Sudo: Call ndsudo wrapper
Sudo->>CLI: Execute with escalated privileges
CLI-->>Sudo: Output
Sudo-->>Lib: Output
Lib-->>Coll: Raw data bytes
end
Note over Coll: Data Parsing & Chart Updates
opt Error Handling (Most Important Unhappy Path)
CLI-->>Lib: Exit Code > 0 / Stderr
Lib->>Lib: CHANGED: Capture & trim stderr
Lib-->>Coll: Error with stderr snippet
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request adds Windows support to six hardware monitoring collectors (megacli, storcli, nvme, adaptecraid, hpssa, nvidia_smi) by implementing direct command execution alongside the existing ndsudo-based execution for Linux/BSD systems.
Changes:
- Added
RunDirectandFindBinaryhelper functions to ndexec package for direct command execution and binary path discovery - Refactored six hardware collectors to use runtime-based platform detection and dispatch to appropriate executor implementations
- Removed Unix-only build constraints from collector files to enable cross-platform compilation
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/go/plugin/go.d/pkg/ndexec/ndexec.go | Added RunDirect and FindBinary helper functions for Windows support |
| src/go/plugin/go.d/pkg/ndexec/ndexec_test.go | Added comprehensive tests for new RunDirect and FindBinary functions |
| src/go/plugin/go.d/collector/storcli/*.go | Removed build constraints, added Windows executor with PATH and default path detection |
| src/go/plugin/go.d/collector/nvme/*.go | Removed build constraints, added Windows executor with direct nvme-cli execution |
| src/go/plugin/go.d/collector/nvidia_smi/*.go | Added Windows fallback paths and direct executor for nvidia-smi |
| src/go/plugin/go.d/collector/megacli/*.go | Removed build constraints, added Windows executor for MegaCLI |
| src/go/plugin/go.d/collector/hpssa/*.go | Added Windows executor for HP ssacli tool |
| src/go/plugin/go.d/collector/adaptecraid/*.go | Removed build constraints, added Windows executor for Adaptec arcconf |
| src/go/plugin/go.d/collector/smartctl/init.go | Refactored to use new FindBinary helper instead of custom path lookup logic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…liability - FindBinary: use strings.Join for readable error messages instead of Go slice formatting; omit PATH mention when no names were searched - nvidia_smi fileExists: use os.IsNotExist so permission/IO errors are not silently treated as "file missing"
ilyam8
approved these changes
Feb 13, 2026
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
Enable Windows support for hardware monitoring Go collectors by implementing direct command execution alongside the existing ndsudo-based execution for Linux/BSD.
Collectors Updated
Changes
runtime.GOOSininit.goThis follows the same pattern established by the smartctl collector for cross-platform support.
Test Plan
Summary by cubic
Add Windows support to hardware monitoring collectors by running vendor CLIs directly with auto-detected paths, while Linux/BSD continue using ndsudo. Centralized direct execution and binary lookup in ndexec to simplify code and improve reliability.
New Features
Refactors
Written for commit f6b1575. Summary will update on new commits.