-
Notifications
You must be signed in to change notification settings - Fork 412
Add DBNL Telemetry Exporter #1111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds DBNL telemetry exporter and docs: a new Changes
Sequence Diagram(s)sequenceDiagram
participant WF as NeMo Workflow
participant Exporter as DBNL Exporter (registered)
participant OTLP as OTLPSpanAdapterExporter
participant DBNL as DBNL Backend
WF->>Exporter: emit span(s)
Exporter->>OTLP: forward spans + endpoint & batch params
Note right of OTLP #f2f4f8: Inject headers:\nAuthorization: Bearer <token>\nx-dbnl-project-id: <id>
OTLP->>DBNL: POST /otel/v1/traces (batched)
DBNL-->>OTLP: 200 / ACK
OTLP-->>Exporter: success
Exporter-->>WF: export complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Potential review focus:
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (5){docs/source/**/*.md,**/README.@(md|ipynb)}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
docs/source/**/*.md📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
**/*⚙️ CodeRabbit configuration file
Files:
docs/source/**/*⚙️ CodeRabbit configuration file
Files:
🪛 markdownlint-cli2 (0.18.1)docs/source/workflows/observe/observe-workflow-with-dbnl.md93-93: Bare URL used (MD034, no-bare-urls) 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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/source/extend/telemetry-exporters.md(3 hunks)docs/source/workflows/observe/index.md(3 hunks)docs/source/workflows/observe/observe-workflow-with-dbnl.md(1 hunks)examples/observability/simple_calculator_observability/README.md(2 hunks)examples/observability/simple_calculator_observability/configs/config-dbnl.yml(1 hunks)packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
{docs/source/**/*.md,**/README.@(md|ipynb)}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{docs/source/**/*.md,**/README.@(md|ipynb)}: Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
Do not use deprecated names (Agent Intelligence toolkit, aiqtoolkit, AgentIQ, AIQ/aiq) in documentation unless explicitly referring to deprecated names
Never use “NAT”/“nat” abbreviations in documentation
Documentation must be clear/comprehensive; avoid TODOs/FIXMEs/placeholders; avoid offensive/outdated terms; ensure spelling is correct
Files:
docs/source/workflows/observe/observe-workflow-with-dbnl.mddocs/source/extend/telemetry-exporters.mddocs/source/workflows/observe/index.mdexamples/observability/simple_calculator_observability/README.md
docs/source/**/*.md
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
docs/source/**/*.md: Documentation sources are Markdown files under docs/source
Surround code entities with backticks in docs to avoid Vale false positives
Do not use words listed in ci/vale/styles/config/vocabularies/nat/reject.txt; accepted words in accept.txt are allowed
Files:
docs/source/workflows/observe/observe-workflow-with-dbnl.mddocs/source/extend/telemetry-exporters.mddocs/source/workflows/observe/index.md
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
docs/source/workflows/observe/observe-workflow-with-dbnl.mdexamples/observability/simple_calculator_observability/configs/config-dbnl.ymldocs/source/extend/telemetry-exporters.mdpackages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.pydocs/source/workflows/observe/index.mdexamples/observability/simple_calculator_observability/README.md
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
docs/source/workflows/observe/observe-workflow-with-dbnl.mdexamples/observability/simple_calculator_observability/configs/config-dbnl.ymldocs/source/extend/telemetry-exporters.mdpackages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.pydocs/source/workflows/observe/index.mdexamples/observability/simple_calculator_observability/README.md
docs/source/**/*
⚙️ CodeRabbit configuration file
This directory contains the source code for the documentation. All documentation should be written in Markdown format. Any image files should be placed in the
docs/source/_staticdirectory.
Files:
docs/source/workflows/observe/observe-workflow-with-dbnl.mddocs/source/extend/telemetry-exporters.mddocs/source/workflows/observe/index.md
**/configs/**
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Configuration files consumed by code should be stored next to that code in a configs/ folder
Files:
examples/observability/simple_calculator_observability/configs/config-dbnl.yml
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.
- If an example contains Python code, it should be placed in a subdirectory named
src/and should
contain apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/directory.- If an example contains YAML files, they should be placed in a subdirectory named
configs/. - If an example contains sample data files, they should be placed in a subdirectory nameddata/, and should
be checked into git-lfs.
Files:
examples/observability/simple_calculator_observability/configs/config-dbnl.ymlexamples/observability/simple_calculator_observability/README.md
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
{src/**/*.py,packages/*/src/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{src/**/*.py,packages/*/src/**/*.py}: All importable Python code must live under src/ or packages//src/
All public APIs must have Python 3.11+ type hints on parameters and return values
Prefer typing/collections.abc abstractions (e.g., Sequence over list)
Use typing.Annotated for units or metadata when useful
Treat pyright warnings as errors during development
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
packages/*/src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
If a package contains Python code, it must have tests in a tests/ directory at the same level as pyproject.toml
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
packages/**/*
⚙️ CodeRabbit configuration file
packages/**/*: - This directory contains optional plugin packages for the toolkit, each should contain apyproject.tomlfile. - Thepyproject.tomlfile should declare a dependency onnvidia-nator another package with a name starting
withnvidia-nat-. This dependency should be declared using~=<version>, and the version should be a two
digit version (ex:~=1.0).
- Not all packages contain Python code, if they do they should also contain their own set of tests, in a
tests/directory at the same level as thepyproject.tomlfile.
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
🧬 Code graph analysis (1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (3)
tests/nat/data_models/test_common.py (1)
BatchConfigMixin(158-159)src/nat/cli/type_registry.py (1)
register_telemetry_exporter(412-420)packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_adapter_exporter.py (1)
OTLPSpanAdapterExporter(25-94)
🪛 LanguageTool
examples/observability/simple_calculator_observability/README.md
[grammar] ~256-~256: Ensure spelling is correct
Context: ...ps://www.distributional.com/) helps you undestand your agent by analyzing your traces. 1...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
docs/source/workflows/observe/observe-workflow-with-dbnl.md
33-33: Bare URL used
(MD034, no-bare-urls)
97-97: Bare URL used
(MD034, no-bare-urls)
🪛 Ruff (0.14.1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
206-206: Unused function argument: builder
(ARG001)
🔇 Additional comments (5)
docs/source/workflows/observe/index.md (1)
145-145: LGTM! Consistent documentation updates.The DBNL references are properly integrated into the observability documentation, following the established patterns for other telemetry exporters.
Also applies to: 163-163, 212-212
examples/observability/simple_calculator_observability/README.md (1)
254-283: LGTM! Well-structured DBNL integration tutorial.The DBNL section follows the established pattern of other observability platform integrations and provides clear setup instructions.
docs/source/workflows/observe/observe-workflow-with-dbnl.md (1)
1-100: LGTM! Comprehensive DBNL workflow guide.The workflow guide provides clear, step-by-step instructions for integrating DBNL observability, consistent with other platform guides in the documentation.
docs/source/extend/telemetry-exporters.md (1)
248-248: LGTM! Consistent DBNL integration documentation.The DBNL exporter is properly documented alongside other pre-built integrations with correct installation and configuration details.
Also applies to: 282-282
examples/observability/simple_calculator_observability/configs/config-dbnl.yml (1)
28-32: LGTM! Proper configuration pattern.The DBNL tracing configuration correctly uses commented-out fields to encourage environment variable usage for sensitive credentials (API token and project ID), following security best practices.
examples/observability/simple_calculator_observability/README.md
Outdated
Show resolved
Hide resolved
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
Show resolved
Hide resolved
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
Outdated
Show resolved
Hide resolved
examples/observability/simple_calculator_observability/README.md
Outdated
Show resolved
Hide resolved
examples/observability/simple_calculator_observability/configs/config-dbnl.yml
Outdated
Show resolved
Hide resolved
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
Outdated
Show resolved
Hide resolved
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
Outdated
Show resolved
Hide resolved
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
Outdated
Show resolved
Hide resolved
|
@dbnl-renaud The Contributing Guidelines require that all contributors must sign-off their commits. We also have a CI pipeline that only runs once triggered by a team member. I would appreciate if you could address the first round of feedback before I kick off a CI run. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed comments. PTAL.
examples/observability/simple_calculator_observability/README.md
Outdated
Show resolved
Hide resolved
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
Outdated
Show resolved
Hide resolved
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
Outdated
Show resolved
Hide resolved
examples/observability/simple_calculator_observability/configs/config-dbnl.yml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (1)
205-226: Add validation for credentials to prevent silent failures.The function uses
config.api_tokenandconfig.project_iddirectly without validating they are non-empty. Even though these fields are required in the config class, users can explicitly pass empty strings, which will cause the exporter to silently fail or send invalid requests.Following the pattern from
langfuse_telemetry_exporter(lines 49-50),langsmith_telemetry_exporter(lines 84-85), andpatronus_telemetry_exporter(lines 149-150), add validation and environment variable fallback:from nat.plugins.opentelemetry import OTLPSpanAdapterExporter + api_token = config.api_token or os.environ.get("DBNL_API_TOKEN") + project_id = config.project_id or os.environ.get("DBNL_PROJECT_ID") + if not api_token or not project_id: + raise ValueError("API token and project ID are required for DBNL") + headers = { - "Authorization": f"Bearer {config.api_token}", - "x-dbnl-project-id": config.project_id, + "Authorization": f"Bearer {api_token}", + "x-dbnl-project-id": project_id, }This addresses the validation concern raised in the past review and adds environment variable support for better user experience.
🧹 Nitpick comments (1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (1)
197-203: Consider adding default values for environment variable fallback.The
api_tokenandproject_idfields are required (no default values), which differs from the pattern used inlangfuse_telemetry_exporter(lines 34-35),langsmith_telemetry_exporter(line 72), andpatronus_telemetry_exporter(line 137), where credentials havedefault=""and support environment variable fallback in the exporter function.For consistency and user convenience, consider following the same pattern:
- api_token: str = Field(description="The DBNL API token.") - project_id: str = Field(description="The DBNL project id.") + api_token: str = Field(description="The DBNL API token.", default="") + project_id: str = Field(description="The DBNL project id.", default="")Then add environment variable support in the exporter function (see next comment).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
{src/**/*.py,packages/*/src/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{src/**/*.py,packages/*/src/**/*.py}: All importable Python code must live under src/ or packages//src/
All public APIs must have Python 3.11+ type hints on parameters and return values
Prefer typing/collections.abc abstractions (e.g., Sequence over list)
Use typing.Annotated for units or metadata when useful
Treat pyright warnings as errors during development
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
packages/*/src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
If a package contains Python code, it must have tests in a tests/ directory at the same level as pyproject.toml
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
packages/**/*
⚙️ CodeRabbit configuration file
packages/**/*: - This directory contains optional plugin packages for the toolkit, each should contain apyproject.tomlfile. - Thepyproject.tomlfile should declare a dependency onnvidia-nator another package with a name starting
withnvidia-nat-. This dependency should be declared using~=<version>, and the version should be a two
digit version (ex:~=1.0).
- Not all packages contain Python code, if they do they should also contain their own set of tests, in a
tests/directory at the same level as thepyproject.tomlfile.
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
🧬 Code graph analysis (1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (3)
tests/nat/data_models/test_common.py (1)
BatchConfigMixin(158-159)src/nat/cli/type_registry.py (1)
register_telemetry_exporter(412-420)packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_adapter_exporter.py (1)
OTLPSpanAdapterExporter(25-94)
🪛 Ruff (0.14.1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
206-206: Unused function argument: builder
(ARG001)
1db6fc6 to
2670e06
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally misread that commits needed to be signed. I amended all the commits to include a signoff on each.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
docs/source/workflows/observe/observe-workflow-with-dbnl.md (1)
96-96: Wrap bare URL in angle brackets.The bare URL should be enclosed in angle brackets for proper markdown rendering and better accessibility, consistent with line 33.
Apply this diff:
-1. Navigate to your DBNL deployment (e.g. http://localhost:8080/) +1. Navigate to your DBNL deployment (e.g. <http://localhost:8080/>)packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (1)
211-214: Add environment variable fallback and credential validation.The exporter directly uses
config.api_tokenandconfig.project_idwithout fallback to environment variables or validation. This is inconsistent with other exporters in this file (langfuse, langsmith, patronus) which follow a pattern of environment variable fallback and validation.Following the pattern established by
langfuse_telemetry_exporter(lines 47-50) andlangsmith_telemetry_exporter(lines 83-85), apply this diff:+ api_token = config.api_token or os.environ.get("DBNL_API_TOKEN") + project_id = config.project_id or os.environ.get("DBNL_PROJECT_ID") + if not api_token or not project_id: + raise ValueError("API token and project ID are required for DBNL") + headers = { - "Authorization": f"Bearer {config.api_token}", - "x-dbnl-project-id": config.project_id, + "Authorization": f"Bearer {api_token}", + "x-dbnl-project-id": project_id, }
🧹 Nitpick comments (1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (1)
201-202: Consider adding default empty strings for consistency.For consistency with other exporters like
LangfuseTelemetryExporter(lines 34-35) andLangsmithTelemetryExporter(line 72), consider addingdefault=""to the api_token and project_id fields. This allows users to rely on environment variables without explicitly setting empty values in config.Apply this diff:
- api_token: str = Field(description="The DBNL API token.") - project_id: str = Field(description="The DBNL project id.") + api_token: str = Field(description="The DBNL API token.", default="") + project_id: str = Field(description="The DBNL project id.", default="")
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/source/extend/telemetry-exporters.md(3 hunks)docs/source/workflows/observe/index.md(3 hunks)docs/source/workflows/observe/observe-workflow-with-dbnl.md(1 hunks)examples/observability/simple_calculator_observability/README.md(2 hunks)examples/observability/simple_calculator_observability/configs/config-dbnl.yml(1 hunks)packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/source/extend/telemetry-exporters.md
- examples/observability/simple_calculator_observability/README.md
🧰 Additional context used
📓 Path-based instructions (11)
**/configs/**
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Configuration files consumed by code should be stored next to that code in a configs/ folder
Files:
examples/observability/simple_calculator_observability/configs/config-dbnl.yml
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
examples/observability/simple_calculator_observability/configs/config-dbnl.ymldocs/source/workflows/observe/index.mdpackages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.pydocs/source/workflows/observe/observe-workflow-with-dbnl.md
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
examples/observability/simple_calculator_observability/configs/config-dbnl.ymldocs/source/workflows/observe/index.mdpackages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.pydocs/source/workflows/observe/observe-workflow-with-dbnl.md
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.
- If an example contains Python code, it should be placed in a subdirectory named
src/and should
contain apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/directory.- If an example contains YAML files, they should be placed in a subdirectory named
configs/. - If an example contains sample data files, they should be placed in a subdirectory nameddata/, and should
be checked into git-lfs.
Files:
examples/observability/simple_calculator_observability/configs/config-dbnl.yml
{docs/source/**/*.md,**/README.@(md|ipynb)}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{docs/source/**/*.md,**/README.@(md|ipynb)}: Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
Do not use deprecated names (Agent Intelligence toolkit, aiqtoolkit, AgentIQ, AIQ/aiq) in documentation unless explicitly referring to deprecated names
Never use “NAT”/“nat” abbreviations in documentation
Documentation must be clear/comprehensive; avoid TODOs/FIXMEs/placeholders; avoid offensive/outdated terms; ensure spelling is correct
Files:
docs/source/workflows/observe/index.mddocs/source/workflows/observe/observe-workflow-with-dbnl.md
docs/source/**/*.md
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
docs/source/**/*.md: Documentation sources are Markdown files under docs/source
Surround code entities with backticks in docs to avoid Vale false positives
Do not use words listed in ci/vale/styles/config/vocabularies/nat/reject.txt; accepted words in accept.txt are allowed
Files:
docs/source/workflows/observe/index.mddocs/source/workflows/observe/observe-workflow-with-dbnl.md
docs/source/**/*
⚙️ CodeRabbit configuration file
This directory contains the source code for the documentation. All documentation should be written in Markdown format. Any image files should be placed in the
docs/source/_staticdirectory.
Files:
docs/source/workflows/observe/index.mddocs/source/workflows/observe/observe-workflow-with-dbnl.md
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
{src/**/*.py,packages/*/src/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{src/**/*.py,packages/*/src/**/*.py}: All importable Python code must live under src/ or packages//src/
All public APIs must have Python 3.11+ type hints on parameters and return values
Prefer typing/collections.abc abstractions (e.g., Sequence over list)
Use typing.Annotated for units or metadata when useful
Treat pyright warnings as errors during development
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
packages/*/src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
If a package contains Python code, it must have tests in a tests/ directory at the same level as pyproject.toml
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
packages/**/*
⚙️ CodeRabbit configuration file
packages/**/*: - This directory contains optional plugin packages for the toolkit, each should contain apyproject.tomlfile. - Thepyproject.tomlfile should declare a dependency onnvidia-nator another package with a name starting
withnvidia-nat-. This dependency should be declared using~=<version>, and the version should be a two
digit version (ex:~=1.0).
- Not all packages contain Python code, if they do they should also contain their own set of tests, in a
tests/directory at the same level as thepyproject.tomlfile.
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
🧬 Code graph analysis (1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (4)
tests/nat/data_models/test_common.py (1)
BatchConfigMixin(158-159)src/nat/cli/type_registry.py (1)
register_telemetry_exporter(412-420)src/nat/builder/builder.py (1)
Builder(68-290)packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_adapter_exporter.py (1)
OTLPSpanAdapterExporter(25-94)
🪛 markdownlint-cli2 (0.18.1)
docs/source/workflows/observe/observe-workflow-with-dbnl.md
30-30: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🪛 Ruff (0.14.1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
206-206: Unused function argument: builder
(ARG001)
|
/ok to test 2670e06 |
|
/ok to test 2670e06 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving with minor changes required for the workflow due to merged changes to develop
examples/observability/simple_calculator_observability/configs/config-dbnl.yml
Show resolved
Hide resolved
examples/observability/simple_calculator_observability/configs/config-dbnl.yml
Outdated
Show resolved
Hide resolved
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
Outdated
Show resolved
Hide resolved
Signed-off-by: Renaud Bourassa <[email protected]>
Signed-off-by: Renaud Bourassa <[email protected]>
Signed-off-by: Renaud Bourassa <[email protected]>
Signed-off-by: Renaud Bourassa <[email protected]>
Signed-off-by: Renaud Bourassa <[email protected]>
Signed-off-by: Renaud Bourassa <[email protected]>
2670e06 to
7ea5730
Compare
Signed-off-by: Renaud Bourassa <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (1)
198-204: Provide a sensible default forapi_url.Match prior feedback and enable near zero‑config local runs.
-class DBNLTelemetryExporter(BatchConfigMixin, TelemetryExporterBaseConfig, name="dbnl"): +class DBNLTelemetryExporter(BatchConfigMixin, TelemetryExporterBaseConfig, name="dbnl"): @@ - api_url: str | None = Field(description="The DBNL API url.", default=None) + api_url: str | None = Field( + description="The DBNL OTEL traces API URL.", + default="http://localhost:8080/api" + )docs/source/workflows/observe/observe-workflow-with-dbnl.md (1)
93-96: Wrap bare URL in angle brackets.Complies with MD034 and improves rendering.
-1. Navigate to your DBNL deployment (e.g. http://localhost:8080/) +1. Navigate to your DBNL deployment (e.g. <http://localhost:8080/>)
🧹 Nitpick comments (4)
docs/source/workflows/observe/observe-workflow-with-dbnl.md (3)
20-24: Use full product name on first mention.Replace the first mention with “NVIDIA NeMo Agent toolkit”; subsequent mentions can use “NeMo Agent toolkit”.
-This guide provides a step-by-step process to enable observability in a NeMo Agent toolkit workflow using DBNL for tracing. By the end of this guide, you will have: +This guide provides a step-by-step process to enable observability in an NVIDIA NeMo Agent toolkit workflow using DBNL for tracing. By the end of this guide, you will have:
52-57: Fix branding capitalization in headings (“Toolkit”).Headings should use “NeMo Agent Toolkit”.
-## Step 4: Install the NeMo Agent toolkit OpenTelemetry Subpackages +## Step 4: Install the NeMo Agent Toolkit OpenTelemetry Subpackages -## Step 5: Modify NeMo Agent toolkit Workflow Configuration +## Step 5: Modify NeMo Agent Toolkit Workflow ConfigurationAlso applies to: 59-61
33-40: Terminology nit: “Project ID” capitalization.Use “ID” (all caps) for consistency.
-7. Note down the **Project Id** for the project +7. Note down the **Project ID** for the projectpackages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (1)
212-218: Resolve TRY003 lint onValueErrormessages.Either shorten messages or add inline ignores to keep style consistent with the rest of this file.
- if not api_token: - raise ValueError("API token is required for DBNL") + if not api_token: + raise ValueError("API token is required for DBNL") # noqa: TRY003 @@ - if not project_id: - raise ValueError("Project id is required for DBNL") + if not project_id: + raise ValueError("Project id is required for DBNL") # noqa: TRY003 @@ - if not api_url: - raise ValueError("API url is required for DBNL") + if not api_url: + raise ValueError("API url is required for DBNL") # noqa: TRY003Also applies to: 224-228
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/source/extend/telemetry-exporters.md(3 hunks)docs/source/workflows/observe/index.md(3 hunks)docs/source/workflows/observe/observe-workflow-with-dbnl.md(1 hunks)examples/observability/simple_calculator_observability/README.md(2 hunks)examples/observability/simple_calculator_observability/configs/config-dbnl.yml(1 hunks)packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/observability/simple_calculator_observability/README.md
- docs/source/extend/telemetry-exporters.md
🧰 Additional context used
📓 Path-based instructions (11)
**/configs/**
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Configuration files consumed by code should be stored next to that code in a configs/ folder
Files:
examples/observability/simple_calculator_observability/configs/config-dbnl.yml
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
examples/observability/simple_calculator_observability/configs/config-dbnl.ymldocs/source/workflows/observe/index.mdpackages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.pydocs/source/workflows/observe/observe-workflow-with-dbnl.md
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
examples/observability/simple_calculator_observability/configs/config-dbnl.ymldocs/source/workflows/observe/index.mdpackages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.pydocs/source/workflows/observe/observe-workflow-with-dbnl.md
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.
- If an example contains Python code, it should be placed in a subdirectory named
src/and should
contain apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/directory.- If an example contains YAML files, they should be placed in a subdirectory named
configs/. - If an example contains sample data files, they should be placed in a subdirectory nameddata/, and should
be checked into git-lfs.
Files:
examples/observability/simple_calculator_observability/configs/config-dbnl.yml
{docs/source/**/*.md,**/README.@(md|ipynb)}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{docs/source/**/*.md,**/README.@(md|ipynb)}: Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
Do not use deprecated names (Agent Intelligence toolkit, aiqtoolkit, AgentIQ, AIQ/aiq) in documentation unless explicitly referring to deprecated names
Never use “NAT”/“nat” abbreviations in documentation
Documentation must be clear/comprehensive; avoid TODOs/FIXMEs/placeholders; avoid offensive/outdated terms; ensure spelling is correct
Files:
docs/source/workflows/observe/index.mddocs/source/workflows/observe/observe-workflow-with-dbnl.md
docs/source/**/*.md
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
docs/source/**/*.md: Documentation sources are Markdown files under docs/source
Surround code entities with backticks in docs to avoid Vale false positives
Do not use words listed in ci/vale/styles/config/vocabularies/nat/reject.txt; accepted words in accept.txt are allowed
Files:
docs/source/workflows/observe/index.mddocs/source/workflows/observe/observe-workflow-with-dbnl.md
docs/source/**/*
⚙️ CodeRabbit configuration file
This directory contains the source code for the documentation. All documentation should be written in Markdown format. Any image files should be placed in the
docs/source/_staticdirectory.
Files:
docs/source/workflows/observe/index.mddocs/source/workflows/observe/observe-workflow-with-dbnl.md
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
{src/**/*.py,packages/*/src/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{src/**/*.py,packages/*/src/**/*.py}: All importable Python code must live under src/ or packages//src/
All public APIs must have Python 3.11+ type hints on parameters and return values
Prefer typing/collections.abc abstractions (e.g., Sequence over list)
Use typing.Annotated for units or metadata when useful
Treat pyright warnings as errors during development
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
packages/*/src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
If a package contains Python code, it must have tests in a tests/ directory at the same level as pyproject.toml
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
packages/**/*
⚙️ CodeRabbit configuration file
packages/**/*: - This directory contains optional plugin packages for the toolkit, each should contain apyproject.tomlfile. - Thepyproject.tomlfile should declare a dependency onnvidia-nator another package with a name starting
withnvidia-nat-. This dependency should be declared using~=<version>, and the version should be a two
digit version (ex:~=1.0).
- Not all packages contain Python code, if they do they should also contain their own set of tests, in a
tests/directory at the same level as thepyproject.tomlfile.
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
🧬 Code graph analysis (1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (3)
tests/nat/data_models/test_common.py (1)
BatchConfigMixin(158-159)src/nat/cli/type_registry.py (1)
register_telemetry_exporter(412-420)packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_adapter_exporter.py (1)
OTLPSpanAdapterExporter(26-99)
🪛 markdownlint-cli2 (0.18.1)
docs/source/workflows/observe/observe-workflow-with-dbnl.md
93-93: Bare URL used
(MD034, no-bare-urls)
🪛 Ruff (0.14.1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
207-207: Unused function argument: builder
(ARG001)
214-214: Avoid specifying long messages outside the exception class
(TRY003)
217-217: Avoid specifying long messages outside the exception class
(TRY003)
226-226: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (2)
docs/source/workflows/observe/index.md (1)
136-146: DBNL additions look correct and sorted.Entries are alphabetized and link to the new page consistently across the list, table, and toctree. No further action needed.
Also applies to: 149-164, 201-213
examples/observability/simple_calculator_observability/configs/config-dbnl.yml (1)
17-30: LGTM: runnable minimal config.
- Uses env-based DBNL credentials (good).
- Updated calculator/current_datetime layout matches recent guidance.
- Tool names reflect both tools.
No changes requested.
Also applies to: 31-38, 46-51
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (1)
206-207: Add return type hint and address unused parameter.The function is missing a return type hint, which violates the coding guideline: "All public APIs must have Python 3.11+ type hints on parameters and return values." Additionally, the
builderparameter is unused, triggering Ruff ARG001.Apply this diff to add the return type and silence ARG001:
+from collections.abc import AsyncIterator +from typing import Any + @register_telemetry_exporter(config_type=DBNLTelemetryExporter) -async def dbnl_telemetry_exporter(config: DBNLTelemetryExporter, builder: Builder): +async def dbnl_telemetry_exporter( + config: DBNLTelemetryExporter, _builder: Builder +) -> AsyncIterator[Any]: """Create a DBNL telemetry exporter."""Note: Based on learnings from the past review, this was flagged but appears not to have been fully resolved yet.
🧹 Nitpick comments (2)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (2)
198-204: Consider documenting environment variable fallbacks in field descriptions.The field descriptions are concise but could be enhanced to inform users about the environment variable fallback mechanism. This would improve the developer experience and make zero-configuration setup more discoverable.
Apply this diff to enhance the field descriptions:
- api_url: str | None = Field(description="The DBNL API URL.", default=None) - api_token: str | None = Field(description="The DBNL API token.", default=None) - project_id: str | None = Field(description="The DBNL project id.", default=None) + api_url: str | None = Field( + description="The DBNL API URL. Falls back to DBNL_API_URL environment variable.", + default=None + ) + api_token: str | None = Field( + description="The DBNL API token. Falls back to DBNL_API_TOKEN environment variable.", + default=None + ) + project_id: str | None = Field( + description="The DBNL project ID. Falls back to DBNL_PROJECT_ID environment variable.", + default=None + )
212-227: Validation logic is solid and follows established patterns.The validation for required credentials with environment variable fallbacks is well-implemented. The error messages are clear, and the validation happens before constructing the exporter.
Optional improvement: The long error messages trigger Ruff TRY003 warnings. While they match existing patterns in this file (lines 50, 85), you could optionally define custom exception classes for more idiomatic error handling:
class DBNLConfigError(ValueError): """Configuration error for DBNL telemetry exporter.""" passHowever, this is a minor stylistic choice and the current approach is acceptable given the codebase patterns.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/source/extend/telemetry-exporters.md(3 hunks)packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/source/extend/telemetry-exporters.md
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: In code comments use the abbreviations: nat (API namespace/CLI), nvidia-nat (package), NAT (env var prefixes); never use these abbreviations in documentation
Follow PEP 20 and PEP 8 for Python style
Run yapf with column_limit=120; yapf is used for formatting (run second)
Indent with 4 spaces (no tabs) and end each file with a single trailing newline
Use ruff (ruff check --fix) as a linter (not formatter) per pyproject.toml; fix warnings unless explicitly ignored
Respect Python naming schemes: snake_case for functions/variables, PascalCase for classes, UPPER_CASE for constants
When re-raising exceptions, use bare raise to preserve stack trace; log with logger.error(), not logger.exception()
When catching and logging without re-raising, use logger.exception() to capture full stack trace
Provide Google-style docstrings for every public module, class, function, and CLI command
Docstring first line must be a concise description ending with a period
Validate and sanitize all user input, especially in web or CLI interfaces
Prefer httpx with SSL verification enabled by default and follow OWASP Top-10 recommendations
Use async/await for I/O-bound work (HTTP, DB, file I/O)
Cache expensive computations with functools.lru_cache or an external cache when appropriate
Leverage NumPy vectorized operations when beneficial and feasible
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
{src/**/*.py,packages/*/src/**/*.py}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{src/**/*.py,packages/*/src/**/*.py}: All importable Python code must live under src/ or packages//src/
All public APIs must have Python 3.11+ type hints on parameters and return values
Prefer typing/collections.abc abstractions (e.g., Sequence over list)
Use typing.Annotated for units or metadata when useful
Treat pyright warnings as errors during development
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
packages/*/src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
If a package contains Python code, it must have tests in a tests/ directory at the same level as pyproject.toml
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
packages/**/*
⚙️ CodeRabbit configuration file
packages/**/*: - This directory contains optional plugin packages for the toolkit, each should contain apyproject.tomlfile. - Thepyproject.tomlfile should declare a dependency onnvidia-nator another package with a name starting
withnvidia-nat-. This dependency should be declared using~=<version>, and the version should be a two
digit version (ex:~=1.0).
- Not all packages contain Python code, if they do they should also contain their own set of tests, in a
tests/directory at the same level as thepyproject.tomlfile.
Files:
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
🧬 Code graph analysis (1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (3)
tests/nat/data_models/test_common.py (1)
BatchConfigMixin(158-159)src/nat/cli/type_registry.py (1)
register_telemetry_exporter(412-420)packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/otlp_span_adapter_exporter.py (1)
OTLPSpanAdapterExporter(26-99)
🪛 Ruff (0.14.1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
207-207: Unused function argument: builder
(ARG001)
214-214: Avoid specifying long messages outside the exception class
(TRY003)
217-217: Avoid specifying long messages outside the exception class
(TRY003)
226-226: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (1)
229-237: LGTM! Exporter configuration is complete and correct.The OTLPSpanAdapterExporter is properly configured with all required parameters (endpoint, headers) and batch configuration is correctly passed through from the config object.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Renaud Bourassa <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/source/workflows/observe/observe-workflow-with-dbnl.md (1)
92-92: Format bare URL properly (MD034 violation).The bare URL at line 92 should be wrapped in angle brackets for proper Markdown rendering. This is a duplicate of the previous feedback that was partially addressed.
Apply this diff:
-1. Navigate to your DBNL deployment (e.g. http://localhost:8080/) +1. Navigate to your DBNL deployment (e.g. <http://localhost:8080/>)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/source/workflows/observe/observe-workflow-with-dbnl.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
{docs/source/**/*.md,**/README.@(md|ipynb)}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{docs/source/**/*.md,**/README.@(md|ipynb)}: Use the full name “NVIDIA NeMo Agent toolkit” on first use in documentation, then “NeMo Agent toolkit”; in headings use “NeMo Agent Toolkit” (capital T)
Do not use deprecated names (Agent Intelligence toolkit, aiqtoolkit, AgentIQ, AIQ/aiq) in documentation unless explicitly referring to deprecated names
Never use “NAT”/“nat” abbreviations in documentation
Documentation must be clear/comprehensive; avoid TODOs/FIXMEs/placeholders; avoid offensive/outdated terms; ensure spelling is correct
Files:
docs/source/workflows/observe/observe-workflow-with-dbnl.md
docs/source/**/*.md
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
docs/source/**/*.md: Documentation sources are Markdown files under docs/source
Surround code entities with backticks in docs to avoid Vale false positives
Do not use words listed in ci/vale/styles/config/vocabularies/nat/reject.txt; accepted words in accept.txt are allowed
Files:
docs/source/workflows/observe/observe-workflow-with-dbnl.md
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
{**/*.py,**/*.sh,**/*.md,**/*.toml,**/*.y?(a)ml,**/*.json,**/*.txt,**/*.ini,**/*.cfg,**/*.ipynb}: Every file must start with the standard SPDX Apache-2.0 header
Confirm copyright years are up to date when a file is changed
All source files must include the SPDX Apache-2.0 header template (copy from an existing file)
Files:
docs/source/workflows/observe/observe-workflow-with-dbnl.md
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
docs/source/workflows/observe/observe-workflow-with-dbnl.md
docs/source/**/*
⚙️ CodeRabbit configuration file
This directory contains the source code for the documentation. All documentation should be written in Markdown format. Any image files should be placed in the
docs/source/_staticdirectory.
Files:
docs/source/workflows/observe/observe-workflow-with-dbnl.md
🪛 markdownlint-cli2 (0.18.1)
docs/source/workflows/observe/observe-workflow-with-dbnl.md
92-92: Bare URL used
(MD034, no-bare-urls)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a fix for the markdown -- otherwise LGTM
|
/ok to test a00c040 |
Signed-off-by: Renaud Bourassa <[email protected]>
aa4055a to
61893fc
Compare
|
/ok to test 61893fc |
|
@dbnl-renaud thank you for your contribution! I will be merging as soon as CI is green :) |
|
/merge |
Description
Add support for exporting telemetry to DBNL.
Closes #1107
By Submitting this PR I confirm:
Summary by CodeRabbit
New Features
Documentation
Examples