@cryptotaxi247 / netdata / commits / 2f1ea6514

Fix SNMP topology index-derived endpoints (#22371)

Co-authored-by: ilyam8 <ilya@netdata.cloud>

Costa Tsaousis committed May 4, 2026 at 22:24 UTC 2f1ea6514078baf7702b05157c2b3e37068bab8e
26 files changed +1682 -61
.agents/skills/project-snmp-profiles-authoring/SKILL.md new
+100
@@ -0,0 +1,100 @@
1 +---
2 +name: project-snmp-profiles-authoring
3 +description: Use when editing Netdata SNMP profile YAMLs, topology SNMP profiles, ddsnmp profile parsing, or profile-format documentation. Requires checking source MIB field accessibility, especially MAX-ACCESS not-accessible INDEX objects, before adding or changing profile symbols.
4 +---
5 +
6 +# SNMP Profile Authoring
7 +
8 +Use this skill before editing files under:
9 +
10 +- `src/go/plugin/go.d/config/go.d/snmp.profiles/`
11 +- `src/go/plugin/go.d/collector/snmp/ddsnmp/`
12 +- `src/go/plugin/go.d/collector/snmp/profile-format.md`
13 +- `src/go/plugin/go.d/collector/snmp_topology/`
14 +
15 +## Required Checks
16 +
17 +1. Identify the source MIB object for every profile field being added or changed.
18 +2. Check the object's `MAX-ACCESS`.
19 +3. If the object is `not-accessible`, do not configure it as a readable `symbol.OID`.
20 +4. If a `not-accessible` object appears in the table `INDEX`, derive it from the row OID index using `index` or `index_transform`.
21 +5. Keep index extraction and value formatting separate:
22 + - use `index` for one index component;
23 + - use `index_transform` for multiple components;
24 + - use `symbol.format` only for final formatting such as `ip_address`, `mac_address`, or `hex`.
25 +
26 +## Index Rules
27 +
28 +- `index` is 1-based.
29 +- `index_transform.start` and `index_transform.end` are 0-based and inclusive.
30 +- `index_transform: [{start: N}]` keeps index component `N` through the last component when `N > 0`.
31 +- `index_transform: [{start: 0, end: 0}]` keeps only the first index component.
32 +- `drop_right` can be used when the right side has fixed trailing components.
33 +
34 +## Common Patterns
35 +
36 +Q-BRIDGE learned FDB MAC:
37 +
38 +```yaml
39 +- tag: dot1q_fdb_mac
40 + symbol:
41 + format: mac_address
42 + index_transform:
43 + - start: 1
44 +```
45 +
46 +IP-MIB `ipNetToPhysicalTable` address:
47 +
48 +```yaml
49 +- tag: arp_ip
50 + symbol:
51 + format: ip_address
52 + index_transform:
53 + - start: 3
54 +```
55 +
56 +The `start: 3` skips `ifIndex`, address type, and the InetAddress length byte.
57 +
58 +LLDP-MIB local management address:
59 +
60 +```yaml
61 +- tag: lldp_loc_mgmt_addr
62 + symbol:
63 + name: lldpLocManAddr
64 + format: hex
65 + index_transform:
66 + - start: 2
67 +```
68 +
69 +The `start: 2` skips management-address subtype and length. Use `hex`, not
70 +`ip_address`, because LLDP management addresses can carry non-IP subtypes; the
71 +topology runtime normalizes IP-compatible bytes later.
72 +
73 +## Audit Recipe
74 +
75 +When a profile reads a table column, verify that the MIB object is readable:
76 +
77 +```bash
78 +rg -n -C 4 'OBJECT-TYPE|MAX-ACCESS[[:space:]]+not-accessible|ACCESS[[:space:]]+not-accessible' path/to/MIB
79 +```
80 +
81 +For known topology-sensitive symbols, scan profile YAMLs before committing:
82 +
83 +```bash
84 +rg -n 'name:[[:space:]]*(dot1qTpFdbAddress|ipNetToPhysicalIfIndex|ipNetToPhysicalNetAddressType|ipNetToPhysicalNetAddress|lldpLocManAddrSubtype|lldpLocManAddr)\b' src/go/plugin/go.d/config/go.d/snmp.profiles
85 +```
86 +
87 +Any hit must be reviewed. It is valid only when the tag is index-derived and does not declare `symbol.OID` for a `not-accessible` object.
88 +
89 +## Validation
90 +
91 +Run the narrow suites for the changed area:
92 +
93 +```bash
94 +cd src/go
95 +go test ./plugin/go.d/collector/snmp/ddsnmp/ddprofiledefinition
96 +go test ./plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector
97 +go test ./plugin/go.d/collector/snmp_topology
98 +```
99 +
100 +See `src/go/plugin/go.d/collector/snmp/profile-format.md` for the full profile syntax and the "Field accessibility" section.
.agents/sow/done/SOW-0001-20260501-qbridge-fdb-mac-from-index.md new
+565
@@ -0,0 +1,565 @@
1 +# SOW-0001 - SNMP topology: index-based extraction for not-accessible columns + observability + profile-engine macro
2 +
3 +## Status
4 +
5 +Status: completed
6 +
7 +Sub-state: implementation, regression fix, PR creation, first review-thread fix, Sonar duplication cleanup, and SOW close completed. Live-device validation against the originally reported affected device was not independently runnable in this workspace; residual runtime confirmation remains a PR/local-testing concern.
8 +
9 +## Requirements
10 +
11 +### Purpose
12 +
13 +Make the topology view show every L2 endpoint reachable through every managed switch on the network — laptops, IoT, printers, servers, AP-attached clients — across all common FDB protocol variations and vendor implementations, not just on the subset that happens to fit the path the code currently exercises. Today, on standards-compliant Q-BRIDGE-MIB-only switches, zero FDB endpoints surface. This is the dominant case in real enterprise LANs that skipped the legacy BRIDGE-MIB FDB.
14 +
15 +The work bundles three confirmed bug sites that share one root cause (profile asks for SNMP fields marked `MAX-ACCESS not-accessible` as if they were readable columns), small operational-visibility improvements that surface alongside the fix, and a profile-engine macro that prevents the bug class from recurring.
16 +
17 +### User Request
18 +
19 +After landing the Netgear-switch profile fix (PR #22366) and seeing LLDP work but FDB stay empty, the user asked for a comprehensive plan covering all FDB variations — not a narrow patch.
20 +
21 +Verbatim user request: *"Have you done a plan on what is needed to fix the issue and support all the variations of FDB properly?"*
22 +
23 +Subsequent user direction (recorded as locked decisions below): single PR, multiple isolated commits per group, full SOW documenting every issue, external verification by 5 multi-agent reviewers, cross-check against LibreNMS source code in mirrored repos.
24 +
25 +### Assistant Understanding
26 +
27 +Facts (verified):
28 +
29 +- Three SNMP table fields are walked in our profiles as if they were readable columns, but the source MIBs declare them `MAX-ACCESS not-accessible`. On strict-spec devices, the fields return no value; the runtime sinks then drop the rows because the relevant tag (MAC, IP, etc.) is empty.
30 + - A1: Q-BRIDGE-MIB FDB MAC (`dot1qTpFdbAddress`) — RFC 4363, used as INDEX of `dot1qTpFdbEntry`.
31 + - A2: IP-MIB modern ARP/ND — `ipNetToPhysicalIfIndex`, `ipNetToPhysicalNetAddressType`, `ipNetToPhysicalNetAddress` are all `not-accessible` per RFC 4293. Note that `ipNetToPhysicalPhysAddress` (the MAC itself) **is accessible** (`MAX-ACCESS read-create`); only the three index components are not-accessible.
32 + - A3: LLDP-MIB local management address — `lldpLocManAddrSubtype` and `lldpLocManAddr` are `not-accessible` per IEEE 802.1AB-2005, used as INDEX of `lldpLocManAddrEntry`.
33 +- LibreNMS, OpenNMS, and `netdisco/snmp-info` all extract the MAC for Q-BRIDGE FDB **from the OID index, not from a column**. None of them read `dot1qTpFdbAddress` as a column. Verified directly in mirrored source:
34 + - `librenms/librenms @ 90115d62d82a`: `includes/discovery/fdb-table/bridge.inc.php:31-98`
35 + - `OpenNMS/opennms @ 032d82cc926f`: `features/enlinkd/adapters/collectors/bridge/src/main/java/org/opennms/netmgt/enlinkd/snmp/Dot1qTpFdbTableTracker.java:49-119`
36 + - `netdisco/snmp-info @ 613d360b629d`: `lib/SNMP/Info/Bridge.pm:131-178`
37 +- The profile engine already supports `format: ip_address` on index-derived values (`index_tag_value.go:51-87`) and `format: mac_address` on column-derived values (`utils.go:61`, `value_processor.go:39`). Adding `format: mac_address` to the index-derived value path is a small additive change parallel to the existing `ip_address` case.
38 +- The existing `index_transform` mechanism (`{start: N, drop_right: M}`) already exists at `metrics.go:184` and is applied at `table_row_processor.go:107`. A2/A3 will use this same mechanism in addition to the new `mac_address` formatter.
39 +- Office field validation (sanitized): of four managed switches polled in a real LAN (vendors: MikroTik routerOS, MikroTik SwOS, Zyxel XS-class, Zyxel GS-class), three are backstopped by BRIDGE-MIB FDB (works), one is accidentally lenient by emitting `dot1qTpFdbAddress` as a column despite the spec (works by accident). None hit the strict-only-Q-BRIDGE failure mode. The originally reported failing device is in that strict-only failure mode.
40 +
41 +Inferences:
42 +
43 +- The Q-BRIDGE-MIB FDB profile, the modern IP-MIB ARP profile, and the LLDP local management address profile were likely shaped by mechanical extension of older profiles whose source MIB columns were accessible. Without per-author MIB-discipline awareness, the same mistake will recur.
44 +- The "lenient vendor returns the not-accessible column" hypothesis was previously unverified; it is now empirically confirmed against a real device but does not change the implementation choice (column-derived and index-derived MACs are byte-identical when both exist, per RFC 4363 §4 — both encode the same MAC address bytes).
45 +- Static FDB tables (`dot1qStaticUnicastTable`, `dot1dStaticTable`) are not just feature gaps; they have different semantics (filtering policy, multi-egress, allowed-port lists) that justify separate design rather than bundling here.
46 +
47 +Unknowns:
48 +
49 +- None block the locked decisions or implementation plan. Per-vendor support for proprietary FDB MIBs and the newer IEEE 802.1Q-2014 `IEEE8021-Q-BRIDGE-MIB` are deferred as listed in Followup.
50 +
51 +### Acceptance Criteria
52 +
53 +- A live SNMP poll of the originally reported affected device (Netgear GS110TP v3, sysObjectID `1.3.6.1.4.1.4526.100.4.19`) produces FDB endpoints whose count matches `dot1qTpFdbPort` row count from a fresh walk. Verification: `topology:snmp` function output + manual diff against the walk.
54 +- ARP/ND entries on a strict-spec L3 device are ingested with non-empty IP, ifIndex, and address type. Verification: regression fixture + targeted test, plus opportunistic real-device validation.
55 +- LLDP local management address is correctly populated on devices that implement the standards-compliant variant of `lldpLocManAddrTable`. Verification: regression fixture.
56 +- Existing behavior on devices that already work (BRIDGE-MIB FDB, lenient Q-BRIDGE-MIB devices, hybrid devices, Cisco-style per-VLAN context) is preserved bit-for-bit. Verification: existing `snmp_topology` and parity test suites pass without weakening; commit 7 includes a "lenient vendor" regression fixture (a Q-BRIDGE FDB walk where the column happens to populate) that exercises the no-regression path.
57 +- Engine macro `format: mac_address` is available for index-derived values. Output format: lowercase hex, colon-separated, two-digit per octet (`%02x` style), e.g. `aa:bb:cc:dd:ee:ff`. Octet validation: each component must be 0-255; on validation failure return empty string and let the row be dropped. Length-prefix tolerance: when the slice contains 7 components AND the first component equals 6 AND each of the remaining 6 components is a valid octet (0-255), treat as length-prefixed and use the trailing 6 components (defensive handling for F10).
58 +- **MAC output parity (column-side and index-side)**: column-side `format: mac_address` at `ddsnmp/ddsnmpcollector/utils.go:54` currently uses `%02X` (uppercase) and is exercised by tests at `collector_table_test.go:934-939` and `collector_device_meta_test.go:276-278` that expect uppercase output. As part of commit 1, switch the column-side format string to `%02x` (lowercase) and update those tests so both paths produce byte-identical lowercase output. Add an explicit column-vs-index parity unit test that runs both formatters on the same input and asserts byte equality. Rationale: `net.ParseMAC` and `normalizeMAC` (`topology_hex_normalization.go:18,31`) both lowercase, and industry tooling (LibreNMS, `ip` command, `ifconfig`) renders MACs lowercase — converging to lowercase is the natural shape.
59 +- Index-based extraction for A2 (modern ARP) handles RFC 4293 InetAddress encoding: a leading length octet (4 for IPv4, 16 for IPv6) preceding the address bytes. **The length octet is stripped at the profile level** via `index_transform: [{start: N, drop_right: M}]` on each affected tag (`arp_ip` tag uses positions after the length byte; `arp_addr_type` and `arp_if_index` use the earlier index positions). This keeps `formatIndexIPAddress` pure and documents the SNMP encoding where the MIB structure lives, rather than burying it in the formatter. (Alternative — extending `formatIndexIPAddress` to detect the prefix — was considered and rejected for keeping the formatter format-only.)
60 +- VLAN attribution falls back to `fdbID == VLAN_ID` when the `dot1qVlanCurrentTable` mapping is absent. Specifically: in `topology_cache_fdb.go:46-49`, if `c.fdbIDToVlanID[entry.fdbID]` returns empty, set `entry.vlanID = entry.fdbID`. This is a NEW fallback being ADDED alongside the existing mapping lookup, mirroring LibreNMS `bridge.inc.php:78`. Verification: unit test.
61 +- FDB rows referencing bridge ports without an `ifIndex` mapping are still ingested but tagged as unmapped. Implementation: an internal counter (not a chart) tracks unmapped-bridge-port FDB rows per poll cycle, plus a single rate-limited log line per poll cycle when the count is nonzero. Verification: unit test.
62 +- Warn-on-drop: when FDB rows are dropped due to empty MAC at `topology_cache_fdb.go:11-13`, emit at most ONE warning log per poll cycle with a count of dropped rows (not one log per row). Verification: unit test simulating a strict-Q-BRIDGE poll.
63 +- The profile-author guardrail exists as a project skill at `.agents/skills/project-snmp-profiles-authoring/SKILL.md` (note `project-` prefix per AGENTS.md:35,196,228 — runtime project skills MUST use this prefix) and as a new "Field accessibility" section in `src/go/plugin/go.d/collector/snmp/profile-format.md`. The profile-format.md section includes an audit recipe (a `grep` pattern for finding profile YAML symbols whose MIB declares `not-accessible`). Verification: file exists, links to profile-format.md from the skill, content reviewed in the multi-agent verification round.
64 +- All eight commits in the PR pass the existing `snmp_topology`, `pkg/topology/engine`, and `ddsnmp` test suites without regression.
65 +- Sensitive-data discipline: no community strings, bearer tokens, customer-identifying IPs, customer hostnames (sysName, sysDescr, ifAlias, ifDescr, LLDP remote names, LLDP port descriptions, chassis IDs, management addresses pointing to customer infrastructure), SNMPv3 usernames/auth/priv secrets, or community-member names appear in any committed file (SOW, profile YAMLs, code comments, tests, fixtures, commit messages, PR body, skill, profile-format.md update).
66 +
67 +## Analysis
68 +
69 +### Master gap & bug list (44 items)
70 +
71 +#### A. Confirmed parsing bugs — strict-spec devices return zero / incomplete data (3)
72 +
73 +| # | Site | Source MIB & spec | File:line | Effect |
74 +|---|---|---|---|---|
75 +| **A1** | Q-BRIDGE-MIB FDB MAC | RFC 4363 — `dot1qTpFdbAddress` is `MAX-ACCESS not-accessible`, INDEX of `dot1qTpFdbEntry` | `_std-topology-q-bridge-mib.yaml:21-25` (column read), drop at `topology_cache_fdb.go:10-13` | 0 FDB endpoints on strict-only-Q-BRIDGE devices (Netgear smart switches, parts of MikroTik SwOS, parts of Zyxel) |
76 +| **A2** | IP-MIB modern ARP/ND index components | RFC 4293 — `ipNetToPhysicalIfIndex`, `ipNetToPhysicalNetAddressType`, `ipNetToPhysicalNetAddress` are all `MAX-ACCESS not-accessible`. The MAC itself (`ipNetToPhysicalPhysAddress`) is `read-create` (accessible). | `_std-topology-fdb-arp-mib.yaml:192-217` reads three not-accessible columns | ARP entries on strict L3 devices lose IP, ifIndex, address type. MAC↔IP correlation breaks. The MAC value continues to be readable. |
77 +| **A3** | LLDP-MIB local management address | IEEE 802.1AB-2005 — `lldpLocManAddrSubtype` and `lldpLocManAddr` are `MAX-ACCESS not-accessible`, INDEX of `lldpLocManAddrEntry` | `_std-topology-lldp-mib.yaml:84-101` reads both not-accessible columns; no compat anchor (the *remote* table at `:184-319` has one — this one does not) | Local LLDP management address absent on strict devices |
78 +
79 +#### B. Latent parsing bugs — would surface if we add these tables (4)
80 +
81 +| # | Site | Why it'd repeat the pattern | Spec |
82 +|---|---|---|---|
83 +| **B1** | Q-BRIDGE static FDB `dot1qStaticUnicastTable` | `Address` AND `ReceivePort` both `not-accessible`. Only `AllowedToGoTo` is `read-write` | RFC 4363 |
84 +| **B2** | Q-BRIDGE multicast FDB `dot1qTpGroupTable` | `GroupAddress` `not-accessible` | RFC 4363 |
85 +| **B3** | IEEE8021-Q-BRIDGE-MIB FDB | Same shape, INDEX adds `ComponentId` (3-component INDEX) | IEEE 802.1Q-2018 |
86 +| **B4** | IP-MIB modern `ipAddressTable` | `AddrType` and `Addr` both `not-accessible` | RFC 4293 |
87 +
88 +#### C. Missing capabilities — not polled today (7)
89 +
90 +| # | Capability | Why it matters | Scope decision |
91 +|---|---|---|---|
92 +| **C1** | BRIDGE-MIB static FDB (`dot1dStaticTable`) | RFC 1493 SMIv1 — columns `read-write`, accessible. Different semantics (filtering policy). | **Defer** |
93 +| **C2** | Q-BRIDGE-MIB static FDB (`dot1qStaticUnicastTable`) | Needs index-based extraction (B1). Same semantics caveat as C1. | **Defer** |
94 +| **C3** | IEEE8021-Q-BRIDGE-MIB FDB | Modern alternative for IEEE 802.1Q-2014-only switches. | **Defer** |
95 +| **C4** | Modern `ipAddressTable` | For routers/firewalls running modern IP-MIB only. | **Defer** |
96 +| **C5** | Vendor proprietary FDB MIBs: HUAWEI-L2MAM-MIB, EXTREME-FDB-MIB, DLINKSW-L2FDB-MIB, HP-ICF-BRIDGE (Aruba/HPE), AX-FDB-MIB (AlaxalA), JUNIPER-VLAN/L2ALD-MIB, ALCATEL-IND1-MAC-ADDRESS-MIB (Nokia/Alcatel). LibreNMS also has handlers for EdgeSwitch (Ubiquiti), FortiSwitch (Fortinet), TiMOS (Nokia SR OS), AOS6/AOS7 (Alcatel-Lucent OmniSwitch), and VRP (Huawei). | LibreNMS handlers exist as references. Each is its own design problem. | **Defer** |
97 +| **C6** | CISCO-MAC-NOTIFICATION-MIB (trap-based) | Out-of-SNMP-poll scope | **Defer** |
98 +| **C7** | LLDP-MED, LLDP-EXT-DOT3, LLDP-EXT-DOT1 | Phone/PoE/voice-VLAN/inventory metadata. Out of scope; opt-in by-vendor. | **Defer** |
99 +
100 +#### D. Quality / attribution gaps — data appears, partially incomplete (6)
101 +
102 +| # | Gap | Evidence | Scope decision |
103 +|---|---|---|---|
104 +| **D1** | No `fdbID == VLAN_ID` fallback when `dot1qVlanCurrentTable` is absent. Today `topology_cache_fdb.go:46-49` only consults `c.fdbIDToVlanID`; if empty, `entry.vlanID` stays empty. The fix ADDS a fallback line that sets `entry.vlanID = entry.fdbID` when the mapping returns nothing | LibreNMS `bridge.inc.php:78` precedent (`$vlan = $vlan_fdb_dict[$vlanIndex] ?? $vlanIndex;`) | **In this PR** |
105 +| **D2** | FDB entries on unmapped bridge ports emit `IfIndex: 0` silently | `topology_observation_local_forwarding.go:29-42`. No counter or warning today | **In this PR** |
106 +| **D3** | No detection of FDB truncation by SNMP agent (large tables on small devices) | No `*counts vs walked* ` validation against `dot1qFdbDynamicCount` | **Defer** |
107 +| **D4** | No deduplication when same MAC is in LLDP remote AND FDB | Could produce two endpoint actors for one device | **Defer** |
108 +| **D5** | Port aggregation (LACP/LAG): FDB → member port ifIndex, no LAG rollup | `bridgePortToIf` is 1:1; LibreNMS also does not roll up | **Defer** |
109 +| **D6** | Cross-protocol freshness (LLDP age vs FDB age vs ARP age) not reconciled | Can produce ghost endpoints | **Defer** |
110 +
111 +#### E. Code smells / blind spots (6)
112 +
113 +| # | Issue | Where | Scope decision |
114 +|---|---|---|---|
115 +| **E1** | `macFromOIDIndexSuffix` lives in `_test.go`, not in production | `topology_snmprec_forwarding_test.go:546` | **In this PR** (helper logic folded into engine macro) |
116 +| **E2** | Test fixture parser has its own MAC-from-index fallback that bypasses production path | `topology_snmprec_forwarding_test.go:305-336` masks A1 | **In this PR** (resolved by adding fixtures that exercise profile→engine→cache end-to-end) |
117 +| **E3** | LLDP octet reassembly in `topology_management_address_normalization.go` is bespoke; no shared helper | Will duplicate when fixing A1/A3 | Engine macro replaces it (commit 1) |
118 +| **E4** | No logging when FDB rows are dropped due to empty MAC | `topology_cache_fdb.go:11-13` — silent data loss | **In this PR** (rate-limited per poll cycle, not per row) |
119 +| **E5** | Two anchors for `lldpRemManAddrTable` (primary `.1`/`.2` + compat `.3`) — unclear interaction with strict devices | `_std-topology-lldp-mib.yaml:184-319` — works in practice but warrants a comment | **In this PR** (one-line comment) |
120 +| **E6** | No engine-level `index_format: mac` macro | ddsnmp engine missing feature | **In this PR** (engine macro) |
121 +
122 +#### F. Vendor / firmware quirks (10)
123 +
124 +| # | Quirk | LibreNMS handler | Status in our code | Scope decision |
125 +|---|---|---|---|---|
126 +| **F1** | MikroTik LLDP RemManAddr exposes columns `.1`/`.2` despite spec | (unknown) | Handled (primary anchor in our profile) | n/a |
127 +| **F2** | Zyxel firmware emits malformed Q-BRIDGE indexes that need reshaping | `includes/discovery/fdb-table/zynos.inc.php:27-35` | Not handled | **Defer** |
128 +| **F3** | TP-Link JetStream ifIndex offset of `+49152` between BRIDGE-MIB and IF-MIB | `includes/discovery/fdb-table/jetstream.inc.php:38` | Not handled | **Defer** |
129 +| **F4** | Cisco IOS classic — per-VLAN BRIDGE-MIB via `community@<vlan>` | `includes/discovery/fdb-table/ios.inc.php:29` | Handled (`topology_vlan_context.go`) | n/a |
130 +| **F5** | Some Aruba IAP firmware — truncated Q-BRIDGE indexes | `arubaos.inc.php` partial | Not handled | **Defer** |
131 +| **F6** | Originally reported device firmware bug — 512-byte zero-filled `lldpLocSysCapSupported`/`Enabled` | (Netdata-specific finding) | Tolerated implicitly by `format: hex` | n/a |
132 +| **F7** | Stacked switches (Cisco StackWise, HP IRF) — aggregated FDB on master, per-member on others | Not handled in LibreNMS either | Not handled | **Defer** |
133 +| **F8** | Cisco SB / SF / SG Small Business — non-standard FDB shape | Cisco-SB-specific profile in our codebase | Partially handled | n/a |
134 +| **F9** | Lenient vendors return `dot1qTpFdbAddress` as a column despite spec | LibreNMS doesn't read the column at all (index-only) | Decision 1B (drop column) makes this irrelevant | n/a |
135 +| **F10** | Length-prefix byte in MAC index encoding — some agents (per LibreNMS comment, observed on Aruba CX, Comtrol) prepend a length octet (value 6) before the 6 MAC bytes, producing a 7-component index suffix | `includes/discovery/fdb-table/bridge.inc.php:80-96` | Not handled today | **In this PR** (defensive: when the slice has 7 components and the first equals 6, drop it; engine macro acceptance criterion above) |
136 +
137 +#### G. Engine / ddsnmp gaps (4)
138 +
139 +| # | Gap | Why it matters | Scope decision |
140 +|---|---|---|---|
141 +| **G1** | No `MAX-ACCESS` validation when authoring profiles | Lets the A-class bug be authored without warning | **In this PR** (skill + profile-format.md) |
142 +| **G2** | No native multi-position index extraction with format coercion | Forces ugly per-octet enumeration; the same fix shape we need for A1, A2, A3 | **In this PR** (engine macro `format: mac_address` for index-derived values) |
143 +| **G3** | No "fail-loud" when a column symbol returns no rows on a populated table | Silent data loss | **Defer** |
144 +| **G4** | No per-poll-cycle stats on rows-fetched vs rows-dropped per metric | Hard to diagnose A-class bugs in production | **Partial** — covered by E4 (warn-on-drop) for the FDB sink |
145 +
146 +#### H. Process / documentation gaps (4)
147 +
148 +| # | Gap | Scope decision |
149 +|---|---|---|
150 +| **H1** | No MIB-authoring spec or project skill — every author can repeat the not-accessible mistake | **In this PR** — `.agents/skills/project-snmp-profiles-authoring/SKILL.md` (note `project-` prefix per AGENTS.md:35,196,228) |
151 +| **H2** | No same-failure scan was performed at PR review time for this profile family | **In this PR** — reviewers ran the scan, results integrated; project skill encodes the rule for future PRs |
152 +| **H3** | Test fixtures cannot trivially be derived from real SNMP walks (snmprec format conversion not documented) | **Defer** |
153 +| **H4** | No reference table linking each topology MIB column we walk to its accessibility class | **In this PR** — `profile-format.md` MAX-ACCESS section, includes a `grep` audit recipe |
154 +
155 +### LibreNMS verification matrix
156 +
157 +For each issue, what LibreNMS does. File paths are relative to `librenms/librenms @ 90115d62d82a`.
158 +
159 +| Issue | LibreNMS handles? | Code path | Approach |
160 +|---|---|---|---|
161 +| A1 (Q-BRIDGE FDB MAC) | Yes | `includes/discovery/fdb-table/bridge.inc.php:31-35` (walk), `:98` (parse), `:80-96` (length-prefix tolerance) | Walk `dot1qTpFdbPort` (column .2, accessible); extract MAC from index. No column read. |
162 +| A2 (IP-MIB ARP `ipNetToPhysicalTable`) | Yes | `LibreNMS/Modules/ArpTable.php:104-118` | Walk `ipNetToPhysicalPhysAddress` and consume the **structured table keys** returned by `->table(1)` (a hierarchical ifIndex→addrType→address map; not a raw OID-suffix split). Shape repair at line 120. The MAC value comes from the column (which is accessible); IP/ifIndex/addrType come from the structured key path. |
163 +| A3 (LLDP local mgmt addr) | No | n/a — table not polled | LibreNMS sidesteps by not reading this table at all. We choose the right thing: index extraction. |
164 +| B1/C2 (Q-BRIDGE static FDB) | No | n/a — table not polled | Same gap |
165 +| B3/C3 (IEEE8021-Q-BRIDGE-MIB) | Partial | `LibreNMS/OS/Traits/QBridgeMib.php:50-58` | Used only for VLAN names; FDB extraction stays on classic Q-BRIDGE-MIB |
166 +| B4/C4 (modern `ipAddressTable`) | Partial | IPv4 still uses deprecated `ipAddrTable` (`LibreNMS/Modules/Ipv4Addresses.php:195`); IPv6 uses modern `ipAddressTable` (`LibreNMS/Modules/Ipv6Addresses.php:148`) | Mixed: deprecated for v4, modern for v6 |
167 +| C5 (vendor proprietary FDBs) | Yes (≥11 vendors) | `includes/discovery/fdb-table/{arubaos,vrp,ios,aos6,aos7,zynos,jetstream,edgeswitch,fortiswitch,timos,...}.inc.php` | Per-vendor override files |
168 +| D1 (VLAN fdbID==VLAN_ID fallback) | Yes | `bridge.inc.php:78` | `$vlan = $vlan_fdb_dict[$vlanIndex] ?? $vlanIndex;` |
169 +| F2 (Zyxel malformed Q-BRIDGE index) | Yes | `zynos.inc.php:27-35` | Reshape pass before normal parsing |
170 +| F3 (TP-Link JetStream offset) | Yes | `jetstream.inc.php:38` | Hardcoded `+49152` ifIndex offset |
171 +| F4 (Cisco per-VLAN context) | Yes | `ios.inc.php:29` | `SnmpQuery::context($vlan_raw, 'vlan-')->walk('BRIDGE-MIB::dot1dTpFdbPort')` |
172 +| F10 (length-prefix byte) | Yes | `bridge.inc.php:80-96` | Defensive: detect and strip the 7-byte index encoding |
173 +| Bridge-port → ifIndex | Yes | `bridge.inc.php:49-54` (build), `:108` (fallback to `basePort==ifIndex`) | Walks `dot1dBasePortIfIndex`; falls back when missing |
174 +| Stack / multi-component bridge | No | n/a | Same gap as ours |
175 +| LACP / port-channel rollup | No | n/a | Same gap as ours |
176 +| Test coverage / fixtures | Limited | `tests/data/timos_fdb-table.json`, `tests/snmpsim/zynos_gs1900-fdb.snmprec` | Two relevant fixtures |
177 +
178 +Cross-references:
179 +
180 +- `netdisco/snmp-info` `Bridge.pm:160-178` — `_qb_fdbtable_index` (lines 160-165) decodes MAC from index; `qb_fw_mac` (lines 167-178) walks `qb_fw_port` and applies the decoder. Never reads the not-accessible column.
181 +- OpenNMS `features/enlinkd/adapters/collectors/bridge/src/main/java/org/opennms/netmgt/enlinkd/snmp/Dot1qTpFdbTableTracker.java:64` (column collection: port + status only), `:113` (decodes address from row index).
182 +
183 +**Conclusion**: index-based extraction is the universal industry approach for A1. A2 and A3 are RFC-backed analogs with the same mechanical fix shape, but the three reference implementations do not directly cover them — A2 has a partial parallel in LibreNMS `ArpTable.php` (which uses the structured-table-key approach, semantically equivalent), A3 has no reference precedent (LibreNMS skips the table). Decision 1B is the right call for all three; A1 is independently proven, A2/A3 are RFC-driven.
184 +
185 +### Risks (cross-cutting)
186 +
187 +- **Engine macro added in this PR**: contained scope. The format handler extends `formatIndexTagValue` with `case "mac_address":` parallel to the existing `case "ip_address":`. Octet validation (each component 0-255) and length-prefix tolerance (drop a leading length-of-6 octet when the slice has 7 components) are part of the formatter, mirroring `formatIndexIPAddress` patterns and LibreNMS behavior. Engine has the existing `format: ip_address` precedent that exercises the same code path.
188 +- **A2/A3 implementation completeness**: the new `format: mac_address` formatter is necessary but not sufficient. A2 and A3 need correct `index` + `index_transform` declarations in the profile YAML to slice the OID suffix to the right components before formatting. For A2 in particular, the IP component of the OID index is RFC 4293 InetAddress-encoded (a length octet followed by address bytes); the length octet is **stripped at the profile level** via `index_transform` so the `format: ip_address` handler stays pure (format-only). The engine macro adds `format: mac_address` for index values; profile-level slicing handles SNMP encoding peculiarities.
189 +- **Removing the column read** (Decision 1B): index-derived MAC and column-derived MAC are byte-identical when both exist (RFC 4363 §4 — both encode the same MAC bytes). On lenient devices that today populate the column, behavior stays correct. On strict devices, FDB starts working. No device regresses. The lenient-vendor regression fixture in commit 7 verifies this.
190 +- **VLAN fallback (D1)** could over-attribute if a device uses `fdbID != VLAN_ID` mapping but doesn't expose `dot1qVlanCurrentTable`. LibreNMS accepts this trade-off; the fallback is a documented best-effort.
191 +- **Warn-on-drop (E4)** could be noisy. Implementation MUST be rate-limited to one log line per poll cycle with a count, not one per dropped row. This is in the acceptance criteria.
192 +
193 +## Pre-Implementation Gate
194 +
195 +Gate state at implementation start: decisions locked; implementation authorized after validation.
196 +
197 +Problem / root-cause model:
198 +
199 +- Three SNMP table fields are walked in our profiles as if they were readable columns, but the source MIBs declare them `MAX-ACCESS not-accessible`. Strict-spec devices correctly return no value for those columns. Our runtime sinks then drop the rows because the relevant tag (MAC, IP, etc.) is empty. The bug class repeats because there is no project-level rule requiring profile authors to consult MIB MAX-ACCESS, and no engine-level macro making the right way (index extraction with format coercion) ergonomic.
200 +
201 +Evidence reviewed:
202 +
203 +- RFCs 1493, 4188, 4293, 4363; IEEE 802.1AB-2005; IEEE 802.1Q-2018 (downloaded into `/tmp/sow-verify/`).
204 +- Profile YAMLs: `_std-topology-q-bridge-mib.yaml`, `_std-topology-fdb-arp-mib.yaml`, `_std-topology-lldp-mib.yaml`, `_std-topology-stp-mib.yaml`, `_std-topology-cisco-vtp-mib.yaml`.
205 +- Runtime sinks: `topology_cache_fdb.go`, `topology_cache_metric_dispatch.go`, `topology_cache_tags.go`, `topology_observation_local_forwarding.go`, `topology_vlan_context*.go`, `topology_observation_local_identity.go`.
206 +- Engine: `ddsnmp/ddprofiledefinition/{metrics.go,validation.go,selector.go}`, `ddsnmp/ddsnmpcollector/{table_row_processor.go,index_tag_value.go,utils.go,value_processor.go,cross_table_lookup.go}`.
207 +- `librenms/librenms @ 90115d62d82a` source (file:line citations in matrix above).
208 +- `netdisco/snmp-info @ 613d360b629d`: `lib/SNMP/Info/Bridge.pm` and `lib/SNMP/Info/IEEE802_Bridge.pm`.
209 +- `OpenNMS/opennms @ 032d82cc926f`: `features/enlinkd/.../Dot1qTpFdbTableTracker.java`.
210 +- Live SNMP walks from one originally reported affected device and four office-network managed switches (3 vendors).
211 +- Two rounds of external multi-agent reviews (5 reviewers per round: Codex, GLM, MiniMax, Qwen, Kimi). Round 2 produced 20 fixes that have been applied to this SOW.
212 +
213 +Affected contracts and surfaces:
214 +
215 +- Profile YAMLs: `_std-topology-q-bridge-mib.yaml`, `_std-topology-fdb-arp-mib.yaml`, `_std-topology-lldp-mib.yaml`.
216 +- Runtime: `topology_cache_fdb.go`, `topology_cache_tags.go`, `topology_observation_local_forwarding.go`.
217 +- Engine: `ddsnmp/ddsnmpcollector/index_tag_value.go` (extending `formatIndexTagValue`).
218 +- New skill: `.agents/skills/project-snmp-profiles-authoring/SKILL.md`.
219 +- Updated docs: `src/go/plugin/go.d/collector/snmp/profile-format.md` (new "Field accessibility" section + audit recipe).
220 +- Test data: new snmprec fixtures under `src/go/plugin/go.d/collector/snmp_topology/testdata/`.
221 +- No public/operator docs change beyond release notes.
222 +- No public CLI / UI / schema change.
223 +- No `AGENTS.md` change required (skill follows the `project-*` convention; no legacy registration needed).
224 +
225 +Existing patterns to reuse:
226 +
227 +- LLDP management-address octet decomposition in `_std-topology-lldp-mib.yaml:206-239` and the runtime reassembly in `topology_management_address_normalization.go:13-35` (`reconstructLldpRemMgmtAddrHex`) — supersedable once the engine macro lands; kept as production reference until then.
228 +- VLAN-context plumbing in `topology_vlan_context.go` is robust and need not change.
229 +- Test helper `macFromOIDIndexSuffix(parts []string)` at `topology_snmprec_forwarding_test.go:546` — its decode logic moves into production as part of the engine macro (commit 1).
230 +- LibreNMS `bridge.inc.php` "fdbID == vlanID" fallback at `:78` — directly mirrored.
231 +- Existing `formatIndexIPAddress` at `index_tag_value.go:58-87` — the new `formatIndexMACAddress` parallels it (validation, length-prefix tolerance, error path returning empty string).
232 +
233 +Risk and blast radius:
234 +
235 +- A1+A2+A3 fixes: low. Additive (column read removed, index extraction added). No device regresses (RFC 4363 §4 guarantees byte equivalence; lenient-vendor fixture in commit 7 verifies).
236 +- Engine `format: mac_address` for index-derived values: low. Mirrors existing `format: ip_address`; tests reuse the same harness.
237 +- D1 VLAN fallback: low. Documented best-effort matching LibreNMS.
238 +- D2 IfIndex tracking: low. Internal counter + one log line per poll cycle. No chart, no public metric.
239 +- E4 warn-on-drop: low. Rate-limited per poll cycle.
240 +- Static FDB additions: deferred — different semantics, separate SOW.
241 +- Per-vendor SOWs: deferred, no risk in this PR.
242 +
243 +Sensitive data handling plan:
244 +
245 +- All durable artifacts in this SOW (the SOW itself, profile YAMLs, code, code comments, tests, fixtures, commit messages, PR body, the new skill, the profile-format.md update) must contain zero raw sensitive data: no community member or customer names, no SNMP communities, no bearer tokens, no SNMPv3 usernames / authentication / privacy secrets, no customer-identifying IPs (private RFC1918 IPs are acceptable when used as illustrative examples in profile-format.md, never in fixtures derived from real walks), no customer-identifying device strings (real-world `sysName`, `sysDescr`, `ifAlias`, `ifDescr`, LLDP remote `sysName`, LLDP remote port descriptions, real chassis IDs, customer-pointing management addresses), no proprietary incident details.
246 +- Use placeholders (`[REDACTED]`, "the user", "the reporter", "originally reported affected device"), public product names (e.g. "Netgear GS110TP v3" is a publicly sold product, not PII), and file:line citations.
247 +- Test fixtures derived from real device walks must be sanitized: replace customer hostnames in port descriptions or sysName with neutral labels (`endpoint-1`, `port-A`, etc.), and strip or stub LLDP remote-name fields and LLDP port descriptions. Sanitization happens before the fixture is staged.
248 +- Pre-commit checklist (run before each commit and before opening the PR). Uses `rg -P` (ripgrep with PCRE) — POSIX `grep -E` does not support negative lookahead and would silently fail. The checklist scans **both staged and unstaged** content, plus commit messages and the PR body separately.
249 + ```bash
250 + # Helper: stream all uncommitted changes (staged + unstaged)
251 + git_diff_all() { { git diff --cached; git diff; }; }
252 +
253 + # 1. credentials and tokens in any uncommitted content
254 + git_diff_all | rg -P '(community|bearer|token|secret|password|auth.?key|priv.?key|snmpv3)\s*[:=]'
255 +
256 + # 2. customer-identifying IPv4 outside RFC1918, and IPv6 global-unicast-like
257 + # addresses (2000::/3). The OID-context exclusion
258 + # (^| [^0-9.])(?!1\.[0-3]\.6\.1\.) avoids flagging SNMP OIDs like
259 + # 1.3.6.1.4.1.x. The lookahead chain after that excludes private IPv4
260 + # ranges. The IPv6 pattern is case-insensitive and intentionally excludes
261 + # ULA fc00::/7.
262 + git_diff_all | rg -P '(?<![0-9.])(?!1\.[0-3]\.6\.1\.)(?!10\.)(?!172\.(1[6-9]|2[0-9]|3[01])\.)(?!192\.168\.)\b[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\b'
263 + git_diff_all | rg -P '(?i)\b(?:2[0-9a-f]{3}|3[0-9a-f]{3}):[0-9a-f:]+\b'
264 +
265 + # 3. customer-identifying device strings (replace with neutral labels in fixtures)
266 + git_diff_all | rg -iP '(sysName|sysDescr|ifAlias|ifDescr|chassis.?id|mgmt.?addr|lldp.?rem.?(sys.?name|port.?desc))'
267 +
268 + # 4. proper names that surfaced in chat or Slack during work on this SOW.
269 + # Maintain the personal-name list in a local environment file (.env or
270 + # AGENTS.local.md, both gitignored). Never embed names in this SOW.
271 + # Example: NAME_PATTERN="Firstname1|Firstname2|Surname1"; export NAME_PATTERN
272 + git_diff_all | rg -iP "$NAME_PATTERN"
273 +
274 + # 5. commit messages — three coverage paths because each catches a different point:
275 + # a) prepare-commit-msg hook: scan $1 (the in-progress message file)
276 + # before commit goes through.
277 + # b) post-commit verification of the most recent commit:
278 + git log -1 --format='%B' | rg -iP '(community|bearer|token|secret|password|auth.?key|priv.?key|snmpv3)\s*[:=]|sysName|sysDescr|ifAlias|ifDescr|chassis.?id|mgmt.?addr'
279 + git log -1 --format='%B' | rg -iP "$NAME_PATTERN"
280 + # c) range scan across all commits not yet merged to upstream/master,
281 + # just before opening the PR:
282 + git log upstream/master..HEAD --format='%B' | rg -iP '(community|bearer|token|secret|password|auth.?key|priv.?key|snmpv3)\s*[:=]|sysName|sysDescr|ifAlias|ifDescr|chassis.?id|mgmt.?addr'
283 +
284 + # 6. PR body — apply the same patterns to the body before `gh pr create` /
285 + # `gh pr edit --body-file`. If using a body file, run the patterns
286 + # against that file directly.
287 + ```
288 + Each `rg` invocation MUST return zero lines before commit / PR open. If `rg` is unavailable, the regex set requires it and the checklist cannot be satisfied with `grep` alone — install ripgrep first.
289 +
290 +Implementation plan: see "Plan" section below — 8 commits, single PR off `upstream/master`, branch `snmp-qbridge-fdb-mac-from-index`. Validation before implementation found one required engine prerequisite: current `index_transform` cannot express "start at N and keep the rest" (`validation.go` rejects `start > end`, and `applyIndexTransform` requires an explicit in-bounds `end`). The user accepted extending `index_transform` so `start > 0` with omitted/zero `end` means "through the last index component"; `start: 0, end: 0` keeps its existing "first component only" behavior.
291 +
292 +Validation plan:
293 +
294 +- Unit tests for: engine `format: mac_address` formatter (octet validation 0-255, length-prefix tolerance, output format `aa:bb:cc:dd:ee:ff`); A1+A2+A3 profile→engine→cache flow on synthetic strict-spec fixtures; D1 VLAN fallback; D2 unmapped-bridge-port counter; E4 rate-limited warn-on-drop.
295 +- Snmprec fixtures: at least one strict-spec Q-BRIDGE FDB fixture (sanitized derivative of the user-reported walk), one strict-spec modern ARP fixture (synthetic, RFC 4293 InetAddress-encoded), one strict-spec LLDP local mgmt addr fixture (synthetic), one **lenient-vendor** Q-BRIDGE FDB fixture (column populated; verifies no-regression on the path that already works).
296 +- Existing test suites must pass without weakened assertions: `snmp_topology`, `pkg/topology/engine`, `ddsnmp`.
297 +- Manual validation: live poll against the originally reported affected device showing FDB endpoint count matches walked row count.
298 +- Same-failure scan output: documented in this SOW under `## Validation` after commit 7. No further not-accessible-as-column sites detected by the multi-agent review.
299 +- Profile-format.md and skill content: reviewed in the multi-agent verification round before any commits.
300 +
301 +Artifact impact plan:
302 +
303 +- AGENTS.md: update required. The new runtime project skill follows the `project-*` convention, and the Project Skills Index must stop saying no `project-*` skills exist.
304 +- Runtime project skills: new `.agents/skills/project-snmp-profiles-authoring/SKILL.md` (in this PR).
305 +- Specs under `.agents/sow/specs/`: no expected change. The skill + profile-format.md cover the authoring rule.
306 +- End-user/operator docs: `profile-format.md` gets a new "Field accessibility" section with the rule + audit recipe.
307 +- End-user/operator skills: none.
308 +- SOW lifecycle plan at implementation start: move from `pending/` to `current/` after user authorization, then close after merge or explicit user request. Actual closure is recorded in the execution log and validation gate.
309 +
310 +Open decisions: none. All five locked below.
311 +
312 +## Locked Decisions
313 +
314 +### Decision 1 — MAC extraction approach for not-accessible columns
315 +
316 +**Locked: Option B — drop column read entirely, extract from index only.**
317 +
318 +Reasoning: For A1, three reference implementations (LibreNMS `bridge.inc.php`, SNMP::Info `Bridge.pm`, OpenNMS `Dot1qTpFdbTableTracker.java`) all use index-only extraction; none reads the not-accessible column. For A2 and A3, the same approach is RFC-backed and mechanically identical, though direct industry parallels are fewer (LibreNMS handles A2 via a structured-table-key path semantically equivalent to index extraction; A3 is sidestepped by not polling the table). The "lenient vendor returns the column" hypothesis is empirically true for at least one vendor (Zyxel GS-class) but does not change the choice — column-derived and index-derived MACs are byte-identical (RFC 4363 §4), so removing the column read produces identical bytes on lenient devices and starts working on strict devices. Two paths multiplied across 3 sites = unjustified maintenance burden.
319 +
320 +### Decision 2 — Authoring guardrail format and path
321 +
322 +**Locked: project skill at `.agents/skills/project-snmp-profiles-authoring/SKILL.md` (note `project-` prefix per AGENTS.md:35,196,228 — runtime project skills MUST use this prefix). Plus a new "Field accessibility" section in `src/go/plugin/go.d/collector/snmp/profile-format.md` containing the rule and a `grep` audit recipe. The skill links to profile-format.md.**
323 +
324 +Reasoning: The user requested skill-shape rather than spec-shape. profile-format.md is the canonical authoring reference (2046 lines, no current MAX-ACCESS mention) and is the right home for the rule itself. The skill points authors at it. The `project-` prefix follows the project convention; using a different prefix would require an explicit registration entry in AGENTS.md, which is unjustified for a new skill.
325 +
326 +### Decision 3 — Quality / observability improvements in this PR
327 +
328 +**Locked: Option A — include all three.**
329 +
330 +- D1 VLAN fallback: resolve `fdbID == VLAN_ID` as a late fallback when producing observations, after the `dot1qVlanCurrentTable` mapping has had a chance to populate. Do not eagerly store `entry.vlanID = entry.fdbID` in `updateFdbEntry`, because Q-BRIDGE FDB rows are processed before the VLAN mapping table in the current profile order and an eager fallback can block a later correct mapping.
331 +- D2 IfIndex tracking: when `parseIndex(c.bridgePortToIf[bridgePort])` returns 0, increment an internal per-poll counter; emit at most ONE log line per poll cycle if the counter is nonzero. No new chart or public metric.
332 +- E4 warn-on-drop: when `topology_cache_fdb.go:11-13` drops rows due to empty MAC, emit at most ONE rate-limited warning per poll cycle with a count of dropped rows (not one log per row).
333 +
334 +Reasoning: Each is file-local, additive, cheap, and ensures the next bug of this class is visible immediately rather than requiring forensic walks.
335 +
336 +### Decision 4 — Engine macro G2 in this PR; static FDB / IEEE8021-Q-BRIDGE-MIB / modern ipAddressTable / vendor proprietary deferred to child SOWs
337 +
338 +**Locked: engine macro IN this PR. Static FDB, IEEE8021-Q-BRIDGE-MIB, modern `ipAddressTable`, vendor-proprietary FDB MIBs all DEFERRED — child SOWs opened before this SOW closes.**
339 +
340 +Reasoning: Engine macro is a small additive change (extends `formatIndexTagValue` with `case "mac_address":` parallel to the existing `case "ip_address":`, plus octet validation and length-prefix tolerance). Static FDB has different semantics (filtering policy, multi-egress) and warrants its own design SOW. IEEE8021-Q-BRIDGE-MIB and modern `ipAddressTable` have no real-device evidence in this SOW justifying immediate work. Vendor-proprietary FDBs are each their own design problem.
341 +
342 +### Decision 5 — `index_transform` variable-tail support
343 +
344 +**Locked: Option A — extend `index_transform` semantics.**
345 +
346 +Reasoning: Q-BRIDGE MAC extraction and variable-length IP/LLDP management-address indexes need "slice from this index component through the end". Current `index_transform` can only select explicit inclusive ranges or drop a fixed number of right-side components. Duplicating tags for every possible IPv4/IPv6 or normal/length-prefixed shape would be brittle and order-sensitive because tag insertion does not overwrite an existing non-empty tag. Extending the engine is low risk because `start > 0, end == 0, drop_right == 0` is currently invalid, while existing `start: 0, end: 0` keeps its current first-component meaning.
347 +
348 +## Plan
349 +
350 +Single PR, branch `snmp-qbridge-fdb-mac-from-index` off `upstream/master`. Eight commits, isolated per group:
351 +
352 +| # | Commit | Files touched | Issues addressed |
353 +|---|---|---|---|
354 +| 1 | `engine: add format mac_address for index-derived tag values; converge column-side to lowercase` | `ddsnmp/ddsnmpcollector/index_tag_value.go` (new `formatIndexMACAddress` + switch case, lowercase output), `ddsnmp/ddsnmpcollector/table_row_processor.go` and `ddsnmp/ddprofiledefinition/validation.go` (extend `index_transform` so `start > 0` with omitted/zero `end` keeps the tail), `ddsnmp/ddsnmpcollector/utils.go:54` (flip `%02X` → `%02x` for parity), `ddsnmp/ddsnmpcollector/collector_table_test.go:934-939` and `collector_device_meta_test.go:276-278` (update existing assertions from uppercase to lowercase), unit tests including an explicit column-side ↔ index-side parity test | G2, E1 (folds in `macFromOIDIndexSuffix` decode logic), F10 (length-prefix tolerance with octet validation), MAC parity (column-side and index-side both produce lowercase `aa:bb:cc:dd:ee:ff`), variable-tail index extraction needed by A1/A2/A3 |
355 +| 2 | `profile + runtime: q-bridge fdb mac from index` | `_std-topology-q-bridge-mib.yaml` (replace octet enumeration with `format: mac_address` + `index_transform`), `topology_cache_tags.go` (rename/cleanup tag constants if needed), `topology_cache_fdb.go` (consume the new tag value), unit tests | A1 |
356 +| 3 | `profile + runtime: ip-mib modern arp from index` | `_std-topology-fdb-arp-mib.yaml` — replace not-accessible column reads with `index` + `index_transform` extraction. For `arp_if_index` and `arp_addr_type`: single `index: N` lookups. For `arp_ip`: `index_transform: [{start: <after-length-byte>}]` to skip the RFC 4293 InetAddress length octet *at the profile level*, then `format: ip_address` consumes a clean 4 / 16 octet sequence (formatter stays format-only; SNMP-encoding logic stays in the profile YAML where the MIB structure is documented). `topology_cache_stp_arp.go` and any tag consumer; unit tests including an IPv4 + IPv6 strict-spec fixture pair. | A2 |
357 +| 4 | `profile + runtime: lldp local management address from index` | `_std-topology-lldp-mib.yaml`, `topology_management_address.go` and/or `topology_management_address_normalization.go` (consumer), unit tests | A3, E5 (one-line comment near the dual anchors of `lldpRemManAddrTable`) |
358 +| 5 | `runtime: fdbID == VLAN_ID fallback when mapping table absent` | `topology_observation_local_forwarding.go` (late fallback to `entry.fdbID` only when no mapped VLAN ID exists at observation-output time), `topology_cache_fdb.go` (keep cache mutation compatible with late mapping), unit tests | D1 |
359 +| 6 | `runtime: warn-on-drop + unmapped-bridge-port counter` | `topology_cache_fdb.go:11-13` (rate-limited per-poll-cycle warning with count), `topology_observation_local_forwarding.go:29-42` (per-poll-cycle counter for `IfIndex == 0` cases + single log line when nonzero), unit tests verifying rate-limiting | D2, E4 |
360 +| 7 | `tests: snmprec fixtures + regression tests` | new fixtures under `src/go/plugin/go.d/collector/snmp_topology/testdata/`: strict-spec Q-BRIDGE FDB (sanitized from real walk), strict-spec ARP (synthetic), strict-spec LLDP local mgmt (synthetic), **lenient-vendor Q-BRIDGE FDB (column populated, verifies no-regression)**; forwarding tests that exercise all four | regression coverage for A1+A2+A3 |
361 +| 8 | `docs/skill: snmp profile authoring guardrail` | new `.agents/skills/project-snmp-profiles-authoring/SKILL.md` (links to profile-format.md), new "Field accessibility" section in `src/go/plugin/go.d/collector/snmp/profile-format.md` with the MAX-ACCESS rule + a `grep` audit recipe, update `AGENTS.md` Project Skills Index | H1, H4 |
362 +
363 +Order matters: commit 1 (engine) lands before 2-4 (profiles that consume the new format). 5-6 (observability) before 7 (tests can exercise them). 8 (docs) last so the skill reflects the final shape of the engine macro and profile pattern.
364 +
365 +Pre-commit checklist (Pre-Implementation Gate § Sensitive data plan) MUST pass before each commit and before PR open.
366 +
367 +## Execution Log
368 +
369 +### 2026-05-01
370 +
371 +- Created branch `snmp-qbridge-fdb-mac-from-index` off `upstream/master`.
372 +- Drafted initial narrow SOW; user pushed back asking for full survey across all FDB variations.
373 +- Re-investigated topology profiles, runtime sinks, ddsnmp engine, RFC 1493/4188/4293/4363 and IEEE 802.1AB-2005 / 802.1Q-2018.
374 +- Sent narrower SOW to 5 external reviewers (round 1: Codex, GLM, MiniMax, Qwen, Kimi). Codex independently identified A2 (IP-MIB ARP). User locked four decisions.
375 +- Verified user's office network: 3 of 4 managed switches backstopped by BRIDGE-MIB FDB; 1 lenient. Confirmed bug-affected population is "strict + Q-BRIDGE-only + no BRIDGE-MIB".
376 +- LibreNMS verification subagent ran; results integrated.
377 +- Sanitized SOW and `AGENTS.local.md`: no community member names, no SNMP communities, no bearer tokens.
378 +- Sent expanded SOW to the 5 reviewers (round 2). Findings consolidated:
379 + - 4 of 5 confirmed Bridge.pm citation off (242-271 → 160-178). Fixed.
380 + - Codex flagged Decision 2 skill path conflict with AGENTS.md (`project-*` convention). Fixed (path now `project-snmp-profiles-authoring`).
381 + - Codex flagged catalog count "28" wrong (actual 43 + F10 = 44). Fixed.
382 + - Codex flagged LibreNMS `ArpTable.php` mischaracterization (uses structured table keys, not raw index). Fixed.
383 + - Codex flagged LibreNMS `ipAddressTable` claim wrong (IPv4 deprecated, IPv6 modern). Fixed.
384 + - 4 of 5 flagged length-prefix byte edge case (F10). Added to catalog and engine macro acceptance.
385 + - 5 of 5 flagged pre-commit checklist as too vague. Replaced with concrete grep commands.
386 + - Codex + MiniMax flagged A2 phrasing — `ipNetToPhysicalPhysAddress` IS accessible. Fixed.
387 + - MiniMax + Codex flagged D1 description ambiguity (must say "ADD a fallback", not "fix existing lookup"). Fixed in acceptance criteria + Decision 3 + commit 5.
388 + - MiniMax flagged D4/D5/D6 explicit "Defer". Fixed (table now has scope column with explicit "Defer").
389 + - Kimi + Qwen + GLM flagged engine macro output format (lowercase hex, colon-separated, octet validation). Added to acceptance criteria.
390 + - Kimi flagged lenient-vendor regression fixture missing from commit 7. Added.
391 + - Codex flagged commit 5 file list incomplete (D1 also touches `topology_cache_fdb.go`). Fixed.
392 + - Codex flagged Decision 1 wording overclaim. Narrowed.
393 + - Codex + GLM flagged D2/E4 metric type undefined. Specified as internal counter + rate-limited log line, no chart.
394 + - Codex flagged vendor followup incomplete. Added EdgeSwitch, FortiSwitch, TiMOS, AOS6/7, VRP.
395 + - Codex flagged OpenNMS direct citation missing. Added.
396 + - Codex flagged sensitive-data checklist insufficient. Expanded to cover SNMPv3 creds, sysName/sysDescr/ifAlias/ifDescr, LLDP remote names/port descriptions, chassis IDs, mgmt addresses; added concrete grep one-liners.
397 + - GLM flagged E4 rate-limit "per-cycle, not per-row" missing from acceptance. Added.
398 + - GLM flagged profile-format.md MAX-ACCESS section should include audit recipe. Added.
399 + - GLM flagged engine `mac_address` output format parity (column-side vs index-side). Both produce `aa:bb:cc:dd:ee:ff`; verification in commit 1 unit tests.
400 +- All 20 fixes applied. SOW v3 produced.
401 +- Sent v3 to the same 5 reviewers (round 3). Findings consolidated:
402 + - 5 of 5 flagged **MAC output parity**: column-side `utils.go:54` uses `%02X` (uppercase); SOW spec requires `%02x` (lowercase). Existing tests at `collector_table_test.go:934-939` and `collector_device_meta_test.go:276-278` expect uppercase. Resolution (user pick A): converge to lowercase — flip column-side `utils.go:54` to `%02x`, update both test files, add explicit column-vs-index parity test. Folded into commit 1.
403 + - 4 of 5 flagged **pre-commit checklist gaps** (commit messages not in `git diff --cached`). Resolution: added `git log -1 --format='%B' | rg ...` post-commit verification, range scan via `git log upstream/master..HEAD`, and a separate PR-body check.
404 + - 2 of 5 (Codex, GLM) flagged **broken pre-commit IP regex** — used `(?!...)` PCRE lookahead under `grep -E` (POSIX ERE) → silently parses as a literal capture group → check passes spuriously. Resolution: switched the entire pre-commit checklist from `grep -E` to `rg -P` (ripgrep with PCRE).
405 + - Codex flagged **OID false positives** in the public-IP regex (SNMP OIDs like `1.3.6.1.4.1.x` look like dotted-quad IPs). Resolution: added `(?!1\.[0-3]\.6\.1\.)` exclusion and a non-digit-or-dot left-context guard so the regex only flags genuine dotted-quad addresses, not OID fragments.
406 + - Qwen flagged **A2 length-prefix path ambiguity** (does `index_transform` skip the byte, or does the formatter detect it?). Resolution: profile-level slicing via `index_transform` chosen — formatter stays pure (format-only). Explicit in commit 3 scope.
407 + - 2 of 5 (MiniMax, Codex wording) flagged **F10 detection rule should explicitly require remaining-6-octets validation**. Resolution: acceptance criterion now reads "7 components AND first equals 6 AND each remaining 6 components is a valid octet (0-255)".
408 + - GLM, Codex separately confirmed all 20 v2 fixes are present at the cited SOW lines.
409 +- All round-3 fixes applied to v4. Pre-commit checklist now uses `rg -P` (POSIX-incompatible — install ripgrep first) and has 6 sub-checks (uncommitted credentials, public IPv4 with OID exclusion, public IPv6, device strings, names from local env, commit messages, PR body).
410 +- SOW v4 ready for implementation.
411 +- Validation pass before implementation found two plan corrections:
412 + - `index_transform` needs variable-tail support (`start > 0` with omitted/zero `end`) before A1/A2/A3 can be represented cleanly. User accepted option A: extend engine semantics.
413 + - D1 VLAN fallback must be late at observation-output time, not an eager cache mutation, so a later real `dot1qVlanCurrentTable` mapping cannot be blocked.
414 +- Moved SOW from `pending/` to `current/`, changed status to `in-progress`, and started implementation.
415 +
416 +### 2026-05-02
417 +
418 +- Investigated local testing report that derived endpoints disappear briefly during some refreshes while SNMP devices and LLDP links remain visible.
419 +- Found the registered topology cache was used as the refresh write buffer and was cleared before replacement data was ready.
420 +- Changed refresh lifecycle to collect into an unregistered scratch cache and publish with `replaceWith()` only after full ingest/finalization.
421 +- Added a regression test that blocks refresh mid-collection and verifies the published snapshot still exposes prior FDB and ARP-derived endpoint data.
422 +- Addressed PR review thread `PRRT_kwDOAKPxd85_EfxH`: the IPv6 sensitive-data checklist regex contradicted its own `fc00::/7` exclusion and missed uppercase global-unicast IPv6 text. Fixed the checklist to scan `2000::/3` with `(?i)` case-insensitive matching and to exclude ULA.
423 +- Addressed SonarCloud duplication signal by collapsing duplicated Q-BRIDGE actual-profile tests into one table-driven test while preserving normal and length-prefixed MAC-index coverage.
424 +- User requested SOW close; marked status `completed` and moved the SOW to `done/`.
425 +- Addressed PR review thread `PRRT_kwDOAKPxd85_Eovf`: LLDP local management addresses were temporarily formatted as `ip_address`, which would drop valid non-IP LLDP management-address subtypes before topology normalization. Added index-derived `format: hex`, switched `lldpLocManAddr` to hex preservation, and added IPv4 plus non-IP-length actual-profile coverage.
426 +- Addressed PR review thread `PRRT_kwDOAKPxd85_Etpy`: `SOW_AUDIT_SENSITIVE_FULL_HISTORY=1` scanned fewer file types than `SOW_AUDIT_SENSITIVE_CHANGED=1`. Aligned full-history sensitive-data scanning with the changed-file code/config/documentation selector.
427 +
428 +## Validation
429 +
430 +Acceptance criteria evidence:
431 +
432 +- A1 Q-BRIDGE FDB MAC extraction is implemented in the actual shipped topology profile: `_std-topology-q-bridge-mib.yaml` derives `dot1q_fdb_mac` from the row index with `format: mac_address` and no `symbol.OID` for `dot1qTpFdbAddress`.
433 +- A2 IP-MIB ARP/ND index extraction is implemented in the actual shipped topology profile: `_std-topology-fdb-arp-mib.yaml` derives `arp_if_index` from `index: 1`, `arp_addr_type` from `index: 2` with top-level mapping, and `arp_ip` from `index_transform: [{start: 3}]` plus `format: ip_address`.
434 +- A3 LLDP local management address extraction is implemented in the actual shipped topology profile: `_std-topology-lldp-mib.yaml` anchors `lldpLocManAddrTable` on readable `lldpLocManAddrLen` and derives subtype/address from the index.
435 +- A3 preserves LLDP local management address bytes with `format: hex`; IP-compatible bytes are normalized by topology runtime, while non-IP subtype payloads are not dropped at collection time.
436 +- Engine support is implemented: `format: mac_address` works for index-derived values, accepts normal 6-octet suffixes and defensive 7-component length-prefixed suffixes, validates octets, and emits lowercase colon-separated MACs. Column-side MAC formatting is also lowercase for parity.
437 +- Runtime behavior is implemented: FDB rows with empty MAC increment a per-cycle drop counter; FDB rows with unmapped bridge ports increment a per-cycle diagnostic counter; VLAN attribution falls back to `fdbID` at observation-output time only after the mapping table has had a chance to populate.
438 +- Guardrails are implemented: `.agents/skills/project-snmp-profiles-authoring/SKILL.md`, the AGENTS.md Project Skills Index, and `src/go/plugin/go.d/collector/snmp/profile-format.md` now document the MAX-ACCESS rule and audit recipe.
439 +
440 +Tests or equivalent validation:
441 +
442 +- PASS: `cd src/go && go test -count=1 ./plugin/go.d/collector/snmp/ddsnmp/ddprofiledefinition`
443 +- PASS: `cd src/go && go test -count=1 ./plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector`
444 +- PASS: `cd src/go && go test -count=1 ./plugin/go.d/collector/snmp_topology`
445 +- PASS: `cd src/go && go test -count=1 ./pkg/topology/engine`
446 +- PASS: `cd src/go && go test -count=1 ./plugin/go.d/collector/snmp`
447 +- PASS: `git diff --check`
448 +- Added refresh-lifecycle regression coverage: `TestCollector_RefreshKeepsPublishedSnapshotWhileCollectionRuns` proves a registered cache is not cleared while a new collection is still running.
449 +- Added actual-profile collector tests loading `_std-topology-q-bridge-mib`, `_std-topology-fdb-arp-mib`, and `_std-topology-lldp-mib` through `ddsnmp.LoadProfileByName` and mocked strict-spec table walks. These verify the shipped YAML emits the index-derived Q-BRIDGE MAC, modern ARP IPv4/IPv6 fields, and LLDP local management address bytes, including a non-IP-length LLDP address payload.
450 +- Added focused unit tests for variable-tail `index_transform`, index-derived MAC formatting, length-prefixed MAC suffixes, invalid MAC octets, column-vs-index MAC parity, VLAN fallback, and FDB diagnostics.
451 +
452 +Real-use evidence:
453 +
454 +- Not run in this session. The originally reported affected Netgear GS110TP v3 live poll still needs access to that device and a fresh SNMP walk to compare `topology:snmp` FDB endpoint count against `dot1qTpFdbPort` row count. The SOW is completed by user request with this live-device check recorded as residual PR/local-testing risk, not as claimed validation evidence.
455 +
456 +Reviewer findings:
457 +
458 +- Pre-implementation review findings from the three multi-agent rounds are recorded in the execution log and were folded into the implementation plan before code changes.
459 +- No new external assistant review was run after implementation in this turn because the active repository instruction allows running external AI assistants only when the user asks for that explicitly.
460 +- PR review thread `PRRT_kwDOAKPxd85_EfxH` was valid and fixed: the IPv6 checklist now scans global-unicast-like `2000::/3` addresses case-insensitively and no longer includes ULA `fc00::/7`.
461 +- SonarCloud Quality Gate reported new-code duplication centered on `topology_profile_index_test.go`; the duplicated Q-BRIDGE test setup was made table-driven and revalidated with the focused `ddsnmpcollector` test package.
462 +- PR review thread `PRRT_kwDOAKPxd85_Eovf` was valid and fixed: LLDP local management address extraction now uses index `format: hex`, preserving non-IP subtype payloads for runtime normalization instead of dropping them during profile collection.
463 +- PR review thread `PRRT_kwDOAKPxd85_Etpy` was valid and fixed: full-history sensitive-data scanning now includes the same code/config/documentation file classes as changed-file scanning.
464 +
465 +Same-failure scan:
466 +
467 +- Command:
468 + `rg -n 'name:[[:space:]]*(dot1qTpFdbAddress|ipNetToPhysicalIfIndex|ipNetToPhysicalNetAddressType|ipNetToPhysicalNetAddress|lldpLocManAddrSubtype|lldpLocManAddr)\b|\.1\.3\.6\.1\.2\.1\.(17\.7\.1\.2\.2\.1\.1|4\.35\.1\.4\.1\.(1|3|4)|8802\.1\.1\.2\.1\.3\.8\.1\.(1|2))' src/go/plugin/go.d/config/go.d/snmp.profiles`
469 +- Result: four expected name-only hits remain, all in the corrected profiles and all without a `symbol.OID` for the not-accessible object:
470 + - `_std-topology-q-bridge-mib.yaml:23` — `dot1qTpFdbAddress`, index-derived MAC.
471 + - `_std-topology-fdb-arp-mib.yaml:206` — `ipNetToPhysicalNetAddressType`, `index: 2`.
472 + - `_std-topology-fdb-arp-mib.yaml:214` — `ipNetToPhysicalNetAddress`, index-derived IP.
473 + - `_std-topology-lldp-mib.yaml:97` — `lldpLocManAddr`, index-derived management address.
474 +
475 +Sensitive data gate:
476 +
477 +- `.agents/sow/audit.sh` sensitive-data guardrail reports: scanned durable artifact files, including completed SOWs under `done/`; no sensitive-data patterns found.
478 +- Targeted diff scan for credential assignments returned zero hits.
479 +- Targeted device-string scan returned only synthetic test constants (`00:11:22:33:44:55`) and profile tag names, not real device identities.
480 +- Targeted dotted-quad scan returned only TEST-NET documentation context and MAC-index numeric suffixes in tests; no raw customer IPs, SNMP communities, bearer tokens, SNMPv3 secrets, customer names, personal data, private endpoints, or proprietary incident details were added.
481 +
482 +Artifact maintenance gate:
483 +
484 +- AGENTS.md: updated Project Skills Index to include `.agents/skills/project-snmp-profiles-authoring/`.
485 +- Runtime project skills: added `.agents/skills/project-snmp-profiles-authoring/SKILL.md`.
486 +- Specs: no `.agents/sow/specs/` update. This change is an SNMP profile authoring/runtime rule and is recorded in the project skill plus `profile-format.md`; no separate durable product contract was changed.
487 +- End-user/operator docs: updated `src/go/plugin/go.d/collector/snmp/profile-format.md` with a Field Accessibility section and audit recipe.
488 +- End-user/operator skills: none affected. `docs/netdata-ai/skills/` and `src/ai-skills/` are not involved in SNMP profile authoring.
489 +- SOW lifecycle: moved from `pending/` to `current/` during implementation; moved from `current/` to `done/` with status `completed` after PR creation, regression fix, review-thread fix, and explicit user request to close. Live-device validation against the originally reported affected device was not independently runnable in this workspace; this is recorded as residual runtime confirmation, not hidden as completed evidence.
490 +
491 +Specs update:
492 +
493 +- No spec update was needed. The durable behavior rule for future work is procedural/authoring guidance, covered by the new runtime project skill and profile-format documentation.
494 +
495 +Project skills update:
496 +
497 +- Added `.agents/skills/project-snmp-profiles-authoring/SKILL.md`.
498 +
499 +End-user/operator docs update:
500 +
501 +- Updated `src/go/plugin/go.d/collector/snmp/profile-format.md`.
502 +
503 +End-user/operator skills update:
504 +
505 +- No output/reference skills were affected by this SNMP collector/profile change.
506 +
507 +Lessons:
508 +
509 +- `index_transform` needed an explicit variable-tail semantic; otherwise standards-compliant INDEX-derived fields cannot be represented cleanly without brittle duplicate tags.
510 +- While adding actual-profile tests, existing behavior was confirmed: mappings nested under `symbol:` are not applied to same-table column tags. This SOW does not change that broader contract; the required ARP address-type mapping is top-level, and changing the generic mapping behavior would alter existing tag outputs outside this fix.
511 +- Topology refresh must be double-buffered. The global registry is a read surface for function calls, so registered caches must not be used as mutable write buffers during SNMP collection.
512 +
513 +Follow-up mapping:
514 +
515 +- Implemented in this SOW: A1, A2, A3, D1, D2, E4, F10, G2, H1, H4.
516 +- Rejected for this SOW: changing generic same-table `symbol.mapping` behavior, because it is broader than the root-cause fix and can alter existing status tag outputs.
517 +- Out of scope for SOW-0001 and requiring separate user-approved SOWs if prioritized later: static FDB tables, IEEE8021-Q-BRIDGE-MIB FDB, modern `ipAddressTable`, vendor proprietary FDB MIBs, D3-D6 attribution work, F2/F3/F5 vendor quirks, G3/G4 diagnostics, and H3 snmprec fixture authoring docs.
518 +
519 +## Outcome
520 +
521 +Completed. Implementation, focused validation, refresh-regression fix, PR creation, first review-thread fix, Sonar duplication cleanup, and SOW lifecycle close are done. The originally reported device was not directly available from this workspace for an independent live walk/count comparison, so that specific live confirmation remains a residual PR/local-testing risk rather than a claimed validation result.
522 +
523 +## Lessons Extracted
524 +
525 +- Future SNMP profile work must verify source MIB `MAX-ACCESS` before adding `symbol.OID` entries.
526 +- Actual-profile collector tests are necessary here; formatter-only tests would not prove the shipped YAML emits the topology tags.
527 +
528 +## Followup
529 +
530 +Child SOWs to open after this one closes (one per item):
531 +
532 +- **Static FDB tables** (B1, B2, C1, C2): coordinated SOW covering both BRIDGE-MIB and Q-BRIDGE-MIB static unicast/multicast tables. Different semantics from learned FDB (filtering policy, multi-egress, allowed-port lists). Needs design.
533 +- **IEEE8021-Q-BRIDGE-MIB FDB** (B3, C3): modern alternative, increasingly relevant. INDEX includes `ComponentId`. Profile + selector design.
534 +- **Modern `ipAddressTable`** (B4, C4): for L3 devices using only the new IP-MIB. Same not-accessible column pattern.
535 +- **Vendor proprietary FDB MIBs** (C5): one SOW per vendor as real demand surfaces — Aruba/HPE (HP-ICF-BRIDGE), Huawei (HUAWEI-L2MAM-MIB / VRP `hwDynFdbPort`), Extreme (EXTREME-FDB-MIB), Nokia/Alcatel (ALCATEL-IND1-MAC-ADDRESS-MIB / TiMOS), Juniper (JUNIPER-VLAN/L2ALD-MIB), AlaxalA (AX-FDB-MIB), Ubiquiti EdgeSwitch, Fortinet FortiSwitch, Alcatel-Lucent OmniSwitch (AOS6/AOS7). LibreNMS handlers serve as references.
536 +- **D3-D6 attribution work**: FDB truncation detection, LLDP-vs-FDB deduplication, LACP/LAG rollup, cross-protocol freshness reconciliation.
537 +- **F2/F3/F5 vendor quirks**: Zyxel malformed-index reshape, TP-Link JetStream offset, Aruba IAP truncation.
538 +- **G3 fail-loud, G4 per-poll stats**: engine-level diagnostic improvements beyond E4's FDB-specific coverage.
539 +- **H3 snmprec fixture authoring docs**: how to derive sanitized fixtures from real walks.
540 +- (Resolved in this PR — was previously listed as a follow-up.) Engine `mac_address` output format parity: column-side (`utils.go:54`) and index-side `format: mac_address` are converged to lowercase `aa:bb:cc:dd:ee:ff` in commit 1, with an explicit column-vs-index parity unit test.
541 +
542 +## Regression - 2026-05-02
543 +
544 +### Derived endpoints disappear during refresh windows
545 +
546 +Observed symptom:
547 +
548 +- Local PR testing showed that SNMP devices and LLDP links could remain visible while all derived FDB/ARP endpoints disappeared for a few seconds, then reappeared.
549 +
550 +Root cause:
551 +
552 +- `refreshDeviceTopology()` called `getOrCreateDeviceCache()` at the start of a refresh.
553 +- `getOrCreateDeviceCache()` reset the registered topology cache in place: it cleared FDB, ARP, bridge-port, LLDP/CDP, interface, VLAN, and STP maps and set `lastUpdate` to zero before the SNMP walk and topology ingest had completed.
554 +- The topology function reads from the global registry concurrently with refreshes. During that window, readers could observe the registered cache after it had been cleared but before the replacement data was ready.
555 +
556 +Fix:
557 +
558 +- Refresh now builds the next device snapshot in an unregistered scratch cache.
559 +- The previously published cache remains visible to function readers during SNMP collection and ingest.
560 +- After the scratch cache is finalized, `replaceWith()` publishes it into the registered cache under the registered cache lock.
561 +
562 +Validation:
563 +
564 +- Added `TestCollector_RefreshKeepsPublishedSnapshotWhileCollectionRuns`, which blocks a refresh mid-collection and verifies the published snapshot still contains the previous FDB and ARP-derived endpoint evidence.
565 +- Re-ran the focused SOW 1 validation suite after the fix; results are recorded under `## Validation`.
.agents/sow/pending/SOW-0002-20260501-unified-multi-layered-topology-schema.md new
+285
@@ -0,0 +1,285 @@
1 +# SOW-0002 - Unified multi-layered topology schema and merge engine
2 +
3 +## Status
4 +
5 +Status: open
6 +
7 +Sub-state: scope captured, awaiting user decisions on merge semantics, identity-matching policy, conflict resolution, storage model, and scale targets before implementation planning advances.
8 +
9 +## Requirements
10 +
11 +### Purpose
12 +
13 +Make Netdata produce a single coherent topology view that unifies every source contributing topology evidence — at the same layer (e.g. multiple L2-speaking switches) and across layers (L2 ↔ L3 ↔ L7) — so users see one map of their network/infrastructure rather than four disconnected maps. This is foundational work: topology in Netdata is new, and the choice of merge semantics will shape every downstream consumer (UI, alerts, automation, AI agents).
14 +
15 +The work is fit-for-purpose for production observability on large enterprise networks where a single physical device routinely shows up under multiple identities at different layers (e.g. a server has a hostname, a Netdata machine GUID, several MAC addresses, multiple IP addresses, container IDs, Kubernetes pod/namespace identifiers, SNMP-discovered LLDP chassis ID).
16 +
17 +### User Request
18 +
19 +Verbatim user request: *"create a pending SOW for the Unified multi-layered topology schema, which should allow merging topologies of the same kind (L2 + L2), but also merge topologies of different kinds (L2 + L3, or L2 + L3 + L7)"*.
20 +
21 +This SOW captures the problem space, references existing prior planning (`TODO-UNIFIED-TOPOLOGY-SCHEMA.md`), and surfaces the decisions that must be locked before any implementation begins. It does not commit to a specific merge algorithm, identity-matching policy, or storage model — those are user decisions captured in `## Implications And Decisions` below.
22 +
23 +### Assistant Understanding
24 +
25 +Facts (verified):
26 +
27 +- The current schema already exists at `src/go/pkg/topology/types.go`. Each `Actor` has a `Layer` field, a `Source` field, and a `Match` struct with extended identity fields: `ChassisIDs`, `MacAddresses`, `IPAddresses`, `Hostnames`, `DNSNames`, `SysObjectID`, `SysName`, `NetdataNodeID`, `NetdataMachineGUID`, `CloudInstanceID`, `CloudAccountID`, `ContainerIDs`, `PodNames`, `NamespaceIDs` (`types.go:7-22`). Each `Link` carries `Layer`, `Protocol`, `LinkType`, source/destination endpoints, direction, state, timestamps, and metrics (`types.go:42-55`).
28 +- Topology data is produced today by four distinct sources (paths verified in repo):
29 + 1. **SNMP L2** — Go, in `src/go/pkg/topology/engine/` and `src/go/plugin/go.d/collector/snmp_topology/`. Produces actors of type `device` and `endpoint` with chassis IDs, MACs, IPs, sysName/sysObjectID. LLDP, CDP, FDB, ARP, STP.
30 + 2. **NetFlow / IPFIX / sFlow L3** — Rust, in `src/crates/netdata-netflow/`, plus Go plumbing in `src/go/`. Produces flow records with src/dst IP, ports, protocol, AS info, geolocation.
31 + 3. **Network-viewer L7** — C, in `src/collectors/network-viewer.plugin/`. eBPF-derived process-to-process connection observations with PIDs, container IDs, hostnames.
32 + 4. **Netdata streaming** — C, in `src/database/contexts/` and streaming subsystem. Produces parent ↔ child agent relationships with `NetdataNodeID`, `NetdataMachineGUID`.
33 +- A 6797-line prior-planning artifact exists at `TODO-UNIFIED-TOPOLOGY-SCHEMA.md` with extensive notes on the design space, source coverage, IP tracking policy, ASN/geo enrichment options, and several decision items already taken or pending. This SOW supersedes the unstructured TODO once committed; the TODO stays in place as historical reference until the SOW closes.
34 +- The codebase has helper code at `src/go/tools/topology-flow-merge/` and a per-poll merge implementation in `src/go/plugin/go.d/collector/snmp_topology/topology_output_merge.go` (sub-second scope: merging successive snapshots of the same SNMP topology). Cross-source / cross-layer merge does NOT exist today.
35 +- The `Match` struct is the right shape for cross-layer correlation in principle: each source contributes whatever identity fields it knows, and a downstream merge step can correlate any two actors that share at least one identity field. This is the "extended match fields, match on any overlapping field" principle stated in `TODO-UNIFIED-TOPOLOGY-SCHEMA.md:69-72`.
36 +
37 +Inferences:
38 +
39 +- "Same-kind merge" (L2 + L2) is the easier case: multiple SNMP-managed switches each produce their own L2 view; merging them means deduplicating actors by identity match and unioning their links. Most algorithmic complexity is in identity normalization (MAC formatting, hostname canonicalization, FQDN vs short-name).
40 +- "Cross-kind merge" (L2 + L3, or L2 + L3 + L7) is the harder case: actors are at different abstraction levels (a physical switch port at L2, an IP address at L3, a process at L7). Naive identity match works for some pairs (server's IP appears in both L3 flows and L2 ARP) but breaks for others (a process at L7 has no MAC; a switch at L2 has no PID). The schema must allow a parent-child or "lives-on" relationship rather than forcing every merge to a strict equality.
41 +- Conflict resolution policy is a real design problem: two sources legitimately disagree (e.g. an LLDP neighbor advertises a chassis ID that disagrees with what the device's own SNMP poll reports because of stale cache). Whichever policy is chosen affects every downstream consumer.
42 +- Performance and storage model interact: a memory-only merged view is simpler but limits historical queries and re-merge after configuration change; a persisted index enables more, at significant complexity cost.
43 +
44 +Unknowns (real, blocking design decisions):
45 +
46 +- Where does merge happen — agent-side per-source, agent-side cross-source, parent-side aggregating from multiple agents, or Cloud-side?
47 +- What's the identity-matching algorithm — strict any-overlap equality, canonicalized equality, probabilistic scoring with thresholds, or learned per-deployment?
48 +- What's the conflict-resolution policy when two sources disagree — newest wins, source-priority order, evidence-set union, or human-resolvable flag?
49 +- What scale targets must the design meet — actors-per-deployment, links-per-deployment, sources-per-actor, merge latency budget?
50 +- What's the storage and re-merge story — recompute on each request, persist a merged snapshot, persist an index of identities, or stream-update?
51 +- How does L7 process-level granularity surface in the merged graph without exploding actor count? (TODO-UNIFIED-TOPOLOGY-SCHEMA.md:1028 notes "L7 Detailed vs Aggregated Views" as an open question.)
52 +- How are stale entries aged out across sources whose freshness windows differ (LLDP cache ~120s, FDB ~5 min, NetFlow flow record duration, network-viewer eBPF connection lifetime)?
53 +
54 +### Acceptance Criteria
55 +
56 +(Final acceptance criteria are gated on user decisions in `## Implications And Decisions`. Until those decisions land, the criteria below are the high-level shape; they will be sharpened to specific verifiable outcomes once decisions are recorded.)
57 +
58 +- A single, documented unified topology schema is in production use by all four current sources (SNMP L2, NetFlow L3, network-viewer L7, Netdata streaming), with the existing `Match`/`Actor`/`Link` types as the canonical contract or a clearly evolved version.
59 +- A merge engine exists that takes N topology graphs (same-kind or cross-kind) and produces one merged graph following the chosen identity-match and conflict-resolution policies. Same-kind (L2 + L2) and cross-kind (L2 + L3, L2 + L3 + L7) cases each have explicit test coverage with expected merged output.
60 +- Identity-matching and conflict-resolution behavior is unit-tested with targeted fixtures covering: actors that should merge, actors that should NOT merge despite an incidental field overlap, sources that legitimately disagree, and stale entries aged across sources with different freshness windows.
61 +- A scale benchmark exists that exercises the merge engine at the chosen actors/links/sources targets and reports merge latency.
62 +- The merged graph can be served as a topology function output without breaking existing UI consumers; if a schema migration is needed, the migration plan is documented.
63 +- No customer-identifying data, community-member names, SNMP communities, bearer tokens, or PII appear in any committed artifact (SOW, code, code comments, tests, fixtures, commit messages, PR body).
64 +
65 +## Analysis
66 +
67 +Sources checked:
68 +
69 +- `src/go/pkg/topology/types.go` — current `Match`, `Actor`, `Link` schema.
70 +- `src/go/pkg/topology/engine/` — current SNMP L2 merge logic (within-source).
71 +- `src/go/plugin/go.d/collector/snmp_topology/topology_output_merge.go` — per-poll snapshot merge (within-source).
72 +- `src/go/tools/topology-flow-merge/` — standalone helper, not currently wired into the runtime path (see TODO-UNIFIED-TOPOLOGY-SCHEMA.md branch-cleanup audit).
73 +- `src/crates/netdata-netflow/` — L3 source.
74 +- `src/collectors/network-viewer.plugin/` — L7 source.
75 +- Streaming subsystem (parent/child agent topology) — under `src/database/contexts/` and adjacent.
76 +- `TODO-UNIFIED-TOPOLOGY-SCHEMA.md` — 6797 lines of prior thinking and partial decisions.
77 +- Adjacent TODOs not yet absorbed: `TODO-streaming-topology.md`, `TODO-TOPOLOGY-ENRICHMENT.md`, `TODO-TOPOLOGY-FLOWS-INCOMPLETE-INTEGRATIONS.md`, `TODO-topology-flows-sync.md`, `TODO-topology-library.md`, `TODO-topology-library-phase2-direct-port.md`, `TODO-topology-netflow-metadata.md`.
78 +
79 +Current state:
80 +
81 +- Schema shape is good. The extended-match principle is already encoded in `Match`. What's missing is the merge layer that uses these identity fields across sources.
82 +- Each source today produces its own topology output as a separate function. There is no merged endpoint.
83 +- Where merge does exist (same-source, per-poll snapshot fusion in the SNMP topology engine), it is implementation-internal — no shared abstraction, no reuse path for cross-source.
84 +- The codebase has hooks for layered presentation in the topology UI (the `PresentationActorType`, `PresentationLinkType` and friends in `types.go`), suggesting prior thinking about layered views, but no live data wiring.
85 +
86 +Risks (cross-cutting):
87 +
88 +- **Schema lock-in**: any merge engine that consumes the current `Match` shape effectively freezes that shape for downstream Cloud consumers. A future schema change costs two migrations (sources + merge + Cloud).
89 +- **Identity correlation false positives**: matching on "any overlapping field" is dangerous if a field is non-unique (e.g. private RFC1918 IP that recurs across customer subnets, hostname `localhost`). Without canonicalization and field-quality weighting, merge can fuse unrelated actors.
90 +- **Scale**: cross-layer merge expands actor count; L7 process granularity especially. If the design persists everything, storage grows; if it doesn't, historical queries degrade.
91 +- **Conflict noise**: two sources legitimately disagreeing produces user-visible warnings unless conflict resolution is automatic. Policy choice affects perceived data quality.
92 +- **Cross-version compatibility**: agents at different versions producing different schema versions. The merge engine must tolerate version drift gracefully.
93 +- **Sensitive data exposure**: merged topology surfaces hostnames, IPs, container/pod names, sysName, sysDescr — all potentially customer-identifying. The merge engine output is a public topology surface that the user sees; the produced data must not leak across tenants in any multi-tenant deployment scenario.
94 +
95 +## Pre-Implementation Gate
96 +
97 +Status: needs-user-decision
98 +
99 +Problem / root-cause model:
100 +
101 +- Topology evidence is produced by four distinct sources at three distinct layers (L2/L3/L7) plus a streaming hierarchy. Each source has its own view, none is reconciled with the others, and there is no merge engine that takes evidence from multiple sources and produces a single coherent graph. The schema (`Match`/`Actor`/`Link`) is already shaped for cross-layer correlation, but the runtime layer that performs the correlation does not exist. Implementation cannot start until merge semantics, identity-matching policy, conflict resolution, and storage model are locked.
102 +
103 +Evidence reviewed:
104 +
105 +- See "Sources checked" above. Direct evidence of the gap: no production code path correlates an SNMP-discovered L2 device with a NetFlow-observed L3 endpoint or a network-viewer-observed L7 process; the four topology functions are independent and produce four independent graphs.
106 +
107 +Affected contracts and surfaces:
108 +
109 +- `src/go/pkg/topology/types.go` — schema may evolve.
110 +- All four source producers (SNMP topology Go path, netflow Rust+Go, network-viewer C, streaming C) — output schema may need conformance changes.
111 +- Topology functions exposed to the UI — possibly a new "merged" function or the existing per-source functions extended.
112 +- Cloud-side consumers — the merged output schema becomes a Cloud contract.
113 +- Test fixtures — new cross-source fixtures required.
114 +- Documentation: `profile-format.md` is unrelated; topology UI documentation will need a section once merge ships.
115 +
116 +Existing patterns to reuse:
117 +
118 +- The `Match` extended-fields model in `types.go:7-22` is the right contract for identity. Reuse, possibly extend.
119 +- Per-source within-source merge logic in `topology_output_merge.go` and the SNMP topology engine — the same shape (deduplicate by identity, union links) likely generalizes; a candidate to lift to a shared package.
120 +- `src/go/tools/topology-flow-merge/` exists but is not wired in. Decide whether it's the seed of the cross-source merge engine or whether it gets retired.
121 +- The Netdata streaming hierarchy already establishes a parent/child actor relation — useful as a precedent for the "lives-on" cross-layer relationship.
122 +
123 +Risk and blast radius:
124 +
125 +- Schema evolution touches every consumer; needs a versioning story.
126 +- Cross-source merge is a new public-facing function; UI breakage risk if rolled out without a feature flag or staged rollout.
127 +- Identity-matching false positives are user-visible and erode trust faster than missing data; conservative defaults are safer.
128 +- Performance regressions on large deployments are likely if storage model isn't sized correctly.
129 +
130 +Sensitive data plan:
131 +
132 +- Same baseline as SOW-0001's plan: no community member names, no customer names, no SNMP communities, no bearer tokens, no SNMPv3 credentials, no customer-identifying IPs, hostnames, sysName/sysDescr/ifAlias/ifDescr, LLDP remote names, port descriptions, chassis IDs, management addresses, container/pod/namespace names.
133 +- Topology fixtures derived from real environments must be sanitized: replace customer-pointing strings with neutral labels (`switch-a`, `endpoint-1`, `service-x`).
134 +- Pre-commit checklist: same `rg -P` rules adopted for SOW-0001 (TBD after SOW-0001 cleanup pass), extended for the additional layer-3 / layer-7 surface (process names, container IDs, k8s pod/namespace names, ASN/geo data).
135 +- Cross-tenant data isolation in the merge engine itself is a runtime concern: the merge function output must respect Cloud tenant boundaries; this is an explicit acceptance check.
136 +
137 +Implementation plan:
138 +
139 +To be filled after the user decisions below are recorded. The plan will likely have these phases (illustrative, not committed):
140 +
141 +1. Lock the schema (extend `Match`/`Actor`/`Link` if needed; document version semantics).
142 +2. Build a shared `topology/merge` package implementing the chosen identity-match + conflict-resolution policy. Unit-test thoroughly with same-kind and cross-kind fixtures.
143 +3. Wire the merge engine behind a new topology function (`topology:unified` or similar). Keep per-source functions intact; the merged function is additive.
144 +4. Add Cloud-side consumer integration plus migration notes for existing UI surfaces.
145 +5. Add scale benchmark + observability counters (rows merged, conflicts detected, identity-match hit/miss rates).
146 +6. Iterate on real-deployment validation: at least two distinct deployments (different vendor mix, different layer mix) should produce coherent merged graphs.
147 +
148 +Validation plan:
149 +
150 +- Unit tests on the merge package with fixture matrices covering same-kind and cross-kind, identity overlaps and disagreements, age-out scenarios.
151 +- Integration test that wires a synthetic L2 + L3 + L7 input set and verifies the merged graph matches a hand-curated expected output.
152 +- Scale benchmark at the chosen targets.
153 +- Real-deployment validation in at least two environments.
154 +
155 +Artifact impact plan:
156 +
157 +- `AGENTS.md`: no expected change.
158 +- Runtime project skills: no expected change.
159 +- Specs under `.agents/sow/specs/`: a new spec documenting the unified-topology contract and identity-match algorithm is likely warranted on close (not at open).
160 +- End-user/operator docs: topology UI section(s) updated when the new function ships.
161 +- End-user/operator skills: none.
162 +- SOW lifecycle: this SOW absorbs `TODO-UNIFIED-TOPOLOGY-SCHEMA.md` once user decisions are recorded; the TODO stays in place as historical reference until this SOW closes.
163 +
164 +Open decisions: see `## Implications And Decisions` below — six decisions are outstanding.
165 +
166 +## Implications And Decisions
167 +
168 +The decisions below are unresolved and block implementation. Each is presented with options, pros/cons/implications/risks, and a recommendation that the user can accept or override.
169 +
170 +### Decision 1 — Where does the merge happen?
171 +
172 +**Options:**
173 +- **A. Agent-side, per-source.** Each source produces an already-merged-within-itself graph. No cross-source merge.
174 +- **B. Agent-side, cross-source on the same node.** The local agent merges all sources it produces. Cross-agent merge happens elsewhere (parent or Cloud).
175 +- **C. Parent-side aggregation.** Streaming parents merge children's contributions. Cloud receives an aggregated view.
176 +- **D. Cloud-side.** All raw evidence flows up, Cloud merges. Maximum flexibility, maximum bandwidth and storage cost.
177 +- **E. Hybrid.** Local same-source merge (A) + parent same-kind merge (C) + Cloud cross-kind merge (D).
178 +
179 +**Implications/Risks:** D leaks raw evidence to Cloud (cardinality concerns), C requires every parent to know all children's sources, A produces nothing useful for cross-source queries, B is the cleanest for single-host deployments but can't unify a fleet. E is the most flexible but has the largest blast radius.
180 +
181 +**Recommendation: B for now, plan E as the long-term shape.** Start by merging on the local agent for sources the local agent produces; defer cross-agent merge until single-agent merge is solid. This minimizes Cloud schema lock-in and lets the merge engine evolve based on real local-agent feedback.
182 +
183 +### Decision 2 — Identity-matching algorithm
184 +
185 +**Options:**
186 +- **A. Strict any-overlap equality on raw `Match` fields.** Two actors merge if any one of their `Match` slices intersects.
187 +- **B. Canonicalized equality.** MACs lowercased / colon-stripped, hostnames lowercased / FQDN-normalized, IPs canonicalized, before equality check.
188 +- **C. Field-quality-weighted scoring.** Each `Match` field has a discriminator weight (high for chassis ID, low for hostname `localhost`). Merge above a threshold.
189 +- **D. Probabilistic / learned per deployment.** A model decides; tunable per environment.
190 +
191 +**Implications/Risks:** A produces false positives on ambient identifiers (`localhost`, RFC1918 reuse). B handles 90% of false positives at low complexity. C handles ambiguity well but introduces a tuning knob users must understand. D is overkill for opening this work.
192 +
193 +**Recommendation: B as MVP; C as a follow-up SOW once real deployments produce telemetry on false-positive rates.** B is implementable, debuggable, and predictable.
194 +
195 +### Decision 3 — Conflict-resolution policy when sources disagree
196 +
197 +**Options:**
198 +- **A. Newest wins (timestamp-based).** Last evidence supersedes earlier.
199 +- **B. Source-priority order.** A defined priority — e.g. SNMP managed > LLDP-derived inferred > NetFlow-implied > network-viewer-implied. Highest priority wins.
200 +- **C. Evidence-set union, no resolution.** Both values are kept, exposed to the consumer as a multi-valued field.
201 +- **D. Human-resolvable flag.** Conflicts produce a UI warning, user picks.
202 +
203 +**Implications/Risks:** A loses provenance; B requires getting the priority right and is hard to change later; C grows attribute payloads without bound but never silently drops information; D produces UX friction.
204 +
205 +**Recommendation: C for `Match` fields, B for `Attributes`/`Derived` fields.** Identity should be inclusive (preserve all evidence so future merges can still match); attributes should be resolvable (one displayed value at a time). Provenance in either case is preserved as a per-value `source` tag.
206 +
207 +### Decision 4 — Storage model
208 +
209 +**Options:**
210 +- **A. Memory-only, recompute every request.** Simplest. No persistence.
211 +- **B. Persisted merged snapshot, recomputed on schedule.** One canonical view, served from cache.
212 +- **C. Persisted identity index.** Identity → actor lookup is persisted; the merged graph is computed on demand from raw source evidence using the index.
213 +- **D. Stream-updated.** Each source emits deltas; the merge engine maintains a live merged graph.
214 +
215 +**Implications/Risks:** A doesn't scale to large fleets; B trades freshness for cost; C is the right shape for query-heavy workloads but is a bigger build; D is a real-time pipeline with all the operational complexity that implies.
216 +
217 +**Recommendation: A for MVP within a single agent (minimal complexity, fast to ship), with the design ensuring the merge engine is deterministic so future migration to B or C is straightforward.**
218 +
219 +### Decision 5 — Scale targets
220 +
221 +**Options (illustrative):**
222 +- **A. Modest.** ≤ 1000 actors, ≤ 5000 links, ≤ 4 sources per actor.
223 +- **B. Mid.** ≤ 10000 actors, ≤ 50000 links, ≤ 8 sources per actor.
224 +- **C. High.** ≤ 100000 actors, ≤ 1M links, ≤ 16 sources per actor.
225 +
226 +**Implications/Risks:** Picking too small understates real enterprise networks; picking too large bloats the design. The right pick depends on whether L7 processes are first-class actors (then C) or aggregated to host level (then B).
227 +
228 +**Recommendation: B as initial target, with the design able to scale to C without re-architecture.** Validates against real enterprise mid-tier deployments and leaves room for L7-detail growth.
229 +
230 +### Decision 6 — L7 process granularity
231 +
232 +**Options:**
233 +- **A. L7 process is a first-class actor.** Each process gets its own actor; potentially explodes count.
234 +- **B. L7 aggregates to host actor with process-level attributes / sub-table.** One actor per host; processes are children or attributes.
235 +- **C. Two views: detailed and aggregated.** UI toggles between per-process and per-host.
236 +
237 +**Implications/Risks:** A is most informative but punishing at scale; B loses detail; C is the most flexible but requires two code paths. (TODO-UNIFIED-TOPOLOGY-SCHEMA.md:1028 notes this as an open item.)
238 +
239 +**Recommendation: B for MVP, C as a follow-up once UI and scale targets are validated.**
240 +
241 +## Plan
242 +
243 +Filled after Decisions 1-6 are recorded. Default skeleton (assuming the recommendations above):
244 +
245 +1. Lock and document the schema (extend `Match` if a decision requires it; otherwise leave as-is). Add a per-value `source` provenance tag mechanism to support Decision 3.
246 +2. Implement a `topology/merge` package on the agent side with the chosen identity-match (Decision 2) and conflict-resolution (Decision 3) policies. Unit tests cover same-kind and cross-kind matrices.
247 +3. Wire the merge engine behind a new topology function on the local agent. Per-source functions remain available unchanged.
248 +4. Sanitized fixture set covering at least: a 2-switch L2 same-kind merge, an L2 + L3 cross-kind merge, an L2 + L3 + L7 cross-kind merge with a host-level L7 aggregation per Decision 6.
249 +5. Scale benchmark at the chosen target (Decision 5).
250 +6. Documentation: add a "Topology unified view" section to user-facing docs; add a project-level spec under `.agents/sow/specs/` describing the unified contract and the merge algorithm.
251 +7. Real-deployment validation across two distinct environments before declaring done.
252 +
253 +## Execution Log
254 +
255 +### 2026-05-01
256 +
257 +- SOW opened in `pending/` per user request following completion of the third review round on SOW-0001.
258 +- Existing prior planning artifact (`TODO-UNIFIED-TOPOLOGY-SCHEMA.md`, 6797 lines) noted as historical context; this SOW is the canonical replacement once user decisions are locked.
259 +- Six open decisions recorded; no implementation work begins until they are resolved.
260 +
261 +## Validation
262 +
263 +Pending — gated on locked decisions and implementation.
264 +
265 +## Outcome
266 +
267 +Pending.
268 +
269 +## Lessons Extracted
270 +
271 +Pending until validation.
272 +
273 +## Followup
274 +
275 +Adjacent TODOs to absorb or supersede when this SOW progresses:
276 +
277 +- `TODO-streaming-topology.md` — streaming-source contributions to the merged graph.
278 +- `TODO-TOPOLOGY-ENRICHMENT.md` — geo / ASN / vendor enrichment as cross-cutting attributes.
279 +- `TODO-TOPOLOGY-FLOWS-INCOMPLETE-INTEGRATIONS.md` — flows-side integration gaps surfaced during the original split-PR cleanup.
280 +- `TODO-topology-flows-sync.md` — flows-side sync semantics.
281 +- `TODO-topology-library.md` — shared-library packaging considerations.
282 +- `TODO-topology-library-phase2-direct-port.md` — phase-2 port plan (likely a separate SOW once this one progresses).
283 +- `TODO-topology-netflow-metadata.md` — netflow metadata fields for cross-layer correlation.
284 +- `src/go/tools/topology-flow-merge/` — decide retirement vs absorb-as-merge-package-seed.
285 +- Possible child SOWs after this one progresses: identity-match scoring (Decision 2C as follow-up), persisted merge index (Decision 4C as follow-up), L7 detailed view (Decision 6C as follow-up), Cloud-side merge (Decision 1D/E as follow-up).
AGENTS.md
+39 -4
@@ -41,11 +41,39 @@ Before non-trivial work:
41
42 Assistants must not create git worktrees on their own. Create a git worktree only when the user explicitly asks for it or approves it.
43
44 +### Sensitive Data In Durable Artifacts
45 +
46 +SOWs, specs, documentation, project skills, agent instructions, and code comments are commit-ready artifacts. Treat them as public unless a repository-specific policy explicitly says otherwise.
47 +
48 +CRITICAL: Never write raw sensitive data to durable artifacts. This includes passwords, API keys, bearer tokens, SNMP communities, private keys, connection strings with embedded credentials, session cookies, community member names, customer names, customer identifiers, personal data, non-private IP addresses that can identify customers, private endpoints, account IDs, and proprietary incident details.
49 +
50 +Write only sanitized evidence:
51 +
52 +- use placeholders such as `[REDACTED_SECRET]`, `[CUSTOMER]`, `[ACCOUNT]`, `[PRIVATE_ENDPOINT]`;
53 +- use stable aliases such as `customer-a` only when the real mapping is not stored in the repository;
54 +- cite file paths, line numbers, command names, schema fields, or error classes instead of copying sensitive values;
55 +- summarize logs and traces; include only minimal redacted snippets.
56 +
57 +If sensitive data is required to continue, stop and ask the user for a secure handling path. If sensitive data is found in a durable artifact, sanitize it before any commit. If sensitive data was already committed, tell the user and do not rewrite history without explicit approval.
58 +
59 +### Open-Source Reference Evidence
60 +
61 +When SOW evidence comes from local mirrored open-source repositories under `/opt/baddisk/monitoring/repos/`, cite the upstream repository and checked commit instead of the workstation absolute path.
62 +
63 +Use:
64 +
65 +```text
66 +owner/repo @ commit
67 +relative/path/inside/repo:line
68 +```
69 +
70 +Resolve `owner/repo` from the repository remote, record the checked commit, and keep paths relative to the upstream repository root. Never write `/opt/baddisk/monitoring/repos/...` paths into SOW evidence.
71 +
72 ### Pre-Implementation Gate
73
74 Implementation must not begin until the active SOW contains a concrete `## Pre-Implementation Gate` section. Before moving a SOW from `pending/open` to `current/in-progress`, or before continuing implementation in an existing current SOW that lacks this section, fill the gate.
75
48 -The gate must record the problem/root-cause model, evidence reviewed, affected contracts and surfaces, existing patterns to reuse, risk and blast radius, implementation plan, validation plan, artifact impact plan, and open decisions. Generic placeholders such as `TBD`, `N/A`, or "to be checked later" are invalid unless the SOW explains why the item truly does not apply. If the gate exposes an unknown that cannot be resolved by investigation, stop and ask the user before implementation.
76 +The gate must record the problem/root-cause model, evidence reviewed, affected contracts and surfaces, existing patterns to reuse, risk and blast radius, sensitive data handling plan, implementation plan, validation plan, artifact impact plan, and open decisions. The sensitive data plan must cover SOWs, specs, documentation, project skills, agent instructions, and code comments. Generic placeholders such as `TBD`, `N/A`, or "to be checked later" are invalid unless the SOW explains why the item truly does not apply. If the gate exposes an unknown that cannot be resolved by investigation, stop and ask the user before implementation.
77
78 ### When A SOW Is Required
79
@@ -156,14 +184,18 @@ Map every remaining item to implemented, rejected, or tracked.
184
185 ### Regressions
186
187 +A regression is discovered after a SOW was considered completed or closed, later testing or use finds broken behavior, and the original SOW's claimed outcome is no longer true.
188 +
189 When behavior that a completed SOW claimed working stops working:
190
191 1. Find the original SOW in `done/`.
192 2. Move it back to `current/`.
163 -3. Mark it `in-progress` with a regression note.
164 -4. Add a `## Regression` section.
165 -5. Fix and validate there.
193 +3. Mark it `in-progress` with a regression note in `## Status`.
194 +4. Append a new dated `## Regression - YYYY-MM-DD` section at the end of the file, after the original outcome, lessons, and follow-up content.
195 +5. In that appended section, record what broke, evidence, why previous validation missed it, the repair plan, validation, and updates needed to specs, skills, docs, audits, or follow-up SOWs.
196 +6. Fix and validate there.
197
198 +Never prepend regression content above the original SOW narrative. The original requirements, analysis, plan, validation, outcome, lessons, and follow-up must remain readable first.
199 Do not create a new SOW for a true regression.
200
201 ### Validation Gate
@@ -235,6 +267,9 @@ Output/reference skills may also exist under product documentation or generated
267
268 Runtime input skills:
269
270 +- `.agents/skills/project-snmp-profiles-authoring/`
271 + Trigger: editing SNMP profile YAMLs, topology SNMP profiles, ddsnmp profile parsing, or SNMP profile-format documentation.
272 + Purpose: require MIB `MAX-ACCESS` checks and index-derived extraction for `not-accessible` INDEX objects.
273 - `.agents/skills/project-writing-collectors/`
274 Trigger: authoring or modifying any Netdata data-collection plugin or module (Go go.d / ibm.d, Rust crates, internal C plugins, external plugins via PLUGINSD). Read before adding a new collector, modifying an existing one, working on NetFlow/sFlow/IPFIX, OTEL ingestion, topology, SNMP profiles, or interactive Functions.
275 Status: live. Updates that close gaps or fix outdated pointers must ship in the same PR that exposed the issue.
src/go/AGENTS.md
+2
@@ -1,5 +1,7 @@
1 # Collector Authoring Checklist
2
3 +CRITICAL: Never write raw sensitive data to durable artifacts. This includes passwords, API keys, bearer tokens, SNMP communities, private keys, connection strings with embedded credentials, session cookies, community member names, customer names, customer identifiers, personal data, non-private IP addresses that can identify customers, private endpoints, account IDs, and proprietary incident details.
4 +
5 This file is a quick landing page for humans and AI assistants contributing to the IBM.d plugin. For the full documentation follow these links:
6
7 - [Plugin overview & build instructions](README.md)
src/go/plugin/go.d/collector/snmp/ddsnmp/ddprofiledefinition/validation.go
+1 -1
@@ -418,7 +418,7 @@ func validateEnrichMetricTag(metricTag *MetricTagConfig) error {
418 if transform.DropRight != 0 && transform.End != 0 {
419 errs = append(errs, fmt.Errorf("transform rule cannot define both end and drop_right. Invalid rule: %#v", transform))
420 }
421 - if transform.DropRight == 0 && transform.Start > transform.End {
421 + if transform.DropRight == 0 && transform.Start > transform.End && transform.End != 0 {
422 errs = append(errs, fmt.Errorf("transform rule end should be greater than start. Invalid rule: %#v", transform))
423 }
424 }
src/go/plugin/go.d/collector/snmp/ddsnmp/ddprofiledefinition/validation_test.go
+13
@@ -1116,6 +1116,19 @@ func Test_validateEnrichMetricTags(t *testing.T) {
1116 },
1117 },
1118 },
1119 + "raw index transform to tail": {
1120 + wantError: false,
1121 + metrics: []MetricTagConfig{
1122 + {
1123 + Tag: "fdb_mac",
1124 + IndexTransform: []MetricIndexTransform{
1125 + {
1126 + Start: 1,
1127 + },
1128 + },
1129 + },
1130 + },
1131 + },
1132 "raw index transform cannot combine end and drop_right": {
1133 wantError: true,
1134 metrics: []MetricTagConfig{
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_device_meta_test.go
+1 -1
@@ -274,7 +274,7 @@ func TestDeviceMetadataCollector_Collect(t *testing.T) {
274 )
275 },
276 expectedResult: map[string]ddsnmp.MetaTag{
277 - "mac_address": {Value: "00:50:56:AB:CD:EF", IsExactMatch: false},
277 + "mac_address": {Value: "00:50:56:ab:cd:ef", IsExactMatch: false},
278 },
279 expectedError: false,
280 },
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_table_test.go
+1 -1
@@ -935,7 +935,7 @@ func TestTableCollector_Collect(t *testing.T) {
935 {
936 Name: "devClientCount",
937 Value: 10,
938 - Tags: map[string]string{"mac_address": "00:50:56:AB:CD:EF"},
938 + Tags: map[string]string{"mac_address": "00:50:56:ab:cd:ef"},
939 MetricType: "rate",
940 IsTable: true,
941 Table: "devTable",
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/index_tag_value.go
+45
@@ -3,6 +3,7 @@
3 package ddsnmpcollector
4
5 import (
6 + "encoding/hex"
7 "fmt"
8 "strconv"
9 "strings"
@@ -50,11 +51,55 @@ func formatIndexTagValue(raw string, format string) (string, error) {
51 return raw, nil
52 case "ip_address":
53 return formatIndexIPAddress(raw)
54 + case "mac_address":
55 + return formatIndexMACAddress(raw)
56 + case "hex":
57 + return formatIndexHex(raw)
58 default:
59 return raw, nil
60 }
61 }
62
63 +func formatIndexHex(raw string) (string, error) {
64 + if strings.TrimSpace(raw) == "" {
65 + return "", fmt.Errorf("cannot convert transformed index '%s' to hex", raw)
66 + }
67 +
68 + parts := strings.Split(raw, ".")
69 +
70 + bytes := make([]byte, 0, len(parts))
71 + for _, part := range parts {
72 + n, err := strconv.Atoi(strings.TrimSpace(part))
73 + if err != nil || n < 0 || n > 255 {
74 + return "", fmt.Errorf("cannot convert transformed index '%s' to hex", raw)
75 + }
76 + bytes = append(bytes, byte(n))
77 + }
78 +
79 + return hex.EncodeToString(bytes), nil
80 +}
81 +
82 +func formatIndexMACAddress(raw string) (string, error) {
83 + parts := strings.Split(raw, ".")
84 + if len(parts) == 7 && strings.TrimSpace(parts[0]) == "6" {
85 + parts = parts[1:]
86 + }
87 + if len(parts) != 6 {
88 + return "", fmt.Errorf("cannot convert transformed index '%s' to MAC address", raw)
89 + }
90 +
91 + octets := make([]string, 0, len(parts))
92 + for _, part := range parts {
93 + n, err := strconv.Atoi(strings.TrimSpace(part))
94 + if err != nil || n < 0 || n > 255 {
95 + return "", fmt.Errorf("cannot convert transformed index '%s' to MAC address", raw)
96 + }
97 + octets = append(octets, fmt.Sprintf("%02x", n))
98 + }
99 +
100 + return strings.Join(octets, ":"), nil
101 +}
102 +
103 func formatIndexIPAddress(raw string) (string, error) {
104 if strings.Contains(raw, ":") {
105 if s, ok := canonicalIPAddressText(raw); ok {
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/index_tag_value_test.go
+125
@@ -5,6 +5,7 @@ package ddsnmpcollector
5 import (
6 "testing"
7
8 + "github.com/gosnmp/gosnmp"
9 "github.com/stretchr/testify/assert"
10 "github.com/stretchr/testify/require"
11
@@ -34,6 +35,130 @@ func TestTableRowProcessor_ProcessIndexTag_DropRightIPAddress(t *testing.T) {
35 assert.Equal(t, "192.0.2.1", tagValue)
36 }
37
38 +func TestTableRowProcessor_ProcessIndexTag_TailMACAddress(t *testing.T) {
39 + p := newTableRowProcessor(logger.New())
40 +
41 + tagName, tagValue, err := p.processIndexTag(ddprofiledefinition.MetricTagConfig{
42 + Tag: "fdb_mac",
43 + Symbol: ddprofiledefinition.SymbolConfigCompat{
44 + Name: "dot1qTpFdbAddress",
45 + Format: "mac_address",
46 + },
47 + IndexTransform: []ddprofiledefinition.MetricIndexTransform{
48 + {Start: 1},
49 + },
50 + }, "7.0.80.86.171.205.239")
51 +
52 + require.NoError(t, err)
53 + assert.Equal(t, "fdb_mac", tagName)
54 + assert.Equal(t, "00:50:56:ab:cd:ef", tagValue)
55 +}
56 +
57 +func TestTableRowProcessor_ProcessIndexTag_LengthPrefixedMACAddress(t *testing.T) {
58 + p := newTableRowProcessor(logger.New())
59 +
60 + _, tagValue, err := p.processIndexTag(ddprofiledefinition.MetricTagConfig{
61 + Tag: "fdb_mac",
62 + Symbol: ddprofiledefinition.SymbolConfigCompat{
63 + Name: "dot1qTpFdbAddress",
64 + Format: "mac_address",
65 + },
66 + IndexTransform: []ddprofiledefinition.MetricIndexTransform{
67 + {Start: 1},
68 + },
69 + }, "7.6.0.80.86.171.205.239")
70 +
71 + require.NoError(t, err)
72 + assert.Equal(t, "00:50:56:ab:cd:ef", tagValue)
73 +}
74 +
75 +func TestTableRowProcessor_ProcessIndexTag_InvalidMACAddress(t *testing.T) {
76 + p := newTableRowProcessor(logger.New())
77 +
78 + _, _, err := p.processIndexTag(ddprofiledefinition.MetricTagConfig{
79 + Tag: "fdb_mac",
80 + Symbol: ddprofiledefinition.SymbolConfigCompat{
81 + Name: "dot1qTpFdbAddress",
82 + Format: "mac_address",
83 + },
84 + IndexTransform: []ddprofiledefinition.MetricIndexTransform{
85 + {Start: 1},
86 + },
87 + }, "7.0.80.86.171.205.999")
88 +
89 + require.Error(t, err)
90 + assert.Contains(t, err.Error(), "cannot convert transformed index")
91 +}
92 +
93 +func TestFormatMACAddress_ColumnAndIndexParity(t *testing.T) {
94 + columnValue, err := convPhysAddressToString(gosnmp.SnmpPDU{
95 + Type: gosnmp.OctetString,
96 + Value: []byte{0x00, 0x50, 0x56, 0xab, 0xcd, 0xef},
97 + })
98 + require.NoError(t, err)
99 +
100 + indexValue, err := formatIndexTagValue("0.80.86.171.205.239", "mac_address")
101 + require.NoError(t, err)
102 +
103 + assert.Equal(t, columnValue, indexValue)
104 + assert.Equal(t, "00:50:56:ab:cd:ef", indexValue)
105 +}
106 +
107 +func TestTableRowProcessor_ProcessIndexTag_Hex(t *testing.T) {
108 + p := newTableRowProcessor(logger.New())
109 +
110 + tagName, tagValue, err := p.processIndexTag(ddprofiledefinition.MetricTagConfig{
111 + Tag: "lldp_loc_mgmt_addr",
112 + Symbol: ddprofiledefinition.SymbolConfigCompat{
113 + Name: "lldpLocManAddr",
114 + Format: "hex",
115 + },
116 + IndexTransform: []ddprofiledefinition.MetricIndexTransform{
117 + {Start: 2},
118 + },
119 + }, "1.4.10.0.0.1")
120 +
121 + require.NoError(t, err)
122 + assert.Equal(t, "lldp_loc_mgmt_addr", tagName)
123 + assert.Equal(t, "0a000001", tagValue)
124 +}
125 +
126 +func TestTableRowProcessor_ProcessIndexTag_HexPreservesNonIPLength(t *testing.T) {
127 + p := newTableRowProcessor(logger.New())
128 +
129 + _, tagValue, err := p.processIndexTag(ddprofiledefinition.MetricTagConfig{
130 + Tag: "lldp_loc_mgmt_addr",
131 + Symbol: ddprofiledefinition.SymbolConfigCompat{
132 + Name: "lldpLocManAddr",
133 + Format: "hex",
134 + },
135 + IndexTransform: []ddprofiledefinition.MetricIndexTransform{
136 + {Start: 2},
137 + },
138 + }, "6.6.0.80.86.171.205.239")
139 +
140 + require.NoError(t, err)
141 + assert.Equal(t, "005056abcdef", tagValue)
142 +}
143 +
144 +func TestTableRowProcessor_ProcessIndexTag_InvalidHex(t *testing.T) {
145 + p := newTableRowProcessor(logger.New())
146 +
147 + _, _, err := p.processIndexTag(ddprofiledefinition.MetricTagConfig{
148 + Tag: "lldp_loc_mgmt_addr",
149 + Symbol: ddprofiledefinition.SymbolConfigCompat{
150 + Name: "lldpLocManAddr",
151 + Format: "hex",
152 + },
153 + IndexTransform: []ddprofiledefinition.MetricIndexTransform{
154 + {Start: 2},
155 + },
156 + }, "1.4.10.0.0.999")
157 +
158 + require.Error(t, err)
159 + assert.Contains(t, err.Error(), "cannot convert transformed index")
160 +}
161 +
162 func TestTableRowProcessor_ProcessIndexTag_RegexMappedFamily(t *testing.T) {
163 p := newTableRowProcessor(logger.New())
164
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/table_row_processor.go
+8 -2
@@ -319,8 +319,12 @@ func (r *crossTableResolver) lookupValue(tagCfg ddprofiledefinition.MetricTagCon
319 return pdu, nil
320 }
321
322 -// applyTransform applies index transformation rules to extract a subset of the index
323 -// Example: index "1.6.0.36.155.53.3.246", transform [{start: 1, end: 7}] → "6.0.36.155.53.3.246"
322 +// applyTransform applies index transformation rules to extract a subset of the index.
323 +// Indices are 0-based; end is inclusive.
324 +// Examples:
325 +// - index "1.6.0.36.155.53.3.246", transform [{start: 1, end: 7}] → "6.0.36.155.53.3.246"
326 +// - index "7.0.80.86.171.205.239", transform [{start: 1}] → "0.80.86.171.205.239" (start>0, end==0 ⇒ to tail)
327 +// - index "1.4.10.0.0.1.99", transform [{start: 0, drop_right: 1}] → "1.4.10.0.0.1"
328 func (r *crossTableResolver) applyIndexTransform(index string, transforms []ddprofiledefinition.MetricIndexTransform) string {
329 if len(transforms) == 0 {
330 return index
@@ -336,6 +340,8 @@ func (r *crossTableResolver) applyIndexTransform(index string, transforms []ddpr
340 return ""
341 }
342 end = uint(len(parts) - int(transform.DropRight) - 1)
343 + } else if transform.Start > 0 && transform.End == 0 {
344 + end = uint(len(parts) - 1)
345 }
346
347 if int(start) >= len(parts) || end < start || int(end) >= len(parts) {
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/topology_profile_index_test.go new
+146
@@ -0,0 +1,146 @@
1 +// SPDX-License-Identifier: GPL-3.0-or-later
2 +
3 +package ddsnmpcollector
4 +
5 +import (
6 + "testing"
7 +
8 + "github.com/gosnmp/gosnmp"
9 + "github.com/stretchr/testify/require"
10 +
11 + "github.com/netdata/netdata/go/plugins/logger"
12 + "github.com/netdata/netdata/go/plugins/plugin/go.d/collector/snmp/ddsnmp"
13 +)
14 +
15 +func TestTopologyProfile_QBridgeFDBUsesMACFromIndex(t *testing.T) {
16 + for _, tc := range []struct {
17 + name string
18 + indexSuffix string
19 + }{
20 + {name: "normal_mac_index", indexSuffix: "7.0.80.86.171.205.239"},
21 + {name: "length_prefixed_mac_index", indexSuffix: "7.6.0.80.86.171.205.239"},
22 + } {
23 + t.Run(tc.name, func(t *testing.T) {
24 + ctrl, mockHandler := setupMockHandler(t)
25 + defer ctrl.Finish()
26 +
27 + expectSNMPWalk(mockHandler, gosnmp.Version2c, "1.3.6.1.2.1.17.7.1.2.2.1", []gosnmp.SnmpPDU{
28 + createIntegerPDU("1.3.6.1.2.1.17.7.1.2.2.1.2."+tc.indexSuffix, 5),
29 + createIntegerPDU("1.3.6.1.2.1.17.7.1.2.2.1.3."+tc.indexSuffix, 3),
30 + })
31 + expectSNMPWalk(mockHandler, gosnmp.Version2c, "1.3.6.1.2.1.17.7.1.4.2.1", nil)
32 +
33 + actual := collectTopologyProfileTables(t, mockHandler, "_std-topology-q-bridge-mib")
34 +
35 + assertTableMetricsEqual(t, []ddsnmp.Metric{qBridgeFDBMetric()}, actual)
36 + })
37 + }
38 +}
39 +
40 +func TestTopologyProfile_IPNetToPhysicalUsesIndexFields(t *testing.T) {
41 + ctrl, mockHandler := setupMockHandler(t)
42 + defer ctrl.Finish()
43 +
44 + expectSNMPWalk(mockHandler, gosnmp.Version2c, "1.3.6.1.2.1.31.1.1", nil)
45 + expectSNMPWalk(mockHandler, gosnmp.Version2c, "1.3.6.1.2.1.2.2", nil)
46 + expectSNMPWalk(mockHandler, gosnmp.Version2c, "1.3.6.1.2.1.10.7.2", nil)
47 + expectSNMPWalk(mockHandler, gosnmp.Version2c, "1.3.6.1.2.1.4.20", nil)
48 + expectSNMPWalk(mockHandler, gosnmp.Version2c, "1.3.6.1.2.1.17.1.4", nil)
49 + expectSNMPWalk(mockHandler, gosnmp.Version2c, "1.3.6.1.2.1.17.4.3", nil)
50 + expectSNMPWalk(mockHandler, gosnmp.Version2c, "1.3.6.1.2.1.4.35.1", []gosnmp.SnmpPDU{
51 + createPDU("1.3.6.1.2.1.4.35.1.4.2.1.4.10.0.2.10", gosnmp.OctetString, []byte{0x00, 0x50, 0x56, 0xab, 0xcd, 0xef}),
52 + createIntegerPDU("1.3.6.1.2.1.4.35.1.6.2.1.4.10.0.2.10", 1),
53 + createPDU("1.3.6.1.2.1.4.35.1.4.3.2.16.254.128.0.0.0.0.0.0.0.0.0.0.0.0.0.1", gosnmp.OctetString, []byte{0x00, 0x50, 0x56, 0xab, 0xcd, 0xf0}),
54 + createIntegerPDU("1.3.6.1.2.1.4.35.1.6.3.2.16.254.128.0.0.0.0.0.0.0.0.0.0.0.0.0.1", 2),
55 + })
56 + expectSNMPWalk(mockHandler, gosnmp.Version2c, "1.3.6.1.2.1.4.22", nil)
57 +
58 + actual := collectTopologyProfileTables(t, mockHandler, "_std-topology-fdb-arp-mib")
59 +
60 + assertTableMetricsEqual(t, []ddsnmp.Metric{
61 + {
62 + Name: "_topology_arp_entry",
63 + Value: 1,
64 + Tags: map[string]string{"arp_if_index": "2", "arp_addr_type": "ipv4", "arp_ip": "10.0.2.10", "arp_mac": "005056abcdef", "arp_state": "1"},
65 + MetricType: "gauge",
66 + IsTable: true,
67 + Table: "ipNetToPhysicalTable",
68 + },
69 + {
70 + Name: "_topology_arp_entry",
71 + Value: 2,
72 + Tags: map[string]string{"arp_if_index": "3", "arp_addr_type": "ipv6", "arp_ip": "fe80::1", "arp_mac": "005056abcdf0", "arp_state": "2"},
73 + MetricType: "gauge",
74 + IsTable: true,
75 + Table: "ipNetToPhysicalTable",
76 + },
77 + }, actual)
78 +}
79 +
80 +func TestTopologyProfile_LLDPManagementAddressUsesIndexFields(t *testing.T) {
81 + ctrl, mockHandler := setupMockHandler(t)
82 + defer ctrl.Finish()
83 +
84 + expectSNMPWalk(mockHandler, gosnmp.Version2c, "1.0.8802.1.1.2.1.3.7", nil)
85 + expectSNMPWalk(mockHandler, gosnmp.Version2c, "1.0.8802.1.1.2.1.3.8", []gosnmp.SnmpPDU{
86 + createIntegerPDU("1.0.8802.1.1.2.1.3.8.1.3.1.4.10.0.0.1", 4),
87 + createIntegerPDU("1.0.8802.1.1.2.1.3.8.1.4.1.4.10.0.0.1", 2),
88 + createIntegerPDU("1.0.8802.1.1.2.1.3.8.1.5.1.4.10.0.0.1", 12),
89 + createStringPDU("1.0.8802.1.1.2.1.3.8.1.6.1.4.10.0.0.1", "0.0"),
90 + createIntegerPDU("1.0.8802.1.1.2.1.3.8.1.3.6.6.0.80.86.171.205.239", 6),
91 + createIntegerPDU("1.0.8802.1.1.2.1.3.8.1.4.6.6.0.80.86.171.205.239", 2),
92 + createIntegerPDU("1.0.8802.1.1.2.1.3.8.1.5.6.6.0.80.86.171.205.239", 12),
93 + createStringPDU("1.0.8802.1.1.2.1.3.8.1.6.6.6.0.80.86.171.205.239", "0.0"),
94 + })
95 + expectSNMPWalk(mockHandler, gosnmp.Version2c, "1.0.8802.1.1.2.1.4.1", nil)
96 + expectSNMPWalk(mockHandler, gosnmp.Version2c, "1.0.8802.1.1.2.1.4.2", nil)
97 +
98 + actual := collectTopologyProfileTables(t, mockHandler, "_std-topology-lldp-mib")
99 +
100 + assertTableMetricsEqual(t, []ddsnmp.Metric{
101 + {
102 + Name: "_topology_lldp_loc_man_addr_entry",
103 + Value: 4,
104 + Tags: map[string]string{"lldp_loc_mgmt_addr_subtype": "1", "lldp_loc_mgmt_addr": "0a000001", "lldp_loc_mgmt_addr_if_subtype": "2", "lldp_loc_mgmt_addr_if_id": "12", "lldp_loc_mgmt_addr_oid": "0.0"},
105 + MetricType: "gauge",
106 + IsTable: true,
107 + Table: "lldpLocManAddrTable",
108 + },
109 + {
110 + Name: "_topology_lldp_loc_man_addr_entry",
111 + Value: 6,
112 + Tags: map[string]string{"lldp_loc_mgmt_addr_subtype": "6", "lldp_loc_mgmt_addr": "005056abcdef", "lldp_loc_mgmt_addr_if_subtype": "2", "lldp_loc_mgmt_addr_if_id": "12", "lldp_loc_mgmt_addr_oid": "0.0"},
113 + MetricType: "gauge",
114 + IsTable: true,
115 + Table: "lldpLocManAddrTable",
116 + },
117 + }, actual)
118 +}
119 +
120 +func collectTopologyProfileTables(t *testing.T, mockHandler gosnmp.Handler, profileName string) []ddsnmp.Metric {
121 + t.Helper()
122 +
123 + profile, err := ddsnmp.LoadProfileByName(profileName)
124 + require.NoError(t, err)
125 +
126 + missingOIDs := make(map[string]bool)
127 + tcache := newTableCache(0, 0)
128 + collector := newTableCollector(mockHandler, missingOIDs, tcache, logger.New(), false)
129 +
130 + var stats ddsnmp.CollectionStats
131 + actual, err := collector.collect(profile, &stats)
132 + require.NoError(t, err)
133 +
134 + return actual
135 +}
136 +
137 +func qBridgeFDBMetric() ddsnmp.Metric {
138 + return ddsnmp.Metric{
139 + Name: "_topology_qbridge_fdb_entry",
140 + Value: 5,
141 + Tags: map[string]string{"dot1q_fdb_id": "7", "dot1q_fdb_mac": "00:50:56:ab:cd:ef", "dot1q_fdb_bridge_port": "5", "dot1q_fdb_status": "3"},
142 + MetricType: "gauge",
143 + IsTable: true,
144 + Table: "dot1qTpFdbTable",
145 + }
146 +}
src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/utils.go
+1 -1
@@ -51,7 +51,7 @@ func convPhysAddressToString(pdu gosnmp.SnmpPDU) (string, error) {
51
52 parts := make([]string, 0, len(address))
53 for _, v := range address {
54 - parts = append(parts, fmt.Sprintf("%02X", v))
54 + parts = append(parts, fmt.Sprintf("%02x", v))
55 }
56 return strings.Join(parts, ":"), nil
57 }
src/go/plugin/go.d/collector/snmp/profile-format.md
+56
@@ -1230,6 +1230,62 @@ If the current row index is `1.192.0.2.1.1.128`, the collector:
1230 - formats it as an IP address
1231 - emits `neighbor="192.0.2.1"`
1232
1233 +### Field Accessibility
1234 +
1235 +SNMP profile symbols must only read objects that the source MIB exposes as
1236 +readable columns. Before adding or changing a `symbol.OID`, check the source
1237 +MIB object's `MAX-ACCESS` (SMIv2) or `ACCESS` (SMIv1).
1238 +
1239 +Rules:
1240 +
1241 +- `read-only`, `read-write`, and `read-create` objects can be read as
1242 + `symbol.OID` values.
1243 +- `not-accessible` objects must not be read as `symbol.OID` values.
1244 +- A `not-accessible` object that is part of a table `INDEX` can be derived from
1245 + the row OID index using `index` or `index_transform`.
1246 +- Keep SNMP index slicing in the profile YAML; keep format conversion in
1247 + `symbol.format`.
1248 +
1249 +The two index extraction mechanisms use different counting bases:
1250 +
1251 +- `index: N` is **1-based**: `index: 1` selects the first index component,
1252 + `index: 2` the second, and so on. Use this to pick a single component.
1253 +- `index_transform: [{start: M, end: K}]` is **0-based** over the row index
1254 + parts. `start: 0` is the first component. `end` is inclusive. Setting
1255 + `end: 0` together with `start: N > 0` slices to the tail (`start: N` to the
1256 + last index part) - useful for length-prefixed `OCTET STRING` index columns
1257 + whose width depends on a sibling index component (e.g.
1258 + `LLDP-MIB::lldpLocManAddr`, `IP-MIB::ipNetToPhysicalNetAddress`).
1259 +
1260 +So `index: 1` and `index_transform: [{start: 0, end: 0}]` both extract the
1261 +first index component.
1262 +
1263 +Examples:
1264 +
1265 +- `Q-BRIDGE-MIB::dot1qTpFdbAddress` is `not-accessible` and is part of the
1266 + `dot1qTpFdbEntry` index. Derive it from the row index and use
1267 + `format: mac_address`.
1268 +- `IP-MIB::ipNetToPhysicalIfIndex`,
1269 + `IP-MIB::ipNetToPhysicalNetAddressType`, and
1270 + `IP-MIB::ipNetToPhysicalNetAddress` are `not-accessible` index components.
1271 + Derive them from the row index. The physical MAC value,
1272 + `ipNetToPhysicalPhysAddress`, is readable and can stay as a column symbol.
1273 +- `LLDP-MIB::lldpLocManAddrSubtype` and `LLDP-MIB::lldpLocManAddr` are
1274 + `not-accessible` index components. Anchor the row on a readable column such as
1275 + `lldpLocManAddrLen`, then derive subtype and address from the row index. Use
1276 + `format: hex` for the address bytes so non-IP management-address subtypes are
1277 + preserved; topology normalization converts IP-compatible bytes later.
1278 +
1279 +Audit recipe:
1280 +
1281 +```bash
1282 +rg -n -C 4 'OBJECT-TYPE|MAX-ACCESS[[:space:]]+not-accessible|ACCESS[[:space:]]+not-accessible' path/to/MIB
1283 +rg -n 'name:[[:space:]]*(dot1qTpFdbAddress|ipNetToPhysicalIfIndex|ipNetToPhysicalNetAddressType|ipNetToPhysicalNetAddress|lldpLocManAddrSubtype|lldpLocManAddr)\b' src/go/plugin/go.d/config/go.d/snmp.profiles
1284 +```
1285 +
1286 +Any profile hit for a `not-accessible` object is valid only when the tag is
1287 +index-derived and does not declare a `symbol.OID` for that object.
1288 +
1289 ## Tag Transformation
1290
1291 Tag transformations let you **modify or extract parts of SNMP values** to produce clear, human-readable tags.
src/go/plugin/go.d/collector/snmp_topology/collector.go
+14 -27
@@ -134,8 +134,6 @@ func (c *Collector) Cleanup(context.Context) {
134
135 // refreshDeviceTopology collects topology data for a single device into its own cache.
136 func (c *Collector) refreshDeviceTopology(key string, dev ddsnmp.DeviceConnectionInfo) {
137 - cache := c.getOrCreateDeviceCache(key, dev)
138 -
137 snmpClient, err := newSNMPClientFromDeviceInfo(c.newSnmpClient, dev)
138 if err != nil {
139 c.Warningf("device '%s': failed to create SNMP client: %v", dev.Hostname, err)
@@ -169,50 +167,39 @@ func (c *Collector) refreshDeviceTopology(key string, dev ddsnmp.DeviceConnectio
167 return
168 }
169
172 - // Point c.topologyCache at this device's cache so the ingestion methods work.
173 - c.topologyCache = cache
170 + // Build the next snapshot off-registry. Function readers keep seeing the
171 + // previous complete snapshot until this collection is fully ingested.
172 + next := c.newDeviceCollectionCache(dev)
173 + c.topologyCache = next
174 + defer func() { c.topologyCache = nil }()
175
176 c.updateTopologyProfileTags(pms)
177 c.ingestTopologyProfileMetrics(pms)
178 c.collectTopologyVTPVLANContexts(dev)
179 c.finalizeTopologyCache()
180
180 - c.topologyCache = nil
181 + cache := c.getOrCreateDeviceCache(key)
182 + cache.mu.Lock()
183 + cache.replaceWith(next)
184 + cache.mu.Unlock()
185 }
186
183 -func (c *Collector) getOrCreateDeviceCache(key string, dev ddsnmp.DeviceConnectionInfo) *topologyCache {
187 +func (c *Collector) getOrCreateDeviceCache(key string) *topologyCache {
188 cache, ok := c.deviceCaches[key]
189 if !ok {
190 cache = newTopologyCache()
191 c.deviceCaches[key] = cache
192 snmpTopologyRegistry.register(cache)
193 }
194 + return cache
195 +}
196
191 - // Reset cache for fresh collection cycle.
192 - cache.mu.Lock()
197 +func (c *Collector) newDeviceCollectionCache(dev ddsnmp.DeviceConnectionInfo) *topologyCache {
198 + cache := newTopologyCache()
199 cache.updateTime = time.Now()
194 - cache.lastUpdate = time.Time{}
200 cache.staleAfter = c.refreshEvery() + time.Duration(c.UpdateEvery*2)*time.Second
201 cache.agentID = dev.Hostname
202 cache.localDevice = buildLocalTopologyDevice(dev)
198 - cache.lldpLocPorts = make(map[string]*lldpLocPort)
199 - cache.lldpRemotes = make(map[string]*lldpRemote)
200 - cache.cdpRemotes = make(map[string]*cdpRemote)
201 - cache.ifNamesByIndex = make(map[string]string)
202 - cache.ifStatusByIndex = make(map[string]ifStatus)
203 - cache.ifIndexByIP = make(map[string]string)
204 - cache.ifNetmaskByIP = make(map[string]string)
205 - cache.bridgePortToIf = make(map[string]string)
206 - cache.fdbEntries = make(map[string]*fdbEntry)
207 - cache.fdbIDToVlanID = make(map[string]string)
208 - cache.vlanIDToName = make(map[string]string)
209 - cache.vtpVersion = ""
210 - cache.stpBaseBridgeAddress = ""
211 - cache.stpDesignatedRoot = ""
212 - cache.stpPorts = make(map[string]*stpPortEntry)
213 - cache.arpEntries = make(map[string]*arpEntry)
214 - cache.mu.Unlock()
215 -
203 return cache
204 }
205
src/go/plugin/go.d/collector/snmp_topology/collector_refresh_test.go new
+152
@@ -0,0 +1,152 @@
1 +// SPDX-License-Identifier: GPL-3.0-or-later
2 +
3 +package snmptopology
4 +
5 +import (
6 + "testing"
7 + "time"
8 +
9 + "github.com/golang/mock/gomock"
10 + "github.com/gosnmp/gosnmp"
11 + snmpmock "github.com/gosnmp/gosnmp/mocks"
12 + "github.com/stretchr/testify/require"
13 +
14 + "github.com/netdata/netdata/go/plugins/plugin/go.d/collector/snmp/ddsnmp"
15 + "github.com/netdata/netdata/go/plugins/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector"
16 +)
17 +
18 +func TestCollector_RefreshKeepsPublishedSnapshotWhileCollectionRuns(t *testing.T) {
19 + previousRegistry := snmpTopologyRegistry
20 + registry := newTopologyRegistry()
21 + snmpTopologyRegistry = registry
22 + t.Cleanup(func() { snmpTopologyRegistry = previousRegistry })
23 +
24 + ctrl := gomock.NewController(t)
25 + defer ctrl.Finish()
26 +
27 + dev := ddsnmp.DeviceConnectionInfo{
28 + Hostname: "10.0.0.10",
29 + Port: 161,
30 + SysObjectID: "1.3.6.1.4.1.9.1.1",
31 + }
32 + mockHandler := snmpmock.NewMockHandler(ctrl)
33 + expectTopologyRefreshSNMPClient(mockHandler, dev)
34 +
35 + key := "10.0.0.10:161"
36 + published := newTopologyCache()
37 + seedPublishedEndpointSnapshot(published)
38 + registry.register(published)
39 +
40 + started := make(chan struct{})
41 + release := make(chan struct{})
42 + done := make(chan struct{})
43 +
44 + coll := New()
45 + coll.deviceCaches[key] = published
46 + coll.newSnmpClient = func() gosnmp.Handler { return mockHandler }
47 + coll.newDdSnmpColl = func(ddsnmpcollector.Config) ddCollector {
48 + return &blockingTopologyCollector{
49 + started: started,
50 + release: release,
51 + result: replacementEndpointProfileMetrics(),
52 + }
53 + }
54 +
55 + go func() {
56 + defer close(done)
57 + coll.refreshDeviceTopology(key, dev)
58 + }()
59 +
60 + <-started
61 +
62 + snapshot, ok := published.snapshotEngineObservations()
63 + require.True(t, ok)
64 + require.Len(t, snapshot.l2Observations, 1)
65 + require.Len(t, snapshot.l2Observations[0].FDBEntries, 1)
66 + require.Len(t, snapshot.l2Observations[0].ARPNDEntries, 1)
67 +
68 + close(release)
69 + <-done
70 +}
71 +
72 +type blockingTopologyCollector struct {
73 + started chan<- struct{}
74 + release <-chan struct{}
75 + result []*ddsnmp.ProfileMetrics
76 +}
77 +
78 +func (c *blockingTopologyCollector) Collect() ([]*ddsnmp.ProfileMetrics, error) {
79 + close(c.started)
80 + <-c.release
81 + return c.result, nil
82 +}
83 +
84 +func expectTopologyRefreshSNMPClient(mockHandler *snmpmock.MockHandler, dev ddsnmp.DeviceConnectionInfo) {
85 + mockHandler.EXPECT().SetTarget(dev.Hostname)
86 + mockHandler.EXPECT().SetPort(uint16(dev.Port))
87 + mockHandler.EXPECT().SetRetries(dev.Retries)
88 + mockHandler.EXPECT().SetTimeout(time.Duration(dev.Timeout) * time.Second)
89 + mockHandler.EXPECT().SetMaxOids(dev.MaxOIDs)
90 + mockHandler.EXPECT().SetMaxRepetitions(uint32(dev.MaxRepetitions))
91 + mockHandler.EXPECT().SetCommunity(dev.Community)
92 + mockHandler.EXPECT().SetVersion(gosnmp.Version2c)
93 + mockHandler.EXPECT().Connect().Return(nil)
94 + mockHandler.EXPECT().Close().Return(nil)
95 +}
96 +
97 +func seedPublishedEndpointSnapshot(cache *topologyCache) {
98 + now := time.Now()
99 + cache.updateTime = now
100 + cache.lastUpdate = now
101 + cache.staleAfter = time.Hour
102 + cache.agentID = "agent-1"
103 + cache.localDevice = topologyDevice{
104 + ManagementIP: "10.0.0.10",
105 + ChassisID: "00:11:22:33:44:55",
106 + ChassisIDType: "macAddress",
107 + SysName: "switch-a",
108 + }
109 + cache.bridgePortToIf["5"] = "5"
110 + cache.fdbEntries["00:50:56:ab:cd:ef|5||"] = &fdbEntry{
111 + mac: "00:50:56:ab:cd:ef",
112 + bridgePort: "5",
113 + status: "learned",
114 + }
115 + cache.arpEntries["5|10.0.0.20|00:50:56:ab:cd:ef"] = &arpEntry{
116 + ifIndex: "5",
117 + ip: "10.0.0.20",
118 + mac: "00:50:56:ab:cd:ef",
119 + addrType: "ipv4",
120 + }
121 +}
122 +
123 +func replacementEndpointProfileMetrics() []*ddsnmp.ProfileMetrics {
124 + return []*ddsnmp.ProfileMetrics{{
125 + HiddenMetrics: []ddsnmp.Metric{
126 + {
127 + Name: metricBridgePortMapEntry,
128 + Tags: map[string]string{
129 + tagBridgeBasePort: "5",
130 + tagBridgeIfIndex: "5",
131 + },
132 + },
133 + {
134 + Name: metricDot1qFdbEntry,
135 + Tags: map[string]string{
136 + tagDot1qFdbID: "7",
137 + tagDot1qFdbMac: "00:50:56:ab:cd:ef",
138 + tagDot1qFdbPort: "5",
139 + },
140 + },
141 + {
142 + Name: metricArpEntry,
143 + Tags: map[string]string{
144 + tagArpIfIndex: "5",
145 + tagArpIP: "10.0.0.20",
146 + tagArpMac: "005056abcdef",
147 + tagArpAddrType: "ipv4",
148 + },
149 + },
150 + },
151 + }}
152 +}
src/go/plugin/go.d/collector/snmp_topology/topology_cache.go
+2
@@ -28,6 +28,8 @@ type topologyCache struct {
28 fdbEntries map[string]*fdbEntry
29 fdbIDToVlanID map[string]string
30 vlanIDToName map[string]string
31 + fdbRowsDroppedNoMAC int
32 + fdbRowsUnmappedPort int
33 vtpVersion string
34 stpBaseBridgeAddress string
35 stpDesignatedRoot string
src/go/plugin/go.d/collector/snmp_topology/topology_cache_fdb.go
+1
@@ -9,6 +9,7 @@ func (c *topologyCache) updateFdbEntry(tags map[string]string) {
9
10 mac := normalizeMAC(firstNonEmpty(tags[tagFdbMac], tags[tagDot1qFdbMac]))
11 if mac == "" {
12 + c.fdbRowsDroppedNoMAC++
13 return
14 }
15
src/go/plugin/go.d/collector/snmp_topology/topology_cache_lifecycle.go
+37 -5
@@ -2,7 +2,10 @@
2
3 package snmptopology
4
5 -import "time"
5 +import (
6 + "strings"
7 + "time"
8 +)
9
10 func newTopologyCache() *topologyCache {
11 return &topologyCache{
@@ -43,6 +46,8 @@ func (c *topologyCache) replaceWith(src *topologyCache) {
46 c.fdbEntries = src.fdbEntries
47 c.fdbIDToVlanID = src.fdbIDToVlanID
48 c.vlanIDToName = src.vlanIDToName
49 + c.fdbRowsDroppedNoMAC = src.fdbRowsDroppedNoMAC
50 + c.fdbRowsUnmappedPort = src.fdbRowsUnmappedPort
51 c.vtpVersion = src.vtpVersion
52 c.stpBaseBridgeAddress = src.stpBaseBridgeAddress
53 c.stpDesignatedRoot = src.stpDesignatedRoot
@@ -61,12 +66,39 @@ func (c *topologyCache) hasFreshSnapshotAt(now time.Time) bool {
66 }
67
68 func (c *Collector) finalizeTopologyCache() {
64 - if c.topologyCache == nil {
69 + cache := c.topologyCache
70 + if cache == nil {
71 return
72 }
73
68 - c.topologyCache.mu.Lock()
69 - defer c.topologyCache.mu.Unlock()
74 + cache.mu.Lock()
75 + cache.updateFDBDiagnostics()
76 + droppedNoMAC := cache.fdbRowsDroppedNoMAC
77 + unmappedPort := cache.fdbRowsUnmappedPort
78 + agentID := cache.agentID
79 + cache.lastUpdate = cache.updateTime
80 + cache.mu.Unlock()
81
71 - c.topologyCache.lastUpdate = c.topologyCache.updateTime
82 + if droppedNoMAC > 0 {
83 + c.Warningf("device '%s': dropped %d topology FDB row(s) with empty MAC", agentID, droppedNoMAC)
84 + }
85 + if unmappedPort > 0 {
86 + c.Warningf("device '%s': observed %d topology FDB row(s) with bridge ports missing ifIndex mapping", agentID, unmappedPort)
87 + }
88 +}
89 +
90 +func (c *topologyCache) updateFDBDiagnostics() {
91 + c.fdbRowsUnmappedPort = 0
92 + for _, entry := range c.fdbEntries {
93 + if entry == nil || strings.TrimSpace(entry.mac) == "" {
94 + continue
95 + }
96 + bridgePort := strings.TrimSpace(entry.bridgePort)
97 + if bridgePort == "" || bridgePort == "0" {
98 + continue
99 + }
100 + if parseIndex(c.bridgePortToIf[bridgePort]) == 0 {
101 + c.fdbRowsUnmappedPort++
102 + }
103 + }
104 }
src/go/plugin/go.d/collector/snmp_topology/topology_cache_test.go
+54
@@ -623,6 +623,60 @@ func TestTopologyCache_Dot1qVLANEnrichment(t *testing.T) {
623 require.Equal(t, "70:49:a2:65:72:cd", obs.FDBEntries[0].MAC)
624 }
625
626 +func TestTopologyCache_Dot1qVLANFallbackUsesFDBIDWhenMapMissing(t *testing.T) {
627 + cache := newTopologyCache()
628 + cache.updateTime = time.Now()
629 + cache.lastUpdate = cache.updateTime
630 + cache.agentID = "agent1"
631 + cache.localDevice = topologyDevice{
632 + ChassisID: "00:11:22:33:44:55",
633 + ChassisIDType: "macAddress",
634 + ManagementIP: "10.0.0.1",
635 + }
636 +
637 + cache.updateBridgePortMap(map[string]string{
638 + tagBridgeBasePort: "7",
639 + tagBridgeIfIndex: "3",
640 + })
641 + cache.updateFdbEntry(map[string]string{
642 + tagDot1qFdbID: "100",
643 + tagDot1qFdbMac: "7049a26572cd",
644 + tagDot1qFdbPort: "7",
645 + tagDot1qFdbStatus: "learned",
646 + })
647 +
648 + obs := cache.buildEngineObservation(cache.localDevice)
649 + require.Len(t, obs.FDBEntries, 1)
650 + require.Equal(t, "100", obs.FDBEntries[0].VLANID)
651 +}
652 +
653 +func TestTopologyCache_FDBDiagnostics(t *testing.T) {
654 + cache := newTopologyCache()
655 +
656 + cache.updateFdbEntry(map[string]string{
657 + tagDot1qFdbID: "100",
658 + tagDot1qFdbPort: "7",
659 + tagDot1qFdbStatus: "learned",
660 + })
661 + require.Equal(t, 1, cache.fdbRowsDroppedNoMAC)
662 +
663 + cache.updateFdbEntry(map[string]string{
664 + tagDot1qFdbID: "100",
665 + tagDot1qFdbMac: "7049a26572cd",
666 + tagDot1qFdbPort: "7",
667 + tagDot1qFdbStatus: "learned",
668 + })
669 + cache.updateFDBDiagnostics()
670 + require.Equal(t, 1, cache.fdbRowsUnmappedPort)
671 +
672 + cache.updateBridgePortMap(map[string]string{
673 + tagBridgeBasePort: "7",
674 + tagBridgeIfIndex: "3",
675 + })
676 + cache.updateFDBDiagnostics()
677 + require.Equal(t, 0, cache.fdbRowsUnmappedPort)
678 +}
679 +
680 func TestTopologyCache_VTPVLANNameEnrichment(t *testing.T) {
681 cache := newTopologyCache()
682 cache.updateTime = time.Now()
src/go/plugin/go.d/collector/snmp_topology/topology_observation_local_forwarding.go
+5 -1
@@ -29,7 +29,11 @@ func (c *topologyCache) appendObservedFDBEntries(observation *topologyengine.L2O
29 ifIndex := parseIndex(c.bridgePortToIf[strings.TrimSpace(entry.bridgePort)])
30 vlanID := strings.TrimSpace(entry.vlanID)
31 if vlanID == "" && strings.TrimSpace(entry.fdbID) != "" {
32 - vlanID = strings.TrimSpace(c.fdbIDToVlanID[strings.TrimSpace(entry.fdbID)])
32 + fdbID := strings.TrimSpace(entry.fdbID)
33 + vlanID = strings.TrimSpace(c.fdbIDToVlanID[fdbID])
34 + if vlanID == "" {
35 + vlanID = fdbID
36 + }
37 }
38 observation.FDBEntries = append(observation.FDBEntries, topologyengine.FDBObservation{
39 MAC: strings.TrimSpace(entry.mac),
src/go/plugin/go.d/config/go.d/snmp.profiles/default/_std-topology-fdb-arp-mib.yaml
+10 -10
@@ -191,9 +191,7 @@ metrics:
191 name: _topology_arp_entry
192 metric_tags:
193 - tag: arp_if_index
194 - symbol:
195 - OID: 1.3.6.1.2.1.4.35.1.1
196 - name: ipNetToPhysicalIfIndex
194 + index: 1
195 - tag: arp_if_name
196 table: ifXTable
197 symbol:
@@ -203,18 +201,20 @@ metrics:
201 - start: 0
202 end: 0
203 - tag: arp_addr_type
204 + index: 2
205 symbol:
207 - OID: 1.3.6.1.2.1.4.35.1.2
206 name: ipNetToPhysicalNetAddressType
209 - mapping:
210 - 0: unknown
211 - 1: ipv4
212 - 2: ipv6
213 - 16: dns
207 + mapping:
208 + 0: unknown
209 + 1: ipv4
210 + 2: ipv6
211 + 16: dns
212 - tag: arp_ip
213 symbol:
216 - OID: 1.3.6.1.2.1.4.35.1.3
214 name: ipNetToPhysicalNetAddress
215 + format: ip_address
216 + index_transform:
217 + - start: 3
218 - tag: arp_mac
219 symbol:
220 OID: 1.3.6.1.2.1.4.35.1.4
src/go/plugin/go.d/config/go.d/snmp.profiles/default/_std-topology-lldp-mib.yaml
+13 -5
@@ -87,18 +87,17 @@ metrics:
87 OID: 1.0.8802.1.1.2.1.3.8
88 name: lldpLocManAddrTable
89 symbols:
90 - - OID: 1.0.8802.1.1.2.1.3.8.1.1
90 + - OID: 1.0.8802.1.1.2.1.3.8.1.3
91 name: _topology_lldp_loc_man_addr_entry
92 metric_tags:
93 - tag: lldp_loc_mgmt_addr_subtype
94 - symbol:
95 - OID: 1.0.8802.1.1.2.1.3.8.1.1
96 - name: lldpLocManAddrSubtype
94 + index: 1
95 - tag: lldp_loc_mgmt_addr
96 symbol:
99 - OID: 1.0.8802.1.1.2.1.3.8.1.2
97 name: lldpLocManAddr
98 format: hex
99 + index_transform:
100 + - start: 2
101 - tag: lldp_loc_mgmt_addr_if_subtype
102 symbol:
103 OID: 1.0.8802.1.1.2.1.3.8.1.4
@@ -180,6 +179,15 @@ metrics:
179 name: lldpRemSysCapEnabled
180 format: hex
181
182 + # TODO: lldpRemManAddrTable still reads INDEX columns directly:
183 + # - .1.1 lldpRemManAddrSubtype (not-accessible per LLDP-MIB)
184 + # - .1.2 lldpRemManAddr (not-accessible per LLDP-MIB)
185 + # Standards-conformant LLDP agents will not return these columns. The two
186 + # blocks below are kept as vendor-specific fallbacks (MikroTik / XS1930)
187 + # that historically expose them anyway. Follow-up: rebuild both blocks the
188 + # same way as lldpLocManAddrTable - anchor on a readable column (.1.3
189 + # lldpRemManAddrIfSubtype) and derive subtype/address from the row index
190 + # via index_transform with format: hex.
191 - MIB: LLDP-MIB
192 table:
193 OID: 1.0.8802.1.1.2.1.4.2
src/go/plugin/go.d/config/go.d/snmp.profiles/default/_std-topology-q-bridge-mib.yaml
+3 -2
@@ -20,9 +20,10 @@ metrics:
20 index: 1
21 - tag: dot1q_fdb_mac
22 symbol:
23 - OID: 1.3.6.1.2.1.17.7.1.2.2.1.1
23 name: dot1qTpFdbAddress
25 - format: hex
24 + format: mac_address
25 + index_transform:
26 + - start: 1
27 - tag: dot1q_fdb_bridge_port
28 symbol:
29 OID: 1.3.6.1.2.1.17.7.1.2.2.1.2
src/go/plugin/ibm.d/AGENTS.md
+3 -1
@@ -1,5 +1,7 @@
1 # IBM.d Plugin Developer Guide
2
3 +CRITICAL: Never write raw sensitive data to durable artifacts. This includes passwords, API keys, bearer tokens, SNMP communities, private keys, connection strings with embedded credentials, session cookies, community member names, customer names, customer identifiers, personal data, non-private IP addresses that can identify customers, private endpoints, account IDs, and proprietary incident details.
4 +
5 This guide is for developers contributing to the IBM.d plugin. For end-user documentation, see [README.md](./README.md).
6
7 ## Architecture Overview
@@ -143,4 +145,4 @@ The flag implicitly enables dump mode and exits once every job has produced at l
145 - Each module provides safe stock health alarms in `src/health/health.d/`.
146 - The plugin supports dynamic configuration through the Netdata Agent.
147
146 -For questions or suggestions, open a GitHub issue or reach out on Netdata's community channels.
\ No newline at end of file
148 +For questions or suggestions, open a GitHub issue or reach out on Netdata's community channels.