From ad0abce45c41eb468fb0a95d5977309af43ff441 Mon Sep 17 00:00:00 2001 From: Michael Dombrowski Date: Fri, 27 Oct 2023 10:36:58 -0400 Subject: [PATCH] fix: handle valid JSON messages which are not GreengrassLogMessages (#214) * fix: handle valid json message which isn't a GreengrassLogMessage * fix(uat): update broken UAT test --- .../CloudWatchAttemptLogsProcessor.java | 10 +++++-- .../CloudWatchAttemptLogsProcessorTest.java | 12 +++++--- .../aws/greengrass/logmanager/testlogs1.log | 3 +- .../com/aws/greengrass/LogGeneratorTest.java | 29 ++----------------- 4 files changed, 20 insertions(+), 34 deletions(-) diff --git a/src/main/java/com/aws/greengrass/logmanager/CloudWatchAttemptLogsProcessor.java b/src/main/java/com/aws/greengrass/logmanager/CloudWatchAttemptLogsProcessor.java index a55975eb..b1932818 100644 --- a/src/main/java/com/aws/greengrass/logmanager/CloudWatchAttemptLogsProcessor.java +++ b/src/main/java/com/aws/greengrass/logmanager/CloudWatchAttemptLogsProcessor.java @@ -331,7 +331,11 @@ private Optional tryGetStructuredLogMessage(String data) { return Optional.empty(); } try { - return Optional.ofNullable(DESERIALIZER.readValue(data, GreengrassLogMessage.class)); + GreengrassLogMessage message = DESERIALIZER.readValue(data, GreengrassLogMessage.class); + if (message == null || Utils.isEmpty(message.getLevel())) { + return Optional.empty(); + } + return Optional.of(message); } catch (JsonProcessingException ignored) { // If unable to deserialize, then we treat it as a normal log line and do not need to smartly upload. return Optional.empty(); @@ -359,7 +363,7 @@ private Pair checkAndAddNewLogEvent(AtomicInteger totalB GreengrassLogMessage logMessage) { Level currentLogLevel = Level.valueOf(logMessage.getLevel()); if (currentLogLevel.toInt() < desiredLogLevel.toInt()) { - return new Pair(false, new AtomicInteger()); + return new Pair<>(false, new AtomicInteger()); } return addNewLogEvent(totalBytesRead, attemptLogInformation, data, dataSize, Instant.ofEpochMilli(logMessage.getTimestamp())); @@ -439,7 +443,7 @@ private Pair addNewLogEvent(AtomicInteger totalBytesRead currChunk++; } - return new Pair(reachedMaxBatchSize, currBytesRead); + return new Pair<>(reachedMaxBatchSize, currBytesRead); } /** diff --git a/src/test/java/com/aws/greengrass/logmanager/CloudWatchAttemptLogsProcessorTest.java b/src/test/java/com/aws/greengrass/logmanager/CloudWatchAttemptLogsProcessorTest.java index ea6d3461..2b98d6fa 100644 --- a/src/test/java/com/aws/greengrass/logmanager/CloudWatchAttemptLogsProcessorTest.java +++ b/src/test/java/com/aws/greengrass/logmanager/CloudWatchAttemptLogsProcessorTest.java @@ -476,8 +476,8 @@ void GIVEN_one_components_two_file_less_than_max_WHEN_merge_THEN_reads_and_merge String logStream2 = "/2020/12/18/thing/testThing"; assertTrue(attempt.getLogStreamsToLogEventsMap().containsKey(logStream)); assertTrue(attempt.getLogStreamsToLogEventsMap().containsKey(logStream2)); - CloudWatchAttemptLogInformation logEventsForStream1 = attempt.getLogStreamsToLogEventsMap().get(logStream); - CloudWatchAttemptLogInformation logEventsForStream2 = attempt.getLogStreamsToLogEventsMap().get(logStream2); + CloudWatchAttemptLogInformation logEventsForStream1 = attempt.getLogStreamsToLogEventsMap().remove(logStream); + CloudWatchAttemptLogInformation logEventsForStream2 = attempt.getLogStreamsToLogEventsMap().remove(logStream2); assertNotNull(logEventsForStream1.getLogEvents()); assertEquals(13, logEventsForStream1.getLogEvents().size()); assertTrue(logEventsForStream1.getAttemptLogFileInformationMap().containsKey(fileHash1)); @@ -494,10 +494,14 @@ void GIVEN_one_components_two_file_less_than_max_WHEN_merge_THEN_reads_and_merge } assertNotNull(logEventsForStream2.getLogEvents()); - assertEquals(4, logEventsForStream2.getLogEvents().size()); + assertEquals(3, logEventsForStream2.getLogEvents().size()); + // Read the 1 remaining cloudwatch log stream which will have today's date because the log lines + // had no parsed timestamp. + assertEquals(2, attempt.getLogStreamsToLogEventsMap() + .get(attempt.getLogStreamsToLogEventsMap().keySet().stream().findFirst().get()).getLogEvents().size()); assertTrue(logEventsForStream2.getAttemptLogFileInformationMap().containsKey(fileHash2)); assertEquals(0, logEventsForStream2.getAttemptLogFileInformationMap().get(fileHash2).getStartPosition()); - assertEquals(1239, logEventsForStream2.getAttemptLogFileInformationMap().get(fileHash2).getBytesRead()); + assertEquals(1237, logEventsForStream2.getAttemptLogFileInformationMap().get(fileHash2).getBytesRead()); assertEquals("TestComponent", logEventsForStream2.getComponentName()); } diff --git a/src/test/resources/com/aws/greengrass/logmanager/testlogs1.log b/src/test/resources/com/aws/greengrass/logmanager/testlogs1.log index 6aa206f7..41c363f9 100644 --- a/src/test/resources/com/aws/greengrass/logmanager/testlogs1.log +++ b/src/test/resources/com/aws/greengrass/logmanager/testlogs1.log @@ -1,5 +1,6 @@ -{"contexts":{"component":"demo","device":"asdf"},"eventType":"th1-event","level":"INFO","loggerName":"com.aws.greengrass.logging.examples.LoggerDemo","message":"test th1 info","timestamp":1608292800000} +{"contexts":{"component":"demo","device":"asdf"},"eventType":"th1-event","level":null,"loggerName":"com.aws.greengrass.logging.examples.LoggerDemo","message":"test th1 info","timestamp":1608292800000} {"contexts":{"component":"demo","device":"asdf"},"eventType":"th2-event","level":"INFO","loggerName":"com.aws.greengrass.logging.examples.LoggerDemo","message":"test th2 info","timestamp":1608292800000} {"contexts":{"component":"th1-override","device":"asdf"},"level":"DEBUG","loggerName":"com.aws.greengrass.logging.examples.LoggerDemo","message":"test th1 debug","timestamp":1608292800000} {"contexts":{"component":"demo","device":"asdf"},"level":"INFO","loggerName":"com.aws.greengrass.logging.examples.LoggerDemo","message":"test main info","timestamp":1608292800000} {"cause":{"localizedMessage":"some error","message":"some error","stackTrace":[{"className":"com.aws.greengrass.logging.examples.LoggerDemo","fileName":"LoggerDemo.java","lineNumber":56,"methodName":"main","nativeMethod":false}],"suppressed":[]},"contexts":{"key2":"value2","component":"demo","device":"asdf"},"eventType":"error-event","level":"ERROR","loggerName":"com.aws.greengrass.logging.examples.LoggerDemo","message":"test error","timestamp":1608292800000} +{"something which parses": "as json, but isn't a GreengrassStructuredLogMessage"} diff --git a/uat/custom-components/src/test/java/com/aws/greengrass/LogGeneratorTest.java b/uat/custom-components/src/test/java/com/aws/greengrass/LogGeneratorTest.java index a6bc6086..31dd2576 100644 --- a/uat/custom-components/src/test/java/com/aws/greengrass/LogGeneratorTest.java +++ b/uat/custom-components/src/test/java/com/aws/greengrass/LogGeneratorTest.java @@ -5,7 +5,6 @@ import org.junit.jupiter.api.io.TempDir; import org.mockito.junit.jupiter.MockitoExtension; -import java.io.File; import java.nio.file.Path; import java.util.Arrays; import java.util.function.Consumer; @@ -17,19 +16,18 @@ public class LogGeneratorTest { @TempDir static Path tempPath; String logFileName = "localtest"; - String logFileExtention = "log"; - String logWriteFreqSeconds = "0.1"; + String logWriteFreqMs = "100"; String totalLogNumbers = "50"; String fileSizeBytes = "1024"; String fileSizeUnit = "KB"; String componentName = "com.aws.greengrass.artifacts.LogGenerator"; - String activeFileName = logFileName + "." + logFileExtention; + String activeFileName = logFileName + ".log"; @Test void GIVEN_request_THEN_log_file_Created() throws ClassNotFoundException, IllegalAccessException, InstantiationException { - String[] args = {logFileName, logFileExtention, fileSizeBytes, fileSizeUnit, logWriteFreqSeconds, + String[] args = {logFileName, fileSizeBytes, fileSizeUnit, logWriteFreqMs, totalLogNumbers, tempPath.toString()}; ((Consumer) Class.forName(componentName).newInstance()).accept(args); @@ -39,25 +37,4 @@ void GIVEN_request_THEN_log_file_Created() assertTrue(Arrays.asList(pathnames).contains(activeFileName)); assertTrue(tempPath.resolve(activeFileName).toFile().length() > 0); } - - /* - if tempPath is empty string, then the log files would be generated in your local machine. ' - This can be used for manually testing if logs are correctly written - */ - @Test - void GIVEN_empty_targetFilePath_in_Paras_THEN_default_path_is_local() - throws ClassNotFoundException, InstantiationException, IllegalAccessException { - - String[] args = {logFileName, logFileExtention, fileSizeBytes, fileSizeUnit, logWriteFreqSeconds, - totalLogNumbers, ""}; - ((Consumer) Class.forName(componentName).newInstance()).accept(args); - - String localPath = System.getProperty("user.dir"); - File directory = new File(localPath); - String[] pathnames = directory.list(); - assertTrue(pathnames.length >= 1); - assertTrue(Arrays.asList(pathnames).contains(activeFileName)); - assertTrue(new File(localPath + "/" + activeFileName).length() > 0); - - } }