feat: add instrument_asyncio() method for asyncio instrumentation#1745
feat: add instrument_asyncio() method for asyncio instrumentation#1745Br1an67 wants to merge 4 commits intopydantic:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Rather please add a logfire.instrument_asyncio method, similar to #1744, which uses both AsyncioInstrumentor and log_slow_async_callbacks which can be deprecated
There was a problem hiding this comment.
Done — logfire.instrument_asyncio() is now implemented, combining AsyncioInstrumentor with log_slow_callbacks. Also fixed the CI failure: test_logfire_api.py had a catch-all loop that called instrument_asyncio() without cleanup, leaving Handle._run patched and causing test_slow_async_callbacks to fail.
Add logfire.instrument_asyncio() which combines the OpenTelemetry asyncio instrumentation (coroutine/future/to_thread tracing) with Logfire's built-in slow callback detection into a single API call. - New integration module: logfire/_internal/integrations/asyncio_.py - Public API: logfire.instrument_asyncio(slow_duration=0.1) - Optional dependency: pip install 'logfire[asyncio]' - Documentation and integration index updated Closes pydantic#939
3ca999e to
36b4c86
Compare
|
Reworked per your feedback — added Changes:
|
| def instrument_asyncio( | ||
| self, | ||
| slow_duration: float = 0.1, | ||
| **kwargs: Any, | ||
| ) -> AbstractContextManager[None]: | ||
| """Instrument asyncio to trace coroutines, futures, and detect slow event loop callbacks. | ||
|
|
||
| This combines the OpenTelemetry asyncio instrumentation (for tracing coroutines, futures, | ||
| and `to_thread` calls) with Logfire's slow callback detection. | ||
|
|
||
| Args: | ||
| slow_duration: the threshold in seconds for when an event loop callback is considered slow. | ||
| **kwargs: Additional keyword arguments to pass to the OpenTelemetry `instrument` methods, | ||
| for future compatibility. | ||
|
|
||
| Returns: | ||
| A context manager that will revert the slow callback patch when exited. | ||
| This context manager doesn't take into account threads or other concurrency. | ||
| Calling this method will immediately apply the instrumentation | ||
| without waiting for the context manager to be opened, | ||
| i.e. it's not necessary to use this as a context manager. | ||
| """ | ||
| from .integrations.asyncio_ import instrument_asyncio | ||
|
|
||
| self._warn_if_not_initialized_for_instrumentation() | ||
| return instrument_asyncio( | ||
| self, | ||
| slow_duration=slow_duration, | ||
| **{ | ||
| 'tracer_provider': self._config.get_tracer_provider(), | ||
| 'meter_provider': self._config.get_meter_provider(), | ||
| **kwargs, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
🚩 Context manager return only covers slow callback patch, not OTel uninstrumentation
The instrument_asyncio method at logfire/_internal/main.py:895-928 returns the context manager from log_slow_callbacks, which only reverts the asyncio.events.Handle._run monkey-patch when exited. It does NOT call AsyncioInstrumentor().uninstrument() when the context manager exits. This means exiting the context manager will undo the slow callback detection but leave the OTel asyncio instrumentation active. The docstring at line 911 says "A context manager that will revert the slow callback patch when exited" which is accurate, but users might expect full cleanup. This matches how log_slow_async_callbacks works separately, but the combined method could be surprising.
Was this helpful? React with 👍 or 👎 to provide feedback.
instrument_asyncio() patches asyncio.events.Handle._run via log_slow_callbacks, causing test_slow_async_callbacks to fail when the catch-all loop in test_logfire_api calls it without cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| aiohttp = ["opentelemetry-instrumentation-aiohttp-client >= 0.42b0"] | ||
| aiohttp-client = ["opentelemetry-instrumentation-aiohttp-client >= 0.42b0"] | ||
| aiohttp-server = ["opentelemetry-instrumentation-aiohttp-server >= 0.55b0"] | ||
| asyncio = ["opentelemetry-instrumentation-asyncio >= 0.42b0"] |
There was a problem hiding this comment.
🚩 uv.lock not updated with opentelemetry-instrumentation-asyncio dependency
The pyproject.toml adds opentelemetry-instrumentation-asyncio>=0.42b0 to both the optional [asyncio] extra and the [dev] dependency group, but uv.lock does not contain this package. This means uv sync in the development environment won't install it, and tests involving this integration won't be runnable without manual installation. The lock file likely needs to be regenerated.
Was this helpful? React with 👍 or 👎 to provide feedback.
Tests that instrument_asyncio patches Handle._run via log_slow_callbacks and properly reverts the patch when exiting the context manager. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove unused 'import asyncio' from test file (ruff F401) - Add '# pragma: no cover' to ImportError handler in asyncio_.py (consistent with aiohttp_server.py, system_metrics.py) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Add
logfire.instrument_asyncio()method that combines OpenTelemetry asyncio instrumentation with Logfire's slow callback detection into a single API call.Changes
logfire/_internal/integrations/asyncio_.pyinstrument_asyncio(slow_duration=0.1)method onLogfireclassAsyncioInstrumentorfor coroutine/future/to_thread tracinglog_slow_async_callbacks()for event loop blocking detectionpip install 'logfire[asyncio]'logfire-apistubsCloses #939