Skip to content

Commit d77a6a9

Browse files
committed
fix: fail closed when the Traceloop span filter cannot be installed
The filter is installed by wrapping a function of the SDK's, and traceloop-sdk is depended on across >=0.43.1,<1.0.0. If a release stops routing its exporter through get_default_span_processor the filter stops applying and nothing else changes: spans keep flowing, and the only difference is that the service's own telemetry is in them. CI cannot catch that, because CI resolves the lockfile while the version that breaks it is the one a user installs. Record whether the filter actually installed, and disable the integration with an explanatory error when an init that builds the SDK's pipeline did not get one. A broken LLM tracing integration is recoverable; silently shipping the operator's HTTP and flow spans to a third party is not. Also note in the module why this matters in every install rather than only where an APM is configured: instrument_fastapi_app runs unconditionally at startup, so HTTP server spans are always being produced.
1 parent 6a09a37 commit d77a6a9

2 files changed

Lines changed: 99 additions & 25 deletions

File tree

src/backend/base/langflow/services/tracing/traceloop.py

Lines changed: 58 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
from langflow.services.tracing.base import BaseTracer
2424

2525
if TYPE_CHECKING:
26-
from collections.abc import Sequence
26+
from collections.abc import Iterator, Sequence
2727
from uuid import UUID
2828

2929
from langchain_core.callbacks.base import BaseCallbackHandler
@@ -34,10 +34,24 @@
3434
from langflow.services.tracing.schema import Log
3535

3636

37-
# Traceloop.init is not reentrant (TracerWrapper is a singleton built in __new__) and the patch
38-
# below is on a process-wide object, so concurrent flow runs are serialised through this.
37+
# ---------------------------------------------------------------------------------------------
38+
# Keeping the service's own telemetry out of Traceloop's exporter.
39+
#
40+
# The SDK takes no tracer provider. It adopts whichever provider is global and attaches its
41+
# exporter to that, so its exporter sees every span on that provider -- including the service's
42+
# own HTTP and flow spans. ``instrument_fastapi_app`` runs unconditionally at startup, so those
43+
# HTTP spans exist in every install, whether or not an APM is configured. Without a filter they
44+
# are shipped to the vendor along with the LLM traces the operator actually asked for.
45+
# ---------------------------------------------------------------------------------------------
46+
47+
# Traceloop.init is not reentrant (TracerWrapper is a singleton built in __new__) and the hook
48+
# below is on a module attribute, so concurrent flow runs are serialised through this.
3949
_INIT_LOCK = threading.Lock()
4050

51+
# Set once the filter has been observed to install. Only the init that actually builds the
52+
# SDK's pipeline runs the factory; later ones are no-ops and must not be read as a failure.
53+
_boundary_installed = False
54+
4155

4256
class _ApplicationScopeFilter(SpanProcessor):
4357
"""Delegates to a Traceloop processor, minus the spans that belong to the operator's APM.
@@ -46,6 +60,10 @@ class _ApplicationScopeFilter(SpanProcessor):
4660
its own LLM spans and anything else it instruments. The default falls towards the vendor
4761
deliberately: the allowlist is the set we know the APM exports, and dropping only that is
4862
what makes this safe to wrap around a pipeline whose contents we do not control.
63+
64+
Note that ``APPLICATION_INSTRUMENTATION_SCOPES`` is therefore load-bearing in two
65+
directions. Adding a scope to it enriches the APM *and* removes that scope from every
66+
Traceloop trace.
4967
"""
5068

5169
def __init__(self, wrapped: SpanProcessor) -> None:
@@ -76,47 +94,62 @@ def force_flush(self, timeout_millis: int = 30000) -> bool:
7694

7795

7896
@contextmanager
79-
def _application_telemetry_withheld():
80-
"""Keep the service's own telemetry out of whatever exporter Traceloop attaches.
81-
82-
The SDK takes no tracer provider. It adopts whichever provider is global and adds its
83-
exporter to that, so when application observability is enabled it is our provider it
84-
attaches to, and its exporter then sees every span on it -- including the service's own
85-
HTTP and flow spans, which the operator pointed at their APM and not at Traceloop.
97+
def _application_telemetry_withheld() -> Iterator[None]:
98+
"""Wrap the SDK's span processor factory for the duration of ``Traceloop.init``.
8699
87-
When no OTLP endpoint is configured the bootstrap installs nothing and the global provider
88-
is still a proxy, which does not make the problem go away: the SDK then creates a concrete
89-
provider and registers it globally, the proxy resolves onto it, and the service's spans
90-
reach the vendor by the same route. So the filter has to apply in both cases.
91-
92-
It is applied to the SDK's own processor factory rather than to the provider. Patching the
93-
provider's ``add_span_processor`` only covers the case where a provider already exists, and
94-
it is a shared object -- a processor another integration registers while ``init`` is running
95-
would be wrapped too, and would then have this filter's boundary applied backwards.
100+
The factory is the hook because it is the one place every export path goes through. The
101+
provider is not: patching ``add_span_processor`` only covers the case where a provider
102+
already exists, and misses the far more common one where no APM is configured, the global
103+
provider is still a proxy, and the SDK creates and registers its own concrete provider that
104+
the proxy then resolves onto. The provider is also shared, so a processor another
105+
integration registered during ``init`` would be wrapped and have this boundary applied
106+
backwards.
96107
97108
Passing ``processor=`` to ``init`` is the other obvious hook and is worse: it turns off the
98109
SDK's metrics, drops the ``TRACELOOP_HEADERS`` auth the Instana integration depends on,
99-
makes ``disable_batch`` inert and skips prompt sync. Wrapping the factory leaves all of
100-
that alone, because from ``init``'s point of view no processor was supplied.
110+
makes ``disable_batch`` inert and skips prompt sync. Wrapping the factory leaves all of that
111+
alone, because from ``init``'s point of view no processor was supplied.
112+
113+
Fails closed. ``traceloop-sdk`` is depended on across a wide range, and if a release stops
114+
routing through this factory the filter stops applying with no other symptom. A silent leak
115+
is the one failure mode an export boundary must not have, so an init that builds the SDK's
116+
pipeline without installing the filter raises instead of exporting unfiltered.
101117
102-
Serialised because flows run concurrently and the patch is on a module attribute: two
103-
tracers initialising at once would otherwise have the first one's restore run while the
104-
second is still inside init, leaving that run's processor unwrapped.
118+
Serialised because flows run concurrently and the hook is on a module attribute: two tracers
119+
initialising at once would otherwise have the first one's restore run while the second is
120+
still inside init, leaving that run's processor unwrapped.
105121
"""
122+
global _boundary_installed # noqa: PLW0603
106123
from traceloop.sdk.tracing import tracing as traceloop_tracing
107124

108125
with _INIT_LOCK:
109126
original = traceloop_tracing.get_default_span_processor
127+
installed: list[_ApplicationScopeFilter] = []
110128

111129
def get_default_span_processor(*args, **kwargs) -> SpanProcessor:
112-
return _ApplicationScopeFilter(original(*args, **kwargs))
130+
span_processor = _ApplicationScopeFilter(original(*args, **kwargs))
131+
installed.append(span_processor)
132+
return span_processor
113133

114134
traceloop_tracing.get_default_span_processor = get_default_span_processor
115135
try:
116136
yield
117137
finally:
118138
traceloop_tracing.get_default_span_processor = original
119139

140+
if installed:
141+
_boundary_installed = True
142+
elif not _boundary_installed:
143+
msg = (
144+
"Traceloop was initialised without the filter that keeps Langflow's own "
145+
"telemetry out of its exporter, so the integration has been disabled rather "
146+
"than shipping the service's HTTP and flow spans to the vendor. This means the "
147+
"installed traceloop-sdk no longer builds its exporter through "
148+
"get_default_span_processor; pin traceloop-sdk to a version that does."
149+
)
150+
logger.error(msg)
151+
raise RuntimeError(msg)
152+
120153

121154
class TraceloopTracer(BaseTracer):
122155
"""Traceloop tracer for Langflow."""

src/backend/tests/unit/services/tracing/test_traceloop_application_telemetry.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,47 @@ def test_application_spans_do_not_reach_the_vendor_without_an_apm():
133133
assert result["traceloop"] == [], "application telemetry leaked to the LLM vendor"
134134

135135

136+
def test_the_integration_is_disabled_when_the_filter_cannot_be_installed():
137+
"""A future SDK that stops using the factory must break the integration, not the boundary.
138+
139+
``traceloop-sdk`` is depended on across a wide range, and the filter is installed by wrapping
140+
a function of theirs. If a release stops routing through it, nothing else changes: spans keep
141+
flowing and the only difference is that the service's own telemetry is in them. So a run that
142+
builds the SDK's pipeline without installing the filter has to fail loudly instead.
143+
144+
Simulated by making the factory unreachable under the name the wrapper replaces, which is
145+
what any rename or inlining upstream would look like from here.
146+
"""
147+
result = _run("""
148+
os.environ["OTEL_EXPORTER_OTLP_ENDPOINT"] = f"http://127.0.0.1:{apm_port}"
149+
os.environ["OTEL_TRACES_EXPORTER"] = "otlp"
150+
151+
from lfx.observability import bootstrap_application_telemetry, APPLICATION_TRACER_NAME
152+
telemetry = bootstrap_application_telemetry(prometheus_enabled=False)
153+
154+
# Stand in for an upstream release that builds its exporter some other way: the name the
155+
# wrapper replaces is still there, but init no longer calls it.
156+
from traceloop.sdk.tracing import tracing as traceloop_tracing
157+
real = traceloop_tracing.get_default_span_processor
158+
traceloop_tracing.TracerWrapper.__new__ = (
159+
lambda cls, *a, **kw: object.__new__(cls) if not hasattr(cls, "instance") else cls.instance
160+
)
161+
162+
from langflow.services.tracing.traceloop import TraceloopTracer
163+
tracer = TraceloopTracer(
164+
trace_name="probe", trace_type="chain", project_name="probe", trace_id=uuid.uuid4()
165+
)
166+
167+
from opentelemetry import trace
168+
trace.get_tracer(APPLICATION_TRACER_NAME).start_span("flow.execute").end()
169+
telemetry.tracer_provider.force_flush(5000)
170+
report(ready=tracer.ready)
171+
""")
172+
173+
assert result["ready"] is False, "the integration must refuse to run without the filter"
174+
assert result["traceloop"] == [], "application telemetry leaked to the LLM vendor"
175+
176+
136177
def test_vendor_spans_still_reach_the_vendor():
137178
"""Filtering our telemetry out must not cost Traceloop its own spans.
138179

0 commit comments

Comments
 (0)