From bcc1c72687a93899ca0da5c77793c08a333fdd28 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 17 Oct 2023 10:58:21 +0200 Subject: [PATCH 01/14] Django resolver experiments --- sentry_sdk/integrations/django/__init__.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 03d0545b1d..0a80741fc4 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -362,12 +362,13 @@ def _set_transaction_name_and_source(scope, transaction_style, request): transaction_name = transaction_from_function(getattr(fn, "view_class", fn)) elif transaction_style == "url": - if hasattr(request, "urlconf"): - transaction_name = LEGACY_RESOLVER.resolve( - request.path_info, urlconf=request.urlconf - ) - else: - transaction_name = LEGACY_RESOLVER.resolve(request.path_info) + transaction_name = request.resolver_match.route + # if hasattr(request, "urlconf"): + # transaction_name = LEGACY_RESOLVER.resolve( + # request.path_info, urlconf=request.urlconf + # ) + # else: + # transaction_name = LEGACY_RESOLVER.resolve(request.path_info) if transaction_name is None: transaction_name = request.path_info From 0da3599da73533fbb59f7199d21532456abfc67e Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Mon, 23 Oct 2023 16:21:43 +0200 Subject: [PATCH 02/14] wip --- .../integrations/django/transactions.py | 30 +++++++--- .../integrations/django/test_transactions.py | 56 ++++++++++--------- 2 files changed, 54 insertions(+), 32 deletions(-) diff --git a/sentry_sdk/integrations/django/transactions.py b/sentry_sdk/integrations/django/transactions.py index 91349c4bf9..2980c792ba 100644 --- a/sentry_sdk/integrations/django/transactions.py +++ b/sentry_sdk/integrations/django/transactions.py @@ -1,6 +1,8 @@ """ -Copied from raven-python. Used for -`DjangoIntegration(transaction_fron="raven_legacy")`. +Copied from raven-python. + +Despite being called "legacy" in some places this resolver is very much still +in use. """ from __future__ import absolute_import @@ -10,17 +12,20 @@ from sentry_sdk._types import TYPE_CHECKING if TYPE_CHECKING: - from django.urls.resolvers import URLResolver from typing import Dict from typing import List from typing import Optional - from django.urls.resolvers import URLPattern from typing import Tuple from typing import Union from re import Pattern + from django.urls.resolvers import URLPattern, URLResolver + from django.http.request import HttpRequest + +from django import VERSION as DJANGO_VERSION try: from django.urls import get_resolver + from django.urls.resolvers import RoutePattern # XXX except ImportError: from django.core.urlresolvers import get_resolver @@ -36,6 +41,9 @@ def get_regex(resolver_or_pattern): class RavenResolver(object): + _new_style_group_matcher = re.compile( + r"<(?:(?P[^>:]+):)?(?P[^>]+)>" + ) # https://github.com/django/django/blob/21382e2743d06efbf5623e7c9b6dccf2a325669b/django/urls/resolvers.py#L245-L247 _optional_group_matcher = re.compile(r"\(\?\:([^\)]+)\)") _named_group_matcher = re.compile(r"\(\?P<(\w+)>[^\)]+\)+") _non_named_group_matcher = re.compile(r"\([^\)]+\)") @@ -59,8 +67,16 @@ def _simplify(self, pattern): # remove optional params # TODO(dcramer): it'd be nice to change these into [%s] but it currently # conflicts with the other rules because we're doing regexp matches + + if DJANGO_VERSION >= (2, 0) and isinstance(pattern.pattern, RoutePattern): + result = self._new_style_group_matcher.sub( + lambda m: "{%s}" % m.group(1), pattern.pattern._route + ) + + result = get_regex(pattern).pattern + # rather than parsing tokens - result = self._optional_group_matcher.sub(lambda m: "%s" % m.group(1), pattern) + result = self._optional_group_matcher.sub(lambda m: "%s" % m.group(1), result) # handle named groups first result = self._named_group_matcher.sub(lambda m: "{%s}" % m.group(1), result) @@ -113,8 +129,8 @@ def _resolve(self, resolver, path, parents=None): except KeyError: pass - prefix = "".join(self._simplify(get_regex(p).pattern) for p in parents) - result = prefix + self._simplify(get_regex(pattern).pattern) + prefix = "".join(self._simplify(p) for p in parents) + result = prefix + self._simplify(pattern) if not result.startswith("/"): result = "/" + result self._cache[pattern] = result diff --git a/tests/integrations/django/test_transactions.py b/tests/integrations/django/test_transactions.py index 4c94a2c955..7f49ba7c8b 100644 --- a/tests/integrations/django/test_transactions.py +++ b/tests/integrations/django/test_transactions.py @@ -2,48 +2,58 @@ import pytest import django +from werkzeug.test import Client +# django<2.0 had only `url` with regex based patterns. +# django>=2.0 renamed `url` to `re_path`, and additionally introduced `path` +# for new style URL patterns, e.g. . if django.VERSION >= (2, 0): - # TODO: once we stop supporting django < 2, use the real name of this - # function (re_path) - from django.urls import re_path as url + from django.urls import path, re_path from django.conf.urls import include + from django.urls.converters import PathConverter else: - from django.conf.urls import url, include - -if django.VERSION < (1, 9): - included_url_conf = (url(r"^foo/bar/(?P[\w]+)", lambda x: ""),), "", "" -else: - included_url_conf = ((url(r"^foo/bar/(?P[\w]+)", lambda x: ""),), "") + from django.conf.urls import url as re_path, include +from sentry_sdk.integrations.django import DjangoIntegration from sentry_sdk.integrations.django.transactions import RavenResolver +from tests.integrations.django.myapp.wsgi import application + + +@pytest.fixture +def client(): + return Client(application) +if django.VERSION < (1, 9): + included_url_conf = (re_path(r"^foo/bar/(?P[\w]+)", lambda x: ""),), "", "" +else: + included_url_conf = ((re_path(r"^foo/bar/(?P[\w]+)", lambda x: ""),), "") + example_url_conf = ( - url(r"^api/(?P[\w_-]+)/store/$", lambda x: ""), - url(r"^api/(?P(v1|v2))/author/$", lambda x: ""), - url( + re_path(r"^api/(?P[\w_-]+)/store/$", lambda x: ""), + re_path(r"^api/(?P(v1|v2))/author/$", lambda x: ""), + re_path( r"^api/(?P[^\/]+)/product/(?P(?:\d+|[A-Fa-f0-9-]{32,36}))/$", lambda x: "", ), - url(r"^report/", lambda x: ""), - url(r"^example/", include(included_url_conf)), + re_path(r"^report/", lambda x: ""), + re_path(r"^example/", include(included_url_conf)), ) -def test_legacy_resolver_no_match(): +def test_resolver_no_match(): resolver = RavenResolver() result = resolver.resolve("/foo/bar", example_url_conf) assert result is None -def test_legacy_resolver_complex_match(): +def test_resolver_complex_match(): resolver = RavenResolver() result = resolver.resolve("/api/1234/store/", example_url_conf) assert result == "/api/{project_id}/store/" -def test_legacy_resolver_complex_either_match(): +def test_resolver_complex_either_match(): resolver = RavenResolver() result = resolver.resolve("/api/v1/author/", example_url_conf) assert result == "/api/{version}/author/" @@ -51,13 +61,13 @@ def test_legacy_resolver_complex_either_match(): assert result == "/api/{version}/author/" -def test_legacy_resolver_included_match(): +def test_resolver_included_match(): resolver = RavenResolver() result = resolver.resolve("/example/foo/bar/baz", example_url_conf) assert result == "/example/foo/bar/{param}" -def test_capture_multiple_named_groups(): +def test_resolver_multiple_groups(): resolver = RavenResolver() result = resolver.resolve( "/api/myproject/product/cb4ef1caf3554c34ae134f3c1b3d605f/", example_url_conf @@ -66,9 +76,7 @@ def test_capture_multiple_named_groups(): @pytest.mark.skipif(django.VERSION < (2, 0), reason="Requires Django > 2.0") -def test_legacy_resolver_newstyle_django20_urlconf(): - from django.urls import path - +def test_resolver_new_style_group(): url_conf = (path("api/v2//store/", lambda x: ""),) resolver = RavenResolver() result = resolver.resolve("/api/v2/1234/store/", url_conf) @@ -76,9 +84,7 @@ def test_legacy_resolver_newstyle_django20_urlconf(): @pytest.mark.skipif(django.VERSION < (2, 0), reason="Requires Django > 2.0") -def test_legacy_resolver_newstyle_django20_urlconf_multiple_groups(): - from django.urls import path - +def test_resolver_new_style_multiple_groups(): url_conf = (path("api/v2//product/", lambda x: ""),) resolver = RavenResolver() result = resolver.resolve("/api/v2/1234/product/5689", url_conf) From f47ff417260ce4808e07c10b6e348a4f48174dc8 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Mon, 23 Oct 2023 16:23:10 +0200 Subject: [PATCH 03/14] wip --- sentry_sdk/integrations/django/__init__.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 0a80741fc4..03d0545b1d 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -362,13 +362,12 @@ def _set_transaction_name_and_source(scope, transaction_style, request): transaction_name = transaction_from_function(getattr(fn, "view_class", fn)) elif transaction_style == "url": - transaction_name = request.resolver_match.route - # if hasattr(request, "urlconf"): - # transaction_name = LEGACY_RESOLVER.resolve( - # request.path_info, urlconf=request.urlconf - # ) - # else: - # transaction_name = LEGACY_RESOLVER.resolve(request.path_info) + if hasattr(request, "urlconf"): + transaction_name = LEGACY_RESOLVER.resolve( + request.path_info, urlconf=request.urlconf + ) + else: + transaction_name = LEGACY_RESOLVER.resolve(request.path_info) if transaction_name is None: transaction_name = request.path_info From c461fd6c875ff8ec286b58d8a404b60447313545 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Mon, 23 Oct 2023 16:49:38 +0200 Subject: [PATCH 04/14] wip --- sentry_sdk/integrations/django/transactions.py | 15 +++++++++------ tests/integrations/django/test_transactions.py | 14 ++++++-------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/sentry_sdk/integrations/django/transactions.py b/sentry_sdk/integrations/django/transactions.py index 2980c792ba..39ecc5d029 100644 --- a/sentry_sdk/integrations/django/transactions.py +++ b/sentry_sdk/integrations/django/transactions.py @@ -19,16 +19,19 @@ from typing import Union from re import Pattern from django.urls.resolvers import URLPattern, URLResolver - from django.http.request import HttpRequest from django import VERSION as DJANGO_VERSION try: from django.urls import get_resolver - from django.urls.resolvers import RoutePattern # XXX except ImportError: from django.core.urlresolvers import get_resolver +try: + from django.urls.resolvers import RoutePattern +except ImportError: + RoutePattern = type(None) + def get_regex(resolver_or_pattern): # type: (Union[URLPattern, URLResolver]) -> Pattern[str] @@ -42,7 +45,7 @@ def get_regex(resolver_or_pattern): class RavenResolver(object): _new_style_group_matcher = re.compile( - r"<(?:(?P[^>:]+):)?(?P[^>]+)>" + r"<(?:([^>:]+):)?([^>]+)>" ) # https://github.com/django/django/blob/21382e2743d06efbf5623e7c9b6dccf2a325669b/django/urls/resolvers.py#L245-L247 _optional_group_matcher = re.compile(r"\(\?\:([^\)]+)\)") _named_group_matcher = re.compile(r"\(\?P<(\w+)>[^\)]+\)+") @@ -70,10 +73,10 @@ def _simplify(self, pattern): if DJANGO_VERSION >= (2, 0) and isinstance(pattern.pattern, RoutePattern): result = self._new_style_group_matcher.sub( - lambda m: "{%s}" % m.group(1), pattern.pattern._route + lambda m: "{%s}" % m.group(2), pattern.pattern._route ) - - result = get_regex(pattern).pattern + else: + result = get_regex(pattern).pattern # rather than parsing tokens result = self._optional_group_matcher.sub(lambda m: "%s" % m.group(1), result) diff --git a/tests/integrations/django/test_transactions.py b/tests/integrations/django/test_transactions.py index 7f49ba7c8b..1dfadd0b09 100644 --- a/tests/integrations/django/test_transactions.py +++ b/tests/integrations/django/test_transactions.py @@ -10,11 +10,9 @@ if django.VERSION >= (2, 0): from django.urls import path, re_path from django.conf.urls import include - from django.urls.converters import PathConverter else: from django.conf.urls import url as re_path, include -from sentry_sdk.integrations.django import DjangoIntegration from sentry_sdk.integrations.django.transactions import RavenResolver from tests.integrations.django.myapp.wsgi import application @@ -47,13 +45,13 @@ def test_resolver_no_match(): assert result is None -def test_resolver_complex_match(): +def test_resolver_re_path_complex_match(): resolver = RavenResolver() result = resolver.resolve("/api/1234/store/", example_url_conf) assert result == "/api/{project_id}/store/" -def test_resolver_complex_either_match(): +def test_resolver_re_path_complex_either_match(): resolver = RavenResolver() result = resolver.resolve("/api/v1/author/", example_url_conf) assert result == "/api/{version}/author/" @@ -61,13 +59,13 @@ def test_resolver_complex_either_match(): assert result == "/api/{version}/author/" -def test_resolver_included_match(): +def test_resolver_re_path_included_match(): resolver = RavenResolver() result = resolver.resolve("/example/foo/bar/baz", example_url_conf) assert result == "/example/foo/bar/{param}" -def test_resolver_multiple_groups(): +def test_resolver_re_path_multiple_groups(): resolver = RavenResolver() result = resolver.resolve( "/api/myproject/product/cb4ef1caf3554c34ae134f3c1b3d605f/", example_url_conf @@ -76,7 +74,7 @@ def test_resolver_multiple_groups(): @pytest.mark.skipif(django.VERSION < (2, 0), reason="Requires Django > 2.0") -def test_resolver_new_style_group(): +def test_resolver_path_group(): url_conf = (path("api/v2//store/", lambda x: ""),) resolver = RavenResolver() result = resolver.resolve("/api/v2/1234/store/", url_conf) @@ -84,7 +82,7 @@ def test_resolver_new_style_group(): @pytest.mark.skipif(django.VERSION < (2, 0), reason="Requires Django > 2.0") -def test_resolver_new_style_multiple_groups(): +def test_resolver_path_multiple_groups(): url_conf = (path("api/v2//product/", lambda x: ""),) resolver = RavenResolver() result = resolver.resolve("/api/v2/1234/product/5689", url_conf) From 1449909e81afcca34b04ae530199b4ffede14675 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 24 Oct 2023 11:01:01 +0200 Subject: [PATCH 05/14] add test --- .../integrations/django/transactions.py | 2 +- .../integrations/django/test_transactions.py | 56 ++++++++++++++----- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/sentry_sdk/integrations/django/transactions.py b/sentry_sdk/integrations/django/transactions.py index 39ecc5d029..5962583bfe 100644 --- a/sentry_sdk/integrations/django/transactions.py +++ b/sentry_sdk/integrations/django/transactions.py @@ -57,7 +57,7 @@ class RavenResolver(object): _cache = {} # type: Dict[URLPattern, str] def _simplify(self, pattern): - # type: (str) -> str + # type: (Union[URLPattern, URLResolver]) -> str r""" Clean up urlpattern regexes into something readable by humans: diff --git a/tests/integrations/django/test_transactions.py b/tests/integrations/django/test_transactions.py index 1dfadd0b09..0d73899377 100644 --- a/tests/integrations/django/test_transactions.py +++ b/tests/integrations/django/test_transactions.py @@ -2,24 +2,23 @@ import pytest import django -from werkzeug.test import Client -# django<2.0 had only `url` with regex based patterns. -# django>=2.0 renamed `url` to `re_path`, and additionally introduced `path` -# for new style URL patterns, e.g. . +try: + from unittest import mock # python 3.3 and above +except ImportError: + import mock # python < 3.3 + if django.VERSION >= (2, 0): from django.urls import path, re_path + from django.urls.converters import PathConverter from django.conf.urls import include else: + # django<2.0 has only `url` with regex based patterns. + # django>=2.0 renames `url` to `re_path`, and additionally introduces `path` + # for new style URL patterns, e.g. . from django.conf.urls import url as re_path, include from sentry_sdk.integrations.django.transactions import RavenResolver -from tests.integrations.django.myapp.wsgi import application - - -@pytest.fixture -def client(): - return Client(application) if django.VERSION < (1, 9): @@ -73,7 +72,10 @@ def test_resolver_re_path_multiple_groups(): assert result == "/api/{project_id}/product/{pid}/" -@pytest.mark.skipif(django.VERSION < (2, 0), reason="Requires Django > 2.0") +@pytest.mark.skipif( + django.VERSION < (2, 0), + reason="Django>=2.0 required for patterns", +) def test_resolver_path_group(): url_conf = (path("api/v2//store/", lambda x: ""),) resolver = RavenResolver() @@ -81,9 +83,35 @@ def test_resolver_path_group(): assert result == "/api/v2/{project_id}/store/" -@pytest.mark.skipif(django.VERSION < (2, 0), reason="Requires Django > 2.0") +@pytest.mark.skipif( + django.VERSION < (2, 0), + reason="Django>=2.0 required for patterns", +) def test_resolver_path_multiple_groups(): - url_conf = (path("api/v2//product/", lambda x: ""),) + url_conf = (path("api/v2//product/", lambda x: ""),) resolver = RavenResolver() - result = resolver.resolve("/api/v2/1234/product/5689", url_conf) + result = resolver.resolve("/api/v2/myproject/product/5689", url_conf) assert result == "/api/v2/{project_id}/product/{pid}" + + +@pytest.mark.skipif( + django.VERSION < (2, 0), + reason="Django>=2.0 required for patterns", +) +def test_resolver_path_complex_path(): + class CustomPathConverter(PathConverter): + regex = r"[^/]+(/[^/]+){0,2}" + + def to_python(self, value): + return value.split("/") + + def to_url(self, value): + return "/".join(value) + + with mock.patch( + "django.urls.resolvers.get_converter", return_value=CustomPathConverter + ): + url_conf = (path("api/v3/", lambda x: ""),) + resolver = RavenResolver() + result = resolver.resolve("/api/v3/abc/def/ghi", url_conf) + assert result == "/api/v3/{custom_path}" From 06cde4a90c0c0d0e8689cee891a4c3d326924ab5 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 24 Oct 2023 12:08:51 +0200 Subject: [PATCH 06/14] add test --- tests/integrations/django/test_transactions.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/integrations/django/test_transactions.py b/tests/integrations/django/test_transactions.py index 0d73899377..a78a644a05 100644 --- a/tests/integrations/django/test_transactions.py +++ b/tests/integrations/django/test_transactions.py @@ -115,3 +115,14 @@ def to_url(self, value): resolver = RavenResolver() result = resolver.resolve("/api/v3/abc/def/ghi", url_conf) assert result == "/api/v3/{custom_path}" + + +@pytest.mark.skipif( + django.VERSION < (2, 0), + reason="Django>=2.0 required for patterns", +) +def test_resolver_path_no_converter(): + url_conf = (path("api/v4/", lambda x: ""),) + resolver = RavenResolver() + result = resolver.resolve("/api/v4/myproject", url_conf) + assert result == "/api/v4/{project_id}" From ea79abbc23dcf265db253604c2199bea3318c85f Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 24 Oct 2023 14:53:18 +0200 Subject: [PATCH 07/14] simplify --- tests/integrations/django/test_transactions.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/tests/integrations/django/test_transactions.py b/tests/integrations/django/test_transactions.py index a78a644a05..937306d227 100644 --- a/tests/integrations/django/test_transactions.py +++ b/tests/integrations/django/test_transactions.py @@ -8,14 +8,15 @@ except ImportError: import mock # python < 3.3 + +# django<2.0 has only `url` with regex based patterns. +# django>=2.0 renames `url` to `re_path`, and additionally introduces `path` +# for new style URL patterns, e.g. . if django.VERSION >= (2, 0): from django.urls import path, re_path from django.urls.converters import PathConverter from django.conf.urls import include else: - # django<2.0 has only `url` with regex based patterns. - # django>=2.0 renames `url` to `re_path`, and additionally introduces `path` - # for new style URL patterns, e.g. . from django.conf.urls import url as re_path, include from sentry_sdk.integrations.django.transactions import RavenResolver @@ -102,19 +103,13 @@ def test_resolver_path_complex_path(): class CustomPathConverter(PathConverter): regex = r"[^/]+(/[^/]+){0,2}" - def to_python(self, value): - return value.split("/") - - def to_url(self, value): - return "/".join(value) - with mock.patch( "django.urls.resolvers.get_converter", return_value=CustomPathConverter ): - url_conf = (path("api/v3/", lambda x: ""),) + url_conf = (path("api/v3/", lambda x: ""),) resolver = RavenResolver() result = resolver.resolve("/api/v3/abc/def/ghi", url_conf) - assert result == "/api/v3/{custom_path}" + assert result == "/api/v3/{my_path}" @pytest.mark.skipif( From 1ff04a63d21f4d50a3144f4c967cea47729ccb9d Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 24 Oct 2023 16:07:04 +0200 Subject: [PATCH 08/14] smaller diff --- sentry_sdk/integrations/django/transactions.py | 3 ++- tests/integrations/django/test_transactions.py | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/integrations/django/transactions.py b/sentry_sdk/integrations/django/transactions.py index 5962583bfe..c222a11ef4 100644 --- a/sentry_sdk/integrations/django/transactions.py +++ b/sentry_sdk/integrations/django/transactions.py @@ -12,13 +12,14 @@ from sentry_sdk._types import TYPE_CHECKING if TYPE_CHECKING: + from django.urls.resolvers import URLResolver from typing import Dict from typing import List from typing import Optional + from django.urls.resolvers import URLPattern from typing import Tuple from typing import Union from re import Pattern - from django.urls.resolvers import URLPattern, URLResolver from django import VERSION as DJANGO_VERSION diff --git a/tests/integrations/django/test_transactions.py b/tests/integrations/django/test_transactions.py index 937306d227..c9914c8ec5 100644 --- a/tests/integrations/django/test_transactions.py +++ b/tests/integrations/django/test_transactions.py @@ -19,14 +19,14 @@ else: from django.conf.urls import url as re_path, include -from sentry_sdk.integrations.django.transactions import RavenResolver - - if django.VERSION < (1, 9): included_url_conf = (re_path(r"^foo/bar/(?P[\w]+)", lambda x: ""),), "", "" else: included_url_conf = ((re_path(r"^foo/bar/(?P[\w]+)", lambda x: ""),), "") +from sentry_sdk.integrations.django.transactions import RavenResolver + + example_url_conf = ( re_path(r"^api/(?P[\w_-]+)/store/$", lambda x: ""), re_path(r"^api/(?P(v1|v2))/author/$", lambda x: ""), From db857fe6074f2ee70c2eaaf20d1148ea90cb0875 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 24 Oct 2023 16:43:12 +0200 Subject: [PATCH 09/14] simplify --- sentry_sdk/integrations/django/transactions.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/django/transactions.py b/sentry_sdk/integrations/django/transactions.py index c222a11ef4..c69d908966 100644 --- a/sentry_sdk/integrations/django/transactions.py +++ b/sentry_sdk/integrations/django/transactions.py @@ -73,11 +73,11 @@ def _simplify(self, pattern): # conflicts with the other rules because we're doing regexp matches if DJANGO_VERSION >= (2, 0) and isinstance(pattern.pattern, RoutePattern): - result = self._new_style_group_matcher.sub( + return self._new_style_group_matcher.sub( lambda m: "{%s}" % m.group(2), pattern.pattern._route ) - else: - result = get_regex(pattern).pattern + + result = get_regex(pattern).pattern # rather than parsing tokens result = self._optional_group_matcher.sub(lambda m: "%s" % m.group(1), result) From 7fe2bfa7b7d434abc89d664b7d3b17d92ad3f0d2 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 24 Oct 2023 16:44:12 +0200 Subject: [PATCH 10/14] inline import --- sentry_sdk/integrations/django/transactions.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/sentry_sdk/integrations/django/transactions.py b/sentry_sdk/integrations/django/transactions.py index c69d908966..8170c3aad9 100644 --- a/sentry_sdk/integrations/django/transactions.py +++ b/sentry_sdk/integrations/django/transactions.py @@ -28,11 +28,6 @@ except ImportError: from django.core.urlresolvers import get_resolver -try: - from django.urls.resolvers import RoutePattern -except ImportError: - RoutePattern = type(None) - def get_regex(resolver_or_pattern): # type: (Union[URLPattern, URLResolver]) -> Pattern[str] @@ -72,10 +67,13 @@ def _simplify(self, pattern): # TODO(dcramer): it'd be nice to change these into [%s] but it currently # conflicts with the other rules because we're doing regexp matches - if DJANGO_VERSION >= (2, 0) and isinstance(pattern.pattern, RoutePattern): - return self._new_style_group_matcher.sub( - lambda m: "{%s}" % m.group(2), pattern.pattern._route - ) + if DJANGO_VERSION >= (2, 0): + from django.urls.resolvers import RoutePattern + + if isinstance(pattern.pattern, RoutePattern): + return self._new_style_group_matcher.sub( + lambda m: "{%s}" % m.group(2), pattern.pattern._route + ) result = get_regex(pattern).pattern From 4af4f6d2f67e16e79ee1a21522f63e88771f55aa Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 24 Oct 2023 17:34:58 +0200 Subject: [PATCH 11/14] move comment --- sentry_sdk/integrations/django/transactions.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/django/transactions.py b/sentry_sdk/integrations/django/transactions.py index 8170c3aad9..2be74b237d 100644 --- a/sentry_sdk/integrations/django/transactions.py +++ b/sentry_sdk/integrations/django/transactions.py @@ -63,9 +63,6 @@ def _simplify(self, pattern): To: > "{sport_slug}/athletes/{athlete_slug}/" """ - # remove optional params - # TODO(dcramer): it'd be nice to change these into [%s] but it currently - # conflicts with the other rules because we're doing regexp matches if DJANGO_VERSION >= (2, 0): from django.urls.resolvers import RoutePattern @@ -77,6 +74,9 @@ def _simplify(self, pattern): result = get_regex(pattern).pattern + # remove optional params + # TODO(dcramer): it'd be nice to change these into [%s] but it currently + # conflicts with the other rules because we're doing regexp matches # rather than parsing tokens result = self._optional_group_matcher.sub(lambda m: "%s" % m.group(1), result) From 75fd9f62b18380181d8ebf0e0df771f7121b4e1d Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 24 Oct 2023 17:35:32 +0200 Subject: [PATCH 12/14] add comment --- sentry_sdk/integrations/django/transactions.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/django/transactions.py b/sentry_sdk/integrations/django/transactions.py index 2be74b237d..cc10c1b9dd 100644 --- a/sentry_sdk/integrations/django/transactions.py +++ b/sentry_sdk/integrations/django/transactions.py @@ -63,7 +63,8 @@ def _simplify(self, pattern): To: > "{sport_slug}/athletes/{athlete_slug}/" """ - + # "new-style" path patterns can be parsed directly without turning them + # into regexes first if DJANGO_VERSION >= (2, 0): from django.urls.resolvers import RoutePattern From d797da525f45b037676930bc790318bd05614d36 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 25 Oct 2023 14:35:48 +0200 Subject: [PATCH 13/14] move import up --- sentry_sdk/integrations/django/transactions.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/sentry_sdk/integrations/django/transactions.py b/sentry_sdk/integrations/django/transactions.py index cc10c1b9dd..617d29ce61 100644 --- a/sentry_sdk/integrations/django/transactions.py +++ b/sentry_sdk/integrations/django/transactions.py @@ -23,6 +23,11 @@ from django import VERSION as DJANGO_VERSION +if DJANGO_VERSION >= (2, 0): + from django.urls.resolvers import RoutePattern +else: + RoutePattern = None + try: from django.urls import get_resolver except ImportError: @@ -65,13 +70,10 @@ def _simplify(self, pattern): """ # "new-style" path patterns can be parsed directly without turning them # into regexes first - if DJANGO_VERSION >= (2, 0): - from django.urls.resolvers import RoutePattern - - if isinstance(pattern.pattern, RoutePattern): - return self._new_style_group_matcher.sub( - lambda m: "{%s}" % m.group(2), pattern.pattern._route - ) + if RoutePattern is not None and isinstance(pattern.pattern, RoutePattern): + return self._new_style_group_matcher.sub( + lambda m: "{%s}" % m.group(2), pattern.pattern._route + ) result = get_regex(pattern).pattern From 9ff7cca6068d2ca24a637f3b882870cfff99ceb8 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 25 Oct 2023 16:37:03 +0200 Subject: [PATCH 14/14] be more defensive --- sentry_sdk/integrations/django/transactions.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/django/transactions.py b/sentry_sdk/integrations/django/transactions.py index 617d29ce61..b2e200b832 100644 --- a/sentry_sdk/integrations/django/transactions.py +++ b/sentry_sdk/integrations/django/transactions.py @@ -70,7 +70,11 @@ def _simplify(self, pattern): """ # "new-style" path patterns can be parsed directly without turning them # into regexes first - if RoutePattern is not None and isinstance(pattern.pattern, RoutePattern): + if ( + RoutePattern is not None + and hasattr(pattern, "pattern") + and isinstance(pattern.pattern, RoutePattern) + ): return self._new_style_group_matcher.sub( lambda m: "{%s}" % m.group(2), pattern.pattern._route )