-
Notifications
You must be signed in to change notification settings - Fork 17
SHARD-2741 - feat: log file rotation strategy changes #549
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
base: dev
Are you sure you want to change the base?
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
main: { | ||
type: 'dateFile', | ||
pattern: 'yyyy-MM-dd-hh', | ||
keepFileExt: true, | ||
maxLogSize: 10000000, | ||
backups: 10, | ||
numBackups: 24, | ||
compress: false, | ||
alwaysIncludePattern: false, | ||
}, | ||
app: { | ||
type: 'dateFile', | ||
pattern: 'yyyy-MM-dd-hh', | ||
keepFileExt: true, | ||
maxLogSize: 10000000, | ||
backups: 10, | ||
numBackups: 24, | ||
compress: false, | ||
alwaysIncludePattern: false, | ||
}, | ||
p2p: { | ||
type: 'dateFile', | ||
pattern: 'yyyy-MM-dd-hh', | ||
keepFileExt: true, | ||
maxLogSize: 10000000, | ||
backups: 10, | ||
numBackups: 24, | ||
compress: false, | ||
alwaysIncludePattern: false, | ||
}, |
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.
Suggestion: The pattern
value 'yyyy-MM-dd-hh'
may cause log files to rotate every hour, potentially leading to excessive file creation and disk usage. Consider using 'yyyy-MM-dd'
for daily rotation unless hourly rotation is strictly required. [general, importance: 7]
main: { | |
type: 'dateFile', | |
pattern: 'yyyy-MM-dd-hh', | |
keepFileExt: true, | |
maxLogSize: 10000000, | |
backups: 10, | |
numBackups: 24, | |
compress: false, | |
alwaysIncludePattern: false, | |
}, | |
app: { | |
type: 'dateFile', | |
pattern: 'yyyy-MM-dd-hh', | |
keepFileExt: true, | |
maxLogSize: 10000000, | |
backups: 10, | |
numBackups: 24, | |
compress: false, | |
alwaysIncludePattern: false, | |
}, | |
p2p: { | |
type: 'dateFile', | |
pattern: 'yyyy-MM-dd-hh', | |
keepFileExt: true, | |
maxLogSize: 10000000, | |
backups: 10, | |
numBackups: 24, | |
compress: false, | |
alwaysIncludePattern: false, | |
}, | |
main: { | |
type: 'dateFile', | |
pattern: 'yyyy-MM-dd', | |
keepFileExt: true, | |
maxLogSize: 10000000, | |
backups: 10, | |
numBackups: 24, | |
compress: false, | |
alwaysIncludePattern: false, | |
}, | |
app: { | |
type: 'dateFile', | |
pattern: 'yyyy-MM-dd', | |
keepFileExt: true, | |
maxLogSize: 10000000, | |
backups: 10, | |
numBackups: 24, | |
compress: false, | |
alwaysIncludePattern: false, | |
}, | |
p2p: { | |
type: 'dateFile', | |
pattern: 'yyyy-MM-dd', | |
keepFileExt: true, | |
maxLogSize: 10000000, | |
backups: 10, | |
numBackups: 24, | |
compress: false, | |
alwaysIncludePattern: false, | |
}, |
if (appender.type !== 'file' && appender.type !== 'dateFile') continue | ||
appender.filename = `${this.logDir}/${key}.log` | ||
} |
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.
Suggestion: Setting a filename
property on appenders of type 'dateFile'
may require the filename to include the pattern placeholder for correct log rotation. Ensure the filename supports pattern substitution if required by the logging library. [general, importance: 6]
if (appender.type !== 'file' && appender.type !== 'dateFile') continue | |
appender.filename = `${this.logDir}/${key}.log` | |
} | |
if (appender.type !== 'file' && appender.type !== 'dateFile') continue | |
appender.filename = appender.type === 'dateFile' | |
? `${this.logDir}/${key}.log` | |
: `${this.logDir}/${key}.log` | |
// If the logging library requires, consider: `${this.logDir}/${key}.log` or `${this.logDir}/${key}-%DATE%.log` |
User description
This is how the logs will look... For local testing, I created file per 100KB - actual code has 10MB size for rollover.
PR Type
Enhancement
Description
Switch log rotation to hourly with
dateFile
appenderAdd new log rotation options: pattern, numBackups, etc.
Update type definitions for new log config options
Ensure logger supports both 'file' and 'dateFile' appenders
Changes walkthrough 📝
logs.ts
Switch log rotation to hourly `dateFile` with new options
src/config/logs.ts
main
,app
, andp2p
to usedateFile
typeyyyy-MM-dd-hh
) for these logskeepFileExt
,numBackups
(set to 24),compress
,alwaysIncludePattern
index.ts
Support filename assignment for `dateFile` appenders
src/logger/index.ts
file
anddateFile
appenders whenassigning filenames
shardus-types.ts
Update log configuration types for new rotation options
src/shardus/shardus-types.ts
main
,app
, andp2p
log appenderspattern
,keepFileExt
,numBackups
,compress
,alwaysIncludePattern