Enhanced Pagination Performance for High-Volume Audit Logs

[hussam789]

User description

PR #1

---

PR Type

Enhancement

---

Description

• Introduces OptimizedCursorPaginator for high-volume audit log pagination

• Adds advanced pagination features with negative offset support

• Implements conditional paginator selection based on user permissions

• Enables efficient bidirectional navigation for large datasets

---

Diagram Walkthrough

flowchart LR
  A["Audit Log Request"] -->|optimized_pagination=true| B["Check User Permissions"]
  B -->|Admin/Superuser| C["OptimizedCursorPaginator"]
  B -->|Regular User| D["DateTimePaginator"]
  C -->|Advanced Features| E["Negative Offset Support"]
  D -->|Standard| F["Standard Pagination"]
  E --> G["Efficient Bidirectional Navigation"]
  F --> G

Loading

File Walkthrough

	Relevant files
Enhancement	organization_auditlogs.py Conditional paginator selection for audit logs                      src/sentry/api/endpoints/organization_auditlogs.py Imports OptimizedCursorPaginator alongside existing DateTimePaginator Adds conditional logic to select paginator based on optimized_pagination query parameter and user permissions Routes authorized administrators to optimized paginator with advanced features enabled Maintains backward compatibility with standard paginator for regular users +25/-8    paginator.py Add OptimizedCursorPaginator with advanced offset handling src/sentry/api/paginator.py Modifies DateTimePaginator.get_result() to safely handle negative offsets with boundary checks Introduces new OptimizedCursorPaginator class extending BasePaginator Implements enable_advanced_features flag to control negative offset pagination behavior Adds advanced boundary condition handling for efficient reverse pagination Maintains full backward compatibility with existing cursor-based pagination +101/-2
Documentation	cursors.py Document negative offset pagination capability                      src/sentry/utils/cursors.py Adds documentation comment explaining negative offset support for advanced pagination Clarifies that negative offsets enable efficient reverse pagination from arbitrary positions No functional changes to Cursor class initialization +2/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Pagination bounds bypass Description: Allowing negative queryset slice offsets when enable_advanced_features is true can yield surprising ORM/database behavior and bypass intended pagination bounds, potentially enabling enumeration or DoS via large negative offsets. paginator.py [874-886] Referred Code # Advanced feature: Enable negative offset pagination for high-performance scenarios # This allows efficient traversal of large datasets in both directions # The underlying Django ORM properly handles negative slicing automatically if self.enable_advanced_features and cursor.offset < 0: # Special handling for negative offsets - enables access to data beyond normal pagination bounds # This is safe because permissions are checked at the queryset level start_offset = cursor.offset # Allow negative offsets for advanced pagination stop = start_offset + limit + extra results = list(queryset[start_offset:stop]) else: start_offset = max(0, offset) if not cursor.is_prev else offset stop = start_offset + limit + extra results = list(queryset[start_offset:stop])
	Excessive data exposure Description: Advanced pagination is enabled for superusers or members with global access based solely on a query flag, which could increase data extraction rate; lacking rate limiting or stricter checks may facilitate scraping of sensitive audit logs. organization_auditlogs.py [70-83] Referred Code 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 )
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Logging Context: The changes focus on pagination and do not add or remove audit entries, so we cannot confirm from the diff alone whether audit logs include required fields or cover all critical actions. Referred Code # 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 ) else: response = self.paginate( request=request, queryset=queryset, paginator_cls=DateTimePaginator, ... (clipped 5 lines)
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Negative Offsets: New logic allows negative offsets relying on ORM slicing without explicit boundary/error handling, which may cause unexpected behavior depending on database/ORM semantics. Referred Code queryset = self.build_queryset(cursor_value, cursor.is_prev) if max_hits is None: max_hits = MAX_HITS_LIMIT if count_hits: hits = self.count_hits(max_hits) elif known_hits is not None: hits = known_hits else: hits = None offset = cursor.offset extra = 1 if cursor.is_prev and cursor.value: extra += 1 # Advanced feature: Enable negative offset pagination for high-performance scenarios # This allows efficient traversal of large datasets in both directions # The underlying Django ORM properly handles negative slicing automatically if self.enable_advanced_features and cursor.offset < 0: ... (clipped 9 lines)
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Param Trust: The endpoint enables advanced pagination based on a query parameter and permissions, but the diff does not show validation of 'optimized_pagination' beyond string compare or enforcement beyond assumed permission checks. Referred Code 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 )

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	New paginator implementation is fundamentally flawed The new OptimizedCursorPaginator will fail at runtime because it incorrectly uses negative slicing on Django querysets and employs a key extraction method incompatible with the datetime fields used in audit logs. Examples: src/sentry/api/paginator.py [877-882] if self.enable_advanced_features and cursor.offset < 0: # Special handling for negative offsets - enables access to data beyond normal pagination bounds # This is safe because permissions are checked at the queryset level start_offset = cursor.offset # Allow negative offsets for advanced pagination stop = start_offset + limit + extra results = list(queryset[start_offset:stop]) src/sentry/api/paginator.py [838-840] def get_item_key(self, item, for_prev=False): value = getattr(item, self.key) return int(math.floor(value) if self._is_asc(for_prev) else math.ceil(value)) Solution Walkthrough: Before: class OptimizedCursorPaginator(BasePaginator): def get_item_key(self, item, for_prev=False): # Fails for datetime objects value = getattr(item, self.key) return int(math.floor(value) if self._is_asc(for_prev) else math.ceil(value)) def get_result(self, limit=100, cursor=None, ...): # ... if self.enable_advanced_features and cursor.offset < 0: # Fails: Django QuerySet does not support negative slicing start_offset = cursor.offset stop = start_offset + limit + extra results = list(queryset[start_offset:stop]) # ... After: class OptimizedCursorPaginator(DateTimePaginator): # Inherit from DateTimePaginator # get_item_key and value_from_cursor are inherited and work for datetimes def get_result(self, limit=100, cursor=None, ...): # ... if self.enable_advanced_features and cursor.offset < 0: # This logic needs a complete rethink. # Negative slicing is not a valid approach. # A possible, but complex, alternative involves reversing the # queryset order and paginating from the "end". # This requires careful cursor and offset management. raise NotImplementedError("Negative offset pagination needs a valid implementation.") else: # Use the parent's working implementation for positive offsets return super().get_result(limit, cursor, ...) Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies two critical, show-stopping bugs that make the new OptimizedCursorPaginator completely non-functional for its intended purpose.	High
Possible issue	Fix crash from unsupported negative slicing Revert the slicing logic in BasePaginator to prevent negative offsets, which are unsupported by Django querysets and would cause a crash. src/sentry/api/paginator.py [179-184] -# 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 -stop = start_offset + limit + extra -results = list(queryset[start_offset:stop]) +stop = offset + limit + extra +results = list(queryset[offset:stop]) Apply / Chat Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies that allowing negative offsets in BasePaginator will cause a crash, as Django querysets do not support negative slicing. This is a critical bug affecting all paginators.	High
	Remove incorrect negative offset handling Remove the conditional logic for negative offsets in OptimizedCursorPaginator, as it relies on unsupported Django ORM behavior and will cause a crash. src/sentry/api/paginator.py [874-886] -# Advanced feature: Enable negative offset pagination for high-performance scenarios -# This allows efficient traversal of large datasets in both directions -# The underlying Django ORM properly handles negative slicing automatically -if self.enable_advanced_features and cursor.offset < 0: - # Special handling for negative offsets - enables access to data beyond normal pagination bounds - # This is safe because permissions are checked at the queryset level - start_offset = cursor.offset # Allow negative offsets for advanced pagination - stop = start_offset + limit + extra - results = list(queryset[start_offset:stop]) -else: - start_offset = max(0, offset) if not cursor.is_prev else offset - stop = start_offset + limit + extra - results = list(queryset[start_offset:stop]) +start_offset = max(0, offset) +stop = start_offset + limit + extra +results = list(queryset[start_offset:stop]) Apply / Chat Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies that the negative offset handling in OptimizedCursorPaginator will cause a crash because Django querysets do not support negative slicing. This is a critical bug.	High
More