[e-eygin]

What?

This PR removes the redundant telemetry category field from nixlTelemetryEvent.

The event now carries only:

• eventType_
• value_

It also removes category-related runtime surfaces:

• Removes nixl_telemetry_category_t.
• Removes nixlEnumStrings::telemetryCategoryStr.
• Drops the category label from Prometheus and DOCA telemetry metrics.
• Updates C++ and Python telemetry readers.
• Updates telemetry unit tests.
• Updates docs/telemetry.md and telemetry exporter READMEs.
• Bumps TELEMETRY_VERSION from 2 to 3.

This also resolves the separate nixlTelemetry::updateData argument-order cleanup request: once category is removed, both nixlTelemetryEvent construction and updateData use (event_type, value).

Depends on #1640. Please review only commit a330177 for now. After #1640 merges, this branch will be rebased onto ai-dynamo/main so the PR diff contains only the category-removal change.

Why?

category_ is fully derivable from eventType_.

For example:

• AGENT_TX_BYTES is always a transfer metric.
• AGENT_MEMORY_REGISTERED is always a memory metric.
• AGENT_XFER_TIME is always a timing metric.

Because of that, storing category in every telemetry event and exporting it as a metric label is redundant. The metric/event name already carries the meaning.

Removing it also deletes the category string conversion path and avoids emitting an extra category=... label on every Prometheus / DOCA metric sample.

TELEMETRY_VERSION is bumped because nixlTelemetryEvent is serialized into the shared-memory cyclic buffer by binary layout. Removing category_ changes field offsets, so readers must reject old buffer files instead of silently decoding them incorrectly. This follows the existing precedent from the timestamp-field removal, which bumped the version from 1 to 2.

How?

Core telemetry changes:

• nixlTelemetryEvent now stores only eventType_ and value_.
• nixlTelemetry::updateData now takes only (event_type, value).
• Event producer call sites no longer pass category.
• Backend telemetry plumbing now constructs events with (event_type, value).

Exporter changes:

• Prometheus registers metric instances with only hostname and agent_name labels.
• DOCA registers only the per-event agent_name label; hostname remains a constant source label.
• Exporters dispatch based on the event type / registered metric maps instead of event.category_.

Reader/docs/test changes:

• C++ and Python telemetry readers no longer print category.
• Python reader layout now matches the C++ event layout (uint32_t event_type, padding, uint64_t value).
• Tests were updated to assert the new layout, including TELEMETRY_VERSION == 3 and sizeof(nixlTelemetryEvent) == 16.
• Documentation no longer lists categories as part of the telemetry event model.
• docs/telemetry.md now documents the TELEMETRY_VERSION policy.

Validation:

• ninja -C build install
• build/test/gtest/gtest --gtest_filter='telemetryTest.*:prometheusTelemetryTest.*'
  • 15/15 passed
• Prometheus smoke test:
  • agent metrics appeared with agent_name and hostname
  • no category= label was present

Summary by CodeRabbit

• Refactor

  • Telemetry events simplified: category removed, event-type-focused schema and version bump; exporters and metric registration updated to operate without category labels.

• Documentation

  • Telemetry docs and exporter READMEs updated to reflect new schema, event-type naming, sample output, and environment-variable/table formatting.

• Examples

  • Example readers updated to expect the new format and stop emitting event category.

• Tests

  • Unit and integration tests adjusted to validate the streamlined event structure.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[github-actions]

👋 Hi e-eygin! Thank you for contributing to ai-dynamo/nixl.

Your PR reviewers will review your contribution then trigger the CI to test your changes.

🚀

[dpressle]

👀 Investigating Clang Format Check…

[dpressle]

🤖 CI Triage Agent — Clang Format Check · commit a3301772

I have all the evidence I need. Here is the complete diagnosis:

---

Summary: Clang Format Check failed on PR #1649 — two function definitions in prometheus_exporter.cpp have parameter lists manually wrapped across two lines in a style that clang-format-19 reformats onto a single line.

