Skip to content

Chore: Preview endpoint logging#10068

Merged
Meldiron merged 1 commit into1.7.xfrom
chore-preview-logging
Jun 26, 2025
Merged

Chore: Preview endpoint logging#10068
Meldiron merged 1 commit into1.7.xfrom
chore-preview-logging

Conversation

@Meldiron
Copy link
Copy Markdown
Contributor

What does this PR do?

Adds logging to help to understand CPU usage of preview endpoint

Test Plan

Manual QA:

CleanShot 2025-06-26 at 20 20 55@2x

Related PRs and Issues

x

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jun 26, 2025

📝 Walkthrough

Walkthrough

The change introduces detailed performance timing and logging to the file preview generation endpoint in the storage API. The action method for the preview route now receives additional injected parameters: the Request and Project objects. Timing checkpoints are added to measure the durations of file download, decryption, decompression, and image rendering steps. After processing, a log entry is generated containing the project ID, bucket ID, file ID, request URI, and all measured timing data. The core logic for file handling and response remains unchanged, and the timing instrumentation does not affect control flow or error handling.


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 368f928 and a356e86.

📒 Files selected for processing (1)
  • app/controllers/api/storage.php (6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (18)
  • GitHub Check: Benchmark
  • GitHub Check: E2E Service Test (Users)
  • GitHub Check: E2E Service Test (Webhooks)
  • GitHub Check: E2E Service Test (Proxy)
  • GitHub Check: E2E Service Test (Messaging)
  • GitHub Check: E2E Service Test (Storage)
  • GitHub Check: E2E Service Test (Sites)
  • GitHub Check: E2E Service Test (Realtime)
  • GitHub Check: E2E Service Test (Health)
  • GitHub Check: E2E Service Test (Projects)
  • GitHub Check: E2E Service Test (Locale)
  • GitHub Check: E2E Service Test (FunctionsSchedule)
  • GitHub Check: E2E Service Test (GraphQL)
  • GitHub Check: E2E Service Test (Functions)
  • GitHub Check: E2E Service Test (Account)
  • GitHub Check: E2E Service Test (Databases)
  • GitHub Check: E2E Service Test (Dev Keys)
  • GitHub Check: E2E General Test
🔇 Additional comments (7)
app/controllers/api/storage.php (7)

21-21: LGTM: Console import for logging functionality.

The Console import is correctly added to support the new logging functionality being introduced in the preview endpoint.


957-957: LGTM: Dependency injection for logging context.

The injection of request and project services provides the necessary context (URI and project ID) for the performance logging without affecting the core functionality.

Also applies to: 963-963


964-964: LGTM: Method signature updated correctly.

The action method signature is properly updated to include the newly injected Request $request and Document $project parameters, maintaining parameter order consistency with the injection declarations.


1041-1046: LGTM: Download timing measurement implementation.

The timing measurement correctly captures the start time before file reading and calculates the download duration immediately after. This provides accurate measurement of file I/O performance.


1058-1059: LGTM: Decryption timing calculation.

The decryption time calculation correctly accounts for the cumulative time by subtracting the previously measured download time, providing an isolated measurement of decryption performance.


1071-1072: LGTM: Decompression timing calculation.

The decompression timing correctly isolates this phase by subtracting both download and decryption times from the total elapsed time.


1103-1108: LGTM: Comprehensive performance logging implementation.

The timing calculations and logging implementation provide excellent visibility into performance bottlenecks:

  • Rendering time correctly isolates image processing duration
  • Total time provides end-to-end measurement
  • Log message includes all relevant context (project, bucket, file, URI) and timing breakdowns
  • The structured format will be useful for performance analysis and monitoring

This implementation successfully addresses the PR objective of understanding CPU usage in the preview endpoint.

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link
Copy Markdown

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
binutils 2.42-r0 CVE-2025-0840 HIGH
libexpat 2.6.4-r0 CVE-2024-8176 HIGH
libxml2 2.12.7-r0 CVE-2024-56171 HIGH
libxml2 2.12.7-r0 CVE-2025-24928 HIGH
libxml2 2.12.7-r0 CVE-2025-27113 HIGH
libxml2 2.12.7-r0 CVE-2025-32414 HIGH
libxml2 2.12.7-r0 CVE-2025-32415 HIGH
pyc 3.12.9-r0 CVE-2024-12718 HIGH
pyc 3.12.9-r0 CVE-2025-4138 HIGH
pyc 3.12.9-r0 CVE-2025-4517 HIGH
python3 3.12.9-r0 CVE-2024-12718 HIGH
python3 3.12.9-r0 CVE-2025-4138 HIGH
python3 3.12.9-r0 CVE-2025-4517 HIGH
python3-pyc 3.12.9-r0 CVE-2024-12718 HIGH
python3-pyc 3.12.9-r0 CVE-2025-4138 HIGH
python3-pyc 3.12.9-r0 CVE-2025-4517 HIGH
python3-pycache-pyc0 3.12.9-r0 CVE-2024-12718 HIGH
python3-pycache-pyc0 3.12.9-r0 CVE-2025-4138 HIGH
python3-pycache-pyc0 3.12.9-r0 CVE-2025-4517 HIGH
sqlite-libs 3.45.3-r1 CVE-2025-29087 HIGH
xz 5.6.2-r0 CVE-2025-31115 HIGH
xz-libs 5.6.2-r0 CVE-2025-31115 HIGH
golang.org/x/crypto v0.31.0 CVE-2025-22869 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

@github-actions
Copy link
Copy Markdown

✨ Benchmark results

  • Requests per second: 919
  • Requests with 200 status code: 165,383
  • P99 latency: 0.207368093

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 919 1,131
200 165,383 203,551
P99 0.207368093 0.168795012

@Meldiron Meldiron merged commit 42a0d64 into 1.7.x Jun 26, 2025
37 of 38 checks passed
@christyjacob4 christyjacob4 deleted the chore-preview-logging branch June 26, 2025 19:01
@coderabbitai coderabbitai bot mentioned this pull request Jul 1, 2025
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants