Browse Source

feat(amplitude): Add first sourcemaps for project event (#59839)

Add the amplitude event `Sent First Sourcemaps For Project` a per-project equivalent of `Sent First Sourcemaps`.

- closes https://github.com/getsentry/sentry/issues/57967
ArthurKnaus 1 year ago
parent
commit
3975aa7b10

+ 1 - 1
migrations_lockfile.txt

@@ -9,5 +9,5 @@ feedback: 0003_feedback_add_env
 hybridcloud: 0008_add_externalactorreplica
 nodestore: 0002_nodestore_no_dictfield
 replays: 0003_add_size_to_recording_segment
-sentry: 0600_eventattachment_metadata
+sentry: 0601_add_has_sourcemaps_project_flag
 social_auth: 0002_default_auto_field

+ 8 - 0
src/sentry/analytics/events/first_sourcemaps_sent.py

@@ -8,7 +8,15 @@ class FirstSourcemapsSentEvent(analytics.Event):
         analytics.Attribute("user_id"),
         analytics.Attribute("organization_id"),
         analytics.Attribute("project_id"),
+        analytics.Attribute("platform", required=False),
+        analytics.Attribute("url", required=False),
+        analytics.Attribute("project_platform", required=False),
     )
 
 
+class FirstSourcemapsSentEventForProject(FirstSourcemapsSentEvent):
+    type = "first_sourcemaps_for_project.sent"
+
+
 analytics.register(FirstSourcemapsSentEvent)
+analytics.register(FirstSourcemapsSentEventForProject)

+ 52 - 0
src/sentry/migrations/0601_add_has_sourcemaps_project_flag.py

@@ -0,0 +1,52 @@
+# Generated by Django 3.2.23 on 2023-11-15 11:34
+
+from django.db import migrations
+
+import bitfield.models
+from sentry.new_migrations.migrations import CheckedMigration
+
+
+class Migration(CheckedMigration):
+    # This flag is used to mark that a migration shouldn't be automatically run in production. For
+    # the most part, this should only be used for operations where it's safe to run the migration
+    # after your code has deployed. So this should not be used for most operations that alter the
+    # schema of a table.
+    # Here are some things that make sense to mark as dangerous:
+    # - Large data migrations. Typically we want these to be run manually by ops so that they can
+    #   be monitored and not block the deploy for a long period of time while they run.
+    # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
+    #   have ops run this and not block the deploy. Note that while adding an index is a schema
+    #   change, it's completely safe to run the operation after the code has deployed.
+    is_dangerous = False
+
+    dependencies = [
+        ("sentry", "0600_eventattachment_metadata"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="project",
+            name="flags",
+            field=bitfield.models.BitField(
+                [
+                    "has_releases",
+                    "has_issue_alerts_targeting",
+                    "has_transactions",
+                    "has_alert_filters",
+                    "has_sessions",
+                    "has_profiles",
+                    "has_replays",
+                    "has_feedbacks",
+                    "spike_protection_error_currently_active",
+                    "spike_protection_transaction_currently_active",
+                    "spike_protection_attachment_currently_active",
+                    "has_minified_stack_trace",
+                    "has_cron_monitors",
+                    "has_cron_checkins",
+                    "has_sourcemaps",
+                ],
+                default=10,
+                null=True,
+            ),
+        ),
+    ]

+ 3 - 1
src/sentry/models/project.py

@@ -275,6 +275,9 @@ class Project(Model, PendingDeletionMixin, OptionMixin, SnowflakeIdMixin):
         # This Project has sent check-ins
         has_cron_checkins: bool
 
+        # This Project has event with sourcemaps
+        has_sourcemaps: bool
+
         bitfield_default = 10
         bitfield_null = True
 
@@ -615,7 +618,6 @@ class Project(Model, PendingDeletionMixin, OptionMixin, SnowflakeIdMixin):
         )
 
     def delete(self, **kwargs):
-
         # There is no foreign key relationship so we have to manually cascade.
         notifications_service.remove_notification_settings_for_project(project_id=self.id)
 

+ 41 - 1
src/sentry/receivers/onboarding.py

@@ -51,6 +51,9 @@ logger = logging.getLogger("sentry")
 START_DATE_TRACKING_FIRST_EVENT_WITH_MINIFIED_STACK_TRACE_PER_PROJ = datetime(
     2022, 12, 14, tzinfo=timezone.utc
 )
+# Used to determine if we should or not record an analytic data
+# for a first sourcemap of a project
+START_DATE_TRACKING_FIRST_SOURCEMAP_PER_PROJ = datetime(2023, 11, 16, tzinfo=timezone.utc)
 
 
 @project_created.connect(weak=False)
@@ -425,7 +428,6 @@ def record_event_with_first_minified_stack_trace_for_project(project, event, **k
 
     # First, only enter this logic if we've never seen a minified stack trace before
     if not project.flags.has_minified_stack_trace:
-
         # Next, attempt to update the flag, but ONLY if the flag is currently not set.
         # The number of affected rows tells us whether we succeeded or not. If we didn't, then skip sending the event.
         # This guarantees us that this analytics event will only be ever sent once.
@@ -476,10 +478,48 @@ def record_sourcemaps_received(project, event, **kwargs):
             user_id=user.id if user else None,
             organization_id=project.organization_id,
             project_id=project.id,
+            platform=event.platform,
+            project_platform=project.platform,
+            url=dict(event.tags).get("url", None),
         )
         try_mark_onboarding_complete(project.organization_id)
 
 
+@event_processed.connect(weak=False)
+def record_sourcemaps_received_for_project(project, event, **kwargs):
+    if not has_sourcemap(event):
+        return
+
+    try:
+        user: RpcUser = Organization.objects.get(id=project.organization_id).get_default_owner()
+    except IndexError:
+        logger.warning(
+            "Cannot record sourcemaps received for organization (%s) due to missing owners",
+            project.organization_id,
+        )
+        return
+
+    # First, only enter this logic if we've never seen a minified stack trace before
+    if not project.flags.has_sourcemaps:
+        # Next, attempt to update the flag, but ONLY if the flag is currently not set.
+        # The number of affected rows tells us whether we succeeded or not. If we didn't, then skip sending the event.
+        # This guarantees us that this analytics event will only be ever sent once.
+        affected = Project.objects.filter(
+            id=project.id, flags=F("flags").bitand(~Project.flags.has_sourcemaps)
+        ).update(flags=F("flags").bitor(Project.flags.has_sourcemaps))
+
+        if project.date_added > START_DATE_TRACKING_FIRST_SOURCEMAP_PER_PROJ and affected > 0:
+            analytics.record(
+                "first_sourcemaps_for_project.sent",
+                user_id=user.id if user else None,
+                organization_id=project.organization_id,
+                project_id=project.id,
+                platform=event.platform,
+                project_platform=project.platform,
+                url=dict(event.tags).get("url", None),
+            )
+
+
 @plugin_enabled.connect(weak=False)
 def record_plugin_enabled(plugin, project, user, **kwargs):
     if isinstance(plugin, IssueTrackingPlugin) or isinstance(plugin, IssueTrackingPlugin2):

+ 179 - 0
tests/sentry/receivers/test_onboarding.py

@@ -744,3 +744,182 @@ class OrganizationOnboardingTaskTest(TestCase):
                 count += 1
 
         assert count == 0
+
+    @patch("sentry.analytics.record")
+    def test_first_event_without_sourcemaps_received(self, record_analytics):
+        """
+        Test that an analytics event is NOT recorded when
+        no event with sourcemaps is received
+        """
+        now = django_timezone.now()
+        project = self.create_project(first_event=now)
+        project_created.send(project=project, user=self.user, sender=type(project))
+        data = load_data("javascript")
+        data["exception"] = {
+            "values": [
+                {
+                    "stacktrace": {"frames": [{"data": {}}]},
+                    "type": "TypeError",
+                }
+            ]
+        }
+        event = self.store_event(
+            project_id=project.id,
+            data=data,
+        )
+
+        event_processed.send(project=project, event=event, sender=type(project))
+
+        count = 0
+        for call_arg in record_analytics.call_args_list:
+            if "first_sourcemaps_for_project.sent" in call_arg[0]:
+                count += 1
+
+        assert count == 0
+
+    @patch("sentry.analytics.record")
+    def test_first_event_with_sourcemaps_received(self, record_analytics):
+        """
+        Test that an analytics event is recorded when
+        a first event with sourcemaps is received
+        """
+        now = django_timezone.now()
+        project = self.create_project(first_event=now, platform="VueJS")
+        project_created.send(project=project, user=self.user, sender=type(project))
+        url = "http://localhost:3000"
+        data = load_data("javascript")
+        data["tags"] = [("url", url)]
+        data["exception"] = {
+            "values": [
+                {
+                    "stacktrace": {
+                        "frames": [
+                            {
+                                "data": {
+                                    "sourcemap": "https://media.sentry.io/_static/29e365f8b0d923bc123e8afa38d890c3/sentry/dist/vendor.js.map"
+                                }
+                            }
+                        ]
+                    },
+                    "type": "TypeError",
+                }
+            ]
+        }
+
+        event = self.store_event(
+            project_id=project.id,
+            data=data,
+        )
+        event_processed.send(project=project, event=event, sender=type(project))
+
+        record_analytics.assert_called_with(
+            "first_sourcemaps_for_project.sent",
+            user_id=self.user.id,
+            organization_id=project.organization_id,
+            project_id=project.id,
+            platform=event.platform,
+            project_platform="VueJS",
+            url=url,
+        )
+
+    @patch("sentry.analytics.record")
+    def test_analytic_triggered_only_once_if_multiple_events_with_sourcemaps_received(
+        self, record_analytics
+    ):
+        """
+        Test that an analytic event is triggered only once when
+        multiple events with sourcemaps are received
+        """
+        now = django_timezone.now()
+        project = self.create_project(first_event=now)
+        project_created.send(project=project, user=self.user, sender=type(project))
+        url = "http://localhost:3000"
+        data = load_data("javascript")
+        data["tags"] = [("url", url)]
+        data["exception"] = {
+            "values": [
+                {
+                    "stacktrace": {
+                        "frames": [
+                            {
+                                "data": {
+                                    "sourcemap": "https://media.sentry.io/_static/29e365f8b0d923bc123e8afa38d890c3/sentry/dist/vendor.js.map"
+                                }
+                            }
+                        ]
+                    },
+                    "type": "TypeError",
+                }
+            ]
+        }
+
+        # Store first event
+        event_1 = self.store_event(
+            project_id=project.id,
+            data=data,
+        )
+        event_processed.send(project=project, event=event_1, sender=type(project))
+
+        # Store second event
+        event_2 = self.store_event(
+            project_id=project.id,
+            data=data,
+        )
+        event_processed.send(project=project, event=event_2, sender=type(project))
+
+        count = 0
+        for call_arg in record_analytics.call_args_list:
+            if "first_sourcemaps_for_project.sent" in call_arg[0]:
+                count += 1
+
+        assert count == 1
+
+    @patch("sentry.analytics.record")
+    def test_old_project_sending_sourcemap_event(self, record_analytics):
+        """
+        Test that an analytics event is NOT recorded when
+        the project creation date is older than the date we defined (START_DATE_TRACKING_FIRST_EVENT_WITH_SOURCEMAPS_PER_PROJ).
+
+        In this test we also check  if the has_sourcemaps is being set to "True" in old projects
+        """
+        old_date = datetime(2022, 12, 10, tzinfo=timezone.utc)
+        project = self.create_project(first_event=old_date, date_added=old_date)
+        project_created.send(project=project, user=self.user, sender=type(project))
+        url = "http://localhost:3000"
+        data = load_data("javascript")
+        data["tags"] = [("url", url)]
+        data["exception"] = {
+            "values": [
+                {
+                    "stacktrace": {
+                        "frames": [
+                            {
+                                "data": {
+                                    "sourcemap": "https://media.sentry.io/_static/29e365f8b0d923bc123e8afa38d890c3/sentry/dist/vendor.js.map"
+                                }
+                            }
+                        ]
+                    },
+                    "type": "TypeError",
+                }
+            ]
+        }
+
+        # project.flags.has_sourcemaps = False
+        assert not project.flags.has_sourcemaps
+
+        event = self.store_event(project_id=project.id, data=data)
+        event_processed.send(project=project, event=event, sender=type(project))
+
+        project.refresh_from_db()
+
+        # project.flags.has_sourcemaps = True
+        assert project.flags.has_sourcemaps
+
+        # The analytic's event "first_event_with_minified_stack_trace_for_project" shall not be sent
+        count = 0
+        for call_arg in record_analytics.call_args_list:
+            if "first_sourcemaps_for_project.sent" in call_arg[0]:
+                count += 1
+
+        assert count == 0