Skip to content

Commit

Permalink
[🍒 7882] Fix AWS Payload Tagging prefix generation related to SdkPojo (…
Browse files Browse the repository at this point in the history 
…#7901)

* Fix AWS Payload Tagging prefix generation related to SdkPojo.
* Do not collect null values from AWS SdkPojos
  • Loading branch information
@ygree
ygree authored Nov 6, 2024
1 parent 92bc5b5 commit 7eed45f
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 38 deletions.
21 changes: 9 additions & 12 deletions ...ava-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecorator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import datadog.trace.payloadtags.PayloadTagsData;
import java.net.URI;
import java.time.Instant;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -461,44 +460,42 @@ protected String getResponseHeader(SdkHttpResponse response, String headerName)

private void awsPojoToTags(AgentSpan span, String tagsPrefix, Object pojo) {
Collection<PayloadTagsData.PathAndValue> payloadTagsData = new ArrayList<>();
ArrayDeque<Object> path = new ArrayDeque<>();
List<Object> path = new ArrayList<>();
collectPayloadTagsData(payloadTagsData, path, pojo);
span.setTag(
tagsPrefix,
new PayloadTagsData(payloadTagsData.toArray(new PayloadTagsData.PathAndValue[0])));
}

private void collectPayloadTagsData(
Collection<PayloadTagsData.PathAndValue> payloadTagsData,
ArrayDeque<Object> path,
Object object) {
Collection<PayloadTagsData.PathAndValue> payloadTagsData, List<Object> path, Object object) {
if (object instanceof SdkPojo) {
SdkPojo pojo = (SdkPojo) object;
for (SdkField<?> field : pojo.sdkFields()) {
Object val = field.getValueOrDefault(pojo);
path.push(field.locationName());
path.add(field.locationName());
collectPayloadTagsData(payloadTagsData, path, val);
path.pop();
path.remove(path.size() - 1);
}
} else if (object instanceof Collection) {
int index = 0;
for (Object value : (Collection<?>) object) {
path.push(index);
path.add(index);
collectPayloadTagsData(payloadTagsData, path, value);
path.pop();
path.remove(path.size() - 1);
index++;
}
} else if (object instanceof Map) {
Map<?, ?> map = (Map<?, ?>) object;
for (Map.Entry<?, ?> entry : map.entrySet()) {
path.push(entry.getKey().toString());
path.add(entry.getKey().toString());
collectPayloadTagsData(payloadTagsData, path, entry.getValue());
path.pop();
path.remove(path.size() - 1);
}
} 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));
}
}
Expand Down
47 changes: 21 additions & 26 deletions .../instrumentation/aws-java-sdk-2.2/src/payloadTaggingTest/groovy/PayloadTaggingTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,8 @@ class PayloadTaggingRedactionForkedTest extends AbstractPayloadTaggingTest {
@Override
protected void configurePreAgent() {
super.configurePreAgent()
def redactTopLevelTags = "\$.*,\$.DisplayName.Owner"
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"() {
Expand All @@ -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 | "DisplayName.Owner" | { s3Client.listBuckets() }
"Kinesis" | NA | "ShardLimit" | { kinesisClient.describeLimits() }
"S3" | NA | "Owner.DisplayName" | { s3Client.listBuckets() }
}
}

Expand Down Expand Up @@ -215,7 +215,7 @@ class PayloadTaggingExpansionForkedTest extends AbstractPayloadTaggingTest {
.message('{ "sms": "sms text", "default": "default text" }')
}
}
"Key.0.Tags" | "foo" | {
"Tags.0.Key" | "foo" | {
snsClient.createTopic {
it.name("testtopic")
.tags(Tag.builder().key("foo").value("bar").build(), Tag.builder().key("t").value("1").build())
Expand All @@ -224,17 +224,17 @@ class PayloadTaggingExpansionForkedTest extends AbstractPayloadTaggingTest {
"nextToken" | null | {
snsClient.listPhoneNumbersOptedOut()
}
"DefaultSMSType.attributes" | "bar" | {
"attributes.DefaultSMSType" | "bar" | {
snsClient.setSMSAttributes { it.attributes(["DefaultSenderID": "foo", "DefaultSMSType": "bar"]) }
}
"BinaryValue.foo\\.bar.MessageAttributes.abc\\.def" | 42 | {
"MessageAttributes.foo\\.bar.BinaryValue.abc\\.def" | 42 | {
snsClient.publish {
it.phoneNumber("+15555555555").message("testmessage")
.messageAttributes(["foo.bar": snsBinaryAttribute('{"abc.def": 42}')
])
}
}
"BinaryValue.foo\\.bar.MessageAttributes" | "<binary>" | {
"MessageAttributes.foo\\.bar.BinaryValue" | "<binary>" | {
snsClient.publish {
it.phoneNumber("+15555555555").message("testmessage").messageAttributes(["foo.bar": snsBinaryAttribute('{"invalid json: 42}')])
}
Expand Down Expand Up @@ -346,11 +346,6 @@ class PayloadTaggingMaxTagsForkedTest extends AbstractPayloadTaggingTest {

where:
apiCall << [
{
snsClient.publish {
it.phoneNumber("+15555555555").message('message')
}
},
{
snsClient.createTopic {
it.name("testtopic")
Expand Down

0 comments on commit 7eed45f

Please sign in to comment.