feat: Add async event processor - #472
Conversation
e7d9e77 to
0c83486
Compare
Implement the new AsyncEventProcessor interface with a concrete DefaultAsyncEventProcessor, matching the sync DefaultEventProcessor naming convention.
Triggers a flush and awaits delivery via a new inbox message, returning whether it completed within the timeout.
0c83486 to
ccb9c58
Compare
The async event delivery concurrency limiter no longer mirrors the sync FixedThreadPool's shape. BoundedTaskSet drops the unused name parameter and the thread vocabulary, reserves a slot synchronously at spawn (so a full set rejects rather than queues), and uses a done-callback plus asyncio.gather for cleanup and draining.
2230024 to
5ed9484
Compare
Remove the banner-style section headings; where they carried a useful note, capture it as a short class docstring instead.
| """A fixed-size pool of concurrent tasks that rejects jobs when its limit | ||
| is reached. Matches the contract of | ||
| ``ldclient.impl.fixed_thread_pool.FixedThreadPool``.""" | ||
| class BoundedTaskSet: |
There was a problem hiding this comment.
The AsyncWorkerPool was previously created to mirror the FixedThreadPool but some of the naming and params didn't seem to align with the async code.
…ction comments Rename drain() back to wait() (it awaits the in-flight tasks; it does not halt intake), simplify the job param to Callable[[], Coroutine], and remove banner-style section-heading comments from the async event/concurrency tests.
…cessors Move the event schema version constant into event_processor_common so both processors advertise the same X-LaunchDarkly-Event-Schema and can't drift.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
_trigger_flush now returns whether it handed the batch to a worker; the flush_and_wait handler retries (waiting for a free worker) until the batch is handed off, so a saturated worker pool no longer causes a false success.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 016e434. Configure here.
| self._diagnostic_flush_workers.stop() | ||
| await self._diagnostic_flush_workers.wait() | ||
|
|
||
| await self._http.close() |
There was a problem hiding this comment.
Stop can drop pending events
Medium Severity
stop promises to deliver all pending events, but _do_shutdown never flushes the outbox. The pre-stop flush() can be dropped when the inbox is full, or leave events buffered when flush workers are saturated—the same case flush_and_wait already retries—so those events are abandoned on shutdown.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 016e434. Configure here.
| log.debug('Sending events payload: ' + json_body) | ||
| payload_id = str(uuid.uuid4()) |
There was a problem hiding this comment.
would this leak redacted properties?
| elif message.type == 'stop': | ||
| await self._do_shutdown() | ||
| message.param.set() | ||
| return |
There was a problem hiding this comment.
If _doShutdown throws then it will go into the catch block, but that would not break out of the loop.
|
Shutdown event-delivery parity (sync vs. async) — decision needed While addressing the The proposed async fix is to have The catch: this gap exists identically in the sync Question: should we fix the sync side too, and if so —
(Leaning toward keeping this PR async-scoped and handling sync separately, but flagging for a call before applying.) |
2 participants
Adds the async analytics event delivery component for the async SDK client, extracted from the async SDK implementation branch (SDK-60).
Stacked on #471 (AsyncConfig) — review that first. Until #471 merges this PR shows its commits too; after merge a rebase drops them out.
What's here
AsyncEventProcessorinterface +DefaultAsyncEventProcessorconcrete implementation, mirroring the syncEventProcessor/DefaultEventProcessorsplit.send_event/flushare sync;stopis a coroutine.event_processor_common.pyso the sync and async processors share buffering, output formatting, and dispatch logic.DefaultEventProcessorsuite over an injected mock aiohttp session.SDK-2769
feat: Add async event processor
Note
Medium Risk
Touches analytics delivery and shared dispatch logic used by both sync and async paths; behavior is intended to mirror the existing sync processor but regressions could affect event loss, flush semantics, or processor disablement on HTTP errors.
Overview
Adds
DefaultAsyncEventProcessorand an asyncioEventDispatcherso the async SDK can buffer, batch, and POST analytics/diagnostic events overAsyncHTTPTransport, with the same inbox-driven model as the sync processor (timers, non-blockingsend_event/flush, asyncstop, andflush_and_waitthat retries flush until a batch is actually handed off under worker saturation).Refactors shared logic into
EventDispatcherBaseinevent_processor_common.py(_process_event, indexing/dedup, debug handling,_handle_response) and movesCURRENT_EVENT_SCHEMAthere; the syncEventDispatchernow subclasses the base instead of duplicating that code.Renames
AsyncWorkerPooltoBoundedTaskSet(try_run/wait/stop) for bounded concurrent flush tasks, with matching test updates intest_aio.py. Adds a largetest_async_event_processorsuite mirroring sync behavior (headers, gzip, diagnostics, HTTP error disablement, full inbox drops).Reviewed by Cursor Bugbot for commit 016e434. Bugbot is set up for automated code reviews on this repo. Configure here.