From a2375b07b809cd83008dcb1e5455c5ac8f3634c8 Mon Sep 17 00:00:00 2001 From: Vladimir Maksimuk Date: Fri, 11 Mar 2022 10:24:47 +0300 Subject: [PATCH 1/6] Change Generator Context Manager to Class-based context manager to avoid thread race at the cost of working with the generator --- sentry_sdk/integrations/django/__init__.py | 6 +- sentry_sdk/integrations/sqlalchemy.py | 4 +- sentry_sdk/tracing_utils.py | 91 ++++++++++++---------- 3 files changed, 53 insertions(+), 48 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index e11d1ab651..db90918529 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -9,7 +9,7 @@ from sentry_sdk.hub import Hub, _should_send_default_pii from sentry_sdk.scope import add_global_event_processor from sentry_sdk.serializer import add_global_repr_processor -from sentry_sdk.tracing_utils import record_sql_queries +from sentry_sdk.tracing_utils import RecordSqlQueries from sentry_sdk.utils import ( HAS_REAL_CONTEXTVARS, CONTEXTVARS_ERROR_MESSAGE, @@ -539,7 +539,7 @@ def execute(self, sql, params=None): if hub.get_integration(DjangoIntegration) is None: return real_execute(self, sql, params) - with record_sql_queries( + with RecordSqlQueries( hub, self.cursor, sql, params, paramstyle="format", executemany=False ): return real_execute(self, sql, params) @@ -550,7 +550,7 @@ def executemany(self, sql, param_list): if hub.get_integration(DjangoIntegration) is None: return real_executemany(self, sql, param_list) - with record_sql_queries( + with RecordSqlQueries( hub, self.cursor, sql, param_list, paramstyle="format", executemany=True ): return real_executemany(self, sql, param_list) diff --git a/sentry_sdk/integrations/sqlalchemy.py b/sentry_sdk/integrations/sqlalchemy.py index 4b0207f5ec..6f776e40c8 100644 --- a/sentry_sdk/integrations/sqlalchemy.py +++ b/sentry_sdk/integrations/sqlalchemy.py @@ -3,7 +3,7 @@ from sentry_sdk._types import MYPY from sentry_sdk.hub import Hub from sentry_sdk.integrations import Integration, DidNotEnable -from sentry_sdk.tracing_utils import record_sql_queries +from sentry_sdk.tracing_utils import RecordSqlQueries try: from sqlalchemy.engine import Engine # type: ignore @@ -50,7 +50,7 @@ def _before_cursor_execute( if hub.get_integration(SqlalchemyIntegration) is None: return - ctx_mgr = record_sql_queries( + ctx_mgr = RecordSqlQueries( hub, cursor, statement, diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index faed37cbb7..1e8a74e6fe 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -106,6 +106,54 @@ def __iter__(self): yield k[len(self.prefix) :] +class RecordSqlQueries: + def __init__( + self, + hub, # type: sentry_sdk.Hub + cursor, # type: Any + query, # type: Any + params_list, # type: Any + paramstyle, # type: Optional[str] + executemany, # type: bool + ) -> None: + # TODO: Bring back capturing of params by default + self._hub = hub + if self._hub.client and self._hub.client.options["_experiments"].get( + "record_sql_params", False + ): + if not params_list or params_list == [None]: + params_list = None + + if paramstyle == "pyformat": + paramstyle = "format" + else: + params_list = None + paramstyle = None + + self._query = _format_sql(cursor, query) + + self._data = {} + if params_list is not None: + self._data["db.params"] = params_list + if paramstyle is not None: + self._data["db.paramstyle"] = paramstyle + if executemany: + self._data["db.executemany"] = True + + with capture_internal_exceptions(): + hub.add_breadcrumb(message=self._query, category="query", data=self._data) + + def __enter__(self): + # type: () -> Span + with self._hub.start_span(op="db", description=self._query) as span: + for k, v in self._data.items(): + span.set_data(k, v) + return span + + def __exit__(self, exc_type, exc_val, exc_tb): + pass + + def has_tracing_enabled(options): # type: (Dict[str, Any]) -> bool """ @@ -150,49 +198,6 @@ def is_valid_sample_rate(rate): return True -@contextlib.contextmanager -def record_sql_queries( - hub, # type: sentry_sdk.Hub - cursor, # type: Any - query, # type: Any - params_list, # type: Any - paramstyle, # type: Optional[str] - executemany, # type: bool -): - # type: (...) -> Generator[Span, None, None] - - # TODO: Bring back capturing of params by default - if hub.client and hub.client.options["_experiments"].get( - "record_sql_params", False - ): - if not params_list or params_list == [None]: - params_list = None - - if paramstyle == "pyformat": - paramstyle = "format" - else: - params_list = None - paramstyle = None - - query = _format_sql(cursor, query) - - data = {} - if params_list is not None: - data["db.params"] = params_list - if paramstyle is not None: - data["db.paramstyle"] = paramstyle - if executemany: - data["db.executemany"] = True - - with capture_internal_exceptions(): - hub.add_breadcrumb(message=query, category="query", data=data) - - with hub.start_span(op="db", description=query) as span: - for k, v in data.items(): - span.set_data(k, v) - yield span - - def maybe_create_breadcrumbs_from_span(hub, span): # type: (sentry_sdk.Hub, Span) -> None if span.op == "redis": From c5f48532d3c06602e7d50faad72b0bcfd4fb5891 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 11 Mar 2022 13:49:57 +0100 Subject: [PATCH 2/6] Lint --- sentry_sdk/tracing_utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 1e8a74e6fe..aeee362226 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -1,5 +1,4 @@ import re -import contextlib import json import math From 046c9713da1ffc3db20f360b868d9c64b84610f1 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 11 Mar 2022 13:56:00 +0100 Subject: [PATCH 3/6] mypy --- sentry_sdk/tracing_utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index aeee362226..5cf5d91003 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -150,6 +150,7 @@ def __enter__(self): return span def __exit__(self, exc_type, exc_val, exc_tb): + # type: (Any, Any, Any) -> None pass From b872386a50b7dca35c6b8fa1c97a0fb5300c100c Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 11 Mar 2022 16:11:13 +0100 Subject: [PATCH 4/6] 2.7 mypy init --- sentry_sdk/tracing_utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 5cf5d91003..ec9fc7a66f 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -114,7 +114,8 @@ def __init__( params_list, # type: Any paramstyle, # type: Optional[str] executemany, # type: bool - ) -> None: + ): + # type: (...) -> None # TODO: Bring back capturing of params by default self._hub = hub if self._hub.client and self._hub.client.options["_experiments"].get( From c25d61e31cb68025dd9d393879f1adef18a92030 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 14 Mar 2022 16:18:40 +0100 Subject: [PATCH 5/6] Move breadcrumb to __enter__ --- sentry_sdk/tracing_utils.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index ec9fc7a66f..cceadebecd 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -140,11 +140,12 @@ def __init__( if executemany: self._data["db.executemany"] = True - with capture_internal_exceptions(): - hub.add_breadcrumb(message=self._query, category="query", data=self._data) def __enter__(self): # type: () -> Span + with capture_internal_exceptions(): + self._hub.add_breadcrumb(message=self._query, category="query", data=self._data) + with self._hub.start_span(op="db", description=self._query) as span: for k, v in self._data.items(): span.set_data(k, v) From 6d37965624b99df75c31841a7ea331d57905190a Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 14 Mar 2022 16:21:54 +0100 Subject: [PATCH 6/6] lint --- sentry_sdk/tracing_utils.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index cceadebecd..d754da409c 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -140,11 +140,12 @@ def __init__( if executemany: self._data["db.executemany"] = True - def __enter__(self): # type: () -> Span with capture_internal_exceptions(): - self._hub.add_breadcrumb(message=self._query, category="query", data=self._data) + self._hub.add_breadcrumb( + message=self._query, category="query", data=self._data + ) with self._hub.start_span(op="db", description=self._query) as span: for k, v in self._data.items():