Skip to content

Commit 842d6ea

Browse files
committed
fix: instrument the database engine, not the global dispatcher
The DB spans this PR added did not deliver what it claimed. Verified against a live backend: one API request produced 13 sqlalchemy connect spans and zero query spans, with no db.statement anywhere. Connection-pool noise dressed up as database visibility. Langflow builds an AsyncEngine, and the instrumentor patches the sync engine underneath it. instrument_dependencies() was called from the app factory with no engine, because the engine does not exist that early, so SQLAlchemy instrumented globally and attached to pool events only. Instrument where the engine is built instead, passing it. Renamed to instrument_database, and the engine argument is required rather than optional: the engine-less call is exactly the broken configuration, and a global instrument() would also win the race and make a later engine-specific call a silent no-op. Same request after the change: 28 query spans carrying db.statement, bound parameters still placeholders, and the flow's input text absent from the trace. The unit tests passed throughout. They assert the scope is allowlisted and the instrumentor is wired, not that a query span reaches an exporter, so this was only visible against a real backend.
1 parent c4377a1 commit 842d6ea

5 files changed

Lines changed: 24 additions & 23 deletions

File tree

.secrets.baseline

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6688,7 +6688,7 @@
66886688
"filename": "src/lfx/src/lfx/cli/serve_app.py",
66896689
"hashed_secret": "b894b81be94cf8fa8d7536475aaec876addf05c8",
66906690
"is_verified": false,
6691-
"line_number": 71,
6691+
"line_number": 70,
66926692
"is_secret": false
66936693
}
66946694
],

src/backend/base/langflow/main.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
from lfx.interface.utils import setup_llm_caching
2323
from lfx.log.logger import configure, logger
2424
from lfx.observability import (
25-
instrument_dependencies,
2625
instrument_fastapi_app,
2726
start_event_loop_lag_monitor,
2827
stop_event_loop_lag_monitor,
@@ -993,9 +992,6 @@ async def exception_handler(_request: Request, exc: Exception):
993992
# lazy-include route patch + instrument_app). The helper lives in lfx so lfx serve
994993
# instruments its own app the same way.
995994
instrument_fastapi_app(app)
996-
# The rest of the trace tree: DB queries and outbound HTTP made during a run. The engine is
997-
# not built yet at app-factory time, so SQLAlchemy instruments its own dispatcher globally.
998-
instrument_dependencies()
999995

1000996
add_pagination(app)
1001997

src/backend/base/langflow/services/database/service.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from alembic import command, util
1717
from alembic.config import Config
1818
from lfx.log.logger import logger
19+
from lfx.observability import instrument_database
1920
from lfx.services.deps import session_scope
2021
from sqlalchemy import event, inspect
2122
from sqlalchemy.dialects import sqlite as dialect_sqlite
@@ -280,6 +281,12 @@ def __init__(self, settings_service: SettingsService):
280281
self.engine = self._create_engine_with_retry()
281282
else:
282283
self.engine = self._create_engine()
284+
# Here rather than in the app factory: this is an AsyncEngine, and the instrumentor
285+
# patches the sync engine underneath it. Instrumenting globally instead (no engine
286+
# argument) attaches to pool events only, which yields a connect span per checkout and
287+
# not a single query span. Verified against a live backend: 13 connect spans and zero
288+
# db.statement for one API request.
289+
instrument_database(self.engine)
283290

284291
# Create async session maker for efficient session creation
285292
# This is the recommended SQLAlchemy 2.0+ pattern

src/lfx/src/lfx/cli/serve_app.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@
4747
from lfx.log.logger import logger
4848
from lfx.observability import (
4949
bootstrap_application_telemetry,
50-
instrument_dependencies,
5150
instrument_fastapi_app,
5251
start_event_loop_lag_monitor,
5352
stop_event_loop_lag_monitor,
@@ -708,7 +707,6 @@ async def _lifespan(_app: FastAPI):
708707
)
709708

710709
instrument_fastapi_app(app)
711-
instrument_dependencies()
712710

713711
app.state.registry = registry
714712
# Snapshot the API key once so per-request auth (verify_api_key, run on a threadpool

src/lfx/src/lfx/observability.py

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -627,37 +627,37 @@ def bootstrap_application_telemetry(*, prometheus_enabled: bool = False) -> Appl
627627
)
628628

629629

630-
def instrument_dependencies(*, engine: object | None = None) -> None:
631-
"""Instrument the database, so a slow request can be attributed to the queries it made.
630+
def instrument_database(engine: object) -> None:
631+
"""Instrument a SQLAlchemy engine, so a slow request can be attributed to its queries.
632632
633-
Deliberately does NOT instrument httpx or requests. Those are the transports the LLM
634-
vendor SDKs ride on, and instrumenting them globally would put one span per outbound
635-
provider call into the operator's APM. That boundary is held elsewhere too (the export
636-
filter does not allowlist those scopes), and outbound provider health is delivered as
637-
leak-safe metrics rather than spans.
633+
Takes the engine rather than instrumenting globally. Langflow's engine is an AsyncEngine and
634+
the instrumentor patches the sync engine underneath it; a global ``instrument()`` with no
635+
engine attaches to pool events only, which produces a connect span per checkout and no query
636+
spans at all. Verified against a live backend: 13 connect spans and zero db.statement for one
637+
API request. That is worse than nothing, because it looks like DB visibility.
638638
639-
The instrumentor is given an explicit ``tracer_provider``. That is not decoration: a bare
640-
``instrument()`` binds to whatever provider is global, which is how vendor SDKs end up
641-
exporting through ours.
639+
Deliberately does not instrument httpx or requests. Those are the transports the LLM vendor
640+
SDKs ride on, and instrumenting them globally would put one span per outbound provider call
641+
into the operator's APM.
642642
643643
Optional and failure-tolerant: a missing package or a double-instrument call must not take
644644
the app down, because none of this is worth a failed boot.
645645
"""
646646
_instrument_sqlalchemy(engine)
647647

648648

649-
def _instrument_sqlalchemy(engine: object | None) -> None:
649+
def _instrument_sqlalchemy(engine: object) -> None:
650650
try:
651651
from opentelemetry import trace
652652
from opentelemetry.instrumentation.sqlalchemy import SQLAlchemyInstrumentor
653653
except ImportError:
654654
return
655655
try:
656-
kwargs = {"tracer_provider": trace.get_tracer_provider()}
657-
if engine is not None:
658-
# An async engine exposes the sync one the instrumentor actually patches.
659-
kwargs["engine"] = getattr(engine, "sync_engine", engine)
660-
SQLAlchemyInstrumentor().instrument(**kwargs)
656+
SQLAlchemyInstrumentor().instrument(
657+
# An AsyncEngine exposes the sync one the instrumentor actually patches.
658+
engine=getattr(engine, "sync_engine", engine),
659+
tracer_provider=trace.get_tracer_provider(),
660+
)
661661
except Exception: # noqa: BLE001 - see above
662662
logger.debug("sqlalchemy instrumentation unavailable; DB spans will be missing", exc_info=True)
663663

0 commit comments

Comments
 (0)