DRAFT: Telemetry exporter plug-in infra #962

alexanderbilk · 2025-10-29T09:51:22Z

What?

Adds infrastructure for dynamic load of telemetry exporters
Beta implementation of prometheus exporter

Why?

Required capability to expose NIXL telemetry in common telemetry formats (e.g OTLP, Prometheus etc.)

How?

Solution works as dynamically loaded user defined plugins

copy-pr-bot · 2025-10-29T09:51:25Z

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 · 2025-10-29T09:51:31Z

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

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

🚀

AlexNvd · 2025-10-30T06:57:47Z

src/plugins/telemetry/prometheus/README.md

+
+# NIXL Prometheus Telemetry Exporter Plugin
+
+This telemetry exporter plugin exports NIXL telemetry events to Prometheus format via an HTTP endpoint that can be scraped by Prometheus servers.


A typo and suggestion - ... in Prometheus format, by_exposing an HTTP endpoint ...

Changed. What exactly here is a typo?

Not exactly a typo, it was about "to => in".

AlexNvd · 2025-10-30T07:06:55Z

src/plugins/telemetry/prometheus/README.md

+
+## Configuration
+
+To enable the Prometheus exporter, set the following environment variable:


I'd add the word "plug-in" and explain that this variable activates a selected plug-in, of a (future) plug-in choices

It may be considered to put an explicit .so name or path, to allow for custom plug-ins (e.g., by a third party)

Added more info here

AlexNvd · 2025-10-30T07:12:52Z

src/plugins/telemetry/prometheus/prometheus_exporter.cpp

+
+    gauges_["agent_xfer_time"] = &xfer_time_gauge.Add({{"category", "NIXL_TELEMETRY_PERFORMANCE"}});
+    gauges_["agent_xfer_post_time"] = &xfer_post_time_gauge.Add({{"category", "NIXL_TELEMETRY_PERFORMANCE"}});
+    gauges_["agent_memory_registered"] = &memory_registered_gauge.Add({{"category", "NIXL_TELEMETRY_MEMORY"}});


Maybe these two should be implemented as counters, such that their difference gives an amount of currently registered memory?

It stated in HLD what this metric operates as Gauge. Also what I can see from memory event implementation confirms it

AlexNvd · 2025-10-30T07:32:40Z

src/api/cpp/telemetry/telemetry_exporter.h

@@ -0,0 +1,65 @@
+/*


A general note, please add some .md file that explains how to write a custom telemetry plug-in. What is the API and what templates can be used.

src/api/cpp/telemetry/telemetry_exporter.h

ColinNV · 2025-10-30T17:29:30Z

src/api/cpp/telemetry/telemetry_exporter.h

+    uint32_t eventLimit_;
+
+public:
+    explicit nixlTelemetryExporter(const nixlTelemetryExporterInitParams *init_params)


Suggest passing by const-reference; if the params are optional they can be wrapped with std::optional (or a default-c'tor can be added).

Changed to const reference

src/api/cpp/telemetry/telemetry_exporter.h

src/core/telemetry_plugin_manager.h

src/api/cpp/telemetry/telemetry_plugin.h

ColinNV · 2025-11-04T17:17:15Z

src/api/cpp/telemetry/telemetry_exporter.h

+ */
+class nixlTelemetryExporter {
+protected:
+    const uint32_t eventLimit_;


This is a uint64_t in the init params, should be consistent (and perhaps size_t?)

Agreed, changed to size_t

ColinNV · 2025-11-04T17:22:30Z

src/api/cpp/telemetry/telemetry_plugin.h

+        const nixlTelemetryExporterInitParams *init_params);
+
+    // Constructor
+    nixlTelemetryPlugin(


This and the two getters can be marked noexcept. The comment is superfluous.

ColinNV · 2025-11-04T17:23:06Z

src/api/cpp/telemetry/telemetry_plugin.h

+    NixlTelemetryPluginApiVersion api_version;
+
+    // Function pointer for creating a new exporter instance (returns smart pointer for RAII)
+    std::unique_ptr<nixlTelemetryExporter> (*create_exporter)(


A type alias for the function pointer could make this and the c'tor argument more readable.

ColinNV · 2025-11-04T17:30:39Z

src/core/telemetry_plugin_manager.cpp

+            }
+        }
+
+        dlclose(handle_);


Should handle_ not also be the unique_ptr with the appropriate deleter?

ColinNV · 2025-11-04T17:31:53Z

src/core/telemetry_plugin_manager.cpp

+      plugin_(plugin) {}
+
+nixlTelemetryPluginHandle::~nixlTelemetryPluginHandle() {
+    if (handle_) {


Replace this with assert(handle) in the c'tor (or // assert(handle) just to make the intention clear)?

ColinNV · 2025-11-04T17:43:59Z

src/plugins/telemetry/prometheus/prometheus_exporter.cpp

+nixl_status_t
+nixlTelemetryPrometheusExporter::exportEvent(const nixlTelemetryEvent &event) {
+    try {
+        std::string event_name(event.eventName_);


This could be a const std::string_view if the map uses std::less<void>.

Or not, since it's only coming to unordered containers in C++20.

Could still be const :-)

Added const

ColinNV · 2025-11-04T17:49:13Z

src/core/telemetry_plugin_manager.cpp

+    // Try to load the plugin from all registered directories
+    for (const auto &dir : plugin_dirs_) {
+        if (dir.empty()) {
+            continue;


Should better be caught at time of insertion into plugin_dirs_.

(Which would also make it easier to not hold the lock the whole time, but that's probably not important for this particular function.)

Omitted for now then.
All this functional is more of a "just in case" placeholder and to make it consistent with backend plugin manager

ColinNV · 2025-11-05T07:05:19Z

src/plugins/telemetry/prometheus/prometheus_exporter.cpp

+    if (init_params && !init_params->outputPath.empty()) {
+        // Validate format: ip_addr:port_num
+        std::string path = init_params->outputPath;
+        size_t colon_pos = path.find(':');


Variables should be declared const when they are not changed (mostly to let the reader know they won't change).

ColinNV · 2025-11-05T07:06:17Z

src/core/telemetry_plugin_manager.h

+     * @param dirpath Directory path to search
+     */
+    void
+    discoverPluginsFromDir(const std::string &dirpath);


Could use std::filesystem::path for paths and filenames?

Yes, replaced

ColinNV · 2025-11-05T07:11:00Z

src/api/cpp/telemetry/telemetry_plugin.h

+enum class NixlTelemetryPluginApiVersion : unsigned int { V1 = 1 };
+
+inline constexpr NixlTelemetryPluginApiVersion NIXL_TELEMETRY_PLUGIN_API_VERSION =
+    NixlTelemetryPluginApiVersion::V1;


Would it make sense to put v1 into the name of all types (and global functions, if any) that are under versioning to make it clearer which parts exactly are in fact versioned?

This is another "just in case" placeholder actually for beta version. I'd consider removing API versioning at all

DRAFT: Prometheus exporter implemetation Signed-off-by: Aleksandr Bilkovskii <[email protected]>

rakhmets · 2025-11-12T10:35:43Z

src/api/cpp/telemetry/telemetry_exporter.h

+inline constexpr char TELEMETRY_EXPORTER_VAR[] = "NIXL_TELEMETRY_EXPORTER";
+inline constexpr char TELEMETRY_EXPORTER_PLUGIN_DIR_VAR[] = "NIXL_TELEMETRY_EXPORTER_PLUGIN_DIR";
+inline constexpr char TELEMETRY_EXPORTER_OUTPUT_PATH_VAR[] = "NIXL_TELEMETRY_EXPORTER_OUTPUT_PATH";


Suggested change

inline constexpr char TELEMETRY_EXPORTER_VAR[] = "NIXL_TELEMETRY_EXPORTER";

inline constexpr char TELEMETRY_EXPORTER_PLUGIN_DIR_VAR[] = "NIXL_TELEMETRY_EXPORTER_PLUGIN_DIR";

inline constexpr char TELEMETRY_EXPORTER_OUTPUT_PATH_VAR[] = "NIXL_TELEMETRY_EXPORTER_OUTPUT_PATH";

inline constexpr char telemetry_exporter_var[] = "NIXL_TELEMETRY_EXPORTER";

inline constexpr char telemetry_exporter_plugin_dir_var[] = "NIXL_TELEMETRY_EXPORTER_PLUGIN_DIR";

inline constexpr char telemetry_exporter_output_path_var[] = "NIXL_TELEMETRY_EXPORTER_OUTPUT_PATH";

https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es9-avoid-all_caps-names

rakhmets · 2025-11-12T10:38:22Z

src/api/cpp/telemetry/telemetry_exporter.h

+ * them to various destinations.
+ */
+class nixlTelemetryExporter {
+protected:


Please use the public before protected order.

https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nl16-use-a-conventional-class-member-declaration-order

rakhmets · 2025-11-12T10:40:41Z

src/api/cpp/telemetry/telemetry_exporter.h

+#include "nixl_types.h"
+#include "telemetry_event.h"
+#include "common/cyclic_buffer.h"
+
+#include <string>
+#include <vector>
+#include <fstream>


Please delete the unused header files.

rakhmets · 2025-11-12T10:42:00Z

src/api/cpp/telemetry/telemetry_plugin.h

+#include <string_view>
+#include <memory>
+
+enum class NixlTelemetryPluginApiVersion : unsigned int { V1 = 1 };


enums should be in snake_case.

https://github.com/ai-dynamo/nixl/blob/main/docs/CodeStyle.md

rakhmets · 2025-11-12T10:43:44Z

src/api/cpp/telemetry/telemetry_plugin.h

+
+enum class NixlTelemetryPluginApiVersion : unsigned int { V1 = 1 };
+
+inline constexpr NixlTelemetryPluginApiVersion NIXL_TELEMETRY_PLUGIN_API_VERSION =


Please avoid ALL_CAPS names.

rakhmets · 2025-11-12T10:44:07Z

src/api/cpp/telemetry/telemetry_plugin.h

+    std::unique_ptr<nixlTelemetryExporter> (*)(const nixlTelemetryExporterInitParams &init_params);
+
+class nixlTelemetryPlugin {
+private:


Please use the public before private order.

rakhmets · 2025-11-12T10:44:57Z

src/api/cpp/telemetry/telemetry_plugin.h

+    NixlTelemetryPluginApiVersion::V1;
+
+// Type alias for exporter creation function
+using ExporterCreatorFn =


Suggested change

using ExporterCreatorFn =

using exporter_creator_fn_t =

https://github.com/ai-dynamo/nixl/blob/main/docs/CodeStyle.md

rakhmets · 2025-11-12T11:21:20Z

src/api/cpp/telemetry/telemetry_plugin.h

+    std::unique_ptr<nixlTelemetryExporter> (*)(const nixlTelemetryExporterInitParams &init_params);
+
+class nixlTelemetryPlugin {
+private:


Please use the public before private order.

rakhmets · 2025-11-12T11:22:29Z

src/core/telemetry.cpp

+        std::lock_guard<std::mutex> lock(mutex_);
+        events_.swap(next_queue);
+    }
+    for (auto &event : next_queue) {


rakhmets · 2025-11-12T11:22:45Z

src/core/telemetry.cpp

+              [](const nixlTelemetryEvent &a, const nixlTelemetryEvent &b) {
+                  return a.timestampUs_ < b.timestampUs_;
+              });
+    for (auto &event : all_events) {


rakhmets · 2025-11-13T08:16:09Z

src/api/cpp/telemetry/telemetry_plugin.h

+    std::unique_ptr<nixlTelemetryExporter> (*)(const nixlTelemetryExporterInitParams &init_params);
+
+class nixlTelemetryPlugin {
+private:


Please use the public before private order.

rakhmets · 2025-11-13T08:17:01Z

src/api/cpp/telemetry/telemetry_plugin.h

+    NixlTelemetryPluginApiVersion::V1;
+
+// Type alias for exporter creation function
+using ExporterCreatorFn =


Suggested change

using ExporterCreatorFn =

using exporter_creator_fn_t =

https://github.com/ai-dynamo/nixl/blob/main/docs/CodeStyle.md

rakhmets · 2025-11-13T08:21:05Z

src/core/nixl_agent.cpp

                std::string telemetry_file = std::string(telemetry_env_dir) + "/" + name;
                telemetry_ = std::make_unique<nixlTelemetry>(telemetry_file, backendEngines);
                NIXL_DEBUG << "NIXL telemetry is enabled with output file: " << telemetry_file;
+            } else if (telemetry_env_exporter_plugin_dir != nullptr &&


The nixlAgentData ctor is overloaded. Refactoring is necessary. Please move telemetry initialization into a separate private function.

rakhmets · 2025-11-13T09:30:33Z

src/api/cpp/telemetry/telemetry_plugin.h

+    }
+};
+
+// Plugin must implement these functions for dynamic loading


There is a lot of code duplication. telemetry_plugin.h looks like backend_plugin.h, and telemetry_plugin_manager.h looks like nixl_plugin_manager.h.

You can use the existing infrastructure by reorganizing the class structure (inheritance, composition, ...).
To better understand which path to choose, it is useful to have an example of using similar telemetry plugins. In other words, this PR should contain tests that use all the new functionality.

At the moment, I'm not entirely sure if telemetry needs to replicate the infrastructure that is used for NIXL plugins.

alexanderbilk requested a review from a team as a code owner October 29, 2025 09:51

pull-request-size bot added the size/XXL label Oct 29, 2025

github-actions bot added the external-contribution label Oct 29, 2025

alexanderbilk changed the title ~~Telemetry exporter plug-in infra~~ DRAFT: Telemetry exporter plug-in infra Oct 29, 2025

alexanderbilk marked this pull request as draft October 29, 2025 09:59

AlexNvd reviewed Oct 30, 2025

View reviewed changes

alexanderbilk force-pushed the telemetry-exporter branch 5 times, most recently from d0b114f to 280b472 Compare October 30, 2025 15:05

alexanderbilk marked this pull request as ready for review October 30, 2025 15:49

ColinNV reviewed Oct 30, 2025

View reviewed changes

alexanderbilk force-pushed the telemetry-exporter branch 3 times, most recently from b575c20 to e412bd1 Compare November 3, 2025 14:31

ColinNV reviewed Nov 4, 2025

View reviewed changes

ColinNV reviewed Nov 5, 2025

View reviewed changes

alexanderbilk force-pushed the telemetry-exporter branch 2 times, most recently from 8419b52 to 5fd32d2 Compare November 11, 2025 16:46

Telemetry exporter plug-in infra

ade7dda

DRAFT: Prometheus exporter implemetation Signed-off-by: Aleksandr Bilkovskii <[email protected]>

alexanderbilk force-pushed the telemetry-exporter branch from 5fd32d2 to ade7dda Compare November 11, 2025 16:49

rakhmets reviewed Nov 13, 2025

View reviewed changes


		# NIXL Prometheus Telemetry Exporter Plugin

		This telemetry exporter plugin exports NIXL telemetry events to Prometheus format via an HTTP endpoint that can be scraped by Prometheus servers.


		## Configuration

		To enable the Prometheus exporter, set the following environment variable:


		enum class NixlTelemetryPluginApiVersion : unsigned int { V1 = 1 };

		inline constexpr NixlTelemetryPluginApiVersion NIXL_TELEMETRY_PLUGIN_API_VERSION =

DRAFT: Telemetry exporter plug-in infra #962

Are you sure you want to change the base?

DRAFT: Telemetry exporter plug-in infra #962

Uh oh!

Conversation

alexanderbilk commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What?

Why?

How?

Uh oh!

copy-pr-bot bot commented Oct 29, 2025

Uh oh!

github-actions bot commented Oct 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ColinNV Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ColinNV Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ColinNV Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

alexanderbilk commented Oct 29, 2025 •

edited

Loading

ColinNV Nov 4, 2025 •

edited

Loading

ColinNV Nov 4, 2025 •

edited

Loading

ColinNV Nov 4, 2025 •

edited

Loading