| 1 | # Migrating go.d Collectors From V1 To V2 |
| 2 | |
| 3 | Requirement language follows the root `AGENTS.md` definitions. |
| 4 | |
| 5 | This guide is for migrating an existing go.d collector from framework V1 to |
| 6 | framework V2. It is not the starting point for a new collector; use |
| 7 | `src/go/plugin/go.d/docs/how-to-write-a-collector.md` for new work. |
| 8 | |
| 9 | V1 collectors are public integrations. A migration MUST preserve their existing |
| 10 | user-visible contracts unless the user explicitly approves a breaking change. |
| 11 | |
| 12 | ## Core Rule |
| 13 | |
| 14 | A V1-to-V2 migration is compatibility work first. The clean end state is a V2 |
| 15 | collector that behaves like the old collector from the user's point of view, |
| 16 | with the old public contracts preserved and the internal collection path moved |
| 17 | to `collectorapi.CollectorV2` and `metrix.CollectorStore`. |
| 18 | |
| 19 | Do not combine a compatibility migration with new enrichment, new topology, |
| 20 | new host scopes, config expansion, chart redesign, or framework changes unless |
| 21 | that work is required for the migration itself. If the migration reveals useful |
| 22 | new work, split it into a later batch. |
| 23 | |
| 24 | ## Before Writing Code |
| 25 | |
| 26 | Create a compatibility manifest before implementation. The manifest can live in |
| 27 | the active TODO or SOW. It MUST cover: |
| 28 | |
| 29 | 1. Module identity. |
| 30 | - collector directory; |
| 31 | - module name; |
| 32 | - chart-template `context_namespace` and any group context namespaces; |
| 33 | - `go.d.conf` toggle; |
| 34 | - stock job config path; |
| 35 | - service-discovery rules, if any, including |
| 36 | `src/go/plugin/go.d/config/go.d/sd/` and |
| 37 | `src/go/plugin/go.d/discovery/sdext/` references. |
| 38 | 2. Registration and lifecycle. |
| 39 | - current `collectorapi.Register` entry; |
| 40 | - `Defaults`; |
| 41 | - `Init`, `Check`, `Collect`, and `Cleanup` behavior; |
| 42 | - any `Once`, reconnect, cache, or retry behavior. |
| 43 | 3. Config contract. |
| 44 | - YAML and JSON keys; |
| 45 | - defaults; |
| 46 | - validation; |
| 47 | - `config_schema.json`; |
| 48 | - stock `.conf`; |
| 49 | - DYNCFG behavior. |
| 50 | 4. Metric/chart contract. |
| 51 | - chart IDs; |
| 52 | - contexts; |
| 53 | - dimension IDs and names; |
| 54 | - algorithms; |
| 55 | - units; |
| 56 | - title, family, type, priority, multiplier, divisor, hidden, and float |
| 57 | flags; |
| 58 | - labels; |
| 59 | - chart variables (`Vars`) -- STOP if present; see `Chart Variables` |
| 60 | before implementation; |
| 61 | - dynamic chart/instance generation rules; |
| 62 | - chart lifecycle and obsoletion timing. |
| 63 | 5. Integration artifacts. |
| 64 | - `metadata.yaml`; |
| 65 | - `taxonomy.yaml`; |
| 66 | - health alerts; |
| 67 | - generated integration page and README symlink; |
| 68 | - `COLLECTORS.md` / plugin README entries when affected; |
| 69 | - service-discovery docs such as `SERVICE-DISCOVERY.md`, when affected; |
| 70 | - secrets docs such as `SECRETS.md`, when affected. |
| 71 | 6. Tests and fixtures. |
| 72 | - existing tests to preserve; |
| 73 | - missing contract tests to add before or during migration; |
| 74 | - real fixture coverage. |
| 75 | |
| 76 | ## What Must Change |
| 77 | |
| 78 | A V2 migration MUST replace the V1 collection path: |
| 79 | |
| 80 | - registration uses `CreateV2`; |
| 81 | - the collector implements `collectorapi.CollectorV2`; |
| 82 | - `New()` creates `metrix.NewCollectorStore()`; |
| 83 | - the collector stores `metrix.CollectorStore`; |
| 84 | - `Configuration()` preserves existing config return behavior; |
| 85 | - `VirtualNode()` preserves existing vnode behavior when the V1 collector has |
| 86 | one; |
| 87 | - `MetricStore()` returns the store; |
| 88 | - `ChartTemplateYAML()` returns embedded `charts.yaml`; |
| 89 | - if the compatibility manifest contains V1 chart `Vars`, the migration stops |
| 90 | until the `Chart Variables` decision path is resolved; |
| 91 | - `Collect(ctx)` returns `error` and writes observations to `metrix`; |
| 92 | - the completed migration removes the V1 `Collect() map[string]int64` output |
| 93 | path and any runtime bridge from V1 maps into V2 `metrix`. |
| 94 | - the completed migration removes `Charts()` and runtime `collectorapi.Charts` |
| 95 | mutation from the production collection path. |
| 96 | |
| 97 | Use `src/go/plugin/framework/collectorapi/collector.go` as the source of truth |
| 98 | for the interface. |
| 99 | |
| 100 | ## Parity During Development |
| 101 | |
| 102 | Temporary V1 logic can be useful while developing the migration. For example, |
| 103 | tests can compare V1 map output against V2 `metrix` observations after mapping |
| 104 | both sides through the V1 chart manifest and any V2 chart-identity tooling that |
| 105 | exists for the migration. |
| 106 | |
| 107 | Do not invent an unreviewed local snapshot format when the comparison needs |
| 108 | compiled chart identity that the framework does not expose. If the migration |
| 109 | needs reusable chart-identity or alert-variable comparison helpers, treat that |
| 110 | as a framework/test-helper prerequisite and follow |
| 111 | `src/go/plugin/framework/docs/changing-framework-code.md` before relying on it. |
| 112 | |
| 113 | That parity bridge is a development tool only. It MUST NOT remain in the final |
| 114 | migrated collector runtime path. A finished migration that runs as |
| 115 | V1-to-bridge-to-V2 is not a clean end state. |
| 116 | |
| 117 | Runtime path means any code reachable from `Init(ctx)`, `Check(ctx)`, |
| 118 | `Collect(ctx)`, or `Cleanup(ctx)` during normal execution. V1 collection logic |
| 119 | MUST NOT remain reachable from that runtime path, regardless of function names |
| 120 | or return types. |
| 121 | |
| 122 | If parity helpers are useful long term, keep only `_test.go` helpers or |
| 123 | fixtures under `testdata/`. Before finishing, audit imports and prove no |
| 124 | non-test file imports the old V1 path or parity bridge. |
| 125 | |
| 126 | From `src/go`, run an import audit for the migrated collector: |
| 127 | |
| 128 | ```bash |
| 129 | go list -deps -test=false ./plugin/go.d/collector/<collector>/... | |
| 130 | rg 'pkg/stm|plugin/go\.d/pkg/oldmetrix' |
| 131 | rg -n 'Collect\(.*map\[string\]int64|map\[string\]int64|collectorapi\.Charts|func .*Charts\(' \ |
| 132 | plugin/go.d/collector/<collector> -g '*.go' |
| 133 | ``` |
| 134 | |
| 135 | The dependency command MUST return no runtime dependencies on old V1-only |
| 136 | helpers such as `stm`, `oldmetrix`, or any collector-local parity bridge |
| 137 | package. The source grep MUST NOT find a remaining runtime V1 map output, |
| 138 | runtime `collectorapi.Charts` mutation, or `Charts()` path. If the old path is |
| 139 | hard to identify, delete the temporary bridge and build/test the collector; the |
| 140 | final runtime must still compile without it. |
| 141 | |
| 142 | ## What Must Stay Stable |
| 143 | |
| 144 | Unless the user approves a breaking change, the migration MUST preserve: |
| 145 | |
| 146 | - module name and job identity; |
| 147 | - config field names and defaults; |
| 148 | - chart contexts; |
| 149 | - chart IDs; |
| 150 | - dimension IDs and names; |
| 151 | - dimension algorithms; |
| 152 | - chart title, family, type, priority, units, multiplier, divisor, hidden, and |
| 153 | float flags; |
| 154 | - chart variable semantics used by health alerts; |
| 155 | - health alert lookups; |
| 156 | - metadata metric descriptions and units; |
| 157 | - source metadata content that drives generated integration docs; |
| 158 | - taxonomy coverage and CI behavior; |
| 159 | - service-discovery behavior; |
| 160 | - vnode behavior; |
| 161 | - user-facing lifecycle behavior. |
| 162 | |
| 163 | If an existing collector has an accidental bug or inconsistent artifact, record |
| 164 | it separately. Fix it in the migration only when preserving the bug would make |
| 165 | the V2 collector incorrect or untestable; otherwise split the fix into its own |
| 166 | tracked batch in the active TODO/SOW with owner-approved disposition. Do not |
| 167 | close a migration with vague deferred items. |
| 168 | |
| 169 | ## Implementation Shape |
| 170 | |
| 171 | Prefer the same file ownership as new V2 collectors: |
| 172 | |
| 173 | ```text |
| 174 | collector.go # registration, New, public lifecycle, MetricStore, ChartTemplateYAML |
| 175 | init.go # Init helper methods when setup is non-trivial |
| 176 | config.go # Config, defaults, validation |
| 177 | collect.go # Collect orchestration |
| 178 | collect_<area>.go # separate upstream operations when there are several |
| 179 | metrix.go # typed instruments built once |
| 180 | write_metrics.go # observations into metrix |
| 181 | charts.yaml # V2 chart template |
| 182 | *_test.go # focused, table-driven tests |
| 183 | ``` |
| 184 | |
| 185 | Keep public lifecycle methods in `collector.go`. Helper methods can move into |
| 186 | focused files when that makes ownership clearer. |
| 187 | |
| 188 | ## Lifecycle Rules |
| 189 | |
| 190 | - `Init(ctx)` MUST perform setup and validation only. It MUST NOT collect the |
| 191 | full metric set just to initialize state. |
| 192 | - `Check(ctx)` SHOULD be a cheap probe when the upstream API supports one. If |
| 193 | V1 used full collection for autodetection, preserve the user-visible result |
| 194 | while making the V2 path as light as the source allows. |
| 195 | - `Collect(ctx)` MUST write observations to `metrix` and return an error only |
| 196 | when the cycle should abort. Fail-soft partial collection must be deliberate, |
| 197 | tested, and logged without per-cycle spam. |
| 198 | - `Cleanup(ctx)` MUST preserve existing cleanup behavior and release any V2 |
| 199 | Function or client resources added by the migration. |
| 200 | |
| 201 | ## Metrics And Charts |
| 202 | |
| 203 | Use typed `metrix` instruments and `charts.yaml`. |
| 204 | |
| 205 | - Prefer creating instruments once when the metric surface is stable. Some |
| 206 | collectors intentionally build instruments in the collection path when the |
| 207 | surface is dynamic; if you keep that pattern, record why it is still the clean |
| 208 | V2 shape for that collector. |
| 209 | - Use `StateSet` for fixed one-active-state values. |
| 210 | - Use `Counter.ObserveTotal()` when the upstream value is a source counter. |
| 211 | - Put multipliers, divisors, hidden flags, float formatting, `instances`, and |
| 212 | `label_promotion` in `charts.yaml`, not ad hoc runtime chart code. |
| 213 | - Preserve V1 dimension algorithms exactly unless the old algorithm was wrong |
| 214 | and the user approves the change. |
| 215 | |
| 216 | When adding labels during migration, verify they are bounded and do not alter |
| 217 | chart identity unexpectedly. Labels that improve filtering are acceptable only |
| 218 | when they do not break existing chart/dimension contracts. |
| 219 | |
| 220 | ### Chart Variables |
| 221 | |
| 222 | V1 `collectorapi.Chart.Vars` have no direct `charts.yaml` / `charttpl` support |
| 223 | today. Some shipped health alerts depend on those variables. |
| 224 | |
| 225 | If the V1 collector uses `Vars`, the migration MUST NOT silently drop them. |
| 226 | Choose one of these paths before implementation: |
| 227 | |
| 228 | - add clean framework support by following |
| 229 | `src/go/plugin/framework/docs/changing-framework-code.md`; |
| 230 | - preserve the alert semantics through an approved equivalent design; |
| 231 | - get explicit user approval for a breaking alert change and update health, |
| 232 | metadata, generated docs, and release notes accordingly. |
| 233 | |
| 234 | Until one of those paths is approved, migrating a collector that uses chart |
| 235 | variables is blocked. Known V1 go.d collectors using chart variables at the |
| 236 | time of writing include `postgres`, `cockroachdb`, `hdfs`, `puppet`, `scaleio`, |
| 237 | `whoisquery`, and `zookeeper`. |
| 238 | |
| 239 | ### Obsoletion Timing |
| 240 | |
| 241 | V1 collectors often obsolete dynamic charts immediately with `MarkRemove()` / |
| 242 | `MarkNotCreated()`. V2 chart templates expire unseen chart instances through |
| 243 | `lifecycle.expire_after_cycles` and related chartengine policy. Exact immediate |
| 244 | V1 timing is not always reproducible in V2. |
| 245 | |
| 246 | For every V1 dynamic chart, record the old obsoletion timing and choose the V2 |
| 247 | lifecycle policy deliberately. If the timing changes, document the behavioral |
| 248 | change and get approval when it affects alerts or user-visible chart lifetime. |
| 249 | |
| 250 | ### Dynamic IDs And Contexts |
| 251 | |
| 252 | V1 collectors often build chart IDs with `fmt.Sprintf`. V2 templates derive |
| 253 | contexts from `context_namespace`, group context namespaces, and chart context |
| 254 | leaves. V2 autogen also has `engine.autogen.max_type_id_len` behavior. |
| 255 | |
| 256 | The migration MUST prove that generated chart IDs, contexts, and dimensions |
| 257 | match the old public contract, or explicitly record and approve any difference. |
| 258 | |
| 259 | ## Config Rules |
| 260 | |
| 261 | Migrations MUST keep existing YAML and JSON field names. Do not rename config |
| 262 | keys to match new code style. |
| 263 | |
| 264 | Do not add public config options as part of a migration unless they are required |
| 265 | to preserve existing behavior. A proposed config option MUST name the concrete |
| 266 | operator decision it enables; "operators may want to tune it" is not enough. |
| 267 | Internal tuning SHOULD use constants. New user choices belong in a later |
| 268 | feature batch with schema, stock config, metadata, and docs updated together. |
| 269 | |
| 270 | `autodetection_retry`, `update_every`, and `vnode` are job/runtime fields in |
| 271 | many existing collectors. Preserve the migrated collector's current YAML/JSON |
| 272 | behavior and keep `config.go`, `config_schema.json`, stock config, and metadata |
| 273 | consistent. Do not copy another collector's schema treatment for these fields |
| 274 | without checking current framework expectations. |
| 275 | |
| 276 | ## Host Scopes, Functions, And Topology |
| 277 | |
| 278 | Host scopes, Functions, and topology are product design choices, not automatic |
| 279 | migration side effects. |
| 280 | |
| 281 | - If the V1 collector already has vnode behavior, preserve it. |
| 282 | - If adding host scopes would be useful but is not required for compatibility, |
| 283 | split it into a later product decision. |
| 284 | - If the collector exposes Functions, isolate Function code in a dedicated |
| 285 | `<name>func/` package behind a narrow `Deps` interface. That interface MUST |
| 286 | NOT expose, return, or embed `*Collector`. |
| 287 | - New topology producers MUST use `src/go/pkg/topology/v1` and validate against |
| 288 | `src/plugins.d/FUNCTION_TOPOLOGY_SCHEMA.json`. |
| 289 | |
| 290 | ## Tests |
| 291 | |
| 292 | Migration tests MUST prove compatibility and V2 behavior. |
| 293 | |
| 294 | Current shared helpers can prove schema validation, template compilation, and |
| 295 | fixture chart coverage. They do not prove full V1-to-V2 chart identity parity, |
| 296 | and they do not prove alert variables unless the migration records the variable |
| 297 | source explicitly. |
| 298 | |
| 299 | The non-negotiable minimum is alert-observable parity. The migration MUST prove |
| 300 | that health alert `on:` contexts, referenced dimensions, referenced variables, |
| 301 | and dimension algorithms/units used by alerts still resolve after migration. |
| 302 | This is provable today with chart-template checks plus manual alert-variable |
| 303 | review. |
| 304 | |
| 305 | Exhaustive compiled chart-identity parity is broader: title, family, type, |
| 306 | priority, multiplier, divisor, hidden, float flags, label promotion, and |
| 307 | lifecycle policy. If existing exported helpers cannot observe those fields, |
| 308 | either add a framework `collecttest` helper under |
| 309 | `src/go/plugin/framework/docs/changing-framework-code.md`, or record a manual |
| 310 | comparison recipe with exact fields and evidence. Do not claim exhaustive chart |
| 311 | identity parity without one of those paths. |
| 312 | |
| 313 | At minimum: |
| 314 | |
| 315 | - config YAML/JSON serialization compatibility; |
| 316 | - `Init`, `Check`, `Collect`, and `Cleanup` lifecycle coverage; |
| 317 | - metric-store cycle behavior, including `BeginCycle`, successful commit, and |
| 318 | abort on expected hard collection errors; |
| 319 | - alert-observable parity for chart contexts, dimension IDs/names, algorithms, |
| 320 | units, and variables used by health alerts; |
| 321 | - chart template schema/decode/validate/compile coverage; |
| 322 | - chart coverage for fixture data expected to materialize all dimensions; |
| 323 | - health alert compatibility when alerts exist: each alert `on:` context must |
| 324 | exist in the compiled template, and every variable referenced by alert |
| 325 | calculations must still be provided by a dimension, variable-equivalent |
| 326 | design, or approved alert change; |
| 327 | - generated integration artifact consistency when metadata/taxonomy changes; |
| 328 | - taxonomy coverage checks when chart contexts change; |
| 329 | - host-scope tests if scopes/vnodes are preserved or introduced. |
| 330 | |
| 331 | `collecttest.AssertChartCoverage` is not a replacement for chart-identity |
| 332 | parity. It verifies that emitted series and the template agree; it can still |
| 333 | pass when both writer and template were renamed consistently. |
| 334 | |
| 335 | Exhaustive chart-identity parity is required before claiming full compatibility. |
| 336 | If the migration lacks a shared helper or recorded manual comparison recipe for |
| 337 | the compiled fields listed above, claim only the narrower compatibility that was |
| 338 | actually proven. |
| 339 | |
| 340 | Until a shared alert-variable helper exists, use a manual grep/review pass for |
| 341 | alert variables: |
| 342 | |
| 343 | ```bash |
| 344 | rg -n "\\$[A-Za-z_][A-Za-z0-9_]*" src/health/health.d/<collector>.conf |
| 345 | rg -n "Vars:" src/go/plugin/go.d/collector/<collector> -g '*.go' |
| 346 | ``` |
| 347 | |
| 348 | Every referenced variable must still be supplied by a dimension, |
| 349 | variable-equivalent design, or approved alert change. |
| 350 | |
| 351 | Prefer table-driven tests using `map[string]struct{}` keyed by case name when |
| 352 | cases share setup and assertion shape. |
| 353 | |
| 354 | ### Parity manifest recipe |
| 355 | |
| 356 | When chart-identity parity needs the compiled fields above and no shared helper |
| 357 | observes them, this is a concrete form of that "manual comparison recipe": prove |
| 358 | context/dimension/value parity by rendering BOTH collectors into one manifest |
| 359 | shape, without depending on chart-ID string equality. |
| 360 | |
| 361 | 1. Freeze the V1 output as a golden manifest. For representative fixtures, run |
| 362 | the V1 `Collect()` + `Charts()` and serialize, per chart: context, type, |
| 363 | family, units, priority, and per dimension the name, algorithm, and de-scaled |
| 364 | value (the V1 `map[string]int64` value divided by the dimension `Div`). Commit |
| 365 | the goldens under `testdata/`. |
| 366 | 2. Render the V2 path into the same shape. Load the collector's LIVE |
| 367 | `ChartTemplateYAML()` into a real `chartengine`, run a cycle through the real |
| 368 | `Collect()` and `metrix` store, plan against the store, and read the plan's |
| 369 | create/update actions into the same per-chart structure. |
| 370 | 3. Compare structurally with a float tolerance. V1 truncates `value*Div` to |
| 371 | `int64` while V2 stores the true float, so compare values within a tolerance |
| 372 | (for example `1e-3`), not for exact equality. Assert that context, family, |
| 373 | units, dimension names, and dimension algorithms match. |
| 374 | |
| 375 | Compare chart IDs too when the migration preserves them (the default). Omit only |
| 376 | chart-ID equality when the user approved a breaking chart-ID change -- then the |
| 377 | preserved contract is the context, and chart-ID-keyed alert examples in |
| 378 | `src/health/REFERENCE.md` must be updated. A gap (NaN or absent value) MUST fail |
| 379 | the comparison loudly; never silently map it to 0. |
| 380 | |
| 381 | The render MUST go through the collector's LIVE `ChartTemplateYAML()` and store, |
| 382 | not a separately rebuilt template -- a test that rebuilds the template can pass |
| 383 | even when the shipped `ChartTemplateYAML()` is wrong. |
| 384 | |
| 385 | ## Validation |
| 386 | |
| 387 | Run the narrowest commands that prove the changed contract. Typical migration |
| 388 | validation includes: |
| 389 | |
| 390 | ```bash |
| 391 | cd src/go |
| 392 | go test -count=1 ./plugin/go.d/collector/<name>/... |
| 393 | go test -race -count=1 ./plugin/go.d/collector/<name>/... |
| 394 | timeout 15s go run ./cmd/godplugin -m <name> -d |
| 395 | ``` |
| 396 | |
| 397 | For the load-verification command, success means the module is registered, a |
| 398 | job starts, and the command keeps running until the timeout stops it. Treat |
| 399 | `unknown module`, `no jobs started`, config-load errors, or an immediate exit |
| 400 | before the timeout as failures. Use `-c <config-dir>` when the migrated test |
| 401 | config lives outside the normal go.d config search path. |
| 402 | |
| 403 | Also run framework or integration checks when the migration touches those |
| 404 | contracts: |
| 405 | |
| 406 | - chart template/framework changes: |
| 407 | `go test -count=1 ./plugin/framework/charttpl ./plugin/framework/chartengine` |
| 408 | - `metrix` changes: |
| 409 | `go test -count=1 ./pkg/metrix/...` |
| 410 | - runtime/framework changes: |
| 411 | follow `src/go/plugin/framework/docs/changing-framework-code.md` |
| 412 | - metadata/taxonomy/generated docs: |
| 413 | follow `.agents/skills/integrations-lifecycle/consistency.md` |
| 414 | |
| 415 | Record exactly what ran. Full validation MUST NOT be claimed from a narrow |
| 416 | command. |
| 417 | |
| 418 | ## Commit Shape |
| 419 | |
| 420 | Prefer small coherent commits: |
| 421 | |
| 422 | 1. Add missing compatibility tests, if needed. |
| 423 | 2. Move registration and collection path to V2. |
| 424 | 3. Convert charts to `charts.yaml` while preserving chart identity. |
| 425 | 4. Update synchronized integration artifacts. |
| 426 | 5. Add follow-up enrichment only in a separate batch. |
| 427 | |
| 428 | If the migration requires a framework change, stop and follow |
| 429 | `src/go/plugin/framework/docs/changing-framework-code.md` before implementing |
| 430 | collector-local glue. |
| 431 | |
| 432 | ## Anti-Patterns |
| 433 | |
| 434 | - Rewriting chart IDs, contexts, or dimensions only because the V2 template |
| 435 | makes a new name easier. |
| 436 | - Adding labels, host scopes, topology, or Functions in the same commit as the |
| 437 | compatibility migration without a product decision. |
| 438 | - Shipping a V1-to-bridge-to-V2 runtime path after the V2 store is in place. |
| 439 | - Hiding framework gaps in collector-local helpers. |
| 440 | - Treating generated integration pages or README symlinks as authoring sources. |
| 441 | - Claiming compatibility without a manifest and tests. |