Skip to content

Commit b17e13d

Browse files
ygreeygree
committed
Fix AWS Payload Tagging prefix generation related to SdkPojo (#7882)
* Fix nested SdkPojo fields ordering for tag generation. * Do not collect null values from AWS SdkPojos
1 parent 42ee2aa commit b17e13d

File tree

2 files changed

+30
-38
lines changed

2 files changed

+30
-38
lines changed

‎dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecorator.java‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import datadog.trace.payloadtags.PayloadTagsData;
2828
import java.net.URI;
2929
import java.time.Instant;
30-
import java.util.ArrayDeque;
3130
import java.util.ArrayList;
3231
import java.util.Collection;
3332
import java.util.Collections;
@@ -461,44 +460,42 @@ protected String getResponseHeader(SdkHttpResponse response, String headerName)
461460

462461
private void awsPojoToTags(AgentSpan span, String tagsPrefix, Object pojo) {
463462
Collection<PayloadTagsData.PathAndValue> payloadTagsData = new ArrayList<>();
464-
ArrayDeque<Object> path = new ArrayDeque<>();
463+
List<Object> path = new ArrayList<>();
465464
collectPayloadTagsData(payloadTagsData, path, pojo);
466465
span.setTag(
467466
tagsPrefix,
468467
new PayloadTagsData(payloadTagsData.toArray(new PayloadTagsData.PathAndValue[0])));
469468
}
470469

471470
private void collectPayloadTagsData(
472-
Collection<PayloadTagsData.PathAndValue> payloadTagsData,
473-
ArrayDeque<Object> path,
474-
Object object) {
471+
Collection<PayloadTagsData.PathAndValue> payloadTagsData, List<Object> path, Object object) {
475472
if (object instanceof SdkPojo) {
476473
SdkPojo pojo = (SdkPojo) object;
477474
for (SdkField<?> field : pojo.sdkFields()) {
478475
Object val = field.getValueOrDefault(pojo);
479-
path.push(field.locationName());
476+
path.add(field.locationName());
480477
collectPayloadTagsData(payloadTagsData, path, val);
481-
path.pop();
478+
path.remove(path.size() - 1);
482479
}
483480
} else if (object instanceof Collection) {
484481
int index = 0;
485482
for (Object value : (Collection<?>) object) {
486-
path.push(index);
483+
path.add(index);
487484
collectPayloadTagsData(payloadTagsData, path, value);
488-
path.pop();
485+
path.remove(path.size() - 1);
489486
index++;
490487
}
491488
} else if (object instanceof Map) {
492489
Map<?, ?> map = (Map<?, ?>) object;
493490
for (Map.Entry<?, ?> entry : map.entrySet()) {
494-
path.push(entry.getKey().toString());
491+
path.add(entry.getKey().toString());
495492
collectPayloadTagsData(payloadTagsData, path, entry.getValue());
496-
path.pop();
493+
path.remove(path.size() - 1);
497494
}
498495
} else if (object instanceof SdkBytes) {
499496
SdkBytes bytes = (SdkBytes) object;
500497
payloadTagsData.add(new PayloadTagsData.PathAndValue(path.toArray(), bytes.asInputStream()));
501-
} else {
498+
} else if (object != null) {
502499
payloadTagsData.add(new PayloadTagsData.PathAndValue(path.toArray(), object));
503500
}
504501
}

‎dd-java-agent/instrumentation/aws-java-sdk-2.2/src/payloadTaggingTest/groovy/PayloadTaggingTest.groovy‎

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,8 @@ class PayloadTaggingRedactionForkedTest extends AbstractPayloadTaggingTest {
114114
@Override
115115
protected void configurePreAgent() {
116116
super.configurePreAgent()
117-
def redactTopLevelTags = "\$.*,\$.DisplayName.Owner"
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")
120119
}
121120

122121
def "test payload tag observability support for #service"() {
@@ -133,45 +132,46 @@ class PayloadTaggingRedactionForkedTest extends AbstractPayloadTaggingTest {
133132
span {
134133
spanType DDSpanTypes.HTTP_CLIENT
135134
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"
137136
assert expectedRespTag == NA || span.tags.get("aws.response.body." + expectedRespTag) == "redacted"
138137

139138
assert !span.tags.containsKey("_dd.payload_tags_incomplete")
140139
assert !span.tags.containsKey("aws.request.body")
141140
assert !span.tags.containsKey("aws.response.body")
141+
assert !span.tags.containsValue(null)
142142
}
143143
}
144144
}
145145

146146
where:
147-
service | expectedReqTag | expectedRespTag | apiCall
148-
"ApiGateway" | "name" | "value" | {
147+
service | expectedReqTag | expectedRespTag | apiCall
148+
"ApiGateway" | "name" | "value" | {
149149
apiGatewayClient.createApiKey {
150-
it.name("testapi")
150+
it.name("tagvalue")
151151
}
152152
}
153-
"EventBridge" | "Name" | "EventBusArn" | {
153+
"EventBridge" | "Name" | "EventBusArn" | {
154154
eventBridgeClient.createEventBus {
155-
it.name("testbus")
155+
it.name("tagvalue")
156156
}
157157
}
158-
"Sns" | "Name" | "TopicArn" | {
158+
"Sns" | "Name" | "TopicArn" | {
159159
snsClient.createTopic {
160-
it.name("testtopic")
160+
it.name("tagvalue")
161161
}
162162
}
163-
"Sqs" | "QueueName" | "QueueUrl" | {
163+
"Sqs" | "QueueName" | "QueueUrl" | {
164164
sqsClient.createQueue {
165-
it.queueName("testqueue")
165+
it.queueName("tagvalue")
166166
}
167167
}
168-
"Kinesis" | "StreamModeDetails" | NA | {
168+
"Kinesis" | "StreamName" | NA | {
169169
kinesisClient.createStream {
170-
it.streamName("teststream")
170+
it.streamName("tagvalue")
171171
}
172172
}
173-
"Kinesis" | NA | "ShardLimit" | { kinesisClient.describeLimits() }
174-
"S3" | NA | "DisplayName.Owner" | { s3Client.listBuckets() }
173+
"Kinesis" | NA | "ShardLimit" | { kinesisClient.describeLimits() }
174+
"S3" | NA | "Owner.DisplayName" | { s3Client.listBuckets() }
175175
}
176176
}
177177

@@ -215,7 +215,7 @@ class PayloadTaggingExpansionForkedTest extends AbstractPayloadTaggingTest {
215215
.message('{ "sms": "sms text", "default": "default text" }')
216216
}
217217
}
218-
"Key.0.Tags" | "foo" | {
218+
"Tags.0.Key" | "foo" | {
219219
snsClient.createTopic {
220220
it.name("testtopic")
221221
.tags(Tag.builder().key("foo").value("bar").build(), Tag.builder().key("t").value("1").build())
@@ -224,17 +224,17 @@ class PayloadTaggingExpansionForkedTest extends AbstractPayloadTaggingTest {
224224
"nextToken" | null | {
225225
snsClient.listPhoneNumbersOptedOut()
226226
}
227-
"DefaultSMSType.attributes" | "bar" | {
227+
"attributes.DefaultSMSType" | "bar" | {
228228
snsClient.setSMSAttributes { it.attributes(["DefaultSenderID": "foo", "DefaultSMSType": "bar"]) }
229229
}
230-
"BinaryValue.foo\\.bar.MessageAttributes.abc\\.def" | 42 | {
230+
"MessageAttributes.foo\\.bar.BinaryValue.abc\\.def" | 42 | {
231231
snsClient.publish {
232232
it.phoneNumber("+15555555555").message("testmessage")
233233
.messageAttributes(["foo.bar": snsBinaryAttribute('{"abc.def": 42}')
234234
])
235235
}
236236
}
237-
"BinaryValue.foo\\.bar.MessageAttributes" | "<binary>" | {
237+
"MessageAttributes.foo\\.bar.BinaryValue" | "<binary>" | {
238238
snsClient.publish {
239239
it.phoneNumber("+15555555555").message("testmessage").messageAttributes(["foo.bar": snsBinaryAttribute('{"invalid json: 42}')])
240240
}
@@ -346,11 +346,6 @@ class PayloadTaggingMaxTagsForkedTest extends AbstractPayloadTaggingTest {
346346

347347
where:
348348
apiCall << [
349-
{
350-
snsClient.publish {
351-
it.phoneNumber("+15555555555").message('message')
352-
}
353-
},
354349
{
355350
snsClient.createTopic {
356351
it.name("testtopic")

0 commit comments

Comments
 (0)