From 19261d357f50c219bf688f4616c34c8ece61d0db Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Wed, 6 Oct 2021 15:18:50 +0100 Subject: [PATCH 1/3] tags request origins in Sentry when loading request data from the body --- posthog/test/test_utils.py | 29 +++++++++++++++++++++++++++++ posthog/utils.py | 1 + 2 files changed, 30 insertions(+) diff --git a/posthog/test/test_utils.py b/posthog/test/test_utils.py index ebcc960603b1a..39c5065667bd1 100644 --- a/posthog/test/test_utils.py +++ b/posthog/test/test_utils.py @@ -1,3 +1,5 @@ +from unittest.mock import Mock, patch + from django.test import TestCase from django.test.client import RequestFactory from freezegun import freeze_time @@ -84,6 +86,22 @@ def test_prefer_pageview(self): class TestLoadDataFromRequest(TestCase): + @patch("posthog.utils.push_scope") + def test_pushes_request_origin_into_sentry_scope(self, push_scope): + origin = "potato.io" + + mock_set_tag = self.mock_sentry_context(push_scope) + + rf = RequestFactory() + post_request = rf.post("/s/", "content", "text/plain") + post_request.META["REMOTE_HOST"] = origin + + with self.assertRaises(RequestParsingError) as ctx: + load_data_from_request(post_request) + + push_scope.assert_called_once() + mock_set_tag.assert_called_once_with(origin) + def test_fails_to_JSON_parse_the_literal_string_undefined_when_not_compressed(self): """ load_data_from_request assumes that any data @@ -110,3 +128,14 @@ def test_raises_specific_error_for_the_literal_string_undefined_when_compressed( "data being loaded from the request body for decompression is the literal string 'undefined'", str(ctx.exception), ) + + def mock_sentry_context(self, push_scope): + mock_scope = Mock() + mock_set_tag = Mock() + mock_scope.set_context = Mock() + mock_scope.set_tag = mock_set_tag + mock_context_manager = Mock() + mock_context_manager.__enter__ = Mock(return_value=mock_scope) + mock_context_manager.__exit__ = Mock(return_value=None) + push_scope.return_value = mock_context_manager + return mock_set_tag diff --git a/posthog/utils.py b/posthog/utils.py index eb5d376293488..47bbc087ac2ac 100644 --- a/posthog/utils.py +++ b/posthog/utils.py @@ -392,6 +392,7 @@ def load_data_from_request(request): # add the data in sentry's scope in case there's an exception with push_scope() as scope: scope.set_context("data", data) + scope.set_tag("origin", request.META.get("REMOTE_HOST")) compression = ( request.GET.get("compression") or request.POST.get("compression") or request.headers.get("content-encoding", "") From e0f450d6aacb5dfe9f683e0ed0acb758be624819 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Wed, 6 Oct 2021 16:16:23 +0100 Subject: [PATCH 2/3] actually make tests pass :/ --- posthog/test/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/test/test_utils.py b/posthog/test/test_utils.py index 39c5065667bd1..4ccd194cd60c1 100644 --- a/posthog/test/test_utils.py +++ b/posthog/test/test_utils.py @@ -100,7 +100,7 @@ def test_pushes_request_origin_into_sentry_scope(self, push_scope): load_data_from_request(post_request) push_scope.assert_called_once() - mock_set_tag.assert_called_once_with(origin) + mock_set_tag.assert_called_once_with("origin", origin) def test_fails_to_JSON_parse_the_literal_string_undefined_when_not_compressed(self): """ From 62eff41b91192431391c65d7dfd6e6798d89ae29 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Thu, 7 Oct 2021 12:46:34 +0100 Subject: [PATCH 3/3] reduces duplication of mocking sentry --- posthog/api/test/mock_sentry.py | 13 +++++++++++++ posthog/api/test/test_capture.py | 17 +++-------------- posthog/test/test_utils.py | 14 ++------------ 3 files changed, 18 insertions(+), 26 deletions(-) create mode 100644 posthog/api/test/mock_sentry.py diff --git a/posthog/api/test/mock_sentry.py b/posthog/api/test/mock_sentry.py new file mode 100644 index 0000000000000..cfa14bde599ab --- /dev/null +++ b/posthog/api/test/mock_sentry.py @@ -0,0 +1,13 @@ +from unittest.mock import Mock + + +def mock_sentry_context_for_tagging(patched_push_scope): + mock_scope = Mock() + mock_set_tag = Mock() + mock_scope.set_context = Mock() + mock_scope.set_tag = mock_set_tag + mock_context_manager = Mock() + mock_context_manager.__enter__ = Mock(return_value=mock_scope) + mock_context_manager.__exit__ = Mock(return_value=None) + patched_push_scope.return_value = mock_context_manager + return mock_set_tag diff --git a/posthog/api/test/test_capture.py b/posthog/api/test/test_capture.py index 57b01b611850d..68afa8a7844f0 100644 --- a/posthog/api/test/test_capture.py +++ b/posthog/api/test/test_capture.py @@ -12,6 +12,7 @@ from freezegun import freeze_time from rest_framework import status +from posthog.api.test.mock_sentry import mock_sentry_context_for_tagging from posthog.constants import ENVIRONMENT_TEST from posthog.models import PersonalAPIKey from posthog.models.feature_flag import FeatureFlag @@ -89,7 +90,7 @@ def test_capture_event(self, patch_process_event_with_plugins): @patch("posthog.api.capture.push_scope") @patch("posthog.api.capture.celery_app.send_task", MagicMock()) def test_capture_event_adds_library_to_sentry(self, patch_push_scope): - mock_set_tag = self.mock_sentry_context(patch_push_scope) + mock_set_tag = mock_sentry_context_for_tagging(patch_push_scope) data = { "event": "$autocapture", @@ -114,7 +115,7 @@ def test_capture_event_adds_library_to_sentry(self, patch_push_scope): @patch("posthog.api.capture.push_scope") @patch("posthog.api.capture.celery_app.send_task", MagicMock()) def test_capture_event_adds_unknown_to_sentry_when_no_properties_sent(self, patch_push_scope): - mock_set_tag = self.mock_sentry_context(patch_push_scope) + mock_set_tag = mock_sentry_context_for_tagging(patch_push_scope) data = { "event": "$autocapture", @@ -134,18 +135,6 @@ def test_capture_event_adds_unknown_to_sentry_when_no_properties_sent(self, patc mock_set_tag.assert_has_calls([call("library", "unknown"), call("library.version", "unknown")]) - @staticmethod - def mock_sentry_context(push_scope): - mock_scope = Mock() - mock_set_tag = Mock() - mock_scope.set_context = Mock() - mock_scope.set_tag = mock_set_tag - mock_context_manager = Mock() - mock_context_manager.__enter__ = Mock(return_value=mock_scope) - mock_context_manager.__exit__ = Mock(return_value=None) - push_scope.return_value = mock_context_manager - return mock_set_tag - @patch("posthog.models.team.TEAM_CACHE", {}) @patch("posthog.api.capture.celery_app.send_task") def test_test_api_key(self, patch_process_event_with_plugins): diff --git a/posthog/test/test_utils.py b/posthog/test/test_utils.py index 4ccd194cd60c1..41a76fa9c67a6 100644 --- a/posthog/test/test_utils.py +++ b/posthog/test/test_utils.py @@ -4,6 +4,7 @@ from django.test.client import RequestFactory from freezegun import freeze_time +from posthog.api.test.mock_sentry import mock_sentry_context_for_tagging from posthog.exceptions import RequestParsingError from posthog.models import EventDefinition from posthog.test.base import BaseTest @@ -90,7 +91,7 @@ class TestLoadDataFromRequest(TestCase): def test_pushes_request_origin_into_sentry_scope(self, push_scope): origin = "potato.io" - mock_set_tag = self.mock_sentry_context(push_scope) + mock_set_tag = mock_sentry_context_for_tagging(push_scope) rf = RequestFactory() post_request = rf.post("/s/", "content", "text/plain") @@ -128,14 +129,3 @@ def test_raises_specific_error_for_the_literal_string_undefined_when_compressed( "data being loaded from the request body for decompression is the literal string 'undefined'", str(ctx.exception), ) - - def mock_sentry_context(self, push_scope): - mock_scope = Mock() - mock_set_tag = Mock() - mock_scope.set_context = Mock() - mock_scope.set_tag = mock_set_tag - mock_context_manager = Mock() - mock_context_manager.__enter__ = Mock(return_value=mock_scope) - mock_context_manager.__exit__ = Mock(return_value=None) - push_scope.return_value = mock_context_manager - return mock_set_tag