Skip to content
Open
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
171 changes: 171 additions & 0 deletions src/sentry/monitors/logic/incident_occurrence.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
from __future__ import annotations

import logging
import uuid
from collections import Counter
from collections.abc import Mapping, Sequence
from datetime import datetime, timezone
from typing import TYPE_CHECKING

from django.utils.text import get_text_list
from django.utils.translation import gettext_lazy as _

from sentry.issues.grouptype import MonitorIncidentType
from sentry.monitors.models import (
CheckInStatus,
MonitorCheckIn,
MonitorEnvironment,
MonitorIncident,
)
from sentry.monitors.types import SimpleCheckIn

if TYPE_CHECKING:
from django.utils.functional import _StrPromise

logger = logging.getLogger(__name__)


def create_incident_occurrence(
failed_checkins: Sequence[SimpleCheckIn],
failed_checkin: MonitorCheckIn,
incident: MonitorIncident,
received: datetime | None,
) -> None:
from sentry.issues.issue_occurrence import IssueEvidence, IssueOccurrence
from sentry.issues.producer import PayloadType, produce_occurrence_to_kafka

monitor_env = failed_checkin.monitor_environment

if monitor_env is None:
return

current_timestamp = datetime.now(timezone.utc)

# Get last successful check-in to show in evidence display
last_successful_checkin_timestamp = "Never"
last_successful_checkin = monitor_env.get_last_successful_checkin()
if last_successful_checkin:
last_successful_checkin_timestamp = last_successful_checkin.date_added.isoformat()

occurrence = IssueOccurrence(
id=uuid.uuid4().hex,
resource_id=None,
project_id=monitor_env.monitor.project_id,
event_id=uuid.uuid4().hex,
fingerprint=[incident.grouphash],
type=MonitorIncidentType,
issue_title=f"Monitor failure: {monitor_env.monitor.name}",
subtitle="Your monitor has reached its failure threshold.",
evidence_display=[
IssueEvidence(
name="Failure reason",
value=str(get_failure_reason(failed_checkins)),
important=True,
),
IssueEvidence(
name="Environment",
value=monitor_env.get_environment().name,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 AI 代码审查发现问题

📋 问题概述

发现 2 个邻近问题(第 66–67 行)

📍 问题详情

🟡 问题 1 | 严重程度: MEDIUM | 行号: 66

💬 详细说明:

  • 潜在空指针解引用
  • line 66 处直接调用 monitor_env.get_environment().name,未检查 get_environment() 是否可能返回 None。line 89 处存在相同模式。当数据库中 monitor_env 的 environment 外键为空或数据损坏时,get_environment() 返回 None,访问 .name 将抛出 AttributeError。

📝 问题代码:

                value=monitor_env.get_environment().name,

💡 修复建议:

修复策略:添加 None 检查,在 get_environment() 返回 None 时使用默认值或跳过该字段。

操作步骤:将 line 66 改为 value=monitor_env.get_environment().name if monitor_env.get_environment() else "unknown",并对 line 89 做相同处理;或提前获取 environment 变量并统一检查。

✅ 修复示例:

    env = monitor_env.get_environment()
    
    occurrence = IssueOccurrence(
        ...
        evidence_display=[
            IssueEvidence(
                name="Failure reason",
                value=str(get_failure_reason(failed_checkins)),
                important=True,
            ),
            IssueEvidence(
                name="Environment",
                value=env.name if env else "unknown",
                important=False,
            ),
            ...
        ],
        ...
    )
    
    event_data = {
        ...
        "environment": env.name if env else "unknown",
        ...
    }
🟡 问题 2 | 严重程度: MEDIUM | 行号: 67

💬 详细说明:

  • 当环境记录异常或不存在时,监控事故处理流程将中断,可能导致监控状态无法正确更新。

📝 问题代码:

environment=monitor_env.get_environment().name,

💡 修复建议:

在访问 name 属性前,先检查 get_environment() 的返回值。可以使用安全访问模式或添加异常处理。

✅ 修复示例:

env = monitor_env.get_environment()
environment=env.name if env else "unknown",

🔗 参考链接

important=False,
),
IssueEvidence(
name="Last successful check-in",
value=last_successful_checkin_timestamp,
important=False,
),
],
evidence_data={},
culprit="",
detection_time=current_timestamp,
level="error",
assignee=monitor_env.monitor.owner_actor,
)

if failed_checkin.trace_id:
trace_id = failed_checkin.trace_id.hex
else:
trace_id = None

event_data = {
"contexts": {"monitor": get_monitor_environment_context(monitor_env)},
"environment": monitor_env.get_environment().name,
"event_id": occurrence.event_id,
"fingerprint": [incident.grouphash],
"platform": "other",
"project_id": monitor_env.monitor.project_id,
# We set this to the time that the checkin that triggered the occurrence was written to relay if available
"received": (received if received else current_timestamp).isoformat(),
"sdk": None,
"tags": {
"monitor.id": str(monitor_env.monitor.guid),
"monitor.slug": str(monitor_env.monitor.slug),
"monitor.incident": str(incident.id),
},
"timestamp": current_timestamp.isoformat(),
}

if trace_id:
event_data["contexts"]["trace"] = {"trace_id": trace_id, "span_id": None}

produce_occurrence_to_kafka(
payload_type=PayloadType.OCCURRENCE,
occurrence=occurrence,
event_data=event_data,
)


HUMAN_FAILURE_STATUS_MAP: Mapping[int, _StrPromise] = {
CheckInStatus.ERROR: _("error"),
CheckInStatus.MISSED: _("missed"),
CheckInStatus.TIMEOUT: _("timeout"),
}

# Exists due to the vowel differences (A vs An) in the statuses
SINGULAR_HUMAN_FAILURE_MAP: Mapping[int, _StrPromise] = {
CheckInStatus.ERROR: _("An error check-in was detected"),
CheckInStatus.MISSED: _("A missed check-in was detected"),
CheckInStatus.TIMEOUT: _("A timeout check-in was detected"),
}


def get_failure_reason(failed_checkins: Sequence[SimpleCheckIn]):
"""
Builds a humam readible string from a list of failed check-ins.

"3 missed check-ins detected"
"2 missed check-ins, 1 timeout check-in and 1 error check-in were detected"
"A failed check-in was detected"
"""

status_counts = Counter(
checkin["status"]
for checkin in failed_checkins
if checkin["status"] in HUMAN_FAILURE_STATUS_MAP.keys()
)

if sum(status_counts.values()) == 1:
return SINGULAR_HUMAN_FAILURE_MAP[list(status_counts.keys())[0]]

human_status = get_text_list(
[
"%(count)d %(status)s" % {"count": count, "status": HUMAN_FAILURE_STATUS_MAP[status]}
for status, count in status_counts.items()
],
last_word=_("and"),
)

return _("%(problem_checkins)s check-ins detected") % {"problem_checkins": human_status}


def get_monitor_environment_context(monitor_environment: MonitorEnvironment):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 AI 代码审查发现问题

📋 问题概述

逻辑错误:config 副本修改无效。line 159-164 处创建了 monitor.config 的副本并修改 schedule_type 为可读字符串,但 line 171 返回时使用了原始的 monitor_environment.monitor.config,导致修改完全无效。

📍 问题详情

🟡 问题 1 | 严重程度: MEDIUM | 行号: 159

💬 详细说明:

  • 监控环境上下文中的 schedule_type 将始终显示原始值而非人类可读的显示值,影响监控信息的可读性和调试。

📝 问题代码:

config = monitor_environment.monitor.config.copy()

💡 修复建议:

在返回字典中使用修改后的 config 副本,而不是原始配置。将 line 171 的 monitor_environment.monitor.config 替换为 config 变量。

✅ 修复示例:

return {
    "id": str(monitor_environment.monitor.guid),
    "slug": str(monitor_environment.monitor.slug),
    "name": monitor_environment.monitor.name,
    "config": config,  # 使用修改后的 config 副本
    "status": monitor_env.status,
    "type": monitor_type,
}

🔗 参考链接

config = monitor_environment.monitor.config.copy()
if "schedule_type" in config:
config["schedule_type"] = monitor_environment.monitor.get_schedule_type_display()

return {
"id": str(monitor_environment.monitor.guid),
"slug": str(monitor_environment.monitor.slug),
"name": monitor_environment.monitor.name,
"config": monitor_environment.monitor.config,
"status": monitor_environment.get_status_display(),
"type": monitor_environment.monitor.get_type_display(),
}
104 changes: 104 additions & 0 deletions src/sentry/monitors/logic/incidents.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
from __future__ import annotations

import logging
from datetime import datetime
from typing import cast

from sentry.monitors.logic.incident_occurrence import create_incident_occurrence
from sentry.monitors.models import CheckInStatus, MonitorCheckIn, MonitorIncident, MonitorStatus
from sentry.monitors.types import SimpleCheckIn

logger = logging.getLogger(__name__)


def try_incident_threshold(
failed_checkin: MonitorCheckIn,
failure_issue_threshold: int,
received: datetime | None,
) -> bool:
from sentry.signals import monitor_environment_failed

monitor_env = failed_checkin.monitor_environment

if monitor_env is None:
return False

# check to see if we need to update the status
if monitor_env.status in [MonitorStatus.OK, MonitorStatus.ACTIVE]:
if failure_issue_threshold == 1:
previous_checkins: list[SimpleCheckIn] = [
{
"id": failed_checkin.id,
"date_added": failed_checkin.date_added,
"status": failed_checkin.status,
}
]
else:
previous_checkins = cast(
list[SimpleCheckIn],
# Using .values for performance reasons
MonitorCheckIn.objects.filter(
monitor_environment=monitor_env, date_added__lte=failed_checkin.date_added
)
.order_by("-date_added")
.values("id", "date_added", "status"),
)

# reverse the list after slicing in order to start with oldest check-in
previous_checkins = list(reversed(previous_checkins[:failure_issue_threshold]))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 AI 代码审查发现问题

📋 问题概述

阈值检查逻辑不完整
line 48 在截取 previous_checkins[:failure_issue_threshold] 后,没有验证返回的记录数量是否达到 failure_issue_threshold。如果数据库中符合条件的记录少于阈值,代码仍然会继续执行并可能错误地判定为 incident 状态。
例如:threshold=5 但数据库只有 3 条记录,这 3 条都是失败的,代码会错误地认为达到了 incident 阈值。

📍 问题详情

🟡 问题 1 | 严重程度: MEDIUM | 行号: 48

💬 详细说明:

  • 在监控器历史数据不足的情况下,可能导致过早或错误地触发 incident,产生误报,影响运维人员对真实故障的判断。

📝 问题代码:

previous_checkins = list(reversed(previous_checkins[:failure_issue_threshold]))

💡 修复建议:

在截取列表后添加长度检查,确保记录数量达到阈值要求。修改建议:
previous_checkins = list(reversed(previous_checkins[:failure_issue_threshold]))
if len(previous_checkins) < failure_issue_threshold:
return False

✅ 修复示例:

            # reverse the list after slicing in order to start with oldest check-in
            previous_checkins = list(reversed(previous_checkins[:failure_issue_threshold]))

            # Ensure we have enough check-ins to meet the threshold
            if len(previous_checkins) < failure_issue_threshold:
                return False

            # If we have any successful check-ins within the threshold of
            # commits we have NOT reached an incident state
            if any([checkin["status"] == CheckInStatus.OK for checkin in previous_checkins]):
                return False

🔗 参考链接


# If we have any successful check-ins within the threshold of
# commits we have NOT reached an incident state
if any([checkin["status"] == CheckInStatus.OK for checkin in previous_checkins]):
return False

# change monitor status + update fingerprint timestamp
monitor_env.status = MonitorStatus.ERROR
monitor_env.save(update_fields=("status",))

starting_checkin = previous_checkins[0]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 AI 代码审查发现问题

📋 问题概述

潜在 IndexError 风险
line 59 直接访问 previous_checkins[0],但没有检查列表是否为空。在 else 分支(line 36-53)中,当 failure_issue_threshold > 1 时,previous_checkins 来自数据库查询结果。如果查询返回空列表(例如新创建的 monitor 还没有 check-in 记录),previous_checkins[0] 将抛出 IndexError。
触发路径:line 27 条件为真 → line 36 else 分支 → line 48 查询返回空列表 → line 59 访问 [0] 崩溃。

📍 问题详情

🟡 问题 1 | 严重程度: MEDIUM | 行号: 59

💬 详细说明:

  • 对于新创建的监控器或历史数据被清理的监控器,当发生失败时会导致事故处理流程崩溃,无法创建 incident 记录,影响监控系统的可靠性。

📝 问题代码:

starting_checkin = previous_checkins[0]

💡 修复建议:

在访问 previous_checkins[0] 前添加空列表检查。如果列表为空,应该返回 False 或采取其他适当的错误处理措施。修改建议:
if not previous_checkins:
return False
starting_checkin = previous_checkins[0]

✅ 修复示例:

        if not previous_checkins:
            logger.warning(
                "No previous check-ins found for monitor environment %s",
                monitor_env.id
            )
            return False

        starting_checkin = previous_checkins[0]

🔗 参考链接


incident: MonitorIncident | None
incident, _ = MonitorIncident.objects.get_or_create(
monitor_environment=monitor_env,
resolving_checkin=None,
defaults={
"monitor": monitor_env.monitor,
"starting_checkin_id": starting_checkin["id"],
"starting_timestamp": starting_checkin["date_added"],
},
)

elif monitor_env.status == MonitorStatus.ERROR:
# if monitor environment has a failed status, use the failed
# check-in and send occurrence
previous_checkins = [
{
"id": failed_checkin.id,
"date_added": failed_checkin.date_added,
"status": failed_checkin.status,
}
]

# get the active incident from the monitor environment
incident = monitor_env.active_incident
else:
# don't send occurrence for other statuses
return False

# Only create an occurrence if:
# - We have an active incident and fingerprint
# - 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:
create_incident_occurrence(
previous_checkins,
checkin,
incident,
received=received,
)

monitor_environment_failed.send(monitor_environment=monitor_env, sender=type(monitor_env))

return True
Loading