Browse Source

fix(hc): Provide region specific urls for chunk-upload config (#66320)

Gabe Villalobos 1 year ago
parent
commit
71a31d9695

+ 7 - 1
src/sentry/api/endpoints/chunk.py

@@ -14,6 +14,7 @@ from sentry.api.api_owners import ApiOwner
 from sentry.api.api_publish_status import ApiPublishStatus
 from sentry.api.base import region_silo_endpoint
 from sentry.api.bases.organization import OrganizationEndpoint, OrganizationReleasePermission
+from sentry.api.utils import generate_region_url
 from sentry.models.files.fileblob import FileBlob
 from sentry.ratelimits.config import RateLimitConfig
 from sentry.utils.files import get_max_file_size
@@ -81,7 +82,12 @@ class ChunkUploadEndpoint(OrganizationEndpoint):
                 url = relative_url.lstrip(API_PREFIX)
             # Otherwise, if we do not support them, return an absolute, versioned endpoint with a default, system-wide prefix
             else:
-                url = absolute_uri(relative_url)
+                # We need to generate region specific upload URLs when possible to avoid hitting the API proxy
+                # which tends to cause timeouts and performance issues for uploads.
+                base_url = None
+                if options.get("hybrid_cloud.use_region_specific_upload_url"):
+                    base_url = generate_region_url()
+                url = absolute_uri(relative_url, base_url)
         else:
             # If user overridden upload url prefix, we want an absolute, versioned endpoint, with user-configured prefix
             url = absolute_uri(relative_url, endpoint)

+ 4 - 0
src/sentry/options/defaults.py

@@ -1653,6 +1653,10 @@ register("hybrid_cloud.multi-region-selector", default=False, flags=FLAG_AUTOMAT
 register("hybrid_cloud.region-domain-allow-list", default=[], flags=FLAG_AUTOMATOR_MODIFIABLE)
 register("hybrid_cloud.region-user-allow-list", default=[], flags=FLAG_AUTOMATOR_MODIFIABLE)
 
+register(
+    "hybrid_cloud.use_region_specific_upload_url", default=False, flags=FLAG_AUTOMATOR_MODIFIABLE
+)
+
 # Retry controls
 register("hybridcloud.regionsiloclient.retries", default=5, flags=FLAG_AUTOMATOR_MODIFIABLE)
 register("hybridcloud.rpc.retries", default=5, flags=FLAG_AUTOMATOR_MODIFIABLE)

+ 40 - 13
tests/sentry/api/endpoints/test_chunk_upload.py

@@ -15,12 +15,14 @@ from sentry.api.endpoints.chunk import (
     MAX_CONCURRENCY,
     MAX_REQUEST_SIZE,
 )
+from sentry.api.utils import generate_region_url
 from sentry.models.apitoken import ApiToken
 from sentry.models.files.fileblob import FileBlob
 from sentry.models.files.utils import MAX_FILE_SIZE
 from sentry.models.organization import Organization
 from sentry.silo import SiloMode
 from sentry.testutils.cases import APITestCase
+from sentry.testutils.helpers import override_options
 from sentry.testutils.silo import assume_test_silo_mode, region_silo_test
 
 
@@ -51,12 +53,18 @@ class ChunkUploadTest(APITestCase):
         assert response.data["url"] == options.get("system.url-prefix") + self.url
         assert response.data["accept"] == CHUNK_UPLOAD_ACCEPT
 
-        options.set("system.upload-url-prefix", "test")
-        response = self.client.get(
-            self.url, HTTP_AUTHORIZATION=f"Bearer {self.token.token}", format="json"
-        )
+        with override_options({"system.upload-url-prefix": "test"}):
+            response = self.client.get(
+                self.url, HTTP_AUTHORIZATION=f"Bearer {self.token.token}", format="json"
+            )
 
-        assert response.data["url"] == options.get("system.upload-url-prefix") + self.url
+            assert response.data["url"] == options.get("system.upload-url-prefix") + self.url
+
+        with override_options({"hybrid_cloud.use_region_specific_upload_url": True}):
+            response = self.client.get(
+                self.url, HTTP_AUTHORIZATION=f"Bearer {self.token.token}", format="json"
+            )
+            assert response.data["url"] == generate_region_url() + self.url
 
     def test_accept_with_artifact_bundles_v2_option(self):
         with self.options({"sourcemaps.artifact_bundles.assemble_with_missing_chunks": False}):
@@ -123,15 +131,34 @@ class ChunkUploadTest(APITestCase):
         )
         assert response.data["url"] == options.get("system.url-prefix") + self.url
 
+        # Test region upload URLs with option set
+        with override_options({"hybrid_cloud.use_region_specific_upload_url": True}):
+            # < 1.70.1
+            response = self.client.get(
+                self.url,
+                HTTP_AUTHORIZATION=f"Bearer {self.token.token}",
+                HTTP_USER_AGENT="sentry-cli/1.70.0",
+                format="json",
+            )
+            assert response.data["url"] == generate_region_url() + self.url
+
+            response = self.client.get(
+                self.url,
+                HTTP_AUTHORIZATION=f"Bearer {self.token.token}",
+                HTTP_USER_AGENT="sentry-cli/0.69.3",
+                format="json",
+            )
+            assert response.data["url"] == generate_region_url() + self.url
+
         # user overridden upload url prefix has priority, even when calling from sentry-cli that supports relative urls
-        options.set("system.upload-url-prefix", "test")
-        response = self.client.get(
-            self.url,
-            HTTP_AUTHORIZATION=f"Bearer {self.token.token}",
-            HTTP_USER_AGENT="sentry-cli/1.70.1",
-            format="json",
-        )
-        assert response.data["url"] == options.get("system.upload-url-prefix") + self.url
+        with override_options({"system.upload-url-prefix": "test"}):
+            response = self.client.get(
+                self.url,
+                HTTP_AUTHORIZATION=f"Bearer {self.token.token}",
+                HTTP_USER_AGENT="sentry-cli/1.70.1",
+                format="json",
+            )
+            assert response.data["url"] == options.get("system.upload-url-prefix") + self.url
 
     def test_large_uploads(self):
         with self.feature("organizations:large-debug-files"):