From b1c938b4515de9b58be7b956bdf37694adf1b9a3 Mon Sep 17 00:00:00 2001 From: Kha Truong <64438356+khatruong2009@users.noreply.github.com> Date: Mon, 9 Oct 2023 16:17:26 -0700 Subject: [PATCH] feat: implement _isLoggable method (#3894) * feat: implement _isLoggable and corrected categoryLogLevel in UserLogLevel class * chore: added _userId private variable instead of using method variable * chore: add aft workflows * chore: added helper method make sure category keys match * chore: removed unnecessary new listener * Update packages/logging_cloudwatch/aws_logging_cloudwatch/lib/src/cloudwatch_logger_plugin.dart Co-authored-by: NikaHsn * chore: removed white spaces * chore: added tests, tests methods, and fixed a typ * Update packages/logging_cloudwatch/aws_logging_cloudwatch/lib/src/cloudwatch_logger_plugin.dart Co-authored-by: NikaHsn * chore: removed testing methods and adjusted tests * chore: WIP - changed tests for handleLogEntry * chore: added future.delay(0) to tests * chore: updated remote_constraint_provider_test * updated the tests * chore: flattened if statement --------- Co-authored-by: NikaHsn Co-authored-by: Nika Hassani --- .../logging/cloudwatch_logging_config.dart | 2 +- .../logging/cloudwatch_logging_config.g.dart | 5 +- .../lib/src/cloudwatch_logger_plugin.dart | 42 +++- .../test/cloudwatch_logger_plugin_test.dart | 195 +++++++++++++++++- .../aws_logging_cloudwatch/test/mocks.dart | 8 +- .../test/remote_constraint_provider_test.dart | 4 +- 6 files changed, 237 insertions(+), 19 deletions(-) diff --git a/packages/amplify_core/lib/src/config/logging/cloudwatch_logging_config.dart b/packages/amplify_core/lib/src/config/logging/cloudwatch_logging_config.dart index 3012fc94a2..581da428de 100644 --- a/packages/amplify_core/lib/src/config/logging/cloudwatch_logging_config.dart +++ b/packages/amplify_core/lib/src/config/logging/cloudwatch_logging_config.dart @@ -161,7 +161,7 @@ class UserLogLevel _$UserLogLevelFromJson(json); final LogLevel defaultLogLevel; - final Map categoryLogLevel; + final Map categoryLogLevel; @override List get props => [defaultLogLevel, categoryLogLevel]; diff --git a/packages/amplify_core/lib/src/config/logging/cloudwatch_logging_config.g.dart b/packages/amplify_core/lib/src/config/logging/cloudwatch_logging_config.g.dart index 72ab29ce57..b7aa0da96a 100644 --- a/packages/amplify_core/lib/src/config/logging/cloudwatch_logging_config.g.dart +++ b/packages/amplify_core/lib/src/config/logging/cloudwatch_logging_config.g.dart @@ -104,7 +104,7 @@ UserLogLevel _$UserLogLevelFromJson(Map json) => UserLogLevel( LogLevel.error, categoryLogLevel: (json['categoryLogLevel'] as Map?)?.map( - (k, e) => MapEntry(k, e as String), + (k, e) => MapEntry(k, $enumDecode(_$LogLevelEnumMap, e)), ) ?? const {}, ); @@ -112,5 +112,6 @@ UserLogLevel _$UserLogLevelFromJson(Map json) => UserLogLevel( Map _$UserLogLevelToJson(UserLogLevel instance) => { 'defaultLogLevel': _$LogLevelEnumMap[instance.defaultLogLevel]!, - 'categoryLogLevel': instance.categoryLogLevel, + 'categoryLogLevel': instance.categoryLogLevel + .map((k, e) => MapEntry(k, _$LogLevelEnumMap[e]!)), }; diff --git a/packages/logging_cloudwatch/aws_logging_cloudwatch/lib/src/cloudwatch_logger_plugin.dart b/packages/logging_cloudwatch/aws_logging_cloudwatch/lib/src/cloudwatch_logger_plugin.dart index a6961e3d3c..ce6fb512b1 100644 --- a/packages/logging_cloudwatch/aws_logging_cloudwatch/lib/src/cloudwatch_logger_plugin.dart +++ b/packages/logging_cloudwatch/aws_logging_cloudwatch/lib/src/cloudwatch_logger_plugin.dart @@ -98,8 +98,12 @@ class CloudWatchLoggerPlugin extends AWSLoggerPlugin if (event.type == AuthHubEventType.signedOut || event.type == AuthHubEventType.userDeleted || event.type == AuthHubEventType.sessionExpired) { + _userId = null; await _clearLogs(); } + if (event.type == AuthHubEventType.signedIn) { + _userId = event.payload?.userId; + } }); } @@ -122,11 +126,15 @@ class CloudWatchLoggerPlugin extends AWSLoggerPlugin if (event.type == AuthHubEventType.signedOut || event.type == AuthHubEventType.userDeleted || event.type == AuthHubEventType.sessionExpired) { + _userId = null; await _clearLogs(); } + if (event.type == AuthHubEventType.signedIn) { + _userId = event.payload?.userId; + } }); } - + String? _userId; final CloudWatchPluginConfig _pluginConfig; final CloudWatchLogsClient _client; final CloudWatchLogStreamProvider _logStreamProvider; @@ -301,11 +309,35 @@ class CloudWatchLoggerPlugin extends AWSLoggerPlugin /// Whether a [logEntry] should be logged by this plugin. bool _isLoggable(LogEntry logEntry) { - if (!_enabled) { - return false; - } + if (!_enabled) return false; + final loggingConstraint = _getLoggingConstraint(); - return logEntry.level >= loggingConstraint.defaultLogLevel; + final hasUserLogLevel = loggingConstraint.userLogLevel.containsKey(_userId); + LogLevel? logLevel; + + if (hasUserLogLevel) { + final userLogLevel = loggingConstraint.userLogLevel[_userId]!; + logLevel = + _getCategoryLogLevel(userLogLevel.categoryLogLevel, logEntry) ?? + userLogLevel.defaultLogLevel; + } else { + logLevel = + _getCategoryLogLevel(loggingConstraint.categoryLogLevel, logEntry); + } + + return logEntry.level >= (logLevel ?? loggingConstraint.defaultLogLevel); + } + + LogLevel? _getCategoryLogLevel( + Map categoryLogLevel, + LogEntry logEntry, + ) { + for (final entry in categoryLogLevel.entries) { + if (logEntry.loggerName.toLowerCase().contains(entry.key.toLowerCase())) { + return entry.value; + } + } + return null; } @override diff --git a/packages/logging_cloudwatch/aws_logging_cloudwatch/test/cloudwatch_logger_plugin_test.dart b/packages/logging_cloudwatch/aws_logging_cloudwatch/test/cloudwatch_logger_plugin_test.dart index 699053c57f..aeaa155959 100644 --- a/packages/logging_cloudwatch/aws_logging_cloudwatch/test/cloudwatch_logger_plugin_test.dart +++ b/packages/logging_cloudwatch/aws_logging_cloudwatch/test/cloudwatch_logger_plugin_test.dart @@ -19,7 +19,29 @@ void main() { late CloudWatchLoggerPlugin plugin; late MockSmithyOperation mockPutLogEventsOperation; - const loggingConstraint = LoggingConstraints(); + const loggingConstraint = LoggingConstraints( + defaultLogLevel: LogLevel.error, + categoryLogLevel: { + 'Auth': LogLevel.warn, + 'DataStore': LogLevel.debug, + }, + userLogLevel: { + 'mockUserId': UserLogLevel( + defaultLogLevel: LogLevel.warn, + categoryLogLevel: { + 'Auth': LogLevel.debug, + 'DataStore': LogLevel.info, + }, + ), + 'userId': UserLogLevel( + defaultLogLevel: LogLevel.error, + categoryLogLevel: { + 'Auth': LogLevel.info, + 'DataStore': LogLevel.warn, + }, + ), + }, + ); const pluginConfig = CloudWatchPluginConfig( logGroupName: 'logGroupName', region: 'region', @@ -38,6 +60,48 @@ void main() { loggerName: 'loggerName', ); + final infoLog = LogEntry( + level: LogLevel.info, + message: 'info message', + loggerName: 'loggerName', + ); + + final datastoreDebugLog = LogEntry( + level: LogLevel.debug, + message: 'debug message', + loggerName: 'DataStore', + ); + + final datastoreInfoLog = LogEntry( + level: LogLevel.info, + message: 'debug message', + loggerName: 'DataStore', + ); + + final authWarnLog = LogEntry( + level: LogLevel.warn, + message: 'debug message', + loggerName: 'Auth', + ); + + final authDebugLog = LogEntry( + level: LogLevel.debug, + message: 'debug message', + loggerName: 'Auth', + ); + + final authInfoLog = LogEntry( + level: LogLevel.info, + message: 'info message', + loggerName: 'Auth', + ); + + final authVerboseLog = LogEntry( + level: LogLevel.verbose, + message: 'verbose message', + loggerName: 'Auth', + ); + final queuedItems = [ QueuedItem( id: 1, @@ -62,6 +126,9 @@ void main() { ]; group('enable/disable: ', () { + final hubEventController = StreamController.broadcast(); + Amplify.Hub.addChannel(HubChannel.Auth, hubEventController.stream); + setUp(() { mockCloudWatchLogsClient = MockCloudWatchLogsClient(); mockQueuedItemStore = MockQueuedItemStore(); @@ -74,7 +141,11 @@ void main() { ); }); - test('when enabled, logs are added to the item store', () async { + tearDownAll(hubEventController.close); + + test( + 'when enabled, logs are added to the item store ' + 'if loggable at default log level', () async { when( () => mockQueuedItemStore.addItem( any(), @@ -87,11 +158,15 @@ void main() { .thenReturn(false); plugin.enable(); - await expectLater( plugin.handleLogEntry(errorLog), completes, ); + // should not log this because it is below default log level. + await expectLater( + plugin.handleLogEntry(warnLog), + completes, + ); verify( () => mockQueuedItemStore.addItem( @@ -100,20 +175,123 @@ void main() { enableQueueRotation: false, ), ).called(1); - verify( () => mockQueuedItemStore.isFull(pluginConfig.localStoreMaxSizeInMB), ).called(1); }); - test('when enabled,logs are not added to the log store if not loggable', - () async { + test( + 'when enabled, logs are added to the item store ' + 'if loggable at category log level', () async { + when( + () => mockQueuedItemStore.addItem( + any(), + any(), + enableQueueRotation: false, + ), + ).thenAnswer((_) async => {}); + + when(() => mockQueuedItemStore.isFull(pluginConfig.localStoreMaxSizeInMB)) + .thenReturn(false); + plugin.enable(); + await expectLater( + plugin.handleLogEntry(authWarnLog), + completes, + ); + await expectLater( + plugin.handleLogEntry(datastoreDebugLog), + completes, + ); + // should not log this because it is below auth category log level. + await expectLater( + plugin.handleLogEntry(authInfoLog), + completes, + ); + + verify( + () => mockQueuedItemStore.addItem( + any(), + any(), + enableQueueRotation: false, + ), + ).called(2); + verify( + () => mockQueuedItemStore.isFull(pluginConfig.localStoreMaxSizeInMB), + ).called(2); + }); + + test( + 'when enabled, logs are added to the log store if' + ' loggable at user log level', () async { + when( + () => mockQueuedItemStore.addItem( + any(), + any(), + enableQueueRotation: false, + ), + ).thenAnswer((_) async => {}); + + when(() => mockQueuedItemStore.isFull(pluginConfig.localStoreMaxSizeInMB)) + .thenReturn(false); + + plugin.enable(); + hubEventController.add(AuthHubEvent.signedIn(MockAuthUser())); + await Future.delayed(Duration.zero); + + await expectLater( + plugin.handleLogEntry(authWarnLog), + completes, + ); + await expectLater( + plugin.handleLogEntry(datastoreInfoLog), + completes, + ); await expectLater( plugin.handleLogEntry(warnLog), completes, ); - verifyZeroInteractions(mockQueuedItemStore); + + // should not log these because they are below user log level. + await expectLater( + plugin.handleLogEntry(authVerboseLog), + completes, + ); + await expectLater( + plugin.handleLogEntry(infoLog), + completes, + ); + + verify( + () => mockQueuedItemStore.addItem( + any(), + any(), + enableQueueRotation: false, + ), + ).called(3); + verify( + () => mockQueuedItemStore.isFull(pluginConfig.localStoreMaxSizeInMB), + ).called(3); + + hubEventController.add(AuthHubEvent.signedOut()); + await Future.delayed(Duration.zero); + + // should not log this because it is below auth category log level. + await expectLater( + plugin.handleLogEntry(authDebugLog), + completes, + ); + + verifyNever( + () => mockQueuedItemStore.addItem( + any(), + any(), + enableQueueRotation: false, + ), + ); + verifyNever( + () => mockQueuedItemStore.isFull(pluginConfig.localStoreMaxSizeInMB), + ); }); test( @@ -732,6 +910,7 @@ void main() { AuthHubEvent.signedOut, AuthHubEvent.userDeleted, ]; + Amplify.Hub.addChannel(HubChannel.Auth, hubEventController.stream); setUp(() { mockCloudWatchLogsClient = MockCloudWatchLogsClient(); @@ -798,7 +977,7 @@ void main() { plugin.handleLogEntry(errorLog), completes, ); - hubEventController.add(AuthHubEvent.signedIn(MockAuhtUser())); + hubEventController.add(AuthHubEvent.signedIn(MockAuthUser())); await Future.delayed(Duration.zero); verify( diff --git a/packages/logging_cloudwatch/aws_logging_cloudwatch/test/mocks.dart b/packages/logging_cloudwatch/aws_logging_cloudwatch/test/mocks.dart index bdf6fba1e7..b9832bfd26 100644 --- a/packages/logging_cloudwatch/aws_logging_cloudwatch/test/mocks.dart +++ b/packages/logging_cloudwatch/aws_logging_cloudwatch/test/mocks.dart @@ -18,4 +18,10 @@ class MockCloudWatchLogStreamProvider extends Mock class MockRemoteLoggingConstraintProvider extends Mock implements RemoteLoggingConstraintProvider {} -class MockAuhtUser extends Mock implements AuthUser {} +class MockAuthUser extends Mock implements AuthUser { + @override + final String userId = 'mockUserId'; + + @override + final String username = 'mockUser'; +} diff --git a/packages/logging_cloudwatch/aws_logging_cloudwatch/test/remote_constraint_provider_test.dart b/packages/logging_cloudwatch/aws_logging_cloudwatch/test/remote_constraint_provider_test.dart index 38ed51af6d..003679c272 100644 --- a/packages/logging_cloudwatch/aws_logging_cloudwatch/test/remote_constraint_provider_test.dart +++ b/packages/logging_cloudwatch/aws_logging_cloudwatch/test/remote_constraint_provider_test.dart @@ -140,8 +140,8 @@ void main() { "cognito-sub-xyz-123": { "defaultLogLevel": "VERBOSE", "categoryLogLevel": { - "API": "error", - "AUTH": "debug" + "API": "ERROR", + "AUTH": "DEBUG" } } }