Skip to content

fix(asyncio integration): Filter SDK internal tasks from span creation #4700

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

Merged

Conversation

srothh
Copy link
Member

@srothh srothh commented Aug 13, 2025

Filter SDK internal tasks from span creation. Implements a new contextmanager for tasks spawned internally by the SDK, which filters it in the asyncio integration.

GH-4699

srothh added 30 commits July 21, 2025 11:44
…ted a sync transport HTTP subclass

Moved shared sync/async logic into a new superclass (HttpTransportCore), and moved sync transport specific code into a new subclass(BaseSyncHttpTransport), from which the current transport implementations inherit

Fixes GH-4568
Removed an unnecessary TODO message and reverted a class name change for BaseHTTPTransport.

GH-4568
Adds test coverage for the error handling path when HTTP requests return
error status codes.

GH-4568
Restore comments accidentally removed during a previous commit.
Refactored class names such that BaseHttpTransport now has the same functionality as before the hierarchy refactor

GH-4568
Add a new flush_async method in the Transport ABC. This is needed for the async transport, as calling it from the client
while preserving execution order in close will require flush to be a coroutine, not a function.

GH-4568
Move flush_async down to the specific async transport subclass. This makes more sense anyway, as
this will only be required by the async transport. If more async transports are expected,
another shared superclass can be created.

GH-4568
Add necessary type annotations to the core HttpTransport to accomodate for async transport.

GH-4568
Add an abstract bass class for implementation of the background worker. This was done to provide a shared interface for the current
implementation of a threaded worker in the sync context as well as the upcoming async task-based worker implementation.

GH-4578
Add a new factory method instead of direct instatiation of the threaded background worker.
This allows for easy extension to other types of workers, such as the upcoming task-based async worker.

GH-4578
Add a new flush_async method to worker ABC. This is necessary because the async transport cannot use a
synchronous blocking flush.

GH-4578
Move the flush_async down to the concrete subclass to not break existing testing. This makes sense,
as this will only really be needed by the async worker anyway and therefore is not shared logic.

GH-4578
Coroutines have a return value, however the current function signature for the worker methods does not
accomodate for this. Therefore, this signature was changed.

GH-4578
Add a new implementation of the worker interface, implementing the worker as an async task. This is
to be used by the upcoming async transport.

GH-4581
Refactor the flush method in the async worker to use the async_flush coroutine.

GH-4581
…unctions

Add a check to see wheter callbacks are awaitable coroutines or functions, as coroutines need to be awaited.

GH-4581
…coroutines

Coroutines do not return None, therefore it is necessary to consider this in the callback parameter of the worker. Previously,
only callbacks with return Type None were accepted.

GH-4581
Enable concurrent callbacks on async task worker by firing them as a task rather than awaiting them. A done callback handles the necessary queue and exception logic.

GH-4581
Changed kill to also use the _TERMINATOR sentinel, so the queue is still drained to this point on kill instead of cancelled immediately. This should also fix potential race conditions with flush_async.

GH-4581
Add proper type annotation to worker task list to fix linting problems

GH-4581
Add an implementation of Transport to work with the async background worker and HTTPCore async.

GH-4582
Async Transport now properly checks for the presence of the event loop in capture_envelop, and drops items
in case the event loop is no longer running for some reason.

GH-4582
Implement a kill method that properly shuts down the async transport. The httpcore async connection pool needs
to be explicitly shutdown at the end of its usage.

GH-4582
Fix type errors resulting from async override and missing type definition in the async transport.

GH-4582
Add a try/catch to ensure silent fail on kill in case the event loop shuts down.

GH-4582
Fix the event loop check in make_transport so that it does not throw a runtime error but rather falls back correctly.

GH-4582
Add a context manager to track internal task creation context. This can be used to track what tasks are created internally by the SDK

GH-4699
@srothh srothh changed the base branch from master to potel-base August 13, 2025 15:12
@srothh srothh changed the base branch from potel-base to srothh/async-transport-integration August 13, 2025 15:13
Copy link

codecov bot commented Aug 13, 2025

❌ 17 Tests Failed:

Tests completed Failed Passed Skipped
22082 17 22065 1232
View the top 3 failed test(s) by shortest run time
tests.integrations.redis.test_redis::test_pii_data_sent
Stack Traces | 0.082s run time
.../integrations/redis/test_redis.py:149: in test_pii_data_sent
    connection = FakeStrictRedis()
.tox/py3.11-redis-v4/lib/python3.11.../site-packages/fakeredis/_connection.py:185: in __init__
    super().__init__(**kwds)
E   TypeError: Redis.__init__() got an unexpected keyword argument 'lib_name'
tests.integrations.redis.test_redis::test_breadcrumbs
Stack Traces | 0.083s run time
.../integrations/redis/test_redis.py:231: in test_breadcrumbs
    connection = FakeStrictRedis()
.tox/py3.11-redis-v4/lib/python3.11.../site-packages/fakeredis/_connection.py:185: in __init__
    super().__init__(**kwds)
E   TypeError: Redis.__init__() got an unexpected keyword argument 'lib_name'
tests.integrations.redis.test_redis::test_redis_pipeline[True-True-expected_first_ten1]
Stack Traces | 0.083s run time
.../integrations/redis/test_redis.py:62: in test_redis_pipeline
    connection = FakeStrictRedis()
.tox/py3.11-redis-v4/lib/python3.11.../site-packages/fakeredis/_connection.py:185: in __init__
    super().__init__(**kwds)
E   TypeError: Redis.__init__() got an unexpected keyword argument 'lib_name'

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@srothh srothh marked this pull request as ready for review August 14, 2025 11:24
@srothh srothh requested a review from a team as a code owner August 14, 2025 11:24
cursor[bot]

This comment was marked as outdated.

Base automatically changed from srothh/async-transport-integration to srothh/transport-class-hierarchy August 14, 2025 13:31
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@sl0thentr0py sl0thentr0py merged commit 371ca96 into srothh/transport-class-hierarchy Aug 14, 2025
122 of 125 checks passed
@sl0thentr0py sl0thentr0py deleted the srothh/async-task-identifier branch August 14, 2025 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants