master
md 8.5 KB

Collector consistency rule

<repo>/AGENTS.md declares ("Collector Consistency Requirements") that any change touching a collector MUST land in one PR with matching changes to all relevant collector artifacts:

  1. The code -- the collector implementation files.
  2. metadata.yaml -- the integration page driver.
  3. taxonomy.yaml -- dashboard table-of-contents placement for the collector's chart contexts.
  4. config_schema.json -- the dashboard's DYNCFG editor.
  5. The stock .conf -- what /etc/netdata/<plugin>/... ships.
  6. health.d/*.conf -- alert definitions for the collector's metrics.
  7. README.md -- comprehensive end-user documentation (often a symlink into the generated integrations/<slug>.md; see artifacts-and-banners.md).

The old "5-file" shorthand is stale. Treat the list above as the durable review checklist; a given PR may legitimately not touch every file, but it must explain why an affected artifact does not need a matching edit.

Because most cross-artifact checks are not enforced by CI, collector PR descriptions MUST enumerate the relevant consistency artifacts and justify every artifact that did not need a matching change. Any SHOULD-level exception or escape hatch used by the implementation MUST be visible in the PR description or design note, not only in a code comment.

The rule covers obvious cases (units change in code -> update metadata.yaml; new config option -> update schema, stock conf, and docs; new metric -> update metadata.yaml, taxonomy.yaml, and README.md) and subtle ones (renaming a metric label affects the alert definition that refers to it; changing a default affects the stock conf example and the documented default value).

What enforces this rule today

Nothing automated, in most cases. Specifically:

  • gen_integrations.py validates each metadata.yaml against its JSON Schema only. It does NOT cross-check against config_schema.json, the stock .conf, or health.d/*.conf.
  • gen_taxonomy.py validates committed collector taxonomy.yaml files, cross-references literal owned contexts and widget references against metadata.yaml, requires declared dynamic selectors, and emits the gitignored integrations/taxonomy.json artifact.
  • check_collector_taxonomy.py is wired into check-markdown.yml for pull requests. It fails when a PR touches a collector taxonomy.yaml, adds/removes it, or edits a metadata.yaml metrics block without a matching taxonomy.yaml. Non-metrics-only metadata.yaml edits such as setup prose, overview text, categories, and troubleshooting do not trigger touched-collector taxonomy coverage by themselves, although the global taxonomy validation still runs.
  • integrations/check_collector_metadata.py is broken (see gotchas.md for details). Its imports refer to symbols that no longer exist in gen_integrations.py. ImportError on first run. NOT invoked from any workflow.
  • No CI workflow runs a "verify metric names in metadata.yaml exist in the collector code" check.
  • No CI workflow runs a "verify health.d/*.conf alert metric names exist in the collector" check.
  • check-markdown.yml only validates that generated markdown links resolve through Learn ingest -- not that metadata.yaml is in sync with config/schema/stock-conf/README.

The one exception is ibm.d modules: their metadata.yaml, README.md, and config_schema.json are GENERATED by the ibm.d docgen tool from contexts.yaml + config.go + module.yaml, so those three files are consistent by construction. ibm.d does NOT generate the stock .conf or health.d/<...>.conf; those still need manual sync. See ibm-d.md.

What reviewers should check

When reviewing a PR that touches a collector, verify:

  1. Code changes have matching metadata.yaml changes. If the diff adds a chart, dimension, label, or unit change in the code, the corresponding entry must appear in metadata.yaml. If a metric is renamed, both files must change.

  2. Chart-context changes have matching taxonomy.yaml changes. Structural items: entries that own contexts and widget contexts: references must name real contexts in the collector's metadata.yaml. Dynamic contexts must use type: selector or selector objects with context_prefix: or collect_plugin: and the corresponding metrics.dynamic_context_prefixes: or metrics.dynamic_collect_plugins: declaration.

  3. Config changes propagate to all four config-related files.

    • The Go struct field (in config.go).
    • config_schema.json -- the field appears with the correct type, default, validation.
    • The stock .conf -- a representative example shows the option.
    • metadata.yaml -- the option appears under setup.configuration.options.list.
  4. Alert changes have matching metadata.yaml.alerts entries. If health.d/<plugin>.conf adds, removes, or renames an alert, metadata.yaml.modules.<m>.alerts[] must reflect the change.

  5. README.md handling. If the plugin directory has a single integration, the README is a symlink to the generated integrations/<slug>.md -- the symlink target already updates when metadata.yaml updates. If the plugin directory has multiple integrations, the README is hand-written and must be updated by the author. agent_notification is a special case: the README itself is the generated artifact (no integrations/ subdir).

  6. integrations/<slug>.md regenerated. The author should have run the pipeline locally and committed the updated .md file. check-markdown.yml will re-run the pipeline in CI; if the author's commit and CI's regen diverge, the PR fails.

  7. Umbrella pages. If the diff added or removed a collector, src/collectors/COLLECTORS.md should reflect it. Same for SECRETS.md (secretstore changes) and SERVICE-DISCOVERY.md (service-discovery changes -- but note this one is NOT in CI; manual regen required).

  8. Generated artifacts are outputs, not source. Files with DO NOT EDIT THIS FILE DIRECTLY or <!--startmeta ... message: "DO NOT EDIT..." --> banners must be regenerated from their source artifacts. Do not hand-edit generated files to fix prose, links, setup text, or metric descriptions.

  9. Gitignored generated catalogs are absent from the PR. Before opening the PR, run:

   git status --porcelain |
     rg '^(\?\?|!!| M|M |A |AM) integrations/(integrations\.(js|json)|taxonomy\.json)$' || true

The command MUST print no output. If it prints integrations/integrations.js, integrations/integrations.json, or integrations/taxonomy.json, remove the local generated artifact from the commit/worktree state rather than committing it.

Why the policy is unenforced

Investigation of the repo found that integrations/check_collector_metadata.py was apparently the first stab at automation; it appears to have bit-rotted without a CI hook to catch the rot. A proper enforcement pipeline would:

  • repair / rewrite check_collector_metadata.py (or replace with a unit test under tests/);
  • wire it into generate-integrations.yml as a pre-flight validator;
  • add a metric-name cross-reference check between metadata.yaml.alerts[].metric and the alert configurations under health.d/;
  • add a config-name cross-reference check between metadata.yaml.setup.configuration.options.list[].name and config_schema.json properties.

Tracked as a followup SOW after integrations-lifecycle ships. For now, the consistency rule is a review-time policy.

Anti-patterns to flag in review

  • "I only changed the code; the docs can be a follow-up PR." -> No. Affected collector consistency artifacts move in one PR.
  • "The integration page on Learn doesn't show my new option." -> Author forgot to update metadata.yaml AND regenerate integrations/<slug>.md.
  • "I edited integrations/<slug>.md directly to fix a description." -> No. That file is generated. Edit metadata.yaml and regenerate.
  • "I edited metadata.yaml for an ibm.d module." -> No. Edit contexts.yaml, config.go, or module.yaml and run go generate.
  • "I changed a default in the stock .conf only." -> Update config_schema.json default, metadata.yaml.setup.configuration.options.list[].default_value, and the README in lockstep.
  • "I added a chart context but skipped taxonomy.yaml because the dashboard will discover it." -> No. Add the context to a placement or use a declared dynamic selector.