diff --git a/sentry_sdk/api.py b/sentry_sdk/api.py index 3dd6f9c737..41c4814146 100644 --- a/sentry_sdk/api.py +++ b/sentry_sdk/api.py @@ -322,7 +322,8 @@ def start_transaction( :param transaction: The transaction to start. If omitted, we create and start a new transaction. - :param instrumenter: This parameter is meant for internal use only. + :param instrumenter: This parameter is meant for internal use only. It + will be removed in the next major version. :param custom_sampling_context: The transaction's custom sampling context. :param kwargs: Optional keyword arguments to be passed to the Transaction constructor. See :py:class:`sentry_sdk.tracing.Transaction` for diff --git a/sentry_sdk/scope.py b/sentry_sdk/scope.py index e6ad86254f..1febbd0ef2 100644 --- a/sentry_sdk/scope.py +++ b/sentry_sdk/scope.py @@ -987,7 +987,8 @@ def start_transaction( :param transaction: The transaction to start. If omitted, we create and start a new transaction. - :param instrumenter: This parameter is meant for internal use only. + :param instrumenter: This parameter is meant for internal use only. It + will be removed in the next major version. :param custom_sampling_context: The transaction's custom sampling context. :param kwargs: Optional keyword arguments to be passed to the Transaction constructor. See :py:class:`sentry_sdk.tracing.Transaction` for @@ -1054,6 +1055,10 @@ def start_span(self, instrumenter=INSTRUMENTER.SENTRY, **kwargs): one is not already in progress. For supported `**kwargs` see :py:class:`sentry_sdk.tracing.Span`. + + The instrumenter parameter is deprecated for user code, and it will + be removed in the next major version. Going forward, it should only + be used by the SDK itself. """ with new_scope(): kwargs.setdefault("scope", self) @@ -1298,6 +1303,7 @@ def _apply_breadcrumbs_to_event(self, event, hint, options): event.setdefault("breadcrumbs", {}).setdefault("values", []).extend( self._breadcrumbs ) + event["breadcrumbs"]["values"].sort(key=lambda crumb: crumb["timestamp"]) def _apply_user_to_event(self, event, hint, options): # type: (Event, Hint, Optional[Dict[str, Any]]) -> None diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index f1f3200035..92d9e7ca49 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -394,6 +394,10 @@ def start_child(self, instrumenter=INSTRUMENTER.SENTRY, **kwargs): Takes the same arguments as the initializer of :py:class:`Span`. The trace id, sampling decision, transaction pointer, and span recorder are inherited from the current span/transaction. + + The instrumenter parameter is deprecated for user code, and it will + be removed in the next major version. Going forward, it should only + be used by the SDK itself. """ configuration_instrumenter = sentry_sdk.Scope.get_client().options[ "instrumenter" diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index ba20dc8436..1c7da7cc7a 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -22,6 +22,7 @@ is_sentry_url, _is_external_source, _module_in_list, + _is_in_project_root, ) from sentry_sdk._types import TYPE_CHECKING @@ -170,6 +171,14 @@ def maybe_create_breadcrumbs_from_span(scope, span): ) +def _get_frame_module_abs_path(frame): + # type: (FrameType) -> str + try: + return frame.f_code.co_filename + except Exception: + return "" + + def add_query_source(span): # type: (sentry_sdk.tracing.Span) -> None """ @@ -200,10 +209,7 @@ def add_query_source(span): # Find the correct frame frame = sys._getframe() # type: Union[FrameType, None] while frame is not None: - try: - abs_path = frame.f_code.co_filename - except Exception: - abs_path = "" + abs_path = _get_frame_module_abs_path(frame) try: namespace = frame.f_globals.get("__name__") # type: Optional[str] @@ -213,20 +219,14 @@ def add_query_source(span): is_sentry_sdk_frame = namespace is not None and namespace.startswith( "sentry_sdk." ) + should_be_included = _module_in_list(namespace, in_app_include) + should_be_excluded = _is_external_source(abs_path) or _module_in_list( + namespace, in_app_exclude + ) - 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 should_be_included - and not is_sentry_sdk_frame + if not is_sentry_sdk_frame and ( + should_be_included + or (_is_in_project_root(abs_path, project_root) and not should_be_excluded) ): break @@ -250,10 +250,7 @@ def add_query_source(span): if namespace is not None: span.set_data(SPANDATA.CODE_NAMESPACE, namespace) - try: - filepath = frame.f_code.co_filename - except Exception: - filepath = None + filepath = _get_frame_module_abs_path(frame) if filepath is not None: if namespace is not None: in_app_path = filename_for_module(namespace, filepath) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 8a805d3d64..b9749c503f 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -1043,7 +1043,7 @@ def event_from_exception( def _module_in_list(name, items): - # type: (str, Optional[List[str]]) -> bool + # type: (Optional[str], Optional[List[str]]) -> bool if name is None: return False diff --git a/tests/integrations/django/test_db_query_data.py b/tests/integrations/django/test_db_query_data.py index 087fc5ad49..4e5d22fbda 100644 --- a/tests/integrations/django/test_db_query_data.py +++ b/tests/integrations/django/test_db_query_data.py @@ -1,7 +1,9 @@ +import contextlib import os import pytest from datetime import datetime +from pathlib import Path from unittest import mock from django import VERSION as DJANGO_VERSION @@ -15,14 +17,19 @@ from werkzeug.test import Client from sentry_sdk import start_transaction +from sentry_sdk._types import TYPE_CHECKING from sentry_sdk.consts import SPANDATA from sentry_sdk.integrations.django import DjangoIntegration -from sentry_sdk.tracing_utils import record_sql_queries +from sentry_sdk.tracing_utils import _get_frame_module_abs_path, record_sql_queries +from sentry_sdk.utils import _module_in_list from tests.conftest import unpack_werkzeug_response from tests.integrations.django.utils import pytest_mark_django_db_decorator from tests.integrations.django.myapp.wsgi import application +if TYPE_CHECKING: + from types import FrameType + @pytest.fixture def client(): @@ -283,7 +290,10 @@ def test_query_source_with_in_app_exclude(sentry_init, client, capture_events): @pytest.mark.forked @pytest_mark_django_db_decorator(transaction=True) -def test_query_source_with_in_app_include(sentry_init, client, capture_events): +@pytest.mark.parametrize("django_outside_of_project_root", [False, True]) +def test_query_source_with_in_app_include( + sentry_init, client, capture_events, django_outside_of_project_root +): sentry_init( integrations=[DjangoIntegration()], send_default_pii=True, @@ -301,8 +311,33 @@ def test_query_source_with_in_app_include(sentry_init, client, capture_events): events = capture_events() - _, status, _ = unpack_werkzeug_response(client.get(reverse("postgres_select_orm"))) - assert status == "200 OK" + # Simulate Django installation outside of the project root + original_get_frame_module_abs_path = _get_frame_module_abs_path + + def patched_get_frame_module_abs_path_function(frame): + # type: (FrameType) -> str + result = original_get_frame_module_abs_path(frame) + namespace = frame.f_globals.get("__name__") + if _module_in_list(namespace, ["django"]): + result = str( + Path("/outside-of-project-root") / frame.f_code.co_filename[1:] + ) + return result + + patched_get_frame_module_abs_path = ( + mock.patch( + "sentry_sdk.tracing_utils._get_frame_module_abs_path", + patched_get_frame_module_abs_path_function, + ) + if django_outside_of_project_root + else contextlib.suppress() + ) + + with patched_get_frame_module_abs_path: + _, status, _ = unpack_werkzeug_response( + client.get(reverse("postgres_select_orm")) + ) + assert status == "200 OK" (event,) = events for span in event["spans"]: diff --git a/tests/test_basics.py b/tests/test_basics.py index 439215e013..52eb5045d8 100644 --- a/tests/test_basics.py +++ b/tests/test_basics.py @@ -1,3 +1,4 @@ +import datetime import logging import os import sys @@ -391,6 +392,37 @@ def test_breadcrumbs(sentry_init, capture_events): assert len(event["breadcrumbs"]["values"]) == 0 +def test_breadcrumb_ordering(sentry_init, capture_events): + sentry_init() + events = capture_events() + + timestamps = [ + datetime.datetime.now() - datetime.timedelta(days=10), + datetime.datetime.now() - datetime.timedelta(days=8), + datetime.datetime.now() - datetime.timedelta(days=12), + ] + + for timestamp in timestamps: + add_breadcrumb( + message="Authenticated at %s" % timestamp, + category="auth", + level="info", + timestamp=timestamp, + ) + + capture_exception(ValueError()) + (event,) = events + + assert len(event["breadcrumbs"]["values"]) == len(timestamps) + timestamps_from_event = [ + datetime.datetime.strptime( + x["timestamp"].replace("Z", ""), "%Y-%m-%dT%H:%M:%S.%f" + ) + for x in event["breadcrumbs"]["values"] + ] + assert timestamps_from_event == sorted(timestamps) + + def test_attachments(sentry_init, capture_envelopes): sentry_init() envelopes = capture_envelopes() diff --git a/tests/test_utils.py b/tests/test_utils.py index c4064729f8..40a3296564 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -26,6 +26,7 @@ serialize_frame, is_sentry_url, _get_installed_modules, + _generate_installed_modules, ensure_integration_enabled, ensure_integration_enabled_async, ) @@ -523,7 +524,7 @@ def test_installed_modules(): installed_distributions = { _normalize_distribution_name(dist): version - for dist, version in _get_installed_modules().items() + for dist, version in _generate_installed_modules() } if importlib_available: