feat(aci): associate groups without matching detector to the issue stream detector #103191
Conversation
github-actions
bot
added
the
Scope: Backend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| else: | ||
| return False | ||
| # We find the issue stream detector for the project | ||
| detector_id = Detector.get_issue_stream_detector_for_project(group.project.id).id |
There was a problem hiding this comment.
Bug: Missing Detector Causes Unhandled Project Crashes
When associate_new_group_with_detector is called for non-error groups without a detector_id, it attempts to fetch the issue stream detector for the project. However, get_issue_stream_detector_for_project will raise Detector.DoesNotExist for projects that don't have an issue stream detector created yet (typically older projects created before this feature). The exception is unhandled, causing crashes when processing groups for these projects.
There was a problem hiding this comment.
You prob don't know this but I actually did a backfill so all projects have issue stream detectors
| if not options.get("workflow_engine.associate_error_detectors", False): | ||
| return False | ||
| detector_id = Detector.get_error_detector_for_project(group.project.id).id | ||
| else: |
There was a problem hiding this comment.
Maybe # If it isn't an error Group and no detector_id was provided, we use the Issue Stream detector. Any other detector types .. <some statement explaining why the absence of a detector specified here is meaningful>.
Previously, we were just picking off errors, this is a catch-all which seems like it requires a bit more justification. I'm mostly concerned because regardless of how this is called, if you're an error, you get the error detector. Now, if someone doesn't pass a detector_id, regardless of type you get issue stream. Arguably, some issue types should be excluded here, so I'm wondering if a flag on group.type or whatever makes sense here in addition.
|
Closing because
|
We expect all Groups to have an entry in the DetectorGroup table so we can easily look up the detector for a group. For groups that have types that haven't been moved over to workflow engine, we can associate them with the issue stream detector.
When new Detectors are created and start emitting issues, this will need to be changed via backfill.
We need DetectorGroup entries for Groups because we want to look up the detector that fired for a group in the WorkflowFireHistory API without populating the detector field on WorkflowFireHistory (decoupling detector from WFH) -- see #102918.
This will also be used to figure out which detector an Activity is associated with for activity notifications in workflow engine -- see #103099