Repository navigation
Conversation
The default OTLP exporter now gzips span batches unless configured otherwise. Resolution order: the otel_compression argument, LANGFUSE_OTEL_COMPRESSION, OTEL_EXPORTER_OTLP_TRACES_COMPRESSION, OTEL_EXPORTER_OTLP_COMPRESSION, then gzip. Values are case-insensitive and an invalid value falls through to the next setting, matching the JS SDK. Set any of them to "none" to send uncompressed. Co-authored-by: Hassieb Pakzad <hassiebp@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@claude review |
There was a problem hiding this comment.
Claude Code Review
No review was started: this request came from a bot account. Manual reviews can only be requested by someone with write access to this repository. Ask a maintainer to comment @claude review, or have your automation post the comment from a user account with write access.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
LGTM, a focused and well-tested config/behavior change. Reviewed the new _parse_compression/_resolve_compression precedence chain in span_processor.py (argument, LANGFUSE_OTEL_COMPRESSION, OTEL_EXPORTER_OTLP_TRACES_COMPRESSION, OTEL_EXPORTER_OTLP_COMPRESSION, then gzip default) and confirmed it matches the updated docstrings in client.py/environment_variables.py. Checked the expanded precedence test and the batch-size-limit tests' new _uncompressed_size helper, which correctly decompresses gzip bodies before comparing against the uncompressed-payload size limit.
Extended reasoning...
The change touches only compression-resolution logic for the OTLP span exporter in a Python SDK (langfuse/_client/span_processor.py, client.py, environment_variables.py) plus unit tests; no auth, crypto, or data-exposure surface is involved. It is a self-contained, mechanical rework of an existing fallback chain with a well-reasoned default (gzip, matching the JS SDK and documented server-compatibility rationale), and the accompanying test suite was substantially expanded to cover case-insensitivity, precedence ordering, and invalid-value fallthrough for all four sources plus the new default. I traced the implementation against the docstring claims and the PR description and found them consistent, with no logic gaps.
What does this PR do?
The default OTLP exporter now gzip-compresses span batches unless configured otherwise, matching the JS SDK (langfuse-js#978). Every Langfuse v4 server accepts gzip, and v5 is v4-only, so the old "gzip requires server v3.30" caveat is gone.
Resolution order (first valid value wins):
otel_compressionclient argumentLANGFUSE_OTEL_COMPRESSIONOTEL_EXPORTER_OTLP_TRACES_COMPRESSIONOTEL_EXPORTER_OTLP_COMPRESSIONgzipValues are case-insensitive (
" GZIP "andNonework). As in JS, an invalid value falls through to the next setting instead of disabling compression:LANGFUSE_OTEL_COMPRESSIONlogs a warning;deflate, which the OTEL SDK would otherwise accept, is treated as invalid, because the Langfuse endpoint only decodes gzip. Set any of these tononeto send uncompressed. Customspan_exporters are unaffected.Docstrings for
otel_compression(client.py) andLANGFUSE_OTEL_COMPRESSION(environment_variables.py) are updated.Type of change
Verification
The precedence test checks the
Content-Encodingactually sent to a local OTLP HTTP server, for 14 combinations. The batch-size-limit tests now compare decompressed body sizes, since the limit applies to the uncompressed payload.Checklist
code_review.md..env.templateif needed.The PR appears safe to merge; no actionable issues were found.
What we checked:
_resolve_compressiononly whenspan_exporteris absent. A supplied exporter keeps its own compression settings.Summary
The default span exporter now uses gzip unless a valid setting selects
none.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Create span processor] --> B{Custom exporter provided?} B -->|Yes| C[Keep custom exporter unchanged] B -->|No| D[Check otel_compression] D --> E{Valid gzip or none?} E -->|Yes| J[Configure default exporter] E -->|No| F[Check LANGFUSE_OTEL_COMPRESSION] F --> G{Valid gzip or none?} G -->|Yes| J G -->|No| H[Check traces-specific then generic OTEL setting] H --> I{Valid gzip or none?} I -->|Yes| J I -->|No| K[Use gzip] K --> JReviews (1) · Last reviewed commit: "feat(otel)!: compress span exports with ..." · Reviewed by Greptile