Skip to content

_setup_cloud_tracer still overrides TracerProviders due to checking the wrong base class#4584

Merged
longcw merged 5 commits intolivekit:mainfrom
hudson-worden:main
Jan 23, 2026
Merged

_setup_cloud_tracer still overrides TracerProviders due to checking the wrong base class#4584
longcw merged 5 commits intolivekit:mainfrom
hudson-worden:main

Conversation

@hudson-worden
Copy link
Contributor

@hudson-worden hudson-worden commented Jan 22, 2026

_setup_cloud_tracer still overrides TracerProviders due to checking the wrong base class.
The previous TracerProvider inherits from the abstract base class TracerProvider which is intended to be the base class of all TracerProviders. Not all TracerProviders need to inherit from the sdk variant.

This specifically happens for the dd-trace TracerProvider which instead inherits from the abstract base class.

What happens is if you use livekit cloud and datadog open telemetry traces, the tracerprovider is being reset by the _setup_cloud_tracer function and you don't get those traces. This is similar to the previous bug here

#4060

Summary by CodeRabbit

  • Refactor
    • Improved telemetry plumbing for safer tracer provider handling and clearer API vs SDK separation.
    • More robust conditional attachment of span processors to avoid incompatible setups.
    • Enhanced shutdown and safety checks for telemetry to reduce errors during provider transitions.
    • Cleaner logging and provider setup behavior to improve reliability and observability.

✏️ Tip: You can customize this high-level summary in your review settings.

…rovider instead of the trace api TraceProvider which all TraceProviders should inherit from
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 22, 2026

📝 Walkthrough

Walkthrough

The tracer provider typing was refactored to use OpenTelemetry's public API types (trace_api.TracerProvider) in public signatures while performing SDK-specific instantiation and processor wiring only when the provider is an instance of trace_sdk.TracerProvider.

Changes

Cohort / File(s) Summary
Tracer Provider Type Differentiation
livekit-agents/livekit/agents/telemetry/traces.py
Added trace as trace_api and trace_sdk imports. Updated public signatures and _DynamicTracer annotations to use trace_api.TracerProvider. Conditional logic now creates/merges an SDK TracerProvider and attaches _MetadataSpanProcessor/BatchSpanProcessor only when provider is trace_sdk.TracerProvider. Updated shutdown/type checks accordingly.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant traces.set_tracer_provider
    participant trace_sdk as trace_sdk (SDK)
    participant tracer_module as tracer (opentelemetry.trace)
    participant processors as SpanProcessors

    Caller->>traces.set_tracer_provider: call with tracer_provider (trace_api.TracerProvider), metadata?
    traces.set_tracer_provider->>trace_sdk: is tracer_provider instance of trace_sdk.TracerProvider?
    alt is SDK provider
        traces.set_tracer_provider->>tracer_module: tracer.set_provider(tracer_provider)
        traces.set_tracer_provider->>processors: attach _MetadataSpanProcessor / BatchSpanProcessor
    else not SDK provider (Proxy/NoOp/API)
        traces.set_tracer_provider->>trace_sdk: create trace_sdk.TracerProvider() as new_provider
        trace_sdk-->>traces.set_tracer_provider: new_provider
        traces.set_tracer_provider->>tracer_module: tracer.set_provider(new_provider)
        traces.set_tracer_provider->>processors: attach processors to new_provider
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰
I hopped through types both API and SDK,
Tucked processors where they ought to be,
Providers now pick the proper hat,
Spans whisper metadata sweetly —
A little hop for telemetry delight!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change in the pull request: fixing the wrong base class check in _setup_cloud_tracer that was causing it to incorrectly override third-party TracerProviders.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

🧹 Recent nitpick comments
livekit-agents/livekit/agents/telemetry/traces.py (2)

101-113: Consider logging when metadata is skipped for non‑SDK providers.

Right now metadata is silently ignored unless the provider is an SDK TracerProvider, which can surprise callers. A debug log would make this explicit.

♻️ Suggested tweak
-    if metadata and isinstance(tracer_provider, trace_sdk.TracerProvider):
-        tracer_provider.add_span_processor(_MetadataSpanProcessor(metadata))
+    if metadata:
+        if isinstance(tracer_provider, trace_sdk.TracerProvider):
+            tracer_provider.add_span_processor(_MetadataSpanProcessor(metadata))
+        else:
+            logger.debug(
+                "metadata provided but tracer_provider doesn't support span processors; skipping"
+            )

166-189: Confirm intended behavior for non‑SDK tracer providers.

When a third‑party provider is present, LiveKit’s span processors/exporter are skipped. Please verify this is intended (or document/log it) so users aren’t surprised.

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3f79463 and 298a993.

📒 Files selected for processing (1)
  • livekit-agents/livekit/agents/telemetry/traces.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Format code with ruff
Run ruff linter and auto-fix issues
Run mypy type checker in strict mode
Maintain line length of 100 characters maximum
Ensure Python 3.9+ compatibility
Use Google-style docstrings

Files:

  • livekit-agents/livekit/agents/telemetry/traces.py
🧠 Learnings (1)
📚 Learning: 2026-01-22T03:28:16.289Z
Learnt from: longcw
Repo: livekit/agents PR: 4563
File: livekit-agents/livekit/agents/beta/tools/end_call.py:65-65
Timestamp: 2026-01-22T03:28:16.289Z
Learning: In code paths that check capabilities or behavior of the LLM processing the current interaction, prefer using the activity's LLM obtained via ctx.session.current_agent._get_activity_or_raise().llm instead of ctx.session.llm. The session-level LLM may be a fallback and not reflect the actual agent handling the interaction. Use the activity LLM to determine capabilities and to make capability checks or feature toggles relevant to the current processing agent.

Applied to files:

  • livekit-agents/livekit/agents/telemetry/traces.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: unit-tests
  • GitHub Check: type-check (3.9)
  • GitHub Check: type-check (3.13)
🔇 Additional comments (3)
livekit-agents/livekit/agents/telemetry/traces.py (3)

14-20: Good separation of API vs SDK trace imports.

Keeps public typing distinct from SDK-only checks for third‑party providers. Please confirm compatibility with the opentelemetry‑api/sdk versions pinned in the repo.

Also applies to: 29-29


46-57: Dynamic tracer typing now aligns with the API base class.

This keeps the dynamic tracer compatible with non‑SDK providers while retaining explicit provider injection.


439-443: Guarding shutdown to SDK providers is the right call.

Prevents calling SDK‑only lifecycle methods on non‑SDK providers.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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

@hudson-worden hudson-worden marked this pull request as ready for review January 22, 2026 03:50
@hudson-worden
Copy link
Contributor Author

image showing that datadog is working again

@hudson-worden hudson-worden requested a review from longcw January 22, 2026 17:29
@longcw longcw merged commit 47ce504 into livekit:main Jan 23, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants