Skip to content

fix(cells) Add rollout option and support for apigateway deployments #118098

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.

Sign up for GitHub

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

Open
markstory wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(cells) Add rollout option and support for apigateway deployments #118098

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/sentry/hybridcloud/apigateway/proxy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = {
Expand Down
10 changes: 8 additions & 2 deletions src/sentry/hybridcloud/apigateway_async/proxy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import asyncio
import logging
import random
from collections.abc import AsyncGenerator, AsyncIterator
from urllib.parse import urljoin

Expand All @@ -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,
Expand Down Expand Up @@ -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 = {
Expand Down
19 changes: 9 additions & 10 deletions src/sentry/hybridcloud/options.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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",
Expand Down Expand Up @@ -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,

@cursor cursor Bot Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

RPC pooling option accidentally deleted

High Severity

Replacing apigateway.proxy.use_gateway_address removed the register call for hybridcloud.rpc.use_pooling_rate, but _get_connection in hybridcloud/rpc/service.py still calls in_random_rollout for that key. options.get then raises UnknownOption on cross-silo RPC traffic.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c22f330. Configure here.

"apigateway.proxy.cell-rollout",
type=Dict,
default={
"us": 1.0,
"de": 0.0,
"us2": 0.0,
"s4s2": 0.0,
},
flags=FLAG_AUTOMATOR_MODIFIABLE,
)
Comment on lines 204 to 214

@sentry sentry Bot Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The removed option hybridcloud.rpc.use_pooling_rate is still in use, causing in_random_rollout to raise an unhandled UnknownOption error on every RPC call.
Severity: CRITICAL

Suggested Fix

Re-add the registration for the hybridcloud.rpc.use_pooling_rate option in src/sentry/hybridcloud/options.py. If this option is no longer needed, remove the corresponding call to in_random_rollout("hybridcloud.rpc.use_pooling_rate") from src/sentry/hybridcloud/rpc/service.py.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/sentry/hybridcloud/options.py#L202-L214

Potential issue: The option `hybridcloud.rpc.use_pooling_rate` was removed from the
options registry in `src/sentry/hybridcloud/options.py`, but it is still referenced in
`src/sentry/hybridcloud/rpc/service.py` through a call to `in_random_rollout`. This
function internally calls `options.get()` without a `silent` parameter, which raises an
`UnknownOption` exception for unregistered options. Because this exception is not
handled in the call stack, every remote RPC call will crash, causing a major failure in
inter-silo communication.

Also affects:

  • src/sentry/hybridcloud/rpc/service.py:527

Did we get this right? 👍 / 👎 to inform future reviews.

10 changes: 7 additions & 3 deletions src/sentry/silo/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import ipaddress
import logging
import random
import socket
from collections.abc import Mapping
from hashlib import sha256
Expand All @@ -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 (
Expand Down Expand Up @@ -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
Expand Down
38 changes: 37 additions & 1 deletion tests/sentry/hybridcloud/apigateway/test_proxy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
70 changes: 69 additions & 1 deletion tests/sentry/silo/test_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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:
Expand Down
Loading