Skip to content

feat(uptime): Use EAP to query in project_uptime_alert_checks_index #94755

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

Merged
merged 2 commits into from
Jul 2, 2025

Conversation

wedamija
Copy link
Member

@wedamija wedamija commented Jul 1, 2025

Relies on #94597

This allows us to switch our queries to use EAP in the check details endpoint

@wedamija wedamija marked this pull request as ready for review July 1, 2025 23:01
@wedamija wedamija requested review from a team as code owners July 1, 2025 23:01
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 1, 2025
cursor[bot]

This comment was marked as outdated.

Copy link

codecov bot commented Jul 1, 2025

Codecov Report

Attention: Patch coverage is 97.67442% with 4 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ime/endpoints/project_uptime_alert_checks_index.py 91.89% 3 Missing ⚠️
tests/sentry/uptime/endpoints/test_base.py 98.14% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master   #94755    +/-   ##
========================================
  Coverage   87.88%   87.89%            
========================================
  Files       10439    10441     +2     
  Lines      603361   603685   +324     
  Branches    23501    23501            
========================================
+ Hits       530254   530580   +326     
+ Misses      72741    72739     -2     
  Partials      366      366            

cursor[bot]

This comment was marked as outdated.

Base automatically changed from danf/uptime-stats-eap-org to master July 2, 2025 15:43
@wedamija wedamija force-pushed the danf/uptime-stats-eap-project branch from 7101600 to 020ab43 Compare July 2, 2025 15:44
cursor[bot]

This comment was marked as outdated.

wedamija added 2 commits July 2, 2025 11:00
Relies on #94597

This allows us to switch our queries to use EAP in the check details endpoint
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Bug: UUID Parsing Breaks Legacy Subscription IDs

The ProjectUptimeAlertCheckIndexEndpoint now attempts to parse uptime_subscription.uptime_subscription.subscription_id as a UUID. This introduces a ValueError if the ID is not a valid UUID string, potentially crashing requests. The previous implementation directly used the ID as a string, making the new code incompatible with existing legacy subscription IDs that may not be UUID-formatted, particularly affecting the TRACE_ITEM_TYPE_UPTIME_CHECK path.

src/sentry/uptime/endpoints/project_uptime_alert_checks_index.py#L133-L135

value=AttributeValue(
val_str=str(uuid.UUID(uptime_subscription.uptime_subscription.subscription_id))
),

Fix in Cursor


Bug: Trace Item Parsing Errors

For UPTIME_RESULT trace items, check_status_reason is always None because the code incorrectly looks for the column label "check_status_reason" instead of the actual attribute name "status_reason_type" in the RPC response. Additionally, accessing http_status_code directly via row_dict[...] can lead to a KeyError if the column is missing from the RPC response; it should use row_dict.get().

src/sentry/uptime/endpoints/project_uptime_alert_checks_index.py#L347-L355

check_status=cast(CheckStatus, row_dict["check_status"].val_str),
check_status_reason=self._extract_check_status_reason(
row_dict.get("check_status_reason")
),
http_status_code=(
None
if row_dict["http_status_code"].is_null
else row_dict["http_status_code"].val_int
),

Fix in Cursor


Was this report helpful? Give feedback by reacting with 👍 or 👎

@wedamija wedamija merged commit 9992a03 into master Jul 2, 2025
65 checks passed
@wedamija wedamija deleted the danf/uptime-stats-eap-project branch July 2, 2025 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants