feat(appconnect): Add performance instrumentation (#27469)

Instrument appstore connect operations with our own performance
monitoring.  This adds a bunch of spans which are useful as part of
observing the behaviour of the App Store Connect functionality.

This also allows separately instrumenting the task, so that we can
have good data even when we do not yet have many customers using it.

src/sentry/lang/native/appconnect.py

@@ -14,6 +14,7 @@ from typing import Any, Dict, List
 import dateutil
 import jsonschema
 import requests
+import sentry_sdk
 from django.db import transaction
 
 from sentry.lang.native.symbolicator import APP_STORE_CONNECT_SCHEMA
@@ -272,17 +273,18 @@ class ITunesClient:
         # itunes_connect.set_provider(self._session, itunes_org)
 
     def download_dsyms(self, build: BuildInfo, path: pathlib.Path) -> None:
-        url = itunes_connect.get_dsym_url(
-            self._session, build.app_id, build.version, build.build_number, build.platform
-        )
-        if not url:
-            raise NoDsymsError
-        logger.debug("Fetching dSYM from: %s", url)
-        with requests.get(url, stream=True) as req:
-            req.raise_for_status()
-            with open(path, "wb") as fp:
-                for chunk in req.iter_content(chunk_size=io.DEFAULT_BUFFER_SIZE):
-                    fp.write(chunk)
+        with sentry_sdk.start_span(op="dsyms", description="Download dSYMs"):
+            url = itunes_connect.get_dsym_url(
+                self._session, build.app_id, build.version, build.build_number, build.platform
+            )
+            if not url:
+                raise NoDsymsError
+            logger.debug("Fetching dSYM from: %s", url)
+            with requests.get(url, stream=True) as req:
+                req.raise_for_status()
+                with open(path, "wb") as fp:
+                    for chunk in req.iter_content(chunk_size=io.DEFAULT_BUFFER_SIZE):
+                        fp.write(chunk)
 
 
 class AppConnectClient:
@@ -346,7 +348,6 @@ class AppConnectClient:
 
     def list_builds(self) -> List[BuildInfo]:
         """Returns the available AppStore builds."""
-
         builds = []
         all_results = appstore_connect.get_build_info(
             self._session, self._api_credentials, self._app_id

src/sentry/tasks/app_store_connect.py

@@ -9,6 +9,8 @@ import pathlib
 import tempfile
 from typing import List, Mapping
 
+import sentry_sdk
+
 from sentry.lang.native import appconnect
 from sentry.models import AppConnectBuild, Project, ProjectOption, debugfile
 from sentry.tasks.base import instrumented_task
@@ -22,7 +24,7 @@ logger = logging.getLogger(__name__)
 # typing annotations.  So we do all the work outside of the decorated task function to work
 # around this.
 # Since all these args must be pickled we keep them to built-in types as well.
-@instrumented_task(name="sentry.tasks.app_store_connect.dsym_download", queue="appstoreconnect")  # type: ignore
+@instrumented_task(name="sentry.tasks.app_store_connect.dsym_download", queue="appstoreconnect", ignore_result=True)  # type: ignore
 def dsym_download(project_id: int, config_id: str) -> None:
     inner_dsym_download(project_id=project_id, config_id=config_id)
 
@@ -41,10 +43,14 @@ def inner_dsym_download(project_id: int, config_id: str) -> None:
 
     # persist all fetched builds into the database as "pending"
     builds = []
-    for build in client.list_builds():
-        build_state = get_or_create_persisted_build(project, config, build)
-        if not build_state.fetched:
-            builds.append((build, build_state))
+    listed_builds = client.list_builds()
+    with sentry_sdk.start_span(
+        op="appconnect-update-builds", description="Update AppStoreConnect builds in database"
+    ):
+        for build in listed_builds:
+            build_state = get_or_create_persisted_build(project, config, build)
+            if not build_state.fetched:
+                builds.append((build, build_state))
 
     itunes_client = client.itunes_client()
     for (build, build_state) in builds:
@@ -71,17 +77,17 @@ def inner_dsym_download(project_id: int, config_id: str) -> None:
 
 
 def create_difs_from_dsyms_zip(dsyms_zip: str, project: Project) -> None:
-    with open(dsyms_zip, "rb") as fp:
-        created = debugfile.create_files_from_dif_zip(fp, project, accept_unknown=True)
-        for proj_debug_file in created:
-            logger.debug("Created %r for project %s", proj_debug_file, project.id)
+    with sentry_sdk.start_span(op="dsym-difs", description="Extract difs dSYM zip"):
+        with open(dsyms_zip, "rb") as fp:
+            created = debugfile.create_files_from_dif_zip(fp, project, accept_unknown=True)
+            for proj_debug_file in created:
+                logger.debug("Created %r for project %s", proj_debug_file, project.id)
 
 
 def get_or_create_persisted_build(
     project: Project, config: appconnect.AppStoreConnectConfig, build: appconnect.BuildInfo
 ) -> AppConnectBuild:
-    """
-    Fetches the sentry-internal :class:`AppConnectBuild`.
+    """Fetches the sentry-internal :class:`AppConnectBuild`.
 
     The build corresponds to the :class:`appconnect.BuildInfo` as returned by the
     AppStore Connect API. If no build exists yet, a new "pending" build is created.
@@ -112,7 +118,9 @@ def get_or_create_persisted_build(
 # Untyped decorator would stop type-checking of entire function, split into an inner
 # function instead which can be type checked.
 @instrumented_task(  # type: ignore
-    name="sentry.tasks.app_store_connect.refresh_all_builds", queue="appstoreconnect"
+    name="sentry.tasks.app_store_connect.refresh_all_builds",
+    queue="appstoreconnect",
+    ignore_result=True,
 )
 def refresh_all_builds() -> None:
     inner_refresh_all_builds()

src/sentry/utils/appleconnect/appstore_connect.py

@@ -3,6 +3,7 @@ import time
 from collections import namedtuple
 from typing import Any, Dict, Generator, List, Mapping, Optional, Union
 
+import sentry_sdk
 from requests import Session
 
 from sentry.utils import jwt, safe
@@ -27,17 +28,18 @@ def _get_authorization_header(
     """
     if expiry_sec is None:
         expiry_sec = 60 * 10  # default to 10 mins
-    token = jwt.encode(
-        {
-            "iss": credentials.issuer_id,
-            "exp": int(time.time()) + expiry_sec,
-            "aud": "appstoreconnect-v1",
-        },
-        credentials.key,
-        algorithm="ES256",
-        headers={"kid": credentials.key_id, "alg": "ES256", "typ": "JWT"},
-    )
-    return jwt.authorization_header(token)
+    with sentry_sdk.start_span(op="jwt", description="Generating AppStoreConnect JWT token"):
+        token = jwt.encode(
+            {
+                "iss": credentials.issuer_id,
+                "exp": int(time.time()) + expiry_sec,
+                "aud": "appstoreconnect-v1",
+            },
+            credentials.key,
+            algorithm="ES256",
+            headers={"kid": credentials.key_id, "alg": "ES256", "typ": "JWT"},
+        )
+        return jwt.authorization_header(token)
 
 
 def _get_appstore_json(
@@ -52,25 +54,27 @@ def _get_appstore_json(
     :raises ValueError: if the request failed or the response body could not be parsed as
        JSON.
     """
-    headers = _get_authorization_header(credentials)
-
-    if not url.startswith("https://"):
-        full_url = "https://api.appstoreconnect.apple.com"
-        if url[0] != "/":
-            full_url += "/"
-    else:
-        full_url = ""
-    full_url += url
-    logger.debug(f"GET {full_url}")
-    response = session.get(full_url, headers=headers)
-    if not response.ok:
-        raise ValueError("Request failed", full_url, response.status_code, response.text)
-    try:
-        return response.json()  # type: ignore
-    except Exception as e:
-        raise ValueError(
-            "Response body not JSON", full_url, response.status_code, response.text
-        ) from e
+    with sentry_sdk.start_span(op="appconnect-request", description="AppStoreConnect API request"):
+        headers = _get_authorization_header(credentials)
+
+        if not url.startswith("https://"):
+            full_url = "https://api.appstoreconnect.apple.com"
+            if url[0] != "/":
+                full_url += "/"
+        else:
+            full_url = ""
+        full_url += url
+        logger.debug(f"GET {full_url}")
+        with sentry_sdk.start_span(op="http", description="AppStoreConnect request"):
+            response = session.get(full_url, headers=headers)
+        if not response.ok:
+            raise ValueError("Request failed", full_url, response.status_code, response.text)
+        try:
+            return response.json()  # type: ignore
+        except Exception as e:
+            raise ValueError(
+                "Response body not JSON", full_url, response.status_code, response.text
+            ) from e
 
 
 def _get_next_page(response_json: Mapping[str, Any]) -> Optional[str]:
@@ -116,87 +120,90 @@ def get_build_info(
        in starship documentation
     build_number: str - the version of the build (e.g. '101'), looks like the build number
     """
-    # https://developer.apple.com/documentation/appstoreconnectapi/list_builds
-    url = (
-        f"v1/builds?filter[app]={app_id}"
-        # we can fetch a maximum of 200 builds at once, so do that
-        "&limit=200"
-        # include related AppStore/PreRelease versions with the response
-        # NOTE: the `iris` web API has related `buildBundles` objects,
-        # which have very useful `includesSymbols` and `dSYMUrl` attributes,
-        # but this is sadly not available in the official API. :-(
-        # Open this in your browser when you are signed into AppStoreConnect:
-        # https://appstoreconnect.apple.com/iris/v1/builds?filter[processingState]=VALID&include=appStoreVersion,preReleaseVersion,buildBundles&limit=1&filter[app]=XYZ
-        "&include=appStoreVersion,preReleaseVersion"
-        # sort newer releases first
-        "&sort=-uploadedDate"
-        # only include valid builds
-        "&filter[processingState]=VALID"
-        # and builds that have not expired yet
-        "&filter[expired]=false"
-    )
-    pages = _get_appstore_info_paged(session, credentials, url)
-    result = []
-
-    for page in pages:
-
-        # Collect the related data sent in this page so we can look it up by (type, id).
-        included_relations = {}
-        for relation in page["included"]:
-            rel_type = relation["type"]
-            rel_id = relation["id"]
-            included_relations[(rel_type, rel_id)] = relation
-
-        def get_related(data: JSONData, relation: str) -> Union[None, JSONData]:
-            """Returns related data by looking it up in all the related data included in the page.
-
-            This first looks up the related object in the data provided under the
-            `relationships` key.  Then it uses the `type` and `id` of this key to look up
-            the actual data in `included_relations` which is an index of all related data
-            returned with the page.
-
-            If the `relation` does not exist in `data` then `None` is returned.
-            """
-            rel_ptr_data = safe.get_path(data, "relationships", relation, "data")
-            if rel_ptr_data is None:
-                # The query asks for both the appStoreVersion and preReleaseVersion
-                # relations to be included.  However for each build there could be only one
-                # of these that will have the data with type and id, the other will have
-                # None for data.
-                return None
-            rel_type = rel_ptr_data["type"]
-            rel_id = rel_ptr_data["id"]
-            return included_relations[(rel_type, rel_id)]
-
-        for build in page["data"]:
-            try:
-                related_appstore_version = get_related(build, "appStoreVersion")
-                related_prerelease_version = get_related(build, "preReleaseVersion")
-
-                # Normally release versions also have a matching prerelease version, the
-                # platform and version number for them should be identical.  Nevertheless
-                # because we would likely see the build first with a prerelease version
-                # before it also has a release version we prefer to stick with that one if
-                # it is available.
-                if related_prerelease_version:
-                    version = related_prerelease_version["attributes"]["version"]
-                    platform = related_prerelease_version["attributes"]["platform"]
-                elif related_appstore_version:
-                    version = related_appstore_version["attributes"]["versionString"]
-                    platform = related_appstore_version["attributes"]["platform"]
-                else:
-                    raise KeyError("missing related version")
-                build_number = build["attributes"]["version"]
-
-                result.append(
-                    {"platform": platform, "version": version, "build_number": build_number}
-                )
-            except Exception:
-                logger.error(
-                    "Failed to process AppStoreConnect build from API: %s", build, exc_info=True
-                )
-
-    return result
+    with sentry_sdk.start_span(
+        op="appconnect-list-builds", description="List all AppStoreConnect builds"
+    ):
+        # https://developer.apple.com/documentation/appstoreconnectapi/list_builds
+        url = (
+            f"v1/builds?filter[app]={app_id}"
+            # we can fetch a maximum of 200 builds at once, so do that
+            "&limit=200"
+            # include related AppStore/PreRelease versions with the response
+            # NOTE: the `iris` web API has related `buildBundles` objects,
+            # which have very useful `includesSymbols` and `dSYMUrl` attributes,
+            # but this is sadly not available in the official API. :-(
+            # Open this in your browser when you are signed into AppStoreConnect:
+            # https://appstoreconnect.apple.com/iris/v1/builds?filter[processingState]=VALID&include=appStoreVersion,preReleaseVersion,buildBundles&limit=1&filter[app]=XYZ
+            "&include=appStoreVersion,preReleaseVersion"
+            # sort newer releases first
+            "&sort=-uploadedDate"
+            # only include valid builds
+            "&filter[processingState]=VALID"
+            # and builds that have not expired yet
+            "&filter[expired]=false"
+        )
+        pages = _get_appstore_info_paged(session, credentials, url)
+        result = []
+
+        for page in pages:
+
+            # Collect the related data sent in this page so we can look it up by (type, id).
+            included_relations = {}
+            for relation in page["included"]:
+                rel_type = relation["type"]
+                rel_id = relation["id"]
+                included_relations[(rel_type, rel_id)] = relation
+
+            def get_related(data: JSONData, relation: str) -> Union[None, JSONData]:
+                """Returns related data by looking it up in all the related data included in the page.
+
+                This first looks up the related object in the data provided under the
+                `relationships` key.  Then it uses the `type` and `id` of this key to look up
+                the actual data in `included_relations` which is an index of all related data
+                returned with the page.
+
+                If the `relation` does not exist in `data` then `None` is returned.
+                """
+                rel_ptr_data = safe.get_path(data, "relationships", relation, "data")
+                if rel_ptr_data is None:
+                    # The query asks for both the appStoreVersion and preReleaseVersion
+                    # relations to be included.  However for each build there could be only one
+                    # of these that will have the data with type and id, the other will have
+                    # None for data.
+                    return None
+                rel_type = rel_ptr_data["type"]
+                rel_id = rel_ptr_data["id"]
+                return included_relations[(rel_type, rel_id)]
+
+            for build in page["data"]:
+                try:
+                    related_appstore_version = get_related(build, "appStoreVersion")
+                    related_prerelease_version = get_related(build, "preReleaseVersion")
+
+                    # Normally release versions also have a matching prerelease version, the
+                    # platform and version number for them should be identical.  Nevertheless
+                    # because we would likely see the build first with a prerelease version
+                    # before it also has a release version we prefer to stick with that one if
+                    # it is available.
+                    if related_prerelease_version:
+                        version = related_prerelease_version["attributes"]["version"]
+                        platform = related_prerelease_version["attributes"]["platform"]
+                    elif related_appstore_version:
+                        version = related_appstore_version["attributes"]["versionString"]
+                        platform = related_appstore_version["attributes"]["platform"]
+                    else:
+                        raise KeyError("missing related version")
+                    build_number = build["attributes"]["version"]
+
+                    result.append(
+                        {"platform": platform, "version": version, "build_number": build_number}
+                    )
+                except Exception:
+                    logger.error(
+                        "Failed to process AppStoreConnect build from API: %s", build, exc_info=True
+                    )
+
+        return result
 
 
 AppInfo = namedtuple("AppInfo", ["name", "bundle_id", "app_id"])

src/sentry/utils/appleconnect/itunes_connect.py

@@ -8,6 +8,7 @@ from collections import namedtuple
 from http import HTTPStatus
 from typing import Any, NewType, Optional
 
+import sentry_sdk
 from requests import Session
 
 from sentry.utils import safe
@@ -315,29 +316,36 @@ def get_dsym_url(
     Returns the url for a dsyms bundle. The session must be logged in.
     :return: The url to use for downloading the dsyms bundle
     """
-    details_url = (
-        f"https://appstoreconnect.apple.com/WebObjects/iTunesConnect.woa/ra/apps/"
-        f"{app_id}/platforms/{platform}/trains/{bundle_short_version}/builds/"
-        f"{bundle_version}/details"
-    )
-
-    logger.debug(f"GET {details_url}")
-
-    details_response = session.get(details_url)
+    with sentry_sdk.start_span(
+        op="itunes-dsym-url", description="Request iTunes dSYM download URL"
+    ):
+        details_url = (
+            f"https://appstoreconnect.apple.com/WebObjects/iTunesConnect.woa/ra/apps/"
+            f"{app_id}/platforms/{platform}/trains/{bundle_short_version}/builds/"
+            f"{bundle_version}/details"
+        )
 
-    # A non-OK status code will probably mean an expired token/session
-    if details_response.status_code == HTTPStatus.UNAUTHORIZED:
-        raise ITunesSessionExpiredException
-    if details_response.status_code == HTTPStatus.OK:
-        try:
-            data = details_response.json()
-            dsym_url = safe.get_path(data, "data", "dsymurl")
-            return dsym_url  # type: ignore
-        except Exception as e:
-            logger.info(
-                f"Could not obtain dSYM info for app id={app_id}, bundle_short={bundle_short_version}, "
-                f"bundle={bundle_version}, platform={platform}",
-                exc_info=True,
-            )
-            raise e
-    return None
+        logger.debug(f"GET {details_url}")
+
+        details_response = session.get(details_url)
+
+        # A non-OK status code will probably mean an expired token/session
+        if details_response.status_code == HTTPStatus.UNAUTHORIZED:
+            raise ITunesSessionExpiredException
+        if details_response.status_code == HTTPStatus.OK:
+            try:
+                data = details_response.json()
+                dsym_url: Optional[str] = safe.get_path(data, "data", "dsymurl")
+                return dsym_url
+            except Exception as e:
+                logger.info(
+                    "Could not obtain dSYM info for "
+                    "app id=%s, bundle_short=%s, bundle=%s, platform=%s",
+                    app_id,
+                    bundle_short_version,
+                    bundle_version,
+                    platform,
+                    exc_info=True,
+                )
+                raise e
+        return None

src/sentry/utils/sdk.py

@@ -69,6 +69,7 @@ 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.assemble.assemble_dif": 0.1,
+    "sentry.tasks.app_store_connect.dsym_download": 1,
 }