Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change code.filepath frame picking logic #2568

Merged
merged 26 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
77b3a14
Move `add_query_source` to `record_sql_queries`
sentrivana Dec 5, 2023
ea74fd9
wip
sentrivana Dec 5, 2023
4b75585
always look for a frame
sentrivana Dec 5, 2023
850b8d0
wip
sentrivana Dec 5, 2023
bb63c6e
Merge branch 'master' into ivana/change-frame-picking-logic
sentrivana Dec 5, 2023
46c859e
fix asyncpg
sentrivana Dec 5, 2023
a2cb3d8
Merge branch 'master' into ivana/change-frame-picking-logic
antonpirker Dec 6, 2023
5401dec
Merge branch 'master' into ivana/change-frame-picking-logic
sentrivana Dec 6, 2023
d522d2f
tweak frame finding logic
sentrivana Dec 7, 2023
fdce522
leave is_external_source be
sentrivana Dec 7, 2023
cc006d4
Merge branch 'master' into ivana/change-frame-picking-logic
antonpirker Dec 7, 2023
6ce6190
Merge branch 'master' into ivana/change-frame-picking-logic
sentrivana Dec 11, 2023
7bead91
Respect in_app_include, exclude
sentrivana Dec 11, 2023
e901f60
typo
sentrivana Dec 11, 2023
9ef3cbb
caution
sentrivana Dec 11, 2023
9feb06b
Merge branch 'master' into ivana/change-frame-picking-logic
sentrivana Dec 11, 2023
aefe488
more guards
sentrivana Dec 11, 2023
e8e0af1
maybe fix 2.7
sentrivana Dec 11, 2023
785e2f9
mypy
sentrivana Dec 11, 2023
1623d4f
compat
sentrivana Dec 11, 2023
a405aa9
small tweak
sentrivana Dec 11, 2023
f38607b
leftovers
sentrivana Dec 11, 2023
2e3c05a
better exclude test that actually does something
sentrivana Dec 11, 2023
10472be
trying something
sentrivana Dec 11, 2023
48d96df
another attempt
sentrivana Dec 11, 2023
c4dfd11
add some capture_internal_exceptions
sentrivana Dec 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@
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 @@
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)

Check warning on line 659 in sentry_sdk/integrations/django/__init__.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/integrations/django/__init__.py#L659

Added line #L659 was not covered by tests

with capture_internal_exceptions():
add_query_source(hub, span)

Check warning on line 662 in sentry_sdk/integrations/django/__init__.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/integrations/django/__init__.py#L662

Added line #L662 was not covered by tests

return result

Check warning on line 664 in sentry_sdk/integrations/django/__init__.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/integrations/django/__init__.py#L664

Added line #L664 was not covered by tests

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 _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

Check warning on line 88 in sentry_sdk/integrations/sqlalchemy.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/integrations/sqlalchemy.py#L88

Added line #L88 was not covered by tests

ctx_mgr = getattr(
context, "_sentry_sql_span_manager", None
) # type: Optional[ContextManager[Any]]
Expand All @@ -92,6 +95,11 @@
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")