-
Notifications
You must be signed in to change notification settings - Fork 16
fix: truncate log files when they take up more than 5mb of space #1530
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
Conversation
WalkthroughThis change removes the custom logrotate configuration management system, including its modification class, patch file, and related tests. Instead, log rotation is now handled directly within the service by truncating the log file if it exceeds 5MB, checked every 20 minutes. The configuration file is also updated with new and modified fields. Changes
Sequence Diagram(s)sequenceDiagram
participant Cron as Cron Scheduler
participant LogRotateService as LogRotateService
participant FS as File System
Cron->>LogRotateService: Trigger handleCron() every 20 minutes
LogRotateService->>FS: stat('/var/log/graphql-api.log')
alt File exists and size > 5MB
LogRotateService->>FS: truncate('/var/log/graphql-api.log')
LogRotateService->>LogRotateService: Log truncation success
else File does not exist
LogRotateService->>LogRotateService: Log file missing, skip truncation
else Other error
LogRotateService->>LogRotateService: Log error
end
Estimated code review effort2 (~15 minutes) Possibly related PRs
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
api/dev/configs/connect.json (2)
2-3: Opening WAN access in a dev profile—double-check exposureSetting
"wanaccess": trueand"wanport": 8443makes the dev API reachable from outside the LAN.
Confirm this is intentional, that firewalls are configured, and that no sensitive data is reachable from an unauthenticated path.
If WAN exposure is only needed in prod, consider keeping the dev profile disabled or gating it behind an env-var toggle.
14-14: Prefer boolean forsandboxflagUsing the string
"yes"can cause truthiness bugs or schema validation failures. A boolean conveys intent more clearly.- "sandbox": "yes", + "sandbox": true,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
api/dev/configs/connect.json(1 hunks)
📓 Path-based instructions (1)
api/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
api/**/*: Use pnpm as the only package manager
Always run scripts from api/package.json unless specifically requested otherwise
Files:
api/dev/configs/connect.json
🧠 Learnings (2)
📓 Common learnings
Learnt from: elibosley
PR: unraid/api#1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:33-37
Timestamp: 2025-01-29T16:36:04.777Z
Learning: In the Unraid API, FileModification implementations (like LogRotateModification) don't need to handle errors internally as error handling is managed at the UnraidFileModifierService level.
Learnt from: elibosley
PR: unraid/api#1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:33-37
Timestamp: 2025-01-29T16:36:04.777Z
Learning: The UnraidFileModifierService in the Unraid API provides comprehensive error handling for all FileModification implementations. It includes try/catch blocks, detailed error logging, and safe rollback mechanisms. Individual FileModification implementations (like LogRotateModification) should allow errors to propagate to this service layer rather than handling them internally.
api/dev/configs/connect.json (2)
Learnt from: elibosley
PR: #1425
File: api/dev/configs/connect.json:5-13
Timestamp: 2025-06-20T20:50:14.050Z
Learning: Connect configuration files like api/dev/configs/connect.json use their own naming conventions (e.g., apikey, accesstoken, idtoken, refreshtoken) as implementation details, and these should not be changed to follow general camelCase patterns.
Learnt from: pujitm
PR: #1211
File: web/codegen.ts:14-14
Timestamp: 2025-03-12T13:35:43.900Z
Learning: The JSON scalar type in web/codegen.ts was temporarily changed from 'string' to 'any' for compatibility with JsonForms integration. This change facilitates the implementation of the Connect settings web component.
🪛 Gitleaks (8.27.2)
api/dev/configs/connect.json
5-5: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🧰 Additional context used
📓 Path-based instructions (1)
api/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/api-rules.mdc)
api/**/*: Use pnpm as the only package manager
Always run scripts from api/package.json unless specifically requested otherwise
Files:
api/dev/configs/connect.json
🧠 Learnings (2)
📓 Common learnings
Learnt from: elibosley
PR: unraid/api#1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:33-37
Timestamp: 2025-01-29T16:36:04.777Z
Learning: In the Unraid API, FileModification implementations (like LogRotateModification) don't need to handle errors internally as error handling is managed at the UnraidFileModifierService level.
Learnt from: elibosley
PR: unraid/api#1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:33-37
Timestamp: 2025-01-29T16:36:04.777Z
Learning: The UnraidFileModifierService in the Unraid API provides comprehensive error handling for all FileModification implementations. It includes try/catch blocks, detailed error logging, and safe rollback mechanisms. Individual FileModification implementations (like LogRotateModification) should allow errors to propagate to this service layer rather than handling them internally.
api/dev/configs/connect.json (2)
Learnt from: elibosley
PR: #1425
File: api/dev/configs/connect.json:5-13
Timestamp: 2025-06-20T20:50:14.050Z
Learning: Connect configuration files like api/dev/configs/connect.json use their own naming conventions (e.g., apikey, accesstoken, idtoken, refreshtoken) as implementation details, and these should not be changed to follow general camelCase patterns.
Learnt from: pujitm
PR: #1211
File: web/codegen.ts:14-14
Timestamp: 2025-03-12T13:35:43.900Z
Learning: The JSON scalar type in web/codegen.ts was temporarily changed from 'string' to 'any' for compatibility with JsonForms integration. This change facilitates the implementation of the Connect settings web component.
🪛 Gitleaks (8.27.2)
api/dev/configs/connect.json
5-5: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build API
- GitHub Check: Build Web App
- GitHub Check: Test API
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
api/dev/configs/connect.json (1)
12-13: VerifyextraOriginsshape matches the consumer codeThe field holds a comma-separated string. If the consuming code expects an array (common for CORS/Origin lists), this will break parsing.
- "extraOrigins": "https://google.com,https://test.com", + "extraOrigins": ["https://google.com", "https://test.com"],Please confirm the expected type and adjust accordingly.
| "apikey": "test-api-key-12345", | ||
| "localApiKey": "_______________________LOCAL_API_KEY_HERE_________________________", |
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.
Possible secret committed – replace with placeholder or env var
"apikey": "test-api-key-12345" is flagged by gitleaks. Even if it’s a dummy value, storing any key-looking string in VCS trains developers to commit real ones later.
- "apikey": "test-api-key-12345",
+ "apikey": "<REPLACE_ME>",Ideally pull this from an environment variable at runtime instead of hard-coding.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "apikey": "test-api-key-12345", | |
| "localApiKey": "_______________________LOCAL_API_KEY_HERE_________________________", | |
| "apikey": "<REPLACE_ME>", | |
| "localApiKey": "_______________________LOCAL_API_KEY_HERE_________________________", |
🧰 Tools
🪛 Gitleaks (8.27.2)
5-5: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In api/dev/configs/connect.json at lines 5 to 6, the "apikey" field contains a
hard-coded key string which is flagged as a possible secret. Replace the
hard-coded "apikey" value with a placeholder string or remove it entirely, and
modify the application code to load the actual API key from an environment
variable at runtime instead of storing it in the config file.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
| this.logger.debug(`Log file size (${stats.size} bytes) exceeds limit, truncating`); | ||
| await execa('truncate', ['-s', '0', this.logFilePath]); |
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.
does the log before truncating immediately get voided into the aether?
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.
lmao yes.
🤖 I have created a release *beep* *boop* --- ## [4.11.0](v4.10.0...v4.11.0) (2025-07-28) ### Features * tailwind v4 ([#1522](#1522)) ([2c62e0a](2c62e0a)) * **web:** install and configure nuxt ui ([#1524](#1524)) ([407585c](407585c)) ### Bug Fixes * add missing breakpoints ([#1535](#1535)) ([f5352e3](f5352e3)) * border color incorrect in tailwind ([#1544](#1544)) ([f14b74a](f14b74a)) * **connect:** omit extraneous fields during connect config validation ([#1538](#1538)) ([45bd736](45bd736)) * **deps:** pin dependencies ([#1528](#1528)) ([a74d935](a74d935)) * **deps:** pin dependency @nuxt/ui to 3.2.0 ([#1532](#1532)) ([8279531](8279531)) * **deps:** update all non-major dependencies ([#1510](#1510)) ([1a8da6d](1a8da6d)) * **deps:** update all non-major dependencies ([#1520](#1520)) ([e2fa648](e2fa648)) * inject Tailwind CSS into client entry point ([#1537](#1537)) ([86b6c4f](86b6c4f)) * make settings grid responsive ([#1463](#1463)) ([9dfdb8d](9dfdb8d)) * **notifications:** gracefully handle & mask invalid notifications ([#1529](#1529)) ([05056e7](05056e7)) * truncate log files when they take up more than 5mb of space ([#1530](#1530)) ([0a18b38](0a18b38)) * use async for primary file read/writes ([#1531](#1531)) ([23b2b88](23b2b88)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [4.11.0](v4.10.0...v4.11.0) (2025-07-28) ### Features * tailwind v4 ([#1522](#1522)) ([2c62e0a](2c62e0a)) * **web:** install and configure nuxt ui ([#1524](#1524)) ([407585c](407585c)) ### Bug Fixes * add missing breakpoints ([#1535](#1535)) ([f5352e3](f5352e3)) * border color incorrect in tailwind ([#1544](#1544)) ([f14b74a](f14b74a)) * **connect:** omit extraneous fields during connect config validation ([#1538](#1538)) ([45bd736](45bd736)) * **deps:** pin dependencies ([#1528](#1528)) ([a74d935](a74d935)) * **deps:** pin dependency @nuxt/ui to 3.2.0 ([#1532](#1532)) ([8279531](8279531)) * **deps:** update all non-major dependencies ([#1510](#1510)) ([1a8da6d](1a8da6d)) * **deps:** update all non-major dependencies ([#1520](#1520)) ([e2fa648](e2fa648)) * inject Tailwind CSS into client entry point ([#1537](#1537)) ([86b6c4f](86b6c4f)) * make settings grid responsive ([#1463](#1463)) ([9dfdb8d](9dfdb8d)) * **notifications:** gracefully handle & mask invalid notifications ([#1529](#1529)) ([05056e7](05056e7)) * truncate log files when they take up more than 5mb of space ([#1530](#1530)) ([0a18b38](0a18b38)) * use async for primary file read/writes ([#1531](#1531)) ([23b2b88](23b2b88)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Bug Fixes
Chores