Skip to content

Commit

Permalink
fix: failing tests due to change in incident name (#2550)
Browse files Browse the repository at this point in the history
  • Loading branch information
talboren authored Nov 20, 2024
1 parent dae291b commit 57df5a1
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 24 deletions.
12 changes: 6 additions & 6 deletions keep/api/routes/preset.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,10 @@ def pull_data_from_providers(
f"Provider {provider.type} ({provider.id}) does not implement pulling incidents",
extra=extra,
)
except Exception as e:
logger.error(
except Exception:
logger.exception(
f"Unknown error pulling incidents from provider {provider.type} ({provider.id})",
extra={**extra, "error": str(e)},
extra={**extra, "trace_id": trace_id},
)
else:
logger.debug(
Expand All @@ -165,10 +165,10 @@ def pull_data_from_providers(
f"Provider {provider.type} ({provider.id}) does not implement pulling topology data",
extra=extra,
)
except Exception as e:
logger.error(
except Exception:
logger.exception(
f"Unknown error pulling topology from provider {provider.type} ({provider.id})",
extra={**extra, "error": str(e)},
extra={**extra},
)

for fingerprint, alert in sorted_provider_alerts_by_fingerprint.items():
Expand Down
5 changes: 3 additions & 2 deletions keep/functions/__init__.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import copy
import datetime
import json
import re
import urllib.parse
from datetime import timedelta
from itertools import groupby

import json5 as json
import json5
import pytz
from dateutil import parser
from dateutil.parser import ParserError
Expand Down Expand Up @@ -179,7 +180,7 @@ def join(
iterable: list | dict | str, delimiter: str = ",", prefix: str | None = None
) -> str:
if isinstance(iterable, str):
iterable = json.loads(iterable)
iterable = json5.loads(iterable)

if isinstance(iterable, dict):
if prefix:
Expand Down
9 changes: 7 additions & 2 deletions keep/providers/pagerduty_provider/pagerduty_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,7 @@ def _query(self, incident_id: str = None):
else incidents
)

@staticmethod
def _format_alert(
event: dict, provider_instance: "BaseProvider" = None
) -> AlertDto:
Expand Down Expand Up @@ -791,11 +792,15 @@ def _get_incidents(self) -> list[IncidentDto]:
raw_incidents = self.__get_all_incidents_or_alerts()
incidents = []
for incident in raw_incidents:
incident_dto = self._format_incident({"event": {"data": incident}})
incident_dto = PagerdutyProvider._format_incident(
{"event": {"data": incident}}
)
incident_alerts = self.__get_all_incidents_or_alerts(
incident_id=incident_dto.fingerprint
)
incident_alerts = [self._format_alert(alert) for alert in incident_alerts]
incident_alerts = [
PagerdutyProvider._format_alert(alert) for alert in incident_alerts
]
incident_dto._alerts = incident_alerts
incidents.append(incident_dto)
return incidents
Expand Down
39 changes: 25 additions & 14 deletions tests/test_rules_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,21 @@
import hashlib
import json
import os
import time
import uuid

import pytest
from sqlalchemy import desc, asc

from keep.api.core.db import create_rule as create_rule_db, get_last_incidents, get_incident_alerts_by_incident_id
from keep.api.core.db import create_rule as create_rule_db
from keep.api.core.db import get_incident_alerts_by_incident_id, get_last_incidents
from keep.api.core.db import get_rules as get_rules_db
from keep.api.core.dependencies import SINGLE_TENANT_UUID
from keep.api.models.alert import AlertDto, AlertSeverity, AlertStatus, IncidentSeverity, IncidentStatus
from keep.api.models.alert import (
AlertDto,
AlertSeverity,
AlertStatus,
IncidentSeverity,
IncidentStatus,
)
from keep.api.models.db.alert import Alert
from keep.api.models.db.rule import ResolveOn
from keep.rulesengine.rulesengine import RulesEngine
Expand Down Expand Up @@ -264,9 +269,12 @@ def test_incident_attributes(db_session):
# check that there are results
assert results is not None
assert len(results) == 1
assert results[0].user_generated_name == "Incident generated by rule {}".format(rules[0].name)
assert results[0].user_generated_name == "{}".format(rules[0].name)
assert results[0].alerts_count == i + 1
assert results[0].last_seen_time.isoformat(timespec='milliseconds')+"Z" == alert.lastReceived
assert (
results[0].last_seen_time.isoformat(timespec="milliseconds") + "Z"
== alert.lastReceived
)
assert results[0].start_time == alerts[0].timestamp


Expand Down Expand Up @@ -322,7 +330,7 @@ def test_incident_severity(db_session):
# check that there are results
assert results is not None
assert len(results) == 1
assert results[0].user_generated_name == "Incident generated by rule {}".format(rules[0].name)
assert results[0].user_generated_name == "{}".format(rules[0].name)
assert results[0].alerts_count == 3
assert results[0].severity.value == IncidentSeverity.INFO.value

Expand All @@ -341,7 +349,7 @@ def test_incident_resolution_on_all(db_session, create_alert):
definition_cel='(severity == "critical")',
created_by="[email protected]",
require_approve=False,
resolve_on=ResolveOn.ALL.value
resolve_on=ResolveOn.ALL.value,
)

incidents, total_count = get_last_incidents(
Expand All @@ -353,13 +361,13 @@ def test_incident_resolution_on_all(db_session, create_alert):
assert total_count == 0

create_alert(
f"Something went wrong",
"Something went wrong",
AlertStatus.FIRING,
datetime.datetime.utcnow(),
{"severity": AlertSeverity.CRITICAL.value},
)
create_alert(
f"Something went wrong again",
"Something went wrong again",
AlertStatus.FIRING,
datetime.datetime.utcnow(),
{"severity": AlertSeverity.CRITICAL.value},
Expand Down Expand Up @@ -387,7 +395,7 @@ def test_incident_resolution_on_all(db_session, create_alert):

# Same fingerprint
create_alert(
f"Something went wrong",
"Something went wrong",
AlertStatus.RESOLVED,
datetime.datetime.utcnow(),
{"severity": AlertSeverity.CRITICAL.value},
Expand Down Expand Up @@ -415,7 +423,7 @@ def test_incident_resolution_on_all(db_session, create_alert):
assert incident.status == IncidentStatus.FIRING.value

create_alert(
f"Something went wrong again",
"Something went wrong again",
AlertStatus.RESOLVED,
datetime.datetime.utcnow(),
{"severity": AlertSeverity.CRITICAL.value},
Expand Down Expand Up @@ -444,9 +452,11 @@ def test_incident_resolution_on_all(db_session, create_alert):

@pytest.mark.parametrize(
"direction,second_fire_order",
[(ResolveOn.FIRST.value, ('fp2', 'fp1')), (ResolveOn.LAST.value, ('fp2', 'fp1'))]
[(ResolveOn.FIRST.value, ("fp2", "fp1")), (ResolveOn.LAST.value, ("fp2", "fp1"))],
)
def test_incident_resolution_on_edge(db_session, create_alert, direction, second_fire_order):
def test_incident_resolution_on_edge(
db_session, create_alert, direction, second_fire_order
):

create_rule_db(
tenant_id=SINGLE_TENANT_UUID,
Expand Down Expand Up @@ -560,6 +570,7 @@ def test_incident_resolution_on_edge(db_session, create_alert, direction, second
assert alert_count == 2
assert incident.status == IncidentStatus.RESOLVED.value


# Next steps:
# - test that alerts in the same group are being updated correctly
# - test group are being updated correctly
Expand Down

0 comments on commit 57df5a1

Please sign in to comment.