-
Notifications
You must be signed in to change notification settings - Fork 572
feat(asyncio): Add on-demand way to enable AsyncioIntegration #5288
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
| from threading import Lock | ||
| from typing import TYPE_CHECKING | ||
|
|
||
| import sentry_sdk | ||
| from sentry_sdk.utils import logger | ||
|
|
||
| if TYPE_CHECKING: | ||
|
|
@@ -279,6 +280,28 @@ def setup_integrations( | |
| return integrations | ||
|
|
||
|
|
||
| def _enable_integration(integration: "Integration") -> "Optional[Integration]": | ||
| identifier = integration.identifier | ||
| client = sentry_sdk.get_client() | ||
|
|
||
| with _installer_lock: | ||
| if identifier in client.integrations: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Race condition allows double-patching of asyncio event loopMedium Severity The Additional Locations (1)There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing global check allows duplicate
|
||
| logger.debug("Integration already enabled: %s", identifier) | ||
| return None | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm explicitly not allowing overwriting the old integration with the new one so that we don't end up double patching the event loop. We could add extra logic to restore the original code and re-patch that, but it seems like that's a lot of extra work for a usecase no one will probably need. |
||
|
|
||
| logger.debug("Setting up integration %s", identifier) | ||
| _processed_integrations.add(identifier) | ||
| try: | ||
| type(integration).setup_once() | ||
| integration.setup_once_with_options(client.options) | ||
| except DidNotEnable as e: | ||
| logger.debug("Did not enable integration %s: %s", identifier, e) | ||
| return None | ||
| else: | ||
| _installed_integrations.add(identifier) | ||
| return integration | ||
|
|
||
|
|
||
| def _check_minimum_version( | ||
| integration: "type[Integration]", | ||
| version: "Optional[tuple[int, ...]]", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |
|
|
||
| import sentry_sdk | ||
| from sentry_sdk.consts import OP | ||
| from sentry_sdk.integrations import Integration, DidNotEnable | ||
| from sentry_sdk.integrations import Integration, DidNotEnable, _enable_integration | ||
| from sentry_sdk.utils import event_from_exception, logger, reraise | ||
|
|
||
| try: | ||
|
|
@@ -138,3 +138,39 @@ class AsyncioIntegration(Integration): | |
| @staticmethod | ||
| def setup_once() -> None: | ||
| patch_asyncio() | ||
|
|
||
|
|
||
| def enable_asyncio_integration(*args: "Any", **kwargs: "Any") -> None: | ||
| """ | ||
| Enable AsyncioIntegration with the provided options. | ||
|
|
||
| This is useful in scenarios where Sentry needs to be initialized before | ||
| an event loop is set up, but you still want to instrument asyncio once there | ||
| is an event loop. In that case, you can sentry_sdk.init() early on without | ||
| the AsyncioIntegration and then, once the event loop has been set up, execute | ||
|
|
||
| ```python | ||
| from sentry_sdk.integrations.asyncio import enable_asyncio_integration | ||
|
|
||
| async def async_entrypoint(): | ||
| enable_asyncio_integration() | ||
| ``` | ||
|
|
||
| Any arguments provided will be passed to AsyncioIntegration() as is. | ||
|
|
||
| If AsyncioIntegration is already enabled (e.g. because it was provided in | ||
| sentry_sdk.init(integrations=[...])), this function won't have any effect. | ||
|
|
||
| If AsyncioIntegration was provided in | ||
| sentry_sdk.init(disabled_integrations=[...]), this function will ignore that | ||
| and the integration will be enabled. | ||
| """ | ||
| client = sentry_sdk.get_client() | ||
| if not client.is_active(): | ||
| return | ||
|
|
||
| integration = _enable_integration(AsyncioIntegration(*args, **kwargs)) | ||
| if integration is None: | ||
| return | ||
|
|
||
| client.integrations[integration.identifier] = integration | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it could make sense to move this assignment to |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my understanding, and no action required:
do you know if there is a reason we have both
Client.integrationsand thesentry_sdk.integrations._installed_integrationsglobal?