Skip to content

Commit

Permalink
fix(query-source): Fix query source relative filepath (#2717)
Browse files Browse the repository at this point in the history
When generating the filename attribute for stack trace frames, the SDK uses the `filename_for_module` function. When generating the `code.filepath` attribute for query spans, the SDK does not use that function. Because of this inconsistency, code mappings that work with stack frames sometimes don't work with queries that come from the same files.

This change makes sure that query sources use `filename_for_module`, so the paths are consistent.
  • Loading branch information
gggritso authored Feb 21, 2024
1 parent e07c0ac commit 2eeb8c5
Show file tree
Hide file tree
Showing 15 changed files with 223 additions and 1 deletion.
5 changes: 4 additions & 1 deletion sentry_sdk/tracing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from sentry_sdk.consts import OP, SPANDATA
from sentry_sdk.utils import (
capture_internal_exceptions,
filename_for_module,
Dsn,
match_regex_list,
to_string,
Expand Down Expand Up @@ -255,7 +256,9 @@ def add_query_source(hub, span):
except Exception:
filepath = None
if filepath is not None:
if project_root is not None and filepath.startswith(project_root):
if namespace is not None and not PY2:
in_app_path = filename_for_module(namespace, filepath)
elif project_root is not None and filepath.startswith(project_root):
in_app_path = filepath.replace(project_root, "").lstrip(os.sep)
else:
in_app_path = filepath
Expand Down
6 changes: 6 additions & 0 deletions tests/integrations/asyncpg/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import os
import sys
import pytest

pytest.importorskip("asyncpg")
pytest.importorskip("pytest_asyncio")

# Load `asyncpg_helpers` into the module search path to test query source path names relative to module. See
# `test_query_source_with_module_in_search_path`
sys.path.insert(0, os.path.join(os.path.dirname(__file__)))
Empty file.
2 changes: 2 additions & 0 deletions tests/integrations/asyncpg/asyncpg_helpers/helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
async def execute_query_in_connection(query, connection):
await connection.execute(query)
51 changes: 51 additions & 0 deletions tests/integrations/asyncpg/test_asyncpg.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
PG_PORT = 5432


from sentry_sdk._compat import PY2
import datetime

import asyncpg
Expand Down Expand Up @@ -592,6 +593,56 @@ async def test_query_source(sentry_init, capture_events):
assert data.get(SPANDATA.CODE_FUNCTION) == "test_query_source"


@pytest.mark.asyncio
async def test_query_source_with_module_in_search_path(sentry_init, capture_events):
"""
Test that query source is relative to the path of the module it ran in
"""
sentry_init(
integrations=[AsyncPGIntegration()],
enable_tracing=True,
enable_db_query_source=True,
db_query_source_threshold_ms=0,
)

events = capture_events()

from asyncpg_helpers.helpers import execute_query_in_connection

with start_transaction(name="test_transaction", sampled=True):
conn: Connection = await connect(PG_CONNECTION_URI)

await execute_query_in_connection(
"INSERT INTO users(name, password, dob) VALUES ('Alice', 'secret', '1990-12-25')",
conn,
)

await conn.close()

(event,) = events

span = event["spans"][-1]
assert span["description"].startswith("INSERT INTO")

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 not PY2:
assert data.get(SPANDATA.CODE_NAMESPACE) == "asyncpg_helpers.helpers"
assert data.get(SPANDATA.CODE_FILEPATH) == "asyncpg_helpers/helpers.py"

is_relative_path = data.get(SPANDATA.CODE_FILEPATH)[0] != os.sep
assert is_relative_path

assert data.get(SPANDATA.CODE_FUNCTION) == "execute_query_in_connection"


@pytest.mark.asyncio
async def test_no_query_source_if_duration_too_short(sentry_init, capture_events):
sentry_init(
Expand Down
6 changes: 6 additions & 0 deletions tests/integrations/django/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
import os
import sys
import pytest

pytest.importorskip("django")

# Load `django_helpers` into the module search path to test query source path names relative to module. See
# `test_query_source_with_module_in_search_path`
sys.path.insert(0, os.path.join(os.path.dirname(__file__)))
Empty file.
9 changes: 9 additions & 0 deletions tests/integrations/django/django_helpers/views.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from django.contrib.auth.models import User
from django.http import HttpResponse
from django.views.decorators.csrf import csrf_exempt


@csrf_exempt
def postgres_select_orm(request, *args, **kwargs):
user = User.objects.using("postgres").all().first()
return HttpResponse("ok {}".format(user))
6 changes: 6 additions & 0 deletions tests/integrations/django/myapp/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def path(path, *args, **kwargs):


from . import views
from django_helpers import views as helper_views

urlpatterns = [
path("view-exc", views.view_exc, name="view_exc"),
Expand Down Expand Up @@ -59,6 +60,11 @@ def path(path, *args, **kwargs):
path("template-test3", views.template_test3, name="template_test3"),
path("postgres-select", views.postgres_select, name="postgres_select"),
path("postgres-select-slow", views.postgres_select_orm, name="postgres_select_orm"),
path(
"postgres-select-slow-from-supplement",
helper_views.postgres_select_orm,
name="postgres_select_slow_from_supplement",
),
path(
"permission-denied-exc",
views.permission_denied_exc,
Expand Down
57 changes: 57 additions & 0 deletions tests/integrations/django/test_db_query_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import pytest
from datetime import datetime

from sentry_sdk._compat import PY2
from django import VERSION as DJANGO_VERSION
from django.db import connections

Expand Down Expand Up @@ -168,6 +169,62 @@ def test_query_source(sentry_init, client, capture_events):
raise AssertionError("No db span found")


@pytest.mark.forked
@pytest_mark_django_db_decorator(transaction=True)
def test_query_source_with_module_in_search_path(sentry_init, client, capture_events):
"""
Test that query source is relative to the path of the module it ran in
"""
client = Client(application)

sentry_init(
integrations=[DjangoIntegration()],
send_default_pii=True,
traces_sample_rate=1.0,
enable_db_query_source=True,
db_query_source_threshold_ms=0,
)

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_slow_from_supplement"))
)
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 not PY2:
assert data.get(SPANDATA.CODE_NAMESPACE) == "django_helpers.views"
assert data.get(SPANDATA.CODE_FILEPATH) == "django_helpers/views.py"

is_relative_path = data.get(SPANDATA.CODE_FILEPATH)[0] != os.sep
assert is_relative_path

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):
Expand Down
6 changes: 6 additions & 0 deletions tests/integrations/sqlalchemy/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
import os
import sys
import pytest

pytest.importorskip("sqlalchemy")

# Load `sqlalchemy_helpers` into the module search path to test query source path names relative to module. See
# `test_query_source_with_module_in_search_path`
sys.path.insert(0, os.path.join(os.path.dirname(__file__)))
Empty file.
7 changes: 7 additions & 0 deletions tests/integrations/sqlalchemy/sqlalchemy_helpers/helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
def add_model_to_session(model, session):
session.add(model)
session.commit()


def query_first_model_from_session(model_klass, session):
return session.query(model_klass).first()
68 changes: 68 additions & 0 deletions tests/integrations/sqlalchemy/test_sqlalchemy.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import sys
from datetime import datetime

from sentry_sdk._compat import PY2
from sqlalchemy import Column, ForeignKey, Integer, String, create_engine
from sqlalchemy.exc import IntegrityError
from sqlalchemy.ext.declarative import declarative_base
Expand Down Expand Up @@ -449,6 +450,73 @@ class Person(Base):
raise AssertionError("No db span found")


def test_query_source_with_module_in_search_path(sentry_init, capture_events):
"""
Test that query source is relative to the path of the module it ran in
"""
sentry_init(
integrations=[SqlalchemyIntegration()],
enable_tracing=True,
enable_db_query_source=True,
db_query_source_threshold_ms=0,
)
events = capture_events()

from sqlalchemy_helpers.helpers import (
add_model_to_session,
query_first_model_from_session,
)

with start_transaction(name="test_transaction", sampled=True):
Base = declarative_base() # noqa: N806

class Person(Base):
__tablename__ = "person"
id = Column(Integer, primary_key=True)
name = Column(String(250), nullable=False)

engine = create_engine("sqlite:///:memory:")
Base.metadata.create_all(engine)

Session = sessionmaker(bind=engine) # noqa: N806
session = Session()

bob = Person(name="Bob")

add_model_to_session(bob, session)

assert query_first_model_from_session(Person, session) == bob

(event,) = events

for span in event["spans"]:
if span.get("op") == "db" and span.get("description").startswith(
"SELECT person"
):
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 not PY2:
assert data.get(SPANDATA.CODE_NAMESPACE) == "sqlalchemy_helpers.helpers"
assert (
data.get(SPANDATA.CODE_FILEPATH) == "sqlalchemy_helpers/helpers.py"
)

is_relative_path = data.get(SPANDATA.CODE_FILEPATH)[0] != os.sep
assert is_relative_path

assert data.get(SPANDATA.CODE_FUNCTION) == "query_first_model_from_session"
break
else:
raise AssertionError("No db span found")


def test_no_query_source_if_duration_too_short(sentry_init, capture_events):
sentry_init(
integrations=[SqlalchemyIntegration()],
Expand Down
1 change: 1 addition & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,7 @@ deps =

setenv =
PYTHONDONTWRITEBYTECODE=1
OBJC_DISABLE_INITIALIZE_FORK_SAFETY=YES
common: TESTPATH=tests
gevent: TESTPATH=tests
aiohttp: TESTPATH=tests/integrations/aiohttp
Expand Down

0 comments on commit 2eeb8c5

Please sign in to comment.