| 1 | # Changing Go Framework Code |
| 2 | |
| 3 | Requirement language follows the root `AGENTS.md` definitions. |
| 4 | |
| 5 | This guide applies to shared Go framework code, not one collector's private |
| 6 | implementation. Framework changes have high blast radius and MUST be designed |
| 7 | before implementation. |
| 8 | |
| 9 | ## Scope |
| 10 | |
| 11 | This guide applies when changing or extending any of these areas: |
| 12 | |
| 13 | - `src/go/plugin/framework/collectorapi` |
| 14 | - `src/go/plugin/framework/jobruntime` |
| 15 | - `src/go/plugin/framework/charttpl` |
| 16 | - `src/go/plugin/framework/chartengine` |
| 17 | - `src/go/plugin/framework/chartemit` |
| 18 | - `src/go/plugin/framework/functions` |
| 19 | - `src/go/plugin/framework/vnodes` |
| 20 | - `src/go/plugin/framework/vnoderegistry` |
| 21 | - `src/go/plugin/framework/dyncfg` |
| 22 | - `src/go/plugin/framework/confgroup` |
| 23 | - `src/go/plugin/framework/runtimecomp` |
| 24 | - `src/go/plugin/go.d/pkg/collecttest` |
| 25 | - `src/go/pkg/netdataapi` |
| 26 | - `src/go/pkg/metrix` |
| 27 | - `src/go/pkg/funcapi` |
| 28 | - `src/go/pkg/topology/v1` |
| 29 | - `src/go/pkg/matcher` |
| 30 | - `src/go/pkg/stm` |
| 31 | - shared collector/runtime helpers under `src/go/pkg/` when their semantics are |
| 32 | used by go.d collectors or framework runtime code, such as `web`, |
| 33 | `prometheus`, `tlscfg`, `netdataapi`, and `netipc` |
| 34 | - shared go.d helper packages under `src/go/plugin/go.d/pkg/`, such as |
| 35 | `collecttest`, `ndexec`, `logs`, `sqlquery`, `cloudauth`, `pinger`, |
| 36 | `snmputils`, `k8sclient`, and `dockerhost` |
| 37 | |
| 38 | It also applies when a collector change requires a new shared framework |
| 39 | capability instead of collector-local code. |
| 40 | |
| 41 | ## Core Rule |
| 42 | |
| 43 | Framework changes MUST optimize for the clean end state, not the smallest local |
| 44 | diff. If a collector exposes a general framework gap, the implementation MUST |
| 45 | consider a framework change before adding collector-local glue, package-level |
| 46 | globals, duplicate helpers, or private coupling. |
| 47 | |
| 48 | Framework changes MUST NOT begin until the applicable approval tier below is |
| 49 | satisfied. |
| 50 | |
| 51 | ## Framework Vs Collector-Local |
| 52 | |
| 53 | Use this split before designing: |
| 54 | |
| 55 | - Collector-local code is appropriate when the behavior depends on one upstream |
| 56 | product, one collector's private model, or one collector's artifact set. |
| 57 | - Calling existing framework APIs from a collector is collector-local work. |
| 58 | Changing those APIs, or changing any package in the scope list above, is |
| 59 | framework work. |
| 60 | - Framework code is appropriate when the behavior affects lifecycle, |
| 61 | chart/template semantics, metric storage, host scopes, Functions, topology, |
| 62 | dynamic config, shared tests, shared matchers, or multiple collectors. |
| 63 | - The test is the generality of the behavior, not only the directory touched. |
| 64 | Collector-local globals, singletons, adapters, duplicated helpers, or package |
| 65 | glue that substitute for a missing general framework capability are framework |
| 66 | work for approval purposes. |
| 67 | - A framework extension is usually appropriate when two collectors would |
| 68 | otherwise need the same helper or workaround. |
| 69 | - A collector-local workaround MUST NOT be used only because it is less churn. |
| 70 | |
| 71 | When uncertain, pause and ask for a design decision with evidence. |
| 72 | |
| 73 | ## Approval Tiers |
| 74 | |
| 75 | Use the smallest tier that honestly fits the change. If the risk is unclear, |
| 76 | use the full design gate. |
| 77 | |
| 78 | ### Full Design Gate |
| 79 | |
| 80 | The full design gate is REQUIRED for framework changes that affect contracts, |
| 81 | runtime behavior, compatibility, lifecycle, chart output, metric storage, |
| 82 | Function protocol, topology payloads, host scopes/vnodes, dyncfg behavior, or |
| 83 | multiple collectors. |
| 84 | |
| 85 | The full design gate requires the design note below and explicit user approval |
| 86 | before implementation. |
| 87 | |
| 88 | ### Short Decision Gate |
| 89 | |
| 90 | The short decision gate is allowed only for additive, backward-compatible |
| 91 | framework changes that do not alter existing behavior or public contracts. This |
| 92 | includes narrow cases such as exposing an existing helper, adding an extension |
| 93 | interface that existing implementations do not need to satisfy, or adding a |
| 94 | test helper that preserves all existing caller semantics. |
| 95 | |
| 96 | The short decision gate MUST NOT be used to disguise a collector-local |
| 97 | workaround, avoid a full design discussion, or reduce the apparent blast radius |
| 98 | of a change that really belongs under the full design gate. Before using this |
| 99 | tier, verify that the change still serves the clean end state. If the change is |
| 100 | a hack, it MUST NOT be implemented under this tier. |
| 101 | |
| 102 | Before implementation, record the short decision in the active TODO or SOW. The |
| 103 | record MUST include: |
| 104 | |
| 105 | 1. Root cause. |
| 106 | 2. Why collector-local code is the wrong place. |
| 107 | 3. Why the change is additive and backward-compatible. |
| 108 | 4. Why this is the clean framework shape rather than a tier-reducing hack. |
| 109 | 5. Approval source: either the exact user request that already approved this |
| 110 | framework addition or the explicit approval response after presenting this |
| 111 | short-gate note. |
| 112 | 6. Affected packages and callers searched. |
| 113 | 7. Representative collectors selected for validation, or why none apply. |
| 114 | 8. Tests that will prove no existing behavior changed. |
| 115 | 9. Documentation, spec, skill, and integration-artifact update decision. |
| 116 | |
| 117 | Ask for explicit user approval when the request does not already cover the |
| 118 | decision, when compatibility is uncertain, or when another package or collector |
| 119 | needs changes to consume the new framework capability. |
| 120 | If the task began as collector work, the short gate still requires explicit |
| 121 | user approval before implementation. |
| 122 | |
| 123 | ## Required Design Note |
| 124 | |
| 125 | For full-gate changes, prepare a design note, record it in the active TODO or |
| 126 | SOW, and get user approval. The design note MUST cover: |
| 127 | |
| 128 | 1. Root cause. |
| 129 | - What exactly is broken or missing? |
| 130 | - Why is a collector-local fix insufficient? |
| 131 | 2. Clean end state. |
| 132 | - What is the intended framework shape after the work is complete? |
| 133 | - Which current compromise or workaround will be removed or avoided? |
| 134 | 3. Scope boundary. |
| 135 | - What is included in this step? |
| 136 | - What is explicitly deferred? |
| 137 | - Does any independent prerequisite need to land first? |
| 138 | 4. Affected contracts. |
| 139 | - Public interfaces, runtime behavior, chart template semantics, |
| 140 | `metrix` read/write semantics, Function protocol, host scopes/vnodes, |
| 141 | topology payloads, generated docs, tests, and collector compatibility. |
| 142 | 5. Compatibility. |
| 143 | - Is this preserving existing contracts? |
| 144 | - If not, what breaking change did the user explicitly accept? |
| 145 | 6. Existing patterns. |
| 146 | - Which framework packages or collectors already solve something similar? |
| 147 | - Which pattern is being reused? |
| 148 | 7. Implementation batches. |
| 149 | - Split into coherent commits when the work is non-trivial. |
| 150 | - Each batch SHOULD build on the previous batch and be reviewable alone. |
| 151 | 8. Validation. |
| 152 | - Framework unit tests. |
| 153 | - Representative collector tests. |
| 154 | - Docs/spec/skill updates. |
| 155 | |
| 156 | ## Scope Checkpoints |
| 157 | |
| 158 | At every coherent batch boundary, you MUST re-check scope: |
| 159 | |
| 160 | - If the branch now contains independent framework work, split it out or defer |
| 161 | it. |
| 162 | - If the collector change is complete but a framework cleanup is separate, |
| 163 | submit the collector change first and continue later. |
| 164 | - If a framework change blocks the clean end state, pause and get approval for |
| 165 | the framework change before continuing. |
| 166 | - If the current branch depends on an independent change, land that change first |
| 167 | and rebase on master before continuing. |
| 168 | |
| 169 | ## Contract Checklist |
| 170 | |
| 171 | Use this checklist when the changed package is involved. |
| 172 | |
| 173 | ### collectorapi |
| 174 | |
| 175 | - Collector interfaces MUST stay compatible unless a breaking change is |
| 176 | explicitly approved. |
| 177 | - Backward-compatible new collector capabilities MUST be expressed as extension |
| 178 | interfaces that existing collectors are not required to implement. Breaking |
| 179 | collector contract changes require explicit approval. |
| 180 | - Registration behavior MUST be covered by tests when changed. |
| 181 | |
| 182 | ### jobruntime |
| 183 | |
| 184 | - Lifecycle semantics MUST be explicit: `Init`, `Check`, `Collect`, `Cleanup`, |
| 185 | commit, abort, cancellation, retry, and runtime metrics. |
| 186 | - Cancellation behavior MUST be tested when changed. |
| 187 | - V1 and V2 behavior MUST be considered separately. |
| 188 | |
| 189 | ### metrix |
| 190 | |
| 191 | - Read/write semantics MUST be documented and tested: snapshot vs stateful, |
| 192 | gauges, counters, StateSet, labels, host scopes, flattening, and cycle abort. |
| 193 | - The `BeginCycle`, `CommitCycleSuccess`, and `AbortCycle` contract MUST be |
| 194 | preserved and tested when changed. |
| 195 | - Identity and label behavior MUST be stable. |
| 196 | - New instrument behavior MUST include tests for both typed and flattened |
| 197 | readers when applicable. |
| 198 | |
| 199 | ### charttpl and chartengine |
| 200 | |
| 201 | - Template schema changes MUST update docs, validation, and compile tests. |
| 202 | - Runtime chart behavior MUST be covered by planner/engine tests. |
| 203 | - Per-host-scope planning MUST keep chart coverage, lifecycle, and labels |
| 204 | isolated per scope. |
| 205 | - Generated chart IDs, contexts, dimensions, labels, and lifecycle behavior are |
| 206 | public contracts and MUST be treated as stable unless a breaking change is |
| 207 | approved. |
| 208 | |
| 209 | ### chartemit |
| 210 | |
| 211 | - Emitted chart and host commands MUST remain compatible with the plugin |
| 212 | protocol. |
| 213 | - Host/vnode identity changes MUST be tested against invalid and edge-case |
| 214 | host information. |
| 215 | |
| 216 | ### host scopes and vnodes |
| 217 | |
| 218 | See `.agents/sow/specs/go-v2-host-scope.md`. |
| 219 | |
| 220 | - Scope identity MUST use deterministic stable IDs. |
| 221 | - Framework changes MUST preserve collector-provided `_vnode_type` labels. The |
| 222 | framework does not synthesize this label for collectors. |
| 223 | - Cardinality MUST be bounded and documented. |
| 224 | - Representative scoped and unscoped collectors MUST be checked when read/write |
| 225 | behavior changes. |
| 226 | |
| 227 | ### Functions |
| 228 | |
| 229 | - Function protocol changes MUST stay compatible with |
| 230 | `src/plugins.d/FUNCTION_UI_SCHEMA.json` unless a breaking change is approved. |
| 231 | - Function handlers MUST remain isolated from collector internals through narrow |
| 232 | dependencies. |
| 233 | - Manager, scheduler, cancellation, and cleanup behavior MUST be tested when |
| 234 | touched. |
| 235 | |
| 236 | ### topology |
| 237 | |
| 238 | See `.agents/skills/project-create-topology/SKILL.md` and |
| 239 | `.agents/sow/specs/topology-function-schema.md`. |
| 240 | |
| 241 | - New topology producers MUST use `src/go/pkg/topology/v1`. |
| 242 | - Payload changes MUST validate against |
| 243 | `src/plugins.d/FUNCTION_TOPOLOGY_SCHEMA.json`. |
| 244 | - Actor/link identity and table merge behavior MUST be treated as public |
| 245 | contracts. |
| 246 | |
| 247 | ### matcher |
| 248 | |
| 249 | - Existing matcher grammar and edge-case behavior MUST stay compatible unless a |
| 250 | breaking change is approved. |
| 251 | - New selector behavior SHOULD use matcher package APIs instead of adding |
| 252 | collector-local grammar. |
| 253 | |
| 254 | ### collecttest |
| 255 | |
| 256 | - Test-helper changes MUST remain compatible with every current caller and MUST |
| 257 | be validated with compile coverage plus representative callers. |
| 258 | - Shared assertions MUST NOT hide per-scope, per-label, or per-chart failures by |
| 259 | over-aggregating results. |
| 260 | |
| 261 | ## Representative Collector Checks |
| 262 | |
| 263 | When a framework change affects collectors, choose representative collectors |
| 264 | from the affected surface. Broad tests MUST NOT be used blindly as a substitute |
| 265 | for choosing the right representatives. |
| 266 | |
| 267 | Common representatives: |
| 268 | |
| 269 | - V2 metrics and chart templates: `cato_networks`, `azure_monitor`, `powerstore`, |
| 270 | `powervault`, `ping`. |
| 271 | - V2 Functions: `mysql`, `cato_networks`. |
| 272 | - Host scopes/vnodes: `azure_monitor`, `cato_networks`. |
| 273 | - Topology Functions: `cato_networks`, `snmp_topology` when the legacy topology |
| 274 | path is affected. |
| 275 | - Legacy V1 compatibility: pick a directly affected V1 collector and one simple |
| 276 | V1 collector such as `apache` when changing shared V1/V2 runtime code. |
| 277 | |
| 278 | The exact list SHOULD be justified in the design note. |
| 279 | |
| 280 | ## Validation |
| 281 | |
| 282 | Validation MUST match the changed contract. |
| 283 | |
| 284 | Examples: |
| 285 | |
| 286 | - Framework package tests: |
| 287 | - `go test -count=1 ./plugin/framework/...` |
| 288 | - `go test -count=1 ./pkg/metrix/...` |
| 289 | - `go test -count=1 ./pkg/matcher/...` |
| 290 | - `go test -count=1 ./pkg/topology/...` |
| 291 | - `go test -count=1 ./plugin/go.d/pkg/collecttest` |
| 292 | - `go test -count=1 ./plugin/go.d/pkg/...` when shared go.d helper semantics |
| 293 | change. |
| 294 | - Collector representatives: |
| 295 | - `go test -count=1 ./plugin/go.d/collector/<name>/...` |
| 296 | - `go test -race -count=1 ./plugin/go.d/collector/<name>/...` when |
| 297 | concurrency, Functions, host scopes, or topology are involved. |
| 298 | - HTTP/web helper changes: include at least one HTTP collector and its config |
| 299 | serialization tests. |
| 300 | - Matcher changes: include selector-using collectors. |
| 301 | - `collecttest` changes: include several representative V2 collectors that use |
| 302 | chart coverage, config serialization, and host scopes where relevant. |
| 303 | - Runtime components: |
| 304 | - `go test -count=1 ./plugin/framework/jobruntime ./plugin/framework/runtimecomp` |
| 305 | - Runtime wiring and dyncfg lifecycle: |
| 306 | - `go test -race -count=1 ./plugin/agent/jobmgr/...` |
| 307 | - REQUIRED when changing `collectorapi`, `jobruntime`, `dyncfg`, |
| 308 | `confgroup`, `vnoderegistry`, or runtime wiring behavior. |
| 309 | - Representative files include `manager_v2_test.go`, `job_factory_test.go`, |
| 310 | `sim_test.go`, `dyncfg_collector_test.go`, and `dyncfg_vnode_test.go`. |
| 311 | - Function/topology payloads: |
| 312 | - schema validation tests in the affected collector or package. |
| 313 | |
| 314 | Record exactly what ran. Full validation MUST NOT be claimed from a narrow |
| 315 | command. |
| 316 | |
| 317 | ## Artifact Updates |
| 318 | |
| 319 | Framework changes often require durable artifact updates. Check each class: |
| 320 | |
| 321 | - `AGENTS.md` and `src/go/AGENTS.md` |
| 322 | - project skills under `.agents/skills/` |
| 323 | - framework package READMEs |
| 324 | - specs under `.agents/sow/specs/` |
| 325 | - collector authoring docs under `src/go/plugin/go.d/docs/` |
| 326 | - integrations-lifecycle skill and artifacts if collector metadata/taxonomy |
| 327 | changes |
| 328 | - public Function/topology schemas and guides if protocol behavior changes |
| 329 | |
| 330 | If no artifact update is needed, record why in the active TODO/SOW. |
| 331 | |
| 332 | If the framework work was discovered while writing a collector, return to |
| 333 | `src/go/plugin/go.d/docs/how-to-write-a-collector.md` after the framework |
| 334 | decision or change is complete. |
| 335 | |
| 336 | ## Anti-Patterns |
| 337 | |
| 338 | - Framework behavior hidden behind a collector-local workaround. |
| 339 | - Package-level globals used to share state between framework packages. |
| 340 | - Shared test helpers that pass by aggregating away the failing dimension. |
| 341 | - Public interface changes without a compatibility decision. |
| 342 | - Runtime behavior changes without representative collector tests. |
| 343 | - New topology work using legacy topology payloads. |
| 344 | - Config knobs added to avoid designing the framework behavior. |
| 345 | - Continuing implementation after discovering independent scope that SHOULD be |
| 346 | landed separately. |