Skip to content

fix: allow @metric_scope sync wrappers to work inside asyncio event loops#141

Open
jaheyy wants to merge 2 commits intoawslabs:masterfrom
jaheyy:fix/metric-scope-inside-asyncio-v2
Open

fix: allow @metric_scope sync wrappers to work inside asyncio event loops#141
jaheyy wants to merge 2 commits intoawslabs:masterfrom
jaheyy:fix/metric-scope-inside-asyncio-v2

Conversation

@jaheyy
Copy link

@jaheyy jaheyy commented Mar 13, 2026

Fixes #131

Summary

Add synchronous flush and environment resolution paths that work regardless of whether an event loop is running:

resolve_environment_sync() in environment_detector.py — checks for a cached environment first, then detects whether an event loop is running. If no loop is running, uses asyncio.run(). If a loop is running, offloads the async resolution to a ThreadPoolExecutor to avoid the RuntimeError.
flush_sync() in MetricsLogger — calls resolve_environment_sync() and flushes the context synchronously. Shares the same __flush_with_environment() helper as the async flush().
• Sync wrappers in metric_scope/__init__.py now call logger.flush_sync() instead of asyncio.run(logger.flush()).

The async flush() path is unchanged — this is purely additive for the sync codepath.

Testing

• 4 new tests covering resolve_environment_sync, flush_sync, and both sync function/generator wrappers inside a running event loop

Dependencies

⚠️ This PR is stacked on #136, as there would be merge conflict anyway. This PR contains one extra commit on top of it.

@jaheyy jaheyy force-pushed the fix/metric-scope-inside-asyncio-v2 branch from d99be00 to 0cf359e Compare March 13, 2026 13:06
…oops

Add resolve_environment_sync() that uses cached environment when available
and falls back to a worker thread for first-time async resolution inside
a running event loop.

Add flush_sync() to MetricsLogger that uses the sync resolver. Sync
wrappers in @metric_scope now call flush_sync() instead of
asyncio.run(flush()), fixing RuntimeError when used inside a running loop.

Added tests to ensure @metric_scope works when sync functions are called
from within an async context.
@jaheyy jaheyy force-pushed the fix/metric-scope-inside-asyncio-v2 branch from 0cf359e to 128b620 Compare March 16, 2026 08:11
self.flush_preserve_dimensions: bool = False

def flush_sync(self) -> None:
environment = resolve_environment_sync()
Copy link
Contributor

@seoberha seoberha Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flush_sync() calls the module-level resolve_environment_sync() directly, while the async flush() uses self.resolve_environment — the callable injected via the constructor. This means flush_sync silently ignores any custom resolver passed at construction time.

Could we have resolve_environment_sync accept an optional async callable that defaults to the module-level resolver?

So something like

def resolve_environment_sync(
    resolve: Optional[Callable[..., Awaitable[Environment]]] = None,
) -> Environment:
    if EnvironmentCache.environment is not None:
        return EnvironmentCache.environment
    coro_fn = resolve or resolve_environment
    try:
        asyncio.get_running_loop()
    except RuntimeError:
        return asyncio.run(coro_fn())
    else:
        with concurrent.futures.ThreadPoolExecutor(max_workers=1) as pool:
            return pool.submit(asyncio.run, coro_fn()).result()

Then flush_sync can pass through the instance's resolver:

def flush_sync(self) -> None:
    environment = resolve_environment_sync(self.resolve_environment)
    self.__flush_with_environment(environment)

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.

@metric_scope makes wrong assumption to detect async event loop

2 participants