Skip to content

Commit

Permalink
feat(crons): add audit logs for upsert and bulk edit endpoint (#83128)
Browse files Browse the repository at this point in the history
- adds `monitor.edit` audit log for bulk edit endpoint
- changes copy on `monitor.add` audit log to distinguish monitors added
with the ui vs upsert

resolves confusion that some customers had about their usage
  • Loading branch information
brendanhsentry authored Jan 15, 2025
1 parent 52e39af commit 2cf9ff6
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 28 deletions.
16 changes: 16 additions & 0 deletions src/sentry/audit_log/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -366,3 +366,19 @@ def render(self, audit_log_entry: AuditLogEntry):
rendered_text += f" from {formatted_start} to {formatted_end}"

return rendered_text


class MonitorAddAuditLogEvent(AuditLogEvent):
def __init__(self):
super().__init__(
event_id=120,
name="MONITOR_ADD",
api_name="monitor.add",
)

def render(self, audit_log_entry: AuditLogEntry):
entry_data = audit_log_entry.data
name = entry_data.get("name")
upsert = entry_data.get("upsert")

return f"added{" upsert " if upsert else " "}monitor {name}"
9 changes: 1 addition & 8 deletions src/sentry/audit_log/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,14 +282,7 @@
template="rotated a client secret for {status} integration {sentry_app}",
)
)
default_manager.add(
AuditLogEvent(
event_id=120,
name="MONITOR_ADD",
api_name="monitor.add",
template="added monitor {name}",
)
)
default_manager.add(events.MonitorAddAuditLogEvent())
default_manager.add(
AuditLogEvent(
event_id=121,
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/monitors/consumers/monitor_consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ def _ensure_monitor_with_config(
},
)
if created:
signal_monitor_created(project, None, True)
signal_monitor_created(project, None, True, monitor, None)

# Update existing monitor
if monitor and not created:
Expand Down
17 changes: 8 additions & 9 deletions src/sentry/monitors/endpoints/organization_monitor_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,16 +308,8 @@ def post(self, request: Request, organization) -> Response:
if seat_outcome != Outcome.ACCEPTED:
monitor.update(status=ObjectStatus.DISABLED)

self.create_audit_entry(
request=request,
organization=organization,
target_object=monitor.id,
event=audit_log.get_event_id("MONITOR_ADD"),
data=monitor.get_audit_log_data(),
)

project = result["project"]
signal_monitor_created(project, request.user, False)
signal_monitor_created(project, request.user, False, monitor, request)

validated_issue_alert_rule = result.get("alert_rule")
if validated_issue_alert_rule:
Expand Down Expand Up @@ -393,6 +385,13 @@ def put(self, request: Request, organization) -> Response:

monitor.update(**result)
updated.append(monitor)
self.create_audit_entry(
request=request,
organization=organization,
target_object=monitor.id,
event=audit_log.get_event_id("MONITOR_EDIT"),
data=monitor.get_audit_log_data(),
)

return self.respond(
{
Expand Down
15 changes: 14 additions & 1 deletion src/sentry/monitors/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from django.utils import timezone
from rest_framework.request import Request

from sentry import audit_log
from sentry.api.serializers.rest_framework.rule import RuleSerializer
from sentry.db.models import BoundedPositiveIntegerField
from sentry.models.group import Group
Expand All @@ -20,6 +21,7 @@
first_cron_monitor_created,
)
from sentry.users.models.user import User
from sentry.utils.audit import create_audit_entry, create_system_audit_entry
from sentry.utils.auth import AuthenticatedHttpRequest


Expand All @@ -42,12 +44,23 @@ def check_and_signal_first_monitor_created(project: Project, user, from_upsert:
)


def signal_monitor_created(project: Project, user, from_upsert: bool):
def signal_monitor_created(project: Project, user, from_upsert: bool, monitor: Monitor, request):
cron_monitor_created.send_robust(
project=project, user=user, from_upsert=from_upsert, sender=Project
)
check_and_signal_first_monitor_created(project, user, from_upsert)

create_audit_log = create_system_audit_entry if from_upsert else create_audit_entry
kwargs = {
"organization": project.organization,
**({"request": request} if not from_upsert else {}),
"target_object": monitor.id,
"event": audit_log.get_event_id("MONITOR_ADD"),
"data": {"upsert": from_upsert, **monitor.get_audit_log_data()},
}

create_audit_log(**kwargs)


def get_max_runtime(max_runtime: int | None) -> timedelta:
"""
Expand Down
21 changes: 15 additions & 6 deletions tests/sentry/monitors/consumers/test_monitor_consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from rest_framework.exceptions import ErrorDetail
from sentry_kafka_schemas.schema_types.ingest_monitors_v1 import CheckIn

from sentry import killswitches
from sentry import audit_log, killswitches
from sentry.constants import ObjectStatus
from sentry.db.models import BoundedPositiveIntegerField
from sentry.models.environment import Environment
Expand All @@ -31,7 +31,9 @@
)
from sentry.monitors.processing_errors.errors import ProcessingErrorsException, ProcessingErrorType
from sentry.monitors.types import CheckinItem
from sentry.testutils.asserts import assert_org_audit_log_exists
from sentry.testutils.cases import TestCase
from sentry.testutils.outbox import outbox_runner
from sentry.utils import json
from sentry.utils.outcomes import Outcome

Expand Down Expand Up @@ -1143,11 +1145,12 @@ def test_monitor_accept_upsert_with_seat(
check_accept_monitor_checkin.return_value = PermitCheckInStatus.ACCEPTED_FOR_UPSERT
assign_monitor_seat.return_value = Outcome.ACCEPTED

self.send_checkin(
"my-monitor",
monitor_config={"schedule": {"type": "crontab", "value": "13 * * * *"}},
environment="my-environment",
)
with outbox_runner():
self.send_checkin(
"my-monitor",
monitor_config={"schedule": {"type": "crontab", "value": "13 * * * *"}},
environment="my-environment",
)

checkin = MonitorCheckIn.objects.get(guid=self.guid)
assert checkin.status == CheckInStatus.OK
Expand All @@ -1158,6 +1161,12 @@ def test_monitor_accept_upsert_with_seat(
check_accept_monitor_checkin.assert_called_with(self.project.id, monitor.slug)
assign_monitor_seat.assert_called_with(monitor)

assert_org_audit_log_exists(
organization=self.organization,
event=audit_log.get_event_id("MONITOR_ADD"),
data={"upsert": True, **monitor.get_audit_log_data()},
)

@mock.patch("sentry.quotas.backend.assign_monitor_seat")
@mock.patch("sentry.quotas.backend.check_accept_monitor_checkin")
def test_monitor_accept_upsert_no_seat(
Expand Down
26 changes: 23 additions & 3 deletions tests/sentry/monitors/endpoints/test_organization_monitor_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,15 @@
from django.test.utils import override_settings
from rest_framework.exceptions import ErrorDetail

from sentry import audit_log
from sentry.constants import ObjectStatus
from sentry.models.rule import Rule, RuleSource
from sentry.monitors.models import Monitor, MonitorStatus, MonitorType, ScheduleType
from sentry.quotas.base import SeatAssignmentResult
from sentry.slug.errors import DEFAULT_SLUG_ERROR_MESSAGE
from sentry.testutils.asserts import assert_org_audit_log_exists
from sentry.testutils.cases import MonitorTestCase
from sentry.testutils.outbox import outbox_runner
from sentry.utils.outcomes import Outcome


Expand Down Expand Up @@ -375,7 +378,8 @@ def test_simple(self, mock_record):
"owner": f"user:{self.user.id}",
"config": {"schedule_type": "crontab", "schedule": "@daily"},
}
response = self.get_success_response(self.organization.slug, **data)
with outbox_runner():
response = self.get_success_response(self.organization.slug, **data)

monitor = Monitor.objects.get(slug=response.data["slug"])
assert monitor.organization_id == self.organization.id
Expand All @@ -393,6 +397,11 @@ def test_simple(self, mock_record):
"failure_issue_threshold": None,
"recovery_threshold": None,
}
assert_org_audit_log_exists(
organization=self.organization,
event=audit_log.get_event_id("MONITOR_ADD"),
data={"upsert": False, **monitor.get_audit_log_data()},
)

self.project.refresh_from_db()
assert self.project.flags.has_cron_monitors
Expand Down Expand Up @@ -597,13 +606,24 @@ def test_bulk_mute_unmute(self):
"ids": [monitor_one.guid, monitor_two.guid],
"isMuted": True,
}
response = self.get_success_response(self.organization.slug, **data)
assert response.status_code == 200
with outbox_runner():
response = self.get_success_response(self.organization.slug, **data)
assert response.status_code == 200

monitor_one.refresh_from_db()
monitor_two.refresh_from_db()
assert monitor_one.is_muted
assert monitor_two.is_muted
assert_org_audit_log_exists(
organization=self.organization,
event=audit_log.get_event_id("MONITOR_EDIT"),
data=monitor_one.get_audit_log_data(),
)
assert_org_audit_log_exists(
organization=self.organization,
event=audit_log.get_event_id("MONITOR_EDIT"),
data=monitor_two.get_audit_log_data(),
)

data = {
"ids": [monitor_one.guid, monitor_two.guid],
Expand Down

0 comments on commit 2cf9ff6

Please sign in to comment.