Skip to content

Commit 90265ba

Browse files
fix(sdk): use sync shutdown flush fallback (#150)
## Summary - replace the atexit shutdown fallback that spawned a helper thread and `asyncio.run()` with a synchronous `httpx.Client` flush path - reuse shared batch request construction for both async and sync send paths and clear the async client reference after fallback shutdown cleanup - add regression coverage for the sync fallback path and for a short-lived subprocess that emits an SDK-evaluated observability event and exits without explicit shutdown
1 parent dea2873 commit 90265ba

3 files changed

Lines changed: 468 additions & 32 deletions

File tree

‎sdks/python/src/agent_control/observability.py‎

Lines changed: 134 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import atexit
4949
import logging
5050
import threading
51+
import time
5152
from dataclasses import dataclass, field
5253
from typing import TYPE_CHECKING, Any
5354

@@ -388,29 +389,142 @@ def _stop_worker(self, *, graceful: bool, join_timeout: float) -> bool:
388389
self._thread = None
389390
return True
390391

391-
def _flush_all_without_worker(self, *, timeout: float) -> None:
392-
"""Flush remaining events in a helper thread when no worker loop is available."""
393-
flush_error: Exception | None = None
392+
def _build_batch_request(
393+
self,
394+
events: list[ControlExecutionEvent],
395+
) -> tuple[str, dict[str, str], dict[str, Any]]:
396+
"""Build request components shared by async and sync send paths."""
397+
url = f"{self.server_url}/api/v1/observability/events"
398+
headers = {"Content-Type": "application/json"}
399+
if self.api_key:
400+
headers["X-API-Key"] = self.api_key
401+
payload = {"events": [event.model_dump(mode="json") for event in events]}
402+
return url, headers, payload
403+
404+
def _send_batch_sync(
405+
self,
406+
events: list[ControlExecutionEvent],
407+
*,
408+
deadline: float | None = None,
409+
) -> bool:
410+
"""Send a batch synchronously for shutdown/atexit fallback."""
411+
url, headers, payload = self._build_batch_request(events)
412+
413+
for attempt in range(get_settings().max_retries):
414+
remaining: float | None = None
415+
if deadline is not None:
416+
remaining = deadline - time.monotonic()
417+
if remaining <= 0:
418+
logger.warning("Fallback shutdown flush timed out before sending events")
419+
return False
420+
421+
request_timeout = 30.0
422+
if remaining is not None:
423+
request_timeout = max(0.001, min(request_timeout, remaining))
394424

395-
def run_flush() -> None:
396-
nonlocal flush_error
397425
try:
398-
asyncio.run(self.flush_all())
426+
with httpx.Client(timeout=request_timeout) as client:
427+
response = client.post(url, json=payload, headers=headers)
428+
429+
if response.status_code == 202:
430+
return True
431+
if response.status_code == 401:
432+
logger.error("Authentication failed - check API key")
433+
return False
434+
435+
logger.warning(
436+
"Server returned %s during shutdown flush: %s",
437+
response.status_code,
438+
response.text,
439+
)
440+
except httpx.TimeoutException:
441+
logger.warning("Timeout sending events during shutdown (attempt %d)", attempt + 1)
442+
except httpx.ConnectError:
443+
logger.warning(
444+
"Connection error sending events during shutdown (attempt %d)",
445+
attempt + 1,
446+
)
399447
except Exception as e:
400-
flush_error = e
448+
logger.error("Error sending events during shutdown: %s", e)
401449

402-
helper_thread = threading.Thread(
403-
target=run_flush,
404-
name="agent-control-event-batcher-shutdown-flush",
405-
daemon=True,
406-
)
407-
helper_thread.start()
408-
helper_thread.join(timeout=timeout)
409-
if helper_thread.is_alive():
410-
logger.warning("Fallback shutdown flush timed out after %.1f seconds", timeout)
411-
return
412-
if flush_error is not None:
413-
logger.error("Error during fallback shutdown flush: %s", flush_error)
450+
if attempt >= get_settings().max_retries - 1:
451+
break
452+
453+
retry_delay = get_settings().retry_delay * (attempt + 1)
454+
if deadline is not None:
455+
remaining = deadline - time.monotonic()
456+
if remaining <= 0:
457+
logger.warning("Fallback shutdown flush timed out during retry backoff")
458+
return False
459+
retry_delay = min(retry_delay, remaining)
460+
461+
if retry_delay > 0:
462+
time.sleep(retry_delay)
463+
464+
return False
465+
466+
def _flush_sync(self, *, deadline: float | None = None) -> bool:
467+
"""Flush a batch synchronously without relying on asyncio state."""
468+
with self._lock:
469+
if not self._events:
470+
return True
471+
events_to_send = self._events[:self.batch_size]
472+
self._events = self._events[self.batch_size:]
473+
474+
success = self._send_batch_sync(events_to_send, deadline=deadline)
475+
476+
if success:
477+
with self._lock:
478+
self._events_sent += len(events_to_send)
479+
self._flush_count += 1
480+
total_sent = self._events_sent
481+
logger.debug(
482+
f"Flushed {len(events_to_send)} events "
483+
f"(total sent: {total_sent})"
484+
)
485+
return True
486+
487+
with self._lock:
488+
self._events = events_to_send + self._events
489+
logger.warning(f"Failed to send batch, re-queued {len(events_to_send)} events")
490+
return False
491+
492+
def _flush_all_without_worker(self, *, timeout: float) -> None:
493+
"""Flush remaining events synchronously when no worker loop is available."""
494+
deadline = time.monotonic() + max(timeout, 0.0)
495+
consecutive_failures = 0
496+
497+
while True:
498+
with self._lock:
499+
if not self._events:
500+
break
501+
502+
if time.monotonic() >= deadline:
503+
logger.warning("Fallback shutdown flush timed out after %.1f seconds", timeout)
504+
break
505+
506+
flushed = self._flush_sync(deadline=deadline)
507+
if flushed:
508+
consecutive_failures = 0
509+
continue
510+
511+
consecutive_failures += 1
512+
if consecutive_failures >= self.shutdown_max_failed_flushes:
513+
with self._lock:
514+
pending = len(self._events)
515+
logger.warning(
516+
"Stopping sync shutdown flush after %d consecutive failed flushes; "
517+
"%d event(s) pending",
518+
consecutive_failures,
519+
pending,
520+
)
521+
break
522+
523+
# The worker thread normally closes the async client. If shutdown races or
524+
# interpreter teardown prevented that path from completing, we intentionally
525+
# do not try to aclose() here because this fallback may be running after
526+
# asyncio teardown. Drop the reference so shutdown can finish deterministically.
527+
self._client = None
414528

415529
def stop(self) -> None:
416530
"""Stop the worker thread. Does not flush remaining events."""
@@ -506,9 +620,6 @@ async def _flush(self) -> bool:
506620
events_to_send = self._events[:self.batch_size]
507621
self._events = self._events[self.batch_size:]
508622

509-
if not events_to_send:
510-
return True
511-
512623
success = await self._send_batch(events_to_send)
513624

514625
if success:
@@ -543,14 +654,7 @@ async def _send_batch(self, events: list[ControlExecutionEvent]) -> bool:
543654
Returns:
544655
True if sent successfully, False otherwise
545656
"""
546-
url = f"{self.server_url}/api/v1/observability/events"
547-
headers = {"Content-Type": "application/json"}
548-
if self.api_key:
549-
headers["X-API-Key"] = self.api_key
550-
551-
payload = {
552-
"events": [event.model_dump(mode="json") for event in events]
553-
}
657+
url, headers, payload = self._build_batch_request(events)
554658

555659
client = await self._get_client()
556660

0 commit comments

Comments
 (0)