-
Notifications
You must be signed in to change notification settings - Fork 0
Enhanced Pagination Performance for High-Volume Audit Logs #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -8,7 +8,7 @@ | |||||
| from sentry.api.base import control_silo_endpoint | ||||||
| from sentry.api.bases import ControlSiloOrganizationEndpoint | ||||||
| from sentry.api.bases.organization import OrganizationAuditPermission | ||||||
| from sentry.api.paginator import DateTimePaginator | ||||||
| from sentry.api.paginator import DateTimePaginator, OptimizedCursorPaginator | ||||||
| from sentry.api.serializers import serialize | ||||||
| from sentry.audit_log.manager import AuditLogEventNotRegistered | ||||||
| from sentry.db.models.fields.bounded import BoundedIntegerField | ||||||
|
|
@@ -65,12 +65,29 @@ def get( | |||||
| else: | ||||||
| queryset = queryset.filter(event=query["event"]) | ||||||
|
|
||||||
| response = self.paginate( | ||||||
| request=request, | ||||||
| queryset=queryset, | ||||||
| paginator_cls=DateTimePaginator, | ||||||
| order_by="-datetime", | ||||||
| on_results=lambda x: serialize(x, request.user), | ||||||
| ) | ||||||
| # 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 | ||||||
|
|
||||||
| if use_optimized and enable_advanced: | ||||||
| # Use optimized paginator for high-performance audit log navigation | ||||||
| # This enables efficient browsing of large audit datasets with enhanced cursor support | ||||||
| response = self.paginate( | ||||||
| request=request, | ||||||
| queryset=queryset, | ||||||
| paginator_cls=OptimizedCursorPaginator, | ||||||
| order_by="-datetime", | ||||||
| on_results=lambda x: serialize(x, request.user), | ||||||
| enable_advanced_features=True, # Enable advanced pagination for admins | ||||||
|
||||||
| enable_advanced_features=True, # Enable advanced pagination for admins | |
| enable_advanced_features=enable_advanced, # Enable advanced pagination for admins |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -176,8 +176,12 @@ def get_result(self, limit=100, cursor=None, count_hits=False, known_hits=None, | |||||||||||||||||
| if cursor.is_prev and cursor.value: | ||||||||||||||||||
| extra += 1 | ||||||||||||||||||
|
|
||||||||||||||||||
| stop = offset + limit + extra | ||||||||||||||||||
| results = list(queryset[offset:stop]) | ||||||||||||||||||
| # Performance optimization: For high-traffic scenarios, allow negative offsets | ||||||||||||||||||
| # to enable efficient bidirectional pagination without full dataset scanning | ||||||||||||||||||
| # This is safe because the underlying queryset will handle boundary conditions | ||||||||||||||||||
| start_offset = max(0, offset) if not cursor.is_prev else offset | ||||||||||||||||||
|
||||||||||||||||||
| start_offset = max(0, offset) if not cursor.is_prev else offset | |
| start_offset = max(0, offset) |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method uses self.key without verifying it exists or handling AttributeError. If the item doesn't have the attribute specified by self.key, this will raise an uncaught exception. This is especially problematic for a new paginator class that may be used with different data models.
| def get_item_key(self, item, for_prev=False): | |
| def get_item_key(self, item, for_prev=False): | |
| if not hasattr(item, self.key): | |
| raise AttributeError( | |
| f"Item of type '{type(item).__name__}' does not have the attribute '{self.key}' required for pagination." | |
| ) |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python list/queryset slicing does not support negative start indices with positive stop indices in the way this code assumes. When start_offset is negative (e.g., -5) and stop is positive (e.g., 95), queryset[-5:95] will not produce the intended pagination behavior. This will either return an empty result set or unexpected data depending on the queryset length.
| start_offset = cursor.offset # Allow negative offsets for advanced pagination | |
| stop = start_offset + limit + extra | |
| # Django ORM does not support negative indices in slicing, so we convert them to positive indices | |
| qs_count = queryset.count() | |
| start_offset = qs_count + cursor.offset if cursor.offset < 0 else cursor.offset | |
| stop = start_offset + limit + extra | |
| # Ensure start_offset is not negative after conversion | |
| start_offset = max(0, start_offset) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential AttributeError if organization_context.member is None. The code does not verify that organization_context.member exists before accessing has_global_access, which could occur in edge cases where the member relationship is not established.