Browse Source

feat(metrics): Collect analytics on places using EventUser (#58069)

## Objective:

Collect metrics on where EventUser model is queried. If we find out that
the endpoints are not used, we can delete them. If we find that code
paths querying EventUser is not used, then we can delete it.
NisanthanNanthakumar 1 year ago
parent
commit
ea29cf980b

+ 1 - 0
src/sentry/analytics/events/__init__.py

@@ -9,6 +9,7 @@ from .codeowners_assignment import *  # noqa: F401,F403
 from .codeowners_created import *  # noqa: F401,F403
 from .codeowners_updated import *  # noqa: F401,F403
 from .comment_webhooks import *  # noqa: F401,F403
+from .eventuser_endpoint_request import *  # noqa: F401,F403
 from .first_cron_checkin_sent import *  # noqa: F401,F403
 from .first_cron_monitor_created import *  # noqa: F401,F403
 from .first_event_sent import *  # noqa: F401,F403

+ 13 - 0
src/sentry/analytics/events/eventuser_endpoint_request.py

@@ -0,0 +1,13 @@
+from sentry import analytics
+
+
+class EventUserEndpointRequest(analytics.Event):
+    type = "eventuser_endpoint.request"
+
+    attributes = (
+        analytics.Attribute("endpoint", required=True),
+        analytics.Attribute("project_id", required=True),
+    )
+
+
+analytics.register(EventUserEndpointRequest)

+ 6 - 1
src/sentry/api/endpoints/group_tagkey_values.py

@@ -1,7 +1,7 @@
 from rest_framework.request import Request
 from rest_framework.response import Response
 
-from sentry import tagstore
+from sentry import analytics, tagstore
 from sentry.api.api_publish_status import ApiPublishStatus
 from sentry.api.base import EnvironmentMixin, region_silo_endpoint
 from sentry.api.bases.group import GroupEndpoint
@@ -29,6 +29,11 @@ class GroupTagKeyValuesEndpoint(GroupEndpoint, EnvironmentMixin):
         :pparam string key: the tag key to look the values up for.
         :auth: required
         """
+        analytics.record(
+            "eventuser_endpoint.request",
+            project_id=group.project_id,
+            endpoint="sentry.api.endpoints.group_tagkey_values.get",
+        )
         lookup_key = tagstore.prefix_reserved_key(key)
 
         environment_ids = [e.id for e in get_environments(request, group.project.organization)]

+ 11 - 0
src/sentry/api/endpoints/project_user_details.py

@@ -2,6 +2,7 @@ from rest_framework import status
 from rest_framework.request import Request
 from rest_framework.response import Response
 
+from sentry import analytics
 from sentry.api.api_publish_status import ApiPublishStatus
 from sentry.api.base import region_silo_endpoint
 from sentry.api.bases.project import ProjectEndpoint
@@ -18,6 +19,11 @@ class ProjectUserDetailsEndpoint(ProjectEndpoint):
     }
 
     def get(self, request: Request, project, user_hash) -> Response:
+        analytics.record(
+            "eventuser_endpoint.request",
+            project_id=project.id,
+            endpoint="sentry.api.endpoints.project_user_details.get",
+        )
         euser = EventUser.objects.get(project_id=project.id, hash=user_hash)
         return Response(serialize(euser, request.user))
 
@@ -32,6 +38,11 @@ class ProjectUserDetailsEndpoint(ProjectEndpoint):
         :pparam string project_slug: the slug of the project.
         :pparam string user_hash: the user hash.
         """
+        analytics.record(
+            "eventuser_endpoint.request",
+            project_id=project.id,
+            endpoint="sentry.api.endpoints.project_user_details.delete",
+        )
         if is_active_superuser(request):
             try:
                 euser = EventUser.objects.get(project_id=project.id, hash=user_hash)

+ 6 - 0
src/sentry/api/endpoints/project_users.py

@@ -1,6 +1,7 @@
 from rest_framework.request import Request
 from rest_framework.response import Response
 
+from sentry import analytics
 from sentry.api.api_publish_status import ApiPublishStatus
 from sentry.api.base import region_silo_endpoint
 from sentry.api.bases.project import ProjectEndpoint
@@ -31,6 +32,11 @@ class ProjectUsersEndpoint(ProjectEndpoint):
                               match on: ``id``, ``email``, ``username``, ``ip``.
                               For example, ``query=email:foo@example.com``
         """
+        analytics.record(
+            "eventuser_endpoint.request",
+            project_id=project.id,
+            endpoint="sentry.api.endpoints.project_users.get",
+        )
         queryset = EventUser.objects.filter(project_id=project.id)
         if request.GET.get("query"):
             try:

+ 8 - 3
src/sentry/tasks/unmerge.py

@@ -7,7 +7,7 @@ from typing import Any, Mapping, Optional, Tuple
 
 from django.db import router, transaction
 
-from sentry import eventstore, similarity, tsdb
+from sentry import analytics, eventstore, similarity, tsdb
 from sentry.constants import DEFAULT_LOGGER_NAME, LOG_LEVELS_MAP
 from sentry.culprit import generate_culprit
 from sentry.eventstore.models import BaseEvent
@@ -374,7 +374,12 @@ def repair_group_release_data(caches, project, events):
             instance.update(first_seen=first_seen)
 
 
-def get_event_user_from_interface(value):
+def get_event_user_from_interface(value, project):
+    analytics.record(
+        "eventuser_endpoint.request",
+        project_id=project.id,
+        endpoint="sentry.tasks.unmerge.get_event_user_from_interface",
+    )
     return EventUser(
         ident=value.get("id"),
         email=value.get("email"),
@@ -399,7 +404,7 @@ def collect_tsdb_data(caches, project, events):
         if user:
             sets[event.datetime][TSDBModel.users_affected_by_group][
                 (event.group_id, environment.id)
-            ].add(get_event_user_from_interface(user).tag_value)
+            ].add(get_event_user_from_interface(user, project).tag_value)
 
         frequencies[event.datetime][TSDBModel.frequent_environments_by_group][event.group_id][
             environment.id

+ 10 - 1
tests/sentry/api/endpoints/test_group_tagkey_values.py

@@ -1,3 +1,5 @@
+from unittest import mock
+
 from sentry.testutils.cases import APITestCase, PerformanceIssueTestCase, SnubaTestCase
 from sentry.testutils.helpers.datetime import before_now, iso_format
 from sentry.testutils.silo import region_silo_test
@@ -5,7 +7,8 @@ from sentry.testutils.silo import region_silo_test
 
 @region_silo_test(stable=True)
 class GroupTagKeyValuesTest(APITestCase, SnubaTestCase, PerformanceIssueTestCase):
-    def test_simple(self):
+    @mock.patch("sentry.analytics.record")
+    def test_simple(self, mock_record):
         key, value = "foo", "bar"
 
         project = self.create_project()
@@ -27,6 +30,12 @@ class GroupTagKeyValuesTest(APITestCase, SnubaTestCase, PerformanceIssueTestCase
 
         assert response.data[0]["value"] == "bar"
 
+        mock_record.assert_called_with(
+            "eventuser_endpoint.request",
+            project_id=project.id,
+            endpoint="sentry.api.endpoints.group_tagkey_values.get",
+        )
+
     def test_simple_perf(self):
         key, value = "foo", "bar"
         event = self.create_performance_issue(

+ 17 - 2
tests/sentry/api/endpoints/test_project_user_details.py

@@ -1,3 +1,5 @@
+from unittest import mock
+
 from sentry.models.eventuser import EventUser
 from sentry.testutils.cases import APITestCase
 from sentry.testutils.silo import region_silo_test
@@ -18,12 +20,20 @@ class ProjectUserDetailsTest(APITestCase):
         self.euser = EventUser.objects.create(email="foo@example.com", project_id=self.project.id)
         self.euser2 = EventUser.objects.create(email="bar@example.com", project_id=self.project.id)
 
-    def test_simple(self):
+    @mock.patch("sentry.analytics.record")
+    def test_simple(self, mock_record):
         self.login_as(user=self.user)
         response = self.get_success_response(self.org.slug, self.project.slug, self.euser.hash)
         assert response.data["id"] == str(self.euser.id)
 
-    def test_delete_event_user(self):
+        mock_record.assert_called_with(
+            "eventuser_endpoint.request",
+            project_id=self.project.id,
+            endpoint="sentry.api.endpoints.project_user_details.get",
+        )
+
+    @mock.patch("sentry.analytics.record")
+    def test_delete_event_user(self, mock_record):
         # Only delete an event user as a superuser
         self.login_as(user=self.user, superuser="true")
 
@@ -34,6 +44,11 @@ class ProjectUserDetailsTest(APITestCase):
 
         assert response.status_code == 200
         assert EventUser.objects.count() == 1
+        mock_record.assert_called_with(
+            "eventuser_endpoint.request",
+            project_id=self.project.id,
+            endpoint="sentry.api.endpoints.project_user_details.delete",
+        )
 
         # User doesn't exist
         path = f"/api/0/projects/{self.org.slug}/{self.project.slug}/users/1234567abcdefg"

+ 9 - 1
tests/sentry/api/endpoints/test_project_users.py

@@ -1,3 +1,5 @@
+from unittest import mock
+
 from django.urls import reverse
 
 from sentry.models.eventuser import EventUser
@@ -35,7 +37,8 @@ class ProjectUsersTest(APITestCase):
             },
         )
 
-    def test_simple(self):
+    @mock.patch("sentry.analytics.record")
+    def test_simple(self, mock_record):
         self.login_as(user=self.user)
 
         response = self.client.get(self.path, format="json")
@@ -45,6 +48,11 @@ class ProjectUsersTest(APITestCase):
         assert sorted(map(lambda x: x["id"], response.data)) == sorted(
             [str(self.euser1.id), str(self.euser2.id)]
         )
+        mock_record.assert_called_with(
+            "eventuser_endpoint.request",
+            project_id=self.project.id,
+            endpoint="sentry.api.endpoints.project_users.get",
+        )
 
     def test_empty_search_query(self):
         self.login_as(user=self.user)

+ 11 - 2
tests/snuba/tasks/test_unmerge.py

@@ -6,6 +6,7 @@ import itertools
 import logging
 import uuid
 from datetime import datetime, timedelta, timezone
+from unittest import mock
 from unittest.mock import patch
 
 from sentry import eventstream, tagstore, tsdb
@@ -170,7 +171,8 @@ class UnmergeTestCase(TestCase, SnubaTestCase):
         }
 
     @with_feature("projects:similarity-indexing")
-    def test_unmerge(self):
+    @mock.patch("sentry.analytics.record")
+    def test_unmerge(self, mock_record):
         now = before_now(minutes=5).replace(microsecond=0, tzinfo=timezone.utc)
 
         def time_from_now(offset=0):
@@ -463,7 +465,14 @@ class UnmergeTestCase(TestCase, SnubaTestCase):
 
         def collect_by_user_tag(aggregate, event):
             aggregate = aggregate if aggregate is not None else set()
-            aggregate.add(get_event_user_from_interface(event.data["user"]).tag_value)
+            aggregate.add(
+                get_event_user_from_interface(event.data["user"], event.group.project).tag_value
+            )
+            mock_record.assert_called_with(
+                "eventuser_endpoint.request",
+                project_id=event.group.project.id,
+                endpoint="sentry.tasks.unmerge.get_event_user_from_interface",
+            )
             return aggregate
 
         for series in [time_series, environment_time_series]: