feat(performance-detectors): Use config from Workflow Engine Detectors if available and enabled #107137
Conversation
github-actions
bot
added
the
Scope: Backend
![sentry[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| def get_detection_settings( | ||
| project_id: int | None = None, organization: Organization | None = None | ||
| project: Project | None = None, | ||
| organization: Organization | None = None, |
There was a problem hiding this comment.
Do you need organization here if we're always fetching org from project.organization?
| PERFORMANCE_DETECTOR_CONFIG_MAPPINGS: dict[DetectorType, PerformanceDetectorConfigMapping] = { | ||
| DetectorType.SLOW_DB_QUERY: PerformanceDetectorConfigMapping( | ||
| settings_key=DetectorType.SLOW_DB_QUERY, | ||
| wfe_detector_type="performance_slow_db_query", |
There was a problem hiding this comment.
Should we use GroupType slugs here instead of hardcoded strings?PerformanceSlowDBQueryGroupType.slug? Not sure if there's a way to force these to specifically be a grouptype slug
There was a problem hiding this comment.
I didn't do that because these are Detector types, not GroupType slugs, and it feels odd to import so much for a few stable constants that are correct mostly by convention.
It's still weird because the Detector types don't actually exist yet and Detector.type is hard to clearly specify, but I clarified comments here and added at test that verifies that they all map to valid GroupTypes.
I think a follow-up change that adds detector_settings will make this all make a bit more sense.
Extend get_merged_settings with a mode parameter that allows us to maintain current operation ("LEGACY", the default, negligible performance cost), read configs from Detector rows and compare them to legacy config ("COMPARE", not yet suitable for prod use at scale), or actually use configs from Detector rows and not ProjectOption configs ("WFE" mode).
We don't currently have Detector rows for performance issues and the flags aren't enabled, so in practice this isn't yet code that will run, but we'll soon be dual-writing and opting in some test projects.
Resolves ISWF-1922.