Skip to content

fix(mosaico): pin pydantic-settings so Langfuse tracing actually loads#2560

Merged
ofir-frd merged 3 commits into
mainfrom
fix/mosaico-langfuse-otel-deps
Jul 26, 2026
Merged

fix(mosaico): pin pydantic-settings so Langfuse tracing actually loads#2560
ofir-frd merged 3 commits into
mainfrom
fix/mosaico-langfuse-otel-deps

Conversation

@ofir-frd

@ofir-frd ofir-frd commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

Langfuse observability is dead in the shipped mosaico_agent image. The MOSAICO agent card advertises the mosaico-observability extension with required: true, and not a single LLM call is traced.

# pragent/pr-agent:0.40.0-mosaico_agent
_init_custom_logger_compatible_class('langfuse_otel', ...) -> None
_init_custom_logger_compatible_class('langfuse', ...)      -> None

Why it was silent

litellm 1.93.0 imports pydantic-settings unconditionally from litellm/integrations/otel/__init__.py, but declares it only under its optional proxy extra. requirements.txt never pinned it, so it is absent from the image — and litellm's callback factory swallows the resulting ImportError and returns None. The failure is silent by construction, which is why a fully green test suite sat alongside a dead deliverable.

pr_agent/mosaico/env_bridge.py registers langfuse_otel precisely to route around the legacy langfuse callback raising a sdk_integration TypeError under langfuse 3.x. With pydantic-settings missing, langfuse_otel could not load either — so the workaround was defeated and both paths were dead.

After the pin, langfuse_otel returns a LangfuseOtelLogger. The legacy callback still returns None; that is the unchanged 3.x incompatibility env_bridge exists to avoid, and is expected.

Traces confirmed flowing, not just initialising

Against a loopback receiver, the rebuilt image emitted:

POST /api/public/otel/v1/traces
content-type: application/x-protobuf   bytes: 2335
service.name=litellm, deployment.environment=production, span: litellm_request

The Proxy Server is not installed warning that remains is benign — it comes from _init_otel_logger_on_litellm_proxy(), which runs after _init_tracing() and only registers the logger into the proxy server's own callback list.

Not verified: acceptance by a live Langfuse instance. The receiver was a local stub.

The regression test

It asserts the environment rather than pr-agent logic, so it lives in its own file. It is meaningful in CI because build-and-test.yaml runs the suite inside the Docker test target, whose dependencies come from requirements.txt via pyproject — the same base layer the shipped mosaico_agent image is built from.

Verified both directions:

  • red — all 3 fail inside pre-fix 0.40.0-mosaico_agent with ModuleNotFoundError: No module named 'pydantic_settings'
  • green — 3 pass locally and inside a freshly built test image

Suite goes 1485 → 1488 passed.

Langfuse observability is dead in the shipped mosaico_agent image. The MOSAICO
agent card advertises the mosaico-observability extension with required:true,
and not a single LLM call is traced.

litellm 1.93.0 imports pydantic-settings unconditionally from
litellm/integrations/otel/__init__.py, but declares it only under its optional
'proxy' extra. requirements.txt never pinned it, so it is absent from the image.
litellm's callback factory swallows the resulting ImportError and returns None,
which is why nothing surfaced: the failure is silent by construction.

pr_agent/mosaico/env_bridge.py registers 'langfuse_otel' precisely to route
around the legacy 'langfuse' callback raising a sdk_integration TypeError under
langfuse 3.x. With pydantic-settings missing, langfuse_otel could not load
either, so the workaround was defeated and both paths were dead.

In the shipped 0.40.0-mosaico_agent image:
    _init_custom_logger_compatible_class('langfuse_otel', ...) -> None
    _init_custom_logger_compatible_class('langfuse', ...)      -> None
After the pin, langfuse_otel returns a LangfuseOtelLogger. The legacy callback
still returns None, which is the unchanged 3.x incompatibility env_bridge exists
to avoid, and is expected.

Traces were confirmed to flow, not merely to initialise: against a loopback
receiver the rebuilt image emitted POST /api/public/otel/v1/traces carrying a
2335-byte protobuf with service.name=litellm and a litellm_request span. The
"Proxy Server is not installed" warning that remains is benign - it comes from
_init_otel_logger_on_litellm_proxy(), which runs after _init_tracing() and only
registers the logger into the proxy server's own callback list.

The regression test asserts the environment rather than pr-agent logic, so it
lives in its own file. It is meaningful in CI because build-and-test.yaml runs
the suite inside the Docker test target, whose dependencies come from
requirements.txt via pyproject - the same base layer the mosaico_agent image is
built from. Verified red against the pre-fix image and green after.
@github-actions github-actions Bot added the bug label Jul 26, 2026
@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Fix MOSAICO Langfuse OTEL tracing by pinning pydantic-settings

🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Pin pydantic-settings to prevent silent litellm OTEL import failures in mosaico_agent.
• Add regression tests ensuring langfuse_otel callback constructs and tracing remains enabled.
• Document why the dependency is required despite being an undeclared litellm runtime import.
Diagram

graph TD
R["requirements.txt"] --> D["Docker image build"] --> L["litellm otel integration"] --> F["callback factory"] --> O["Langfuse OTEL logger"] --> X{{"OTLP receiver"}}
T["deps regression tests"] --> R
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Install litellm with the 'proxy' extra (litellm[proxy])
  • ➕ Aligns with litellm's declared extras model rather than manually pinning an undeclared dependency
  • ➕ May pull in other proxy/OTEL-adjacent dependencies litellm expects
  • ➖ Potentially brings additional transitive dependencies not desired in the mosaico_agent image
  • ➖ Harder to reason about/keep minimal than pinning the single missing runtime import
2. Pin/patch litellm to a version that correctly declares pydantic-settings
  • ➕ Fixes the issue at the source (dependency metadata correctness)
  • ➕ May remove the need for a local comment/guard test
  • ➖ Higher operational risk: upgrading/downgrading litellm can change runtime behavior beyond tracing
  • ➖ Depends on availability/timing of an upstream fix
3. Add a startup self-check for required callbacks (fail fast)
  • ➕ Turns silent observability failures into explicit startup errors
  • ➕ Catches future missing deps even if the test suite is bypassed
  • ➖ May be too strict for environments where tracing is optional/disabled
  • ➖ Requires additional runtime wiring and policy decisions

Recommendation: Proceed with the current approach: explicitly pinning pydantic-settings is the smallest, most controlled fix that restores langfuse_otel in the shipped image. The added tests are an appropriate guardrail because litellm’s failure mode is silent; alternatives (litellm[proxy] or changing litellm versions) have wider dependency/runtime blast radius for a MOSAICO-specific packaging issue.

Files changed (2) +81 / -0

Tests (1) +77 / -0
test_mosaico_observability_deps.pyAdd regression tests for langfuse_otel dependency closure and logger construction +77/-0

Add regression tests for langfuse_otel dependency closure and logger construction

• Introduces tests that assert pydantic_settings is installed, litellm.integrations.otel is importable, and the 'langfuse_otel' callback constructs a LangfuseOtelLogger. Includes a fixture to restore litellm’s in-memory logger registry to avoid cross-test pollution.

tests/unittest/test_mosaico_observability_deps.py

Other (1) +4 / -0
requirements.txtPin pydantic-settings to unblock litellm OTEL (langfuse_otel) callback +4/-0

Pin pydantic-settings to unblock litellm OTEL (langfuse_otel) callback

• Adds an explicit pydantic-settings==2.14.2 pin with explanatory comments. This ensures litellm.integrations.otel can import successfully in the mosaico_agent image, preventing silent callback initialization failure.

requirements.txt

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Informational

1. Brittle LiteLLM internal test ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The new regression test reaches into LiteLLM private internals (_in_memory_loggers,
_init_custom_logger_compatible_class) and asserts an implementation class name string, so it can
fail on LiteLLM internal refactors/upgrades unrelated to MOSAICO tracing correctness. This increases
ongoing maintenance cost and can produce CI failures even when the externally relevant behavior
(callback registration/tracing) remains functional.
Code

tests/unittest/test_mosaico_observability_deps.py[R42-77]

+@pytest.fixture
+def restore_in_memory_loggers():
+    """Constructing the callback appends it to litellm's module-global logger registry."""
+    from litellm.litellm_core_utils import litellm_logging
+    snapshot = list(litellm_logging._in_memory_loggers)
+    yield
+    litellm_logging._in_memory_loggers[:] = snapshot
+
+
+class TestLangfuseOtelCallbackDeps:
+    def test_pydantic_settings_is_installed(self):
+        err = _import_error(UNDECLARED_LITELLM_DEP)
+        assert err is None, f"{_REMEDY} Import failed with: {err!r}"
+
+    def test_litellm_otel_integration_imports(self):
+        """The unconditional import site itself -- fails before the callback factory is reached."""
+        err = _import_error("litellm.integrations.otel")
+        assert err is None, f"litellm.integrations.otel is not importable ({err!r}). {_REMEDY}"
+
+    def test_langfuse_otel_callback_constructs(self, restore_in_memory_loggers):
+        """The behaviour MOSAICO actually depends on: env_bridge registers this callback name,
+        and litellm must be able to build a logger for it or every trace is dropped."""
+        from litellm.litellm_core_utils.litellm_logging import \
+            _init_custom_logger_compatible_class
+
+        logger = _init_custom_logger_compatible_class(
+            CALLBACK_NAME, internal_usage_cache=None, llm_router=None)
+
+        assert logger is not None, (
+            f"litellm returned no logger for the '{CALLBACK_NAME}' callback, so MOSAICO's Langfuse "
+            f"tracing is dead. The factory swallows the underlying error; the likely cause is "
+            f"{_import_error('litellm.integrations.otel')!r}. {_REMEDY}"
+        )
+        assert type(logger).__name__ == "LangfuseOtelLogger", (
+            f"Expected a LangfuseOtelLogger for the '{CALLBACK_NAME}' callback, got {type(logger).__name__}."
+        )
Evidence
The added test directly imports and mutates underscore-prefixed LiteLLM internals and asserts a
concrete implementation class name, which are not stable public interfaces and therefore are a clear
source of brittleness/maintenance churn.

tests/unittest/test_mosaico_observability_deps.py[42-49]
tests/unittest/test_mosaico_observability_deps.py[61-77]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test `test_mosaico_observability_deps.py` depends on LiteLLM private/internal APIs (`_in_memory_loggers`, `_init_custom_logger_compatible_class`) and asserts `type(logger).__name__ == "LangfuseOtelLogger"`. This makes the test brittle: any LiteLLM internal refactor (even with unchanged callback behavior) can break the suite.
### Issue Context
The goal is to ensure the environment includes the missing transitive dependency so the `langfuse_otel` integration path is importable/constructible.
### Fix Focus Areas
- tests/unittest/test_mosaico_observability_deps.py[42-77]
### Suggested remediation direction
- Prefer asserting through a more stable/public surface where possible:
- Keep the environment-level import checks (`pydantic_settings` and `litellm.integrations.otel` importability).
- Replace the private factory call and class-name assertion with a behavior/interface assertion (e.g., that constructing/obtaining the callback does not raise and returns an object with expected callable methods), or route via the same public path MOSAICO uses (env bridge + handler wiring) and assert that callbacks are enabled/registered without requiring private registry access.
- If you must use internals, reduce coupling:
- Avoid asserting on the concrete class name string; assert on properties/behavior instead.
- Avoid mutating `_in_memory_loggers` directly; encapsulate cleanup with safer guards (e.g., check attribute existence before snapshot/restore) to prevent hard failures if LiteLLM changes internals.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

  • Author self-review: I have reviewed the code review findings, and addressed the relevant ones.

Qodo Logo

Reaching into litellm privates is unavoidable here: there is no public API that
reports a callback litellm declined to build, and the swallowed ImportError is
exactly what this test exists to surface. But two details were brittle without
adding signal, so an upstream refactor could fail CI over nothing.

The logger-registry fixture now tolerates the attribute being absent and skips
its cleanup instead of erroring. The type assertion matches on "langfuse" rather
than the exact class name, so a rename survives; `is not None` above it remains
the assertion that catches the real defect.

Still verified red against the pre-fix image and green after.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 8a09ca0

The module docstring justified required=True by pointing at docstring-agent,
which no longer sets it. Following that pointer now argues for the opposite of
what the code does, and every live peer reinforces the wrong conclusion:
docstring-agent advertises required=False, ip-solution-agent and mini-swe-agent
omit the flag, and even the reference agent's own card omits it.

The demonstrator's docs/agent-requirements.md states "Required: true" for this
extension, so pr-agent is correct and the peers are out of spec. Cite that
instead, and say plainly not to relax the flag to match them - the comment as
written invited exactly that change.

No behaviour change.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit e095b97

@ofir-frd

Copy link
Copy Markdown
Collaborator Author

Verification reproduced, Qodo finding evaluated

Independently reproduced both directions on top of e095b977.

Red (pre-fix image). Mounting the test into pragent/pr-agent:0.40.0-mosaico_agent still fails all 3, and litellm logs the swallow explicitly:

ERROR LiteLLM:litellm_logging.py:4115 [Non-Blocking Error] Error initializing custom logger: No module named 'pydantic_settings'

Green. 3 passed. pip show pydantic-settings reports an empty Required-by:, so the explicit pin is the only thing keeping it in the environment; nothing pulls it in transitively.

Qodo finding 1, "Brittle LiteLLM internal test": partially accepted, partially rejected

Accepted, and already applied in 8a09ca0:

  • the fixture tolerates _in_memory_loggers being absent rather than erroring
  • the type assertion matches the substring langfuse instead of the exact class name, so an upstream rename cannot fail CI on its own

Rejected: replacing the private construction call _init_custom_logger_compatible_class.

Qodo suggests routing through "a more stable/public surface". litellm does expose an apparently public get_custom_logger_compatible_class(...), so I tested it rather than assuming it was unsuitable. It is not a viable substitute, for two reasons:

  1. It is a lookup, not a factory. It scans the same private _in_memory_loggers registry, so it never exercises construction, which is exactly where the failure occurs.
  2. It has no branch for langfuse_otel at all, so it returns None even in a fully working environment:
lookup-only, before construction: None
after explicit construction:      LangfuseOtelLogger
lookup-only, after construction:  None

A test built on it would report the same result in the fixed environment as in the broken one, so it cannot distinguish them. The private factory is the only surface that returns a logger when tracing works and None when it does not, and that None is the entire defect this test exists to catch.

@ofir-frd
ofir-frd merged commit 93b0dd0 into main Jul 26, 2026
6 checks passed
@ofir-frd
ofir-frd deleted the fix/mosaico-langfuse-otel-deps branch July 26, 2026 14:05
ofir-frd added a commit that referenced this pull request Jul 26, 2026
The bundle pinned 0.40.0, an image built before every MOSAICO fix merged
since: pydantic-settings (#2560), the forwarded-conversation router fix
(#2561), and failure fidelity (#2563). Version tags are immutable, so the
pin has to move for the consortium to get them.

Verified against the published image before opening:
digest sha256:6cf24cd58c0c78bfcaa458e576eb9fb4a1fecb79cadfbfe92d9307486b89015f,
dispatch.py carries _split_turns and propagate_tool_errors, pydantic_settings
imports at 2.14.2, the langfuse_otel callback constructs as LangfuseOtelLogger
where 0.40.0 returned None, and get_version() reports 0.41.0.

The README upgrade example moves to 0.41.0 -> 0.42.0; left alone it would
tell a reader to upgrade to the version they are already running.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant