Skip to content

feat(observability)!: unify OpenTelemetry exporters#556

Open
willkill07 wants to merge 1 commit into
NVIDIA:mainfrom
willkill07:feat/opentelemetry-sinks
Open

feat(observability)!: unify OpenTelemetry exporters#556
willkill07 wants to merge 1 commit into
NVIDIA:mainfrom
willkill07:feat/opentelemetry-sinks

Conversation

@willkill07

@willkill07 willkill07 commented Jul 25, 2026

Copy link
Copy Markdown
Member

Warning

BREAKING CHANGE — observability configuration and exporter APIs: Observability configuration version 3 replaces the standalone opentelemetry and openinference exporter shapes with typed opentelemetry.endpoints. Direct Rust, Python, Node.js, Go, and C callers must replace OpenInferenceConfig/OpenInferenceSubscriber with OpenTelemetryConfig/OpenTelemetrySubscriber and select type = "openinference". Version-2 OTLP component shapes are rejected under version 3. Projection mark, alias, semantic-selector, attribute-mapping, exclusion, and content-capture controls are removed in favor of fixed full, gen_ai, and openinference behavior.

Overview

Unifies Relay's OTLP exporters behind typed OpenTelemetry endpoints and adds a standards-only GenAI semantic projection while preserving the complete Relay and OpenInference projections as explicit endpoint types.

  • I confirm this contribution is my own work, or I have the right to submit it under this project's license.
  • I searched existing issues and open pull requests, and this does not duplicate existing work.

Details

  • Adds the public OpenTelemetryType discriminator with full, gen_ai, and openinference projections and one independently managed OpenTelemetrySubscriber API.
  • Replaces the standalone OpenInference public surface across Rust, Python, Node.js, Go, and the C FFI with typed OpenTelemetry construction.
  • Adds observability configuration version 3 with repeatable opentelemetry.endpoints, per-endpoint header_env, heterogeneous and repeated endpoint fan-out, atomic construction, and aggregate flush/shutdown handling.
  • Makes top-level plugin configuration lists concatenate across layers, including ATOF sinks, ATIF storage destinations, and OpenTelemetry endpoints.
  • Implements the pinned OpenTelemetry GenAI semantic projection for agents, LLMs, tools, embedders, and retrievers without emitting nemo_relay.* or opt-in content attributes; unsupported scopes are omitted while preserving descendant ancestry.
  • Keeps fixed default behavior for the complete Relay and OpenInference projections while removing configurable mark, alias, mapping, selector, exclusion, and content-capture controls.
  • Makes OpenTelemetry and OpenInference support unconditional instead of Cargo feature-gated.
  • Updates OpenTelemetry crates to the 0.32 release line, updates object_store to 0.14.1, removes the resolved deny.toml advisory exceptions, and refreshes generated Rust attribution data.
  • Updates CLI diagnostics, editor metadata, integrations, examples, package READMEs, migration guidance, observability documentation, and the maintainer observability skill.
  • Adds projection, lifecycle, fan-out, configuration layering, binding, OTLP wire, and migration regression coverage.

Migration examples and the version-2 to version-3 mapping are documented in docs/reference/migration-guides.mdx and docs/configure-plugins/observability/opentelemetry.mdx.

Where should the reviewer start?

Start with crates/core/src/observability/otel.rs for the unified subscriber and endpoint construction, crates/core/src/observability/plugin_component.rs for version-3 multi-endpoint configuration and fan-out, and crates/core/src/observability/otel_genai.rs for the GenAI projection contract. Binding migrations are anchored by crates/ffi/src/api/observability.rs.

Validation performed on commit d3d3703f:

just test-rust
just test-python                 # 540 passed
just test-node                   # 285 passed
XDG_CONFIG_HOME=<isolated> just test-go
just docs
just docs-linkcheck
uv run pre-commit run --all-files

The first local Go invocation inherited a user-level version-2 observability file; the isolated CI-equivalent rerun passed. Documentation validation completed with zero errors; the external redirect comparison was skipped after FDR returned 403.

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

Summary by CodeRabbit

  • New Features

    • Added typed OpenTelemetry projections: full, gen_ai, and openinference.
    • Added multi-endpoint observability configuration with endpoint-specific headers, transports, and resource attributes.
    • Added GenAI semantic-convention tracing and improved trace/span ID correlation.
    • Added endpoint fan-out with isolated delivery and lifecycle management across supported language bindings.
  • Breaking Changes

    • Observability configuration now requires version 3 and explicit nonblank endpoints.
    • Standalone OpenInference configuration and subscriber APIs were replaced by typed OpenTelemetry interfaces.
  • Documentation

    • Updated configuration, migration, integration, and troubleshooting guidance.

Closes RELAY-503

Signed-off-by: Will Killian <wkillian@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The observability system now uses version 3 typed OpenTelemetry endpoints for full, gen_ai, and openinference projections. Standalone OpenInference APIs and feature gates were removed, multi-endpoint fan-out and validation were added, bindings were updated, and documentation and tests were migrated.

Changes

Typed OpenTelemetry observability

Layer / File(s) Summary
Core projection and subscriber model
crates/core/src/observability/*
Adds typed projection selection, GenAI attribute projection, Relay-derived IDs, endpoint validation, owned exporter runtimes, projection-specific marks, and revised shutdown behavior.
Version 3 plugin configuration
crates/core/src/observability/plugin_component.rs, crates/core/src/plugin.rs
Replaces legacy OTLP/OpenInference sections with typed endpoint arrays, validates endpoint and header fields, supports transactional construction, and concatenates selected configuration lists by precedence.
Binding API migration
crates/ffi/*, go/nemo_relay/*, crates/node/*, crates/python/*
Updates C FFI, Go, Node.js, and Python APIs to construct typed OpenTelemetry subscribers and removes standalone OpenInference and attribute-mapping surfaces.
CLI diagnostics and integration wiring
crates/cli/*, integrations/*
Adds gRPC connectivity checks, renders one destination per endpoint, updates lifecycle/error handling, and migrates integration configuration to version 3.
Tests, docs, and metadata
crates/*/tests/*, docs/*, README.md, ATTRIBUTIONS-Rust.md, deny.toml
Migrates fixtures and coverage, documents typed endpoint configuration and migration behavior, updates license handling, and refreshes generated attributions.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Config
  participant ObservabilityPlugin
  participant OpenTelemetrySubscriber
  participant OTLPCollector
  Config->>ObservabilityPlugin: load version 3 endpoint list
  ObservabilityPlugin->>OpenTelemetrySubscriber: build one subscriber per endpoint
  ObservabilityPlugin->>OpenTelemetrySubscriber: register fan-out dispatcher
  OpenTelemetrySubscriber->>OTLPCollector: export typed projection
  ObservabilityPlugin->>OpenTelemetrySubscriber: deregister, flush, shutdown
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 70.19% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title follows Conventional Commits and accurately summarizes the breaking observability exporter unification.
Description check ✅ Passed The description matches the template with Overview, Details, reviewer start, and related issue sections filled in and sufficiently specific.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added size:XXL PR is very large Feature a new feature breaking PR introduces a breaking change labels Jul 25, 2026
@github-actions github-actions Bot added lang:go PR changes/introduces Go code lang:js PR changes/introduces Javascript/Typescript code lang:python PR changes/introduces Python code lang:rust PR changes/introduces Rust code labels Jul 25, 2026
@willkill07
willkill07 marked this pull request as ready for review July 25, 2026 02:34
@willkill07
willkill07 requested review from a team as code owners July 25, 2026 02:34
@github-actions

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

License Diff

Compared against origin/main.

Lockfile license changes

Lockfile License Changes

Rust

Added

  • aws-lc-rs 1.17.3 (ISC)
  • aws-lc-sys 0.43.0 (Apache-2.0 OR BSD-3-Clause OR ISC OR MIT)
  • cmake 0.1.58 (Apache-2.0)
  • crc-fast 1.10.0 (Apache-2.0)
  • dunce 1.0.5 (Apache-2.0)
  • fs_extra 1.3.0 (MIT)
  • jni 0.22.4 (Apache-2.0)
  • jni-macros 0.22.4 (Apache-2.0)
  • jni-sys 0.4.1 (Apache-2.0)
  • jni-sys-macros 0.4.1 (Apache-2.0)
  • opentelemetry-semantic-conventions 0.32.1 (Apache-2.0)
  • rustls-platform-verifier 0.7.0 (Apache-2.0)
  • rustls-platform-verifier-android 0.1.1 (Apache-2.0)
  • same-file 1.0.6 (MIT)
  • simd_cesu8 1.2.0 (Apache-2.0)
  • simdutf8 0.1.5 (Apache-2.0)
  • spin 0.10.1 (MIT)
  • tonic-types 0.14.5 (MIT)
  • walkdir 2.5.0 (MIT)
  • webpki-root-certs 1.0.9 (CDLA-Permissive-2.0)
  • winapi-util 0.1.11 (MIT)

Removed

  • openinference-semantic-conventions 0.1.1 (Apache-2.0)

Updated/Changed

itertools

Before:

  • itertools 0.14.0 (Apache-2.0)

After:

  • itertools 0.14.0 (Apache-2.0)
  • itertools 0.15.0 (Apache-2.0)

md-5

Before:

  • md-5 0.10.6 (MIT OR Apache-2.0)

After:

  • md-5 0.11.0 (MIT OR Apache-2.0)

object_store

Before:

  • object_store 0.13.2 (Apache-2.0)

After:

  • object_store 0.14.1 (Apache-2.0)

opentelemetry

Before:

  • opentelemetry 0.31.0 (Apache-2.0)

After:

  • opentelemetry 0.32.0 (Apache-2.0)

opentelemetry-http

Before:

  • opentelemetry-http 0.31.0 (Apache-2.0)

After:

  • opentelemetry-http 0.32.0 (Apache-2.0)

opentelemetry-otlp

Before:

  • opentelemetry-otlp 0.31.1 (Apache-2.0)

After:

  • opentelemetry-otlp 0.32.0 (Apache-2.0)

opentelemetry-proto

Before:

  • opentelemetry-proto 0.31.0 (Apache-2.0)

After:

  • opentelemetry-proto 0.32.0 (Apache-2.0)

opentelemetry_sdk

Before:

  • opentelemetry_sdk 0.31.0 (Apache-2.0)

After:

  • opentelemetry_sdk 0.32.1 (Apache-2.0)

quick-xml

Before:

  • quick-xml 0.39.4 (MIT)

After:

  • quick-xml 0.41.0 (MIT)

reqwest

Before:

  • reqwest 0.12.28 (Apache-2.0)

After:

  • reqwest 0.12.28 (Apache-2.0)
  • reqwest 0.13.4 (Apache-2.0)

wasm-streams

Before:

  • wasm-streams 0.4.2 (Apache-2.0)

After:

  • wasm-streams 0.4.2 (Apache-2.0)
  • wasm-streams 0.5.0 (Apache-2.0)

Node

Added

  • None

Removed

  • None

Updated/Changed

  • None

Python

Added

  • None

Removed

  • None

Updated/Changed

  • None
Status output
[license-diff] selected languages: rust, node, python
[license-diff] generating current inventory
[license-diff] current: generating Rust inventory
[license-diff] current: Rust inventory complete (449 packages)
[license-diff] current: generating Node inventory
[license-diff] current: Node inventory complete (367 packages)
[license-diff] current: generating Python inventory
[license-diff] current: Python inventory complete (105 packages)
[license-diff] current inventory complete
[license-diff] checking out base ref origin/main into a temporary worktree
[license-diff] base: generating Rust inventory
[license-diff] base: Rust inventory complete (426 packages)
[license-diff] base: generating Node inventory
[license-diff] base: Node inventory complete (367 packages)
[license-diff] base: generating Python inventory
[license-diff] base: Python inventory complete (105 packages)
[license-diff] base inventory complete
[license-diff] removing temporary base worktree
[license-diff] comparing inventories
[license-diff] rendering Markdown output
[license-diff] done

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 24

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
python/tests/test_types.py (1)

566-598: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Parametrize duplicated config tests across OpenTelemetry/OpenInference.

TestOpenTelemetryTypes.test_config_defaults_mutation_and_repr/test_config_rejects_invalid_map_values and their TestOpenInferenceTypes counterparts are now nearly identical bodies, differing only by the otel_type string literal ("full" vs "openinference"), since both now share the same OpenTelemetryConfig class. Consider merging into @pytest.mark.parametrize("otel_type", ["full", "openinference"]) to remove the duplication.

As per path instructions for python/tests/**/*.py: "Prefer pytest.mark.parametrize over creating individual tests for different input types."

Also applies to: 690-721

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@python/tests/test_types.py` around lines 566 - 598, Merge the duplicated
OpenTelemetry and OpenInference configuration tests into parametrized tests
using pytest.mark.parametrize("otel_type", ["full", "openinference"]). Update
both test_config_defaults_mutation_and_repr and
test_config_rejects_invalid_map_values to accept otel_type and construct
OpenTelemetryConfig with it, preserving all existing assertions and validation
coverage.

Source: Path instructions

crates/python/tests/coverage/py_types_coverage_tests.rs (1)

446-501: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Deduplicate the per-type lifecycle/invalid-input tests.

test_openinference_typed_otel_config_and_subscriber_cover_lifecycle and test_openinference_typed_otel_config_rejects_invalid_inputs are near-verbatim copies of the full variants; only the type string and subscriber name prefix differ. Driving both from a loop over ["full", "gen_ai", "openinference"] also gains gen_ai coverage, which is currently missing here.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/python/tests/coverage/py_types_coverage_tests.rs` around lines 446 -
501, Deduplicate the lifecycle and invalid-input coverage by replacing the
separate full/openinference test variants with loop-driven tests over the type
strings "full", "gen_ai", and "openinference". Parameterize
PyOpenTelemetryConfig construction and subscriber-name prefixes from the loop
while preserving all existing assertions and lifecycle operations, thereby
adding gen_ai coverage without duplicating test bodies.
crates/core/src/observability/otel.rs (1)

687-698: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

u32::MAX attribute caps remove the only bound on per-span memory.

The rationale for lifting the 128 default is sound, but u32::MAX is effectively unbounded. The full projection flattens arbitrary event JSON into attributes via push_top_level_json_attributes, so a long-running agent span that accumulates many marks and large payloads now grows the SDK's in-memory span without limit until it ends. Prefer a large finite ceiling that still fails safe.

🔧 Proposed fix
-        .with_max_attributes_per_span(u32::MAX)
-        .with_max_attributes_per_event(u32::MAX);
+        // Large but finite: consumers legitimately emit hundreds of attributes on
+        // long-running spans, while a hard ceiling keeps per-span memory bounded.
+        .with_max_attributes_per_span(65_536)
+        .with_max_attributes_per_event(4_096);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/core/src/observability/otel.rs` around lines 687 - 698, Replace the
unbounded u32::MAX limits in the SdkTracerProvider builder with a large finite
attribute cap that accommodates expanded event payloads while preserving a safe
upper bound for per-span memory. Apply the same bounded value to both
with_max_attributes_per_span and with_max_attributes_per_event, leaving the
resource and ID generator configuration unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.agents/skills/maintain-observability/SKILL.md:
- Around line 36-37: Update the validation checklist in the observability skill
to explicitly require the Rust test target for exporter-configuration and
lifecycle changes, in addition to event-field changes. Ensure both exporter and
lifecycle paths cannot bypass the existing required Rust validation.
- Around line 29-32: Update the affected lists in maintain-observability
documentation with complete lead-in sentences, including “Keep these surfaces
synchronized:” and “Use the following design checklist:” where applicable, and
add question marks to every checklist question. Preserve the existing list
content and structure while applying the same punctuation fixes to the sections
referenced by the additional ranges.

In `@ATTRIBUTIONS-Rust.md`:
- Around line 2911-2915: Update the attribution generator/template that emits
dependency license sections such as the aws-lc-rs 1.17.3 block, adding blank
lines around Markdown headings and fenced license content and specifying a
language such as text on license fences. Regenerate ATTRIBUTIONS-Rust.md so all
affected license blocks are corrected consistently rather than editing a single
occurrence.
- Around line 24039-24048: Update the Apache License boilerplate in the
attribution block to use the appendix’s exact “[yyyy]” and “[name of copyright
owner]” placeholders, preserving the surrounding text verbatim.

In `@crates/cli/src/diagnostics/mod.rs`:
- Around line 653-679: The endpoint probes in the loop are awaited serially,
causing total diagnostics time to grow with the number of endpoints. Update this
endpoint-checking flow to create one future per endpoint and join them
concurrently, preserving the existing per-endpoint probe selection, details,
failure handling, and output ordering.

In `@crates/cli/src/diagnostics/probes.rs`:
- Around line 87-98: Update the endpoint connection logic around
parsed.host_str() and TcpStream::connect to pass an unbracketed IPv6 host, using
Url::host() or stripping surrounding brackets before connecting. Preserve
existing host validation and port handling, and add an IPv6 coverage case to
tcp_tests.

In `@crates/core/src/observability/openinference.rs`:
- Around line 683-711: Define module-local constants for all OpenInference
attribute keys used by start_attributes, end_attributes,
push_projected_mark_attributes, common_attributes, and the push/remove helpers,
following the const-block pattern in otel_genai.rs. Replace every matching
inline key literal, including the duplicated tool.name and
tool_call.function.name keys, with the shared constants while preserving the
existing attribute values and behavior.

In `@crates/core/src/observability/otel_genai.rs`:
- Around line 307-312: Update the tool description lookup in the event attribute
handling to remove the generic "description" fallback or constrain it to the
top-level event object. Preserve the existing namespaced and tool_description
lookups, and ensure nested usage/request/response values cannot populate
gen_ai.tool.description.
- Around line 404-417: Align the status-code comparison in push_error_attributes
and set_span_status_from_event_metadata so both use the same case-sensitivity
policy. Preserve consistent handling of metadata values such as "error": the
span status and error.type attribute must agree, and valid case variants must
not trigger invalid_status_code warnings.

In `@crates/core/src/observability/otel.rs`:
- Around line 562-603: Bound both owned exporter runtime handoffs: in
crates/core/src/observability/otel.rs lines 562-603, update
build_owned_tracer_provider to use result_receiver.recv_timeout with a duration
derived from config.timeout, and document that construction blocks the calling
thread; in lines 324-339, call Runtime::shutdown_timeout inside the spawned
runtime closure with the configured timeout so teardown cannot wait indefinitely
before thread joining.

In `@crates/core/src/observability/plugin_component.rs`:
- Around line 2032-2048: Update the diagnostic message passed to
push_policy_diag in the REMOVED-field branch so removed OpenTelemetry fields
explicitly state that the control was removed and provide the version-3
migration guidance, matching the legacy openinference wording near line 1895;
retain the existing unknown-field message for non-REMOVED fields.
- Around line 1762-1775: Update the blank-endpoint and invalid-transport errors
in the OpenTelemetry endpoint validation to prefix messages with the in-scope
endpoint index using the existing `endpoints[{index}]` format. Also update the
`initialization_fails_for_invalid_enabled_file_exporters` assertion to match the
new indexed transport error while preserving its existing validation intent.

In `@crates/core/tests/integration/middleware_tests.rs`:
- Around line 848-862: Restore explicit tool-projection coverage in the OTel
setup around from_tracer_provider by constructing the subscriber with
MarkProjection::Tool where required, while preserving the existing pending-mark
assertions. Add an OpenInference-specific assertion for openinference.span.kind
equal to "CHAIN" on the OpenInference span, so its exporter-specific projection
remains distinguished from the full projection.

In `@crates/core/tests/unit/observability/openinference_tests.rs`:
- Around line 716-734: Update invalid_grpc_headers_are_rejected to call
OpenTelemetrySubscriber::new directly instead of creating a Tokio runtime and
wrapping the call in runtime.block_on; preserve the existing invalid-header
error assertion and configuration unchanged.
- Around line 684-700: Strengthen
config_defaults_and_builder_overrides_are_applied by reading back and asserting
every configured builder value, including the header via
OpenTelemetryConfig::header(), rather than only checking construction and
lifecycle calls. Update grpc_metadata_and_runtime_builder_paths_succeed
similarly to assert returned results and use unwrap() for expected success
instead of discarding them with ok(). Avoid starting real runtimes and exporters
for these unit tests where possible, while preserving coverage of the promised
configuration and error behavior.
- Around line 3210-3214: Replace the hardcoded 4096 in the span eviction test’s
span_count calculation with the reachable COMPLETED_SPAN_CONTEXT_LIMIT constant,
preserving the existing +2 setup so the test continues to exercise eviction when
the configured limit changes.

In `@crates/core/tests/unit/observability/otel_tests.rs`:
- Around line 585-593: Update grpc_config_owns_its_tokio_runtime to call
subscriber.force_flush() before shutdown, asserting the result succeeds so the
test exercises dispatch through the subscriber’s owned Tokio runtime rather than
only validating construction.
- Around line 641-672: Update direct_config_rejects_process_global_otel_headers
so each child process captures its output and verifies the targeted test
actually ran, such as requiring the libtest summary to report one test passed,
before accepting status.success(). Preserve the existing error assertion and
environment-variable loop, while ensuring a renamed or relocated test cannot
silently pass with zero tests executed.

In `@crates/core/tests/unit/observability/plugin_component_tests.rs`:
- Around line 166-201: Harden start_otlp_capture_server by matching the
buffered, tolerant approach used by start_otlp_http_collector: replace the
single-byte header loop with buffered reads, handle chunked or missing
content-length without an unconditional unwrap, and provide an explicit
assertion message when the header is absent. Send the captured body before
writing the response, while preserving the existing endpoint and channel
behavior.

In `@crates/core/tests/unit/plugin_tests.rs`:
- Around line 497-499: Update the assertions for the non-observability component
alpha in the merge_plugin_config_value tests to expect its overlay config.list
as json!([9]) rather than concatenating lists; adjust the nearby test comment to
state that list concatenation applies only to observability components while
non-observability lists are replaced.

In `@docs/build-plugins/language-binding/code-examples.mdx`:
- Around line 183-187: Update both Python and Node.js examples to invoke each
exporter’s documented flush operation before calling plugin.clear(). Preserve
the existing shutdown sequence otherwise, ensuring queued native subscriber
callbacks are delivered before the plugin is cleared.

In `@docs/resources/troubleshooting/trace-incident-runbook.mdx`:
- Around line 152-156: Update the troubleshooting guidance around the typed
OpenTelemetry export settings and the following attribute-check bullet so
`nemo_relay.uuid` and `nemo_relay.parent_uuid` are expected only for `full` and
`openinference` endpoints, not `gen_ai` endpoints; preserve the existing
validation guidance for non-`gen_ai` projections.

In `@examples/switchyard/hermes-ollama-plugins.toml`:
- Around line 57-59: Remove the duplicate TOML table headers in
examples/switchyard/hermes-ollama-plugins.toml: retain exactly one
[components.config] declaration at lines 57-59 and exactly one
[[components.config.opentelemetry.endpoints]] declaration before its endpoint
fields at lines 91-100.

In `@go/nemo_relay/openinference_test.go`:
- Around line 22-24: Prevent the shared NewOtelTestServer handler from blocking
during shutdown by making its request channel send non-blocking or increasing
its buffer; update go/nemo_relay/openinference_test.go lines 22-24 and
go/nemo_relay/otel_test.go lines 205-246 consistently, including the requests
channel in TestOpenTelemetrySubscriberExportsGenAIAgentProjection.

---

Outside diff comments:
In `@crates/core/src/observability/otel.rs`:
- Around line 687-698: Replace the unbounded u32::MAX limits in the
SdkTracerProvider builder with a large finite attribute cap that accommodates
expanded event payloads while preserving a safe upper bound for per-span memory.
Apply the same bounded value to both with_max_attributes_per_span and
with_max_attributes_per_event, leaving the resource and ID generator
configuration unchanged.

In `@crates/python/tests/coverage/py_types_coverage_tests.rs`:
- Around line 446-501: Deduplicate the lifecycle and invalid-input coverage by
replacing the separate full/openinference test variants with loop-driven tests
over the type strings "full", "gen_ai", and "openinference". Parameterize
PyOpenTelemetryConfig construction and subscriber-name prefixes from the loop
while preserving all existing assertions and lifecycle operations, thereby
adding gen_ai coverage without duplicating test bodies.

In `@python/tests/test_types.py`:
- Around line 566-598: Merge the duplicated OpenTelemetry and OpenInference
configuration tests into parametrized tests using
pytest.mark.parametrize("otel_type", ["full", "openinference"]). Update both
test_config_defaults_mutation_and_repr and
test_config_rejects_invalid_map_values to accept otel_type and construct
OpenTelemetryConfig with it, preserving all existing assertions and validation
coverage.
🪄 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: ASSERTIVE

Plan: Enterprise

Run ID: accc008a-b705-4c25-a893-4fe6a54a6e66

📥 Commits

Reviewing files that changed from the base of the PR and between 9d16c07 and d3d3703.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (118)
  • .agents/skills/maintain-observability/SKILL.md
  • ATTRIBUTIONS-Rust.md
  • Cargo.toml
  • README.md
  • about.toml
  • crates/cli/Cargo.toml
  • crates/cli/README.md
  • crates/cli/src/diagnostics/mod.rs
  • crates/cli/src/diagnostics/probes.rs
  • crates/cli/src/error.rs
  • crates/cli/src/gateway/tls.rs
  • crates/cli/src/process/launcher.rs
  • crates/cli/tests/cli_tests.rs
  • crates/cli/tests/coverage/agents/launcher_tests.rs
  • crates/cli/tests/coverage/shared/doctor_tests.rs
  • crates/cli/tests/coverage/shared/error_tests.rs
  • crates/cli/tests/coverage/shared/plugins_tests.rs
  • crates/cli/tests/coverage/shared/server_tests.rs
  • crates/cli/tests/coverage/shared/session_tests.rs
  • crates/core/Cargo.toml
  • crates/core/README.md
  • crates/core/src/lib.rs
  • crates/core/src/observability/manual.rs
  • crates/core/src/observability/mod.rs
  • crates/core/src/observability/openinference.rs
  • crates/core/src/observability/otel.rs
  • crates/core/src/observability/otel_genai.rs
  • crates/core/src/observability/plugin_component.rs
  • crates/core/src/plugin.rs
  • crates/core/tests/integration/atif_storage_tests.rs
  • crates/core/tests/integration/middleware_tests.rs
  • crates/core/tests/unit/observability/exporter_parity_tests.rs
  • crates/core/tests/unit/observability/manual_tests.rs
  • crates/core/tests/unit/observability/openinference_tests.rs
  • crates/core/tests/unit/observability/otel_tests.rs
  • crates/core/tests/unit/observability/plugin_component_tests.rs
  • crates/core/tests/unit/plugin_tests.rs
  • crates/ffi/Cargo.toml
  • crates/ffi/README.md
  • crates/ffi/nemo_relay.h
  • crates/ffi/src/api/mod.rs
  • crates/ffi/src/api/observability.rs
  • crates/ffi/src/types/mod.rs
  • crates/ffi/tests/integration/api/coverage_sweeps_tests.rs
  • crates/ffi/tests/unit/api/core_tests.rs
  • crates/ffi/tests/unit/api/coverage_sweeps_tests.rs
  • crates/ffi/tests/unit/api/plugin_tests.rs
  • crates/ffi/tests/unit/api/registry_tests.rs
  • crates/ffi/tests/unit/types_tests.rs
  • crates/node/Cargo.toml
  • crates/node/README.md
  • crates/node/observability.d.ts
  • crates/node/observability.js
  • crates/node/src/api/mod.rs
  • crates/node/tests/observability_plugin_tests.mjs
  • crates/node/tests/openinference_tests.mjs
  • crates/node/tests/otel_tests.mjs
  • crates/pii-redaction/Cargo.toml
  • crates/pii-redaction/tests/unit/component_tests.rs
  • crates/python/Cargo.toml
  • crates/python/src/py_types/mod.rs
  • crates/python/src/py_types/observability.rs
  • crates/python/tests/coverage/py_types_coverage_tests.rs
  • deny.toml
  • docs/about-nemo-relay/agent-runtime-primer.mdx
  • docs/about-nemo-relay/concepts/codecs.mdx
  • docs/about-nemo-relay/concepts/events.mdx
  • docs/about-nemo-relay/concepts/plugins.mdx
  • docs/about-nemo-relay/concepts/subscribers.mdx
  • docs/about-nemo-relay/ecosystem.mdx
  • docs/about-nemo-relay/overview.mdx
  • docs/about-nemo-relay/release-notes/index.mdx
  • docs/build-plugins/language-binding/code-examples.mdx
  • docs/configure-plugins/about.mdx
  • docs/configure-plugins/observability/about.mdx
  • docs/configure-plugins/observability/atif.mdx
  • docs/configure-plugins/observability/atof.mdx
  • docs/configure-plugins/observability/configuration.mdx
  • docs/configure-plugins/observability/openinference.mdx
  • docs/configure-plugins/observability/opentelemetry.mdx
  • docs/configure-plugins/pii-redaction/configuration.mdx
  • docs/configure-plugins/plugin-configuration-files.mdx
  • docs/configure-plugins/switchyard/configuration.mdx
  • docs/contribute/runtime-contract-docs.mdx
  • docs/getting-started/configuration.mdx
  • docs/integrate-into-frameworks/provider-response-codecs.mdx
  • docs/nemo-relay-cli/claude-code.mdx
  • docs/reference/migration-guides.mdx
  • docs/resources/glossary.mdx
  • docs/resources/troubleshooting/trace-incident-runbook.mdx
  • docs/supported-integrations/openclaw-plugin.mdx
  • examples/switchyard/hermes-ollama-plugins.toml
  • go/nemo_relay/README.md
  • go/nemo_relay/coverage_gap_test.go
  • go/nemo_relay/nemo_relay.go
  • go/nemo_relay/observability_plugin.go
  • go/nemo_relay/observability_plugin_test.go
  • go/nemo_relay/openinference_test.go
  • go/nemo_relay/otel_test.go
  • go/nemo_relay/top_level_coverage_test.go
  • integrations/coding-agents/README.md
  • integrations/openclaw/README.md
  • integrations/openclaw/index.ts
  • integrations/openclaw/openclaw.plugin.json
  • integrations/openclaw/src/config.ts
  • integrations/openclaw/src/health.ts
  • integrations/openclaw/test/config.test.ts
  • integrations/openclaw/test/live-smoke.test.ts
  • python/nemo_relay/README.md
  • python/nemo_relay/__init__.py
  • python/nemo_relay/__init__.pyi
  • python/nemo_relay/_native.pyi
  • python/nemo_relay/observability.py
  • python/nemo_relay/observability.pyi
  • python/tests/integrations/deepagents_tests/test_deepagents_integration.py
  • python/tests/test_observability_plugin.py
  • python/tests/test_types.py
  • scripts/licensing/attributions_lockfile_md.py
💤 Files with no reviewable changes (7)
  • crates/core/src/observability/manual.rs
  • crates/python/src/py_types/mod.rs
  • crates/ffi/tests/unit/types_tests.rs
  • python/nemo_relay/init.pyi
  • crates/cli/tests/coverage/shared/error_tests.rs
  • crates/ffi/src/types/mod.rs
  • crates/core/tests/unit/observability/manual_tests.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

🛑 Comments failed to post (24)
.agents/skills/maintain-observability/SKILL.md (2)

29-32: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add lead-in sentences and list punctuation.

These changed lists need complete lead-ins, and the checklist questions should end with ? to comply with the Markdown documentation guidelines. For example, introduce the sections with “Keep these surfaces synchronized:” and “Use the following design checklist:”.

Also applies to: 34-51, 64-66

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.agents/skills/maintain-observability/SKILL.md around lines 29 - 32, Update
the affected lists in maintain-observability documentation with complete lead-in
sentences, including “Keep these surfaces synchronized:” and “Use the following
design checklist:” where applicable, and add question marks to every checklist
question. Preserve the existing list content and structure while applying the
same punctuation fixes to the sections referenced by the additional ranges.

Source: Coding guidelines


36-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover exporter and lifecycle validation explicitly.

This skill covers exporter behavior and subscriber configuration, but its validation guidance only requires the Rust test target when event fields change. Add an explicit validation requirement for exporter-configuration and lifecycle changes so those paths cannot bypass the required Rust checks.

Also applies to: 49-51

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.agents/skills/maintain-observability/SKILL.md around lines 36 - 37, Update
the validation checklist in the observability skill to explicitly require the
Rust test target for exporter-configuration and lifecycle changes, in addition
to event-field changes. Ensure both exporter and lifecycle paths cannot bypass
the existing required Rust validation.

Source: Learnings

ATTRIBUTIONS-Rust.md (2)

2911-2915: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Regenerate the attribution file without Markdown lint violations.

This block—and the other reported license blocks—needs blank lines around headings and fences, and the license fence should specify a language such as text. Fix the attribution generator/template rather than hand-editing one occurrence.

🧰 Tools
🪛 markdownlint-cli2 (0.23.0)

[warning] 2911-2911: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


[warning] 2914-2914: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above

(MD022, blanks-around-headings)


[warning] 2914-2914: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


[warning] 2915-2915: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)


[warning] 2915-2915: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ATTRIBUTIONS-Rust.md` around lines 2911 - 2915, Update the attribution
generator/template that emits dependency license sections such as the aws-lc-rs
1.17.3 block, adding blank lines around Markdown headings and fenced license
content and specifying a language such as text on license fences. Regenerate
ATTRIBUTIONS-Rust.md so all affected license blocks are corrected consistently
rather than editing a single occurrence.

Source: Linters/SAST tools


24039-24048: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the affected section with line numbers.
sed -n '24030,24055p' ATTRIBUTIONS-Rust.md

# Check whether the file uses square-bracket placeholders elsewhere in Apache text.
rg -n '\{yyyy\}|\{name of copyright owner\}|\[yyyy\]|\[name of copyright owner\]' ATTRIBUTIONS-Rust.md

# Find nearby Apache license text blocks to compare formatting.
rg -n 'Apache License|Copyright \{yyyy\}|Copyright \[yyyy\]' ATTRIBUTIONS-Rust.md | head -n 40

Repository: NVIDIA/NeMo-Relay

Length of output: 17841


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect only the changed hunks for the affected Apache appendix block.
git diff --unified=0 -- ATTRIBUTIONS-Rust.md | rg -n -C 3 'Copyright \{yyyy\} \{name of copyright owner\}|\bfields enclosed by brackets\b|\[yyyy\] \[name of copyright owner\]'

# Show the exact final-state lines around the reported location.
sed -n '24036,24050p' ATTRIBUTIONS-Rust.md

Repository: NVIDIA/NeMo-Relay

Length of output: 155


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Compare a bracket-style and brace-style Apache appendix block side by side.
sed -n '300,316p' ATTRIBUTIONS-Rust.md
printf '\n---\n'
sed -n '590,606p' ATTRIBUTIONS-Rust.md

Repository: NVIDIA/NeMo-Relay

Length of output: 3648


Restore the Apache appendix placeholders
Keep this block verbatim with [yyyy] and [name of copyright owner]; the {} placeholders diverge from the Apache 2.0 appendix text.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ATTRIBUTIONS-Rust.md` around lines 24039 - 24048, Update the Apache License
boilerplate in the attribution block to use the appendix’s exact “[yyyy]” and
“[name of copyright owner]” placeholders, preserving the surrounding text
verbatim.

Source: MCP tools

crates/cli/src/diagnostics/mod.rs (1)

653-679: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Probe endpoints concurrently; serialized timeouts now scale with endpoint count.

Each iteration awaits a probe with NETWORK_TIMEOUT. With the version-3 model an operator can configure many endpoints, so doctor worst-case latency becomes endpoints × NETWORK_TIMEOUT instead of the previous bounded two sections. Collecting futures and joining keeps ordering while bounding wall time to a single timeout.

♻️ Suggested direction
-    let mut checks = Vec::with_capacity(endpoints.len());
-    for (index, endpoint) in endpoints.iter().enumerate() {
+    let mut probes = Vec::with_capacity(endpoints.len());
+    for (index, endpoint) in endpoints.iter().enumerate() {
         ...
-        checks.push(match endpoint.get("endpoint").and_then(Value::as_str) {
+        probes.push(async move { /* same match, awaited inside */ });
     }
-    checks
+    futures::future::join_all(probes).await
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    let mut probes = Vec::with_capacity(endpoints.len());
    for (index, endpoint) in endpoints.iter().enumerate() {
        let endpoint_type = endpoint
            .get("type")
            .and_then(Value::as_str)
            .unwrap_or("unknown");
        let label = "OpenTelemetry endpoint";
        let transport = endpoint
            .get("transport")
            .and_then(Value::as_str)
            .unwrap_or("http_binary");
        probes.push(async move {
            match endpoint.get("endpoint").and_then(Value::as_str) {
                Some(url) => {
                    let mut check = if transport == "grpc" {
                        probe_tcp_named(label, url).await
                    } else {
                        probe_http_named(label, url).await
                    };
                    check.details = format!("endpoints[{index}] ({endpoint_type}): {}", check.details);
                    check
                }
                None => Check {
                    name: label,
                    status: Status::Fail,
                    details: format!("endpoints[{index}] ({endpoint_type}): endpoint is required"),
                },
            }
        });
    }
    futures::future::join_all(probes).await
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/cli/src/diagnostics/mod.rs` around lines 653 - 679, The endpoint
probes in the loop are awaited serially, causing total diagnostics time to grow
with the number of endpoints. Update this endpoint-checking flow to create one
future per endpoint and join them concurrently, preserving the existing
per-endpoint probe selection, details, failure handling, and output ordering.
crates/cli/src/diagnostics/probes.rs (1)

87-98: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

rust url crate host_str IPv6 brackets included Url::host Host::Ipv6

💡 Result:

In the Rust url crate, the behavior regarding IPv6 brackets is as follows: When you use Url::host_str, the returned string representation of an IPv6 address always includes square brackets ([ and ]) [1][2][3]. This aligns with the requirement to distinguish the colons in an IPv6 address from the port separator [1][4]. Regarding the Host enum: - The Host::Ipv6 variant holds the parsed IPv6 address as an Ipv6Addr type, not as a string [1][5]. - Therefore, the variant itself does not contain brackets [1][5]. - When you use Url::host, it returns an Option<Host<&str>> where the Ipv6 variant contains the structured Ipv6Addr [3]. If you are manually parsing a host string and need to handle brackets, the Host::parse method expects the input to be either a domain or an IPv6 address enclosed in square brackets [1][4]. When using the internal parsing logic, the brackets are consumed during the parsing of the IPv6 address [4][6].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant probe implementation and its tests.
sed -n '1,180p' crates/cli/src/diagnostics/probes.rs

printf '\n--- tests mentioning tcp/grpc/ipv6 ---\n'
rg -n "tcp_tests|ipv6|host_str|host\(|TcpStream::connect|grpc_endpoint_port" crates/cli/src/diagnostics/probes.rs

Repository: NVIDIA/NeMo-Relay

Length of output: 6555


🌐 Web query:

Rust url crate Url::host_str IPv6 returns brackets or bare address docs

💡 Result:

Yes, in the Rust url crate, the Url::host_str() method returns the IPv6 address enclosed within square brackets ([ and ]) [1][2][3]. According to the official documentation and source code, for an Ipv6 host, Url::host_str() provides the serialized address in the format specified by RFC 5952, which includes enclosing the address in brackets [1][4][5]. This behavior is consistent across versions of the crate [2][6].

Citations:


🌐 Web query:

tokio TcpStream::connect bracketed IPv6 hostname resolve fail ToSocketAddrs

💡 Result:

In Rust and the Tokio runtime, the ToSocketAddrs trait is used to resolve various address formats into SocketAddr instances, which TcpStream::connect then uses to establish connections [1][2][3]. The issue where a bracketed IPv6 address (e.g., "[::1]:8080") might appear to fail or cause unexpected behavior usually stems from how specific libraries or downstream components handle string parsing, rather than a failure in Tokio's core ToSocketAddrs implementation itself [4]. Key points regarding IPv6 bracketed hostnames and ToSocketAddrs: 1. Parsing and ToSocketAddrs: The standard library's ToSocketAddrs implementation for &str is designed to handle strings by either parsing them as a SocketAddr (which correctly supports the [ipv6]:port format) or by performing a DNS lookup [3][4]. If the string is a valid bracketed IPv6 address and port, it should resolve correctly as a local address [3][5]. 2. Downstream library issues: Many reported failures involving bracketed IPv6 addresses are not rooted in tokio::net::TcpStream::connect, but in libraries that consume these strings (e.g., rustls or tonic) [6][7]. These libraries often parse the host component of a URL separately. If they expect a raw IP address (without brackets) but receive a URI-formatted host (with brackets), they will fail to validate the address [6][7]. 3. Troubleshooting tips: - Verify if the error is coming from Tokio's connection attempt or a higher-level parsing step (like TLS configuration or URI parsing) [7]. - If you are building a URI or manually constructing a string, ensure that the consumer of that string expects the bracketed format. - If you need to manually resolve a host to avoid ambiguous errors from ToSocketAddrs (which returns multiple addresses and can hide specific connection errors), use tokio::net::lookup_host to inspect individual addresses before attempting to connect [8][2][4]. If TcpStream::connect is returning an error, check the specific I/O error returned. Because ToSocketAddrs can return multiple addresses, if all connection attempts fail, TcpStream::connect returns the error from the last attempted address, which might not be the primary or expected address, potentially leading to misleading error messages [9][8].

Citations:


🌐 Web query:

Rust ToSocketAddrs bracketed IPv6 string "[::1]" hostname resolution

💡 Result:

In Rust, the ToSocketAddrs trait supports bracketed IPv6 strings (e.g., "[::1]:8080") because it treats them as valid string representations of a SocketAddr [1][2]. When you pass a string to to_socket_addrs, the implementation first attempts to parse the string as a SocketAddr [3]. Because SocketAddr and SocketAddrV6 have FromStr implementations that recognize the bracketed IPv6 format—consistent with IETF RFC 5952—this parsing succeeds [2][4]. If the string is successfully parsed as a SocketAddr, the trait returns an iterator containing that specific address rather than attempting DNS hostname resolution [3]. In summary: - Bracketed IPv6 strings are supported and parsed directly as socket addresses [2][4]. - DNS resolution is only triggered if the string cannot be parsed as a valid SocketAddr (or an IpAddr with a port) [3]. - To use an IPv6 address with a port, the bracketed format [ipv6]:port is required, as the brackets are necessary to disambiguate the IPv6 colons from the port separator [2][5]. Top results: [1][3][2][4]

Citations:


🌐 Web query:

std net ToSocketAddrs (&str, u16) bracketed IPv6 host "[::1]" invalid

💡 Result:

In Rust, the implementation of ToSocketAddrs for (&str, u16) does not support passing a bracketed IPv6 address as the host string because it expects the host to be either a raw IP address (parsable as IpAddr) or a hostname for DNS resolution [1][2]. When you provide a bracketed IPv6 string (e.g., "[::1]") to the host parameter, the following occurs: 1. The implementation first attempts to parse the host string as an IpAddr [1][2]. Because "[::1]" includes brackets, it is not a valid IpAddr, and this parsing step fails [1][3][2]. 2. It then falls back to treating the host string as a hostname and passes it to the system's DNS resolver [1][2]. Since "[::1]" is not a valid hostname, the lookup fails [3][4]. To use an IPv6 address with ToSocketAddrs in this way, you have two primary options: - Use the raw IP address string: Pass the IPv6 address without brackets, such as ("::1", 8080) [5][6]. - Parse the address manually: If you must handle bracketed strings (often because they originate from URI parsers), strip the brackets before passing the address to the tuple [3][4]. Note that the single &str implementation of ToSocketAddrs does support the bracketed format (e.g., "[::1]:8080") because it first attempts to parse the entire string as a SocketAddr, which explicitly supports that format [5][1][6][7]. The limitation is specific to the split (&str, u16) tuple implementation [1][2].

Citations:


🌐 Web query:

Tokio TcpStream::connect tuple host port bracketed IPv6 host string

💡 Result:

When connecting to an IPv6 address using tokio::net::TcpStream::connect, the host portion of the address must be enclosed in square brackets if a port is specified [1][2]. The expected format is "[host]:port" [1][2]. For example: TcpStream::connect("[2001:db8::1]:8080").await?; This bracketed format is necessary to disambiguate the colon used in the IPv6 address from the colon used to separate the host from the port [1]. If you pass a raw IPv6 address without brackets to a function expecting a "host:port" string, parsing will fail because the multiple colons make the port identification ambiguous [3]. It is important to note that this requirement applies to the string representation of the address [1][2]. If you are working with higher-level URI parsing (such as those provided by the url crate), be aware that the host field may contain the brackets, and you should ensure the final string passed to TcpStream::connect is formatted correctly as [host]:port [3]. If you are manually constructing strings, always include the brackets around the IPv6 portion [2]. Finally, TcpStream::connect accepts any type that implements the ToSocketAddrs trait [4]. While passing a formatted string is common, you can also resolve addresses into SocketAddr objects (which include IPv6 variants) beforehand to avoid string parsing issues entirely [5][6].

Citations:


Strip brackets before TcpStream::connect Url::host_str() returns [::1], but TcpStream::connect((host, port)) expects a raw socket host, so bracketed IPv6 literals are treated as hostnames and fail. Use Url::host() or strip the brackets before connecting, and add an IPv6 case to tcp_tests.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/cli/src/diagnostics/probes.rs` around lines 87 - 98, Update the
endpoint connection logic around parsed.host_str() and TcpStream::connect to
pass an unbracketed IPv6 host, using Url::host() or stripping surrounding
brackets before connecting. Preserve existing host validation and port handling,
and add an IPv6 coverage case to tcp_tests.
crates/core/src/observability/openinference.rs (1)

683-711: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Hoist the OpenInference attribute keys into module-local consts.

Dropping openinference-semantic-conventions leaves ~20 spec-defined keys as inline literals spread across start_attributes, end_attributes, push_projected_mark_attributes, common_attributes, and the new push_*/remove_* helpers. tool.name and tool_call.function.name already appear in two places each, and there is no longer compile-time protection against a typo. otel_genai.rs lines 27-30 already uses the const-block pattern for keys missing from the generated crate; mirroring it here keeps the spec surface auditable in one place.

♻️ Sketch
+const OI_SPAN_KIND: &str = "openinference.span.kind";
+const OI_LLM_MODEL_NAME: &str = "llm.model_name";
+const OI_TOOL_NAME: &str = "tool.name";
+const OI_TOOL_FUNCTION_NAME: &str = "tool_call.function.name";
+const OI_OUTPUT_VALUE: &str = "output.value";
+const OI_OUTPUT_MIME_TYPE: &str = "output.mime_type";
+const OI_METADATA: &str = "metadata";
+
 fn push_projected_mark_attributes(attributes: &mut Vec<KeyValue>, event: &Event) {
     let mark_name = event.name().to_string();
-    attributes.push(KeyValue::new("tool.name", mark_name.clone()));
-    attributes.push(KeyValue::new("tool_call.function.name", mark_name));
+    attributes.push(KeyValue::new(OI_TOOL_NAME, mark_name.clone()));
+    attributes.push(KeyValue::new(OI_TOOL_FUNCTION_NAME, mark_name));

Also applies to: 747-758

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/core/src/observability/openinference.rs` around lines 683 - 711,
Define module-local constants for all OpenInference attribute keys used by
start_attributes, end_attributes, push_projected_mark_attributes,
common_attributes, and the push/remove helpers, following the const-block
pattern in otel_genai.rs. Replace every matching inline key literal, including
the duplicated tool.name and tool_call.function.name keys, with the shared
constants while preserving the existing attribute values and behavior.
crates/core/src/observability/otel_genai.rs (2)

307-312: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

The bare "description" fallback can pick up an unrelated nested value.

object_value (lines 475-482) falls back into nested usage/request/response objects, so an un-namespaced key like description will match, for example, response.description on an error payload and emit it as gen_ai.tool.description. Every other lookup in this module uses a namespaced or clearly scoped key. Drop the generic fallback or restrict it to the top-level object.

🔧 Proposed fix
     if let Some(value) = scalar_string(
         event,
-        &["gen_ai.tool.description", "tool_description", "description"],
+        &["gen_ai.tool.description", "tool_description"],
     ) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    if let Some(value) = scalar_string(
        event,
        &["gen_ai.tool.description", "tool_description"],
    ) {
        attributes.push(KeyValue::new("gen_ai.tool.description", value));
    }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/core/src/observability/otel_genai.rs` around lines 307 - 312, Update
the tool description lookup in the event attribute handling to remove the
generic "description" fallback or constrain it to the top-level event object.
Preserve the existing namespaced and tool_description lookups, and ensure nested
usage/request/response values cannot populate gen_ai.tool.description.

404-417: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Error detection here is case-insensitive but span status derivation is not — align them.

Line 410 uses eq_ignore_ascii_case("ERROR"), while set_span_status_from_event_metadata in crates/core/src/observability/mod.rs (lines 559-578) matches the literal "ERROR" and logs invalid_status_code otherwise. Metadata carrying "error" therefore emits error.type on the GenAI span while the span status stays Unset and a warning is logged. Pick one comparison policy so status and error attribute cannot disagree.

🔧 Proposed fix
-        .is_some_and(|status| status.eq_ignore_ascii_case("ERROR"));
+        .is_some_and(|status| status == "ERROR");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

fn push_error_attributes(attributes: &mut Vec<KeyValue>, event: &Event) {
    let is_error = event
        .metadata()
        .and_then(Value::as_object)
        .and_then(|metadata| metadata.get("otel.status_code"))
        .and_then(Value::as_str)
        .is_some_and(|status| status == "ERROR");
    if !is_error {
        return;
    }
    let error_type = scalar_string(event, &[semconv::ERROR_TYPE, "error_type"])
        .unwrap_or_else(|| "_OTHER".to_string());
    attributes.push(KeyValue::new(semconv::ERROR_TYPE, error_type));
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/core/src/observability/otel_genai.rs` around lines 404 - 417, Align
the status-code comparison in push_error_attributes and
set_span_status_from_event_metadata so both use the same case-sensitivity
policy. Preserve consistent handling of metadata values such as "error": the
span status and error.type attribute must agree, and valid case variants must
not trigger invalid_status_code warnings.
crates/core/src/observability/otel.rs (1)

562-603: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

No handoff with the owned exporter runtime thread is time-bounded. The new owned-runtime design coordinates with a dedicated OS thread at two points — readiness on construction and teardown on drop — and neither has a deadline, so a wedged exporter (unreachable collector, stalled TLS root loading, blocked flush) hangs the caller indefinitely in both directions.

  • crates/core/src/observability/otel.rs#L562-L603: replace the unbounded result_receiver.recv() with recv_timeout derived from config.timeout, and document that new() blocks the calling thread (the tests call it from inside block_on).
  • crates/core/src/observability/otel.rs#L324-L339: bound teardown by calling runtime.shutdown_timeout(...) inside the spawned closure rather than relying on Runtime::drop before thread.join(), so subscriber drop at process teardown cannot block forever.
📍 Affects 1 file
  • crates/core/src/observability/otel.rs#L562-L603 (this comment)
  • crates/core/src/observability/otel.rs#L324-L339
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/core/src/observability/otel.rs` around lines 562 - 603, Bound both
owned exporter runtime handoffs: in crates/core/src/observability/otel.rs lines
562-603, update build_owned_tracer_provider to use result_receiver.recv_timeout
with a duration derived from config.timeout, and document that construction
blocks the calling thread; in lines 324-339, call Runtime::shutdown_timeout
inside the spawned runtime closure with the configured timeout so teardown
cannot wait indefinitely before thread joining.
crates/core/src/observability/plugin_component.rs (2)

1762-1775: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Include the endpoint index in these two errors.

Every other failure in this function is prefixed with endpoints[{index}], but the blank-endpoint and bad-transport messages are not, so with a fan-out configuration the operator cannot tell which entry failed. index is already in scope.

♻️ Proposed change
     if section.endpoint.trim().is_empty() {
         return Err(PluginError::InvalidConfig(
-            "OpenTelemetry endpoint must be a nonblank string".to_string(),
+            format!("OpenTelemetry endpoints[{index}].endpoint must be a nonblank string"),
         ));
     }
     let transport = match section.transport.as_str() {
         "http_binary" => OtlpTransport::HttpBinary,
         "grpc" => OtlpTransport::Grpc,
         other => {
             return Err(PluginError::InvalidConfig(format!(
-                "OpenTelemetry transport must be 'http_binary' or 'grpc', got {other:?}"
+                "OpenTelemetry endpoints[{index}].transport must be 'http_binary' or 'grpc', got {other:?}"
             )));
         }
     };

initialization_fails_for_invalid_enabled_file_exporters asserts on the substring "OpenTelemetry transport", so update that assertion accordingly.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    if section.endpoint.trim().is_empty() {
        return Err(PluginError::InvalidConfig(
            format!("OpenTelemetry endpoints[{index}].endpoint must be a nonblank string"),
        ));
    }
    let transport = match section.transport.as_str() {
        "http_binary" => OtlpTransport::HttpBinary,
        "grpc" => OtlpTransport::Grpc,
        other => {
            return Err(PluginError::InvalidConfig(format!(
                "OpenTelemetry endpoints[{index}].transport must be 'http_binary' or 'grpc', got {other:?}"
            )));
        }
    };
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/core/src/observability/plugin_component.rs` around lines 1762 - 1775,
Update the blank-endpoint and invalid-transport errors in the OpenTelemetry
endpoint validation to prefix messages with the in-scope endpoint index using
the existing `endpoints[{index}]` format. Also update the
`initialization_fails_for_invalid_enabled_file_exporters` assertion to match the
new indexed transport error while preserving its existing validation intent.

2032-2048: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Removed fields get an "unknown field" message, hiding the migration path.

Fields in REMOVED are correctly escalated to Error with code observability.legacy_opentelemetry_field, but the message text is the shared unknown OpenTelemetry endpoint field {field:?}. For a breaking version-3 migration this is the one place that should tell the operator the control was removed, matching the wording used for the legacy openinference section at Line 1895.

♻️ Proposed change
-            let behavior = if REMOVED.contains(&field.as_str()) {
-                UnsupportedBehavior::Error
-            } else {
-                policy.unknown_field
-            };
+            let removed = REMOVED.contains(&field.as_str());
+            let (behavior, code, message) = if removed {
+                (
+                    UnsupportedBehavior::Error,
+                    "observability.legacy_opentelemetry_field",
+                    format!(
+                        "OpenTelemetry endpoint field {field:?} was removed in observability config version 3"
+                    ),
+                )
+            } else {
+                (
+                    policy.unknown_field,
+                    "observability.unknown_field",
+                    format!("unknown OpenTelemetry endpoint field {field:?}"),
+                )
+            };
             push_policy_diag(
                 diagnostics,
                 behavior,
-                if REMOVED.contains(&field.as_str()) {
-                    "observability.legacy_opentelemetry_field"
-                } else {
-                    "observability.unknown_field"
-                },
+                code,
                 Some("opentelemetry".to_string()),
                 Some(format!("endpoints[{index}].{field}")),
-                format!("unknown OpenTelemetry endpoint field {field:?}"),
+                message,
             );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

            let removed = REMOVED.contains(&field.as_str());
            let (behavior, code, message) = if removed {
                (
                    UnsupportedBehavior::Error,
                    "observability.legacy_opentelemetry_field",
                    format!(
                        "OpenTelemetry endpoint field {field:?} was removed in observability config version 3"
                    ),
                )
            } else {
                (
                    policy.unknown_field,
                    "observability.unknown_field",
                    format!("unknown OpenTelemetry endpoint field {field:?}"),
                )
            };
            push_policy_diag(
                diagnostics,
                behavior,
                code,
                Some("opentelemetry".to_string()),
                Some(format!("endpoints[{index}].{field}")),
                message,
            );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/core/src/observability/plugin_component.rs` around lines 2032 - 2048,
Update the diagnostic message passed to push_policy_diag in the REMOVED-field
branch so removed OpenTelemetry fields explicitly state that the control was
removed and provide the version-3 migration guidance, matching the legacy
openinference wording near line 1895; retain the existing unknown-field message
for non-REMOVED fields.
crates/core/tests/integration/middleware_tests.rs (1)

848-862: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

This test lost both its tool-projection and its OpenInference-specific coverage.

from_tracer_provider resolves to MarkProjection::default() (Inherit), so the OTel exporter no longer exercises MarkProjection::Tool in this test despite the name ..._pending_marks_project_through_trace_exporters_only. The new assertions at lines 947-948 and 979-980 are now byte-identical, so nothing distinguishes the OpenInference projection from the full projection here — the previous openinference.span.kind check was the only exporter-specific signal.

The assertions themselves are correct (the mark arrives after the tool span ends, so process_mark takes the orphan branch while parent_context still resolves through completed_span_contexts). The gap is coverage: add back an openinference.span.kind == "CHAIN" assertion on the OpenInference span, and keep a MarkProjection::Tool variant if that path still matters at the integration level.

As per path instructions, tests should "prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests."

Also applies to: 979-980

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/core/tests/integration/middleware_tests.rs` around lines 848 - 862,
Restore explicit tool-projection coverage in the OTel setup around
from_tracer_provider by constructing the subscriber with MarkProjection::Tool
where required, while preserving the existing pending-mark assertions. Add an
OpenInference-specific assertion for openinference.span.kind equal to "CHAIN" on
the OpenInference span, so its exporter-specific projection remains
distinguished from the full projection.

Source: Path instructions

crates/core/tests/unit/observability/openinference_tests.rs (3)

684-700: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

These builder tests assert nothing about what they claim to verify.

config_defaults_and_builder_overrides_are_applied sets seven builder options and then only checks that construction, flush, and shutdown do not error — no override is ever read back. otel.rs line 246 adds a #[cfg(test)] header() accessor for exactly this purpose and it is unused here. grpc_metadata_and_runtime_builder_paths_succeed goes further and discards both results with .ok() (lines 3797-3798), so it can only fail if new() errors.

All three also now spin up a real Tokio runtime, OS thread, and batch OTLP exporter aimed at a port with no listener, so they pay real setup cost for near-zero signal. Assert on the configured values, and use unwrap() where the operation is expected to succeed.

As per path instructions, tests should "cover the behavior promised by the changed API surface, including error paths" and prefer real assertions "over shallow smoke tests."

💚 Sketch for the defaults test
     .with_instrumentation_scope("demo-scope")
     .with_timeout(Duration::from_millis(1250));
+    assert_eq!(config.header("authorization"), Some("Bearer token"));
     let subscriber =
         crate::observability::otel::OpenTelemetrySubscriber::new(config).expect("valid config");

Also applies to: 3763-3800

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/core/tests/unit/observability/openinference_tests.rs` around lines 684
- 700, Strengthen config_defaults_and_builder_overrides_are_applied by reading
back and asserting every configured builder value, including the header via
OpenTelemetryConfig::header(), rather than only checking construction and
lifecycle calls. Update grpc_metadata_and_runtime_builder_paths_succeed
similarly to assert returned results and use unwrap() for expected success
instead of discarding them with ok(). Avoid starting real runtimes and exporters
for these unit tests where possible, while preserving coverage of the promised
configuration and error behavior.

Source: Path instructions


716-734: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The Tokio runtime here is now dead scaffolding.

OpenTelemetrySubscriber::new owns its exporter runtime — that is precisely what grpc_config_owns_its_tokio_runtime (lines 703-714) asserts. Wrapping this call in block_on no longer tests anything and instead exercises the blocking-in-async pattern flagged on build_owned_tracer_provider. Call new() directly.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/core/tests/unit/observability/openinference_tests.rs` around lines 716
- 734, Update invalid_grpc_headers_are_rejected to call
OpenTelemetrySubscriber::new directly instead of creating a Tokio runtime and
wrapping the call in runtime.block_on; preserve the existing invalid-header
error assertion and configuration unchanged.

3210-3214: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use COMPLETED_SPAN_CONTEXT_LIMIT instead of hardcoding 4096.

This file compiles inside the crate, so the constant at crates/core/src/observability/otel.rs line 52 is reachable. Hardcoding means raising the limit turns this into a confusing failure at line 3233 rather than a test that still measures eviction.

♻️ Proposed change
-    let span_count = 4096 + 2;
+    let span_count = crate::observability::otel::COMPLETED_SPAN_CONTEXT_LIMIT + 2;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    let mut processor = crate::observability::otel::OtelEventProcessor::new_openinference(
        provider.clone(),
        "test-scope".to_string(),
    );
    let span_count = crate::observability::otel::COMPLETED_SPAN_CONTEXT_LIMIT + 2;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/core/tests/unit/observability/openinference_tests.rs` around lines
3210 - 3214, Replace the hardcoded 4096 in the span eviction test’s span_count
calculation with the reachable COMPLETED_SPAN_CONTEXT_LIMIT constant, preserving
the existing +2 setup so the test continues to exercise eviction when the
configured limit changes.
crates/core/tests/unit/observability/otel_tests.rs (2)

585-593: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the owned runtime is actually used, not just that construction succeeds.

The test only proves new doesn't panic without an ambient runtime. shutdown() alone may not drive an export path. Adding a force_flush() (which must dispatch onto the owned runtime) makes this test actually cover the contract its name claims.

♻️ Exercise the owned runtime before shutdown
     .expect("gRPC construction should not require an ambient Tokio runtime");
+    subscriber.force_flush().unwrap();
     subscriber.shutdown().unwrap();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

fn grpc_config_owns_its_tokio_runtime() {
    let subscriber = OpenTelemetrySubscriber::new(
        OpenTelemetryConfig::new(OpenTelemetryType::Full, "http://localhost:4317")
            .with_transport(OtlpTransport::Grpc)
            .with_service_name("demo-agent"),
    )
    .expect("gRPC construction should not require an ambient Tokio runtime");
    subscriber.force_flush().unwrap();
    subscriber.shutdown().unwrap();
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/core/tests/unit/observability/otel_tests.rs` around lines 585 - 593,
Update grpc_config_owns_its_tokio_runtime to call subscriber.force_flush()
before shutdown, asserting the result succeeds so the test exercises dispatch
through the subscriber’s owned Tokio runtime rather than only validating
construction.

641-672: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

This test can pass while asserting nothing.

status.success() is also true when the filter matches zero tests — libtest exits 0 on "0 passed; 0 failed". The hardcoded path observability::otel::tests::direct_config_rejects_process_global_otel_headers (Line 660) is a stringly-typed copy of the module layout; any module rename or relocation of this include!d test file silently turns both child runs into no-ops, and the GlobalHeaderEnvironmentUnsupported assertion never executes.

Capture the child's output and require evidence it actually ran the case.

🛡️ Require proof the child executed the assertion
-        let status = std::process::Command::new(std::env::current_exe().unwrap())
+        let output = std::process::Command::new(std::env::current_exe().unwrap())
             .arg("--exact")
             .arg("observability::otel::tests::direct_config_rejects_process_global_otel_headers")
             .env(CHILD_MARKER, variable)
             .env(variable, "authorization=secret")
             .env_remove(if variable == "OTEL_EXPORTER_OTLP_HEADERS" {
                 "OTEL_EXPORTER_OTLP_TRACES_HEADERS"
             } else {
                 "OTEL_EXPORTER_OTLP_HEADERS"
             })
-            .status()
+            .output()
             .unwrap();