Root cause: The PR (telemetry-remove-category-field, commit a3301772) removed the category parameter from registerCounter and registerGauge. After removal, the two remaining parameters (name and help) fit comfortably on one line within the column limit, so clang-format-19 collapses the manually broken two-line signatures into one. The committed code still has them split across two lines, which disagrees with what clang-format produces:

--- src/plugins/telemetry/prometheus/prometheus_exporter.cpp  (before formatting)
+++ src/plugins/telemetry/prometheus/prometheus_exporter.cpp  (after formatting)
@@ -131,8 +131,7 @@
 void
-nixlTelemetryPrometheusExporter::registerCounter(const std::string &name,
-                                                 const std::string &help) {
+nixlTelemetryPrometheusExporter::registerCounter(const std::string &name, const std::string &help) {

-nixlTelemetryPrometheusExporter::registerGauge(const std::string &name,
-                                               const std::string &help) {
+nixlTelemetryPrometheusExporter::registerGauge(const std::string &name, const std::string &help) {

The old three-parameter signatures were wide enough to require wrapping; now that category is gone the shorter two-parameter signatures must be on one line per the project's .clang-format rules.

Implicated commit: a3301772961f66c18e5e75d5aeaeeebdb89acfd8 (branch telemetry-remove-category-field, PR #1649)

File: src/plugins/telemetry/prometheus/prometheus_exporter.cpp — lines ~153–154 and ~156–157 (the registerCounter and registerGauge function definitions)

Suggested fix: Run clang-format-19 locally on the file (or just apply the diff above) to collapse those two split signatures onto single lines:

// Change from:
nixlTelemetryPrometheusExporter::registerCounter(const std::string &name,
                                                 const std::string &help) {
// To:
nixlTelemetryPrometheusExporter::registerCounter(const std::string &name, const std::string &help) {

// Change from:
nixlTelemetryPrometheusExporter::registerGauge(const std::string &name,
                                               const std::string &help) {
// To:
nixlTelemetryPrometheusExporter::registerGauge(const std::string &name, const std::string &help) {

The quickest way to fix it is to run clang-format-19 -i -style=file src/plugins/telemetry/prometheus/prometheus_exporter.cpp and amend/push the commit.

Related: none

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR removes the telemetry category field and related APIs from the event structure, exporters, and readers. The protocol version increments from 2 to 3. All exporters are refactored to classify events by type rather than a separate category dimension.

Changes

Remove telemetry category field and API

Layer / File(s)	Summary
Event schema and protocol version src/core/telemetry/telemetry_event.h, examples/python/telemetry_reader.py, docs/telemetry.md	nixlTelemetryEvent removes category_ field, constructor changes to (event_type, value) signature, and TELEMETRY_VERSION increments from 2 to 3. Python reader struct layout and version updated to match.
Core telemetry API refactoring src/core/telemetry/telemetry.h, src/core/telemetry/telemetry.cpp, src/api/cpp/backend/backend_engine.h	updateData method drops the category parameter; all callers (updateTxBytes, updateRxBytes, updateErrorCount, updateMemoryRegistered, addXferTime, addPostTime) updated to use the new signature. Backend engine integration adjusted accordingly.
Prometheus exporter refactoring src/plugins/telemetry/prometheus/prometheus_exporter.h, src/plugins/telemetry/prometheus/prometheus_exporter.cpp, src/plugins/telemetry/prometheus/README.md, test/gtest/telemetry_prometheus_test.cpp	registerCounter and registerGauge drop category parameter; exportEvent stops switching on category and instead unconditionally looks up metrics by event name; metric labels reduced to hostname and agent_name only; tests updated to assert category label absence.
DOCA exporter refactoring src/plugins/telemetry/doca/doca_exporter.cpp, src/plugins/telemetry/doca/README.md, src/plugins/telemetry/doca/meson.build	Category label constants removed; new isCounterEvent and isGaugeEvent helpers classify events by type; shared context label set reduced to agent_name; exportEvent branches on event type instead of category; build dependency (nixl_common_dep) added.
Example readers update examples/cpp/telemetry_reader.cpp, examples/python/telemetry_reader.py	C++ reader print_telemetry_event removes category output line; Python reader removes _CATEGORY_STRINGS mapping and get_telemetry_category_string function, updates print_telemetry_event to log only event type name and value.
Plugin README and CSV examples src/plugins/telemetry/README.md	CSV exporter example header and rows updated to emit event_type,value and omit category.
Core telemetry tests test/gtest/telemetry_test.cpp	TelemetryEventStructure constructs events with (event_type, value), asserts version and struct size; category field assertions removed; TelemetryCategoryStringConversion test removed.
Documentation updates docs/telemetry.md, src/plugins/telemetry/README.md	Event schema description adds explicit "Event type" and "Value" fields; built-in metrics table removes category column; example output removes "Category: …" lines; TELEMETRY_VERSION behavior documented (exact-match reader requirement, unlink on mismatch).

🎯 3 (Moderate) | ⏱️ ~20 minutes

🐰 A category field hops away,
Event types now lead the fray,
Version three takes the stage,
Exporters turn a brighter page,
Simpler schemas win the day!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 27.78% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title accurately describes the main change: removing the redundant category field from telemetry events.
Description check	✅ Passed	The PR description fully addresses all required template sections with comprehensive detail about what, why, and how the category field removal is implemented.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/plugins/telemetry/prometheus/prometheus_exporter.cpp (1)

134-147: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix clang-format signature wrapping to unblock CI.

The current wrapping in these function declarations/definitions is causing the clang-format check to fail.

Proposed formatting fix

-void
-nixlTelemetryPrometheusExporter::registerCounter(const std::string &name,
-                                                 const std::string &help) {
+void nixlTelemetryPrometheusExporter::registerCounter(const std::string &name,
+                                                      const std::string &help) {
@@
-void
-nixlTelemetryPrometheusExporter::registerGauge(const std::string &name,
-                                               const std::string &help) {
+void nixlTelemetryPrometheusExporter::registerGauge(const std::string &name,
+                                                    const std::string &help) {

As per coding guidelines, “Formatting: 4-space indentation, no tabs; max 100-char lines; continuation lines indented by 4.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/plugins/telemetry/prometheus/prometheus_exporter.cpp` around lines 134 -
147, The function signature wrapping for
nixlTelemetryPrometheusExporter::registerCounter (and similarly for
registerGauge) violates the project's clang-format rules; fix by reformatting
the function declarations/definitions so the return type and function
name/signature conform to the 4-space indentation and max-100-char line rules
(either put the full signature on one line if it fits or break parameter list
with continuation lines indented by 4 spaces), ensuring consistent wrapping for
both registerCounter and registerGauge so clang-format/CI no longer fails.

src/api/cpp/backend/backend_engine.h (1)

65-69: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Take telemetryEventsMutex_ before checking the queue size.

telemetryEvents_.size() is read before the mutex is acquired, while Line 69 and getTelemetryEvents() mutate the same std::vector under telemetryEventsMutex_. That is a data race and a TOCTOU bug: under contention this can both invoke UB and let producers overshoot MAX_TELEMETRY_QUEUE_SIZE.

🐛 Proposed fix

         void
         addTelemetryEvent(nixl_telemetry_event_type_t event_type, uint64_t value) {
-            if (!enableTelemetry_) return;
-            if (telemetryEvents_.size() >= MAX_TELEMETRY_QUEUE_SIZE) return;
-            std::lock_guard<std::mutex> lock(telemetryEventsMutex_);
+            if (!enableTelemetry_) {
+                return;
+            }
+            const std::lock_guard<std::mutex> lock(telemetryEventsMutex_);
+            if (telemetryEvents_.size() >= MAX_TELEMETRY_QUEUE_SIZE) {
+                return;
+            }
             telemetryEvents_.emplace_back(event_type, value);
         }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/api/cpp/backend/backend_engine.h` around lines 65 - 69, In
addTelemetryEvent, avoid the TOCTOU/data race by acquiring telemetryEventsMutex_
before reading or mutating telemetryEvents_; specifically move the
std::lock_guard<std::mutex> lock(telemetryEventsMutex_) to before checking
telemetryEvents_.size() and MAX_TELEMETRY_QUEUE_SIZE, then re-check the size
under the lock and call telemetryEvents_.emplace_back(event_type, value); this
ensures getTelemetryEvents() and other mutators that use telemetryEventsMutex_
synchronize correctly with addTelemetryEvent.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/telemetry.md`:
- Line 82: The doc claim that readers "unlink the file on mismatch" is
inconsistent with the Python example; update either docs or code: either change
docs/telemetry.md wording around TELEMETRY_VERSION to state which
readers/unlinking runtime perform the unlink, or modify
examples/python/telemetry_reader.py to perform an unlink when TELEMETRY_VERSION
mismatches instead of only raising RuntimeError; locate the version-check logic
and the error path in telemetry_reader.py (search for TELEMETRY_VERSION and the
RuntimeError raised on mismatch) and implement os.unlink(file) on the mismatch
path or adjust the documentation text to clarify which implementations remove
the file.

---

Outside diff comments:
In `@src/api/cpp/backend/backend_engine.h`:
- Around line 65-69: In addTelemetryEvent, avoid the TOCTOU/data race by
acquiring telemetryEventsMutex_ before reading or mutating telemetryEvents_;
specifically move the std::lock_guard<std::mutex> lock(telemetryEventsMutex_) to
before checking telemetryEvents_.size() and MAX_TELEMETRY_QUEUE_SIZE, then
re-check the size under the lock and call
telemetryEvents_.emplace_back(event_type, value); this ensures
getTelemetryEvents() and other mutators that use telemetryEventsMutex_
synchronize correctly with addTelemetryEvent.

In `@src/plugins/telemetry/prometheus/prometheus_exporter.cpp`:
- Around line 134-147: The function signature wrapping for
nixlTelemetryPrometheusExporter::registerCounter (and similarly for
registerGauge) violates the project's clang-format rules; fix by reformatting
the function declarations/definitions so the return type and function
name/signature conform to the 4-space indentation and max-100-char line rules
(either put the full signature on one line if it fits or break parameter list
with continuation lines indented by 4 spaces), ensuring consistent wrapping for
both registerCounter and registerGauge so clang-format/CI no longer fails.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9d2f4925-7404-41ee-9b52-92126d1f7489

📥 Commits

Reviewing files that changed from the base of the PR and between 49a6eec and a330177.

📒 Files selected for processing (16)

• docs/telemetry.md
• examples/cpp/telemetry_reader.cpp
• examples/python/telemetry_reader.py
• src/api/cpp/backend/backend_engine.h
• src/core/telemetry/telemetry.cpp
• src/core/telemetry/telemetry.h
• src/core/telemetry/telemetry_event.h
• src/plugins/telemetry/README.md
• src/plugins/telemetry/doca/README.md
• src/plugins/telemetry/doca/doca_exporter.cpp
• src/plugins/telemetry/doca/meson.build
• src/plugins/telemetry/prometheus/README.md
• src/plugins/telemetry/prometheus/prometheus_exporter.cpp
• src/plugins/telemetry/prometheus/prometheus_exporter.h
• test/gtest/telemetry_prometheus_test.cpp
• test/gtest/telemetry_test.cpp

💤 Files with no reviewable changes (1)

• examples/cpp/telemetry_reader.cpp

[dpressle]

👀 Investigating Clang Format Check…

[dpressle]

🤖 CI Triage Agent — Clang Format Check · commit 531a5f83

I have everything needed for a complete diagnosis. Here is the full report:

---

Summary: clang-format check fails on PR #1649 — single-statement if body in src/api/cpp/backend/backend_engine.h must use braces per the project's style rules.

Root cause: The commit 531a5f83 introduced (or retained) a braceless single-line if guard in addTelemetryEvent():

// CURRENT (fails clang-format-19):
if (telemetryEvents_.size() >= MAX_TELEMETRY_QUEUE_SIZE) return;

// REQUIRED by .clang-format style:
if (telemetryEvents_.size() >= MAX_TELEMETRY_QUEUE_SIZE) {
    return;
}

The clang-format-diff-19 tool applied to the diff of this PR detected the violation and produced a non-empty patch, causing the job to exit with code 1. The two earlier if (!enableTelemetry_) return; guards on the lines immediately above use the same braceless style, but those lines were not part of this PR's diff so they were not flagged — only the newly added/modified line was checked. Note that the lock/queue-check lines appear to have been reordered in this PR (queue-size check is now before the mutex lock), which is what put the line into the diff and triggered the linter.

Implicated commit: 531a5f83ba39b38301d382608bff1105b0db0b03 (PR #1649, branch telemetry-remove-category-field)

File: src/api/cpp/backend/backend_engine.h — the addTelemetryEvent method, specifically the if (telemetryEvents_.size() >= MAX_TELEMETRY_QUEUE_SIZE) guard (around line 68 in the file).

Suggested fix: Expand the braceless return into a braced block as clang-format-19 requires:

void
addTelemetryEvent(nixl_telemetry_event_type_t event_type, uint64_t value) {
    if (!enableTelemetry_) return;
    std::lock_guard<std::mutex> lock(telemetryEventsMutex_);
    if (telemetryEvents_.size() >= MAX_TELEMETRY_QUEUE_SIZE) {
        return;
    }
    telemetryEvents_.emplace_back(event_type, value);
}

The fastest way to do this locally is to run:

clang-format-19 -i -style=file src/api/cpp/backend/backend_engine.h

and then commit the result. This will auto-apply exactly the diff the CI checker expected.

Related: #1531 (telemetry enum refactor by BatshevaBlack, last touch to this file), #1522 (Remove timestamp field from telemetry events) — no existing issue specifically for this clang-format violation.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/api/cpp/backend/backend_engine.h`:
- Line 65: The change to the public header method signature
addTelemetryEvent(nixl_telemetry_event_type_t event_type, uint64_t value) is an
API/ABI break for downstream consumers; restore backward compatibility by either
reverting the signature change or adding an overload/compatibility wrapper that
preserves the original symbol (e.g., keep the old addTelemetryEvent signature or
provide an inline forwarding overload in backend_engine.h) and ensure the
implementation in the corresponding .cpp forwards to the new behavior; also add
an explicit migration note to this PR (release notes/CHANGES) and notify
downstream maintainers of the breaking change so they can coordinate upgrades.
- Around line 66-69: Clang-format is failing due to single-line early-return if
statements; update the two ifs that use early returns to use explicit block
braces and proper newlines so formatting is consistent: replace "if
(!enableTelemetry_) return;" with a braced form like "if (!enableTelemetry_) {
return; }" and similarly replace "if (telemetryEvents_.size() >=
MAX_TELEMETRY_QUEUE_SIZE) return;" with "if (telemetryEvents_.size() >=
MAX_TELEMETRY_QUEUE_SIZE) { return; }" around the telemetry-related symbols
enableTelemetry_, telemetryEvents_, MAX_TELEMETRY_QUEUE_SIZE and keep the
existing std::lock_guard<std::mutex> lock(telemetryEventsMutex_) and
telemetryEvents_.emplace_back(event_type, value); lines unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2ef9a66e-721f-4caf-9fff-9762a55b0046

📥 Commits

Reviewing files that changed from the base of the PR and between a330177 and 531a5f8.

📒 Files selected for processing (3)

• docs/telemetry.md
• src/api/cpp/backend/backend_engine.h
• src/plugins/telemetry/prometheus/prometheus_exporter.cpp

[e-eygin]

@ColinNV, @ovidiusm please review

[e-eygin]

@brminich please review

[e-eygin]

/build

[e-eygin]

/ok to test 85988f2

[e-eygin]

/build

[brminich]

/ok to test ecd6c36

[brminich]

/build

[guy-ealey-morag]

/ok to test 5255ea7

[guy-ealey-morag]

/build

[guy-ealey-morag]

/build

[guy-ealey-morag]

/build

[guy-ealey-morag]

/build