Do not collect null values from AWS SdkPojos

ygree · ygree · commit 99608c45eaae · 2024-11-06T10:33:12.000-08:00
diff --git a/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecorator.java b/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecorator.java
@@ -495,7 +495,7 @@ private void collectPayloadTagsData(
     } else if (object instanceof SdkBytes) {
       SdkBytes bytes = (SdkBytes) object;
       payloadTagsData.add(new PayloadTagsData.PathAndValue(path.toArray(), bytes.asInputStream()));
-    } else {
+    } else if (object != null) {
       payloadTagsData.add(new PayloadTagsData.PathAndValue(path.toArray(), object));
     }
   }
diff --git a/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/payloadTaggingTest/groovy/PayloadTaggingTest.groovy b/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/payloadTaggingTest/groovy/PayloadTaggingTest.groovy
@@ -114,9 +114,8 @@ class PayloadTaggingRedactionForkedTest extends AbstractPayloadTaggingTest {
   @Override
   protected void configurePreAgent() {
     super.configurePreAgent()
-    def redactTopLevelTags = "\$.*,\$.Owner.DisplayName"
-    injectSysConfig(TracerConfig.TRACE_CLOUD_REQUEST_PAYLOAD_TAGGING, redactTopLevelTags)
-    injectSysConfig(TracerConfig.TRACE_CLOUD_RESPONSE_PAYLOAD_TAGGING, redactTopLevelTags)
+    injectSysConfig(TracerConfig.TRACE_CLOUD_REQUEST_PAYLOAD_TAGGING, "all")
+    injectSysConfig(TracerConfig.TRACE_CLOUD_RESPONSE_PAYLOAD_TAGGING, "\$.*,\$.Owner.DisplayName")
   }
 
   def "test payload tag observability support for #service"() {
@@ -133,45 +132,46 @@ class PayloadTaggingRedactionForkedTest extends AbstractPayloadTaggingTest {
         span {
           spanType DDSpanTypes.HTTP_CLIENT
           childOf(span(0))
-          assert expectedReqTag == NA || span.tags.get("aws.request.body." + expectedReqTag) == "redacted"
+          assert expectedReqTag == NA || span.tags.get("aws.request.body." + expectedReqTag) == "tagvalue"
           assert expectedRespTag == NA || span.tags.get("aws.response.body." + expectedRespTag) == "redacted"
 
           assert !span.tags.containsKey("_dd.payload_tags_incomplete")
           assert !span.tags.containsKey("aws.request.body")
           assert !span.tags.containsKey("aws.response.body")
+          assert !span.tags.containsValue(null)
         }
       }
     }
 
     where:
-    service       | expectedReqTag                | expectedRespTag      | apiCall
-    "ApiGateway"  | "name"                        | "value"              | {
+    service       | expectedReqTag | expectedRespTag     | apiCall
+    "ApiGateway"  | "name"         | "value"             | {
       apiGatewayClient.createApiKey {
-        it.name("testapi")
+        it.name("tagvalue")
       }
     }
-    "EventBridge" | "Name"                        | "EventBusArn"        | {
+    "EventBridge" | "Name"         | "EventBusArn"       | {
       eventBridgeClient.createEventBus {
-        it.name("testbus")
+        it.name("tagvalue")
       }
     }
-    "Sns"         | "Name"                        | "TopicArn"           | {
+    "Sns"         | "Name"         | "TopicArn"          | {
       snsClient.createTopic {
-        it.name("testtopic")
+        it.name("tagvalue")
       }
     }
-    "Sqs"         | "QueueName"                   | "QueueUrl"           | {
+    "Sqs"         | "QueueName"    | "QueueUrl"          | {
       sqsClient.createQueue {
-        it.queueName("testqueue")
+        it.queueName("tagvalue")
       }
     }
-    "Kinesis"     | "StreamModeDetails"           | NA                   | {
+    "Kinesis"     | "StreamName"   | NA                  | {
       kinesisClient.createStream {
-        it.streamName("teststream")
+        it.streamName("tagvalue")
       }
     }
-    "Kinesis"     | NA                            | "ShardLimit"         | { kinesisClient.describeLimits() }
-    "S3"          | NA                            | "Owner.DisplayName"  | { s3Client.listBuckets() }
+    "Kinesis"     | NA             | "ShardLimit"        | { kinesisClient.describeLimits() }
+    "S3"          | NA             | "Owner.DisplayName" | { s3Client.listBuckets() }
   }
 }
 
@@ -346,11 +346,6 @@ class PayloadTaggingMaxTagsForkedTest extends AbstractPayloadTaggingTest {
 
     where:
     apiCall << [
-      {
-        snsClient.publish {
-          it.phoneNumber("+15555555555").message('message')
-        }
-      },
       {
         snsClient.createTopic {
           it.name("testtopic")

Original file line number	Diff line number	Diff line change
`@@ -495,7 +495,7 @@ private void collectPayloadTagsData(`
`495`	`495`	`} else if (object instanceof SdkBytes) {`
`496`	`496`	`SdkBytes bytes = (SdkBytes) object;`
`497`	`497`	`payloadTagsData.add(new PayloadTagsData.PathAndValue(path.toArray(), bytes.asInputStream()));`
`498`		`- } else {`
	`498`	`+ } else if (object != null) {`
`499`	`499`	`payloadTagsData.add(new PayloadTagsData.PathAndValue(path.toArray(), object));`
`500`	`500`	`}`
`501`	`501`	`}`
Original file line number	Diff line number	Diff line change
`@@ -114,9 +114,8 @@ class PayloadTaggingRedactionForkedTest extends AbstractPayloadTaggingTest {`
`114`	`114`	`@Override`
`115`	`115`	`protected void configurePreAgent() {`
`116`	`116`	`super.configurePreAgent()`
`117`		`- def redactTopLevelTags = "\$.*,\$.Owner.DisplayName"`
`118`		`- injectSysConfig(TracerConfig.TRACE_CLOUD_REQUEST_PAYLOAD_TAGGING, redactTopLevelTags)`
`119`		`- injectSysConfig(TracerConfig.TRACE_CLOUD_RESPONSE_PAYLOAD_TAGGING, redactTopLevelTags)`
	`117`	`+ injectSysConfig(TracerConfig.TRACE_CLOUD_REQUEST_PAYLOAD_TAGGING, "all")`
	`118`	`+ injectSysConfig(TracerConfig.TRACE_CLOUD_RESPONSE_PAYLOAD_TAGGING, "\$.*,\$.Owner.DisplayName")`
`120`	`119`	`}`
`121`	`120`
`122`	`121`	`def "test payload tag observability support for #service"() {`
`@@ -133,45 +132,46 @@ class PayloadTaggingRedactionForkedTest extends AbstractPayloadTaggingTest {`
`133`	`132`	`span {`
`134`	`133`	`spanType DDSpanTypes.HTTP_CLIENT`
`135`	`134`	`childOf(span(0))`
`136`		`- assert expectedReqTag == NA \|\| span.tags.get("aws.request.body." + expectedReqTag) == "redacted"`
	`135`	`+ assert expectedReqTag == NA \|\| span.tags.get("aws.request.body." + expectedReqTag) == "tagvalue"`
`137`	`136`	`assert expectedRespTag == NA \|\| span.tags.get("aws.response.body." + expectedRespTag) == "redacted"`
`138`	`137`
`139`	`138`	`assert !span.tags.containsKey("_dd.payload_tags_incomplete")`
`140`	`139`	`assert !span.tags.containsKey("aws.request.body")`
`141`	`140`	`assert !span.tags.containsKey("aws.response.body")`
	`141`	`+ assert !span.tags.containsValue(null)`
`142`	`142`	`}`
`143`	`143`	`}`
`144`	`144`	`}`
`145`	`145`
`146`	`146`	`where:`
`147`		`- service \| expectedReqTag \| expectedRespTag \| apiCall`
`148`		`- "ApiGateway" \| "name" \| "value" \| {`
	`147`	`+ service \| expectedReqTag \| expectedRespTag \| apiCall`
	`148`	`+ "ApiGateway" \| "name" \| "value" \| {`
`149`	`149`	`apiGatewayClient.createApiKey {`
`150`		`- it.name("testapi")`
	`150`	`+ it.name("tagvalue")`
`151`	`151`	`}`
`152`	`152`	`}`
`153`		`- "EventBridge" \| "Name" \| "EventBusArn" \| {`
	`153`	`+ "EventBridge" \| "Name" \| "EventBusArn" \| {`
`154`	`154`	`eventBridgeClient.createEventBus {`
`155`		`- it.name("testbus")`
	`155`	`+ it.name("tagvalue")`
`156`	`156`	`}`
`157`	`157`	`}`
`158`		`- "Sns" \| "Name" \| "TopicArn" \| {`
	`158`	`+ "Sns" \| "Name" \| "TopicArn" \| {`
`159`	`159`	`snsClient.createTopic {`
`160`		`- it.name("testtopic")`
	`160`	`+ it.name("tagvalue")`
`161`	`161`	`}`
`162`	`162`	`}`
`163`		`- "Sqs" \| "QueueName" \| "QueueUrl" \| {`
	`163`	`+ "Sqs" \| "QueueName" \| "QueueUrl" \| {`
`164`	`164`	`sqsClient.createQueue {`
`165`		`- it.queueName("testqueue")`
	`165`	`+ it.queueName("tagvalue")`
`166`	`166`	`}`
`167`	`167`	`}`
`168`		`- "Kinesis" \| "StreamModeDetails" \| NA \| {`
	`168`	`+ "Kinesis" \| "StreamName" \| NA \| {`
`169`	`169`	`kinesisClient.createStream {`
`170`		`- it.streamName("teststream")`
	`170`	`+ it.streamName("tagvalue")`
`171`	`171`	`}`
`172`	`172`	`}`
`173`		`- "Kinesis" \| NA \| "ShardLimit" \| { kinesisClient.describeLimits() }`
`174`		`- "S3" \| NA \| "Owner.DisplayName" \| { s3Client.listBuckets() }`
	`173`	`+ "Kinesis" \| NA \| "ShardLimit" \| { kinesisClient.describeLimits() }`
	`174`	`+ "S3" \| NA \| "Owner.DisplayName" \| { s3Client.listBuckets() }`
`175`	`175`	`}`
`176`	`176`	`}`
`177`	`177`
`@@ -346,11 +346,6 @@ class PayloadTaggingMaxTagsForkedTest extends AbstractPayloadTaggingTest {`
`346`	`346`
`347`	`347`	`where:`
`348`	`348`	`apiCall << [`
`349`		`- {`
`350`		`- snsClient.publish {`
`351`		`- it.phoneNumber("+15555555555").message('message')`
`352`		`- }`
`353`		`- },`
`354`	`349`	`{`
`355`	`350`	`snsClient.createTopic {`
`356`	`351`	`it.name("testtopic")`