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

feat(uptime): Accept timeout customization in uptime APIs #78788

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions src/sentry/uptime/endpoints/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ class UptimeMonitorValidator(CamelSnakeSerializer):
interval_seconds = serializers.ChoiceField(
required=True, choices=[int(i.total_seconds()) for i in VALID_INTERVALS]
)
timeout_ms = serializers.IntegerField(
required=True,
min_value=1000,
max_value=30 * 1000,
)
mode = serializers.IntegerField(required=False)
method = serializers.ChoiceField(
required=False, choices=list(zip(SUPPORTED_HTTP_METHODS, SUPPORTED_HTTP_METHODS))
Expand Down Expand Up @@ -172,6 +177,7 @@ def create(self, validated_data):
environment=environment,
url=validated_data["url"],
interval_seconds=validated_data["interval_seconds"],
timeout_ms=validated_data["timeout_ms"],
name=validated_data["name"],
mode=validated_data.get("mode", ProjectUptimeSubscriptionMode.MANUAL),
owner=validated_data.get("owner"),
Expand Down Expand Up @@ -201,6 +207,9 @@ def update(self, instance: ProjectUptimeSubscription, data):
if "interval_seconds" in data
else instance.uptime_subscription.interval_seconds
)
timeout_ms = (
data["timeout_ms"] if "timeout_ms" in data else instance.uptime_subscription.timeout_ms
)
method = data["method"] if "method" in data else instance.uptime_subscription.method
headers = data["headers"] if "headers" in data else instance.uptime_subscription.headers
body = data["body"] if "body" in data else instance.uptime_subscription.body
Expand All @@ -223,6 +232,7 @@ def update(self, instance: ProjectUptimeSubscription, data):
environment=environment,
url=url,
interval_seconds=interval_seconds,
timeout_ms=timeout_ms,
method=method,
headers=headers,
body=body,
Expand Down
21 changes: 13 additions & 8 deletions src/sentry/uptime/subscriptions/subscriptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@
UPTIME_SUBSCRIPTION_TYPE = "uptime_monitor"
MAX_AUTO_SUBSCRIPTIONS_PER_ORG = 1
MAX_MANUAL_SUBSCRIPTIONS_PER_ORG = 100
# Default timeout for all subscriptions
DEFAULT_SUBSCRIPTION_TIMEOUT_MS = 10000


class MaxManualUptimeSubscriptionsReached(ValueError):
Expand All @@ -39,14 +37,18 @@ class MaxManualUptimeSubscriptionsReached(ValueError):
def retrieve_uptime_subscription(
url: str,
interval_seconds: int,
timeout_ms: int,
method: str,
headers: Sequence[tuple[str, str]],
body: str | None,
) -> UptimeSubscription | None:
try:
subscription = (
UptimeSubscription.objects.filter(
url=url, interval_seconds=interval_seconds, method=method
url=url,
interval_seconds=interval_seconds,
timeout_ms=timeout_ms,
method=method,
)
.annotate(
headers_md5=MD5("headers", output_field=TextField()),
Expand All @@ -66,7 +68,7 @@ def retrieve_uptime_subscription(
def get_or_create_uptime_subscription(
url: str,
interval_seconds: int,
timeout_ms: int = DEFAULT_SUBSCRIPTION_TIMEOUT_MS,
timeout_ms: int,
method: str = "GET",
headers: Sequence[tuple[str, str]] | None = None,
body: str | None = None,
Expand All @@ -81,7 +83,9 @@ def get_or_create_uptime_subscription(
# domain.
result = extract_domain_parts(url)

subscription = retrieve_uptime_subscription(url, interval_seconds, method, headers, body)
subscription = retrieve_uptime_subscription(
url, interval_seconds, timeout_ms, method, headers, body
)
created = False

if subscription is None:
Expand All @@ -102,7 +106,7 @@ def get_or_create_uptime_subscription(
except IntegrityError:
# Handle race condition where we tried to retrieve an existing subscription while it was being created
subscription = retrieve_uptime_subscription(
url, interval_seconds, method, headers, body
url, interval_seconds, timeout_ms, method, headers, body
)

if subscription is None:
Expand Down Expand Up @@ -146,7 +150,7 @@ def get_or_create_project_uptime_subscription(
environment: Environment | None,
url: str,
interval_seconds: int,
timeout_ms: int = DEFAULT_SUBSCRIPTION_TIMEOUT_MS,
timeout_ms: int,
method: str = "GET",
headers: Sequence[tuple[str, str]] | None = None,
body: str | None = None,
Expand Down Expand Up @@ -190,6 +194,7 @@ def update_project_uptime_subscription(
environment: Environment | None,
url: str,
interval_seconds: int,
timeout_ms: int,
method: str,
headers: Sequence[tuple[str, str]],
body: str | None,
Expand All @@ -201,7 +206,7 @@ def update_project_uptime_subscription(
"""
cur_uptime_subscription = uptime_monitor.uptime_subscription
new_uptime_subscription = get_or_create_uptime_subscription(
url, interval_seconds, cur_uptime_subscription.timeout_ms, method, headers, body
url, interval_seconds, timeout_ms, method, headers, body
)
updated_subscription = cur_uptime_subscription.id != new_uptime_subscription.id

Expand Down
4 changes: 2 additions & 2 deletions static/app/views/monitors/components/detailsTimeline.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type {Organization} from 'sentry/types/organization';
import {setApiQueryData, useQueryClient} from 'sentry/utils/queryClient';
import useApi from 'sentry/utils/useApi';
import {useDimensions} from 'sentry/utils/useDimensions';
import useRouter from 'sentry/utils/useRouter';
import {useLocation} from 'sentry/utils/useLocation';
import type {Monitor} from 'sentry/views/monitors/types';
import {makeMonitorDetailsQueryKey} from 'sentry/views/monitors/utils';

Expand All @@ -27,7 +27,7 @@ interface Props {
}

export function DetailsTimeline({monitor, organization}: Props) {
const {location} = useRouter();
const location = useLocation();
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated change?

Copy link
Member Author

Choose a reason for hiding this comment

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

oops

const api = useApi();
const queryClient = useQueryClient();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ def test_all(self):
owner=f"user:{self.user.id}",
url="https://santry.io",
interval_seconds=300,
timeoout_ms=1500,
headers=[["hello", "world"]],
body="something",
)
Expand All @@ -56,6 +57,7 @@ def test_all(self):
uptime_sub.refresh_from_db()
assert uptime_sub.url == "https://santry.io"
assert uptime_sub.interval_seconds == 300
assert uptime_sub.timeout_ms == 1500
assert uptime_sub.headers == [["hello", "world"]]
assert uptime_sub.body == "something"

Expand All @@ -67,6 +69,7 @@ def test_all(self):
owner=f"user:{self.user.id}",
url="https://santry.io",
interval_seconds=300,
timeoout_ms=1500,
headers=[["hello", "world"]],
body=None,
)
Expand All @@ -79,6 +82,7 @@ def test_all(self):
uptime_sub.refresh_from_db()
assert uptime_sub.url == "https://santry.io"
assert uptime_sub.interval_seconds == 300
assert uptime_sub.timeout_ms == 1500
assert uptime_sub.headers == [["hello", "world"]]
assert uptime_sub.body is None

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from sentry.testutils.helpers import with_feature
from sentry.uptime.endpoints.validators import MAX_REQUEST_SIZE_BYTES
from sentry.uptime.models import ProjectUptimeSubscription, ProjectUptimeSubscriptionMode
from sentry.uptime.subscriptions.subscriptions import DEFAULT_SUBSCRIPTION_TIMEOUT_MS
from tests.sentry.uptime.endpoints import UptimeAlertBaseEndpointTest


Expand All @@ -25,6 +24,7 @@ def test_no_feature(self):
owner=f"user:{self.user.id}",
url="http://sentry.io",
interval_seconds=60,
timeout_ms=1000,
status_code=404,
)

Expand All @@ -38,6 +38,7 @@ def test(self):
owner=f"user:{self.user.id}",
url="http://sentry.io",
interval_seconds=60,
timeout_ms=1500,
body=None,
)
uptime_monitor = ProjectUptimeSubscription.objects.get(id=resp.data["id"])
Expand All @@ -51,7 +52,7 @@ def test(self):
assert uptime_monitor.mode == ProjectUptimeSubscriptionMode.MANUAL
assert uptime_subscription.url == "http://sentry.io"
assert uptime_subscription.interval_seconds == 60
assert uptime_subscription.timeout_ms == DEFAULT_SUBSCRIPTION_TIMEOUT_MS
assert uptime_subscription.timeout_ms == 1500
assert uptime_subscription.body is None

@with_feature("organizations:uptime-api-create-update")
Expand All @@ -63,6 +64,7 @@ def test_no_environment(self):
owner=f"user:{self.user.id}",
url="http://sentry.io",
interval_seconds=60,
timeout_ms=1000,
body=None,
)
uptime_monitor = ProjectUptimeSubscription.objects.get(id=resp.data["id"])
Expand All @@ -74,7 +76,7 @@ def test_no_environment(self):
assert uptime_monitor.mode == ProjectUptimeSubscriptionMode.MANUAL
assert uptime_subscription.url == "http://sentry.io"
assert uptime_subscription.interval_seconds == 60
assert uptime_subscription.timeout_ms == DEFAULT_SUBSCRIPTION_TIMEOUT_MS
assert uptime_subscription.timeout_ms == 1000
assert uptime_subscription.body is None

@with_feature("organizations:uptime-api-create-update")
Expand All @@ -87,6 +89,7 @@ def test_no_owner(self):
url="http://sentry.io",
owner=None,
interval_seconds=60,
timeout_ms=1000,
)
uptime_monitor = ProjectUptimeSubscription.objects.get(id=resp.data["id"])
uptime_subscription = uptime_monitor.uptime_subscription
Expand All @@ -96,7 +99,7 @@ def test_no_owner(self):
assert uptime_monitor.mode == ProjectUptimeSubscriptionMode.MANUAL
assert uptime_subscription.url == "http://sentry.io"
assert uptime_subscription.interval_seconds == 60
assert uptime_subscription.timeout_ms == DEFAULT_SUBSCRIPTION_TIMEOUT_MS
assert uptime_subscription.timeout_ms == 1000

# Test without passing the owner
resp = self.get_success_response(
Expand All @@ -106,6 +109,7 @@ def test_no_owner(self):
name="test",
url="http://getsentry.com",
interval_seconds=60,
timeout_ms=1000,
)
uptime_monitor = ProjectUptimeSubscription.objects.get(id=resp.data["id"])
assert uptime_monitor.owner_user_id is None
Expand All @@ -121,6 +125,7 @@ def test_mode_no_superadmin(self):
owner=f"user:{self.user.id}",
url="http://sentry.io",
interval_seconds=60,
timeout_ms=1000,
mode=ProjectUptimeSubscriptionMode.AUTO_DETECTED_ACTIVE,
status_code=400,
)
Expand All @@ -139,6 +144,7 @@ def test_mode_superadmin(self):
owner=f"user:{self.user.id}",
url="http://sentry.io",
interval_seconds=60,
timeout_ms=1000,
mode=ProjectUptimeSubscriptionMode.AUTO_DETECTED_ACTIVE,
)
uptime_monitor = ProjectUptimeSubscription.objects.get(id=resp.data["id"])
Expand All @@ -149,7 +155,7 @@ def test_mode_superadmin(self):
assert uptime_monitor.mode == ProjectUptimeSubscriptionMode.AUTO_DETECTED_ACTIVE
assert uptime_subscription.url == "http://sentry.io"
assert uptime_subscription.interval_seconds == 60
assert uptime_subscription.timeout_ms == DEFAULT_SUBSCRIPTION_TIMEOUT_MS
assert uptime_subscription.timeout_ms == 1000

@with_feature("organizations:uptime-api-create-update")
def test_headers_body_method(self):
Expand All @@ -161,6 +167,7 @@ def test_headers_body_method(self):
owner=f"user:{self.user.id}",
url="http://sentry.io",
interval_seconds=60,
timeout_ms=1000,
method="POST",
body='{"key": "value"}',
headers=[["header", "value"]],
Expand All @@ -173,7 +180,7 @@ def test_headers_body_method(self):
assert uptime_monitor.mode == ProjectUptimeSubscriptionMode.MANUAL
assert uptime_subscription.url == "http://sentry.io"
assert uptime_subscription.interval_seconds == 60
assert uptime_subscription.timeout_ms == DEFAULT_SUBSCRIPTION_TIMEOUT_MS
assert uptime_subscription.timeout_ms == 1000
assert uptime_subscription.body == '{"key": "value"}'
assert uptime_subscription.headers == [["header", "value"]]

Expand All @@ -187,6 +194,7 @@ def test_headers_body_method_already_exists(self):
owner=f"user:{self.user.id}",
url="http://sentry.io",
interval_seconds=60,
timeout_ms=1000,
method="POST",
body='{"key": "value"}',
headers=[["header", "value"]],
Expand All @@ -201,6 +209,7 @@ def test_headers_body_method_already_exists(self):
owner=f"user:{self.user.id}",
url="http://sentry.io",
interval_seconds=60,
timeout_ms=1000,
method="POST",
body='{"key": "value"}',
headers=[["header", "value"]],
Expand All @@ -216,6 +225,7 @@ def test_headers_body_method_already_exists(self):
owner=f"user:{self.user.id}",
url="http://sentry.io",
interval_seconds=60,
timeout_ms=1000,
method="POST",
body='{"key": "value"}',
headers=[["header", "different value"]],
Expand All @@ -235,6 +245,7 @@ def test_headers_invalid_format(self):
owner=f"user:{self.user.id}",
url="http://sentry.io",
interval_seconds=60,
timeout_ms=1000,
method="POST",
body='{"key": "value"}',
headers={"header", "value"},
Expand All @@ -254,6 +265,7 @@ def test_size_too_big(self):
owner=f"user:{self.user.id}",
url="http://sentry.io",
interval_seconds=60,
timeout_ms=1000,
method="POST",
body="body" * 250,
headers=[["header", "value"]],
Expand All @@ -279,6 +291,7 @@ def test_over_limit(self):
name="test",
url="http://sentry.io",
interval_seconds=60,
timeout_ms=1000,
owner=f"user:{self.user.id}",
)
self.get_error_response(
Expand All @@ -288,5 +301,6 @@ def test_over_limit(self):
name="test",
url="http://santry.io",
interval_seconds=60,
timeout_ms=1000,
owner=f"user:{self.user.id}",
)
3 changes: 3 additions & 0 deletions tests/sentry/uptime/subscriptions/test_subscriptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ def test(self):
environment=self.environment,
url="https://santry.io",
interval_seconds=60,
timeout_ms=1000,
method="POST",
headers=[("some", "header")],
body="a body",
Expand Down Expand Up @@ -286,6 +287,7 @@ def test_removes_old(self):
environment=self.environment,
url="https://santry.io",
interval_seconds=proj_sub.uptime_subscription.interval_seconds,
timeout_ms=1000,
method=proj_sub.uptime_subscription.method,
headers=proj_sub.uptime_subscription.headers,
body=proj_sub.uptime_subscription.body,
Expand Down Expand Up @@ -329,6 +331,7 @@ def test_already_exists(self):
environment=self.environment,
url=proj_sub.uptime_subscription.url,
interval_seconds=other_proj_sub.uptime_subscription.interval_seconds,
timeout_ms=1000,
method=other_proj_sub.uptime_subscription.method,
headers=other_proj_sub.uptime_subscription.headers,
body=other_proj_sub.uptime_subscription.body,
Expand Down
Loading