Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,9 @@ private static void addReportToFlare(ZipOutputStream zip) throws IOException {
"SymbolDB stats:",
symbolDBStats,
"Exception Fingerprints:",
exceptionFingerprints);
exceptionFingerprints,
"SourceFile tracking entries:",
String.valueOf(classesToRetransformFinder.getClassNamesBySourceFile().size()));
TracerFlare.addText(zip, "dynamic_instrumentation.txt", content);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.datadog.debugger.util.ClassFileHelper;
import com.datadog.debugger.util.ClassNameFiltering;
import datadog.trace.api.Config;
import datadog.trace.relocate.api.RatelimitedLogger;
import datadog.trace.util.AgentTaskScheduler;
import datadog.trace.util.Strings;
import java.lang.instrument.ClassFileTransformer;
Expand All @@ -14,6 +15,7 @@
import java.util.Queue;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -24,10 +26,15 @@
*/
public class SourceFileTrackingTransformer implements ClassFileTransformer {
private static final Logger LOGGER = LoggerFactory.getLogger(SourceFileTrackingTransformer.class);
private static final int MINUTES_BETWEEN_ERROR_LOG = 5;
static final int MAX_QUEUE_SIZE = 4096;

private final RatelimitedLogger ratelimitedLogger =
new RatelimitedLogger(LOGGER, MINUTES_BETWEEN_ERROR_LOG, TimeUnit.MINUTES);
private final ClassesToRetransformFinder finder;
private final Queue<SourceFileItem> queue = new ConcurrentLinkedQueue<>();
private final AgentTaskScheduler scheduler = AgentTaskScheduler.INSTANCE;
private final AtomicInteger queueSize = new AtomicInteger(0);
private AgentTaskScheduler.Scheduled<Runnable> scheduled;
// this field MUST only be used in flush() calling thread
private ClassNameFiltering classNameFilter;
Expand Down Expand Up @@ -55,14 +62,23 @@ void flush() {
if (queue.isEmpty()) {
return;
}
int size = queue.size();
int itemCount = 0;
long start = System.nanoTime();
SourceFileItem item;
while ((item = queue.poll()) != null) {
queueSize.decrementAndGet();
registerSourceFile(item.className, item.classfileBuffer);
itemCount++;
}
LOGGER.debug(
"flushing {} source file items in {}ms", size, (System.nanoTime() - start) / 1_000_000);
"flushing {} source file items in {}ms, totalentries: {}",
itemCount,
(System.nanoTime() - start) / 1_000_000,
finder.getClassNamesBySourceFile().size());
}

int getQueueSize() {
return queueSize.get();
}

@Override
Expand All @@ -76,7 +92,12 @@ public byte[] transform(
if (className == null) {
return null;
}
if (queueSize.get() >= MAX_QUEUE_SIZE) {
ratelimitedLogger.warn("SourceFile Tracking queue full, dropping class: {}", className);
return null;
}
queue.add(new SourceFileItem(className, classfileBuffer));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably no real concerns of a race condition here given where this lives but thought I'd call it out.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah multiple class loads can happen concurrently here, we can overshoot the limit but this is not a problem. What we want here is putting a limitation of the total item that are queued to not fill up the memory. 4096 is arbitrary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's what I thought but wanted to double check.

queueSize.incrementAndGet();
return null;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.datadog.debugger.agent;

import static com.datadog.debugger.agent.SourceFileTrackingTransformer.MAX_QUEUE_SIZE;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static utils.TestClassFileHelper.getClassFileBytes;

Expand Down Expand Up @@ -138,6 +139,22 @@ void thirdPartyExcludes() throws IllegalClassFormatException {
assertEquals(0, finder.getClassNamesBySourceFile().size());
}

@Test
void maxQueue() throws IllegalClassFormatException {
ClassesToRetransformFinder finder = new ClassesToRetransformFinder();
SourceFileTrackingTransformer sourceFileTrackingTransformer =
new SourceFileTrackingTransformer(finder);
for (int i = 0; i < MAX_QUEUE_SIZE + 10; i++) {
sourceFileTrackingTransformer.transform(
null,
getInternalName(TopLevelHelper.class),
null,
null,
getClassFileBytes(TopLevelHelper.class));
}
assertEquals(MAX_QUEUE_SIZE, sourceFileTrackingTransformer.getQueueSize());
}

private static void replaceInByteArray(byte[] buffer, byte[] oldBytes, byte[] newBytes) {
int oldIdx = 0;
for (int i = 0; i < buffer.length; i++) {
Expand Down
Loading