From ebb2cfb555acbb63a2200ac70ca523450be3965d Mon Sep 17 00:00:00 2001 From: yuhengshs Date: Thu, 12 Sep 2024 10:17:34 -0700 Subject: [PATCH 1/5] set log output limits to 5 --- .../src/Providers/AWSCloudWatchProvider.ts | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/core/src/Providers/AWSCloudWatchProvider.ts b/packages/core/src/Providers/AWSCloudWatchProvider.ts index 278f0f10ee8..8ce9d94b274 100644 --- a/packages/core/src/Providers/AWSCloudWatchProvider.ts +++ b/packages/core/src/Providers/AWSCloudWatchProvider.ts @@ -55,6 +55,8 @@ class AWSCloudWatchProvider implements LoggingProvider { private _currentLogBatch: InputLogEvent[]; private _timer; private _nextSequenceToken: string | undefined; + private _maxRetries = 5; + private _retryCount; constructor(config?: AWSCloudWatchProviderOptions) { this.configure(config); @@ -494,11 +496,22 @@ class AWSCloudWatchProvider implements LoggingProvider { try { if (this._getDocUploadPermissibility()) { await this._safeUploadLogEvents(); + this._retryCount = 0; } } catch (err) { - logger.error( - `error when calling _safeUploadLogEvents in the timer interval - ${err}` - ); + this._retryCount++; + if (this._retryCount < this._maxRetries) { + logger.error( + `error when calling _safeUploadLogEvents in the timer interval - ${err}` + ); + logger.error( + `Max retries (${this._maxRetries}) reached. Stopping log uploads.` + ); + } else if (this._retryCount == this._maxRetries) { + logger.error( + `Max retries (${this._maxRetries}) reached. Stopping log uploads.` + ); + } } }, 2000); } From b92ff49f18c065e21fa3a9cf9f0f6a7c91f2d134 Mon Sep 17 00:00:00 2001 From: yuhengshs Date: Thu, 12 Sep 2024 10:44:59 -0700 Subject: [PATCH 2/5] updated error message --- packages/core/src/Providers/AWSCloudWatchProvider.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/core/src/Providers/AWSCloudWatchProvider.ts b/packages/core/src/Providers/AWSCloudWatchProvider.ts index 8ce9d94b274..604616ee6a8 100644 --- a/packages/core/src/Providers/AWSCloudWatchProvider.ts +++ b/packages/core/src/Providers/AWSCloudWatchProvider.ts @@ -504,12 +504,9 @@ class AWSCloudWatchProvider implements LoggingProvider { logger.error( `error when calling _safeUploadLogEvents in the timer interval - ${err}` ); - logger.error( - `Max retries (${this._maxRetries}) reached. Stopping log uploads.` - ); } else if (this._retryCount == this._maxRetries) { logger.error( - `Max retries (${this._maxRetries}) reached. Stopping log uploads.` + `CloudWatch log upload failed after ${this._maxRetries} attempts. Suppressing further error logs. Upload attempts will continue in the background.` ); } } From 6d98cda16dd1b523e3d5de57331e1664a98b3a7e Mon Sep 17 00:00:00 2001 From: yuhengshs Date: Thu, 12 Sep 2024 11:02:38 -0700 Subject: [PATCH 3/5] updated syntax error --- packages/core/src/Providers/AWSCloudWatchProvider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/Providers/AWSCloudWatchProvider.ts b/packages/core/src/Providers/AWSCloudWatchProvider.ts index 604616ee6a8..37d4a688649 100644 --- a/packages/core/src/Providers/AWSCloudWatchProvider.ts +++ b/packages/core/src/Providers/AWSCloudWatchProvider.ts @@ -504,7 +504,7 @@ class AWSCloudWatchProvider implements LoggingProvider { logger.error( `error when calling _safeUploadLogEvents in the timer interval - ${err}` ); - } else if (this._retryCount == this._maxRetries) { + } else if (this._retryCount === this._maxRetries) { logger.error( `CloudWatch log upload failed after ${this._maxRetries} attempts. Suppressing further error logs. Upload attempts will continue in the background.` ); From 2a28c2e3ff7ab0e53c851d66d40b0e4ce7c0beeb Mon Sep 17 00:00:00 2001 From: yuhengshs Date: Tue, 17 Sep 2024 08:29:06 -0700 Subject: [PATCH 4/5] added unit tests for retry and max retry mechanism --- .../Providers/AWSCloudWatchProvider-test.ts | 134 ++++++++++++++++-- .../src/Providers/AWSCloudWatchProvider.ts | 26 +++- 2 files changed, 138 insertions(+), 22 deletions(-) diff --git a/packages/core/__tests__/Providers/AWSCloudWatchProvider-test.ts b/packages/core/__tests__/Providers/AWSCloudWatchProvider-test.ts index 6079438114d..ee5b362f50a 100644 --- a/packages/core/__tests__/Providers/AWSCloudWatchProvider-test.ts +++ b/packages/core/__tests__/Providers/AWSCloudWatchProvider-test.ts @@ -14,6 +14,7 @@ import { DescribeLogStreamsCommand, PutLogEventsCommand, } from '@aws-sdk/client-cloudwatch-logs'; +import { loggers } from 'winston'; const credentials = { accessKeyId: 'accessKeyId', @@ -178,33 +179,52 @@ describe('AWSCloudWatchProvider', () => { }); describe('pushLogs test', () => { + let provider; + let mockInitiateLogPushInterval; + beforeEach(() => { + provider = new AWSCloudWatchProvider(testConfig); + mockInitiateLogPushInterval = jest + .spyOn(provider as any, '_initiateLogPushInterval') + .mockImplementation(); + }); + afterEach(() => { + jest.clearAllMocks(); + }); it('should add the provided logs to the log queue', () => { - const provider = new AWSCloudWatchProvider(testConfig); provider.pushLogs([{ message: 'hello', timestamp: 1111 }]); let logQueue = provider.getLogQueue(); - expect(logQueue).toHaveLength(1); provider.pushLogs([ - { - message: 'goodbye', - timestamp: 1112, - }, - { - message: 'ohayou', - timestamp: 1113, - }, - { - message: 'konbanwa', - timestamp: 1114, - }, + { message: 'goodbye', timestamp: 1112 }, + { message: 'ohayou', timestamp: 1113 }, + { message: 'konbanwa', timestamp: 1114 }, ]); logQueue = provider.getLogQueue(); - expect(logQueue).toHaveLength(4); }); + it('should reset retry mechanism when _maxRetriesReached is true', () => { + provider['_maxRetriesReached'] = true; + provider['_retryCount'] = 6; + + provider.pushLogs([{ message: 'test', timestamp: Date.now() }]); + + expect(provider['_retryCount']).toBe(0); + expect(provider['_maxRetriesReached']).toBe(false); + expect(mockInitiateLogPushInterval).toHaveBeenCalledTimes(2); + }); + it('should not reset retry mechanism when _maxRetriesReached is false', () => { + provider['_maxRetriesReached'] = false; + provider['_retryCount'] = 3; + + provider.pushLogs([{ message: 'test', timestamp: Date.now() }]); + + expect(provider['_retryCount']).toBe(3); + expect(provider['_maxRetriesReached']).toBe(false); + expect(mockInitiateLogPushInterval).toHaveBeenCalledTimes(1); + }); }); describe('_safeUploadLogEvents test', () => { @@ -397,4 +417,88 @@ describe('AWSCloudWatchProvider', () => { }); }); }); + describe('_initiateLogPushInterval', () => { + let provider: AWSCloudWatchProvider; + let safeUploadLogEventsSpy: jest.SpyInstance; + let getDocUploadPermissibilitySpy: jest.SpyInstance; + let setIntervalSpy: jest.SpyInstance; + + beforeEach(() => { + jest.useFakeTimers(); + provider = new AWSCloudWatchProvider(testConfig); + safeUploadLogEventsSpy = jest.spyOn( + provider as any, + '_safeUploadLogEvents' + ); + getDocUploadPermissibilitySpy = jest.spyOn( + provider as any, + '_getDocUploadPermissibility' + ); + setIntervalSpy = jest.spyOn(global, 'setInterval'); + }); + + afterEach(() => { + jest.useRealTimers(); + jest.restoreAllMocks(); + }); + + it('should clear existing timer and set a new one', () => { + (provider as any)._timer = setInterval(() => {}, 1000); + (provider as any)._initiateLogPushInterval(); + + expect(setIntervalSpy).toHaveBeenCalledTimes(1); + }); + + it('should not upload logs if max retries are reached', async () => { + (provider as any)._maxRetriesReached = true; + (provider as any)._initiateLogPushInterval(); + + jest.advanceTimersByTime(2000); + await Promise.resolve(); + + expect(safeUploadLogEventsSpy).not.toHaveBeenCalled(); + }); + + it('should upload logs if conditions are met', async () => { + getDocUploadPermissibilitySpy.mockReturnValue(true); + safeUploadLogEventsSpy.mockResolvedValue(undefined); + + (provider as any)._initiateLogPushInterval(); + + jest.advanceTimersByTime(2000); + await Promise.resolve(); + + expect(safeUploadLogEventsSpy).toHaveBeenCalledTimes(1); + expect((provider as any)._retryCount).toBe(0); + }); + + it('should increment retry count on error', async () => { + getDocUploadPermissibilitySpy.mockReturnValue(true); + safeUploadLogEventsSpy.mockRejectedValue(new Error('Test error')); + + (provider as any)._initiateLogPushInterval(); + + jest.advanceTimersByTime(2000); + await Promise.resolve(); + + expect((provider as any)._retryCount).toBe(0); + }); + + it('should stop retrying after max retries', async () => { + getDocUploadPermissibilitySpy.mockReturnValue(true); + safeUploadLogEventsSpy.mockRejectedValue(new Error('Test error')); + (provider as any)._maxRetries = 3; + + (provider as any)._initiateLogPushInterval(); + + for (let i = 0; i < 4; i++) { + jest.advanceTimersByTime(2000); + await Promise.resolve(); // Allow any pending promise to resolve + } + + expect((provider as any)._retryCount).toBe(2); + + expect(safeUploadLogEventsSpy).toHaveBeenCalledTimes(4); + }); + }); }); diff --git a/packages/core/src/Providers/AWSCloudWatchProvider.ts b/packages/core/src/Providers/AWSCloudWatchProvider.ts index 37d4a688649..6348c846697 100644 --- a/packages/core/src/Providers/AWSCloudWatchProvider.ts +++ b/packages/core/src/Providers/AWSCloudWatchProvider.ts @@ -56,7 +56,8 @@ class AWSCloudWatchProvider implements LoggingProvider { private _timer; private _nextSequenceToken: string | undefined; private _maxRetries = 5; - private _retryCount; + private _retryCount = 0; + private _maxRetriesReached: boolean = false; constructor(config?: AWSCloudWatchProviderOptions) { this.configure(config); @@ -211,6 +212,12 @@ class AWSCloudWatchProvider implements LoggingProvider { public pushLogs(logs: InputLogEvent[]): void { logger.debug('pushing log events to Cloudwatch...'); this._dataTracker.logEvents = [...this._dataTracker.logEvents, ...logs]; + + if (this._maxRetriesReached) { + this._retryCount = 0; + this._maxRetriesReached = false; + this._initiateLogPushInterval(); + } } private async _validateLogGroupExistsAndCreate( @@ -493,6 +500,10 @@ class AWSCloudWatchProvider implements LoggingProvider { } this._timer = setInterval(async () => { + if (this._maxRetriesReached) { + return; + } + try { if (this._getDocUploadPermissibility()) { await this._safeUploadLogEvents(); @@ -500,15 +511,16 @@ class AWSCloudWatchProvider implements LoggingProvider { } } catch (err) { this._retryCount++; - if (this._retryCount < this._maxRetries) { + if (this._retryCount > this._maxRetries) { logger.error( - `error when calling _safeUploadLogEvents in the timer interval - ${err}` - ); - } else if (this._retryCount === this._maxRetries) { - logger.error( - `CloudWatch log upload failed after ${this._maxRetries} attempts. Suppressing further error logs. Upload attempts will continue in the background.` + `Max retries (${this._maxRetries}) reached. Stopping log uploads.` ); + this._maxRetriesReached = true; + return; } + logger.error( + `error when calling _safeUploadLogEvents in the timer interval - ${err}` + ); } }, 2000); } From 9a0f779bd3ba9e487801d40950285dbb3e8588fa Mon Sep 17 00:00:00 2001 From: yuhengshs Date: Tue, 17 Sep 2024 08:31:24 -0700 Subject: [PATCH 5/5] removed unused import --- packages/core/__tests__/Providers/AWSCloudWatchProvider-test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/__tests__/Providers/AWSCloudWatchProvider-test.ts b/packages/core/__tests__/Providers/AWSCloudWatchProvider-test.ts index ee5b362f50a..4f7683e5304 100644 --- a/packages/core/__tests__/Providers/AWSCloudWatchProvider-test.ts +++ b/packages/core/__tests__/Providers/AWSCloudWatchProvider-test.ts @@ -14,7 +14,6 @@ import { DescribeLogStreamsCommand, PutLogEventsCommand, } from '@aws-sdk/client-cloudwatch-logs'; -import { loggers } from 'winston'; const credentials = { accessKeyId: 'accessKeyId',