Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions src/sentry/features/temporary.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,8 @@ def register_temporary_features(manager: FeatureManager):
manager.add("organizations:issue-taxonomy", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
manager.add("organizations:metric-issue-poc", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
manager.add("projects:metric-issue-creation", ProjectFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
# Enable writing to the IncidentGroupOpenPeriod model
manager.add("organizations:incident-group-open-period-write", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
manager.add("organizations:issue-open-periods", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
manager.add("organizations:mep-rollout-flag", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
manager.add("organizations:mep-use-default-tags", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
Expand Down
9 changes: 9 additions & 0 deletions src/sentry/incidents/logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
from sentry.utils.not_set import NOT_SET, NotSet
from sentry.utils.snuba import is_measurement
from sentry.workflow_engine.endpoints.validators.utils import toggle_detector
from sentry.workflow_engine.models import IncidentGroupOpenPeriod
from sentry.workflow_engine.models.detector import Detector

# We can return an incident as "windowed" which returns a range of points around the start of the incident
Expand Down Expand Up @@ -181,6 +182,14 @@ def create_incident(
incident_type=incident_type.value,
)

# If this is a metric alert incident, check for pending group relationships
if (
alert_rule
and incident_type == IncidentType.ALERT_TRIGGERED
and features.has("organizations:incident-group-open-period-write", organization)
):
IncidentGroupOpenPeriod.create_pending_relationships_for_incident(incident, alert_rule)

return incident


Expand Down
24 changes: 22 additions & 2 deletions src/sentry/issues/ingest.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from django.conf import settings
from django.db import router, transaction

from sentry import eventstream
from sentry import eventstream, features
from sentry.constants import LOG_LEVELS_MAP, MAX_CULPRIT_LENGTH
from sentry.event_manager import (
GroupInfo,
Expand All @@ -21,6 +21,7 @@
get_event_type,
save_grouphash_and_group,
)
from sentry.incidents.grouptype import MetricIssue
from sentry.issues.grouptype import FeedbackGroup, should_create_group
from sentry.issues.issue_occurrence import IssueOccurrence, IssueOccurrenceData
from sentry.issues.priority import PriorityChangeReason, update_priority
Expand All @@ -34,7 +35,7 @@
from sentry.utils import json, metrics, redis
from sentry.utils.strings import truncatechars
from sentry.utils.tag_normalization import normalized_sdk_tag_from_event
from sentry.workflow_engine.models.detector_group import DetectorGroup
from sentry.workflow_engine.models import DetectorGroup, IncidentGroupOpenPeriod

issue_rate_limiter = RedisSlidingWindowRateLimiter(
**settings.SENTRY_ISSUE_PLATFORM_RATE_LIMITER_OPTIONS
Expand Down Expand Up @@ -70,6 +71,25 @@ def save_issue_occurrence(
group_info.group.project, environment, release, [group_info]
)
_get_or_create_group_release(environment, release, event, [group_info])

# Create IncidentGroupOpenPeriod relationship for metric issues
if occurrence.type == MetricIssue and features.has(
"organizations:incident-group-open-period-write", event.organization
):
open_period = get_latest_open_period(group_info.group)
if open_period:
IncidentGroupOpenPeriod.create_from_occurrence(
occurrence, group_info.group, open_period
)
else:
logger.error(
"save_issue_occurrence.no_open_period",
extra={
"group_id": group_info.group.id,
"occurrence_id": occurrence.id,
},
)

send_issue_occurrence_to_eventstream(event, occurrence, group_info)
return occurrence, group_info

Expand Down
162 changes: 162 additions & 0 deletions src/sentry/workflow_engine/models/incident_groupopenperiod.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import logging

from django.db import models
from django.db.models import Q

Expand All @@ -8,6 +10,12 @@
FlexibleForeignKey,
region_silo_model,
)
from sentry.incidents.models.alert_rule import AlertRule
from sentry.incidents.models.incident import Incident
from sentry.models.groupopenperiod import GroupOpenPeriod
from sentry.workflow_engine.models.alertrule_detector import AlertRuleDetector

logger = logging.getLogger(__name__)


@region_silo_model
Expand All @@ -32,3 +40,157 @@ class Meta:
name="inc_id_inc_identifier_together",
)
]

@classmethod
def create_from_occurrence(self, occurrence, group, open_period):
"""
Creates an IncidentGroupOpenPeriod relationship from an issue occurrence.
This method handles the case where the incident might not exist yet.

Args:
occurrence: The IssueOccurrence that triggered the group creation
group: The Group that was created
open_period: The GroupOpenPeriod for the group
"""
try:
# Extract alert_id from evidence_data using the detector_id
detector_id = occurrence.evidence_data.get("detector_id")
if detector_id:
alert_id = AlertRuleDetector.objects.get(detector_id=detector_id).alert_rule_id
else:
Comment on lines +59 to +60
Copy link
Contributor

Choose a reason for hiding this comment

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

Potential bug: The call to AlertRuleDetector.objects.get() lacks error handling, which will cause a server crash if the object does not exist, breaking issue ingestion.
  • Description: The method create_from_occurrence calls AlertRuleDetector.objects.get(detector_id=detector_id) without handling the AlertRuleDetector.DoesNotExist exception. The detector_id value, derived from occurrence.evidence_data, can be missing or invalid. Other parts of the codebase, such as in src/sentry/incidents/grouptype.py, consistently wrap similar queries in try...except blocks, establishing a pattern that this is a known and expected failure case. An unhandled DoesNotExist exception will propagate up the call stack, causing a server crash during the critical issue ingestion process.

  • Suggested fix: Wrap the AlertRuleDetector.objects.get() call in a try...except AlertRuleDetector.DoesNotExist block. In the except block, log a warning and handle the case gracefully, for example by setting alert_id to None and allowing execution to continue. This matches established patterns elsewhere in the codebase.
    severity: 0.85, confidence: 0.95

Did we get this right? 👍 / 👎 to inform future reviews.

raise Exception("No detector_id found in evidence_data for metric issue")

# Try to find the active incident for this alert rule and project
try:
alert_rule = AlertRule.objects.get(id=alert_id)
incident = Incident.objects.get_active_incident(
alert_rule=alert_rule,
project=group.project,
)
except AlertRule.DoesNotExist:
logger.warning(
"AlertRule not found for alert_id",
extra={
"alert_id": alert_id,
"group_id": group.id,
},
)
incident = None

if incident:
# Incident exists, create the relationship immediately
return self.create_relationship(incident, open_period)
else:
# Incident doesn't exist yet, create a placeholder relationship
# that will be updated when the incident is created
return self.create_placeholder_relationship(detector_id, open_period, group.project)

except Exception as e:
logger.exception(
"Failed to create IncidentGroupOpenPeriod relationship",
extra={
"group_id": group.id,
"occurrence_id": occurrence.id,
"error": str(e),
},
)
return None

@classmethod
def create_relationship(self, incident, open_period):
"""
Creates IncidentGroupOpenPeriod relationship.

Args:
incident: The Incident to link
open_period: The GroupOpenPeriod to link
"""
try:
incident_group_open_period, _ = self.objects.get_or_create(
group_open_period=open_period,
defaults={
"incident_id": incident.id,
"incident_identifier": incident.identifier,
},
)

return incident_group_open_period

except Exception as e:
logger.exception(
"Failed to create/update IncidentGroupOpenPeriod relationship",
extra={
"incident_id": incident.id,
"open_period_id": open_period.id,
"error": str(e),
},
)
return None

@classmethod
def create_placeholder_relationship(self, detector_id, open_period, project):
"""
Creates a placeholder relationship when the incident doesn't exist yet.
This will be updated when the incident is created.

Args:
detector_id: The detector ID
open_period: The GroupOpenPeriod to link
project: The project for the group
"""
try:
# Store the alert_id in the open_period data for later lookup
data = open_period.data or {}
data["pending_incident_detector_id"] = detector_id
open_period.update(data=data)

return None

except Exception as e:
logger.exception(
"Failed to create placeholder IncidentGroupOpenPeriod relationship",
extra={
"detector_id": detector_id,
"open_period_id": open_period.id,
"error": str(e),
},
)
return None

@classmethod
def create_pending_relationships_for_incident(self, incident, alert_rule):
"""
Creates IncidentGroupOpenPeriod relationships for any groups that were created
before the incident. This handles the timing issue where groups might be created
before incidents.

Args:
incident: The Incident that was just created
alert_rule: The AlertRule that triggered the incident
"""
try:
# Find all open periods that have a pending incident detector_id for this alert rule
detector_id = AlertRuleDetector.objects.get(alert_rule_id=alert_rule.id).detector_id
pending_open_periods = GroupOpenPeriod.objects.filter(
data__pending_incident_detector_id=detector_id,
group__project__in=list(incident.projects.all()),
)

for open_period in pending_open_periods:
# Create the relationship
relationship = self.create_relationship(incident, open_period)
if relationship:
# Remove the pending flag from the open_period data
data = open_period.data or {}
data.pop("pending_incident_detector_id", None)
open_period.update(data=data)

except Exception as e:
logger.exception(
"Failed to create pending IncidentGroupOpenPeriod relationships",
extra={
"incident_id": incident.id,
"alert_rule_id": alert_rule.id,
"error": str(e),
},
)
Loading
Loading