Added max file size rotation limits for logging by praveen902012 · Pull Request #27107 · TryGhost/Ghost

praveen902012 · 2026-04-03T09:46:50Z

Why are you making it?
Currently, Ghost logs strictly rely on time-based rotation through Bunyan's default settings (e.g., rotating cleanly after 1d). However, sites with unexpected spikes in traffic or active diagnostics generate individual log files that expand wildly before the time threshold triggers. As prominently discussed by the community in this forum thread, this lack of strict bounds frequently causes out-of-control memory bloat, eventually crashing instances outright with "No space left on device" errors.

While the underlying logic inside @tryghost/logging already possesses an advanced @tryghost/bunyan-rotating-filestream module capable of solving this securely, it isn't transparently exposed to administrators natively unless they know the exact legacy fallback keys. This change natively hooks those missing parameters up for out-of-the-box configuration.

You can seamlessly drop that straight into your GitHub PR block! It adds immediate validity and community backing to why your Pull Request needs to be merged safely.

What does it do?
This change surfaces advanced logging features (size-based limits, archiving limits, and compression) directly into the core defaults.json configuration and intercepts them gently via loggingrc.js. Specifically:

It establishes formal defaults for advanced rotation settings such as threshold: "10m" and native gzip: true log compression.
It safely identifies when advanced parameters like threshold, gzip, or rotateExisting are present in loggingConfig.rotation and natively shifts the logger's implementation to useLibrary = true to actively embrace TryGhost's robust rotating-filestream instance.
Why is this something Ghost users or developers need?
Server administrators and self-hosters need predictable, finite hardware capacity planning. Providing a strict size cap boundary (threshold) ensures log files will never breach safe boundaries and fill up a disk unpredictably, while adding out-of-the-box gzip support radically shrinks the disk footprint dynamically. Making these configurations accessible out-of-the-box brings Ghost in line with modern twelve-factor infrastructure expectations.

Please check your PR against these items:
I've read and followed the Contributor Guide
I've explained my change
I've written an automated test to prove my change works.
(https://github.com/user-attachments/assets/27fbb6a2-fff3-4e41-ae7f-20935e81dc05)

Note

Medium Risk
Changes default logging rotation behavior (now enabled with size/total limits and gzip), which can affect operational logging and disk usage in production. Logic is small and covered by unit tests, but defaults changing may surprise existing deployments relying on previous rotation settings.

Overview
Updates the default logging.rotation configuration to enable rotation by default and add advanced controls like threshold, totalSize, rotateExisting, and gzip.

Adjusts loggingrc.js to automatically set rotation.useLibrary = true when any advanced rotation fields are present so the logger switches to the rotating-filestream implementation, and adds unit tests to validate the useLibrary toggle behavior.

^{Written by Cursor Bugbot for commit 1f928df. This will update automatically on new commits. Configure here.}

Logging size bounds and gzip compression metrics were previously ignored by native defaults because classical Bunyan strictly utilizes period-based rotation. This change safely exposes threshold, totalSize, rotateExisting, and gzip capabilities inside defaults.json and formally triggers Ghost's advanced secondary logging stream whenever detected.

coderabbitai · 2026-04-03T09:47:11Z

Walkthrough

The JSON defaults file had only its final newline removed. loggingrc.js adds logic to set loggingConfig.rotation.useLibrary = true when rotation is configured and any of threshold, gzip, or rotateExisting is present. A new unit test verifies useLibrary is true for threshold and gzip, and undefined when only enabled: true.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: adding size-based rotation limits (threshold) for logging, which aligns with the core objective of preventing oversized log files.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check	✅ Passed	The PR description comprehensively explains the motivation, implementation details, and rationale for exposing advanced logging rotation features to prevent disk space exhaustion from log file growth.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

ghost/core/loggingrc.js

ghost/core/core/shared/config/defaults.json

coderabbitai

🧹 Nitpick comments (1)

ghost/core/test/unit/loggingrc.test.js (1)

14-39: Add test coverage for rotateExisting field.

The code in loggingrc.js checks for rotateExisting alongside threshold and gzip, but there's no test verifying this behavior. Consider adding a test case:

🧪 Proposed test for rotateExisting

     assert.equal(loggingrc.rotation.useLibrary, true);
 });

+it('sets useLibrary to true when rotateExisting is provided', function () {
+    configUtils.set('logging:rotation', {rotateExisting: true});
+    
+    delete require.cache[require.resolve('../../loggingrc')];
+    const loggingrc = require('../../loggingrc');
+    
+    assert.equal(loggingrc.rotation.useLibrary, true);
+});
+
 it('does not set useLibrary when advanced fields are missing', function () {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ghost/core/test/unit/loggingrc.test.js` around lines 14 - 39, Add a unit test
in loggingrc.test.js that mirrors the existing cases for threshold/gzip: call
configUtils.set('logging:rotation', {rotateExisting: true}); clear the require
cache for '../../loggingrc', require('../../loggingrc') and assert that
loggingrc.rotation.useLibrary === true; place it alongside the existing "sets
useLibrary to true" tests so the module's handling of the rotateExisting field
is covered.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@ghost/core/test/unit/loggingrc.test.js`:
- Around line 14-39: Add a unit test in loggingrc.test.js that mirrors the
existing cases for threshold/gzip: call configUtils.set('logging:rotation',
{rotateExisting: true}); clear the require cache for '../../loggingrc',
require('../../loggingrc') and assert that loggingrc.rotation.useLibrary ===
true; place it alongside the existing "sets useLibrary to true" tests so the
module's handling of the rotateExisting field is covered.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9596ca8f-4436-4e96-90af-969c94b03628

📥 Commits

Reviewing files that changed from the base of the PR and between 07b4eb1 and 1f928df.

📒 Files selected for processing (3)

ghost/core/core/shared/config/defaults.json
ghost/core/loggingrc.js
ghost/core/test/unit/loggingrc.test.js

cursor · 2026-04-03T10:04:16Z

You have used all of your free Bugbot PR reviews.