Skip to content

Commit

Permalink
Change code.filepath frame picking logic (#2568)
Browse files Browse the repository at this point in the history
- 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)
  • Loading branch information
sentrivana committed Dec 12, 2023
1 parent ddf37a3 commit 7df152b
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 31 deletions.
11 changes: 9 additions & 2 deletions sentry_sdk/integrations/asyncpg.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
17 changes: 14 additions & 3 deletions sentry_sdk/integrations/django/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
14 changes: 11 additions & 3 deletions sentry_sdk/integrations/sqlalchemy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]]
Expand All @@ -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
Expand Down
2 changes: 0 additions & 2 deletions sentry_sdk/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand Down
30 changes: 19 additions & 11 deletions sentry_sdk/tracing_utils.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import contextlib
import os
import re
import sys

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]]]
"""
Expand Down
120 changes: 110 additions & 10 deletions tests/integrations/django/test_db_query_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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")

0 comments on commit 7df152b

Please sign in to comment.