| 1 | # Collector consistency rule |
| 2 | |
| 3 | `<repo>/AGENTS.md` declares ("Collector Consistency |
| 4 | Requirements") that any change touching a collector MUST land |
| 5 | in one PR with matching changes to all relevant collector artifacts: |
| 6 | |
| 7 | 1. **The code** -- the collector implementation files. |
| 8 | 2. **`metadata.yaml`** -- the integration page driver. |
| 9 | 3. **`taxonomy.yaml`** -- dashboard table-of-contents placement |
| 10 | for the collector's chart contexts. |
| 11 | 4. **`config_schema.json`** -- the dashboard's DYNCFG editor. |
| 12 | 5. **The stock `.conf`** -- what `/etc/netdata/<plugin>/...` |
| 13 | ships. |
| 14 | 6. **`health.d/*.conf`** -- alert definitions for the |
| 15 | collector's metrics. |
| 16 | 7. **`README.md`** -- comprehensive end-user documentation |
| 17 | (often a symlink into the generated |
| 18 | `integrations/<slug>.md`; see `artifacts-and-banners.md`). |
| 19 | |
| 20 | The old "5-file" shorthand is stale. Treat the list above as |
| 21 | the durable review checklist; a given PR may legitimately not |
| 22 | touch every file, but it must explain why an affected artifact |
| 23 | does not need a matching edit. |
| 24 | |
| 25 | Because most cross-artifact checks are not enforced by CI, collector PR |
| 26 | descriptions MUST enumerate the relevant consistency artifacts and justify every |
| 27 | artifact that did not need a matching change. Any SHOULD-level exception or |
| 28 | escape hatch used by the implementation MUST be visible in the PR description |
| 29 | or design note, not only in a code comment. |
| 30 | |
| 31 | The rule covers obvious cases (units change in code -> update |
| 32 | metadata.yaml; new config option -> update schema, stock conf, |
| 33 | and docs; new metric -> update metadata.yaml, taxonomy.yaml, |
| 34 | and README.md) |
| 35 | and subtle ones (renaming a metric label affects the alert |
| 36 | definition that refers to it; changing a default affects the |
| 37 | stock conf example and the documented default value). |
| 38 | |
| 39 | ## What enforces this rule today |
| 40 | |
| 41 | **Nothing automated, in most cases.** Specifically: |
| 42 | |
| 43 | - **`gen_integrations.py`** validates each `metadata.yaml` |
| 44 | against its JSON Schema only. It does NOT cross-check |
| 45 | against `config_schema.json`, the stock `.conf`, or |
| 46 | `health.d/*.conf`. |
| 47 | - **`gen_taxonomy.py`** validates committed collector |
| 48 | `taxonomy.yaml` files, cross-references literal owned contexts |
| 49 | and widget references against `metadata.yaml`, requires declared |
| 50 | dynamic selectors, and emits the gitignored |
| 51 | `integrations/taxonomy.json` artifact. |
| 52 | - **`check_collector_taxonomy.py`** is wired into |
| 53 | `check-markdown.yml` for pull requests. It fails when a PR |
| 54 | touches a collector `taxonomy.yaml`, adds/removes it, or edits |
| 55 | a `metadata.yaml` metrics block without a matching |
| 56 | `taxonomy.yaml`. Non-metrics-only `metadata.yaml` edits such as |
| 57 | setup prose, overview text, categories, and troubleshooting do not |
| 58 | trigger touched-collector taxonomy coverage by themselves, although |
| 59 | the global taxonomy validation still runs. |
| 60 | - **`integrations/check_collector_metadata.py`** is broken |
| 61 | (see `gotchas.md` for details). Its imports refer to symbols that no longer |
| 62 | exist in `gen_integrations.py`. ImportError on first run. NOT invoked from any |
| 63 | workflow. |
| 64 | - **No CI workflow** runs a "verify metric names in |
| 65 | metadata.yaml exist in the collector code" check. |
| 66 | - **No CI workflow** runs a "verify `health.d/*.conf` alert |
| 67 | metric names exist in the collector" check. |
| 68 | - **`check-markdown.yml`** only validates that generated |
| 69 | markdown links resolve through Learn ingest -- not that |
| 70 | metadata.yaml is in sync with config/schema/stock-conf/README. |
| 71 | |
| 72 | The one exception is **ibm.d modules**: their `metadata.yaml`, |
| 73 | `README.md`, and `config_schema.json` are GENERATED by the |
| 74 | ibm.d `docgen` tool from `contexts.yaml` + `config.go` + |
| 75 | `module.yaml`, so those three files are consistent by |
| 76 | construction. ibm.d does NOT generate the stock `.conf` or |
| 77 | `health.d/<...>.conf`; those still need manual sync. See |
| 78 | `ibm-d.md`. |
| 79 | |
| 80 | ## What reviewers should check |
| 81 | |
| 82 | When reviewing a PR that touches a collector, verify: |
| 83 | |
| 84 | 1. **Code changes have matching `metadata.yaml` changes.** If |
| 85 | the diff adds a chart, dimension, label, or unit change in |
| 86 | the code, the corresponding entry must appear in |
| 87 | `metadata.yaml`. If a metric is renamed, both files must |
| 88 | change. |
| 89 | |
| 90 | 2. **Chart-context changes have matching `taxonomy.yaml` |
| 91 | changes.** Structural `items:` entries that own contexts and |
| 92 | widget `contexts:` references must name real contexts in the |
| 93 | collector's `metadata.yaml`. Dynamic contexts must use |
| 94 | `type: selector` or selector objects with `context_prefix:` or |
| 95 | `collect_plugin:` and the corresponding |
| 96 | `metrics.dynamic_context_prefixes:` or |
| 97 | `metrics.dynamic_collect_plugins:` declaration. |
| 98 | |
| 99 | 3. **Config changes propagate to all four config-related |
| 100 | files.** |
| 101 | - The Go struct field (in `config.go`). |
| 102 | - `config_schema.json` -- the field appears with the |
| 103 | correct type, default, validation. |
| 104 | - The stock `.conf` -- a representative example shows the |
| 105 | option. |
| 106 | - `metadata.yaml` -- the option appears under |
| 107 | `setup.configuration.options.list`. |
| 108 | |
| 109 | 4. **Alert changes have matching `metadata.yaml.alerts` |
| 110 | entries.** If `health.d/<plugin>.conf` adds, removes, or |
| 111 | renames an alert, `metadata.yaml.modules.<m>.alerts[]` must |
| 112 | reflect the change. |
| 113 | |
| 114 | 5. **README.md handling.** If the plugin directory has a |
| 115 | single integration, the README is a symlink to the |
| 116 | generated `integrations/<slug>.md` -- the symlink target |
| 117 | already updates when `metadata.yaml` updates. If the |
| 118 | plugin directory has multiple integrations, the README is |
| 119 | hand-written and must be updated by the author. |
| 120 | `agent_notification` is a special case: the README itself |
| 121 | is the generated artifact (no `integrations/` subdir). |
| 122 | |
| 123 | 6. **`integrations/<slug>.md` regenerated.** The author |
| 124 | should have run the pipeline locally and committed the |
| 125 | updated `.md` file. `check-markdown.yml` will re-run the |
| 126 | pipeline in CI; if the author's commit and CI's regen |
| 127 | diverge, the PR fails. |
| 128 | |
| 129 | 7. **Umbrella pages.** If the diff added or removed a |
| 130 | collector, `src/collectors/COLLECTORS.md` should reflect |
| 131 | it. Same for `SECRETS.md` (secretstore changes) and |
| 132 | `SERVICE-DISCOVERY.md` (service-discovery changes -- but |
| 133 | note this one is NOT in CI; manual regen required). |
| 134 | |
| 135 | 8. **Generated artifacts are outputs, not source.** Files with |
| 136 | `DO NOT EDIT THIS FILE DIRECTLY` or `<!--startmeta ... message: |
| 137 | "DO NOT EDIT..." -->` banners must be regenerated from their source |
| 138 | artifacts. Do not hand-edit generated files to fix prose, links, setup text, |
| 139 | or metric descriptions. |
| 140 | |
| 141 | 9. **Gitignored generated catalogs are absent from the PR.** Before opening the |
| 142 | PR, run: |
| 143 | |
| 144 | ```bash |
| 145 | git status --porcelain | |
| 146 | rg '^(\?\?|!!| M|M |A |AM) integrations/(integrations\.(js|json)|taxonomy\.json)$' || true |
| 147 | ``` |
| 148 | |
| 149 | The command MUST print no output. If it prints |
| 150 | `integrations/integrations.js`, `integrations/integrations.json`, or |
| 151 | `integrations/taxonomy.json`, remove the local generated artifact from the |
| 152 | commit/worktree state rather than committing it. |
| 153 | |
| 154 | ## Why the policy is unenforced |
| 155 | |
| 156 | Investigation of the repo found that |
| 157 | `integrations/check_collector_metadata.py` was apparently the |
| 158 | first stab at automation; it appears to have bit-rotted |
| 159 | without a CI hook to catch the rot. A proper enforcement |
| 160 | pipeline would: |
| 161 | |
| 162 | - repair / rewrite `check_collector_metadata.py` (or replace |
| 163 | with a unit test under `tests/`); |
| 164 | - wire it into `generate-integrations.yml` as a pre-flight |
| 165 | validator; |
| 166 | - add a metric-name cross-reference check between |
| 167 | `metadata.yaml.alerts[].metric` and the alert |
| 168 | configurations under `health.d/`; |
| 169 | - add a config-name cross-reference check between |
| 170 | `metadata.yaml.setup.configuration.options.list[].name` |
| 171 | and `config_schema.json` properties. |
| 172 | |
| 173 | Tracked as a followup SOW after `integrations-lifecycle` ships. |
| 174 | For now, the consistency rule is a review-time policy. |
| 175 | |
| 176 | ## Anti-patterns to flag in review |
| 177 | |
| 178 | - "I only changed the code; the docs can be a follow-up PR." |
| 179 | -> No. Affected collector consistency artifacts move in one PR. |
| 180 | - "The integration page on Learn doesn't show my new option." |
| 181 | -> Author forgot to update `metadata.yaml` AND regenerate |
| 182 | `integrations/<slug>.md`. |
| 183 | - "I edited `integrations/<slug>.md` directly to fix a |
| 184 | description." -> No. That file is generated. Edit |
| 185 | `metadata.yaml` and regenerate. |
| 186 | - "I edited `metadata.yaml` for an ibm.d module." -> No. Edit |
| 187 | `contexts.yaml`, `config.go`, or `module.yaml` and run |
| 188 | `go generate`. |
| 189 | - "I changed a default in the stock `.conf` only." -> Update |
| 190 | `config_schema.json` `default`, `metadata.yaml.setup.configuration.options.list[].default_value`, |
| 191 | and the README in lockstep. |
| 192 | - "I added a chart context but skipped `taxonomy.yaml` because |
| 193 | the dashboard will discover it." -> No. Add the context to a |
| 194 | placement or use a declared dynamic selector. |