-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
feat(autopilot): Create instrumentation issues #106249
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(autopilot): Create instrumentation issues #106249
Conversation
| newest_version = update["newestSdkVersion"] | ||
|
|
||
| create_instrumentation_issue( | ||
| project_id=project_id, | ||
| detector_name=AutopilotDetectorName.SDK_UPDATE, | ||
| title=f"SDK Update Available: {sdk_name}", | ||
| subtitle=f"Update from {current_version} to {newest_version}", | ||
| description=f"A newer version of {sdk_name} is available. " | ||
| f"Consider updating from version {current_version} to {newest_version} " | ||
| f"to gain access to bug fixes, performance improvements, and new features.", | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The run_sdk_update_detector_for_organization task doesn't handle the Project.DoesNotExist exception, which could crash the task if a project is deleted mid-execution.
Severity: CRITICAL
Suggested Fix
Wrap the call to create_instrumentation_issue inside the for loop in run_sdk_update_detector_for_organization with a try...except Project.DoesNotExist block. This will gracefully handle cases where a project has been deleted and allow the task to continue processing other projects.
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/autopilot/tasks.py#L216-L230
Potential issue: In the `run_sdk_update_detector_for_organization` task, the code
iterates through a list of updates and calls `create_instrumentation_issue` for each
`project_id`. This function internally calls
`Project.objects.get_from_cache(id=project_id)`. If a project is deleted between the
initial data query and this call, a `Project.DoesNotExist` exception will be raised.
Since there is no `try...except` block to handle this exception, the entire background
task will crash, preventing the processing of remaining projects for the organization.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while true, that's ok for now as we are in PoC phase
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we'll refactor it later, so no need to do anything now, but it is also possible to add a patch decorator on the class so that there is no need to add it for each test case
| newest_version = update["newestSdkVersion"] | ||
|
|
||
| create_instrumentation_issue( | ||
| project_id=project_id, | ||
| detector_name=AutopilotDetectorName.SDK_UPDATE, | ||
| title=f"SDK Update Available: {sdk_name}", | ||
| subtitle=f"Update from {current_version} to {newest_version}", | ||
| description=f"A newer version of {sdk_name} is available. " | ||
| f"Consider updating from version {current_version} to {newest_version} " | ||
| f"to gain access to bug fixes, performance improvements, and new features.", | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The sdk_update_detector task may crash due to an unhandled Project.DoesNotExist exception if a project is deleted after being queried from Snuba but before being processed.
Severity: HIGH
Suggested Fix
Wrap the call to create_instrumentation_issue() within the loop in a try...except Project.DoesNotExist block. This will allow the task to gracefully skip deleted projects and continue processing the rest of the batch, consistent with other error handling patterns in the same file.
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/autopilot/tasks.py#L214-L228
Potential issue: The `sdk_update_detector` task fetches project IDs from Snuba. A race
condition exists where a project might be deleted after being fetched but before the
task processes it. When the task later calls
`Project.objects.get_from_cache(id=project_id)`, it will raise an unhandled
`Project.DoesNotExist` exception. This will cause the entire task to fail, preventing it
from creating SDK update issues for any remaining, valid projects in the batch. A
similar task in the same file already handles this scenario with a `try...except` block.
Did we get this right? 👍 / 👎 to inform future reviews.
| event_id = uuid.uuid4().hex | ||
|
|
||
| # Fetch the project to get its platform | ||
| project = Project.objects.get_from_cache(id=project_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The create_instrumentation_issue function lacks exception handling for Project.DoesNotExist, creating a race condition that can crash the autopilot task.
Severity: MEDIUM
Suggested Fix
Wrap the Project.objects.get_from_cache(id=project_id) call within a try...except Project.DoesNotExist block. In the except block, you should likely log that the project was deleted and then return, allowing the task to continue processing other projects.
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/autopilot/tasks.py#L49
Potential issue: The `create_instrumentation_issue` function calls
`Project.objects.get_from_cache(id=project_id)` without handling a potential
`Project.DoesNotExist` exception. A race condition can occur if a project is deleted
between the time its ID is retrieved from a Snuba query and when this function attempts
to fetch the project object. This unhandled exception will cause the entire autopilot
task to crash, preventing it from processing other projects within the same run. This
omission violates a well-established pattern in the codebase where this exception is
explicitly handled in numerous other places.
Did we get this right? 👍 / 👎 to inform future reviews.
| id=uuid.uuid4().hex, | ||
| project_id=project_id, | ||
| event_id=event_id, | ||
| fingerprint=[detector_name], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SDK updates incorrectly grouped due to static fingerprint
High Severity
The fingerprint in create_instrumentation_issue uses only [detector_name] (e.g., ["sdk-update"]), which is a constant value. When a project has multiple SDKs needing updates, each call to create_instrumentation_issue produces the same fingerprint. Since Sentry groups occurrences by (project, fingerprint), all SDK update issues for a single project merge into one issue, despite having SDK-specific titles and subtitles. The fingerprint needs to include distinguishing data like the SDK name to ensure separate trackable issues per SDK.
Create instrumentation issues from the detectors