chore: add telemetry sending by mckornfield · Pull Request #408 · NVIDIA-NeMo/Safe-Synthesizer

mckornfield · 2026-04-15T19:48:45Z

Summary

Pre-Review Checklist

Ensure that the following pass:

make format && make check or via prek validation.
make test passes locally
make test-e2e passes locally
make test-ci-container passes locally (recommended)
GPU CI status check passes -- comment /sync on this PR to trigger a run (auto-triggers on ready-for-review)

Pre-Merge Checklist

New or updated tests for any fix or new behavior
Updated documentation for new features and behaviors, including docstrings for API docs.

Other Notes

Closes Add opt out telemetry to understand library usage #382

Copilot

Pull request overview

Adds a new telemetry subsystem and wires it into the Safe Synthesizer SDK pipeline so runs can enqueue/send anonymized, bucketed event metrics.

Changes:

Introduces src/nemo_safe_synthesizer/telemetry.py with event schemas, bucketing helpers, payload builder, and an async telemetry handler with retry/DLQ behavior.
Adds unit tests for telemetry helpers, payload formatting, and handler send/retry behavior.
Emits telemetry from SafeSynthesizer lifecycle (completed/canceled/error) and adds httpx as a dependency.

Reviewed changes

Copilot reviewed 4 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`src/nemo_safe_synthesizer/telemetry.py`	New telemetry event models + handler (batching/sending/retry) and payload construction.
`tests/telemetry/test_telemetry.py`	New unit tests covering bucketing, event serialization, payload structure, and send/retry logic.
`src/nemo_safe_synthesizer/sdk/library_builder.py`	Emits a telemetry event from SDK execution paths and tracks idempotent emission.
`src/nemo_safe_synthesizer/evaluation/assets/text/multi_modal_tooltips.py`	Minor whitespace cleanup in tooltip text.
`pyproject.toml`	Adds `httpx` dependency required by telemetry sender.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-15T19:54:00Z

+    Used to avoid transmitting exact record counts in telemetry.
+    """


bucket_records() treats n <= 0 as the lowest bucket (e.g., 0 -> '1-100'), which is misleading and can happen for empty DataFrames. Consider handling non-positive values explicitly (e.g., return 'undefined' or a 0 bucket) before the first if n <= 100: branch.

Suggested change

Used to avoid transmitting exact record counts in telemetry.

"""

Used to avoid transmitting exact record counts in telemetry.

Non-positive counts are reported as undefined.

"""

if n <= 0:

return "undefined"

we don't run if there are too few records. might want to make the lower bucket the preflight threshold.

do we not have any way to run for under 200? I thought you could if you set holdout to 0

With the validation checks now, we hard crash on <200 rows. Not sure if that's in a constant we could (or would want to) import here.

I guess we'll get an error event back if someone runs with 150 records, so would be tidier to have that all fall under the same 1-200 bucket (or 1-199, not sure on a >= or > check) where we should see 100% errors. And then the 201-1000 bucket wouldn't ever have the <200 row error. Instead of 1-100 being all errors and 101-1000 being a mix of known error when <200 but success otherwise.

Copilot · 2026-04-15T19:54:01Z

            report=self.evaluator.report,
            generate_results=self.generator.gen_results,
        )
+        _emit_nss_telemetry(self, TaskStatusEnum.COMPLETED)
        return self


Telemetry is emitted as COMPLETED at the end of evaluate(). In run(), this happens before save_results(), so if saving results fails you’ll still have already emitted a completed event and the later error handler won’t emit due to _telemetry_emitted. Consider emitting the final COMPLETED status after save_results() in run() (and only emitting from evaluate() when evaluate() is called standalone), or adjust the idempotence logic so failures after evaluation can be reported accurately.

worth double checking

@mckornfield

moved these around for CLI and SDK

Copilot · 2026-04-15T19:54:02Z

+_deployment_type_raw = os.getenv("NEMO_DEPLOYMENT_TYPE", "sdk").lower()
+try:
+    DEPLOYMENT_TYPE = DeploymentTypeEnum(_deployment_type_raw)
+except ValueError:
+    valid_values = [e.value for e in DeploymentTypeEnum]
+    raise ValueError(
+        f"Invalid NEMO_DEPLOYMENT_TYPE: {_deployment_type_raw!r}. Must be one of: {valid_values}"
+    ) from None


DEPLOYMENT_TYPE is validated at import time and raises ValueError for an unexpected NEMO_DEPLOYMENT_TYPE. That can prevent importing the whole library even when telemetry is disabled/best-effort. Consider defaulting to DeploymentTypeEnum.UNDEFINED (and optionally logging) instead of raising during module import.

Copilot

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov · 2026-05-04T18:30:15Z

Codecov Report

❌ Patch coverage is 96.45777% with 13 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/nemo_safe_synthesizer/sdk/library_builder.py	83.63%	9 Missing ⚠️
src/nemo_safe_synthesizer/cli/run.py	90.00%	2 Missing ⚠️
src/nemo_safe_synthesizer/telemetry.py	95.12%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>

Signed-off-by: mkornfield <mkornfield@nvidia.com>

mckornfield · 2026-05-07T22:11:51Z

@@ -0,0 +1,58 @@
+# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.


I can also delete this hehe

binaryaaron

thanks, let's make this a top-level config object so it's more easily managed and piped with everything else.

need to update the user guide/docs to note the flag as well.

binaryaaron · 2026-05-07T23:57:50Z

            report=self.evaluator.report,
            generate_results=self.generator.gen_results,
        )
+        _emit_nss_telemetry(self, TaskStatusEnum.COMPLETED)
        return self


worth double checking

kendrickb-nvidia · 2026-05-08T00:22:21Z

Are we detecting/tracking CLI vs SDK usage? That was in the doc list last I saw.

If so, maybe that's the way to track internal slurm usage. Use an env var to add a suffix so it would show up as cli_nvidia_slurm or something?

Signed-off-by: mkornfield <mkornfield@nvidia.com>

binaryaaron · 2026-05-08T18:19:31Z

+    Used to avoid transmitting exact record counts in telemetry.
+    """


we don't run if there are too few records. might want to make the lower bucket the preflight threshold.

binaryaaron · 2026-05-08T18:20:39Z

            report=self.evaluator.report,
            generate_results=self.generator.gen_results,
        )
+        _emit_nss_telemetry(self, TaskStatusEnum.COMPLETED)
        return self


@mckornfield

Signed-off-by: mkornfield <mkornfield@nvidia.com>

kendrickb-nvidia

A few small items, but looks ready.

Signed-off-by: mkornfield <mkornfield@nvidia.com>

OOO

Copilot AI review requested due to automatic review settings April 15, 2026 19:48

mckornfield requested review from a team as code owners April 15, 2026 19:48

Copilot started reviewing on behalf of mckornfield April 15, 2026 19:49 View session

Copilot AI reviewed Apr 15, 2026

View reviewed changes

kendrickb-nvidia mentioned this pull request Apr 16, 2026

chore: Plan around telemetry implementation #407

Closed

7 tasks

mckornfield closed this Apr 27, 2026

mckornfield reopened this May 1, 2026

mckornfield force-pushed the nemo-telemetry-pt2/mck branch from f6b8263 to 6fc651e Compare May 1, 2026 13:45

github-code-quality Bot found potential problems May 1, 2026

View reviewed changes

Comment thread tests/telemetry/test_telemetry.py Fixed

Comment thread tests/telemetry/test_telemetry.py Fixed

Comment thread src/nemo_safe_synthesizer/telemetry.py Fixed

Comment thread src/nemo_safe_synthesizer/telemetry.py Fixed

Copilot AI review requested due to automatic review settings May 1, 2026 13:49

mckornfield force-pushed the nemo-telemetry-pt2/mck branch from 6fc651e to a1cc23c Compare May 1, 2026 13:49

Copilot started reviewing on behalf of mckornfield May 1, 2026 13:49 View session

Copilot AI reviewed May 1, 2026

View reviewed changes

Comment thread src/nemo_safe_synthesizer/telemetry.py Outdated

Comment thread src/nemo_safe_synthesizer/telemetry.py

Comment thread src/nemo_safe_synthesizer/sdk/library_builder.py Outdated

Comment thread README.md Outdated

Comment thread src/nemo_safe_synthesizer/telemetry.py Outdated

mckornfield force-pushed the nemo-telemetry-pt2/mck branch from a1cc23c to c23bf05 Compare May 4, 2026 18:25

mckornfield added 3 commits May 7, 2026 15:28

chore: add telemetry sending and notice

72e6565

Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>

fix casing on method

8cc0f09

Signed-off-by: mkornfield <mkornfield@nvidia.com>

some more tests, smoke test, run through a couple times

ac308c5

Signed-off-by: mkornfield <mkornfield@nvidia.com>

mckornfield force-pushed the nemo-telemetry-pt2/mck branch from c23bf05 to ac308c5 Compare May 7, 2026 17:33

lol

ae0a4f0

Signed-off-by: mkornfield <mkornfield@nvidia.com>

mckornfield commented May 7, 2026

View reviewed changes

Comment thread README.md Outdated

mckornfield requested review from alexahaushalter, binaryaaron and kendrickb-nvidia May 7, 2026 22:41

binaryaaron previously requested changes May 7, 2026

View reviewed changes

kendrickb-nvidia reviewed May 8, 2026

View reviewed changes

kendrick feedback

e367faf

Signed-off-by: mkornfield <mkornfield@nvidia.com>

github-code-quality Bot found potential problems May 8, 2026

View reviewed changes

Comment thread tests/telemetry/test_telemetry.py Dismissed

mckornfield requested review from binaryaaron and kendrickb-nvidia May 8, 2026 12:14

binaryaaron reviewed May 8, 2026

View reviewed changes

another one bites the dust

b0801c2

Signed-off-by: mkornfield <mkornfield@nvidia.com>

mckornfield requested a review from binaryaaron May 9, 2026 05:22

mckornfield added 3 commits May 9, 2026 05:35

lame

01bc35f

Signed-off-by: mkornfield <mkornfield@nvidia.com>

we have ways of making you pass

59ab25d

Signed-off-by: mkornfield <mkornfield@nvidia.com>

address kendrick comments

6bd0a74

Signed-off-by: mkornfield <mkornfield@nvidia.com>

nina-xu reviewed May 13, 2026

View reviewed changes

Comment thread script/slurm/slurm_srun.sh Outdated

Comment thread src/nemo_safe_synthesizer/sdk/config_builder.py Outdated

Comment thread src/nemo_safe_synthesizer/sdk/library_builder.py

Comment thread src/nemo_safe_synthesizer/sdk/library_builder.py

Comment thread README.md

address nina issues

181e1d6

Signed-off-by: mkornfield <mkornfield@nvidia.com>

kendrickb-nvidia previously approved these changes May 13, 2026

View reviewed changes

Comment thread .github/workflows/ci-checks.yml

Comment thread script/slurm/env_variables.sh Outdated

Comment thread src/nemo_safe_synthesizer/telemetry.py Outdated

Comment thread src/nemo_safe_synthesizer/telemetry.py Outdated

address more comments

e511159

Signed-off-by: mkornfield <mkornfield@nvidia.com>

mckornfield dismissed kendrickb-nvidia’s stale review via e511159 May 13, 2026 17:10

bucket assertions kms

5f4584e

Signed-off-by: mkornfield <mkornfield@nvidia.com>

kendrickb-nvidia approved these changes May 13, 2026

View reviewed changes

mckornfield merged commit dc0e9b3 into main May 13, 2026
19 checks passed

mckornfield deleted the nemo-telemetry-pt2/mck branch May 13, 2026 21:17

		Used to avoid transmitting exact record counts in telemetry.
		"""

-    Used to avoid transmitting exact record counts in telemetry.
-    """
+    Used to avoid transmitting exact record counts in telemetry.
+    Non-positive counts are reported as undefined.
+    """
+    if n <= 0:
+        return "undefined"

		@@ -0,0 +1,58 @@
		# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.

Uh oh!

Conversation

mckornfield commented Apr 15, 2026 • edited by kendrickb-nvidia Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Pre-Review Checklist

Pre-Merge Checklist

Other Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 15, 2026

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!

Copilot AI Apr 15, 2026

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!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

binaryaaron left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kendrickb-nvidia commented May 8, 2026

Uh oh!

mckornfield commented Apr 15, 2026 •

edited by kendrickb-nvidia

Loading

codecov Bot commented May 4, 2026 •

edited

Loading