integration for aiomysql #4703
integration for aiomysql #4703
Conversation
![seer-by-sentry[bot]](https://avatars.githubusercontent.com/in/801464?s=60&v=4){kind=small}
sentry_sdk/integrations/aiomysql.py
Outdated
| aiomysql.Connection.cursor = _wrap_cursor_creation( | ||
| aiomysql.Connection.cursor | ||
| ) | ||
|
|
||
| aiomysql.connect = _wrap_connect(aiomysql.connect) | ||
|
|
||
|
|
||
| T = TypeVar("T") | ||
|
|
||
|
|
||
| def _wrap_execute(f: Callable[..., Awaitable[T]]) -> Callable[..., Awaitable[T]]: | ||
| async def _inner(*args: Any, **kwargs: Any) -> T: | ||
| if sentry_sdk.get_client().get_integration(AioMySQLIntegration) is None: | ||
| return await f(*args, **kwargs) | ||
|
|
||
| conn = args[0] | ||
| query = args[1] # В aiomysql запрос передается первым аргументом | ||
| with record_sql_queries( | ||
| cursor=None, | ||
| query=query, | ||
| params_list=None, | ||
| paramstyle=None, | ||
| executemany=False, | ||
| span_origin=AioMySQLIntegration.origin, | ||
| ) as span: | ||
| res = await f(*args, **kwargs) | ||
| span.set_data("db.affected_rows", res) | ||
|
|
||
| with capture_internal_exceptions(): | ||
| add_query_source(span) | ||
|
|
||
| return res | ||
|
|
||
| return _inner | ||
|
|
||
|
|
||
| SubCursor = TypeVar("SubCursor", bound=Cursor) | ||
|
|
||
|
|
||
| @contextlib.contextmanager | ||
| def _record( | ||
| cursor: SubCursor | None, | ||
| query: str, | ||
| params_list: tuple[Any, ...] | None, | ||
| *, | ||
| executemany: bool = False, | ||
| ) -> Iterator[Span]: | ||
| integration = sentry_sdk.get_client().get_integration(AioMySQLIntegration) | ||
| if integration is not None and not integration._record_params: | ||
| params_list = None | ||
|
|
||
| param_style = "pyformat" if params_list else None | ||
|
|
||
| with record_sql_queries( | ||
| cursor=cursor, | ||
| query=query, | ||
| params_list=params_list, | ||
| paramstyle=param_style, | ||
| executemany=executemany, | ||
| record_cursor_repr=cursor is not None, | ||
| span_origin=AioMySQLIntegration.origin, | ||
| ) as span: | ||
| yield span | ||
|
|
||
|
|
||
| def _wrap_cursor_creation(f: Callable[..., T]) -> Callable[..., T]: | ||
| @ensure_integration_enabled(AioMySQLIntegration, f) | ||
| def _inner(*args: Any, **kwargs: Any) -> T: | ||
| query = args[0] | ||
| params_list = args[1] if len(args) > 1 else None | ||
|
|
||
| with _record(None, query, params_list, executemany=False) as span: | ||
| _set_db_data(span, args[0]) | ||
| res = f(*args, **kwargs) | ||
| span.set_data("db.cursor", res._coro.result()) | ||
|
|
||
| return res | ||
|
|
||
| return _inner |
There was a problem hiding this comment.
Potential bug: The wrapper for aiomysql.Connection.cursor incorrectly assumes it receives a query argument, leading to an IndexError when accessing non-existent arguments and causing a crash.
-
Description: The
_wrap_cursor_creationfunction incorrectly wrapsaiomysql.Connection.cursor. It was adapted from theasyncpgintegration and assumes the method signature includes a query and parameters. However,aiomysql.Connection.cursor()does not accept a query; it only creates a cursor object. When a standard call likeconn.cursor()is made, the wrapper attempts to accessargs[1]for query parameters. Since this argument does not exist, anIndexErroris raised, causing the application to crash. The instrumentation should be applied to cursor execution methods, not the creation method. -
Suggested fix: The wrapper logic is flawed because
aiomysql.Connection.cursor()does not execute queries. The instrumentation should target the cursor's execution methods, such ascursor.execute()andcursor.executemany(), which actually receive the SQL query and parameters. Remove the wrapper forConnection.cursor()and apply it to the appropriate cursor methods instead.
severity: 0.85, confidence: 0.9
Did we get this right? 👍 / 👎 to inform future reviews.
sentry_sdk/integrations/aiomysql.py
Outdated
| host = kwargs["host"] | ||
| port = kwargs.get("port") or 3306 | ||
| user = kwargs["user"] | ||
| db = kwargs["db"] |
There was a problem hiding this comment.
Potential bug: The _wrap_connect function will crash with a KeyError because it directly accesses optional keyword arguments like host and user which may not be provided.
-
Description: In the
_wrap_connectfunction, connection parametershost,user, anddbare accessed directly from thekwargsdictionary using bracket notation (e.g.,kwargs["host"]). Theaiomysql.connectfunction treats these as optional parameters with default values, so they may not be present in a given call. If a developer relies on these defaults and omits the keys, aKeyErrorwill be raised. This unhandled exception will propagate and crash the application. The inconsistent use of safe access (kwargs.get()) for theportparameter in the same function suggests this was an oversight. -
Suggested fix: To prevent a
KeyError, modify the directkwargsaccess forhost,user, anddbto use the safekwargs.get()method. This change allows the underlyingaiomysqllibrary to handle default values as intended when these optional parameters are not explicitly provided in the function call.
severity: 0.78, confidence: 0.95
Did we get this right? 👍 / 👎 to inform future reviews.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
|
||
| user = conn.user | ||
| if user: | ||
| span.set_data(SPANDATA.DB_USER, user) |
There was a problem hiding this comment.
Bug: Unused Function in Integration Code
The _set_db_data function is defined but unused in this integration. It appears to be dead code, likely copied from the asyncpg integration without being integrated into aiomysql's logic.
Thank you for contributing to
sentry-python! Please add tests to validate your changes, and lint your code usingtox -e linters.Running the test suite on your PR might require maintainer approval.