-        assert!(status.success());
+        let stdout = String::from_utf8_lossy(&output.stdout);
+        assert!(output.status.success(), "child failed for {variable}: {stdout}");
+        assert!(
+            stdout.contains("1 passed"),
+            "child did not run the filtered test for {variable}: {stdout}"
+        );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

#[test]
fn direct_config_rejects_process_global_otel_headers() {
    const CHILD_MARKER: &str = "NEMO_RELAY_TEST_GLOBAL_OTEL_HEADER_CHILD";
    if let Ok(variable) = std::env::var(CHILD_MARKER) {
        assert!(matches!(
            reject_global_header_environment(),
            Err(OpenTelemetryError::GlobalHeaderEnvironmentUnsupported {
                variable: rejected
            }) if rejected == variable
        ));
        return;
    }

    for variable in [
        "OTEL_EXPORTER_OTLP_HEADERS",
        "OTEL_EXPORTER_OTLP_TRACES_HEADERS",
    ] {
        let output = std::process::Command::new(std::env::current_exe().unwrap())
            .arg("--exact")
            .arg("observability::otel::tests::direct_config_rejects_process_global_otel_headers")
            .env(CHILD_MARKER, variable)
            .env(variable, "authorization=secret")
            .env_remove(if variable == "OTEL_EXPORTER_OTLP_HEADERS" {
                "OTEL_EXPORTER_OTLP_TRACES_HEADERS"
            } else {
                "OTEL_EXPORTER_OTLP_HEADERS"
            })
            .output()
            .unwrap();
        let stdout = String::from_utf8_lossy(&output.stdout);
        assert!(output.status.success(), "child failed for {variable}: {stdout}");
        assert!(
            stdout.contains("1 passed"),
            "child did not run the filtered test for {variable}: {stdout}"
        );
    }
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/core/tests/unit/observability/otel_tests.rs` around lines 641 - 672,
Update direct_config_rejects_process_global_otel_headers so each child process
captures its output and verifies the targeted test actually ran, such as
requiring the libtest summary to report one test passed, before accepting
status.success(). Preserve the existing error assertion and environment-variable
loop, while ensuring a renamed or relocated test cannot silently pass with zero
tests executed.
crates/core/tests/unit/observability/plugin_component_tests.rs (1)

166-201: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Harden the capture server: the content-length unwrap and single-byte read loop make this fan-out suite flake-prone.

If the exporter ever sends a chunked body or the client hangs up before the response is written, one of the unwraps panics in the spawned thread, the channel disconnects, and the three new fan-out tests fail after a 10-second recv_timeout with a misleading message instead of the real cause. crates/ffi/tests/unit/api/registry_tests.rs::start_otlp_http_collector already implements a buffered, tolerant version of exactly this helper — reuse that shape here (buffered reads, explicit assertion message when content-length is absent, send the body before writing the response) rather than a second byte-at-a-time implementation.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/core/tests/unit/observability/plugin_component_tests.rs` around lines
166 - 201, Harden start_otlp_capture_server by matching the buffered, tolerant
approach used by start_otlp_http_collector: replace the single-byte header loop
with buffered reads, handle chunked or missing content-length without an
unconditional unwrap, and provide an explicit assertion message when the header
is absent. Send the captured body before writing the response, while preserving
the existing endpoint and channel behavior.
crates/core/tests/unit/plugin_tests.rs (1)

497-499: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Keep non-observability lists replacement-based.

alpha is not an observability component, so its config.list is replaced by the overlay under merge_plugin_config_value; this test currently expects the observability-only concatenation behavior and will fail. Expect json!([9]) and qualify the comment accordingly.

Proposed fix
-    // nested config objects, concatenates config lists, appends overlay-only kinds, preserves
+    // nested config objects, replaces non-observability config lists, appends overlay-only kinds, preserves
...
-        json!([9, 1, 2, 3]),
-        "config lists concatenate with higher-precedence entries first"
+        json!([9]),
+        "non-observability config lists are replaced by higher-precedence entries"

Also applies to: 555-557

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/core/tests/unit/plugin_tests.rs` around lines 497 - 499, Update the
assertions for the non-observability component alpha in the
merge_plugin_config_value tests to expect its overlay config.list as json!([9])
rather than concatenating lists; adjust the nearby test comment to state that
list concatenation applies only to observability components while
non-observability lists are replaced.
docs/build-plugins/language-binding/code-examples.mdx (1)

183-187: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Flush Subscriber Delivery Before Clearing the Plugin

The examples promise event logging but clear the plugin without flushing queued native subscriber callbacks. Add the documented flush operation before plugin.clear() in both examples so terminal events are not lost.

As per coding guidelines, “Examples and documentation must use each exporter's documented flush/deregister order before shutdown.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/build-plugins/language-binding/code-examples.mdx` around lines 183 -
187, Update both Python and Node.js examples to invoke each exporter’s
documented flush operation before calling plugin.clear(). Preserve the existing
shutdown sequence otherwise, ensuring queued native subscriber callbacks are
delivered before the plugin is cleared.

Source: Coding guidelines

docs/resources/troubleshooting/trace-incident-runbook.mdx (1)

152-156: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Scope the nemo_relay.* attribute check to non-gen_ai endpoints.

This section now admits gen_ai endpoints, but the following bullet (Lines 158-159) still tells readers to expect spans carrying nemo_relay.uuid and nemo_relay.parent_uuid. The gen_ai projection deliberately omits nemo_relay.* attributes, so an operator debugging a gen_ai endpoint would treat correct behavior as a failure.

📝 Proposed clarification
 - The exporter is enabled in the active configuration file or plugin document.
-- The backend receives spans with `nemo_relay.uuid` and
-  `nemo_relay.parent_uuid` attributes.
+- For `full` and `openinference` endpoints, the backend receives spans with
+  `nemo_relay.uuid` and `nemo_relay.parent_uuid` attributes. The `gen_ai`
+  projection omits `nemo_relay.*` attributes by design.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

For typed OpenTelemetry export, confirm these settings:

- Each endpoint has the intended `full`, `gen_ai`, or `openinference` type.
- The OpenTelemetry Protocol (OTLP) endpoint, headers, `header_env` variables,
  credentials, and network egress are available in the target environment.
- The exporter is enabled in the active configuration file or plugin document.
- For `full` and `openinference` endpoints, the backend receives spans with
  `nemo_relay.uuid` and `nemo_relay.parent_uuid` attributes. The `gen_ai`
  projection omits `nemo_relay.*` attributes by design.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/resources/troubleshooting/trace-incident-runbook.mdx` around lines 152 -
156, Update the troubleshooting guidance around the typed OpenTelemetry export
settings and the following attribute-check bullet so `nemo_relay.uuid` and
`nemo_relay.parent_uuid` are expected only for `full` and `openinference`
endpoints, not `gen_ai` endpoints; preserve the existing validation guidance for
non-`gen_ai` projections.
examples/switchyard/hermes-ollama-plugins.toml (1)

57-59: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Remove the Duplicated TOML Table Headers. The first duplicate makes the TOML invalid; the second creates an empty endpoint that fails endpoint validation.

  • examples/switchyard/hermes-ollama-plugins.toml#L57-L59: retain exactly one [components.config] declaration.
  • examples/switchyard/hermes-ollama-plugins.toml#L91-L100: retain exactly one [[components.config.opentelemetry.endpoints]] declaration before its endpoint fields.
📍 Affects 1 file
  • examples/switchyard/hermes-ollama-plugins.toml#L57-L59 (this comment)
  • examples/switchyard/hermes-ollama-plugins.toml#L91-L100
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/switchyard/hermes-ollama-plugins.toml` around lines 57 - 59, Remove
the duplicate TOML table headers in
examples/switchyard/hermes-ollama-plugins.toml: retain exactly one
[components.config] declaration at lines 57-59 and exactly one
[[components.config.opentelemetry.endpoints]] declaration before its endpoint
fields at lines 91-100.
go/nemo_relay/openinference_test.go (1)

22-24: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Shared NewOtelTestServer handler can block and hang server.Close(). Both new tests pass a one-slot channel to the shared helper, whose handler sends unconditionally; a second export (for example a shutdown flush from the deferred subscriber.Close()) has no reader, so the handler blocks and httptest.Server.Close waits on it forever.

  • go/nemo_relay/openinference_test.go#L22-L24: increase the channel buffer, or change the handler in NewOtelTestServer to a select/default non-blocking send.
  • go/nemo_relay/otel_test.go#L205-L246: apply the same buffering change to the requests channel in TestOpenTelemetrySubscriberExportsGenAIAgentProjection.
📍 Affects 2 files
  • go/nemo_relay/openinference_test.go#L22-L24 (this comment)
  • go/nemo_relay/otel_test.go#L205-L246
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@go/nemo_relay/openinference_test.go` around lines 22 - 24, Prevent the shared
NewOtelTestServer handler from blocking during shutdown by making its request
channel send non-blocking or increasing its buffer; update
go/nemo_relay/openinference_test.go lines 22-24 and go/nemo_relay/otel_test.go
lines 205-246 consistently, including the requests channel in
TestOpenTelemetrySubscriberExportsGenAIAgentProjection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking PR introduces a breaking change Feature a new feature lang:go PR changes/introduces Go code lang:js PR changes/introduces Javascript/Typescript code lang:python PR changes/introduces Python code lang:rust PR changes/introduces Rust code size:XXL PR is very large

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant