Skip to content

Commit 484fcca

Browse files
pankajagrawal16Pankaj Agrawalmsailes
authored
fix: LoggingAspect precedence to be last for accepting mutated args (#567)
* fix: LoggingAspect precedence to be last for accepting mutated args * new test-suite module for testing multi module use cases. * chore: Build new module in actions * chore: Remove Unnecessary sdk warnings * chore: skip deploying module Co-authored-by: Pankaj Agrawal <[email protected]> Co-authored-by: Mark Sailes <[email protected]>
1 parent 71dbff7 commit 484fcca

File tree

9 files changed

+400
-0
lines changed

9 files changed

+400
-0
lines changed

.github/workflows/build.yml

+2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ on:
1212
- 'powertools-validation/**'
1313
- 'powertools-parameters/**'
1414
- 'powertools-metrics/**'
15+
- 'powertools-test-suite/**'
1516
- 'pom.xml'
1617
- '.github/workflows/**'
1718
push:
@@ -25,6 +26,7 @@ on:
2526
- 'powertools-validation/**'
2627
- 'powertools-parameters/**'
2728
- 'powertools-metrics/**'
29+
- 'powertools-test-suite/**'
2830
- 'pom.xml'
2931
- '.github/workflows/**'
3032
jobs:

.github/workflows/spotbugs.yml

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ on:
1212
- 'powertools-validation/**'
1313
- 'powertools-parameters/**'
1414
- 'powertools-metrics/**'
15+
- 'powertools-test-suite/**'
1516
- 'pom.xml'
1617
- '.github/workflows/**'
1718
jobs:

pom.xml

+16
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
<module>powertools-metrics</module>
3535
<module>powertools-parameters</module>
3636
<module>powertools-validation</module>
37+
<module>powertools-test-suite</module>
3738
</modules>
3839

3940
<scm>
@@ -88,6 +89,21 @@
8889
<artifactId>powertools-core</artifactId>
8990
<version>${project.version}</version>
9091
</dependency>
92+
<dependency>
93+
<groupId>software.amazon.lambda</groupId>
94+
<artifactId>powertools-logging</artifactId>
95+
<version>${project.version}</version>
96+
</dependency>
97+
<dependency>
98+
<groupId>software.amazon.lambda</groupId>
99+
<artifactId>powertools-sqs</artifactId>
100+
<version>${project.version}</version>
101+
</dependency>
102+
<dependency>
103+
<groupId>software.amazon.lambda</groupId>
104+
<artifactId>powertools-tracing</artifactId>
105+
<version>${project.version}</version>
106+
</dependency>
91107
<dependency>
92108
<groupId>com.amazonaws</groupId>
93109
<artifactId>aws-lambda-java-core</artifactId>

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

+2
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.aspectj.lang.ProceedingJoinPoint;
3838
import org.aspectj.lang.annotation.Around;
3939
import org.aspectj.lang.annotation.Aspect;
40+
import org.aspectj.lang.annotation.DeclarePrecedence;
4041
import org.aspectj.lang.annotation.Pointcut;
4142
import software.amazon.lambda.powertools.logging.Logging;
4243
import software.amazon.lambda.powertools.logging.LoggingUtils;
@@ -57,6 +58,7 @@
5758
import static software.amazon.lambda.powertools.logging.LoggingUtils.objectMapper;
5859

5960
@Aspect
61+
@DeclarePrecedence("*, SqsLargeMessageAspect, LambdaLoggingAspect")
6062
public final class LambdaLoggingAspect {
6163
private static final Logger LOG = LogManager.getLogger(LambdaLoggingAspect.class);
6264
private static final Random SAMPLER = new Random();

powertools-test-suite/pom.xml

+151
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<project xmlns="http://maven.apache.org/POM/4.0.0"
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
5+
<modelVersion>4.0.0</modelVersion>
6+
7+
<artifactId>powertools-test-suite</artifactId>
8+
<packaging>jar</packaging>
9+
10+
<parent>
11+
<artifactId>powertools-parent</artifactId>
12+
<groupId>software.amazon.lambda</groupId>
13+
<version>1.7.3</version>
14+
</parent>
15+
16+
<name>AWS Lambda Powertools Java library Test Suite</name>
17+
<description>
18+
A suite of tests for interactions between the various Powertools modules.
19+
</description>
20+
<url>https://aws.amazon.com/lambda/</url>
21+
<issueManagement>
22+
<system>GitHub Issues</system>
23+
<url>https://github.com/awslabs/aws-lambda-powertools-java/issues</url>
24+
</issueManagement>
25+
<scm>
26+
<url>https://github.com/awslabs/aws-lambda-powertools-java.git</url>
27+
</scm>
28+
<developers>
29+
<developer>
30+
<name>AWS Lambda Powertools team</name>
31+
<organization>Amazon Web Services</organization>
32+
<organizationUrl>https://aws.amazon.com/</organizationUrl>
33+
</developer>
34+
</developers>
35+
36+
<properties>
37+
<maven.deploy.skip>true</maven.deploy.skip>
38+
</properties>
39+
40+
<distributionManagement>
41+
<snapshotRepository>
42+
<id>ossrh</id>
43+
<url>https://aws.oss.sonatype.org/content/repositories/snapshots</url>
44+
</snapshotRepository>
45+
</distributionManagement>
46+
47+
<dependencies>
48+
<dependency>
49+
<groupId>software.amazon.lambda</groupId>
50+
<artifactId>powertools-core</artifactId>
51+
</dependency>
52+
<dependency>
53+
<groupId>org.apache.logging.log4j</groupId>
54+
<artifactId>log4j-jcl</artifactId>
55+
<version>2.14.1</version>
56+
</dependency>
57+
<dependency>
58+
<groupId>com.amazonaws</groupId>
59+
<artifactId>aws-lambda-java-core</artifactId>
60+
</dependency>
61+
<dependency>
62+
<groupId>com.amazonaws</groupId>
63+
<artifactId>aws-lambda-java-events</artifactId>
64+
</dependency>
65+
<dependency>
66+
<groupId>software.amazon.lambda</groupId>
67+
<artifactId>powertools-logging</artifactId>
68+
</dependency>
69+
<dependency>
70+
<groupId>software.amazon.lambda</groupId>
71+
<artifactId>powertools-tracing</artifactId>
72+
</dependency>
73+
<dependency>
74+
<groupId>software.amazon.lambda</groupId>
75+
<artifactId>powertools-sqs</artifactId>
76+
</dependency>
77+
78+
<!-- Test dependencies -->
79+
<dependency>
80+
<groupId>org.junit.jupiter</groupId>
81+
<artifactId>junit-jupiter-api</artifactId>
82+
<scope>test</scope>
83+
</dependency>
84+
<dependency>
85+
<groupId>org.junit.jupiter</groupId>
86+
<artifactId>junit-jupiter-engine</artifactId>
87+
<scope>test</scope>
88+
</dependency>
89+
<dependency>
90+
<groupId>org.apache.commons</groupId>
91+
<artifactId>commons-lang3</artifactId>
92+
<scope>test</scope>
93+
</dependency>
94+
<dependency>
95+
<groupId>org.mockito</groupId>
96+
<artifactId>mockito-core</artifactId>
97+
<scope>test</scope>
98+
</dependency>
99+
<dependency>
100+
<groupId>org.aspectj</groupId>
101+
<artifactId>aspectjweaver</artifactId>
102+
<scope>test</scope>
103+
</dependency>
104+
<dependency>
105+
<groupId>org.assertj</groupId>
106+
<artifactId>assertj-core</artifactId>
107+
<scope>test</scope>
108+
</dependency>
109+
<dependency>
110+
<groupId>org.skyscreamer</groupId>
111+
<artifactId>jsonassert</artifactId>
112+
<scope>test</scope>
113+
</dependency>
114+
</dependencies>
115+
116+
<build>
117+
<plugins>
118+
<plugin>
119+
<groupId>org.codehaus.mojo</groupId>
120+
<artifactId>aspectj-maven-plugin</artifactId>
121+
<version>1.14.0</version>
122+
<configuration>
123+
<source>${maven.compiler.source}</source>
124+
<target>${maven.compiler.target}</target>
125+
<complianceLevel>${maven.compiler.target}</complianceLevel>
126+
<aspectLibraries>
127+
<aspectLibrary>
128+
<groupId>software.amazon.lambda</groupId>
129+
<artifactId>powertools-logging</artifactId>
130+
</aspectLibrary>
131+
<aspectLibrary>
132+
<groupId>software.amazon.lambda</groupId>
133+
<artifactId>powertools-tracing</artifactId>
134+
</aspectLibrary>
135+
<aspectLibrary>
136+
<groupId>software.amazon.lambda</groupId>
137+
<artifactId>powertools-sqs</artifactId>
138+
</aspectLibrary>
139+
</aspectLibraries>
140+
</configuration>
141+
<executions>
142+
<execution>
143+
<goals>
144+
<goal>compile</goal>
145+
</goals>
146+
</execution>
147+
</executions>
148+
</plugin>
149+
</plugins>
150+
</build>
151+
</project>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
package software.amazon.lambda.powertools.testsuite;
2+
3+
4+
import java.io.ByteArrayInputStream;
5+
import java.io.ByteArrayOutputStream;
6+
import java.io.IOException;
7+
import java.lang.reflect.InvocationTargetException;
8+
import java.lang.reflect.Method;
9+
import java.nio.channels.FileChannel;
10+
import java.nio.charset.StandardCharsets;
11+
import java.nio.file.Files;
12+
import java.nio.file.Paths;
13+
import java.nio.file.StandardOpenOption;
14+
import java.util.Map;
15+
16+
import com.amazonaws.services.lambda.runtime.Context;
17+
import com.amazonaws.services.lambda.runtime.events.SQSEvent;
18+
import com.amazonaws.services.lambda.runtime.events.models.s3.S3EventNotification;
19+
import com.amazonaws.services.s3.AmazonS3;
20+
import com.amazonaws.services.s3.model.S3Object;
21+
import com.amazonaws.xray.AWSXRay;
22+
import com.fasterxml.jackson.core.JsonProcessingException;
23+
import com.fasterxml.jackson.databind.ObjectMapper;
24+
import org.apache.logging.log4j.Level;
25+
import org.apache.logging.log4j.ThreadContext;
26+
import org.junit.jupiter.api.AfterEach;
27+
import org.junit.jupiter.api.BeforeEach;
28+
import org.junit.jupiter.api.Test;
29+
import org.mockito.Mock;
30+
import software.amazon.lambda.powertools.core.internal.LambdaHandlerProcessor;
31+
import software.amazon.lambda.powertools.logging.internal.LambdaLoggingAspect;
32+
import software.amazon.lambda.powertools.sqs.internal.SqsLargeMessageAspect;
33+
import software.amazon.lambda.powertools.testsuite.handler.LoggingOrderMessageHandler;
34+
import software.amazon.lambda.powertools.testsuite.handler.TracingLoggingStreamMessageHandler;
35+
36+
import static java.util.Collections.emptyMap;
37+
import static java.util.Collections.singletonList;
38+
import static org.apache.commons.lang3.reflect.FieldUtils.writeStaticField;
39+
import static org.assertj.core.api.Assertions.assertThat;
40+
import static org.assertj.core.api.Assertions.fail;
41+
import static org.mockito.Mockito.when;
42+
import static org.mockito.MockitoAnnotations.openMocks;
43+
44+
public class LoggingOrderTest {
45+
46+
private static final String BUCKET_NAME = "ms-extended-sqs-client";
47+
private static final String BUCKET_KEY = "c71eb2ae-37e0-4265-8909-32f4153faddf";
48+
49+
@Mock
50+
private Context context;
51+
52+
@Mock
53+
private AmazonS3 amazonS3;
54+
55+
@BeforeEach
56+
void setUp() throws IllegalAccessException, IOException, NoSuchMethodException, InvocationTargetException {
57+
openMocks(this);
58+
writeStaticField(SqsLargeMessageAspect.class, "amazonS3", amazonS3, true);
59+
ThreadContext.clearAll();
60+
writeStaticField(LambdaHandlerProcessor.class, "IS_COLD_START", null, true);
61+
setupContext();
62+
//Make sure file is cleaned up before running full stack logging regression
63+
FileChannel.open(Paths.get("target/logfile.json"), StandardOpenOption.WRITE).truncate(0).close();
64+
resetLogLevel(Level.INFO);
65+
AWSXRay.beginSegment(LoggingOrderTest.class.getName());
66+
}
67+
68+
@AfterEach
69+
void tearDown() {
70+
AWSXRay.endSegment();
71+
}
72+
73+
/**
74+
* The SQSEvent payload will be altered by the @SqsLargeMessage annotation. Logging of the event should happen
75+
* after the event has been altered
76+
*/
77+
@Test
78+
public void testThatLoggingAnnotationActsLast() throws IOException {
79+
S3Object s3Response = new S3Object();
80+
s3Response.setObjectContent(new ByteArrayInputStream("A big message".getBytes()));
81+
82+
when(amazonS3.getObject(BUCKET_NAME, BUCKET_KEY)).thenReturn(s3Response);
83+
SQSEvent sqsEvent = messageWithBody("[\"software.amazon.payloadoffloading.PayloadS3Pointer\",{\"s3BucketName\":\"" + BUCKET_NAME + "\",\"s3Key\":\"" + BUCKET_KEY + "\"}]");
84+
85+
LoggingOrderMessageHandler requestHandler = new LoggingOrderMessageHandler();
86+
requestHandler.handleRequest(sqsEvent, context);
87+
88+
assertThat(Files.lines(Paths.get("target/logfile.json")))
89+
.hasSize(2)
90+
.satisfies(line -> {
91+
Map<String, Object> actual = parseToMap(line.get(0));
92+
93+
String message = actual.get("message").toString();
94+
95+
assertThat(message)
96+
.contains("A big message");
97+
});
98+
}
99+
100+
@Test
101+
public void testLoggingAnnotationActsAfterTracingForStreamingHandler() throws IOException {
102+
103+
ByteArrayOutputStream output = new ByteArrayOutputStream();
104+
S3EventNotification s3EventNotification = s3EventNotification();
105+
106+
TracingLoggingStreamMessageHandler handler = new TracingLoggingStreamMessageHandler();
107+
handler.handleRequest(new ByteArrayInputStream(new ObjectMapper().writeValueAsBytes(s3EventNotification)), output, context);
108+
109+
assertThat(new String(output.toByteArray(), StandardCharsets.UTF_8))
110+
.isNotEmpty();
111+
}
112+
113+
private void setupContext() {
114+
when(context.getFunctionName()).thenReturn("testFunction");
115+
when(context.getInvokedFunctionArn()).thenReturn("testArn");
116+
when(context.getFunctionVersion()).thenReturn("1");
117+
when(context.getMemoryLimitInMB()).thenReturn(10);
118+
when(context.getAwsRequestId()).thenReturn("RequestId");
119+
}
120+
121+
private void resetLogLevel(Level level) throws NoSuchMethodException, IllegalAccessException, InvocationTargetException {
122+
Method resetLogLevels = LambdaLoggingAspect.class.getDeclaredMethod("resetLogLevels", Level.class);
123+
resetLogLevels.setAccessible(true);
124+
resetLogLevels.invoke(null, level);
125+
writeStaticField(LambdaLoggingAspect.class, "LEVEL_AT_INITIALISATION", level, true);
126+
}
127+
128+
private Map<String, Object> parseToMap(String stringAsJson) {
129+
try {
130+
return new ObjectMapper().readValue(stringAsJson, Map.class);
131+
} catch (JsonProcessingException e) {
132+
fail("Failed parsing logger line " + stringAsJson);
133+
return emptyMap();
134+
}
135+
}
136+
137+
private S3EventNotification s3EventNotification() {
138+
S3EventNotification.S3EventNotificationRecord record = new S3EventNotification.S3EventNotificationRecord("us-west-2",
139+
"ObjectCreated:Put",
140+
"aws:s3",
141+
null,
142+
"2.1",
143+
new S3EventNotification.RequestParametersEntity("127.0.0.1"),
144+
new S3EventNotification.ResponseElementsEntity("C3D13FE58DE4C810", "FMyUVURIY8/IgAtTv8xRjskZQpcIZ9KG4V5Wp6S7S/JRWeUWerMUE5JgHvANOjpD"),
145+
new S3EventNotification.S3Entity("testConfigRule",
146+
new S3EventNotification.S3BucketEntity("mybucket",
147+
new S3EventNotification.UserIdentityEntity("A3NL1KOZZKExample"),
148+
"arn:aws:s3:::mybucket"),
149+
new S3EventNotification.S3ObjectEntity("HappyFace.jpg",
150+
1024L,
151+
"d41d8cd98f00b204e9800998ecf8427e",
152+
"096fKKXTRTtl3on89fVO.nfljtsv6qko",
153+
"0055AED6DCD90281E5"),
154+
"1.0"),
155+
new S3EventNotification.UserIdentityEntity("AIDAJDPLRKLG7UEXAMPLE")
156+
);
157+
158+
return new S3EventNotification(singletonList(record));
159+
}
160+
161+
private SQSEvent messageWithBody(String messageBody) {
162+
SQSEvent.SQSMessage sqsMessage = new SQSEvent.SQSMessage();
163+
sqsMessage.setBody(messageBody);
164+
SQSEvent sqsEvent = new SQSEvent();
165+
sqsEvent.setRecords(singletonList(sqsMessage));
166+
return sqsEvent;
167+
}
168+
}

0 commit comments

Comments
 (0)