-
Notifications
You must be signed in to change notification settings - Fork 355
feat(otel): cap oversized export batches via LANGFUSE_OTEL_MAX_BATCH_SIZE_BYTES #1909
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,10 +12,11 @@ | |
| """ | ||
|
|
||
| import base64 | ||
| import inspect | ||
| import logging | ||
| import os | ||
| import threading | ||
| from typing import Callable, Dict, List, Optional, cast | ||
| from typing import Any, Callable, Dict, List, Optional, cast | ||
|
|
||
| from opentelemetry import context as context_api | ||
| from opentelemetry.context import Context | ||
|
|
@@ -28,6 +29,7 @@ | |
| from langfuse._client.environment_variables import ( | ||
| LANGFUSE_FLUSH_AT, | ||
| LANGFUSE_FLUSH_INTERVAL, | ||
| LANGFUSE_OTEL_MAX_BATCH_SIZE_BYTES, | ||
| LANGFUSE_OTEL_TRACES_EXPORT_PATH, | ||
| ) | ||
| from langfuse._client.propagation import ( | ||
|
|
@@ -47,6 +49,32 @@ | |
| from langfuse.types import MaskOtelSpansFunction | ||
|
|
||
|
|
||
| def _resolve_max_batch_size_bytes() -> Optional[int]: | ||
| """Return the configured batch byte limit, or None to keep the exporter default.""" | ||
| raw_value = os.environ.get(LANGFUSE_OTEL_MAX_BATCH_SIZE_BYTES, "").strip() | ||
| if not raw_value: | ||
| return None | ||
|
|
||
| if raw_value.isascii() and raw_value.isdigit() and int(raw_value) > 0: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
On Python 3.11+, if Knowledge Base Used: Client initialization and resource management Prompt To Fix With AIThis is a comment left during a code review.
Path: langfuse/_client/span_processor.py
Line: 58
Comment:
**Long numbers can block initialization**
On Python 3.11+, if `LANGFUSE_OTEL_MAX_BATCH_SIZE_BYTES` contains more digits than the runtime permits for integer conversion, `int(raw_value)` raises `ValueError` before the invalid-value warning runs. The exception is not caught while the default exporter is created, so a tracing-enabled client fails to initialize instead of retaining the default limit.
**Knowledge Base Used:** [Client initialization and resource management](https://app.greptile.com/personal-org-4986/-/custom-context/knowledge-base/langfuse/langfuse-python/-/docs/client-initialization-and-resources.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+52
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 (optional) Setting LANGFUSE_OTEL_MAX_BATCH_SIZE_BYTES to an all-digit value longer than 4300 characters crashes Langfuse() client construction instead of falling back with a warning like other invalid values. _resolve_max_batch_size_bytes() (span_processor.py:52-66) checks Why this was flagged…Fix: wrap the int() conversion in try/except (or bound the string length first) so any malformed value, including an overlong digit string, falls back to the warning+default path like '0', '-5', 'abc', and '1.5' already do. Trigger: LANGFUSE_OTEL_MAX_BATCH_SIZE_BYTES is set to a string of only ASCII digits but longer than CPython's default int<->str conversion limit (sys.get_int_max_str_digits(), default 4300 since Python 3.11). Entry point: LangfuseSpanProcessor.init (span_processor.py:155) calls _resolve_max_batch_size_bytes() whenever span_exporter is None. There, Verification: nit. Real but trivial edge case. At span_processor.py:58 the validation is |
||
| return int(raw_value) | ||
|
Comment on lines
+58
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
On supported Python 3.11+ versions, converting a decimal string longer than the interpreter's default 4,300-digit limit raises Useful? React with 👍 / 👎. |
||
|
|
||
| langfuse_logger.warning( | ||
| "Invalid %s=%r. Expected a positive integer. Using the default limit.", | ||
| LANGFUSE_OTEL_MAX_BATCH_SIZE_BYTES, | ||
| raw_value, | ||
| ) | ||
| return None | ||
|
|
||
|
|
||
| def _otlp_exporter_supports_max_request_size() -> bool: | ||
| try: | ||
| parameters = inspect.signature(OTLPSpanExporter.__init__).parameters | ||
| except (TypeError, ValueError): | ||
| return False | ||
|
|
||
| return "max_request_size" in parameters | ||
|
|
||
|
|
||
| class LangfuseSpanProcessor(BatchSpanProcessor): | ||
| """OpenTelemetry span processor that exports spans to the Langfuse API. | ||
|
|
||
|
|
@@ -123,10 +151,23 @@ def __init__( | |
| else f"{base_url}/api/public/otel/v1/traces" | ||
| ) | ||
|
|
||
| exporter_kwargs: Dict[str, Any] = {} | ||
| max_request_size = _resolve_max_batch_size_bytes() | ||
| if max_request_size is not None: | ||
| if _otlp_exporter_supports_max_request_size(): | ||
| exporter_kwargs["max_request_size"] = max_request_size | ||
| else: | ||
| langfuse_logger.warning( | ||
| "%s is set but not enforced. It requires " | ||
| "opentelemetry-exporter-otlp-proto-http>=1.45.0.", | ||
| LANGFUSE_OTEL_MAX_BATCH_SIZE_BYTES, | ||
| ) | ||
|
|
||
| span_exporter = OTLPSpanExporter( | ||
| endpoint=endpoint, | ||
| headers=headers, | ||
| timeout=timeout, | ||
| **exporter_kwargs, | ||
| ) | ||
|
|
||
| if media_manager is not None or mask_otel_spans is not None: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
LANGFUSE_OTEL_MAX_BATCH_SIZE_BYTESis explicitly set to an empty or whitespace-only value, stripping it makes the function return as though the variable were absent. The exporter keeps its default limit without an invalid-value warning, making the configuration mistake harder to detect.Prompt To Fix With AI