Skip to content
Merged
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
2 changes: 2 additions & 0 deletions src/sentry/api/helpers/deprecation.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ def get(self, request):
:param key: The key prefix for an option use for the brownout schedule and duration
If not set 'api.deprecation.brownout' will be used, which currently
is using schedule of a 1 minute blackout at noon UTC.
:param url_names: A list of URL names that are deprecated if an endpoint has multiple URLs
and you need to deprecate one of the URLs.
Comment on lines +135 to +136
Copy link
Member

Choose a reason for hiding this comment

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

note: not a blocker, but this feels a pretty circular to me.

in general i think urls are supposed to reference views rather than the other way round

If possible, I would probably prefer to go the other way and create a new view (which subclasses + reuses the other one) with the correct deprecation markers, or maybe write a helper function that dynamically attaches the deprecations to the original view...

curious what you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

If possible, I would probably prefer to go the other way and create a new view (which subclasses + reuses the other one) with the correct deprecation markers, or maybe write a helper function that dynamically attaches the deprecations to the original view...

Isn't that quite a bit more code to add (and later remove)? For issue endpoints we would also need to rebuild the function that generates issue URLs as currently, the same function is reused with different prefixes.

"""

def decorator(func: EndpointT[SelfT, P]) -> EndpointT[SelfT, P]:
Expand Down
6 changes: 5 additions & 1 deletion src/sentry/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import os.path
from collections import namedtuple
from collections.abc import Sequence
from datetime import timedelta
from datetime import UTC, datetime, timedelta
from enum import Enum
from typing import cast

Expand Down Expand Up @@ -1053,3 +1053,7 @@ class InsightModules(Enum):
"dsr": "visual basic 6.0",
"frm": "visual basic 6.0",
}

# After this date APIs that are incompatible with cell routing
# will begin periodic brownouts.
CELL_API_DEPRECATION_DATE = datetime(2025, 5, 15, 0, 0, 0, tzinfo=UTC)
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@
from sentry import features
from sentry.api.api_publish_status import ApiPublishStatus
from sentry.api.base import region_silo_endpoint
from sentry.api.helpers.deprecation import deprecated
from sentry.api.helpers.environments import get_environments
from sentry.api.utils import get_date_range_from_params
from sentry.constants import CELL_API_DEPRECATION_DATE
from sentry.issues.endpoints.bases.group import GroupEndpoint
from sentry.issues.suspect_flags import Distribution, get_suspect_flag_scores
from sentry.models.group import Group
Expand All @@ -32,6 +34,7 @@ class ResponseData(TypedDict):
class OrganizationGroupSuspectFlagsEndpoint(GroupEndpoint):
publish_status = {"GET": ApiPublishStatus.PRIVATE}

@deprecated(CELL_API_DEPRECATION_DATE, url_names=["sentry-api-0-suspect-flags"])
def get(self, request: Request, group: Group) -> Response:
"""Stats bucketed by time."""
if not features.has(
Comment on lines 34 to 40
Copy link

Choose a reason for hiding this comment

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

Bug: The deprecated decorator uses incorrect url_names, causing the deprecation logic to be entirely skipped for the target endpoints.
Severity: CRITICAL | Confidence: High

🔍 Detailed Analysis

The @deprecated decorator is configured with url_names such as "sentry-api-0-suspect-flags" and "sentry-api-0-suspect-tags". However, the actual registered URL names, as constructed by create_group_urls(), are "sentry-api-0-organization-group-suspect-flags" and "sentry-api-0-organization-group-suspect-tags". This mismatch causes the url_name in url_names check within the decorator to always evaluate to False, leading to the deprecation logic being entirely skipped. Consequently, deprecation headers are not added, brownout blocking is not applied, and deprecation metrics are not incremented.

💡 Suggested Fix

Update the url_names parameter in the @deprecated decorator to match the full registered URL names, for example, url_names=["sentry-api-0-organization-group-suspect-flags"].

🤖 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/issues/endpoints/organization_group_suspect_flags.py#L34-L40

Potential issue: The `@deprecated` decorator is configured with `url_names` such as
`"sentry-api-0-suspect-flags"` and `"sentry-api-0-suspect-tags"`. However, the actual
registered URL names, as constructed by `create_group_urls()`, are
`"sentry-api-0-organization-group-suspect-flags"` and
`"sentry-api-0-organization-group-suspect-tags"`. This mismatch causes the `url_name in
url_names` check within the decorator to always evaluate to `False`, leading to the
deprecation logic being entirely skipped. Consequently, deprecation headers are not
added, brownout blocking is not applied, and deprecation metrics are not incremented.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 5887053

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
from sentry import features
from sentry.api.api_publish_status import ApiPublishStatus
from sentry.api.base import region_silo_endpoint
from sentry.api.helpers.deprecation import deprecated
from sentry.api.helpers.environments import get_environments
from sentry.api.utils import get_date_range_from_params
from sentry.constants import CELL_API_DEPRECATION_DATE
from sentry.issues.endpoints.bases.group import GroupEndpoint
from sentry.issues.suspect_tags import get_suspect_tag_scores
from sentry.models.group import Group
Expand All @@ -17,6 +19,7 @@
class OrganizationGroupSuspectTagsEndpoint(GroupEndpoint):
publish_status = {"GET": ApiPublishStatus.PRIVATE}

@deprecated(CELL_API_DEPRECATION_DATE, url_names=["sentry-api-0-suspect-tags"])
def get(self, request: Request, group: Group) -> Response:
"""Stats bucketed by time."""
if not features.has(
Expand Down
Loading