Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: sunset appstore connect integration data #74100

Merged
merged 7 commits into from
Sep 9, 2024

Conversation

anonrig
Copy link
Contributor

@anonrig anonrig commented Jul 10, 2024

This is the last remaining part of the AppStore connect integration. This is a follow-up to #73960

It's official. Sentry sunsets the AppStore connect integration.

Ref: #51994

PS: Let's merge this once the previous PR lands production.

Copy link
Contributor

This PR has a migration; here is the generated SQL for src/sentry/migrations/0736_remove_appstore_connect_integration_tables.py ()

--
-- Raw SQL operation
--
DROP TABLE IF EXISTS "sentry_latestappconnectbuildscheck";
--
-- Raw SQL operation
--
DROP TABLE IF EXISTS "sentry_appconnectbuild";

Copy link
Contributor

@loewenheim loewenheim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥳

Comment on lines 454 to 502
for source in custom_sources
if source["type"] != "appStoreConnect"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets keep this, and the above filter for some indefinite time, because there is for sure still project options floating around which still have these config options in them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll keep this as it is, and just pass filter_appconnect=True to parse_sources function on line 449 or 452, depending on which side of the changes you're looking at

@anonrig
Copy link
Contributor Author

anonrig commented Jul 17, 2024

I couldn't figure out how to push to this branch on a non-sentry computer, so here's the diff, in case someone wants to takeover this pull-request:

diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt
index 31c5de415d6..d282cc36e97 100644
--- a/migrations_lockfile.txt
+++ b/migrations_lockfile.txt
@@ -10,6 +10,6 @@ hybridcloud: 0016_add_control_cacheversion
 nodestore: 0002_nodestore_no_dictfield
 remote_subscriptions: 0003_drop_remote_subscription
 replays: 0004_index_together
-sentry: 0741_metric_alert_anomaly_detection
+sentry: 0736_remove_appstore_connect_integration_tables
 social_auth: 0002_default_auto_field
 uptime: 0005_uptime_status
diff --git a/src/sentry/api/endpoints/project_details.py b/src/sentry/api/endpoints/project_details.py
index 633743b9d8e..feb7463e49a 100644
--- a/src/sentry/api/endpoints/project_details.py
+++ b/src/sentry/api/endpoints/project_details.py
@@ -347,12 +347,7 @@ class ProjectAdminSerializer(ProjectMemberSerializer):
         # * negative cache entries (eg auth errors) are retried immediately.
         # * positive caches are re-fetches as well, making it less effective.
         for source in added_or_modified_sources:
-            # This should only apply to sources which are being fed to symbolicator.
-            # App Store Connect in particular is managed in a completely different
-            # way, and needs its `id` to stay valid for a longer time.
-            # TODO(@anonrig): Remove this when all AppStore connect data is removed.
-            if source["type"] != "appStoreConnect":
-                source["id"] = str(uuid4())
+            source["id"] = str(uuid4())
 
         sources_json = orjson.dumps(sources).decode() if sources else ""
 
diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py
index b7f48ac5d83..d1408c89bfb 100644
--- a/src/sentry/conf/server.py
+++ b/src/sentry/conf/server.py
@@ -740,8 +740,6 @@ CELERY_IMPORTS = (
     "sentry.replays.tasks",
     "sentry.monitors.tasks.clock_pulse",
     "sentry.monitors.tasks.detect_broken_monitor_envs",
-    # TODO(@anonrig): Remove this when AppStore integration is removed.
-    "sentry.tasks.app_store_connect",
     "sentry.tasks.assemble",
     "sentry.tasks.auth",
     "sentry.tasks.auto_remove_inbox",
@@ -844,8 +842,6 @@ CELERY_QUEUES_REGION = [
     Queue("auth", routing_key="auth"),
     Queue("alerts", routing_key="alerts"),
     Queue("app_platform", routing_key="app_platform"),
-    # TODO(@anonrig): Remove this when all AppStore connect data is removed.
-    Queue("appstoreconnect", routing_key="sentry.tasks.app_store_connect.#"),
     Queue("assemble", routing_key="assemble"),
     Queue("backfill_seer_grouping_records", routing_key="backfill_seer_grouping_records"),
     Queue("buffers.process_pending", routing_key="buffers.process_pending"),
@@ -1523,10 +1519,6 @@ SENTRY_RELAY_TASK_APM_SAMPLING = 0
 # sample rate for ingest consumer processing functions
 SENTRY_INGEST_CONSUMER_APM_SAMPLING = 0
 
-# TODO(@anonrig): Remove this when all AppStore connect data is removed.
-# sample rate for Apple App Store Connect tasks transactions
-SENTRY_APPCONNECT_APM_SAMPLING = SENTRY_BACKEND_APM_SAMPLING
-
 # sample rate for suspect commits task
 SENTRY_SUSPECT_COMMITS_APM_SAMPLING = 0
 
diff --git a/src/sentry/lang/native/sources.py b/src/sentry/lang/native/sources.py
index a6d517d9cdc..67801953f56 100644
--- a/src/sentry/lang/native/sources.py
+++ b/src/sentry/lang/native/sources.py
@@ -385,7 +385,7 @@ def validate_sources(sources, schema=SOURCES_WITHOUT_APPSTORE_CONNECT):
         ids.add(source["id"])
 
 
-def parse_sources(config, filter_appconnect):
+def parse_sources(config: str | None, filter_appconnect: bool):
     """
     Parses the given sources in the config string (from JSON).
     """
@@ -496,11 +496,7 @@ def get_sources_for_project(project):
     if sources_config:
         try:
             custom_sources = parse_sources(sources_config, filter_appconnect=True)
-            sources.extend(
-                normalize_user_source(source)
-                for source in custom_sources
-                if source["type"] != "appStoreConnect"
-            )
+            sources.extend(normalize_user_source(source) for source in custom_sources)
         except InvalidSourcesError:
             # Source configs should be validated when they are saved. If this
             # did not happen, this indicates a bug. Record this, but do not stop
diff --git a/src/sentry/migrations/0736_remove_appstore_connect_integration_tables.py b/src/sentry/migrations/0736_remove_appstore_connect_integration_tables.py
new file mode 100644
index 00000000000..81cf7fad587
--- /dev/null
+++ b/src/sentry/migrations/0736_remove_appstore_connect_integration_tables.py
@@ -0,0 +1,36 @@
+# Generated by Django 5.0.6 on 2024-07-10 22:56
+
+from django.db import migrations
+
+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.
+    # 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 post deployment:
+    # - Large data migrations. Typically we want these to be run manually 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
+    #   run this outside deployments so that we don't block them. Note that while adding an index
+    #   is a schema change, it's completely safe to run the operation after the code has deployed.
+    # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment
+
+    is_post_deployment = False
+
+    dependencies = [
+        ("sentry", "0735_sunset_appstore_connect_integration"),
+    ]
+
+    operations = [
+        migrations.RunSQL(
+            sql='DROP TABLE IF EXISTS "sentry_latestappconnectbuildscheck"',
+            hints={"tables": ["sentry_latestappconnectbuildscheck"]},
+        ),
+        migrations.RunSQL(
+            sql='DROP TABLE IF EXISTS "sentry_appconnectbuild"',
+            hints={"tables": ["sentry_appconnectbuild"]},
+        ),
+    ]
diff --git a/src/sentry/tasks/app_store_connect.py b/src/sentry/tasks/app_store_connect.py
deleted file mode 100644
index 5c3747998f4..00000000000
--- a/src/sentry/tasks/app_store_connect.py
+++ /dev/null
@@ -1,16 +0,0 @@
-"""Tasks for managing Debug Information Files from Apple App Store Connect.
-
-Users can instruct Sentry to download dSYM from App Store Connect and put them into Sentry's
-debug files.  These tasks enable this functionality.
-"""
-
-from sentry.tasks.base import instrumented_task
-
-
-@instrumented_task(
-    name="sentry.tasks.app_store_connect.dsym_download", queue="appstoreconnect", ignore_result=True
-)
-def dsym_download(project_id: int, config_id: str) -> None:
-    # Noop
-    # TODO(@anonrig): Remove when AppStore connect integration is sunset.
-    return None
diff --git a/src/sentry/utils/sdk.py b/src/sentry/utils/sdk.py
index 39ced26f02c..7437dbeca1b 100644
--- a/src/sentry/utils/sdk.py
+++ b/src/sentry/utils/sdk.py
@@ -54,8 +54,6 @@ SAMPLED_TASKS = {
     "sentry.tasks.store.process_event": settings.SENTRY_PROCESS_EVENT_APM_SAMPLING,
     "sentry.tasks.store.process_event_from_reprocessing": settings.SENTRY_PROCESS_EVENT_APM_SAMPLING,
     "sentry.tasks.store.save_event_transaction": settings.SENTRY_PROCESS_EVENT_APM_SAMPLING,
-    # TODO(@anonrig): Remove this once AppStore connect integration is removed.
-    "sentry.tasks.app_store_connect.dsym_download": settings.SENTRY_APPCONNECT_APM_SAMPLING,
     "sentry.tasks.process_suspect_commits": settings.SENTRY_SUSPECT_COMMITS_APM_SAMPLING,
     "sentry.tasks.process_commit_context": settings.SENTRY_SUSPECT_COMMITS_APM_SAMPLING,
     "sentry.tasks.post_process.post_process_group": settings.SENTRY_POST_PROCESS_GROUP_APM_SAMPLING,

@sl0thentr0py sl0thentr0py removed the request for review from a team August 13, 2024 11:50
@getsantry
Copy link
Contributor

getsantry bot commented Sep 4, 2024

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you add the label WIP, I will leave it alone unless WIP is removed ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added the Stale label Sep 4, 2024
@markstory markstory self-assigned this Sep 5, 2024
@getsantry getsantry bot removed the Stale label Sep 6, 2024
Copy link
Contributor

github-actions bot commented Sep 6, 2024

This PR has a migration; here is the generated SQL for src/sentry/migrations/0758_remove_appstore_connect_integration_tables.py ()

--
-- Raw SQL operation
--
DROP TABLE IF EXISTS "sentry_latestappconnectbuildscheck";
--
-- Raw SQL operation
--
DROP TABLE IF EXISTS "sentry_appconnectbuild";

Copy link

codecov bot commented Sep 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #74100       +/-   ##
===========================================
- Coverage   88.39%   78.18%   -10.22%     
===========================================
  Files        2976     6910     +3934     
  Lines      185534   307265   +121731     
  Branches    30552    50320    +19768     
===========================================
+ Hits       164004   240227    +76223     
- Misses      15519    60597    +45078     
- Partials     6011     6441      +430     

@anonrig
Copy link
Contributor Author

anonrig commented Sep 6, 2024

@markstory thank you for following up with this pr

@markstory
Copy link
Member

@anonrig You're welcome ❤️

@markstory markstory added the Trigger: getsentry tests once code is reviewed: apply label to PR to trigger getsentry tests label Sep 9, 2024
Copy link
Contributor

github-actions bot commented Sep 9, 2024

This PR has a migration; here is the generated SQL for src/sentry/migrations/0759_remove_appstore_connect_integration_tables.py ()

--
-- Raw SQL operation
--
DROP TABLE IF EXISTS "sentry_latestappconnectbuildscheck";
--
-- Raw SQL operation
--
DROP TABLE IF EXISTS "sentry_appconnectbuild";

@github-actions github-actions bot removed the Trigger: getsentry tests once code is reviewed: apply label to PR to trigger getsentry tests label Sep 9, 2024
@markstory markstory added the Trigger: getsentry tests once code is reviewed: apply label to PR to trigger getsentry tests label Sep 9, 2024
@markstory markstory merged commit 9f1f2a9 into master Sep 9, 2024
51 of 52 checks passed
@markstory markstory deleted the remove-appstore-models branch September 9, 2024 17:04
@markstory markstory added the Trigger: Revert add to a merged PR to revert it (skips CI) label Sep 9, 2024
@getsentry-bot
Copy link
Contributor

PR reverted: b03a884

getsentry-bot added a commit that referenced this pull request Sep 9, 2024
This reverts commit 9f1f2a9.

Co-authored-by: markstory <24086+markstory@users.noreply.github.com>
markstory added a commit that referenced this pull request Sep 9, 2024
Mulligan on #74100 which failed to deploy because of k8s configuration
issues. Once those issues are resolved with getsentry/ops#12041 this
will be safe to merge again.

Revert "Revert "chore: sunset appstore connect integration data (#74100)""

This reverts commit b03a884.
c298lee pushed a commit that referenced this pull request Sep 10, 2024
This is the last remaining part of the AppStore connect integration.
This is a follow-up to #73960

It's official. Sentry sunsets the AppStore connect integration.

Ref: #51994

PS: Let's merge this once the previous PR lands production.

---------

Co-authored-by: Mark Story <mark@mark-story.com>
c298lee pushed a commit that referenced this pull request Sep 10, 2024
This reverts commit 9f1f2a9.

Co-authored-by: markstory <24086+markstory@users.noreply.github.com>
markstory added a commit that referenced this pull request Sep 16, 2024
Mulligan on #74100 which failed to deploy because of k8s configuration
issues. Once those issues are resolved with getsentry/ops#12041 this
will be safe to merge again.

Revert "Revert "chore: sunset appstore connect integration data
(#74100)""

This reverts commit b03a884.
@github-actions github-actions bot locked and limited conversation to collaborators Sep 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components Trigger: getsentry tests once code is reviewed: apply label to PR to trigger getsentry tests Trigger: Revert add to a merged PR to revert it (skips CI)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants