Skip to content

Conversation

constantinius
Copy link
Contributor

Closes https://linear.app/getsentry/issue/TET-767/auditlogsevents-improvements

Unifies the auditlog event definition from the split definition in register.py and events.py. Subclassing AuditLogEvent is no longer necessary, as the custom render function is now just an attribute for the object.

The benefit is that now everything is registered in a single file and event names, IDs and API names are not split. Additionally the inheritance of AuditLogEvent was only necessary for changing the render() function, adding a lot of boilerplate.

For example, this:

class ProjectEditAuditLogEvent(AuditLogEvent):
    def __init__(self):
        super().__init__(event_id=31, name="PROJECT_EDIT", api_name="project.edit")

    def render(self, audit_log_entry: AuditLogEntry):
        if "old_slug" in audit_log_entry.data:
            return "renamed project slug from {old_slug} to {new_slug}".format(
                **audit_log_entry.data
            )
        items_string = " ".join(
            f"in {key} to {value}" for (key, value) in audit_log_entry.data.items()
        )
        return "edited project settings " + items_string

becomes this:

@add_with_render_func(
    AuditLogEvent(
        event_id=31,
        name="PROJECT_EDIT",
        api_name="project.edit",
    )
)
def render_project_edit(audit_log_entry: AuditLogEntry):
    if "old_slug" in audit_log_entry.data:
        return "renamed project slug from {old_slug} to {new_slug}".format(**audit_log_entry.data)
    items_string = " ".join(f"in {key} to {value}" for (key, value) in audit_log_entry.data.items())
    return "edited project settings " + items_string

I think we can even simplify add() and add_with_render_func() to not take an AuditLogEvent so that it becomes:

add(
    event_id=30,
    name="PROJECT_ADD",
    api_name="project.create",
    template="created project {slug}",
)


@add_with_render_func(
    event_id=31,
    name="PROJECT_EDIT",
    api_name="project.edit",
)
def render_project_edit(audit_log_entry: AuditLogEntry):
    if "old_slug" in audit_log_entry.data:
        return "renamed project slug from {old_slug} to {new_slug}".format(**audit_log_entry.data)
    items_string = " ".join(f"in {key} to {value}" for (key, value) in audit_log_entry.data.items())
    return "edited project settings " + items_string

Copy link

linear bot commented Sep 1, 2025

cursor[bot]

This comment was marked as outdated.

name=name,
api_name=api_name,
template=template,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Method Overloads Mismatch Causes Potential Registry Issues

The add and add_with_render_func methods have a type signature mismatch. Their overloads declare name and api_name as required str parameters, but the implementation makes them optional. This allows AuditLogEvent objects to be created with None for these critical fields, which are then used as dictionary keys in the manager's internal registries, potentially causing issues.

Additional Locations (1)

Fix in Cursor Fix in Web

Copy link

codecov bot commented Sep 1, 2025

Codecov Report

❌ Patch coverage is 74.60317% with 64 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/sentry/audit_log/register.py 73.07% 63 Missing ⚠️
src/sentry/audit_log/manager.py 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #98574      +/-   ##
==========================================
- Coverage   81.25%   81.25%   -0.01%     
==========================================
  Files        8530     8529       -1     
  Lines      376354   376283      -71     
  Branches    23845    23845              
==========================================
- Hits       305823   305759      -64     
+ Misses      70163    70156       -7     
  Partials      368      368              

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant