Skip to content

Commit 15efff4

Browse files
phipagphipag
committed
Address Sonar findings.
1 parent 30c1644 commit 15efff4

File tree

6 files changed

+70
-54
lines changed

6 files changed

+70
-54
lines changed

‎powertools-logging/powertools-logging-log4j/src/test/java/software/amazon/lambda/powertools/logging/log4j/BufferingAppenderTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ void shouldClearBufferManually() {
116116
void shouldLogOverflowWarningWhenBufferOverflows() {
117117
// When - fill buffer beyond capacity to trigger overflow
118118
for (int i = 0; i < 100; i++) {
119-
logger.debug("Debug message " + i);
119+
logger.debug("Debug message {}", i);
120120
}
121121

122122
// When - flush buffer to trigger overflow warning

‎powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/LogbackLoggingManagerTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class LogbackLoggingManagerTest {
4747
@BeforeEach
4848
void setUp() throws JoranException, IOException {
4949
resetLogbackConfig("/logback-test.xml");
50-
50+
5151
try {
5252
FileChannel.open(Paths.get("target/logfile.json"), StandardOpenOption.WRITE).truncate(0).close();
5353
} catch (NoSuchFileException e) {
@@ -75,7 +75,7 @@ void resetLogLevel() {
7575
}
7676

7777
@Test
78-
void shouldDetectMultipleBufferingAppendersRegardlessOfName() throws IOException, JoranException {
78+
void shouldDetectMultipleBufferingAppendersRegardlessOfName() throws JoranException {
7979
// Given - configuration with multiple BufferingAppenders with different names
8080
resetLogbackConfig("/logback-multiple-buffering.xml");
8181

‎powertools-logging/powertools-logging-logback/src/test/java/software/amazon/lambda/powertools/logging/logback/BufferingAppenderTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ void shouldClearBufferManually() {
130130
void shouldLogOverflowWarningWhenBufferOverflows() {
131131
// When - fill buffer beyond capacity to trigger overflow
132132
for (int i = 0; i < 100; i++) {
133-
logger.debug("Debug message " + i);
133+
logger.debug("Debug message {}", i);
134134
}
135135

136136
// When - flush buffer to trigger overflow warning

‎powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/Logging.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@
7272

7373
/**
7474
* Set to true if you want to log the response sent by the Lambda function handler.<br/>
75-
* Can also be configured with the 'POWERTOOLS_LOGGER_LOG_RESPONE' environment variable
75+
* Can also be configured with the 'POWERTOOLS_LOGGER_LOG_RESPONSE' environment variable
7676
*/
7777
boolean logResponse() default false;
7878

‎powertools-logging/src/main/java/software/amazon/lambda/powertools/logging/internal/LambdaLoggingAspect.java‎

Lines changed: 58 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -130,71 +130,38 @@ public Object around(ProceedingJoinPoint pjp,
130130
boolean isOnRequestStreamHandler = placedOnStreamHandler(pjp);
131131

132132
setLogLevelBasedOnSamplingRate(pjp, logging);
133-
134133
addLambdaContextToLoggingContext(pjp);
135-
136134
getXrayTraceId().ifPresent(xRayTraceId -> MDC.put(FUNCTION_TRACE_ID.getName(), xRayTraceId));
137135

138-
// Log Event
139136
Object[] proceedArgs = logEvent(pjp, logging, isOnRequestHandler, isOnRequestStreamHandler);
140137

141138
if (!logging.correlationIdPath().isEmpty()) {
142139
captureCorrelationId(logging.correlationIdPath(), proceedArgs, isOnRequestHandler,
143140
isOnRequestStreamHandler);
144141
}
145142

146-
// To log the result of a RequestStreamHandler (OutputStream), we need to do the following:
147-
// 1. backup a reference to the OutputStream provided by Lambda
148-
// 2. create a temporary OutputStream and pass it to the handler method
149-
// 3. retrieve this temporary stream to log it (if enabled)
150-
// 4. write it back to the OutputStream provided by Lambda
151143
OutputStream backupOutputStream = null;
152-
if ((logging.logResponse() || POWERTOOLS_LOG_RESPONSE) && isOnRequestStreamHandler) {
153-
backupOutputStream = (OutputStream) proceedArgs[1];
154-
proceedArgs[1] = new ByteArrayOutputStream();
144+
if (isOnRequestStreamHandler) {
145+
// To log the result of a RequestStreamHandler (OutputStream), we need to do the following:
146+
// 1. backup a reference to the OutputStream provided by Lambda
147+
// 2. create a temporary OutputStream and pass it to the handler method
148+
// 3. retrieve this temporary stream to log it (if enabled)
149+
// 4. write it back to the OutputStream provided by Lambda
150+
backupOutputStream = prepareOutputStreamForLogging(logging, proceedArgs);
155151
}
156152

157153
Object lambdaFunctionResponse;
158-
159154
try {
160-
// Call Function Handler
161155
lambdaFunctionResponse = pjp.proceed(proceedArgs);
162156
} catch (Throwable t) {
163-
if (LOGGING_MANAGER instanceof BufferManager) {
164-
if (logging.flushBufferOnUncaughtError()) {
165-
((BufferManager) LOGGING_MANAGER).flushBuffer();
166-
} else {
167-
// Clear buffer before error logging to prevent unintended flush. If flushOnErrorLog is enabled on
168-
// the appender the next line would otherwise cause an unintended flush by the appender directly.
169-
((BufferManager) LOGGING_MANAGER).clearBuffer();
170-
}
171-
}
172-
if (logging.logError() || POWERTOOLS_LOG_ERROR) {
173-
// logging the exception with additional context
174-
logger(pjp).error(MarkerFactory.getMarker("FATAL"), "Exception in Lambda Handler", t);
175-
}
157+
handleException(pjp, logging, t);
176158
throw t;
177159
} finally {
178-
if (logging.clearState()) {
179-
MDC.clear();
180-
}
181-
// Clear buffer after each handler invocation
182-
if (LOGGING_MANAGER instanceof BufferManager) {
183-
((BufferManager) LOGGING_MANAGER).clearBuffer();
184-
}
185-
coldStartDone();
160+
performCleanup(logging);
186161
}
187162

188-
// Log Response
189-
if ((logging.logResponse() || POWERTOOLS_LOG_RESPONSE)) {
190-
if (isOnRequestHandler) {
191-
logRequestHandlerResponse(pjp, lambdaFunctionResponse);
192-
} else if (isOnRequestStreamHandler && backupOutputStream != null) {
193-
byte[] bytes = ((ByteArrayOutputStream) proceedArgs[1]).toByteArray();
194-
logRequestStreamHandlerResponse(pjp, bytes);
195-
backupOutputStream.write(bytes);
196-
}
197-
}
163+
logResponse(pjp, logging, lambdaFunctionResponse, isOnRequestHandler, isOnRequestStreamHandler,
164+
backupOutputStream, proceedArgs);
198165

199166
return lambdaFunctionResponse;
200167
}
@@ -355,6 +322,53 @@ private byte[] bytesFromInputStreamSafely(final InputStream inputStream) throws
355322
}
356323
}
357324

325+
private OutputStream prepareOutputStreamForLogging(Logging logging,
326+
Object[] proceedArgs) {
327+
if ((logging.logResponse() || POWERTOOLS_LOG_RESPONSE)) {
328+
OutputStream backupOutputStream = (OutputStream) proceedArgs[1];
329+
proceedArgs[1] = new ByteArrayOutputStream();
330+
return backupOutputStream;
331+
}
332+
return null;
333+
}
334+
335+
private void handleException(ProceedingJoinPoint pjp, Logging logging, Throwable t) {
336+
if (LOGGING_MANAGER instanceof BufferManager) {
337+
if (logging.flushBufferOnUncaughtError()) {
338+
((BufferManager) LOGGING_MANAGER).flushBuffer();
339+
} else {
340+
((BufferManager) LOGGING_MANAGER).clearBuffer();
341+
}
342+
}
343+
if (logging.logError() || POWERTOOLS_LOG_ERROR) {
344+
logger(pjp).error(MarkerFactory.getMarker("FATAL"), "Exception in Lambda Handler", t);
345+
}
346+
}
347+
348+
private void performCleanup(Logging logging) {
349+
if (logging.clearState()) {
350+
MDC.clear();
351+
}
352+
if (LOGGING_MANAGER instanceof BufferManager) {
353+
((BufferManager) LOGGING_MANAGER).clearBuffer();
354+
}
355+
coldStartDone();
356+
}
357+
358+
private void logResponse(ProceedingJoinPoint pjp, Logging logging, Object lambdaFunctionResponse,
359+
boolean isOnRequestHandler, boolean isOnRequestStreamHandler,
360+
OutputStream backupOutputStream, Object[] proceedArgs) throws IOException {
361+
if (logging.logResponse() || POWERTOOLS_LOG_RESPONSE) {
362+
if (isOnRequestHandler) {
363+
logRequestHandlerResponse(pjp, lambdaFunctionResponse);
364+
} else if (isOnRequestStreamHandler && backupOutputStream != null) {
365+
byte[] bytes = ((ByteArrayOutputStream) proceedArgs[1]).toByteArray();
366+
logRequestStreamHandlerResponse(pjp, bytes);
367+
backupOutputStream.write(bytes);
368+
}
369+
}
370+
}
371+
358372
private Logger logger(final ProceedingJoinPoint pjp) {
359373
return LoggerFactory.getLogger(pjp.getSignature().getDeclaringType());
360374
}

‎powertools-logging/src/test/java/software/amazon/lambda/powertools/logging/internal/LoggingManagerRegistryTest.java‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,9 @@ void testSingleLoggingManager_shouldReturnWithoutWarning() throws UnsupportedEnc
9191
// THEN
9292
String output = outputStream.toString("UTF-8");
9393
assertThat(output).isEmpty();
94-
assertThat(loggingManager).isSameAs(testManager);
95-
assertThat(loggingManager).isInstanceOf(BufferManager.class);
94+
assertThat(loggingManager)
95+
.isSameAs(testManager)
96+
.isInstanceOf(BufferManager.class);
9697
}
9798

9899
@Test
@@ -102,9 +103,10 @@ void testGetLoggingManager_shouldReturnSameInstance() {
102103
LoggingManager second = LoggingManagerRegistry.getLoggingManager();
103104

104105
// THEN
105-
assertThat(first).isSameAs(second);
106-
assertThat(first).isNotNull();
107-
assertThat(first).isInstanceOf(BufferManager.class);
106+
assertThat(first)
107+
.isSameAs(second)
108+
.isNotNull()
109+
.isInstanceOf(BufferManager.class);
108110
}
109111

110112
@Test

0 commit comments

Comments
 (0)