feat(topology): SNMP LLDP/CDP and NetFlow/IPFIX/sFlow v5 collectors with topology functions - #21702

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/go/plugin/go.d/collector/netflow/collector.go">

<violation number="1" location="src/go/plugin/go.d/collector/netflow/collector.go:117">
P2: Init discards the caller context and always uses context.Background(), so module cancellation from the framework won't propagate to the collector goroutines.</violation>

<violation number="2" location="src/go/plugin/go.d/collector/netflow/collector.go:158">
P2: Cleanup ignores the caller’s context and uses context.Background(), which can block shutdowns when the framework expects cancellation/timeout to be honored.</violation>
</file>

<file name="src/go/plugin/go.d/collector/netflow/flow_aggregator.go">

<violation number="1" location="src/go/plugin/go.d/collector/netflow/flow_aggregator.go:255">
P2: maxBuckets enforcement can fail when the incoming bucket is older than all existing buckets, allowing the map to exceed the configured limit.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/crates/netdata-netflow/netflow-plugin/src/network_sources.rs">

<violation number="1" location="src/crates/netdata-netflow/netflow-plugin/src/network_sources.rs:178">
P2: Silent failure on lock poisoning: if `state.by_source.write()` fails due to lock poisoning, records are silently dropped without logging. Consider at minimum logging a warning when the lock cannot be acquired.</violation>
</file>

<file name="src/crates/netdata-netflow/netflow-plugin/src/plugin_config.rs">

<violation number="1" location="src/crates/netdata-netflow/netflow-plugin/src/plugin_config.rs:984">
P2: The gRPC address validation only checks for any ':' when no scheme is provided, so IPv6 literals without a port pass validation even though the error message requires host:port. This lets invalid configs through and will fail later at connection time.</violation>
</file>

<file name="src/go/plugin/go.d/collector/snmp/topology_cache_test.go">

<violation number="1" location="src/go/plugin/go.d/collector/snmp/topology_cache_test.go:203">
P3: The helper treats any non-nil attribute type as a valid non-empty list, so the test can pass even when the attribute is the wrong type or empty. To keep the test meaningful, only return true for non-empty list types and treat unknown types as missing.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/web/api/functions/function-streaming.c">

<violation number="1" location="src/web/api/functions/function-streaming.c:18">
P1: Avoid unbounded variable-length stack allocations from the user-controlled `function` string; a large request can exhaust the stack and crash the agent. Use a bounded buffer with a length check or heap allocation with proper cleanup.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/go/plugin/go.d/collector/snmp/integrations/snmp_devices.md">

<violation number="1" location="src/go/plugin/go.d/collector/snmp/integrations/snmp_devices.md:208">
P2: The security description for `Snmp:topology` incorrectly claims it only exposes interface names and traffic counters, but this function returns LLDP/CDP topology data (devices, links, management addresses). This is misleading for users reading the documentation.</violation>

<violation number="2" location="src/go/plugin/go.d/collector/snmp/integrations/snmp_devices.md:209">
P2: The availability description references interface data caching instead of LLDP/CDP topology data, which doesn’t match this function’s purpose. It should reference topology cache readiness.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/crates/journalctl/src/main.rs">

<violation number="1" location="src/crates/journalctl/src/main.rs:152">
P2: Guard against relative times that underflow the UNIX epoch; the current subtraction can go negative and wrap when cast to u64.</violation>
</file>

<file name="src/crates/journal-session/src/cursor.rs">

<violation number="1" location="src/crates/journal-session/src/cursor.rs:103">
P2: Broken rustdoc intra-doc link: `Cursor::fields` does not exist. The method is `payloads()`, so the link target should be `Cursor::payloads`.</violation>

<violation number="2" location="src/crates/journal-session/src/cursor.rs:475">
P2: This `expect()` panics in library code, which violates the project convention ("Avoid panics in library paths; return typed errors instead"). Additionally, the panic message references `fields()` but the method is named `payloads()`. Return a typed `SessionError` variant instead.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/go/plugin/go.d/agent/jobmgr/manager.go">

<violation number="1" location="src/go/plugin/go.d/agent/jobmgr/manager.go:174">
P2: The new topology alias registration isn't mirrored in cleanup; `topology:*` aliases remain registered after shutdown, leaving stale handlers if the manager is re-initialized. Add corresponding unregistration for the alias names.</violation>
</file>

<file name="src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_table.go">

<violation number="1" location="src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_table.go:330">
P1: Caching auxiliary tables (tables with no symbols, used only for cross-table tags) prevents them from being re-walked. However, dependent tables that *do* need walking (e.g., due to cache expiry or first run) rely on `walkedData` to resolve cross-table tags. Since `walkedData` only contains the results of the *current* walk, skipping the auxiliary table means its data is missing from `walkedData`, causing tag resolution to fail for the dependent table.

The fix is to ensure auxiliary tables are always present in `walkedData` when needed. The safest way with the current architecture is to NOT cache them (remove the `cacheData` call), forcing them to be walked every cycle. Alternatively, the cross-table resolution logic would need to be updated to fallback to reading from the cache.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/go/tools/topology-parity-evidence/main.go">

<violation number="1">
P2: Assertion detection uses `rawLine`, so `assert*` tokens in comments or string literals are counted as real assertions, which can inflate the inventory and create false parity gaps. Scan `cleanLine` (the comment/string-stripped version) instead.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/go/pkg/topology/engine/parity/evidence/office-live-reliability-report.md">

<violation number="1" location="src/go/pkg/topology/engine/parity/evidence/office-live-reliability-report.md:6">
P2: Avoid committing real SNMP community strings in documentation; replace with a redacted placeholder to prevent leaking credentials.</violation>
</file>

<file name="src/go/plugin/go.d/collector/snmp/topology_registry.go">

<violation number="1" location="src/go/plugin/go.d/collector/snmp/topology_registry.go:177">
P2: L3 topology responses skip augmentLocalActorFromCache, so local device attributes (management addresses, capabilities, sys_descr, etc.) are missing compared with L2. Consider augmenting the L3 data before returning.</violation>

<violation number="2" location="src/go/plugin/go.d/collector/snmp/topology_registry.go:182">
P2: Merged topology responses skip augmentLocalActorFromCache, so local device attributes are dropped in the combined L2/L3 output. Consider augmenting the merged data before returning.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/go/pkg/topology/engine/topology_adapter.go">

<violation number="1">
P1: Segment actors are always appended to the output regardless of identity-key deduplication. Unlike device and endpoint actors which `continue` (skip) when keys are empty or already present in `actorIndex`, this code only gates `addTopologyIdentityKeys` but not the `append`. This will produce duplicate actors when identity keys overlap.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="TODO-topology-library-phase2-direct-port.md">

<violation number="1">
P1: Do not commit real SNMP community strings in documentation. Replace `atadteN` with a redacted placeholder (e.g., `<community>`) everywhere it appears to avoid leaking credentials.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/go/plugin/go.d/collector/snmp/topology_cache.go">

<violation number="1">
P3: The first loop over `c.fdbEntries` (setting `vlanName`) is entirely redundant — the second loop immediately below iterates the same entries with the same filter and performs the same `vlanName` assignment in addition to setting `vlanID`. Remove the first loop to avoid the unnecessary O(n) pass.</violation>
</file>

<file name="src/go/pkg/topology/engine/topology_adapter.go">

<violation number="1">
P2: Inconsistent string literals: `HasPrefix` checks against `"macaddress:"` but slicing uses `len("macAddress:")`. While both are currently 11 bytes, this is a maintenance hazard — any future rename could silently break the offset. Use a single constant or the same literal for both, as the `management_ip:` case does.</violation>

<violation number="2">
P2: Missing overflow guard for `float64` → `int` conversion. The `int64` case properly checks `math.MaxInt`, but the `float64` case converts directly with `int(typed)`, which yields implementation-defined behavior for large values. Add a bounds check consistent with the `int64` branch.</violation>
</file>

<file name="src/go/plugin/go.d/config/go.d/snmp.profiles/default/_std-topology-cisco-vtp-mib.yaml">

<violation number="1">
P2: The vtpVersion scalar OID is missing the `.0` instance suffix, so the SNMP GET will not resolve and the vtp_version tag will be empty.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/go/pkg/topology/engine/topology_adapter.go">

<violation number="1">
P2: `"protocols"` and `"protocols_collected"` are set to the exact same value (`labelsCSVToSlice(dev.Labels, "protocols_observed")`). If they are meant to represent different concepts, one likely has the wrong source label key. If they are intentionally identical, this is redundant data that will increase payload size for every actor.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/crates/netdata-netflow/netflow-plugin/src/decoder/protocol/ipfix/templates/data.rs">

<violation number="1" location="src/crates/netdata-netflow/netflow-plugin/src/decoder/protocol/ipfix/templates/data.rs:17">
P2: field_count is untrusted and used directly for Vec::with_capacity, which can trigger oversized allocations on malformed/truncated packets. Cap the capacity by the remaining bytes (similar to the options template path) to avoid a memory-exhaustion DoS.</violation>
</file>

<file name="src/crates/netdata-netflow/netflow-plugin/src/decoder/protocol/ipfix/special/packet.rs">

<violation number="1" location="src/crates/netdata-netflow/netflow-plugin/src/decoder/protocol/ipfix/special/packet.rs:49">
P2: Guard against `consumed == 0` before advancing `cursor`; otherwise a zero-length template/record will cause an infinite loop in the parser.</violation>
</file>

<file name=".github/scripts/prepare-topology-ip-intel-stock.sh">

<violation number="1" location=".github/scripts/prepare-topology-ip-intel-stock.sh:94">
P3: Use an EXIT trap for temp directory cleanup; RETURN won’t fire when this script exits, so the temp dir can be left behind.</violation>
</file>

<file name="src/crates/netdata-netflow/netflow-plugin/src/decoder/protocol/shared/merge/enrich.rs">

<violation number="1" location="src/crates/netdata-netflow/netflow-plugin/src/decoder/protocol/shared/merge/enrich.rs:77">
P2: Merge the ASN name fields alongside `src_as`/`dst_as` so incoming enrichment doesn’t get dropped during record merge.</violation>

<violation number="2" location="src/crates/netdata-netflow/netflow-plugin/src/decoder/protocol/shared/merge/enrich.rs:95">
P3: Merge geo latitude/longitude fields so enrichment from the incoming record isn’t lost during merge.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/go/plugin/go.d/collector/snmp/topology_cache_lldp.go">

<violation number="1" location="src/go/plugin/go.d/collector/snmp/topology_cache_lldp.go:59">
P2: Guard against an empty remIndex before building the remote key. Without this, multiple remote entries on the same local port can overwrite each other in the cache.</violation>
</file>

<file name="src/go/pkg/topology/engine/node_topology_router.go">

<violation number="1" location="src/go/pkg/topology/engine/node_topology_router.go:118">
P2: Edge IDs are keyed only by a single IP ID, so edges that share that IP (e.g., the same interface appearing in both the point‑to‑point and prefix subnet loops) will collide and be dropped by edgeByID. Use a composite ID that includes both endpoints (or subnet + endpoint) to keep edges unique.</violation>

<violation number="2" location="src/go/pkg/topology/engine/node_topology_router.go:143">
P2: Subnet edge IDs are only the target IP ID, so edges that share that interface can overwrite each other in edgeByID. Include the subnet ID (or other endpoint info) in the edge ID to keep them unique.</violation>
</file>

<file name="src/go/pkg/topology/engine/l2_pipeline_match_lldp.go">

<violation number="1" location="src/go/pkg/topology/engine/l2_pipeline_match_lldp.go:244">
P1: Self-loop guard falsely drops valid links when both `localSysName` and `remoteSysName` are empty. `"" == ""` is always true, so any link where neither side advertises a sysName is silently discarded as a self-loop. The sysName check should require at least one side to be non-empty.</violation>
</file>

<file name="src/go/pkg/topology/engine/topology_adapter_pruning.go">

<violation number="1" location="src/go/pkg/topology/engine/topology_adapter_pruning.go:213">
P3: `pruneUnlinkedEndpointActors` is never called, so this new function is dead code and adds maintenance overhead. Remove it or wire it into the pruning flow if it is meant to run.</violation>
</file>

<file name="src/go/pkg/topology/engine/node_topology_subnet.go">

<violation number="1" location="src/go/pkg/topology/engine/node_topology_subnet.go:14">
P1: The IPv6 point-to-point mask constant is incorrect (`...:fffd`), so valid /127 IPv6 links are not detected as point-to-point.</violation>
</file>

<file name="src/go/pkg/topology/engine/l2_pipeline_remote_resolution.go">

<violation number="1" location="src/go/pkg/topology/engine/l2_pipeline_remote_resolution.go:29">
P1: MAC compatibility checks are skipped when management IP is missing, which can incorrectly merge different devices that share a hostname.</violation>
</file>

<file name="src/go/pkg/topology/engine/topology_adapter_endpoint_device_hints.go">

<violation number="1" location="src/go/pkg/topology/engine/topology_adapter_endpoint_device_hints.go:24">
P1: When no hints match managed devices, this branch returns unmanaged hints instead of an empty result, bypassing managed-only fallback logic.</violation>
</file>

<file name="src/go/plugin/go.d/collector/snmp/topology_observation_identity.go">

<violation number="1" location="src/go/plugin/go.d/collector/snmp/topology_observation_identity.go:34">
P2: When an existing ID is found, the resolver returns without registering the new aliases (host/chassis/IP). This can cause later observations of the same device under a different alias to create a duplicate device ID. Consider registering the new aliases before returning.</violation>
</file>

<file name="src/go/pkg/topology/engine/topology_adapter_identity_assignment.go">

<violation number="1" location="src/go/pkg/topology/engine/topology_adapter_identity_assignment.go:200">
P3: `canonicalTopologyMACListKey` is unused in the codebase, so this new function is dead code. Remove it or wire it into the canonicalization flow to avoid carrying untested logic.</violation>
</file>

<file name="src/go/pkg/topology/engine/topology_adapter_collections.go">

<violation number="1" location="src/go/pkg/topology/engine/topology_adapter_collections.go:45">
P2: Deduplicate canonicalized endpoint IPs before returning; mapped and unmapped IPv4 forms can currently produce duplicate entries.</violation>
</file>

<file name="src/go/plugin/go.d/collector/snmp/topology_metadata_helpers.go">

<violation number="1" location="src/go/plugin/go.d/collector/snmp/topology_metadata_helpers.go:90">
P2: Canonical-key collisions are resolved using randomized map iteration, so metadata values can change nondeterministically when equivalent label aliases are both present.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/go/plugin/go.d/collector/snmp/ddsnmp/ddsnmpcollector/collector_table.go">

<violation number="1">
P1: Do not mark a walked table OID as missing when the walk is just empty; this permanently suppresses future collection for dynamic tables.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/go/pkg/topology/engine/topology_adapter_bridge_links_observation.go">

<violation number="1" location="src/go/pkg/topology/engine/topology_adapter_bridge_links_observation.go:22">
P3: This newly added switch-facing key-set helper chain is currently unused, so the logic is dead code and adds maintenance overhead without affecting runtime behavior.</violation>
</file>

<file name="src/go/plugin/go.d/collector/snmp/topology_output_cleanup.go">

<violation number="1" location="src/go/plugin/go.d/collector/snmp/topology_output_cleanup.go:121">
P2: Trim link actor IDs before checking membership in actorSet; otherwise links with whitespace in SrcActorID/DstActorID are incorrectly removed as dangling.</violation>
</file>

<file name="src/go/pkg/topology/engine/topology_adapter_builder.go">

<violation number="1" location="src/go/pkg/topology/engine/topology_adapter_builder.go:239">
P2: The second `pruneSegmentArtifacts` call discards its suppressed count (`_`), so `segments_suppressed` in the stats may undercount. If the stat should reflect total suppressed segments across both passes, accumulate both counts.</violation>
</file>

<file name="src/go/pkg/topology/engine/topology_adapter_bridge_ports.go">

<violation number="1" location="src/go/pkg/topology/engine/topology_adapter_bridge_ports.go:90">
P2: `bridgeAttachmentSortKey` omits VLAN/method fields, so attachment ordering can be non-deterministic before dedupe and may retain inconsistent records when duplicates differ only by VLAN/method.</violation>
</file>

<file name="src/go/plugin/go.d/collector/snmp/topology_output_map_type.go">

<violation number="1">
P3: This `!topologyActorIsInferred(actor)` branch is unreachable for endpoints and should be removed to avoid misleading logic in map-type filtering.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/go/plugin/go.d/collector/snmp/topology_observation_device_identity.go">

<violation number="1" location="src/go/plugin/go.d/collector/snmp/topology_observation_device_identity.go:23">
P2: Using a constant fallback `"local-device"` can collapse multiple unidentified devices into one topology actor during multi-snapshot aggregation. The fallback should be uniquely scoped (for example by agent/job or a deterministic per-device fingerprint).</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/go/plugin/go.d/collector/snmp/topology_metadata_values.go">

<violation number="1">
P2: Canonical-key collisions are resolved by first-seen map iteration, which is nondeterministic. This can produce unstable metadata values when multiple label keys normalize to the same canonical alias.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/go/pkg/topology/engine/l2_pipeline_address_normalization.go">

<violation number="1" location="src/go/pkg/topology/engine/l2_pipeline_address_normalization.go:14">
P2: Reject all-zero MAC in `canonicalBridgeAddr` to avoid invalid bridge-address matching in STP resolution.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/crates/netdata-netflow/netflow-plugin/src/decoder/protocol/shared/merge/enrich.rs">

<violation number="1" location="src/crates/netdata-netflow/netflow-plugin/src/decoder/protocol/shared/merge/enrich.rs:76">
P2: Synchronize ASN name when ASN is merged; current logic can keep stale `*_as_name` values that no longer match the merged `*_as`.

(Based on your team's feedback about clearing stale ASN names and allowing explicit ASN name overrides.) [FEEDBACK_USED]</violation>
</file>

<file name="src/crates/netdata-netflow/netflow-plugin/src/decoder/protocol/ipfix/templates/data.rs">

<violation number="1" location="src/crates/netdata-netflow/netflow-plugin/src/decoder/protocol/ipfix/templates/data.rs:18">
P2: Skip `field_count == 0` template records before persisting; otherwise an empty template can overwrite valid state for the same template ID.</violation>
</file>

<file name="src/crates/netdata-netflow/netflow-plugin/proto/cmd/ris/api/ris.proto">

<violation number="1" location="src/crates/netdata-netflow/netflow-plugin/proto/cmd/ris/api/ris.proto:10">
P1: Remove the empty `{}` block (or remove the trailing `;`) from the RPC definitions. The current `rpc ... {} ;` syntax is invalid proto and will break code generation.</violation>
</file>

<file name="src/crates/netdata-netflow/netflow-plugin/src/decoder/protocol/legacy.rs">

<violation number="1" location="src/crates/netdata-netflow/netflow-plugin/src/decoder/protocol/legacy.rs:49">
P2: V5 records compute flow start/end timestamps but never write them into the flow record, so exported `FLOW_START_USEC`/`FLOW_END_USEC` remain unset.</violation>
</file>

<file name="src/crates/netdata-netflow/netflow-plugin/src/decoder/protocol/shared/merge/identity.rs">

<violation number="1" location="src/crates/netdata-netflow/netflow-plugin/src/decoder/protocol/shared/merge/identity.rs:25">
P2: Update identity_index after a merge that changes identity fields; otherwise later incoming flows in the same batch can miss the updated identity and be appended as duplicates.</violation>
</file>

<file name="src/crates/journal-log-writer/src/log/mod.rs">

<violation number="1" location="src/crates/journal-log-writer/src/log/mod.rs:359">
P2: write_entry_with_timestamps passes the caller’s timestamp overrides to the remapping entry, so when new mappings are emitted the remapping entry consumes the override and the real entry gets clamped to +1µs. That makes the override ineffective whenever remapping occurs. Use default/current timestamps for remapping entries instead.</violation>
</file>

<file name=".github/scripts/prepare-topology-ip-intel-stock.sh">

<violation number="1" location=".github/scripts/prepare-topology-ip-intel-stock.sh:97">
P3: The synthetic ASN CSV row has an unquoted comma in the org name, so the organization is truncated during parsing.</violation>
</file>

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/crates/journal-log-writer/src/log/mod.rs">

<violation number="1" location="src/crates/journal-log-writer/src/log/mod.rs:359">
P2: Passing `EntryTimestamps::default()` here discards caller-provided entry timestamp overrides. When a remapping entry is emitted, it advances the realtime clock to “now”, so the next entry’s overridden timestamps get clamped forward and no longer reflect the historical values the caller supplied.</violation>
</file>

<file name="src/crates/netdata-netflow/netflow-plugin/src/decoder/protocol/shared/merge/enrich.rs">

<violation number="1" location="src/crates/netdata-netflow/netflow-plugin/src/decoder/protocol/shared/merge/enrich.rs:59">
P1: Guard ASN-name fallback assignment so a name is only merged when it matches the destination ASN; otherwise you can attach the wrong label to an existing ASN.

(Based on your team's feedback about keeping ASN names aligned with ASN replacement/override semantics.) [FEEDBACK_USED]</violation>
</file>