All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Bruce Richardson <bruce.richardson@intel.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH v3 0/7] Add script for real-time telemetry monitoring
Date: Thu, 15 Jan 2026 22:57:50 -0800	[thread overview]
Message-ID: <20260115225750.3a7fe9a3@phoenix.local> (raw)
In-Reply-To: <20260115190331.3721281-1-bruce.richardson@intel.com>

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
> 
> --
> 2.51.0

AI also had some good feedback on the documentation here.



### Spelling Issues

✓ No spelling errors found.

---

### Grammar Issues

| Line/Section | Issue | Suggestion |
|--------------|-------|------------|
| Options: `-i` | "...when multiple applications are running with the same file-prefix." | Consider: "...when multiple applications **share** the same file-prefix." (more concise) |
| Options: `-t` | "Number of iterations to run before stopping." | Should be: "Number of iterations to run before **exiting**." (matches the behavior—tool exits, not just stops) |
| Statistics Format | "To discover available commands and fields:" followed by numbered list | The colon introduces an incomplete sentence. Better: "To discover available commands and fields, use the following methods:" or remove the colon and use "you can:" |

---

### Technical Writing Style Issues

| Location | Issue | Recommendation |
|----------|-------|----------------|
| Title | "dpdk-telemetry-watcher Application" | Consider: "dpdk-telemetry-watcher Tool" — the word "application" might confuse readers since DPDK applications are the *targets* being monitored |
| Opening paragraph | "Data Plane Development Kit (DPDK)" | Unnecessary expansion—DPDK docs assume readers know what DPDK is. Simplify to just "DPDK" |
| Options: `-d` | "Display delta (incremental) values" | Redundant parenthetical. Use either "delta values" or "incremental values", not both |
| Options: `-d` | "which is useful for monitoring rates of change" | Vague. Better: "which is useful for monitoring per-second rates" |
| Shortcuts section | "These shortcuts automatically detect all available ethernet devices" | Should be "Ethernet" (capital E) per DPDK style, or better: "ethdev ports" to match DPDK terminology |
| Output Format | "Values are formatted with locale-specific number formatting (e.g., comma separators)." | Awkward repetition of "formatting". Rewrite: "Values use locale-specific formatting (e.g., comma as thousands separator)." |
| Dependencies | "A running DPDK application with telemetry enabled" | Contradicts the "Running the Application" section which says the tool "can be run at any time, whether or not a DPDK application is currently running." Remove this item or clarify it's needed for actual monitoring. |

---

### Passive Voice (per DPDK documentation standards)

| Location | Passive Construction | Active Alternative |
|----------|---------------------|-------------------|
| Opening | "Statistics are specified in the format..." | "Specify statistics in the format..." |
| Output Format | "Values are formatted with..." | "The tool formats values with..." |
| Output Format | "the tool displays the change in each statistic" | ✓ Already active (good) |

---

### Consistency Issues

| Issue | Details |
|-------|---------|
| "ethernet" vs "Ethernet" | Line uses lowercase "ethernet devices" — DPDK typically uses "Ethernet" or avoids it entirely in favor of "ethdev" |
| "file-prefix" hyphenation | Used consistently (good), but DPDK code often uses "file_prefix" — verify against dpdk-telemetry.py docs |
| Option argument style | Mix of `FILE_PREFIX` (all caps) and `TIMEOUT` (all caps) — consistent (good) |
| Example commands | Some use full paths (`/ethdev/stats,0.ipackets`), shortcuts (`eth.rx`) — good variety showing both |

---

### RST Formatting Issues

| Location | Issue | Fix |
|----------|-------|-----|
| Line 1 | `..  SPDX` has two spaces after `..` | Should be single space: `.. SPDX` |
| Cross-references | `` `Statistics Format`_ `` and `` `Examples`_ `` | These RST references work but would be more robust as `:ref:` targets for cross-document linking |

---

### Missing Information

| Gap | Suggestion |
|-----|------------|
| No mention of Ctrl+C | Add note that Ctrl+C cleanly exits the monitoring loop (already in code) |
| No sample output | Consider adding a brief example of what the output looks like |
| No error handling description | What happens if a stat doesn't exist? Document the error messages |

---

### Suggested Rewrites

**Opening paragraph** (current):
> The ``dpdk-telemetry-watcher`` tool is a Data Plane Development Kit (DPDK) utility that provides continuous monitoring of DPDK telemetry statistics on the command line. It wraps the ``dpdk-telemetry.py`` script to provide real-time statistics display capabilities.

**Suggested**:
> The ``dpdk-telemetry-watcher`` tool monitors DPDK telemetry statistics continuously on the command line. It wraps ``dpdk-telemetry.py`` to provide real-time display of selected statistics.

---

**Dependencies section** (current):
> The tool requires:
> * Python 3
> * The ``dpdk-telemetry.py`` script must be available in the same directory or in the system PATH
> * A running DPDK application with telemetry enabled

**Suggested**:
> The tool requires:
> 
> * Python 3
> * The ``dpdk-telemetry.py`` script (in the same directory or in PATH)
> 
> For monitoring, a DPDK application with telemetry enabled must be running, though the watcher can start before the application and will connect automatically.

---

### Summary

| Category | Count |
|----------|-------|
| Grammar issues | 3 |
| Style issues | 7 |
| Passive voice | 2 |
| Consistency issues | 1 |
| RST formatting | 1 |
| Missing information | 3 |

**Overall**: The documentation is functional and reasonably clear. The issues are minor and mostly stylistic. The most important fix is resolving the contradiction in the Dependencies section about whether a running DPDK application is required.

  parent reply	other threads:[~2026-01-16  6:57 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-10 16:55 [RFC PATCH 0/7] Add script for real-time telemetry monitoring Bruce Richardson
2025-12-10 16:55 ` [RFC PATCH 1/7] usertools: add new script to monitor telemetry on terminal Bruce Richardson
2025-12-10 16:55 ` [RFC PATCH 2/7] usertools/telemetry-watcher: add displaying stats Bruce Richardson
2025-12-10 16:55 ` [RFC PATCH 3/7] usertools/telemetry-watcher: add delta and timeout opts Bruce Richardson
2025-12-10 16:55 ` [RFC PATCH 4/7] usertools/telemetry-watcher: add total and one-line opts Bruce Richardson
2025-12-10 16:55 ` [RFC PATCH 5/7] usertools/telemetry-watcher: add thousands separator Bruce Richardson
2025-12-10 16:55 ` [RFC PATCH 6/7] usertools/telemetry-watcher: add eth name shortcuts Bruce Richardson
2025-12-10 16:55 ` [RFC PATCH 7/7] usertools/telemetry-watcher: support reconnection Bruce Richardson
2025-12-11  1:09 ` [RFC PATCH 0/7] Add script for real-time telemetry monitoring Stephen Hemminger
2025-12-11  9:10   ` Bruce Richardson
2025-12-12  5:32 ` Stephen Hemminger
2025-12-12 17:52   ` Bruce Richardson
2025-12-12 23:23     ` Stephen Hemminger
2026-01-05 17:55 ` [PATCH v2 " Bruce Richardson
2026-01-05 17:55   ` [PATCH v2 1/7] usertools: add new script to monitor telemetry on terminal Bruce Richardson
2026-01-05 17:56   ` [PATCH v2 2/7] usertools/telemetry-watcher: add displaying stats Bruce Richardson
2026-01-05 17:56   ` [PATCH v2 3/7] usertools/telemetry-watcher: add delta and timeout opts Bruce Richardson
2026-01-05 17:56   ` [PATCH v2 4/7] usertools/telemetry-watcher: add total and one-line opts Bruce Richardson
2026-01-05 17:56   ` [PATCH v2 5/7] usertools/telemetry-watcher: add thousands separator Bruce Richardson
2026-01-05 17:56   ` [PATCH v2 6/7] usertools/telemetry-watcher: add eth name shortcuts Bruce Richardson
2026-01-05 17:56   ` [PATCH v2 7/7] usertools/telemetry-watcher: support reconnection Bruce Richardson
2026-01-14  2:00   ` [PATCH v2 0/7] Add script for real-time telemetry monitoring Stephen Hemminger
2026-01-15 19:03 ` [PATCH v3 " Bruce Richardson
2026-01-15 19:03   ` [PATCH v3 1/7] usertools: add new script to monitor telemetry on terminal Bruce Richardson
2026-01-15 19:03   ` [PATCH v3 2/7] usertools/telemetry-watcher: add displaying stats Bruce Richardson
2026-01-15 19:03   ` [PATCH v3 3/7] usertools/telemetry-watcher: add delta and timeout opts Bruce Richardson
2026-01-15 19:03   ` [PATCH v3 4/7] usertools/telemetry-watcher: add total and one-line opts Bruce Richardson
2026-01-15 19:03   ` [PATCH v3 5/7] usertools/telemetry-watcher: add thousands separator Bruce Richardson
2026-01-15 19:03   ` [PATCH v3 6/7] usertools/telemetry-watcher: add eth name shortcuts Bruce Richardson
2026-01-15 19:03   ` [PATCH v3 7/7] usertools/telemetry-watcher: support reconnection Bruce Richardson
2026-01-16  6:53   ` [PATCH v3 0/7] Add script for real-time telemetry monitoring Stephen Hemminger
2026-01-16  6:57   ` Stephen Hemminger [this message]
2026-02-05 15:02 ` [PATCH v4 " Bruce Richardson
2026-02-05 15:02   ` [PATCH v4 1/7] usertools: add new script to monitor telemetry on terminal Bruce Richardson
2026-02-05 15:02   ` [PATCH v4 2/7] usertools/telemetry-watcher: add displaying stats Bruce Richardson
2026-02-05 15:02   ` [PATCH v4 3/7] usertools/telemetry-watcher: add delta and timeout opts Bruce Richardson
2026-02-05 15:02   ` [PATCH v4 4/7] usertools/telemetry-watcher: add total and one-line opts Bruce Richardson
2026-02-05 15:02   ` [PATCH v4 5/7] usertools/telemetry-watcher: add thousands separator Bruce Richardson
2026-02-05 15:02   ` [PATCH v4 6/7] usertools/telemetry-watcher: add eth name shortcuts Bruce Richardson
2026-02-05 15:02   ` [PATCH v4 7/7] usertools/telemetry-watcher: support reconnection Bruce Richardson
2026-03-31 18:10   ` [PATCH v4 0/7] Add script for real-time telemetry monitoring Stephen Hemminger
2026-05-05 10:08 ` [PATCH v5 " Bruce Richardson
2026-05-05 10:08   ` [PATCH v5 1/7] usertools: add new script to monitor telemetry on terminal Bruce Richardson
2026-05-05 10:08   ` [PATCH v5 2/7] usertools/telemetry-watcher: add displaying stats Bruce Richardson
2026-05-05 10:08   ` [PATCH v5 3/7] usertools/telemetry-watcher: add delta and timeout opts Bruce Richardson
2026-05-05 10:08   ` [PATCH v5 4/7] usertools/telemetry-watcher: add total and one-line opts Bruce Richardson
2026-05-05 10:08   ` [PATCH v5 5/7] usertools/telemetry-watcher: add thousands separator Bruce Richardson
2026-05-05 10:08   ` [PATCH v5 6/7] usertools/telemetry-watcher: add eth name shortcuts Bruce Richardson
2026-05-05 10:08   ` [PATCH v5 7/7] usertools/telemetry-watcher: support reconnection Bruce Richardson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260115225750.3a7fe9a3@phoenix.local \
    --to=stephen@networkplumber.org \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.