- Notifications
You must be signed in to change notification settings - Fork6.3k
nagios plugin support#21294
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
nagios plugin support#21294
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
6 issues found across 41 files
Prompt for AI agents (all 6 issues)
Understand the root cause of the following 6 issues and fix them.<file name="src/go/plugin/nagios.d/modules/nagios/config_schema.json"><violation number="1" location="src/go/plugin/nagios.d/modules/nagios/config_schema.json:35">The duration regex only allows a single integer segment with ns/us/ms/s/m/h, but the parser supports decimals, multiple segments, and broader units (e.g., 1.5s, 1h30m, 2d). The schema should reflect the parser’s capabilities to avoid rejecting valid configs.</violation></file><file name="src/go/plugin/nagios.d/pkg/timeperiod/compile.go"><violation number="1" location="src/go/plugin/nagios.d/pkg/timeperiod/compile.go:277">This exclusion check recurses without detecting self or cyclic references, so periods that exclude themselves (or mutually exclude one another) will trigger infinite recursion and eventually crash the agent. Please add cycle detection or skip self references before invoking ex.Allows.</violation></file><file name="src/go/plugin/nagios.d/pkg/ids/ids.go"><violation number="1" location="src/go/plugin/nagios.d/pkg/ids/ids.go:31">Trimming all underscores allows Sanitize to return an empty ID when the input name has no ASCII letters or digits, collapsing different jobs/labels to the same key and breaking perfdata chart registration.</violation></file><file name="src/go/plugin/nagios.d/pkg/runtime/executor.go"><violation number="1" location="src/go/plugin/nagios.d/pkg/runtime/executor.go:129">Closing the results channel in Stop leaves it closed on the next Start, so workerLoop will panic when sending to e.results after a restart. Consider keeping the channel open or recreating it before relaunching workers.</violation></file><file name="src/go/plugin/nagios.d/config/nagios.d.conf"><violation number="1" location="src/go/plugin/nagios.d/config/nagios.d.conf:38">The directory discovery shard forces every discovered plugin to receive `-H $HOSTADDRESS$`, but the `include: ["check_*"]` glob pulls in utilities like `check_load` and `check_disk` that do not accept `-H`, causing them to fail immediately. Please drop the hard-coded `-H` arguments or narrow the include pattern so only plugins that need them are discovered.</violation></file><file name="src/go/cmd/nagiosdplugin/main.go"><violation number="1" location="src/go/cmd/nagiosdplugin/main.go:69">Do not log the raw HTTP_PROXY/HTTPS_PROXY values because they may include credentials; log a sanitized indicator instead.</violation></file>React with 👍 or 👎 to teach cubic. Mention@cubic-dev-ai to give feedback, ask questions, or re-run the review.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR introduces an experimental Nagios compatibility plugin that allows Netdata to run unmodified Nagios plugins. The implementation includes configuration parsing, directory discovery with filesystem watching, a macro system for Nagios-style variable substitution, an async scheduler/executor with single-flight execution semantics, comprehensive charts, optional OTLP log emission, and full build/packaging support.
Key Changes
- New
nagios.d.pluginwith YAML configuration schema and time period compiler - Directory discovery with fsnotify-based watching and include/exclude glob patterns
- Macro system supporting
$USERn$ ,$ARGn$ , and host/service macros with perfdata parsing - Async scheduler/executor with resource usage tracking (CPU, memory, disk I/O)
- Charts for job state, runtime metrics, latency, and scheduler statistics
- Optional OTLP gRPC emitter for structured logging
- Hot-reload support via new Reloadable interface in go.d manager
- ndexec enhancements: working directory support, formatted command return, resource usage counters
- CMake build flag
ENABLE_PLUGIN_NAGIOSand Debian package with postinst capability setup
Reviewed Changes
Copilot reviewed 40 out of 41 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/go/plugin/nagios.d/pkg/timeperiod/* | Time period configuration and compiler for scheduling windows |
| src/go/plugin/nagios.d/pkg/spec/* | Job configuration structures with validation and defaults |
| src/go/plugin/nagios.d/pkg/runtime/* | Core scheduler, executor, macro system, and result emitters |
| src/go/plugin/nagios.d/pkg/output/* | Nagios plugin output parser with perfdata extraction |
| src/go/plugin/nagios.d/pkg/config/* | Directory discovery with glob patterns and defaults merging |
| src/go/plugin/nagios.d/modules/nagios/* | Module implementation with hot-reload and watcher support |
| src/go/plugin/nagios.d/charts/* | Chart definitions for metrics visualization |
| src/go/plugin/go.d/pkg/ndexec/* | Enhanced execution wrapper with resource usage tracking |
| src/go/plugin/go.d/agent/* | Hot-reload interface and manager integration |
| CMakeLists.txt, packaging/* | Build configuration and Debian package setup |
Comments suppressed due to low confidence (1)
src/go/plugin/nagios.d/pkg/runtime/scheduler.go:1
- The NextAllowed function iterates minute-by-minute up to 90 days (129,600 iterations) which could be extremely slow for far-future searches. Consider implementing a more efficient algorithm that calculates the next allowed time directly from the rules rather than brute-force iteration. For example, for weekly rules, you could calculate the next matching weekday directly.
💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.
| dialOpts := []grpc.DialOption{grpc.WithBlock()} | ||
| if cfg.Insecure || endpoint == DefaultOTLPEndpoint { | ||
| dialOpts = append(dialOpts, grpc.WithTransportCredentials(insecure.NewCredentials())) | ||
| } else { | ||
| return nil, fmt.Errorf("secure TLS connection not implemented yet") | ||
| } |
CopilotAINov 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The conditioncfg.Insecure || endpoint == DefaultOTLPEndpoint automatically treats the default endpoint as insecure, which could be surprising behavior. If users explicitly setendpoint: 127.0.0.1:4317 withinsecure: false, they would expect secure TLS but get insecure connection instead. Consider removing theendpoint == DefaultOTLPEndpoint check and only rely on the explicitcfg.Insecure flag.
Uh oh!
There was an error while loading.Please reload this page.
| func replaceMacro(value string, env map[string]string) string { | ||
| replaced := value | ||
| for key, val := range env { | ||
| macro := fmt.Sprintf("$%s$", strings.TrimPrefix(key, "NAGIOS_")) | ||
| replaced = strings.ReplaceAll(replaced, macro, val) | ||
| } | ||
| return replaced | ||
| } |
CopilotAINov 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The replaceMacro function iterates through all environment variables and performs string replacements for each one, even if the value doesn't contain any macros. This is inefficient when the env map is large. Consider checking if the value contains "$" first, or using a more efficient approach like regexp with a callback for replacements.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| funcconvertMaxRSS(rawint64)int64 { | ||
| bytes:=raw | ||
| switchruntime.GOOS { | ||
| case"linux","android": | ||
| bytes*=1024 | ||
| } | ||
| returnbytes | ||
| } |
CopilotAINov 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The convertMaxRSS function assumes that macOS/BSD return RSS in bytes while Linux returns it in kilobytes, but this conversion logic lacks documentation explaining the platform-specific differences. Add a comment explaining why Linux/Android need the 1024 multiplier (rusage.Maxrss is in KB on Linux but bytes on macOS/BSD).
ktsaou commentedNov 12, 2025
@ilyam8 yours now... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
7 issues found across 60 files
Prompt for AI agents (all 7 issues)
Understand the root cause of the following 7 issues and fix them.<file name="src/go/plugin/go.d/agent/module/job.go"><violation number="1" location="src/go/plugin/go.d/agent/module/job.go:731">TypeOverride should let charts bypass the default type prefix, but the length check still uses the original FullName/ID combination, so overridden charts stay ignored. Please make the guard use getChartType/getChartID (or otherwise respect TypeOverride) before deciding to drop the chart.</violation></file><file name="src/go/plugin/go.d/agent/vnodes/vnodes.go"><violation number="1" location="src/go/plugin/go.d/agent/vnodes/vnodes.go:51">Labels may become nil in Copy(), causing JSON to emit null instead of {} (json tag lacks omitempty); initialize to empty map to preserve prior semantics.</violation></file><file name="src/go/plugin/scripts.d/pkg/ids/ids.go"><violation number="1" location="src/go/plugin/scripts.d/pkg/ids/ids.go:35">Trimming leading/trailing underscores here causes different job names that only differ by punctuation at the edges (e.g. "Disk usage" vs. "Disk usage /") to sanitize to the same identifier, so JobKey-based chart and perfdata IDs collide and merge distinct jobs. Please keep edge underscores or otherwise disambiguate sanitized IDs.</violation></file><file name="src/go/plugin/scripts.d/pkg/units/scale.go"><violation number="1" location="src/go/plugin/scripts.d/pkg/units/scale.go:73">Lowercasing the unit before calling byteMultiplier collapses Mbps (bits/sec) and MBps (bytes/sec) into the same branch, so Nagios perfdata reported in bits/s is scaled as bytes/s (8× too high). Please preserve the original letter case or otherwise distinguish bits from bytes before choosing the multiplier.</violation></file><file name="src/go/plugin/go.d/pkg/ndexec/resource_usage.go"><violation number="1" location="src/go/plugin/go.d/pkg/ndexec/resource_usage.go:49">ru_maxrss is also reported in KiB on BSD-derived platforms (FreeBSD/OpenBSD/NetBSD/DragonFly), so limiting the conversion to Linux/Android returns RSS values that are too small by 1024× on those systems. Please extend the case to include the BSD GOOS values so they are scaled to bytes as well.</violation></file><file name="src/go/plugin/scripts.d/pkg/runtime/scheduler.go"><violation number="1" location="src/go/plugin/scripts.d/pkg/runtime/scheduler.go:737">Soft-state results are discarded because js.state is forced to the hard state. As a result, metrics and macros keep reporting the previous hard state (often OK) during retries instead of the current plugin state, so soft failures are invisible until a hard failure happens.</violation></file><file name="src/go/plugin/scripts.d/pkg/output/parser.go"><violation number="1" location="src/go/plugin/scripts.d/pkg/output/parser.go:85">Using TrimSpace here strips leading indentation from the long-output payload, collapsing any intentional spacing on the first line and altering how Nagios plugins present their details. Consider only trimming trailing whitespace so indentation is preserved.</violation></file>Reply to cubic to teach it or ask questions. Re-run a review with@cubic-dev-ai review this PR
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
ktsaou commentedNov 15, 2025
@ilyam8 I have also zabbix. Bigger than nagios. I will push it here. Working on it currently. |
ba27aa2 to10b1447CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This should be issues on the repository, not a file in the repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
Copilot reviewed 90 out of 122 changed files in this pull request and generated 2 comments.
💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.
| import ( | ||
| "testing" | ||
| "github.com/netdata/netdata/go/plugins/plugin/scripts.d/pkg/spec" |
CopilotAINov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Import path contains incorrect directory segment 'plugins/plugin'; should be 'go/plugins/plugin' or verify the actual module path structure.
| "github.com/netdata/netdata/go/plugins/plugin/scripts.d/pkg/spec" | |
| "github.com/netdata/netdata/go/plugins/scripts.d/pkg/spec" |
| iferr:=m.wrapJobPayloadIfNeeded(mn,jn,&fn);err!=nil { | ||
| m.Warningf("dyncfg: %s: module %s job %s: failed to prepare payload: %v",cmd,mn,jn,err) | ||
| m.dyncfgApi.SendCodef(fn,400,"Invalid configuration payload: %v.",err) | ||
| return | ||
| } |
CopilotAINov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Duplicate call towrapJobPayloadIfNeeded at lines 218 and 224 with identical error handling; remove one of these duplicate blocks.
| iferr:=m.wrapJobPayloadIfNeeded(mn,jn,&fn);err!=nil { | |
| m.Warningf("dyncfg: %s: module %s job %s: failed to prepare payload: %v",cmd,mn,jn,err) | |
| m.dyncfgApi.SendCodef(fn,400,"Invalid configuration payload: %v.",err) | |
| return | |
| } |
Also use the correct minimum version for the conflicts line.
Uh oh!
There was an error while loading.Please reload this page.
This PR enables Netdata to run unmodified Nagios plugins
Summary by cubic
Adds an experimental scripts.d.plugin that runs stock Nagios checks inside Netdata without changes and introduces a Zabbix module with preprocessing so Zabbix-style jobs can run natively. It ships a pure-Go Zabbix preprocessing library with full compatibility and includes scheduling/execution, config schemas, charts, OTLP logs, hot-reload hooks, and build/packaging.
New Features
Migration
Written for commita384445. Summary will update automatically on new commits.