Back to Netdata

Collector consistency rule

.agents/skills/integrations-lifecycle/consistency.md

2.11.010.6 KB
Original Source

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.

    The option's group and the DynCfg tab that shows it MUST name the same thing. metadata.yaml group becomes the doc's Group column; config_schema.json uiSchema.ui:options.tabs[].title becomes the UI tab. When the two vocabularies differ, an operator reading the doc cannot find the option in the UI. Rules:

    • Every group MUST equal a tab title, or take the Tab / Subgroup form whose first segment is a tab title. Tab / Subgroup exists because tabs can only group whole top-level schema properties, so a doc group refining a nested concern (query timing, tag filters) cannot be its own tab.
    • Every tab title MUST be named by at least one group, or the doc can never point at that tab.
    • Name the section after the config key the operator types, so the doc column, the UI tab, and the YAML key read alike.
    • List tabs in the order the doc lists the groups.
    • Most collectors predate this rule and still disagree, so do NOT copy grouping from a neighbouring collector; derive it from that collector's own keys.
    • Once a collector's two artifacts agree, keep them that way by calling collecttest.AssertConfigSchemaMatchesMetadata(t, "config_schema.json", "metadata.yaml") from its tests. It checks per option that the tab listing the option's root property is the first segment of its group, and that every tab is named by some group. The call is opt-in because most collectors would fail it today; cloudwatch and azure_monitor are the worked examples.
  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. Generated integration documentation has an explicit delivery route. The author MUST run the metadata pipeline locally for source validation, then choose and document one route:

    • regenerate and commit integrations/<slug>.md in the source PR; or
    • leave committed generated pages unchanged so generate-integrations.yml opens the follow-up regeneration PR after the source reaches master.

    check-markdown.yml regenerates pages before Learn link validation but does not assert a clean Git diff. Earlier guidance incorrectly claimed that an uncommitted regeneration diff failed the source PR.

  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:

    bash
    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 for source artifacts. Metadata, schema, stock config, alerts, taxonomy, and hand-written docs move with the behavior. Only generated integration pages may use the documented automatic post-merge route.
  • "The integration page on Learn doesn't show my new option." -> Verify that metadata.yaml changed and that either the source PR committed integrations/<slug>.md or the post-merge regeneration PR completed.
  • "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.