Review Tech Debt in Alerting code
With the introduction of Alert Management
in 13.0, we now persist alerts.
This has given us the opportunity to review the code and uncover some opportunities to improve how things works and reduce tech debt
-
NotificationPayloadParser
should only need to be called once, which we can use to build a model (AlertManagement::Alert
andAlerting::Alert
) - The comment "it will not be excluded in the alert's parsed payload, and will be displayed in the list of additional attributes on the page" (from !31549 (diffs, comment 340141291)) shows we're mixing display logic with storage/model logic.
- I see we're passing a payload hash to
IncidentManagement::ProcessAlertWorker
. Is there any reason why we can just use a model object? - Can we merge
Gitlab::Alerting::Alert
andAlertManagement::Alert
? Do we still need two models? - Ideally defaults will be stored at the model, not the parser level.
- How can we normalise Prometheus and non-prometheus data as soon as possible in the request cycle so that we can reduce the number of Prometheus specific handlers and files etc?