Enhanced Pagination Performance for High-Volume Audit Logs#15
Enhanced Pagination Performance for High-Volume Audit Logs#15ShashankFC wants to merge 1 commit into
Conversation
…loyments This change introduces optimized cursor-based pagination for audit log endpoints to improve performance in enterprise environments with large audit datasets. Key improvements: - Added OptimizedCursorPaginator with advanced boundary handling - Enhanced cursor offset support for efficient bi-directional navigation - Performance optimizations for administrative audit log access patterns - Backward compatible with existing DateTimePaginator implementation The enhanced paginator enables more efficient traversal of large audit datasets while maintaining security boundaries and access controls. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
CodeAnt AI is reviewing your PR. |
Nitpicks 🔍
|
| # Performance optimization for high-volume audit log access patterns | ||
| # Enable advanced pagination features for authorized administrators | ||
| use_optimized = request.GET.get("optimized_pagination") == "true" | ||
| enable_advanced = request.user.is_superuser or organization_context.member.has_global_access |
There was a problem hiding this comment.
Suggestion: Accessing organization_context.member.has_global_access without checking if member is None can raise an AttributeError when the user has organization access through means that don't populate member, breaking the audit logs endpoint for such callers. [null pointer]
Severity Level: Critical 🚨
- ❌ OrganizationAuditLogsEndpoint 500s for some token-authenticated callers.
- ❌ Optimized pagination unusable with org/API tokens lacking membership.
- ⚠️ Automation using audit logs API can intermittently fail.| enable_advanced = request.user.is_superuser or organization_context.member.has_global_access | |
| member = organization_context.member | |
| enable_advanced = request.user.is_superuser or (member is not None and member.has_global_access) |
Steps of Reproduction ✅
1. Note that `OrganizationAuditLogsEndpoint` is registered in the API URLs at
`src/sentry/api/urls.py:1425` and implemented in
`src/sentry/api/endpoints/organization_auditlogs.py:35-93` with `permission_classes =
(OrganizationAuditPermission,)` (line 40).
2. Observe in `ControlSiloOrganizationEndpoint.convert_args`
(`src/sentry/api/bases/organization.py:250-313`) that `organization_context` is fetched
via `organization_service.get_organization_by_id(..., user_id=request.user.id)` and passed
into `kwargs["organization_context"]`, and that `check_object_permissions` is then called
with this `organization_context`.
3. In `RpcUserOrganizationContext`
(`src/sentry/organizations/services/organization/model.py:331-347`), see the comment and
fields indicating `member: RpcOrganizationMember | None = None` and that "member can be
None when the given user_id does not have membership with the given organization."
4. In `SentryPermission.determine_access` (`src/sentry/api/permissions.py:148-193`), see
that for requests authenticated via org/API tokens where `request.auth` is set but
`request.user` is not an authenticated org member, `request.access` is built from
`access.from_rpc_auth(...)` (`permissions.py:178-180`), which returns an
`ApiBackedOrganizationGlobalAccess` instance (`src/sentry/auth/access.py:681-703`) with
full scopes, independent of `organization_context.member`.
5. Because `OrganizationAuditPermission.scope_map` requires `"org:write"` for `GET`
(`src/sentry/api/bases/organization.py:110-112`), such a token-based request with
appropriate scopes passes `has_object_permission` even though
`organization_context.member` is still `None` (membership is derived from `user_id` in the
RPC context, not from the token).
6. Now issue a GET request to the organization audit logs endpoint (route using
`OrganizationAuditLogsEndpoint.as_view()` from `src/sentry/api/urls.py:1425`) with: (a) an
org/API auth token granting `org:write` for the organization, (b) no corresponding user
membership in that organization (so `organization_context.member is None` per step 3), and
(c) query parameter `optimized_pagination=true`. After `convert_args` and permission
checks succeed, `OrganizationAuditLogsEndpoint.get` executes the lines at
`src/sentry/api/endpoints/organization_auditlogs.py:68-71`, evaluating `enable_advanced =
request.user.is_superuser or organization_context.member.has_global_access`. For this
non-superuser token-based call, `request.user.is_superuser` is `False`, so Python
evaluates `organization_context.member.has_global_access` on a `None` member, raising
`AttributeError: 'NoneType' object has no attribute 'has_global_access'` and causing a 500
response.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/sentry/api/endpoints/organization_auditlogs.py
**Line:** 71:71
**Comment:**
*Null Pointer: Accessing `organization_context.member.has_global_access` without checking if `member` is `None` can raise an `AttributeError` when the user has organization access through means that don't populate `member`, breaking the audit logs endpoint for such callers.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| return int(math.floor(value) if self._is_asc(for_prev) else math.ceil(value)) | ||
|
|
||
| def value_from_cursor(self, cursor): | ||
| return cursor.value |
There was a problem hiding this comment.
Suggestion: OptimizedCursorPaginator computes cursor keys with math.floor/math.ceil directly on the ordered field value and returns the raw cursor value for filtering, which will raise a TypeError and generate invalid query parameters when used with a datetime sort field like the audit log datetime column. [type error]
Severity Level: Major ⚠️
- ❌ Optimized audit log mode 500s for admin users.
- ⚠️ New high-volume audit pagination unusable with datetime ordering.
- ⚠️ Default DateTimePaginator path still works; impact scoped. | return int(math.floor(value) if self._is_asc(for_prev) else math.ceil(value)) | |
| def value_from_cursor(self, cursor): | |
| return cursor.value | |
| # Mirror DateTimePaginator semantics: convert datetimes to a numeric timestamp for cursor storage | |
| value = float(value.strftime("%s.%f")) * DateTimePaginator.multiplier | |
| return int(math.floor(value) if self._is_asc(for_prev) else math.ceil(value)) | |
| def value_from_cursor(self, cursor): | |
| # Convert stored numeric cursor value back into a datetime for queryset filtering | |
| return datetime.fromtimestamp(float(cursor.value) / DateTimePaginator.multiplier).replace( | |
| tzinfo=timezone.utc | |
| ) |
Steps of Reproduction ✅
1. Hit the organization audit logs endpoint backed by
`OrganizationAuditLogsEndpoint.get()` in
`src/sentry/api/endpoints/organization_auditlogs.py:34-47`, which builds a queryset of
`AuditLogEntry` ordered by `-datetime` and chooses `OptimizedCursorPaginator` when
`use_optimized` is true and `enable_advanced` is true (lines 68-83).
2. Make a GET request as a superuser (or a user with
`organization_context.member.has_global_access`) with `optimized_pagination=true` so that
`use_optimized` is true and `enable_advanced` is true (lines 70-73), causing
`self.paginate(..., paginator_cls=OptimizedCursorPaginator, order_by="-datetime",
enable_advanced_features=True, ...)` (lines 76-83) to be used instead of
`DateTimePaginator`.
3. Inside `OptimizedCursorPaginator.get_result()` in
`src/sentry/api/paginator.py:845-887`, `BasePaginator.__init__` has already set `self.key
= "datetime"` and `self.desc = True` based on `order_by="-datetime"` (lines 58-67), and
`queryset = self.build_queryset(cursor_value, cursor.is_prev)` (lines 852-857) returns a
`QuerySet` of `AuditLogEntry` ordered by the `datetime` field defined in
`src/sentry/models/auditlogentry.py:69`.
4. `get_result()` then calls `build_cursor(..., key=self.get_item_key, ...)` at
`src/sentry/api/paginator.py:897-905`, which invokes
`OptimizedCursorPaginator.get_item_key()` at lines 838-840 for each `AuditLogEntry`; there
`value = getattr(item, self.key)` is a `datetime.datetime` (from the `datetime` field on
`AuditLogEntry`), and `math.floor(value)` raises `TypeError: must be real number, not
datetime.datetime`, resulting in a 500 error whenever optimized audit-log pagination is
used.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/sentry/api/paginator.py
**Line:** 840:843
**Comment:**
*Type Error: `OptimizedCursorPaginator` computes cursor keys with `math.floor`/`math.ceil` directly on the ordered field value and returns the raw cursor value for filtering, which will raise a `TypeError` and generate invalid query parameters when used with a datetime sort field like the audit log `datetime` column.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.|
CodeAnt AI finished reviewing your PR. |
User description
Test 1
Replicated from ai-code-review-evaluation/sentry-cursor#1
EntelligenceAI PR Summary
This PR implements optimized pagination for organization audit logs with conditional performance improvements for authorized administrators.
Note
Medium Risk
Changes core pagination behavior (including negative offsets) and introduces a new paginator path, which could affect cursor correctness and page boundary handling on high-traffic endpoints.
Overview
Adds an opt-in, admin-only "optimized pagination" mode for the organization audit logs endpoint via
?optimized_pagination=true, switching fromDateTimePaginatorto a newOptimizedCursorPaginator.Introduces
OptimizedCursorPaginatorand adjustsBasePaginatorslicing to support negative/previous-page offsets to improve navigation over large datasets, while keeping the existing paginator as the default behavior.Written by Cursor Bugbot for commit 8ab8814. Configure here.
CodeAnt-AI Description
Add admin-only optimized pagination for organization audit logs
What Changed
Impact
✅ Faster audit log browsing for admins✅ Fewer audit log timeouts on very large datasets✅ Admin-only optimized pagination prevents accidental use by non-privileged users💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.