Re: [PATCH v3 0/7] Add script for real-time telemetry monitoring

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 15 Jan 2026 19:03:24 +0000
Bruce Richardson <bruce.richardson@intel.com> 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 <bruce.richardson@intel.com>  
**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