Add OTel exporter for weave + example config#1827
Open
zbirenbaum wants to merge 2 commits intoNVIDIA:developfrom
Open
Add OTel exporter for weave + example config#1827zbirenbaum wants to merge 2 commits intoNVIDIA:developfrom
zbirenbaum wants to merge 2 commits intoNVIDIA:developfrom
Conversation
WalkthroughAdds a Weave OTel OpenTelemetry exporter (config + factory) and supporting docs/examples, plus 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 (weave_otel)
TelemetryFactory->>Runtime: Resolve config (endpoint, project, entity)
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: Send spans (via OTLP exporter)
OTLPCollector-->>WandB: (metadata/auth validated)
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 |
Signed-off-by: zbirenbaum <zachary.birenbaum@wandb.com>
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
Outdated
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.
Member
There was a problem hiding this comment.
You added this configuration, but the README wasn't updated.
examples/observability/simple_calculator_observability/README.md
The documentation should also be updated accordingly to reflect that W&B Weave has an OTel exporter.
Author
There was a problem hiding this comment.
Got it! It should be updated now. I put the OTel guide in the existing Weave docs but happy to place it into a separate one if that's preferable
Signed-off-by: zbirenbaum <zachary.birenbaum@wandb.com>
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
Comment on lines
+227
to
+230
| async def weave_otel_telemetry_exporter( | ||
| config: WeaveOtelTelemetryExporter, builder: Builder, | ||
| ) -> AsyncGenerator["OTLPSpanAdapterExporter", None]: | ||
| """Create a Weave OTel telemetry exporter.""" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the reported lint error on this file.
ruff check --select F821 packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.pyRepository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 648
🏁 Script executed:
#!/bin/bash
# Find where OTLPSpanAdapterExporter is defined
rg -n "class OTLPSpanAdapterExporter" --type pyRepository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 237
🏁 Script executed:
#!/bin/bash
# Check the current imports and structure of the file around the problematic section
head -230 packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py | tail -50Repository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 2318
🏁 Script executed:
#!/bin/bash
# Check the imports at the top of the register.py file
head -50 packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.pyRepository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 2392
🏁 Script executed:
#!/bin/bash
# Verify the full function context and check if there are any existing TYPE_CHECKING imports
grep -n "TYPE_CHECKING\|from typing\|from collections" packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.py | head -20Repository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 114
🏁 Script executed:
#!/bin/bash
# Check if there are TYPE_CHECKING imports elsewhere in the codebase for similar patterns
rg -n "TYPE_CHECKING" packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/ --type pyRepository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Verify the exact line numbers in the function to understand the context better
sed -n '225,260p' packages/nvidia_nat_opentelemetry/src/nat/plugins/opentelemetry/register.pyRepository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 1484
Add TYPE_CHECKING import to resolve Ruff F821 for forward reference in return annotation.
Line 229 has a forward reference "OTLPSpanAdapterExporter" in the return type annotation that triggers Ruff F821 because the name is not in scope at type-checking time. The local import on line 231 executes at runtime, not during type checking.
Add a module-level import guard to satisfy the type checker:
Suggested fix
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+ from nat.plugins.opentelemetry import OTLPSpanAdapterExporter
+
async def weave_otel_telemetry_exporter(
config: WeaveOtelTelemetryExporter, builder: Builder,
) -> AsyncGenerator["OTLPSpanAdapterExporter", None]:The local import inside the function (line 231) can remain for backward compatibility with runtime behavior.
🧰 Tools
🪛 Ruff (0.15.7)
[error] 229-229: Undefined name OTLPSpanAdapterExporter
(F821)
🤖 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 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.
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