Skip to content

Commit 16f3647

Browse files
authored
Fixed kafka produce extractor error (#10610)
Fixed kafka produce extractor and added defensive try/catch.
1 parent 7f66c79 commit 16f3647

5 files changed

Lines changed: 163 additions & 24 deletions

File tree

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpClientDecorator.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,13 @@ protected boolean shouldSetResourceName() {
7272

7373
private final DataStreamsTransactionTracker.TransactionSourceReader
7474
DSM_TRANSACTION_SOURCE_READER =
75-
(source, headerName) -> getRequestHeader((REQUEST) source, headerName);
75+
(source, headerName) -> {
76+
try {
77+
return getRequestHeader((REQUEST) source, headerName);
78+
} catch (Throwable ignored) {
79+
return null;
80+
}
81+
};
7682

7783
public AgentSpan onRequest(final AgentSpan span, final REQUEST request) {
7884
if (request != null) {

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,13 @@ protected AgentSpanContext startInferredProxySpan(Context context, AgentSpanCont
185185

186186
private final DataStreamsTransactionTracker.TransactionSourceReader
187187
DSM_TRANSACTION_SOURCE_READER =
188-
(source, headerName) -> getRequestHeader((REQUEST) source, headerName);
188+
(source, headerName) -> {
189+
try {
190+
return getRequestHeader((REQUEST) source, headerName);
191+
} catch (Throwable ignored) {
192+
return null;
193+
}
194+
};
189195

190196
public AgentSpan onRequest(
191197
final AgentSpan span,

dd-java-agent/instrumentation/kafka/kafka-clients-0.11/src/main/java/datadog/trace/instrumentation/kafka_clients/KafkaProducerInstrumentation.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ record =
211211
.trackTransaction(
212212
span,
213213
DataStreamsTransactionExtractor.Type.KAFKA_PRODUCE_HEADERS,
214-
record,
214+
record.headers(),
215215
DSM_TRANSACTION_SOURCE_READER);
216216
return activateSpan(span);
217217
}

dd-java-agent/instrumentation/kafka/kafka-clients-0.11/src/test/groovy/KafkaClientTestBase.groovy

Lines changed: 141 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import datadog.trace.api.datastreams.DataStreamsTags
2+
import datadog.trace.api.datastreams.DataStreamsTransactionExtractor
23
import datadog.trace.instrumentation.kafka_common.ClusterIdHolder
34

45
import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
@@ -1047,6 +1048,126 @@ abstract class KafkaClientTestBase extends VersionedNamingTestBase {
10471048
producer?.close()
10481049
}
10491050

1051+
def "test producer DSM transaction tracking extracts transaction id from headers"() {
1052+
setup:
1053+
if (!isDataStreamsEnabled()) {
1054+
return
1055+
}
1056+
1057+
injectEnvConfig("DD_DATA_STREAMS_ENABLED", "true")
1058+
1059+
// Configure a DSM transaction extractor for KAFKA_PRODUCE_HEADERS
1060+
def extractorsByTypeField = TEST_DATA_STREAMS_MONITORING.getClass().getDeclaredField("extractorsByType")
1061+
extractorsByTypeField.setAccessible(true)
1062+
def oldExtractorsByType = extractorsByTypeField.get(TEST_DATA_STREAMS_MONITORING)
1063+
1064+
def extractor = new DataStreamsTransactionExtractor() {
1065+
String getName() {
1066+
return "kafka-produce-test"
1067+
}
1068+
DataStreamsTransactionExtractor.Type getType() {
1069+
return DataStreamsTransactionExtractor.Type.KAFKA_PRODUCE_HEADERS
1070+
}
1071+
String getValue() {
1072+
return "x-transaction-id"
1073+
}
1074+
}
1075+
def extractorsByType = new EnumMap<>(DataStreamsTransactionExtractor.Type)
1076+
extractorsByType.put(DataStreamsTransactionExtractor.Type.KAFKA_PRODUCE_HEADERS, [extractor])
1077+
extractorsByTypeField.set(TEST_DATA_STREAMS_MONITORING, extractorsByType)
1078+
1079+
def senderProps = KafkaTestUtils.senderProps(embeddedKafka.getBrokersAsString())
1080+
def producer = new KafkaProducer<>(senderProps, new StringSerializer(), new StringSerializer())
1081+
1082+
def headers = new RecordHeaders()
1083+
headers.add(new RecordHeader("x-transaction-id", "txn-123".getBytes(StandardCharsets.UTF_8)))
1084+
1085+
when:
1086+
def record = new ProducerRecord(SHARED_TOPIC, 0, null, "test-dsm-transaction", headers)
1087+
producer.send(record).get()
1088+
1089+
then:
1090+
TEST_WRITER.waitForTraces(1)
1091+
def producedSpan = TEST_WRITER[0][0]
1092+
producedSpan.getTag(Tags.DSM_TRANSACTION_ID) == "txn-123"
1093+
producedSpan.getTag(Tags.DSM_TRANSACTION_CHECKPOINT) == "kafka-produce-test"
1094+
1095+
cleanup:
1096+
extractorsByTypeField?.set(TEST_DATA_STREAMS_MONITORING, oldExtractorsByType)
1097+
producer?.close()
1098+
}
1099+
1100+
def "test consumer DSM transaction tracking extracts transaction id from headers"() {
1101+
setup:
1102+
if (!isDataStreamsEnabled()) {
1103+
return
1104+
}
1105+
1106+
injectEnvConfig("DD_DATA_STREAMS_ENABLED", "true")
1107+
1108+
// Configure a DSM transaction extractor for KAFKA_CONSUME_HEADERS
1109+
def extractorsByTypeField = TEST_DATA_STREAMS_MONITORING.getClass().getDeclaredField("extractorsByType")
1110+
extractorsByTypeField.setAccessible(true)
1111+
def oldExtractorsByType = extractorsByTypeField.get(TEST_DATA_STREAMS_MONITORING)
1112+
1113+
def extractor = new DataStreamsTransactionExtractor() {
1114+
String getName() {
1115+
return "kafka-consume-test"
1116+
}
1117+
DataStreamsTransactionExtractor.Type getType() {
1118+
return DataStreamsTransactionExtractor.Type.KAFKA_CONSUME_HEADERS
1119+
}
1120+
String getValue() {
1121+
return "x-transaction-id"
1122+
}
1123+
}
1124+
def extractorsByType = new EnumMap<>(DataStreamsTransactionExtractor.Type)
1125+
extractorsByType.put(DataStreamsTransactionExtractor.Type.KAFKA_CONSUME_HEADERS, [extractor])
1126+
extractorsByTypeField.set(TEST_DATA_STREAMS_MONITORING, extractorsByType)
1127+
1128+
def kafkaPartition = 0
1129+
def consumerProperties = KafkaTestUtils.consumerProps("sender", "false", embeddedKafka)
1130+
consumerProperties.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, "earliest")
1131+
def consumer = new KafkaConsumer<String, String>(consumerProperties)
1132+
1133+
def senderProps = KafkaTestUtils.senderProps(embeddedKafka.getBrokersAsString())
1134+
def producer = new KafkaProducer<>(senderProps, new StringSerializer(), new StringSerializer())
1135+
1136+
consumer.assign(Arrays.asList(new TopicPartition(SHARED_TOPIC, kafkaPartition)))
1137+
1138+
def headers = new RecordHeaders()
1139+
headers.add(new RecordHeader("x-transaction-id", "txn-456".getBytes(StandardCharsets.UTF_8)))
1140+
1141+
when:
1142+
def record = new ProducerRecord(SHARED_TOPIC, kafkaPartition, null, "test-dsm-consume-transaction", headers)
1143+
producer.send(record).get()
1144+
1145+
then:
1146+
TEST_WRITER.waitForTraces(1)
1147+
def pollResult = KafkaTestUtils.getRecords(consumer)
1148+
def recs = pollResult.records(new TopicPartition(SHARED_TOPIC, kafkaPartition)).iterator()
1149+
recs.hasNext()
1150+
recs.next().value() == "test-dsm-consume-transaction"
1151+
1152+
// The consume span is created by TracingIterator when iterating over records
1153+
// Find the consumer span with the DSM transaction tags
1154+
TEST_WRITER.waitForTraces(2)
1155+
def allTraces = TEST_WRITER.toArray() as List<List<DDSpan>>
1156+
def consumerSpan = allTraces.collectMany {
1157+
it
1158+
}.find {
1159+
it.getTag(Tags.DSM_TRANSACTION_ID) == "txn-456"
1160+
}
1161+
consumerSpan != null
1162+
consumerSpan.getTag(Tags.DSM_TRANSACTION_ID) == "txn-456"
1163+
consumerSpan.getTag(Tags.DSM_TRANSACTION_CHECKPOINT) == "kafka-consume-test"
1164+
1165+
cleanup:
1166+
extractorsByTypeField?.set(TEST_DATA_STREAMS_MONITORING, oldExtractorsByType)
1167+
consumer?.close()
1168+
producer?.close()
1169+
}
1170+
10501171
def containerProperties() {
10511172
try {
10521173
// Different class names for test and latestDepTest.
@@ -1057,12 +1178,12 @@ abstract class KafkaClientTestBase extends VersionedNamingTestBase {
10571178
}
10581179

10591180
def producerSpan(
1060-
TraceAssert trace,
1061-
Map<String, ?> config,
1062-
DDSpan parentSpan = null,
1063-
boolean partitioned = true,
1064-
boolean tombstone = false,
1065-
String schema = null
1181+
TraceAssert trace,
1182+
Map<String, ?> config,
1183+
DDSpan parentSpan = null,
1184+
boolean partitioned = true,
1185+
boolean tombstone = false,
1186+
String schema = null
10661187
) {
10671188
trace.span {
10681189
serviceName service()
@@ -1104,8 +1225,8 @@ abstract class KafkaClientTestBase extends VersionedNamingTestBase {
11041225
}
11051226

11061227
def queueSpan(
1107-
TraceAssert trace,
1108-
DDSpan parentSpan = null
1228+
TraceAssert trace,
1229+
DDSpan parentSpan = null
11091230
) {
11101231
trace.span {
11111232
serviceName splitByDestination() ? "$SHARED_TOPIC" : serviceForTimeInQueue()
@@ -1128,12 +1249,12 @@ abstract class KafkaClientTestBase extends VersionedNamingTestBase {
11281249
}
11291250

11301251
def consumerSpan(
1131-
TraceAssert trace,
1132-
Map<String, Object> config,
1133-
DDSpan parentSpan = null,
1134-
Range offset = 0..0,
1135-
boolean tombstone = false,
1136-
boolean distributedRootSpan = !hasQueueSpan()
1252+
TraceAssert trace,
1253+
Map<String, Object> config,
1254+
DDSpan parentSpan = null,
1255+
Range offset = 0..0,
1256+
boolean tombstone = false,
1257+
boolean distributedRootSpan = !hasQueueSpan()
11371258
) {
11381259
trace.span {
11391260
serviceName service()
@@ -1169,12 +1290,12 @@ abstract class KafkaClientTestBase extends VersionedNamingTestBase {
11691290
}
11701291

11711292
def pollSpan(
1172-
TraceAssert trace,
1173-
int recordCount = 1,
1174-
DDSpan parentSpan = null,
1175-
Range offset = 0..0,
1176-
boolean tombstone = false,
1177-
boolean distributedRootSpan = !hasQueueSpan()
1293+
TraceAssert trace,
1294+
int recordCount = 1,
1295+
DDSpan parentSpan = null,
1296+
Range offset = 0..0,
1297+
boolean tombstone = false,
1298+
boolean distributedRootSpan = !hasQueueSpan()
11781299
) {
11791300
trace.span {
11801301
serviceName Config.get().getServiceName()

dd-java-agent/instrumentation/kafka/kafka-common/src/main/java/datadog/trace/instrumentation/kafka_common/Utils.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,13 @@ private Utils() {} // prevent instantiation
1111

1212
public static DataStreamsTransactionTracker.TransactionSourceReader
1313
DSM_TRANSACTION_SOURCE_READER =
14-
(source, headerName) -> new String(((Headers) source).lastHeader(headerName).value());
14+
(source, headerName) -> {
15+
try {
16+
return new String(((Headers) source).lastHeader(headerName).value());
17+
} catch (Throwable ignored) {
18+
return null;
19+
}
20+
};
1521

1622
// this method is used in kafka-clients and kafka-streams instrumentations
1723
public static long computePayloadSizeBytes(ConsumerRecord<?, ?> val) {

0 commit comments

Comments
 (0)