On Thu, 15 Jan 2026 19:03:24 +0000 Bruce Richardson <[email protected]> wrote:
> TL;DR > ------ > > For a quick demo, apply patches, run e.g. testpmd and then in a separate > terminal run: > > ./usertools/dpdk-telemetry-watcher.py -d1T eth.tx > > Output, updated once per second, will be traffic rate per port e.g.: > > Connected to application: "dpdk-testpmd" > Time /ethdev/stats,0.opackets /ethdev/stats,1.opackets Total > 16:29:12 5,213,119 5,214,304 10,427,423 > > > Fuller details > -------------- > > While we have the dpdk-telemetry.py CLI app for interactive querying of > telemetry on the commandline, and a telemetry exporter script for > sending telemetry to external tools for real-time monitoring, we don't > have an app that can print real-time stats for DPDK apps on the > terminal. This patchset adds such a script, developed with the help of > Github copilot to fill a need that I found in my testing. Submitting it > here in the hopes that others find it of use. > > The script acts as a wrapper around the existing dpdk-telemetry.py > script, and pipes the commands to that script and reads the responses, > querying it once per second. It takes a number of flag parameters, such > as the ones above: > - "-d" for delta values, i.e. PPS rather than total packets > - "-1" for single-line output, i.e. no scrolling up the screen > - "-T" to display a total column > > Other flag parameters can be seen by looking at the help output. > > Beyond the flags, the script also takes a number of positional > parameters, which refer to specific stats to display. These stats must > be numeric values, and should take the form of the telemetry command to > send, followed by a "." and the stat within the result which is to be > tracked. As above, a stat would be e.g. "/ethdev/stats,0.opackets", > where we send "/ethdev/stats,0" to telemetry and extract the "opackets" > part of the result. > > However, specifying individual stats can be awkward, so some shortcuts > are provided too for the common case of monitoring ethernet ports. Any > positional arg starting with "eth" will be replaced by the set of > equivalent values for each port, e.g. "eth.imissed" will track the > imissed value on all ports in use in the app. The ipackets and opackets > values, as common metrics, are also available as shortened values as > just "rx" and "tx", so in the example above, "eth.tx" means to track the > opackets stat for every ethdev port. > > Finally, the script also has reconnection support so you can leave it > running while you start and stop your application in another terminal. > The watcher will try and reconnect to a running instance every second. > > v3: > Updated following AI review > - removed unnecessary f-string > - added documnentation in guides/tools > - added release note entry > > v2: > - improve reconnection handling, eliminating some crashes seen in testing. > > > Bruce Richardson (7): > usertools: add new script to monitor telemetry on terminal > usertools/telemetry-watcher: add displaying stats > usertools/telemetry-watcher: add delta and timeout opts > usertools/telemetry-watcher: add total and one-line opts > usertools/telemetry-watcher: add thousands separator > usertools/telemetry-watcher: add eth name shortcuts > usertools/telemetry-watcher: support reconnection > > doc/guides/rel_notes/release_26_03.rst | 7 + > doc/guides/tools/index.rst | 1 + > doc/guides/tools/telemetrywatcher.rst | 184 +++++++++++ > usertools/dpdk-telemetry-watcher.py | 435 +++++++++++++++++++++++++ > usertools/meson.build | 1 + > 5 files changed, 628 insertions(+) > create mode 100644 doc/guides/tools/telemetrywatcher.rst > create mode 100755 usertools/dpdk-telemetry-watcher.py Automated AI review of this patch series. Yes AI seems to be pickier about python code (wonder what it is trained on?) --- ## DPDK Patch Review: Telemetry Watcher (v3, 7 patches) **Series**: `[PATCH v3 1/7]` through `[PATCH v3 7/7]` **Author**: Bruce Richardson <[email protected]> **Reviewed against**: AGENTS.md review criteria --- ### Summary | Category | Status | |----------|--------| | Commit Messages | ✓ Pass | | SPDX/License | ⚠ Warning (minor) | | Code Style | ⚠ Warning (minor) | | Documentation | ✓ Pass | | Release Notes | ✓ Pass | | Build Integration | ✓ Pass | --- ### Commit Message Review **All 7 patches pass commit message requirements:** | Patch | Subject | Length | Format | |-------|---------|--------|--------| | 1/7 | `usertools: add new script to monitor telemetry on terminal` | 52 chars | ✓ | | 2/7 | `usertools/telemetry-watcher: add displaying stats` | 47 chars | ✓ | | 3/7 | `usertools/telemetry-watcher: add delta and timeout opts` | 53 chars | ✓ | | 4/7 | `usertools/telemetry-watcher: add total and one-line opts` | 55 chars | ✓ | | 5/7 | `usertools/telemetry-watcher: add thousands separator` | 51 chars | ✓ | | 6/7 | `usertools/telemetry-watcher: add eth name shortcuts` | 51 chars | ✓ | | 7/7 | `usertools/telemetry-watcher: support reconnection` | 49 chars | ✓ | - ✓ All subjects ≤60 characters - ✓ Lowercase after colon (except acronyms) - ✓ Imperative mood - ✓ No trailing periods - ✓ `Signed-off-by:` present with valid name/email - ✓ `Acked-by:` present (your ack is included) - ✓ Body lines appear wrapped at 75 characters - ✓ Body does not start with "It" --- ### License/SPDX Review **Python script** (`dpdk-telemetry-watcher.py`): ```python #!/usr/bin/env python3 # SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2025 Intel Corporation ``` ✓ SPDX on line 2 (correct for shebang scripts) ✓ Copyright follows immediately ✓ Blank line before code ✓ BSD-3-Clause (appropriate for usertools) **Documentation** (`telemetrywatcher.rst`): ```rst .. SPDX-License-Identifier: BSD-3-Clause Copyright(c) 2026 Intel Corporation ``` ✓ SPDX on first line ✓ Copyright follows ⚠ **Warning**: Copyright year mismatch — Python script says **2025**, documentation says **2026**. Should be consistent (likely both should be 2025 or 2026 depending on when work started). --- ### Code Style Review **Patch 5** (thousands separator): ⚠ **Warning**: The patch adds `import locale` and uses `f"{display_value:n}"` for locale-aware number formatting, but **does not call `locale.setlocale(locale.LC_ALL, '')`** to activate locale-aware formatting. Without this call, the `:n` format specifier may not produce thousands separators on all systems. **Suggested fix** — add to `main()`: ```python import locale locale.setlocale(locale.LC_ALL, '') ``` **Patch 6** (eth shortcuts): ⚠ **Info** (minor): Unnecessary f-string at line 1401: ```python print(f"Error: Failed to get ethernet device list", file=sys.stderr) ``` Should be: ```python print("Error: Failed to get ethernet device list", file=sys.stderr) ``` **General Python style**: - ✓ Good docstrings throughout - ✓ Consistent formatting (appears black-compliant) - ✓ Line lengths within 100 characters - ✓ Good error handling with informative messages --- ### Documentation Review ✓ **Comprehensive documentation** in `telemetrywatcher.rst`: - Application description - Command-line options documented - Statistics format explained - Shortcuts documented - Multiple usage examples - Dependencies listed - Output format described ✓ **Documentation matches code behavior** ✓ **Index updated** in `doc/guides/tools/index.rst` --- ### Release Notes Review ✓ **Release notes added** in patch 7/7 (`doc/guides/rel_notes/release_26_03.rst`): ```rst * **Added Script for Real-time Telemetry Monitoring.** Introduced the `dpdk-telemetry-watcher.py` script, enabling users to monitor real-time telemetry statistics from running DPDK applications. The tool supports customizable display options, including delta values, total statistics, and single-line output for compact monitoring. ``` ✓ Appropriate placement — release notes in final patch when feature is complete --- ### Build Integration Review ✓ **meson.build updated** (patch 1/7): ```python install_data([ ... 'dpdk-telemetry-watcher.py', ], install_dir: 'bin') ``` --- ### Structural Review ✓ Each patch appears independently functional (incremental development) ✓ Code and documentation updated together in relevant patches ✓ No forward dependencies within patchset **Note**: No automated tests added for this usertools script. This is common practice for Python usertools in DPDK, though the `black` formatter should be run on the code. --- ### Issues Summary | Severity | Issue | Location | |----------|-------|----------| | **Warning** | Copyright year mismatch (2025 vs 2026) | Python script vs RST doc | | **Warning** | Missing `locale.setlocale()` call for `:n` format | Patch 5/7 | | **Info** | Unnecessary f-string | Patch 6/7, line with "Failed to get ethernet device list" | --- ### Recommendation **Acceptable with minor fixes**. The patch series is well-structured, properly documented, and follows DPDK conventions. The two warnings should be addressed: 1. Harmonize copyright years 2. Add `locale.setlocale(locale.LC_ALL, '')` to ensure thousands separators work correctly
