Skip to content

Conversation

dvora-ns
Copy link
Contributor

@dvora-ns dvora-ns commented Aug 14, 2025

Closes #5253

📑 Description

This PR adds a new workflow trigger event for incident called "alert_association_changed". This helps user configure workflows that trigger whenever incident alert association changes.

Following events cause the incident alert association to change:

  1. Alert addition/linkage to an incident
  2. Alert unlinking from an incident.

Whenever this event takes place, an additional key called "linked_alerts" is added to the incident object that's passed to the workflow. The linked_alerts key is basically a list of alert strings currently linked to the incident sorted in descending order of timestamp. Each alert string present in the list is of the following form:
{status} {lastReceived} [{severity}] {description}
e.g.
Firing 2025-01-30T10:00:00.000Z [high] Test alert 1 description

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

Copy link

vercel bot commented Aug 14, 2025

@dvora-ns is attempting to deploy a commit to the KeepHQ Team on Vercel.

A member of the Team first needs to authorize it.

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. Feature A new feature labels Aug 14, 2025
@dvora-ns dvora-ns changed the title Add workflow trigger for incident called 'alert_association_changed' feat: Add workflow trigger for incident called 'alert_association_changed' Aug 14, 2025
Copy link

codecov bot commented Aug 14, 2025

Codecov Report

❌ Patch coverage is 0% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 30.70%. Comparing base (74e4c9f) to head (d7f7b2f).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
keep/workflowmanager/workflowmanager.py 0.00% 12 Missing ⚠️
keep/api/bl/incidents_bl.py 0.00% 4 Missing ⚠️
keep/rulesengine/rulesengine.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5254      +/-   ##
==========================================
- Coverage   30.73%   30.70%   -0.04%     
==========================================
  Files         101      101              
  Lines       11494    11511      +17     
==========================================
+ Hits         3533     3534       +1     
- Misses       7961     7977      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dvora-ns dvora-ns marked this pull request as draft August 14, 2025 07:51
@dvora-ns dvora-ns marked this pull request as ready for review August 21, 2025 05:36
@dosubot dosubot bot added the Incident label Aug 21, 2025
@dvora-ns dvora-ns marked this pull request as draft August 21, 2025 05:37
@dvora-ns dvora-ns marked this pull request as ready for review August 22, 2025 08:21
@shahargl shahargl requested a review from Copilot August 27, 2025 11:32
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new workflow trigger event called "alert_association_changed" for incidents, which triggers whenever alert associations with incidents change (addition/linking or unlinking of alerts). The key enhancement is the addition of a linked_alerts attribute to incident objects, providing a formatted list of currently associated alerts for workflow consumption.

  • Add "alert_association_changed" trigger event for incident workflows
  • Implement linked_alerts attribute generation containing formatted alert information
  • Update UI components and schema to support the new trigger type

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
keep/workflowmanager/workflowmanager.py Core logic to process alert_association_changed trigger and generate linked_alerts attribute
keep/api/bl/incidents_bl.py Send workflow event when alerts are added/removed from incidents
keep/rulesengine/rulesengine.py Trigger alert_association_changed event during rule processing
keep/topologies/topology_processor.py Trigger alert_association_changed event for application-based incidents
keep-ui/entities/workflows/model/schema.ts Update schema to include new trigger event
keep-ui/features/workflows/builder/ui/Editor/TriggerEditor.tsx Add UI support for the new trigger
tests/test_workflowmanager.py Unit tests for linked_alerts functionality
tests/test_workflow_execution.py Integration test for alert_association_changed workflow
tests/test_incidents.py Update test expectations for additional workflow events

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

# Handle multiline description
alert_description = alert.description.split("\n")[0]

processed_alert = f"{alert.status.capitalize()} {alert.lastReceived} [{alert.severity}] {alert_description}"
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

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

The alert.status.capitalize() call will fail if alert.status is an enum. AlertStatus enum values need to be converted to string first before calling capitalize().

Suggested change
processed_alert = f"{alert.status.capitalize()} {alert.lastReceived} [{alert.severity}] {alert_description}"
processed_alert = f"{alert.status.value.capitalize()} {alert.lastReceived} [{alert.severity}] {alert_description}"

Copilot uses AI. Check for mistakes.

# Handle multiline description
alert_description = alert.description.split("\n")[0]

processed_alert = f"{alert.status.capitalize()} {alert.lastReceived} [{alert.severity}] {alert_description}"
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

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

The timestamp formatting in alert.lastReceived should be standardized. Consider using a consistent datetime format (e.g., ISO format with milliseconds as shown in the expected test output: '2025-01-30T10:00:00.000Z').

Suggested change
processed_alert = f"{alert.status.capitalize()} {alert.lastReceived} [{alert.severity}] {alert_description}"
# Standardize lastReceived to ISO 8601 with milliseconds and 'Z'
last_received = alert.lastReceived
if isinstance(last_received, datetime.datetime):
# Ensure UTC and add 'Z'
if last_received.tzinfo is not None:
last_received = last_received.astimezone(datetime.timezone.utc)
last_received_str = last_received.isoformat(timespec='milliseconds').replace('+00:00', 'Z')
else:
last_received_str = str(last_received)
processed_alert = f"{alert.status.capitalize()} {last_received_str} [{alert.severity}] {alert_description}"

Copilot uses AI. Check for mistakes.

Comment on lines +228 to +231
incident_dto._alerts = [mock_alert_1, mock_alert_2]

# Create a mock workflow with alert_association_changed trigger
mock_workflow = Mock(spec=Workflow)
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

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

Setting a private attribute _alerts directly is not a good practice. Consider using a proper setter method or mocking the alerts property instead.

Suggested change
incident_dto._alerts = [mock_alert_1, mock_alert_2]
# Create a mock workflow with alert_association_changed trigger
mock_workflow = Mock(spec=Workflow)
with patch.object(type(incident_dto), "alerts", new_callable=property) as mock_alerts_prop:
mock_alerts_prop.return_value = [mock_alert_1, mock_alert_2]
# Create a mock workflow with alert_association_changed trigger
mock_workflow = Mock(spec=Workflow)

Copilot uses AI. Check for mistakes.

Comment on lines +284 to +285
expected_alert_1 = "Firing 2025-01-30T10:00:00.000Z [high] Test alert 1 description"
expected_alert_2 = "Resolved 2025-01-30T11:00:00.000Z [critical] Test alert 2 description"
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

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

The test expects '2025-01-30T10:00:00.000Z' but the mock alert has lastReceived="2025-01-30T10:00:00Z" (without milliseconds). This mismatch will cause the test to fail.

Suggested change
expected_alert_1 = "Firing 2025-01-30T10:00:00.000Z [high] Test alert 1 description"
expected_alert_2 = "Resolved 2025-01-30T11:00:00.000Z [critical] Test alert 2 description"
expected_alert_1 = "Firing 2025-01-30T10:00:00Z [high] Test alert 1 description"
expected_alert_2 = "Resolved 2025-01-30T11:00:00Z [critical] Test alert 2 description"

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature A new feature Incident size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[➕ Feature]: Add a new workflow incident trigger event called "alert_association_changed"

2 participants