[zhihao1998]

Summary

• Opt-in telemetry: [netflow, prometheus, syslog] on Kathara and Containerlab environments (nika env run --telemetry or the same field on a task).
• Attach one collector (telemetry_collector) on an out-of-band management network (Kathara) or the lab management network (Containerlab). Probe exporters at runtime and configure only devices that support the requested kind.
• Agents read IPFIX/sFlow with netflow_query, device metrics with prometheus_query, and device logs with syslog_query. SDN, P4, SR Linux, XRd, and RouterOS export through the same tools.

Rebased onto current dev (the earlier cherry-pick of these commits on dev was reverted). Devices without an exporter stay unconfigured instead of aborting the lab, so mixed-vendor and INT-collector nodes do not block deploy. Compatibility is probed per device at lab check; there is no image-digest capability cache yet.

Test plan

• Unit tests for NetFlow/Prometheus/syslog query APIs, MCP tool catalog, attach, and lab-neutral names
• Live Kathara MCP tool path (tests/nika/service/test_mcp_tools_live.py)
• Live FRR/SDN/P4/Containerlab/vendor telemetry attach and query
• Full diagnosis with byo.langgraph + Qwen3.8-27B-FP8: dc_clos link_down called syslog_query; frr_service_down called prometheus_query and syslog_query; both submitted the correct root cause
• Confirm CI on this branch against current dev

Fixes #217

[github-actions]

Documentation review: FAIL

Reviewed all three changed human-facing documents: docs/agents/mcp-servers.md, docs/benchmarks/benchmark-configuration.md, and docs/operations/network-scenarios.md. No README files changed. This review covers documentation only; source reads were limited to checking documented interfaces and examples.

docs/operations/network-scenarios.md

• [blocking] Lines 678–685: Give readers a valid benchmark example and say where it goes. The indented ... is not valid YAML here. Removing it still leaves a bare list without the cases: wrapper required by load_benchmark_yaml, and problem: link_down requires a nonempty inject mapping. The example also does not identify a file or command to run it. This violates “keep simplification accurate,” “keep essential contracts visible,” and the human-first requirement for runnable examples. Change: provide one complete minimal benchmark configuration, its filename, and the matching uv run nika benchmark run --config … command. Alternatively, explicitly label this as an excerpt, replace the ellipsis with a YAML comment, and link directly to the schema explaining its placement and required fields.

• [blocking] Lines 687–713: Give telemetry compatibility one clear home. The exporter table and the long paragraph at line 713 repeat the same per-platform inventory; metric families are then listed again in the MCP reference. Readers must sift through exporter processes, NIC placement, and repeated metric lists to discover practical limitations such as unsupported device/type combinations producing no data. This violates “give each idea one home,” “keep sections in proportion,” and “choose structure for readability.” Change: keep the enablement examples, use one compact platform/type compatibility table that makes unsupported combinations explicit, retain the subnet/empty-data caveats in short paragraphs, and link to the MCP reference for metric names and queries. Separate any necessary management-network implementation detail from the main setup path rather than repeating the inventory.

• [optional] Lines 715–720: Illustrate an actual subnet change. The text says to choose another range when 172.28.0.0/16 conflicts, but the example repeats that default. Following the principle of concrete, useful examples, show a different illustrative subnet and state that it must be unused in the reader's environment.

docs/agents/mcp-servers.md

• [optional] Line 60: Make the PromQL examples easier to scan. The opening sentence packs Linux, FRR, OVS, P4, SR Linux, XRd, and RouterOS queries into one long catalog. Applying “choose structure for readability,” consider a small platform/query/purpose table, with time-window and label semantics in a separate paragraph. The documented tool parameters and metric names checked against the sources are consistent.

docs/benchmarks/benchmark-configuration.md

No findings in the changed text; the new telemetry link resolves to the added section.

Merge verdict: FAIL — the two blocking findings above remain.

[github-actions]

Documentation review: FAIL

Reviewed the changed human-facing documentation at 59868f35:

• docs/agents/mcp-servers.md
• docs/benchmarks/benchmark-configuration.md
• docs/operations/cli-reference.md
• docs/operations/configuration.md
• docs/operations/network-scenarios.md

No README files changed. Applied the NIKA documentation guidelines: reader purpose, clear structure and conventional terminology, accurate commands and paths, working links, minimal duplication, and sufficient prerequisites and examples. Source reads were limited to verifying documentation claims; this is not a code review or runtime validation.

docs/operations/network-scenarios.md

• [blocking] Line 730: Distinguish syslog delivery from gNMI configuration. “Prometheus and syslog scrape SR Linux over gNMI” incorrectly describes how logs reach the collector. SR Linux pushes syslog over UDP port 514; gNMI configures its remote logging destination. A reader troubleshooting missing logs would be directed to the wrong collection protocol. This violates “keep simplification accurate” and “use conventional network-engineering language.”

  Change: replace the final sentence with: “Prometheus collects SR Linux metrics through gNMI on the lab management network. SR Linux sends syslog to the collector over UDP port 514 on that network; NIKA configures the logging destination through gNMI.” The transport is specified in srl.enable_syslog.

The previous findings are resolved: the benchmark example now has valid YAML, required injection fields, a filename and a run command; platform support has one compatibility table with explicit unsupported combinations; management details are separated; the subnet example uses an alternative range; and PromQL examples are tabulated.

Other reviewed files

No findings in the changed text of docs/agents/mcp-servers.md, docs/benchmarks/benchmark-configuration.md, docs/operations/cli-reference.md, or docs/operations/configuration.md.

Merge verdict: FAIL — one blocking documentation correction remains.

[github-actions]

Documentation review: PASS

Reviewed the four changed human-facing docs at 8b17da32 (no README files changed):

• docs/agents/mcp-servers.md
• docs/benchmarks/benchmark-configuration.md
• docs/operations/cli-reference.md
• docs/operations/network-scenarios.md

Applied the documentation guidelines: reader purpose first, clear headings and conventional terminology, accurate commands and paths, working links, one home per concept, and sufficient prerequisites and examples. No blocking or optional findings remain in the changed documentation.

Previous findings

The remaining blocker is resolved: network-scenarios.md, line 724 now correctly distinguishes Prometheus collection through gNMI from syslog delivery over UDP 514, with gNMI used to configure the logging destination. This satisfies “keep simplification accurate” and “use conventional network-engineering language.” The runnable benchmark example, compatibility table, and tabulated PromQL examples also retain the earlier corrections.

Documentation-only review; source reads were limited to checking documented interfaces and examples. No runtime tests or code review performed.

Merge verdict: PASS.