lazywithclass / winston-cloudwatch

Send logs to Amazon Cloudwatch using Winston.
MIT License
258 stars 104 forks source link

Fix memory leaks in cloudwatch-integration #214

Closed StenAL closed 4 months ago

StenAL commented 1 year ago

Description

Stop memory leaking every stream name seen in cloudwatch-integration

The _postingEvents variable saved every single seen streamName as a key and these entries were never getting deleted. Since cloudwatch-integration stores _postingEvents as a top level variable, it gets shared between all instances of WinstonCloudWatch and leaks memory.

To fix this, delete entries for streams that are not posting events. This is safe to do because the only place where _postingEvents entries get read checks whether the value is falsy. If a key doesn't exist in an object, accessing it returns undefined which is falsy and the code works the same as before.

Stop memory leaking CloudWatch sequence tokens in cloudwatch-integration

The _nextToken variable saved every sequence token for every stream seen. Since cloudwatch-integration stores _nextToken as a top level variable, it gets shared between all instances of WinstonCloudWatch and leaks memory.

To fix this, add a clearSequenceToken method and call it from kthxbye to delete the entry for the WinstonCloudWatch instance that is done sending logs.

Testing

const logger = createLogger({ level: 'debug', });

const createWinstonCloudWatchAndLogMessage = (logger, logStreamName, message) => { return new Promise(async resolve => { const winstonCloudWatch = new WinstonCloudWatch({ level: 'debug', logGroupName: 'winston-cloudwatch-test', logStreamName: logStreamName, awsRegion: 'eu-west-1' }); logger.add(winstonCloudWatch); logger.info(message); winstonCloudWatch.kthxbye(() => { resolve() }); logger.remove(winstonCloudWatch); } ) }

createWinstonCloudWatchAndLogMessage(logger, "log-stream-1", "hello") .then(() => createWinstonCloudWatchAndLogMessage(logger, "log-stream-2", "world")) .then(() => createWinstonCloudWatchAndLogMessage(logger, "log-stream-3", "!")) .then(() => console.log("_postingEvents:", cloudWatchIntegration._postingEvents)) .then(() => console.log("_nextToken:", cloudWatchIntegration._nextToken));

rawpixel-vincent commented 1 year ago

tested and its working as expected for me, thank you

StenAL commented 1 year ago

Pinging @lazywithclass, is there any chance of getting this reviewed/merged?

lazywithclass commented 4 months ago

@StenAL I'm very sorry your efforts were left ignored. I realise you might not be using this anymore. In the off chance you're still using this code, do you still want this PR to be merged?

StenAL commented 4 months ago

It's not personally relevant to me anymore but I still recommend merging this PR. winston-cloudwatch gets ~100k weekly downloads and the memory leaks might be adversly affecting other consumers of this package.

lazywithclass commented 4 months ago

Sure, totally makes sense. I wanted to understand if you still used the project to get some feedback about how relevant the changes are. I tested and everything looks great. Thank you for your help!

Released in winston-cloudwatch@6.3.0