Add OTel exporter for weave + example config#1827
Open
zbirenbaum wants to merge 5 commits intoNVIDIA:developfrom
Open
Add OTel exporter for weave + example config#1827zbirenbaum wants to merge 5 commits intoNVIDIA:developfrom
zbirenbaum wants to merge 5 commits intoNVIDIA:developfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new Weave OTel OpenTelemetry telemetry exporter and factory, a new example YAML enabling Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Runtime as "Nat Runtime"
participant TelemetryFactory as "WeaveOtel Factory"
participant OTLPCollector as "OTLP Endpoint"
participant WandB as "W&B Auth"
Runtime->>TelemetryFactory: Initialize telemetry using `weave_otel` config
TelemetryFactory->>WandB: Resolve API key (secret or WANDB_API_KEY)
WandB-->>TelemetryFactory: Return API key
TelemetryFactory->>OTLPCollector: Create OTLP exporter (headers: wandb-api-key, resource attrs)
Runtime->>OTLPCollector: Export spans via OTLP exporter
OTLPCollector-->>WandB: Validate auth/metadata
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
zbirenbaum
force-pushed
the
feat/weave-otel-config
branch
from
277e192 to
e1497fd
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (1)
213-217: Normalize user-facing term casing toOTelfor consistency.The new docstring/description uses
OpenTelemetry/OTEL; useOTelconsistently to match repository vocabulary expectations.As per coding guidelines, "Vale vocabulary explicitly allows ... 'OTel' ... prefer the exact spellings/casing in this allowlist (e.g., 'OTel' not 'OpenTelemetry')."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py` around lines 213 - 217, Update user-facing casing from "OpenTelemetry"/"OTEL" to "OTel" in the telemetry exporter docstring and the Field description: change the module/class docstring line that reads "A telemetry exporter to transmit traces to Weights & Biases Weave via OpenTelemetry." to use "OTel", and change the endpoint Field description text that currently says "The W&B Weave OTEL endpoint" to "The W&B Weave OTel endpoint". Ensure you only change the human-readable text (docstring and Field description) and do not alter the endpoint URL string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py`:
- Line 226: The public async generator function weave_otel_telemetry_exporter is
missing a return type annotation; update its signature to annotate the return
type as AsyncGenerator[OTLPSpanAdapterExporter, None] (importing AsyncGenerator
from typing and OTLPSpanAdapterExporter from its module if needed) so the
function's return type is explicit and conforms to Python 3.11+ type-hint
requirements.
---
Nitpick comments:
In `@packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py`:
- Around line 213-217: Update user-facing casing from "OpenTelemetry"/"OTEL" to
"OTel" in the telemetry exporter docstring and the Field description: change the
module/class docstring line that reads "A telemetry exporter to transmit traces
to Weights & Biases Weave via OpenTelemetry." to use "OTel", and change the
endpoint Field description text that currently says "The W&B Weave OTEL
endpoint" to "The W&B Weave OTel endpoint". Ensure you only change the
human-readable text (docstring and Field description) and do not alter the
endpoint URL string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f78776c7-7e41-4ef3-b5bc-bc5b8e0ffe70
📒 Files selected for processing (2)
examples/observability/simple_calculator_observability/configs/config-weave-otel.ymlpackages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (1)
226-226:⚠️ Potential issue | 🟠 MajorAdd explicit return type for this public async generator.
Line 226 is missing a return annotation; this exporter should declare an async-generator return type.
Suggested fix
+from collections.abc import AsyncGenerator @@ async def weave_otel_telemetry_exporter( - config: WeaveOtelTelemetryExporter, builder: Builder): + config: WeaveOtelTelemetryExporter, builder: Builder +) -> AsyncGenerator["OTLPSpanAdapterExporter", None]:#!/bin/bash # Verify telemetry exporter async defs in this file that still lack return annotations. rg -nP '^\s*async\s+def\s+\w+telemetry_exporter\s*\([^)]*\)\s*:' packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.pyExpected result: no matches for unannotated public exporter defs after fixes.
As per coding guidelines, all public APIs require Python 3.11+ type hints on parameters and return values.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py` at line 226, The public async generator weave_otel_telemetry_exporter lacks a return type; add an explicit async-generator annotation (e.g. -> AsyncGenerator[Any, None]) to its signature and import the corresponding typing symbols (AsyncGenerator and Any) at the top of the module so the function has a declared return type per the public API guidelines; update the function signature for weave_otel_telemetry_exporter and ensure any existing tests/type checks import names still resolve.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py`:
- Line 216: The description string for the W&B Weave OTEL endpoint uses all-caps
"OTEL"; update the literal to use repo-standard casing "OTel" by editing the
description argument (the string currently "The W&B Weave OTEL endpoint") in
register.py so it reads "The W&B Weave OTel endpoint" where the field/argument
is defined.
---
Duplicate comments:
In `@packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py`:
- Line 226: The public async generator weave_otel_telemetry_exporter lacks a
return type; add an explicit async-generator annotation (e.g. ->
AsyncGenerator[Any, None]) to its signature and import the corresponding typing
symbols (AsyncGenerator and Any) at the top of the module so the function has a
declared return type per the public API guidelines; update the function
signature for weave_otel_telemetry_exporter and ensure any existing tests/type
checks import names still resolve.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 78c38ad4-a222-4cb1-8d60-14474b0cb14c
📒 Files selected for processing (2)
examples/observability/simple_calculator_observability/configs/config-weave-otel.ymlpackages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
✅ Files skipped from review due to trivial changes (1)
- examples/observability/simple_calculator_observability/configs/config-weave-otel.yml
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
Outdated
Show resolved
Hide resolved
willkill07
added
feature request
willkill07
requested changes
Mar 30, 2026
Member
willkill07
left a comment
willkill07
left a comment
There was a problem hiding this comment.
Overall the new integration looks good. We just need to surface this to end users through documentation and example README updates.
examples/observability/simple_calculator_observability/configs/config-weave-otel.yml
Show resolved
Hide resolved
zbirenbaum
force-pushed
the
feat/weave-otel-config
branch
from
17010a4 to
3e15f51
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/source/run-workflows/observe/observe-workflow-with-weave.md (1)
256-259: Optional: vary repeated “Learn more…” phrasing in Resources.All links are useful; slight wording variation would read cleaner.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/source/run-workflows/observe/observe-workflow-with-weave.md` around lines 256 - 259, The four resource bullets that all start with "Learn more about..." should be reworded for variety: update the lines currently referencing https://weave-docs.wandb.ai/guides/tracking/tracing, https://weave-docs.wandb.ai/guides/tracking/trace-tree, https://weave-docs.wandb.ai/guides/tracking/redact-pii, and https://weave-docs.wandb.ai/guides/tracking/otel to use varied lead-ins (e.g., "Tracing overview:", "Navigate logged traces:", "PII redaction guide:", "OTel integration details:") while preserving the same link targets and intent. Ensure tone and punctuation match the surrounding doc style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py`:
- Around line 227-230: The forward reference "OTLPSpanAdapterExporter" used in
the return annotation of weave_otel_telemetry_exporter triggers Ruff F821
because the symbol isn't visible at type-check time; add a module-level guarded
import under if TYPE_CHECKING: from <module> import OTLPSpanAdapterExporter (and
import TYPE_CHECKING from typing) so the type checker sees the name, while
keeping the existing local runtime import inside weave_otel_telemetry_exporter
for backward compatibility. Ensure you reference the exact class name
OTLPSpanAdapterExporter and the function weave_otel_telemetry_exporter when
making the change.
---
Nitpick comments:
In `@docs/source/run-workflows/observe/observe-workflow-with-weave.md`:
- Around line 256-259: The four resource bullets that all start with "Learn more
about..." should be reworded for variety: update the lines currently referencing
https://weave-docs.wandb.ai/guides/tracking/tracing,
https://weave-docs.wandb.ai/guides/tracking/trace-tree,
https://weave-docs.wandb.ai/guides/tracking/redact-pii, and
https://weave-docs.wandb.ai/guides/tracking/otel to use varied lead-ins (e.g.,
"Tracing overview:", "Navigate logged traces:", "PII redaction guide:", "OTel
integration details:") while preserving the same link targets and intent. Ensure
tone and punctuation match the surrounding doc style.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 10d251b5-dbfc-4ee8-ba3d-e1e0700f5a0f
📒 Files selected for processing (3)
docs/source/run-workflows/observe/observe-workflow-with-weave.mdexamples/observability/simple_calculator_observability/README.mdpackages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
✅ Files skipped from review due to trivial changes (1)
- examples/observability/simple_calculator_observability/README.md
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
Outdated
Show resolved
Hide resolved
Member
|
/ok to test 3e15f51 |
Member
|
@zbirenbaum formatting check is failing. Please ensure that running CI locally passes |
zbirenbaum
force-pushed
the
feat/weave-otel-config
branch
from
5ea4d7c to
1a2f211
Compare
Author
Just updated, lint and format both pass locally now |
Member
|
/ok to test 1a2f211 |
Author
Sorry looks like vale flagged 'env var' and didn't run locally. Just fixed it |
zbirenbaum
force-pushed
the
feat/weave-otel-config
branch
from
704ad92 to
b53c86b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py (1)
245-245:⚠️ Potential issue | 🟠 MajorAdd explicit return type for the new public async generator.
Line 245 defines a public exporter factory without a return annotation. Please add
AsyncGenerator[...]to satisfy the Python API typing requirement.Proposed fix
+from collections.abc import AsyncGenerator +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from nat.plugins.opentelemetry import OTLPSpanAdapterExporter ... `@register_telemetry_exporter`(config_type=WeaveOtelTelemetryExporter) -async def weave_otel_telemetry_exporter(config: WeaveOtelTelemetryExporter, builder: Builder): +async def weave_otel_telemetry_exporter( + config: WeaveOtelTelemetryExporter, builder: Builder +) -> AsyncGenerator["OTLPSpanAdapterExporter", None]:#!/bin/bash # Verify whether the function has an explicit return annotation. rg -nP '^\s*async\s+def\s+weave_otel_telemetry_exporter\s*\([^)]*\)\s*->\s*AsyncGenerator' \ packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py # Show the full signature for manual confirmation. sed -n '240,250p' packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.pyAs per coding guidelines, “All public APIs require Python 3.11+ type hints on parameters and return values.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py` at line 245, The public async generator weave_otel_telemetry_exporter(config: WeaveOtelTelemetryExporter, builder: Builder) lacks an explicit return type; update its signature to include an AsyncGenerator[...] return annotation (e.g. -> AsyncGenerator[Any, None]) and add the necessary import (from typing import AsyncGenerator, Any) at the top of the module so the public API meets the Python 3.11+ typing requirement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/source/run-workflows/observe/observe-workflow-with-weave.md`:
- Line 259: Replace the non-descriptive link label "here" with a descriptive
phrase to satisfy markdownlint MD059; update the text in the sentence that
currently reads "Learn more about the OTel-based integration
[here](https://weave-docs.wandb.ai/guides/tracking/otel)." to something like
"Learn more about the OpenTelemetry-based integration in the OpenTelemetry
integration guide" (or similar descriptive text) and keep the existing URL
unchanged so the link target remains the same.
---
Duplicate comments:
In `@packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py`:
- Line 245: The public async generator weave_otel_telemetry_exporter(config:
WeaveOtelTelemetryExporter, builder: Builder) lacks an explicit return type;
update its signature to include an AsyncGenerator[...] return annotation (e.g.
-> AsyncGenerator[Any, None]) and add the necessary import (from typing import
AsyncGenerator, Any) at the top of the module so the public API meets the Python
3.11+ typing requirement.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5b235d40-a420-4c53-84af-d534416dd062
📒 Files selected for processing (2)
docs/source/run-workflows/observe/observe-workflow-with-weave.mdpackages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py
Member
|
/ok to test b53c86b |
Member
|
@zbirenbaum please see the CI failure log: https://github.com/NVIDIA/NeMo-Agent-Toolkit/actions/runs/24102872847/job/70318534500?pr=1827 You can run pre-commit (and all of CI) locally so please ensure that things pass locally so I don't have to continuously trigger CI. More information is in the contributing guide: https://docs.nvidia.com/nemo/agent-toolkit/latest/resources/contributing/testing/running-ci-locally.html |
zbirenbaum
force-pushed
the
feat/weave-otel-config
branch
from
03809ed to
2955097
Compare
Signed-off-by: zbirenbaum <zachary.birenbaum@wandb.com>
Signed-off-by: zbirenbaum <zachary.birenbaum@wandb.com>
Signed-off-by: zbirenbaum <zachary.birenbaum@wandb.com>
Signed-off-by: zbirenbaum <zachary.birenbaum@wandb.com>
zbirenbaum
force-pushed
the
feat/weave-otel-config
branch
from
2955097 to
4113127
Compare
Author
Ran the full CI, some failures in unrelated langchain example files I had to ignore but otherwise passes |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Weave is moving towards an OTel focused ecosystem. The existing weave integration makes use of Feedback and Evaluation callbacks via the SDK which aren't available via OTel, so this lives alongside the existing setup instead of replacing it.
By Submitting this PR I confirm:
Summary by CodeRabbit
New Features
Documentation