Change code.filepath frame picking logic (#2568)

- search for the frame directly from the execute wrappers
- honor `in_app_include` and `in_app_exclude`
- fix Python 2 compatibility (`co_filename` is not always absolute)

Author: sentrivana

sentry_sdk/integrations/asyncpg.py

@@ -8,7 +8,7 @@
 from sentry_sdk.consts import OP, SPANDATA
 from sentry_sdk.integrations import Integration, DidNotEnable
 from sentry_sdk.tracing import Span
-from sentry_sdk.tracing_utils import record_sql_queries
+from sentry_sdk.tracing_utils import add_query_source, record_sql_queries
 from sentry_sdk.utils import parse_version, capture_internal_exceptions
 
 try:
@@ -66,8 +66,14 @@ async def _inner(*args: Any, **kwargs: Any) -> T:
             return await f(*args, **kwargs)
 
         query = args[1]
-        with record_sql_queries(hub, None, query, None, None, executemany=False):
+        with record_sql_queries(
+            hub, None, query, None, None, executemany=False
+        ) as span:
             res = await f(*args, **kwargs)
+
+        with capture_internal_exceptions():
+            add_query_source(hub, span)
+
         return res
 
     return _inner
@@ -118,6 +124,7 @@ async def _inner(*args: Any, **kwargs: Any) -> T:
         with _record(hub, None, query, params_list, executemany=executemany) as span:
             _set_db_data(span, args[0])
             res = await f(*args, **kwargs)
+
         return res
 
     return _inner

sentry_sdk/integrations/django/__init__.py

@@ -15,7 +15,7 @@
 from sentry_sdk.scope import add_global_event_processor
 from sentry_sdk.serializer import add_global_repr_processor
 from sentry_sdk.tracing import SOURCE_FOR_STYLE, TRANSACTION_SOURCE_URL
-from sentry_sdk.tracing_utils import record_sql_queries
+from sentry_sdk.tracing_utils import add_query_source, record_sql_queries
 from sentry_sdk.utils import (
     AnnotatedValue,
     HAS_REAL_CONTEXTVARS,
@@ -638,7 +638,12 @@ def execute(self, sql, params=None):
                         self.mogrify,
                         options,
                     )
-            return real_execute(self, sql, params)
+            result = real_execute(self, sql, params)
+
+        with capture_internal_exceptions():
+            add_query_source(hub, span)
+
+        return result
 
     def executemany(self, sql, param_list):
         # type: (CursorWrapper, Any, List[Any]) -> Any
@@ -650,7 +655,13 @@ def executemany(self, sql, param_list):
             hub, self.cursor, sql, param_list, paramstyle="format", executemany=True
         ) as span:
             _set_db_data(span, self)
-            return real_executemany(self, sql, param_list)
+
+            result = real_executemany(self, sql, param_list)
+
+        with capture_internal_exceptions():
+            add_query_source(hub, span)
+
+        return result
 
     def connect(self):
         # type: (BaseDatabaseWrapper) -> None

sentry_sdk/integrations/sqlalchemy.py

@@ -6,9 +6,8 @@
 from sentry_sdk.db.explain_plan.sqlalchemy import attach_explain_plan_to_span
 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.utils import parse_version
+from sentry_sdk.tracing_utils import add_query_source, record_sql_queries
+from sentry_sdk.utils import capture_internal_exceptions, parse_version
 
 try:
     from sqlalchemy.engine import Engine  # type: ignore
@@ -84,6 +83,10 @@ def _before_cursor_execute(
 
 def _after_cursor_execute(conn, cursor, statement, parameters, context, *args):
     # type: (Any, Any, Any, Any, Any, *Any) -> None
+    hub = Hub.current
+    if hub.get_integration(SqlalchemyIntegration) is None:
+        return
+
     ctx_mgr = getattr(
         context, "_sentry_sql_span_manager", None
     )  # type: Optional[ContextManager[Any]]
@@ -92,6 +95,11 @@ def _after_cursor_execute(conn, cursor, statement, parameters, context, *args):
         context._sentry_sql_span_manager = None
         ctx_mgr.__exit__(None, None, None)
 
+    span = context._sentry_sql_span
+    if span is not None:
+        with capture_internal_exceptions():
+            add_query_source(hub, span)
+
 
 def _handle_error(context, *args):
     # type: (Any, *Any) -> None

sentry_sdk/tracing.py

@@ -488,7 +488,6 @@ def finish(self, hub=None, end_timestamp=None):
             self.timestamp = datetime_utcnow()
 
         maybe_create_breadcrumbs_from_span(hub, self)
-        add_additional_span_data(hub, self)
 
         return None
 
@@ -1021,7 +1020,6 @@ async def my_async_function():
 from sentry_sdk.tracing_utils import (
     Baggage,
     EnvironHeaders,
-    add_additional_span_data,
     extract_sentrytrace_data,
     has_tracing_enabled,
     maybe_create_breadcrumbs_from_span,

sentry_sdk/tracing_utils.py

@@ -1,4 +1,5 @@
 import contextlib
+import os
 import re
 import sys
 
@@ -11,6 +12,7 @@
     to_string,
     is_sentry_url,
     _is_external_source,
+    _module_in_list,
 )
 from sentry_sdk._compat import PY2, iteritems
 from sentry_sdk._types import TYPE_CHECKING
@@ -190,29 +192,44 @@ def add_query_source(hub, span):
         return
 
     project_root = client.options["project_root"]
+    in_app_include = client.options.get("in_app_include")
+    in_app_exclude = client.options.get("in_app_exclude")
 
     # Find the correct frame
     frame = sys._getframe()  # type: Union[FrameType, None]
     while frame is not None:
         try:
             abs_path = frame.f_code.co_filename
+            if abs_path and PY2:
+                abs_path = os.path.abspath(abs_path)
         except Exception:
             abs_path = ""
 
         try:
-            namespace = frame.f_globals.get("__name__")
+            namespace = frame.f_globals.get("__name__")  # type: Optional[str]
         except Exception:
             namespace = None
 
         is_sentry_sdk_frame = namespace is not None and namespace.startswith(
             "sentry_sdk."
         )
+
+        should_be_included = not _is_external_source(abs_path)
+        if namespace is not None:
+            if in_app_exclude and _module_in_list(namespace, in_app_exclude):
+                should_be_included = False
+            if in_app_include and _module_in_list(namespace, in_app_include):
+                # in_app_include takes precedence over in_app_exclude, so doing it
+                # at the end
+                should_be_included = True
+
         if (
             abs_path.startswith(project_root)
-            and not _is_external_source(abs_path)
+            and should_be_included
             and not is_sentry_sdk_frame
         ):
             break
+
         frame = frame.f_back
     else:
         frame = None
@@ -250,15 +267,6 @@ def add_query_source(hub, span):
             span.set_data(SPANDATA.CODE_FUNCTION, frame.f_code.co_name)
 
 
-def add_additional_span_data(hub, span):
-    # type: (sentry_sdk.Hub, sentry_sdk.tracing.Span) -> None
-    """
-    Adds additional data to the span
-    """
-    if span.op == OP.DB:
-        add_query_source(hub, span)
-
-
 def extract_sentrytrace_data(header):
     # type: (Optional[str]) -> Optional[Dict[str, Union[str, bool, None]]]
     """

tests/integrations/django/test_db_query_data.py

@@ -2,16 +2,16 @@
 
 import pytest
 
+from django import VERSION as DJANGO_VERSION
+from django.db import connections
+
 try:
     from django.urls import reverse
 except ImportError:
     from django.core.urlresolvers import reverse
 
-from django.db import connections
-
 from werkzeug.test import Client
 
-from sentry_sdk._compat import PY2
 from sentry_sdk.consts import SPANDATA
 from sentry_sdk.integrations.django import DjangoIntegration
 
@@ -102,24 +102,124 @@ def test_query_source(sentry_init, client, capture_events):
             assert type(data.get(SPANDATA.CODE_LINENO)) == int
             assert data.get(SPANDATA.CODE_LINENO) > 0
 
-            if PY2:
+            assert (
+                data.get(SPANDATA.CODE_NAMESPACE)
+                == "tests.integrations.django.myapp.views"
+            )
+            assert data.get(SPANDATA.CODE_FILEPATH).endswith(
+                "tests/integrations/django/myapp/views.py"
+            )
+            assert data.get(SPANDATA.CODE_FUNCTION) == "postgres_select_orm"
+
+            break
+    else:
+        raise AssertionError("No db span found")
+
+
+@pytest.mark.forked
+@pytest_mark_django_db_decorator(transaction=True)
+def test_query_source_with_in_app_exclude(sentry_init, client, capture_events):
+    sentry_init(
+        integrations=[DjangoIntegration()],
+        send_default_pii=True,
+        traces_sample_rate=1.0,
+        enable_db_query_source=True,
+        db_query_source_threshold_ms=0,
+        in_app_exclude=["tests.integrations.django.myapp.views"],
+    )
+
+    if "postgres" not in connections:
+        pytest.skip("postgres tests disabled")
+
+    # trigger Django to open a new connection by marking the existing one as None.
+    connections["postgres"].connection = None
+
+    events = capture_events()
+
+    _, status, _ = unpack_werkzeug_response(client.get(reverse("postgres_select_orm")))
+    assert status == "200 OK"
+
+    (event,) = events
+    for span in event["spans"]:
+        if span.get("op") == "db" and "auth_user" in span.get("description"):
+            data = span.get("data", {})
+
+            assert SPANDATA.CODE_LINENO in data
+            assert SPANDATA.CODE_NAMESPACE in data
+            assert SPANDATA.CODE_FILEPATH in data
+            assert SPANDATA.CODE_FUNCTION in data
+
+            assert type(data.get(SPANDATA.CODE_LINENO)) == int
+            assert data.get(SPANDATA.CODE_LINENO) > 0
+
+            if DJANGO_VERSION >= (1, 11):
                 assert (
                     data.get(SPANDATA.CODE_NAMESPACE)
-                    == "tests.integrations.django.test_db_query_data"
+                    == "tests.integrations.django.myapp.settings"
                 )
                 assert data.get(SPANDATA.CODE_FILEPATH).endswith(
-                    "tests/integrations/django/test_db_query_data.py"
+                    "tests/integrations/django/myapp/settings.py"
                 )
-                assert data.get(SPANDATA.CODE_FUNCTION) == "test_query_source"
+                assert data.get(SPANDATA.CODE_FUNCTION) == "middleware"
             else:
                 assert (
                     data.get(SPANDATA.CODE_NAMESPACE)
-                    == "tests.integrations.django.myapp.views"
+                    == "tests.integrations.django.test_db_query_data"
                 )
                 assert data.get(SPANDATA.CODE_FILEPATH).endswith(
-                    "tests/integrations/django/myapp/views.py"
+                    "tests/integrations/django/test_db_query_data.py"
                 )
-                assert data.get(SPANDATA.CODE_FUNCTION) == "postgres_select_orm"
+                assert (
+                    data.get(SPANDATA.CODE_FUNCTION)
+                    == "test_query_source_with_in_app_exclude"
+                )
+
+            break
+    else:
+        raise AssertionError("No db span found")
+
+
+@pytest.mark.forked
+@pytest_mark_django_db_decorator(transaction=True)
+def test_query_source_with_in_app_include(sentry_init, client, capture_events):
+    sentry_init(
+        integrations=[DjangoIntegration()],
+        send_default_pii=True,
+        traces_sample_rate=1.0,
+        enable_db_query_source=True,
+        db_query_source_threshold_ms=0,
+        in_app_include=["django"],
+    )
+
+    if "postgres" not in connections:
+        pytest.skip("postgres tests disabled")
+
+    # trigger Django to open a new connection by marking the existing one as None.
+    connections["postgres"].connection = None
+
+    events = capture_events()
+
+    _, status, _ = unpack_werkzeug_response(client.get(reverse("postgres_select_orm")))
+    assert status == "200 OK"
+
+    (event,) = events
+    for span in event["spans"]:
+        if span.get("op") == "db" and "auth_user" in span.get("description"):
+            data = span.get("data", {})
+
+            assert SPANDATA.CODE_LINENO in data
+            assert SPANDATA.CODE_NAMESPACE in data
+            assert SPANDATA.CODE_FILEPATH in data
+            assert SPANDATA.CODE_FUNCTION in data
+
+            assert type(data.get(SPANDATA.CODE_LINENO)) == int
+            assert data.get(SPANDATA.CODE_LINENO) > 0
+
+            assert data.get(SPANDATA.CODE_NAMESPACE) == "django.db.models.sql.compiler"
+            assert data.get(SPANDATA.CODE_FILEPATH).endswith(
+                "django/db/models/sql/compiler.py"
+            )
+            assert data.get(SPANDATA.CODE_FUNCTION) == "execute_sql"
             break
     else:
         raise AssertionError("No db span found")