Skip to content

Commit 86f2f96

Browse files
authored
fix: move otel check to client init time to avoid timing issue (#1958)
**Issue:** for dynamic values like env var, you usually want to check them inside the instance or function, not at import time. because under the hood, python Imports are cached (in sys.modules), so in the case of some customer who's using pytest with otel, their second import of langsmith.client (during pytest unit test run time) weren't able to override initial one (during langsmith_plugin pytest plugin import, happened at very first of pytest set up). This is causing subtle issue like if pytest plugin run first (`has_otel=False`), then later even if env var got reset (`has_otel=True`) the python still doesn't recognize the change. I also found that the env var cache a bit problematic in such case, but if we fix the import, that might be less concerning, so i just reduced max size a little bit to make it less likely to be an issue.
1 parent 6340d14 commit 86f2f96

File tree

7 files changed

+282
-170
lines changed

7 files changed

+282
-170
lines changed

python/langsmith/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121

2222
# Avoid calling into importlib on every call to __version__
2323

24-
__version__ = "0.4.16"
24+
__version__ = "0.4.17"
2525
version = __version__ # for backwards compatibility
2626

2727

python/langsmith/_internal/otel/_otel_client.py

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,21 @@
11
"""Client configuration for OpenTelemetry integration with LangSmith."""
22

33
import os
4+
import warnings
5+
from typing import TYPE_CHECKING
6+
7+
if TYPE_CHECKING:
8+
try:
9+
from opentelemetry.sdk.trace import TracerProvider # type: ignore[import]
10+
except ImportError:
11+
TracerProvider = object # type: ignore[assignment, misc]
412

513
from langsmith import utils as ls_utils
614

7-
HAS_OTEL = False
8-
try:
9-
if ls_utils.is_truish(ls_utils.get_env_var("OTEL_ENABLED")):
15+
16+
def _import_otel_client():
17+
"""Dynamically import OTEL client modules when needed."""
18+
try:
1019
from opentelemetry.exporter.otlp.proto.http.trace_exporter import ( # type: ignore[import]
1120
OTLPSpanExporter,
1221
)
@@ -19,9 +28,18 @@
1928
BatchSpanProcessor,
2029
)
2130

22-
HAS_OTEL = True
23-
except ImportError:
24-
pass
31+
return (
32+
OTLPSpanExporter,
33+
SERVICE_NAME,
34+
Resource,
35+
TracerProvider,
36+
BatchSpanProcessor,
37+
)
38+
except ImportError as e:
39+
warnings.warn(
40+
f"OTEL_ENABLED is set but OpenTelemetry packages are not installed: {e}"
41+
)
42+
return None
2543

2644

2745
def get_otlp_tracer_provider() -> "TracerProvider":
@@ -40,12 +58,20 @@ def get_otlp_tracer_provider() -> "TracerProvider":
4058
Returns:
4159
TracerProvider: The OTLP tracer provider.
4260
"""
43-
# Set LangSmith-specific defaults if not already set in environment
44-
if not HAS_OTEL:
61+
# Import OTEL modules dynamically
62+
otel_imports = _import_otel_client()
63+
if otel_imports is None:
4564
raise ImportError(
4665
"OpenTelemetry packages are required to use this function. "
4766
"Please install with `pip install langsmith[otel]`"
4867
)
68+
(
69+
OTLPSpanExporter,
70+
SERVICE_NAME,
71+
Resource,
72+
TracerProvider,
73+
BatchSpanProcessor,
74+
) = otel_imports
4975

5076
if "OTEL_EXPORTER_OTLP_ENDPOINT" not in os.environ:
5177
ls_endpoint = ls_utils.get_api_url(None)

python/langsmith/_internal/otel/_otel_exporter.py

Lines changed: 74 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,16 @@
66
import logging
77
import uuid
88
import warnings
9-
from typing import Any, Optional
9+
from typing import TYPE_CHECKING, Any, Optional
10+
11+
if TYPE_CHECKING:
12+
try:
13+
from opentelemetry.context.context import Context # type: ignore[import]
14+
from opentelemetry.trace import Span # type: ignore[import]
15+
except ImportError:
16+
Context = Any # type: ignore[assignment, misc]
17+
Span = Any # type: ignore[assignment, misc]
1018

11-
from langsmith import utils as ls_utils
1219
from langsmith._internal import _orjson
1320
from langsmith._internal._operations import (
1421
SerializedRunOperation,
@@ -18,9 +25,10 @@
1825
get_otel_trace_id_from_uuid,
1926
)
2027

21-
HAS_OTEL = False
22-
try:
23-
if ls_utils.is_truish(ls_utils.get_env_var("OTEL_ENABLED")):
28+
29+
def _import_otel_exporter():
30+
"""Dynamically import OTEL exporter modules when needed."""
31+
try:
2432
from opentelemetry import trace # type: ignore[import]
2533
from opentelemetry.context.context import Context # type: ignore[import]
2634
from opentelemetry.trace import ( # type: ignore[import]
@@ -32,9 +40,22 @@
3240
set_span_in_context,
3341
)
3442

35-
HAS_OTEL = True
36-
except ImportError:
37-
pass
43+
return (
44+
trace,
45+
Context,
46+
NonRecordingSpan,
47+
Span,
48+
SpanContext,
49+
TraceFlags,
50+
TraceState,
51+
set_span_in_context,
52+
)
53+
except ImportError as e:
54+
warnings.warn(
55+
f"OTEL_ENABLED is set but OpenTelemetry packages are not installed: {e}"
56+
)
57+
return None
58+
3859

3960
logger = logging.getLogger(__name__)
4061

@@ -97,7 +118,7 @@ def _get_operation_name(run_type: str) -> str:
97118

98119

99120
class OTELExporter:
100-
__slots__ = ["_tracer", "_spans"]
121+
__slots__ = ["_tracer", "_spans", "_otel_available", "_trace"]
101122
"""OpenTelemetry exporter for LangSmith runs."""
102123

103124
def __init__(self, tracer_provider=None):
@@ -107,15 +128,30 @@ def __init__(self, tracer_provider=None):
107128
tracer_provider: Optional tracer provider to use. If not provided,
108129
the global tracer provider will be used.
109130
"""
110-
if not HAS_OTEL:
111-
warnings.warn(
112-
"OTEL_ENABLED is set but OpenTelemetry packages are not installed. "
113-
"Install with `pip install langsmith[otel]`"
131+
otel_imports = _import_otel_exporter()
132+
if otel_imports is None:
133+
self._tracer = None
134+
self._spans = {}
135+
self._otel_available = False
136+
self._trace = None
137+
else:
138+
(
139+
trace,
140+
Context,
141+
NonRecordingSpan,
142+
Span,
143+
SpanContext,
144+
TraceFlags,
145+
TraceState,
146+
set_span_in_context,
147+
) = otel_imports
148+
149+
self._tracer = trace.get_tracer(
150+
"langsmith", tracer_provider=tracer_provider
114151
)
115-
return
116-
117-
self._tracer = trace.get_tracer("langsmith", tracer_provider=tracer_provider)
118-
self._spans = {}
152+
self._spans = {}
153+
self._otel_available = True
154+
self._trace = trace
119155

120156
def export_batch(
121157
self,
@@ -191,6 +227,21 @@ def _create_span_for_run(
191227
trace_id_int = get_otel_trace_id_from_uuid(op.trace_id)
192228
span_id_int = get_otel_span_id_from_uuid(op.id)
193229

230+
# Get OTEL imports for this operation
231+
otel_imports = _import_otel_exporter()
232+
if otel_imports is None:
233+
return None
234+
(
235+
trace,
236+
Context,
237+
NonRecordingSpan,
238+
Span,
239+
SpanContext,
240+
TraceFlags,
241+
TraceState,
242+
set_span_in_context,
243+
) = otel_imports
244+
194245
# Create SpanContext with deterministic IDs
195246
span_context = SpanContext(
196247
trace_id=trace_id_int,
@@ -268,10 +319,10 @@ def _update_span_for_run(self, op: SerializedRunOperation, run_info: dict) -> No
268319
self._set_span_attributes(span, run_info, op)
269320
# Update status based on error
270321
if run_info.get("error"):
271-
span.set_status(trace.StatusCode.ERROR)
322+
span.set_status(self._trace.StatusCode.ERROR)
272323
span.record_exception(Exception(run_info.get("error")))
273324
else:
274-
span.set_status(trace.StatusCode.OK)
325+
span.set_status(self._trace.StatusCode.OK)
275326

276327
# End the span if end_time is present
277328
end_time = run_info.get("end_time")
@@ -316,7 +367,10 @@ def _extract_model_name(self, run_info: dict) -> Optional[str]:
316367
return None
317368

318369
def _set_span_attributes(
319-
self, span: Span, run_info: dict, op: SerializedRunOperation
370+
self,
371+
span: Span,
372+
run_info: dict,
373+
op: SerializedRunOperation,
320374
) -> None:
321375
"""Set attributes on the span.
322376

0 commit comments

Comments
 (0)