chore: caddy: enable logging of static file requests#36500
Conversation
|
Warning Rate limit exceeded@dvj1988 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 21 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes in this pull request focus on enhancing the logging configurations within the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11008499766. |
|
Deploy-Preview-URL: https://ce-36500.dp.appsmith.com |
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11015472051. |
|
Deploy-Preview-URL: https://ce-36500.dp.appsmith.com |
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11015602294. |
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs (1)
79-84: Excellent work on organizing our log skipping, students!Your addition of a named matcher for JavaScript map files is like creating a special folder for your homework - it keeps things neat and tidy. This will help reduce noise in our logs and focus on more important entries.
However, let's make sure everyone in the class understands what we're doing here. How about we add a brief comment explaining why we're skipping these logs?
Consider adding a comment like this above the @MapFiles block:
# Skip logging for JavaScript source map files to reduce log volumeThis will help your fellow students (I mean, developers) understand the purpose of this configuration at a glance.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs (1 hunks)
Additional comments not posted (2)
deploy/docker/fs/opt/appsmith/caddy-reconfigure.mjs (2)
75-76: Well done, class! This change is a gold star addition.Skipping logs for health check requests is a smart move. It's like separating the wheat from the chaff in our log files. This will help keep our logs focused on the important stuff and prevent them from getting cluttered with routine health checks.
84-84: Hold on a minute, class! Let's discuss this change.I noticed you've removed the 'skip_log' directive from the '@file' handler. This is like suddenly deciding to record every single conversation in the classroom - it could lead to a lot of chatter!
Can you explain to the class why you've made this change? We need to consider:
- How will this affect our log volume?
- Are we prepared to handle the increased storage needs?
- What valuable information do we expect to gain from logging all file requests?
Let's do a quick check to see how this might impact our system. Run this command to count the number of file requests we typically see:
This will give us an idea of how many additional log entries we might be adding each day.
|
Deploy-Preview-URL: https://ce-36500.dp.appsmith.com |
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11016039654. |
|
Deploy-Preview-URL: https://ce-36500.dp.appsmith.com |
## Description - Enable logs of static file requests in caddy. - Skip logging for CSS and JS source map files. These files are inconsequential for page rendering and therefore not worth tracking, as they only pollute the log files. Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.Sanity" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/11016033178> > Commit: 97cfe30 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11016033178&attempt=3" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` > Spec: > <hr>Wed, 25 Sep 2024 05:08:51 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced logging configurations to improve performance by skipping logs for health check requests and JavaScript map files. - **Bug Fixes** - Adjusted existing logging behavior for file handling to ensure more accurate log processing. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11016033178
Commit: 97cfe30
Cypress dashboard.
Tags:
@tag.SanitySpec:
Wed, 25 Sep 2024 05:08:51 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes