Skip to content

ref(crons): Reorganize incident creation / issue occurrence logic#3

Open
adamsaimi wants to merge 1 commit intomonitor-incident-refactor-beforefrom
monitor-incident-refactor-after
Open

ref(crons): Reorganize incident creation / issue occurrence logic#3
adamsaimi wants to merge 1 commit intomonitor-incident-refactor-beforefrom
monitor-incident-refactor-after

Conversation

@adamsaimi
Copy link
Owner

Test 8

…0528)

Since we'll be doing more with issue occurrences split out the concept
of incidents into it's own logic module, as well as incident_occurrence
into it's own module

Part of GH-80527
# - The monitor and env are not muted
if not monitor_env.monitor.is_muted and not monitor_env.is_muted and incident:
checkins = MonitorCheckIn.objects.filter(id__in=[c["id"] for c in previous_checkins])
for checkin in checkins:
Copy link
Owner Author

Choose a reason for hiding this comment

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

[Bug] Duplicate Incident Creation

  • Problem: When failure_issue_threshold > 1, create_incident_occurrence is incorrectly called multiple times, resulting in failure_issue_threshold Kafka messages for a single incident and unnecessary load.
  • Fix: Modify the logic to ensure create_incident_occurrence is called only once per incident creation, using the most recent failed check-in and all previous_checkins for context.
        # Only create one occurrence for the incident, using the most recent failed_checkin
        # and the full list of previous_checkins for context.
        create_incident_occurrence(
            previous_checkins,
            failed_checkin, # Use the single failed_checkin that triggered this call
            incident,
            received=received,
        )```

"id": str(monitor_environment.monitor.guid),
"slug": str(monitor_environment.monitor.slug),
"name": monitor_environment.monitor.name,
"config": monitor_environment.monitor.config,
Copy link
Owner Author

Choose a reason for hiding this comment

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

[Security] Potential Information Leakage

  • Problem: The monitor_environment.monitor.config is directly included in the event context, which could expose sensitive information like API keys or internal paths.
  • Fix: Explicitly filter or sanitize the config dictionary to include only non-sensitive, relevant fields before adding it to the event context.
        # ... other fields ...
        \"config\": {k: v for k, v in monitor_environment.monitor.config.items() if k not in [\"sensitive_key_1\", \"sensitive_key_2\"]}, # Example filtering
        # ... other fields ...
    }```

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants