From 23ef8cadc796e936744140c6179d674a89542a28 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 13 Sep 2024 12:23:46 +0200 Subject: [PATCH] Removed experimental explain_plan feature. (#3534) Attaching the database explain plan to a db span was an experimental feature done in an Sentry Hackweek. As we are moving into an Otel world, we remove this experiment from our Repository. There is still a branch experiment/explain_plans on Github to keep the code for future reference: https://github.com/getsentry/sentry-python/tree/experiment/explain_plans (maybe we can copy the code into the Opentelemetry instrumentation if we want to see this feature in the future) --- sentry_sdk/consts.py | 1 - sentry_sdk/db/__init__.py | 0 sentry_sdk/db/explain_plan/__init__.py | 59 ---------------------- sentry_sdk/db/explain_plan/django.py | 48 ------------------ sentry_sdk/db/explain_plan/sqlalchemy.py | 48 ------------------ sentry_sdk/integrations/django/__init__.py | 15 ------ sentry_sdk/integrations/sqlalchemy.py | 13 ----- 7 files changed, 184 deletions(-) delete mode 100644 sentry_sdk/db/__init__.py delete mode 100644 sentry_sdk/db/explain_plan/__init__.py delete mode 100644 sentry_sdk/db/explain_plan/django.py delete mode 100644 sentry_sdk/db/explain_plan/sqlalchemy.py diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 5f79031787..803b159299 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -53,7 +53,6 @@ class EndpointType(Enum): Experiments = TypedDict( "Experiments", { - "attach_explain_plans": dict[str, Any], "max_spans": Optional[int], "record_sql_params": Optional[bool], "continuous_profiling_auto_start": Optional[bool], diff --git a/sentry_sdk/db/__init__.py b/sentry_sdk/db/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/sentry_sdk/db/explain_plan/__init__.py b/sentry_sdk/db/explain_plan/__init__.py deleted file mode 100644 index 1cc475f0f4..0000000000 --- a/sentry_sdk/db/explain_plan/__init__.py +++ /dev/null @@ -1,59 +0,0 @@ -from datetime import datetime, timedelta, timezone -from typing import TYPE_CHECKING - -if TYPE_CHECKING: - from typing import Any - - -EXPLAIN_CACHE = {} -EXPLAIN_CACHE_SIZE = 50 -EXPLAIN_CACHE_TIMEOUT_SECONDS = 60 * 60 * 24 - - -def cache_statement(statement, options): - # type: (str, dict[str, Any]) -> None - global EXPLAIN_CACHE - - now = datetime.now(timezone.utc) - explain_cache_timeout_seconds = options.get( - "explain_cache_timeout_seconds", EXPLAIN_CACHE_TIMEOUT_SECONDS - ) - expiration_time = now + timedelta(seconds=explain_cache_timeout_seconds) - - EXPLAIN_CACHE[hash(statement)] = expiration_time - - -def remove_expired_cache_items(): - # type: () -> None - """ - Remove expired cache items from the cache. - """ - global EXPLAIN_CACHE - - now = datetime.now(timezone.utc) - - for key, expiration_time in EXPLAIN_CACHE.items(): - expiration_in_the_past = expiration_time < now - if expiration_in_the_past: - del EXPLAIN_CACHE[key] - - -def should_run_explain_plan(statement, options): - # type: (str, dict[str, Any]) -> bool - """ - Check cache if the explain plan for the given statement should be run. - """ - global EXPLAIN_CACHE - - remove_expired_cache_items() - - key = hash(statement) - if key in EXPLAIN_CACHE: - return False - - explain_cache_size = options.get("explain_cache_size", EXPLAIN_CACHE_SIZE) - cache_is_full = len(EXPLAIN_CACHE.keys()) >= explain_cache_size - if cache_is_full: - return False - - return True diff --git a/sentry_sdk/db/explain_plan/django.py b/sentry_sdk/db/explain_plan/django.py deleted file mode 100644 index 21ebc9c81a..0000000000 --- a/sentry_sdk/db/explain_plan/django.py +++ /dev/null @@ -1,48 +0,0 @@ -from typing import TYPE_CHECKING - -from sentry_sdk.db.explain_plan import cache_statement, should_run_explain_plan - -if TYPE_CHECKING: - from typing import Any - from typing import Callable - - from sentry_sdk.tracing import Span - - -def attach_explain_plan_to_span( - span, connection, statement, parameters, mogrify, options -): - # type: (Span, Any, str, Any, Callable[[str, Any], bytes], dict[str, Any]) -> None - """ - Run EXPLAIN or EXPLAIN ANALYZE on the given statement and attach the explain plan to the span data. - - Usage: - ``` - sentry_sdk.init( - dsn="...", - _experiments={ - "attach_explain_plans": { - "explain_cache_size": 1000, # Run explain plan for the 1000 most run queries - "explain_cache_timeout_seconds": 60 * 60 * 24, # Run the explain plan for each statement only every 24 hours - "use_explain_analyze": True, # Run "explain analyze" instead of only "explain" - } - } - ``` - """ - if not statement.strip().upper().startswith("SELECT"): - return - - if not should_run_explain_plan(statement, options): - return - - analyze = "ANALYZE" if options.get("use_explain_analyze", False) else "" - explain_statement = ("EXPLAIN %s " % analyze) + mogrify( - statement, parameters - ).decode("utf-8") - - with connection.cursor() as cursor: - cursor.execute(explain_statement) - explain_plan = [row for row in cursor.fetchall()] - - span.set_data("db.explain_plan", explain_plan) - cache_statement(statement, options) diff --git a/sentry_sdk/db/explain_plan/sqlalchemy.py b/sentry_sdk/db/explain_plan/sqlalchemy.py deleted file mode 100644 index 9320ff8fb3..0000000000 --- a/sentry_sdk/db/explain_plan/sqlalchemy.py +++ /dev/null @@ -1,48 +0,0 @@ -from typing import TYPE_CHECKING - -from sentry_sdk.db.explain_plan import cache_statement, should_run_explain_plan -from sentry_sdk.integrations import DidNotEnable - -try: - from sqlalchemy.sql import text # type: ignore -except ImportError: - raise DidNotEnable("SQLAlchemy not installed.") - -if TYPE_CHECKING: - from typing import Any - - from sentry_sdk.tracing import Span - - -def attach_explain_plan_to_span(span, connection, statement, parameters, options): - # type: (Span, Any, str, Any, dict[str, Any]) -> None - """ - Run EXPLAIN or EXPLAIN ANALYZE on the given statement and attach the explain plan to the span data. - - Usage: - ``` - sentry_sdk.init( - dsn="...", - _experiments={ - "attach_explain_plans": { - "explain_cache_size": 1000, # Run explain plan for the 1000 most run queries - "explain_cache_timeout_seconds": 60 * 60 * 24, # Run the explain plan for each statement only every 24 hours - "use_explain_analyze": True, # Run "explain analyze" instead of only "explain" - } - } - ``` - """ - if not statement.strip().upper().startswith("SELECT"): - return - - if not should_run_explain_plan(statement, options): - return - - analyze = "ANALYZE" if options.get("use_explain_analyze", False) else "" - explain_statement = (("EXPLAIN %s " % analyze) + statement) % parameters - - result = connection.execute(text(explain_statement)) - explain_plan = [row for row in result] - - span.set_data("db.explain_plan", explain_plan) - cache_statement(statement, options) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index f6821dfa18..fce93503e9 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -6,7 +6,6 @@ import sentry_sdk from sentry_sdk.consts import OP, SPANDATA -from sentry_sdk.db.explain_plan.django import attach_explain_plan_to_span from sentry_sdk.scope import add_global_event_processor, should_send_default_pii from sentry_sdk.serializer import add_global_repr_processor from sentry_sdk.tracing import SOURCE_FOR_STYLE, TRANSACTION_SOURCE_URL @@ -634,20 +633,6 @@ def execute(self, sql, params=None): span_origin=DjangoIntegration.origin_db, ) as span: _set_db_data(span, self) - options = ( - sentry_sdk.get_client() - .options["_experiments"] - .get("attach_explain_plans") - ) - if options is not None: - attach_explain_plan_to_span( - span, - self.cursor.connection, - sql, - params, - self.mogrify, - options, - ) result = real_execute(self, sql, params) with capture_internal_exceptions(): diff --git a/sentry_sdk/integrations/sqlalchemy.py b/sentry_sdk/integrations/sqlalchemy.py index a968b7db9e..0a54108e75 100644 --- a/sentry_sdk/integrations/sqlalchemy.py +++ b/sentry_sdk/integrations/sqlalchemy.py @@ -1,6 +1,4 @@ -import sentry_sdk from sentry_sdk.consts import SPANSTATUS, SPANDATA -from sentry_sdk.db.explain_plan.sqlalchemy import attach_explain_plan_to_span from sentry_sdk.integrations import Integration, DidNotEnable from sentry_sdk.tracing_utils import add_query_source, record_sql_queries from sentry_sdk.utils import ( @@ -68,17 +66,6 @@ def _before_cursor_execute( if span is not None: _set_db_data(span, conn) - options = ( - sentry_sdk.get_client().options["_experiments"].get("attach_explain_plans") - ) - if options is not None: - attach_explain_plan_to_span( - span, - conn, - statement, - parameters, - options, - ) context._sentry_sql_span = span