fix(langfuse): send v4 ingestion header for otel callback - #33907
Conversation
…actored config builder
|
|
Greptile SummaryAdds
Confidence Score: 5/5Safe to merge — the change is additive (an extra OTLP header), touches no auth logic or endpoint resolution, and is fully covered by updated and new tests. Header construction is centralized in two small static helpers, both the env-based and dynamic-credential paths are updated consistently, and all affected test assertions are strengthened rather than relaxed. No pre-existing concerns were introduced or left unaddressed. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/integrations/langfuse/langfuse_otel.py | Adds two constants and two static helpers; wires both the env-config and dynamic-header paths through _build_langfuse_otel_headers to include x-langfuse-ingestion-version: 4. Logic is correct and serialization is consistent with the pre-existing key=value OTLP header format. |
| tests/test_litellm/integrations/test_langfuse_otel.py | Existing assertions on config.headers and exporter._headers are strengthened (not weakened) to require the new x-langfuse-ingestion-version=4 entry; no regression risk. |
| tests/test_service_logger_otel.py | Adds two new async tests (env-config path and dynamic-header path) that verify the v4 header is present; relies on the existing setUp credentials, which are set unconditionally before every test in the class. |
Reviews (5): Last reviewed commit: "chore(langfuse): drop stale development ..." | Re-trigger Greptile
Greptile SummaryThis PR appends
Confidence Score: 4/5The production code change is straightforward and correct; the only concern is a new test that will silently pass or fail depending on whether Langfuse env vars happen to be set in the runner's environment. The core change — adding a constant header via two well-isolated helper methods — is clean and matches the intended fix. The existing test_langfuse_otel.py assertions are updated correctly and cover the key paths. The test_langfuse_otel_env_config_includes_v4_ingestion_header test in test_service_logger_otel.py does not set up LANGFUSE_PUBLIC_KEY/LANGFUSE_SECRET_KEY, so in a clean environment it exercises the wrong code branch and raises KeyError rather than verifying the v4 header; this makes the test unreliable across environments. tests/test_service_logger_otel.py — the new env-config test needs explicit env-var setup to be reliable.
|
| Filename | Overview |
|---|---|
| litellm/integrations/langfuse/langfuse_otel.py | Adds LANGFUSE_INGESTION_VERSION_HEADER constant and two new helpers (_build_langfuse_otel_headers, _format_otel_headers) to attach x-langfuse-ingestion-version: 4 to every Langfuse OTLP export, both env-configured and per-request dynamic. Minor stale comment on import json line. |
| tests/test_litellm/integrations/test_langfuse_otel.py | Updates three existing assertions to include x-langfuse-ingestion-version: 4 alongside the Authorization header; changes correctly reflect the new production behavior. |
| tests/test_service_logger_otel.py | Adds two new async tests for the v4 header, but test_langfuse_otel_env_config_includes_v4_ingestion_header is environment-dependent — it raises KeyError on x-langfuse-ingestion-version when LANGFUSE_PUBLIC_KEY/SECRET_KEY are absent, making it unreliable in clean CI runs. |
Reviews (2): Last reviewed commit: "test(langfuse): assert v4 ingestion head..." | Re-trigger Greptile
|
@greptileai on the 4/5 concern about
def setUp(self):
# Reset callbacks before each test
litellm.service_callback = []
os.environ["LANGFUSE_PUBLIC_KEY"] = "pk-lf-123"
os.environ["LANGFUSE_SECRET_KEY"] = "sk-lf-123"Those four lines are pre-existing on Verified empirically at commit No KeyError, and the test asserts the v4 header on the real Langfuse branch rather than a fallback. Separately, the previous commit fixed the |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6e38363. Configure here.
|
Both findings from the last review are addressed on
Please re-score on |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 181b880. Configure here.
Relevant issues
Adopted from #26105
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
No live-proxy capture against a Langfuse project yet. What is verified so far is the header construction itself, at commit
afca40da82:The three previously-passing assertions in
tests/test_litellm/integrations/test_langfuse_otel.pythat pinned the exporter headers to{"Authorization": ...}alone now assert the v4 header alongside it, which is the behavior change this PR is makingType
Bug Fix
Changes
langfuse_otelwas sending only the OTLPAuthorizationheader. Langfuse routes spans that arrive withoutx-langfuse-ingestion-version: 4through its older transformation path rather than the real-time v4 ingestion path, which surfaces as delayed trace visibilityThis adds
x-langfuse-ingestion-version: 4to every place LiteLLM builds Langfuse OTLP headers:_build_langfuse_otel_config, which now backs both the env-based config and the per-request key/team dynamic configconstruct_dynamic_otel_headers, the dynamic per-request header pathTwo helpers carry it:
_build_langfuse_otel_headersreturns the header mapping,_format_otel_headersserializes that mapping into the comma-separated OTLP header stringBehavior changes
x-langfuse-ingestion-version: 4in addition toAuthorization, on both the env-configured exporter and dynamically-credentialed per-key exporters. Langfuse will ingest these spans through its v4 path instead of the legacy transformation pathCredit
Adopted from #26105 by @hassiebp. Mirrored onto a
litellm_branch so CircleCI and the internal lint workflow runThe original branch conflicted with
litellm_internal_staging, which had since refactored both config paths into a single_build_langfuse_otel_confighelper. The v4 header is reapplied on top of that refactor, so the change now lands in one place instead of two. The original author's commits are preserved in this branch's historyFinal Attestation
Note
Low Risk
Narrow observability export change: only OTLP HTTP headers are extended; auth, endpoints, and span payloads are unchanged.
Overview
Langfuse OTLP exports from
LangfuseOtelLoggernow includex-langfuse-ingestion-version: 4alongside Basic auth, so spans use Langfuse’s v4 ingestion path instead of the legacy transformation route (which could delay trace visibility).Header construction is centralized in
_build_langfuse_otel_headersand_format_otel_headers, used by env-based_build_langfuse_otel_configand per-keyconstruct_dynamic_otel_headers. Tests were updated to expect the new header on static config, dynamic config, and exporter instances;test_service_logger_oteladds coverage for env and dynamic header paths.Reviewed by Cursor Bugbot for commit 181b880. Bugbot is set up for automated code reviews on this repo. Configure here.