diff --git a/src/sentry/hybridcloud/apigateway/proxy.py b/src/sentry/hybridcloud/apigateway/proxy.py index a245755a1464ff..0661124e5564c8 100644 --- a/src/sentry/hybridcloud/apigateway/proxy.py +++ b/src/sentry/hybridcloud/apigateway/proxy.py @@ -5,6 +5,7 @@ from __future__ import annotations import logging +import random from collections.abc import Callable, Generator from http.cookiejar import Cookie from threading import local @@ -128,7 +129,12 @@ def proxy_cell_request(request: HttpRequest, cell: Cell, url_name: str) -> HttpR """Take a django request object and proxy it to a cell silo""" host = cell.address - if cell.api_gateway_address and in_random_rollout("apigateway.proxy.use_gateway_address"): + rollout_option = options.get("apigateway.proxy.cell-rollout") + if ( + cell.api_gateway_address + and isinstance(rollout_option, dict) + and random.random() < rollout_option.get(cell.name, 0.0) + ): host = cell.api_gateway_address metric_tags = { diff --git a/src/sentry/hybridcloud/apigateway_async/proxy.py b/src/sentry/hybridcloud/apigateway_async/proxy.py index 0c7c7b8909ba8a..86677db399275b 100644 --- a/src/sentry/hybridcloud/apigateway_async/proxy.py +++ b/src/sentry/hybridcloud/apigateway_async/proxy.py @@ -6,6 +6,7 @@ import asyncio import logging +import random from collections.abc import AsyncGenerator, AsyncIterator from urllib.parse import urljoin @@ -15,8 +16,8 @@ from django.http import HttpRequest, HttpResponse, JsonResponse, StreamingHttpResponse from django.http.response import HttpResponseBase +from sentry import options from sentry.objectstore.endpoints.organization import get_raw_body_async -from sentry.options.rollout import in_random_rollout from sentry.silo.util import ( PROXY_APIGATEWAY_HEADER, PROXY_DIRECT_LOCATION_HEADER, @@ -118,7 +119,12 @@ async def proxy_cell_request( ) -> HttpResponseBase: """Take a django request object and proxy it to a cell silo""" host = cell.address - if cell.api_gateway_address and in_random_rollout("apigateway.proxy.use_gateway_address"): + rollout_option = await sync_to_async(options.get)("apigateway.proxy.cell-rollout") + if ( + cell.api_gateway_address + and isinstance(rollout_option, dict) + and random.random() < rollout_option.get(cell.name, 0.0) + ): host = cell.api_gateway_address metric_tags = { diff --git a/src/sentry/hybridcloud/options.py b/src/sentry/hybridcloud/options.py index e858de4a14d500..6be2443acb507e 100644 --- a/src/sentry/hybridcloud/options.py +++ b/src/sentry/hybridcloud/options.py @@ -1,5 +1,5 @@ from sentry.options import FLAG_AUTOMATOR_MODIFIABLE, register -from sentry.utils.types import Bool, Dict, Float, Int, Sequence +from sentry.utils.types import Bool, Dict, Int, Sequence register( "outbox_replication.sentry_organizationmember.replication_version", @@ -202,14 +202,13 @@ flags=FLAG_AUTOMATOR_MODIFIABLE, ) register( - "apigateway.proxy.use_gateway_address", - type=Float, - default=0.0, - flags=FLAG_AUTOMATOR_MODIFIABLE, -) -register( - "hybridcloud.rpc.use_pooling_rate", - type=Float, - default=1.0, + "apigateway.proxy.cell-rollout", + type=Dict, + default={ + "us": 1.0, + "de": 0.0, + "us2": 0.0, + "s4s2": 0.0, + }, flags=FLAG_AUTOMATOR_MODIFIABLE, ) diff --git a/src/sentry/silo/client.py b/src/sentry/silo/client.py index a601e1f0888c47..d715e1bfb89a90 100644 --- a/src/sentry/silo/client.py +++ b/src/sentry/silo/client.py @@ -2,6 +2,7 @@ import ipaddress import logging +import random import socket from collections.abc import Mapping from hashlib import sha256 @@ -19,7 +20,6 @@ from sentry import options from sentry.http import build_session from sentry.net.http import SafeSession -from sentry.options.rollout import in_random_rollout from sentry.shared_integrations.client.base import BaseApiClient from sentry.silo.base import SiloMode from sentry.silo.util import ( @@ -121,8 +121,12 @@ def __init__(self, cell: Cell, retry: bool = False) -> None: # Ensure the cell is registered self.cell = get_cell_by_name(cell.name) self.base_url = self.cell.address - if self.cell.api_gateway_address and in_random_rollout( - "apigateway.proxy.use_gateway_address" + + gateway_rollout = options.get("apigateway.proxy.cell-rollout") + if ( + self.cell.api_gateway_address + and isinstance(gateway_rollout, dict) + and random.random() < gateway_rollout.get(cell.name, 0.0) ): self.base_url = self.cell.api_gateway_address self.retry = retry diff --git a/tests/sentry/hybridcloud/apigateway/test_proxy.py b/tests/sentry/hybridcloud/apigateway/test_proxy.py index 4702177d437956..5a44a82b55653f 100644 --- a/tests/sentry/hybridcloud/apigateway/test_proxy.py +++ b/tests/sentry/hybridcloud/apigateway/test_proxy.py @@ -407,7 +407,7 @@ def raise_connect_error(request: httpx.Request) -> NoReturn: @control_silo_test(cells=[api_gateway_address_cell], include_monolith_run=True) class ApiGatewayAddressProxyTestCase(ApiGatewayTestCase): @responses.activate - @override_options({"apigateway.proxy.use_gateway_address": 1.0}) + @override_options({"apigateway.proxy.cell-rollout": {"us": 1.0}}) def test_sync_post(self) -> None: responses.add( responses.POST, @@ -423,3 +423,39 @@ def test_sync_post(self) -> None: assert resp.status_code == 200 assert resp_json["test"] assert resp.has_header(PROXY_DIRECT_LOCATION_HEADER) + + @responses.activate + @override_options({"apigateway.proxy.cell-rollout": "lol"}) + def test_sync_post_corrupt_rollout_option(self) -> None: + responses.add( + responses.POST, + "http://sentry-rpc:8999/post", + body=json.dumps({"test": "value"}), + ) + request = RequestFactory().post( + "http://sentry.io/post", data={"test": "header"}, content_type="application/json" + ) + resp = sync_proxy.proxy_request(request, self.organization.slug, url_name) + resp_json = json.loads(close_streaming_response(resp)) + + assert resp.status_code == 200 + assert resp_json["test"] + assert resp.has_header(PROXY_DIRECT_LOCATION_HEADER) + + @responses.activate + @override_options({"apigateway.proxy.cell-rollout": {"nope": 1.0}}) + def test_sync_post_undefined_cell_in_option(self) -> None: + responses.add( + responses.POST, + "http://sentry-rpc:8999/post", + body=json.dumps({"test": "value"}), + ) + request = RequestFactory().post( + "http://sentry.io/post", data={"test": "header"}, content_type="application/json" + ) + resp = sync_proxy.proxy_request(request, self.organization.slug, url_name) + resp_json = json.loads(close_streaming_response(resp)) + + assert resp.status_code == 200 + assert resp_json["test"] + assert resp.has_header(PROXY_DIRECT_LOCATION_HEADER) diff --git a/tests/sentry/silo/test_client.py b/tests/sentry/silo/test_client.py index 539bc1f7c2c204..a9658a0ae4153d 100644 --- a/tests/sentry/silo/test_client.py +++ b/tests/sentry/silo/test_client.py @@ -351,7 +351,7 @@ def test_invalid_cell_silo_ip_address(self) -> None: assert err.args == ("Disallowed Cell Silo IP address: 172.31.255.31",) @override_settings(SILO_MODE=SiloMode.CONTROL) - @override_options({"apigateway.proxy.use_gateway_address": 1.0}) + @override_options({"apigateway.proxy.cell-rollout": {"us": 1.0}}) @responses.activate def test_cell_gateway_address_is_allowed(self) -> None: cell = Cell( @@ -386,6 +386,74 @@ def test_cell_gateway_address_is_allowed(self) -> None: assert mock_capture_exception.call_count == 0 + @override_settings(SILO_MODE=SiloMode.CONTROL) + @override_options({"apigateway.proxy.cell-rollout": {"nope": 1.0}}) + @responses.activate + def test_cell_gateway_address_option_key_undefined(self) -> None: + cell = Cell( + name="us", + snowflake_id=1, + address="http://10.2.0.88:9000", + api_gateway_address="http://10.2.0.66:9000", + ) + + responses.add( + responses.GET, + "http://10.2.0.88:9000/api/0/imaginary-public-endpoint/", + json={"ok": "88"}, + headers={"X-Some-Header": "Some-Value", PROXY_SIGNATURE_HEADER: "123"}, + ) + + # We're not mocking allowed_cell_silo_ip_addresses, so the cell attributes + # above are used. + with ( + override_cells((cell,)), + patch("sentry_sdk.capture_exception") as mock_capture_exception, + ): + client = CellSiloClient(cell) + assert client.base_url == "http://10.2.0.88:9000" + request = self.factory.get( + "/api/0/imaginary-public-endpoint/", HTTP_HOST="http://control.sentry.io" + ) + res = client.proxy_request(request) + assert res.content == b'{"ok": "88"}' + + assert mock_capture_exception.call_count == 0 + + @override_settings(SILO_MODE=SiloMode.CONTROL) + @override_options({"apigateway.proxy.cell-rollout": "oops"}) + @responses.activate + def test_cell_gateway_address_corrupt_option(self) -> None: + cell = Cell( + name="us", + snowflake_id=1, + address="http://10.2.0.88:9000", + api_gateway_address="http://10.2.0.66:9000", + ) + + responses.add( + responses.GET, + "http://10.2.0.88:9000/api/0/imaginary-public-endpoint/", + json={"ok": "88"}, + headers={"X-Some-Header": "Some-Value", PROXY_SIGNATURE_HEADER: "123"}, + ) + + # We're not mocking allowed_cell_silo_ip_addresses, so the cell attributes + # above are used. + with ( + override_cells((cell,)), + patch("sentry_sdk.capture_exception") as mock_capture_exception, + ): + client = CellSiloClient(cell) + assert client.base_url == "http://10.2.0.88:9000" + request = self.factory.get( + "/api/0/imaginary-public-endpoint/", HTTP_HOST="http://control.sentry.io" + ) + res = client.proxy_request(request) + assert res.content == b'{"ok": "88"}' + + assert mock_capture_exception.call_count == 0 + @override_settings(SILO_MODE=SiloMode.CONTROL) @override_allowed_cell_silo_ip_addresses("172.31.255.255") def test_client_restricted_ip_address(self) -> None: