Skip to content

PR Incidents Filtering for better CFR and MTTR Reporting #656

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 7 commits into from
May 23, 2025
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 39 additions & 10 deletions backend/analytics_server/mhq/api/incidents.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@
adapt_mean_time_to_recovery_metrics,
)
from mhq.store.models.incidents import Incident

from mhq.api.request_utils import coerce_workflow_filter, queryschema
from mhq.service.query_validator import get_query_validator


app = Blueprint("incidents", __name__)


Expand All @@ -36,19 +36,26 @@
{
Required("from_time"): All(str, Coerce(datetime.fromisoformat)),
Required("to_time"): All(str, Coerce(datetime.fromisoformat)),
Optional("pr_filter"): All(str, Coerce(json.loads)),
}
),
)
def get_resolved_incidents(team_id: str, from_time: datetime, to_time: datetime):
def get_resolved_incidents(
team_id: str, from_time: datetime, to_time: datetime, pr_filter: dict = None
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
team_id: str, from_time: datetime, to_time: datetime, pr_filter: dict = None
team_id: str, from_time: datetime, to_time: datetime, pr_filter: typeOptional[dict] = None

Type of Dict cannot be assigned to None. Use correct type. Import the correct optional from typing and ensure it is not conflicting with voluptuous.Optional

):

query_validator = get_query_validator()
interval = query_validator.interval_validator(from_time, to_time)
query_validator.team_validator(team_id)

pr_filter: PRFilter = apply_pr_filter(
pr_filter, EntityType.TEAM, team_id, [SettingType.EXCLUDED_PRS_SETTING]
)

incident_service = get_incident_service()

resolved_incidents: List[Incident] = incident_service.get_resolved_team_incidents(
team_id, interval
team_id, interval, pr_filter
)

# ToDo: Generate a user map
Expand Down Expand Up @@ -90,7 +97,9 @@ def get_deployments_with_related_incidents(

incident_service = get_incident_service()

incidents: List[Incident] = incident_service.get_team_incidents(team_id, interval)
incidents: List[Incident] = incident_service.get_team_incidents(
team_id, interval, pr_filter
)

deployment_incidents_map: Dict[Deployment, List[Incident]] = (
incident_service.get_deployment_incidents_map(deployments, incidents)
Expand All @@ -112,18 +121,25 @@ def get_deployments_with_related_incidents(
{
Required("from_time"): All(str, Coerce(datetime.fromisoformat)),
Required("to_time"): All(str, Coerce(datetime.fromisoformat)),
Optional("pr_filter"): All(str, Coerce(json.loads)),
}
),
)
def get_team_mttr(team_id: str, from_time: datetime, to_time: datetime):
def get_team_mttr(
team_id: str, from_time: datetime, to_time: datetime, pr_filter: dict = None
):
query_validator = get_query_validator()
interval = query_validator.interval_validator(from_time, to_time)
query_validator.team_validator(team_id)

pr_filter: PRFilter = apply_pr_filter(
pr_filter, EntityType.TEAM, team_id, [SettingType.EXCLUDED_PRS_SETTING]
)

incident_service = get_incident_service()

team_mean_time_to_recovery_metrics = (
incident_service.get_team_mean_time_to_recovery(team_id, interval)
incident_service.get_team_mean_time_to_recovery(team_id, interval, pr_filter)
)

return adapt_mean_time_to_recovery_metrics(team_mean_time_to_recovery_metrics)
Expand All @@ -135,18 +151,27 @@ def get_team_mttr(team_id: str, from_time: datetime, to_time: datetime):
{
Required("from_time"): All(str, Coerce(datetime.fromisoformat)),
Required("to_time"): All(str, Coerce(datetime.fromisoformat)),
Optional("pr_filter"): All(str, Coerce(json.loads)),
}
),
)
def get_team_mttr_trends(team_id: str, from_time: datetime, to_time: datetime):
def get_team_mttr_trends(
team_id: str, from_time: datetime, to_time: datetime, pr_filter: dict = None
):
query_validator = get_query_validator()
interval = query_validator.interval_validator(from_time, to_time)
query_validator.team_validator(team_id)

pr_filter: PRFilter = apply_pr_filter(
pr_filter, EntityType.TEAM, team_id, [SettingType.EXCLUDED_PRS_SETTING]
)

incident_service = get_incident_service()

weekly_mean_time_to_recovery_metrics = (
incident_service.get_team_mean_time_to_recovery_trends(team_id, interval)
incident_service.get_team_mean_time_to_recovery_trends(
team_id, interval, pr_filter
)
)

return {
Expand Down Expand Up @@ -192,7 +217,9 @@ def get_team_cfr(

incident_service = get_incident_service()

incidents: List[Incident] = incident_service.get_team_incidents(team_id, interval)
incidents: List[Incident] = incident_service.get_team_incidents(
team_id, interval, pr_filter
)

team_change_failure_rate: ChangeFailureRateMetrics = (
incident_service.get_change_failure_rate_metrics(deployments, incidents)
Expand Down Expand Up @@ -236,7 +263,9 @@ def get_team_cfr_trends(

incident_service = get_incident_service()

incidents: List[Incident] = incident_service.get_team_incidents(team_id, interval)
incidents: List[Incident] = incident_service.get_team_incidents(
team_id, interval, pr_filter
)

team_weekly_change_failure_rate: Dict[datetime, ChangeFailureRateMetrics] = (
incident_service.get_weekly_change_failure_rate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
ExcludedPRsSetting,
IncidentTypesSetting,
IncidentSourcesSetting,
IncidentPRsSetting,
)
from mhq.store.models import EntityType

Expand Down Expand Up @@ -55,6 +56,12 @@ def _add_setting_data(config_settings: ConfigurationSettings, response):
"default_sync_days": config_settings.specific_settings.default_sync_days
}

if isinstance(config_settings.specific_settings, IncidentPRsSetting):
response["setting"] = {
"include_revert_prs": config_settings.specific_settings.include_revert_prs,
"filters": config_settings.specific_settings.filters,
}

# ADD NEW API ADAPTER HERE

return response
Expand Down
9 changes: 8 additions & 1 deletion backend/analytics_server/mhq/service/code/pr_filter.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from typing import List, Dict, Any

from mhq.service.settings.configuration_settings import get_settings_service
from mhq.service.settings.models import ExcludedPRsSetting
from mhq.service.settings.models import ExcludedPRsSetting, IncidentPRsSetting
from mhq.store.models.code import PRFilter
from mhq.store.models.settings.configuration_settings import SettingType
from mhq.store.models.settings.enums import EntityType
Expand Down Expand Up @@ -103,6 +103,8 @@ def apply(self) -> PRFilter:
)
if setting_type == SettingType.EXCLUDED_PRS_SETTING:
self._apply_excluded_pr_ids_setting(setting=setting)
if setting_type == SettingType.INCIDENT_PRS_SETTING:
self._apply_incident_prs_setting(setting=setting)

return self.pr_filter

Expand All @@ -111,3 +113,8 @@ def _apply_excluded_pr_ids_setting(self, setting: ExcludedPRsSetting):
self.pr_filter.excluded_pr_ids = (
self.pr_filter.excluded_pr_ids or []
) + setting.excluded_pr_ids

def _apply_incident_prs_setting(self, setting: IncidentPRsSetting):
self.pr_filter.incident_pr_filters = (
self.pr_filter.incident_pr_filters or []
) + setting.filters
17 changes: 17 additions & 0 deletions backend/analytics_server/mhq/service/incidents/incident_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
get_settings_service,
IncidentSettings,
IncidentTypesSetting,
IncidentPRsSetting,
)
from mhq.store.models.incidents import IncidentFilter
from mhq.store.models.incidents.enums import IncidentType

from mhq.store.models.settings import EntityType

Expand Down Expand Up @@ -106,4 +108,19 @@ def __incident_type_setting(self) -> List[str]:
incident_types = []
if setting and isinstance(setting, IncidentTypesSetting):
incident_types = setting.incident_types

if SettingType.INCIDENT_PRS_SETTING in self.setting_types:
incident_prs_setting: Optional[IncidentPRsSetting] = (
self.setting_type_to_settings_map.get(SettingType.INCIDENT_PRS_SETTING)
)
if (
isinstance(incident_prs_setting, IncidentPRsSetting)
and not incident_prs_setting.include_revert_prs
):
incident_types = [
incident_type
for incident_type in incident_types
if incident_type != IncidentType.REVERT_PR
]

return incident_types
Loading