From 144ab64a973e82195057ba35c063cae7368c5983 Mon Sep 17 00:00:00 2001 From: jean-philippe bempel Date: Mon, 10 Feb 2025 15:38:30 +0100 Subject: [PATCH 1/2] Add support for scanning jar from loaded class when a class is loaded we are now locating the jar containing the class and push the jar to a queue to be scanned by a background thread like for the initial process of SymDB enablement Add more information into SymDB report of total class count processed total jars scanned and histogram of class count by scanned jar --- .../datadog/debugger/agent/DebuggerAgent.java | 13 +- .../debugger/symbol/BasicSymDBReport.java | 56 ++++++ .../datadog/debugger/symbol/JarScanner.java | 4 +- .../debugger/symbol/SymDBEnablement.java | 96 +--------- .../datadog/debugger/symbol/SymDBReport.java | 73 ++++---- .../debugger/symbol/SymbolAggregator.java | 166 +++++++++++++++++- .../debugger/symbol/JarScannerTest.java | 6 +- .../debugger/symbol/SymDBEnablementTest.java | 47 +++-- .../debugger/symbol/SymbolAggregatorTest.java | 42 +++++ .../SymbolExtractionTransformerTest.java | 9 +- 10 files changed, 346 insertions(+), 166 deletions(-) create mode 100644 dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/BasicSymDBReport.java create mode 100644 dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/symbol/SymbolAggregatorTest.java diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java index ee61fa1c331..4b8cef476ca 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java @@ -147,13 +147,14 @@ private static void startDynamicInstrumentation( configurationPoller = sco.configurationPoller(config); if (configurationPoller != null) { if (config.isSymbolDatabaseEnabled()) { + SymbolAggregator symbolAggregator = + new SymbolAggregator( + classNameFilter, + debuggerSink.getSymbolSink(), + config.getSymbolDatabaseFlushThreshold()); + symbolAggregator.start(); symDBEnablement = - new SymDBEnablement( - instrumentation, - config, - new SymbolAggregator( - debuggerSink.getSymbolSink(), config.getSymbolDatabaseFlushThreshold()), - classNameFilter); + new SymDBEnablement(instrumentation, config, symbolAggregator, classNameFilter); if (config.isSymbolDatabaseForceUpload()) { symDBEnablement.startSymbolExtraction(); } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/BasicSymDBReport.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/BasicSymDBReport.java new file mode 100644 index 00000000000..3bb871a9e6c --- /dev/null +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/BasicSymDBReport.java @@ -0,0 +1,56 @@ +package com.datadog.debugger.symbol; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class BasicSymDBReport implements SymDBReport { + private static final Logger LOGGER = LoggerFactory.getLogger(BasicSymDBReport.class); + + private final Set missingJars = new HashSet<>(); + private final Map ioExceptions = new HashMap<>(); + private final List locationErrors = new ArrayList<>(); + private final Map classCountByJar = new HashMap<>(); + private final List scannedJars = new ArrayList<>(); + + public void addMissingJar(String jarPath) { + missingJars.add(jarPath); + } + + public void addIOException(String jarPath, IOException e) { + ioExceptions.put(jarPath, e.toString()); + } + + public void addLocationError(String locationStr) { + locationErrors.add(locationStr); + } + + public void incClassCount(String jarPath) { + classCountByJar.compute(jarPath, (k, v) -> v == null ? 1 : v + 1); + } + + public void addScannedJar(String jarPath) { + scannedJars.add(jarPath); + } + + public void report() { + int totalClasses = classCountByJar.values().stream().mapToInt(Integer::intValue).sum(); + String content = + String.format( + "SymDB Report: Scanned jar count=%d, Total class count=%d, class count by jar: %s, Scanned jars: %s, Location errors: %s Missing jars: %s IOExceptions: %s", + scannedJars.size(), + totalClasses, + classCountByJar, + scannedJars, + locationErrors, + missingJars, + ioExceptions); + LOGGER.info(content); + } +} diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/JarScanner.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/JarScanner.java index 2f106413931..bbddbd37a4e 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/JarScanner.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/JarScanner.java @@ -51,9 +51,7 @@ public static Path extractJarPath(ProtectionDomain protectionDomain, SymDBReport } else if (locationStr.startsWith(FILE_PREFIX)) { return getPathFromPrefixedFileName(locationStr, FILE_PREFIX, locationStr.length()); } - if (symDBReport != null) { - symDBReport.addLocationError(locationStr); - } + symDBReport.addLocationError(locationStr); return null; } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/SymDBEnablement.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/SymDBEnablement.java index 8c44a14b371..12b35740d05 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/SymDBEnablement.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/SymDBEnablement.java @@ -1,7 +1,5 @@ package com.datadog.debugger.symbol; -import static com.datadog.debugger.symbol.JarScanner.trimPrefixes; - import com.datadog.debugger.util.MoshiHelper; import com.squareup.moshi.JsonAdapter; import datadog.remoteconfig.PollingRateHinter; @@ -10,34 +8,24 @@ import datadog.trace.api.Config; import datadog.trace.bootstrap.debugger.DebuggerContext.ClassNameFilter; import datadog.trace.util.AgentTaskScheduler; -import datadog.trace.util.Strings; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; -import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.lang.instrument.Instrumentation; import java.net.URISyntaxException; import java.nio.file.Files; -import java.nio.file.LinkOption; import java.nio.file.Path; import java.time.Instant; import java.time.LocalDateTime; import java.time.ZoneId; import java.util.Arrays; -import java.util.HashSet; -import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.jar.JarEntry; -import java.util.jar.JarFile; -import java.util.regex.Pattern; import okio.Okio; import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class SymDBEnablement implements ProductListener { private static final Logger LOGGER = LoggerFactory.getLogger(SymDBEnablement.class); - private static final Pattern COMMA_PATTERN = Pattern.compile(","); private static final JsonAdapter SYM_DB_JSON_ADAPTER = MoshiHelper.createMoshiConfig().adapter(SymDbRemoteConfigRecord.class); private static final String SYM_DB_RC_KEY = "symDb"; @@ -120,7 +108,7 @@ public void startSymbolExtraction() { symbolExtractionTransformer = new SymbolExtractionTransformer(symbolAggregator, classNameFilter); instrumentation.addTransformer(symbolExtractionTransformer); - SymDBReport symDBReport = new SymDBReport(); + SymDBReport symDBReport = new BasicSymDBReport(); extractSymbolForLoadedClasses(symDBReport); symDBReport.report(); lastUploadTimestamp = System.currentTimeMillis(); @@ -145,7 +133,6 @@ private void extractSymbolForLoadedClasses(SymDBReport symDBReport) { LOGGER.debug("Failed to get all loaded classes", ex); return; } - Set alreadyScannedJars = new HashSet<>(); byte[] buffer = new byte[READ_BUFFER_SIZE]; ByteArrayOutputStream baos = new ByteArrayOutputStream(CLASSFILE_BUFFER_SIZE); for (Class clazz : classesToExtract) { @@ -162,86 +149,7 @@ private void extractSymbolForLoadedClasses(SymDBReport symDBReport) { symDBReport.addMissingJar(jarPath.toString()); continue; } - File jarPathFile = jarPath.toFile(); - if (jarPathFile.isDirectory()) { - scanDirectory(jarPath, alreadyScannedJars, baos, buffer, symDBReport); - alreadyScannedJars.add(jarPath.toString()); - continue; - } - if (alreadyScannedJars.contains(jarPath.toString())) { - continue; - } - try { - try (JarFile jarFile = new JarFile(jarPathFile)) { - jarFile.stream() - .filter(jarEntry -> jarEntry.getName().endsWith(".class")) - .filter( - jarEntry -> - !classNameFilter.isExcluded( - Strings.getClassName(trimPrefixes(jarEntry.getName())))) - .forEach(jarEntry -> parseJarEntry(jarEntry, jarFile, jarPath, baos, buffer)); - } - alreadyScannedJars.add(jarPath.toString()); - } catch (IOException e) { - symDBReport.addIOException(jarPath.toString(), e); - throw new RuntimeException(e); - } - } - } - - private void scanDirectory( - Path jarPath, - Set alreadyScannedJars, - ByteArrayOutputStream baos, - byte[] buffer, - SymDBReport symDBReport) { - try { - Files.walk(jarPath) - // explicitly no follow links walking the directory to avoid cycles - .filter(path -> Files.isRegularFile(path, LinkOption.NOFOLLOW_LINKS)) - .filter(path -> path.toString().endsWith(".class")) - .filter( - path -> - !classNameFilter.isExcluded( - Strings.getClassName(trimPrefixes(jarPath.relativize(path).toString())))) - .forEach(path -> parseFileEntry(path, jarPath, baos, buffer)); - alreadyScannedJars.add(jarPath.toString()); - } catch (IOException e) { - symDBReport.addIOException(jarPath.toString(), e); - throw new RuntimeException(e); - } - } - - private void parseFileEntry(Path path, Path jarPath, ByteArrayOutputStream baos, byte[] buffer) { - LOGGER.debug("parsing file class: {}", path.toString()); - try { - try (InputStream inputStream = Files.newInputStream(path)) { - int readBytes; - baos.reset(); - while ((readBytes = inputStream.read(buffer)) != -1) { - baos.write(buffer, 0, readBytes); - } - symbolAggregator.parseClass( - path.getFileName().toString(), baos.toByteArray(), jarPath.toString()); - } - } catch (IOException ex) { - LOGGER.debug("Exception during parsing file class: {}", path, ex); - } - } - - private void parseJarEntry( - JarEntry jarEntry, JarFile jarFile, Path jarPath, ByteArrayOutputStream baos, byte[] buffer) { - LOGGER.debug("parsing jarEntry class: {}", jarEntry.getName()); - try { - InputStream inputStream = jarFile.getInputStream(jarEntry); - int readBytes; - baos.reset(); - while ((readBytes = inputStream.read(buffer)) != -1) { - baos.write(buffer, 0, readBytes); - } - symbolAggregator.parseClass(jarEntry.getName(), baos.toByteArray(), jarPath.toString()); - } catch (IOException ex) { - LOGGER.debug("Exception during parsing jarEntry class: {}", jarEntry.getName(), ex); + symbolAggregator.scanJar(symDBReport, jarPath, baos, buffer); } } } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/SymDBReport.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/SymDBReport.java index af70957a8d9..d407acd5726 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/SymDBReport.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/SymDBReport.java @@ -1,42 +1,39 @@ package com.datadog.debugger.symbol; import java.io.IOException; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public class SymDBReport { - private static final Logger LOGGER = LoggerFactory.getLogger(SymDBReport.class); - - private final Set missingJars = new HashSet<>(); - private final Map ioExceptions = new HashMap<>(); - private final List locationErrors = new ArrayList<>(); - - public void addMissingJar(String jarPath) { - missingJars.add(jarPath); - } - - public void addIOException(String jarPath, IOException e) { - ioExceptions.put(jarPath, e.toString()); - } - - public void addLocationError(String locationStr) { - locationErrors.add(locationStr); - } - - public void report() { - String content = - "== SymDB Report == Location errors:" - + locationErrors - + " Missing jars: " - + missingJars - + " IOExceptions: " - + ioExceptions; - LOGGER.info(content); - } + +public interface SymDBReport { + + void addMissingJar(String jarPath); + + void addIOException(String jarPath, IOException e); + + void addLocationError(String locationStr); + + void incClassCount(String jarPath); + + void addScannedJar(String jarPath); + + void report(); + + SymDBReport NO_OP = + new SymDBReport() { + @Override + public void addMissingJar(String jarPath) {} + + @Override + public void addIOException(String jarPath, IOException e) {} + + @Override + public void addLocationError(String locationStr) {} + + @Override + public void incClassCount(String jarPath) {} + + @Override + public void addScannedJar(String jarPath) {} + + @Override + public void report() {} + }; } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/SymbolAggregator.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/SymbolAggregator.java index a7e7005520e..04705ed1ca6 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/SymbolAggregator.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/SymbolAggregator.java @@ -3,58 +3,105 @@ import static com.datadog.debugger.symbol.JarScanner.trimPrefixes; import com.datadog.debugger.sink.SymbolSink; +import datadog.trace.bootstrap.debugger.DebuggerContext; import datadog.trace.util.AgentTaskScheduler; import datadog.trace.util.ClassNameTrie; import datadog.trace.util.Strings; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.Path; +import java.nio.file.Paths; import java.security.ProtectionDomain; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Queue; +import java.util.Set; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class SymbolAggregator { private static final Logger LOGGER = LoggerFactory.getLogger(SymbolAggregator.class); private static final String CLASS_SUFFIX = ".class"; + private static final int READ_BUFFER_SIZE = 4096; + private static final int CLASSFILE_BUFFER_SIZE = 8192; + private final DebuggerContext.ClassNameFilter classNameFilter; private final SymbolSink sink; private final int symbolFlushThreshold; private final Map jarScopesByName = new HashMap<>(); - private final AgentTaskScheduler.Scheduled scheduled; private final Object jarScopeLock = new Object(); + private AgentTaskScheduler.Scheduled flushRemainingScopeScheduled; + private AgentTaskScheduler.Scheduled scanJarsScheduled; private int totalClasses; // ClassNameTrie is not thread safe, All accesses must be protected by a lock private final ClassNameTrie.Builder loadedClasses = new ClassNameTrie.Builder(); + private final Queue jarsToScanQueue = new ArrayBlockingQueue<>(64); + private final Set alreadyScannedJars = ConcurrentHashMap.newKeySet(); - public SymbolAggregator(SymbolSink sink, int symbolFlushThreshold) { + public SymbolAggregator( + DebuggerContext.ClassNameFilter classNameFilter, SymbolSink sink, int symbolFlushThreshold) { + this.classNameFilter = classNameFilter; this.sink = sink; this.symbolFlushThreshold = symbolFlushThreshold; - scheduled = + } + + public void start() { + flushRemainingScopeScheduled = AgentTaskScheduler.INSTANCE.scheduleAtFixedRate( this::flushRemainingScopes, this, 0, 1, TimeUnit.SECONDS); + scanJarsScheduled = + AgentTaskScheduler.INSTANCE.scheduleAtFixedRate( + this::scanQueuedJars, this, 0, 1, TimeUnit.SECONDS); + } + + public void stop() { + cancelSchedule(flushRemainingScopeScheduled); + cancelSchedule(scanJarsScheduled); + } + + private void cancelSchedule(AgentTaskScheduler.Scheduled scheduled) { + if (scheduled != null) { + scheduled.cancel(); + } } public void parseClass( String className, byte[] classfileBuffer, ProtectionDomain protectionDomain) { try { String jarName = "DEFAULT"; - Path jarPath = JarScanner.extractJarPath(protectionDomain, null); + Path jarPath = JarScanner.extractJarPath(protectionDomain, SymDBReport.NO_OP); if (jarPath != null && Files.exists(jarPath)) { LOGGER.debug("jarpath: {}", jarPath); jarName = jarPath.toString(); + if (!alreadyScannedJars.contains(jarName)) { // filter out already scanned jars + if (!jarsToScanQueue.contains(jarName)) { // filter out already queued jars + LOGGER.debug("Queuing jar to scan: {}", jarPath); + if (!jarsToScanQueue.offer(jarName)) { + LOGGER.debug("jarToScan queue is full, skipping jar: {}", jarName); + } + } + } } - parseClass(className, classfileBuffer, jarName); + parseClass(SymDBReport.NO_OP, className, classfileBuffer, jarName); } catch (Exception ex) { LOGGER.debug("Error parsing class: {}", className, ex); } } - public void parseClass(String className, byte[] classfileBuffer, String jarName) { + public void parseClass( + SymDBReport symDBReport, String className, byte[] classfileBuffer, String jarName) { if (className == null) { return; } @@ -73,6 +120,7 @@ public void parseClass(String className, byte[] classfileBuffer, String jarName) LOGGER.debug("Extracting Symbols from: {}, located in: {}", className, jarName); Scope jarScope = SymbolExtractor.extract(classfileBuffer, jarName); addJarScope(jarScope, false); + symDBReport.incClassCount(jarName); } private void flushRemainingScopes(SymbolAggregator symbolAggregator) { @@ -85,6 +133,19 @@ private void flushRemainingScopes(SymbolAggregator symbolAggregator) { } } + void scanQueuedJars(SymbolAggregator symbolAggregator) { + if (jarsToScanQueue.isEmpty()) { + return; + } + byte[] buffer = new byte[READ_BUFFER_SIZE]; + ByteArrayOutputStream baos = new ByteArrayOutputStream(CLASSFILE_BUFFER_SIZE); + while (!jarsToScanQueue.isEmpty()) { + String jarPath = jarsToScanQueue.poll(); + LOGGER.debug("Scanning queued jar: {}", jarPath); + scanJar(SymDBReport.NO_OP, Paths.get(jarPath), baos, buffer); + } + } + private void addJarScope(Scope jarScope, boolean forceFlush) { List scopes = Collections.emptyList(); synchronized (jarScopeLock) { @@ -114,4 +175,97 @@ private void addJarScope(Scope jarScope, boolean forceFlush) { } } } + + public void scanJar( + SymDBReport symDBReport, Path jarPath, ByteArrayOutputStream baos, byte[] buffer) { + if (alreadyScannedJars.contains(jarPath.toString())) { + return; + } + File jarPathFile = jarPath.toFile(); + if (jarPathFile.isDirectory()) { + scanDirectory(jarPath, alreadyScannedJars, baos, buffer, symDBReport); + } else { + try { + try (JarFile jarFile = new JarFile(jarPathFile)) { + jarFile.stream() + .filter(jarEntry -> jarEntry.getName().endsWith(".class")) + .filter( + jarEntry -> + !classNameFilter.isExcluded( + Strings.getClassName(trimPrefixes(jarEntry.getName())))) + .forEach( + jarEntry -> parseJarEntry(symDBReport, jarEntry, jarFile, jarPath, baos, buffer)); + } + } catch (IOException e) { + symDBReport.addIOException(jarPath.toString(), e); + throw new RuntimeException(e); + } + } + symDBReport.addScannedJar(jarPath.toString()); + alreadyScannedJars.add(jarPath.toString()); + } + + private void scanDirectory( + Path jarPath, + Set alreadyScannedJars, + ByteArrayOutputStream baos, + byte[] buffer, + SymDBReport symDBReport) { + try { + Files.walk(jarPath) + // explicitly no follow links walking the directory to avoid cycles + .filter(path -> Files.isRegularFile(path, LinkOption.NOFOLLOW_LINKS)) + .filter(path -> path.toString().endsWith(".class")) + .filter( + path -> + !classNameFilter.isExcluded( + Strings.getClassName(trimPrefixes(jarPath.relativize(path).toString())))) + .forEach(path -> parseFileEntry(symDBReport, path, jarPath, baos, buffer)); + alreadyScannedJars.add(jarPath.toString()); + } catch (IOException e) { + symDBReport.addIOException(jarPath.toString(), e); + throw new RuntimeException(e); + } + } + + private void parseFileEntry( + SymDBReport symDBReport, Path path, Path jarPath, ByteArrayOutputStream baos, byte[] buffer) { + LOGGER.debug("parsing file class: {}", path.toString()); + try { + try (InputStream inputStream = Files.newInputStream(path)) { + int readBytes; + baos.reset(); + while ((readBytes = inputStream.read(buffer)) != -1) { + baos.write(buffer, 0, readBytes); + } + parseClass( + symDBReport, path.getFileName().toString(), baos.toByteArray(), jarPath.toString()); + } + } catch (IOException ex) { + symDBReport.addIOException(jarPath.toString(), ex); + LOGGER.debug("Exception during parsing file class: {}", path, ex); + } + } + + private void parseJarEntry( + SymDBReport symDBReport, + JarEntry jarEntry, + JarFile jarFile, + Path jarPath, + ByteArrayOutputStream baos, + byte[] buffer) { + LOGGER.debug("parsing jarEntry class: {}", jarEntry.getName()); + try { + InputStream inputStream = jarFile.getInputStream(jarEntry); + int readBytes; + baos.reset(); + while ((readBytes = inputStream.read(buffer)) != -1) { + baos.write(buffer, 0, readBytes); + } + parseClass(symDBReport, jarEntry.getName(), baos.toByteArray(), jarPath.toString()); + } catch (IOException ex) { + symDBReport.addIOException(jarPath.toString(), ex); + LOGGER.debug("Exception during parsing jarEntry class: {}", jarEntry.getName(), ex); + } + } } diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/symbol/JarScannerTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/symbol/JarScannerTest.java index 54f84f99499..e9c5b8d90fe 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/symbol/JarScannerTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/symbol/JarScannerTest.java @@ -22,7 +22,8 @@ public void extractJarPathFromJar() URL jarUrl = new URL("jar:file:" + jarFileUrl.getFile() + "!/"); URLClassLoader urlClassLoader = new URLClassLoader(new URL[] {jarUrl}, null); Class testClass = urlClassLoader.loadClass(CLASS_NAME); - assertEquals(jarFileUrl.getFile(), JarScanner.extractJarPath(testClass, null).toString()); + assertEquals( + jarFileUrl.getFile(), JarScanner.extractJarPath(testClass, SymDBReport.NO_OP).toString()); assertEquals( jarFileUrl.getFile(), JarScanner.extractJarPath(testClass.getProtectionDomain(), null).toString()); @@ -34,7 +35,8 @@ public void extractJarPathFromFile() throws ClassNotFoundException, URISyntaxExc URL jarFileUrl = getClass().getResource("/debugger-symbol.jar"); URLClassLoader urlClassLoader = new URLClassLoader(new URL[] {jarFileUrl}, null); Class testClass = urlClassLoader.loadClass(CLASS_NAME); - assertEquals(jarFileUrl.getFile(), JarScanner.extractJarPath(testClass, null).toString()); + assertEquals( + jarFileUrl.getFile(), JarScanner.extractJarPath(testClass, SymDBReport.NO_OP).toString()); } @Test diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/symbol/SymDBEnablementTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/symbol/SymDBEnablementTest.java index 920de7b85df..c5fefef7534 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/symbol/SymDBEnablementTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/symbol/SymDBEnablementTest.java @@ -9,6 +9,7 @@ import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -33,6 +34,7 @@ import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledIf; import org.mockito.ArgumentCaptor; class SymDBEnablementTest { @@ -56,9 +58,13 @@ public void before() { @Test public void enableDisableSymDBThroughRC() throws Exception { + ClassNameFiltering classNameFiltering = ClassNameFiltering.allowAll(); SymDBEnablement symDBEnablement = new SymDBEnablement( - instr, config, new SymbolAggregator(symbolSink, 1), ClassNameFiltering.allowAll()); + instr, + config, + new SymbolAggregator(classNameFiltering, symbolSink, 1), + classNameFiltering); symDBEnablement.accept(ParsedConfigKey.parse(CONFIG_KEY), UPlOAD_SYMBOL_TRUE, null); waitForUpload(symDBEnablement); verify(instr).addTransformer(any(SymbolExtractionTransformer.class)); @@ -68,9 +74,13 @@ public void enableDisableSymDBThroughRC() throws Exception { @Test public void removeSymDBConfig() throws Exception { + ClassNameFiltering classNameFiltering = ClassNameFiltering.allowAll(); SymDBEnablement symDBEnablement = new SymDBEnablement( - instr, config, new SymbolAggregator(symbolSink, 1), ClassNameFiltering.allowAll()); + instr, + config, + new SymbolAggregator(classNameFiltering, symbolSink, 1), + classNameFiltering); symDBEnablement.accept(ParsedConfigKey.parse(CONFIG_KEY), UPlOAD_SYMBOL_TRUE, null); waitForUpload(symDBEnablement); symDBEnablement.remove(ParsedConfigKey.parse(CONFIG_KEY), null); @@ -81,21 +91,25 @@ public void removeSymDBConfig() throws Exception { public void noIncludesFilterOutDatadogClass() { when(config.getThirdPartyExcludes()).thenReturn(Collections.emptySet()); when(config.getThirdPartyIncludes()).thenReturn(Collections.singleton("com.datadog.debugger.")); + ClassNameFiltering classNameFiltering = new ClassNameFiltering(config); SymDBEnablement symDBEnablement = new SymDBEnablement( - instr, config, new SymbolAggregator(symbolSink, 1), new ClassNameFiltering(config)); + instr, + config, + new SymbolAggregator(classNameFiltering, symbolSink, 1), + classNameFiltering); symDBEnablement.startSymbolExtraction(); ArgumentCaptor captor = ArgumentCaptor.forClass(SymbolExtractionTransformer.class); verify(instr).addTransformer(captor.capture()); SymbolExtractionTransformer transformer = captor.getValue(); - ClassNameFilter classNameFiltering = transformer.getClassNameFiltering(); - assertTrue(classNameFiltering.isExcluded("com.datadog.debugger.test.TestClass")); - assertFalse(classNameFiltering.isExcluded("org.foobar.test.TestClass")); + ClassNameFilter classNameFilter = transformer.getClassNameFiltering(); + assertTrue(classNameFilter.isExcluded("com.datadog.debugger.test.TestClass")); + assertFalse(classNameFilter.isExcluded("org.foobar.test.TestClass")); } @Test - public void parseLoadedClass() throws ClassNotFoundException, IOException, URISyntaxException { + public void parseLoadedClass() throws ClassNotFoundException, IOException { final String CLASS_NAME = "com.datadog.debugger.symbol.SymbolExtraction01"; URL jarFileUrl = getClass().getResource("/debugger-symbol.jar"); URL jarUrl = new URL("jar:file:" + jarFileUrl.getFile() + "!/"); @@ -106,14 +120,16 @@ public void parseLoadedClass() throws ClassNotFoundException, IOException, URISy .thenReturn( Stream.of("com.datadog.debugger.", "org.springframework.samples.") .collect(Collectors.toSet())); - SymbolAggregator symbolAggregator = mock(SymbolAggregator.class); + ClassNameFiltering classNameFiltering = ClassNameFiltering.allowAll(); + SymbolAggregator symbolAggregator = + spy(new SymbolAggregator(classNameFiltering, symbolSink, 1)); SymDBEnablement symDBEnablement = - new SymDBEnablement(instr, config, symbolAggregator, ClassNameFiltering.allowAll()); + new SymDBEnablement(instr, config, symbolAggregator, classNameFiltering); symDBEnablement.startSymbolExtraction(); verify(instr).addTransformer(any(SymbolExtractionTransformer.class)); ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); verify(symbolAggregator, times(2)) - .parseClass(captor.capture(), any(), eq(jarFileUrl.getFile())); + .parseClass(any(), captor.capture(), any(), eq(jarFileUrl.getFile())); assertEquals( "com/datadog/debugger/symbol/SymbolExtraction01.class", captor.getAllValues().get(0)); assertEquals( @@ -132,26 +148,29 @@ public void parseLoadedClassFromDirectory() .thenReturn( Stream.of("com.datadog.debugger.", "org.springframework.samples.") .collect(Collectors.toSet())); - SymbolAggregator symbolAggregator = mock(SymbolAggregator.class); + ClassNameFiltering classNameFiltering = ClassNameFiltering.allowAll(); + SymbolAggregator symbolAggregator = + spy(new SymbolAggregator(classNameFiltering, symbolSink, 1)); SymDBEnablement symDBEnablement = - new SymDBEnablement(instr, config, symbolAggregator, ClassNameFiltering.allowAll()); + new SymDBEnablement(instr, config, symbolAggregator, classNameFiltering); symDBEnablement.startSymbolExtraction(); verify(instr).addTransformer(any(SymbolExtractionTransformer.class)); ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); - verify(symbolAggregator, atLeastOnce()).parseClass(captor.capture(), any(), anyString()); + verify(symbolAggregator, atLeastOnce()).parseClass(any(), captor.capture(), any(), anyString()); // verify that we called parseClass on this test class assertTrue(captor.getAllValues().contains(getClass().getSimpleName() + ".class")); } @Test + @DisabledIf(value = "datadog.trace.api.Platform#isJ9", disabledReason = "Flaky on J9 JVMs") public void noDuplicateSymbolExtraction() { final String CLASS_NAME_PATH = "com/datadog/debugger/symbol/SymbolExtraction01"; SymbolSink mockSymbolSink = mock(SymbolSink.class); - SymbolAggregator symbolAggregator = new SymbolAggregator(mockSymbolSink, 1); ClassNameFiltering classNameFiltering = new ClassNameFiltering( Collections.singleton("org.springframework."), Collections.singleton("com.datadog.debugger.")); + SymbolAggregator symbolAggregator = new SymbolAggregator(classNameFiltering, mockSymbolSink, 1); SymDBEnablement symDBEnablement = new SymDBEnablement(instr, config, symbolAggregator, classNameFiltering); doAnswer( diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/symbol/SymbolAggregatorTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/symbol/SymbolAggregatorTest.java new file mode 100644 index 00000000000..ffa270c6e0b --- /dev/null +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/symbol/SymbolAggregatorTest.java @@ -0,0 +1,42 @@ +package com.datadog.debugger.symbol; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; + +import com.datadog.debugger.sink.SymbolSink; +import com.datadog.debugger.util.ClassNameFiltering; +import java.net.URL; +import java.security.CodeSource; +import java.security.ProtectionDomain; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +class SymbolAggregatorTest { + + @Test + void testScanQueuedJars() { + SymbolSink symbolSink = mock(SymbolSink.class); + SymbolAggregator symbolAggregator = + spy(new SymbolAggregator(ClassNameFiltering.allowAll(), symbolSink, 1)); + URL jarFileUrl = getClass().getResource("/debugger-symbol.jar"); + CodeSource codeSource = new CodeSource(jarFileUrl, (java.security.cert.Certificate[]) null); + ProtectionDomain protectionDomain = new ProtectionDomain(codeSource, null); + symbolAggregator.parseClass(null, null, protectionDomain); + symbolAggregator.scanQueuedJars(null); + ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); + verify(symbolAggregator, atLeastOnce()) + .parseClass(any(), captor.capture(), any(), eq(jarFileUrl.getFile())); + // captor.getAllValues().get(0) is the first argument of the first invocation of parseClass with + // null + assertEquals( + "com/datadog/debugger/symbol/SymbolExtraction01.class", captor.getAllValues().get(1)); + assertEquals( + "BOOT-INF/classes/org/springframework/samples/petclinic/vet/VetController.class", + captor.getAllValues().get(2)); + } +} diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/symbol/SymbolExtractionTransformerTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/symbol/SymbolExtractionTransformerTest.java index 79bf6b2cbbb..bffb1748292 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/symbol/SymbolExtractionTransformerTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/symbol/SymbolExtractionTransformerTest.java @@ -884,10 +884,11 @@ public void filterOutClassesFromExcludedPackages() throws IOException, URISyntax when(config.getFinalDebuggerSymDBUrl()).thenReturn("http://localhost:8126/symdb/v1/input"); final String CLASS_NAME = EXCLUDED_PACKAGE + "SymbolExtraction16"; SymbolSinkMock symbolSinkMock = new SymbolSinkMock(config); + ClassNameFiltering classNameFiltering = + new ClassNameFiltering(Collections.singleton(EXCLUDED_PACKAGE)); SymbolExtractionTransformer transformer = new SymbolExtractionTransformer( - new SymbolAggregator(symbolSinkMock, 1), - new ClassNameFiltering(Collections.singleton(EXCLUDED_PACKAGE))); + new SymbolAggregator(classNameFiltering, symbolSinkMock, 1), classNameFiltering); instr.addTransformer(transformer); Class testClass = compileAndLoadClass(CLASS_NAME); Reflect.on(testClass).call("main", "1").get(); @@ -898,6 +899,7 @@ public void filterOutClassesFromExcludedPackages() throws IOException, URISyntax } @Test + @DisabledIf(value = "datadog.trace.api.Platform#isJ9", disabledReason = "Flaky on J9 JVMs") public void duplicateClassThroughDifferentClassLoader() throws IOException, URISyntaxException { final String CLASS_NAME = SYMBOL_PACKAGE + "SymbolExtraction01"; SymbolSinkMock symbolSinkMock = new SymbolSinkMock(config); @@ -986,7 +988,8 @@ private SymbolExtractionTransformer createTransformer( private SymbolExtractionTransformer createTransformer( SymbolSink symbolSink, int symbolFlushThreshold, ClassNameFiltering classNameFiltering) { return new SymbolExtractionTransformer( - new SymbolAggregator(symbolSink, symbolFlushThreshold), classNameFiltering); + new SymbolAggregator(classNameFiltering, symbolSink, symbolFlushThreshold), + classNameFiltering); } static class SymbolSinkMock extends SymbolSink { From 62c40a3a4ce2c07529d860452b44772b6ebd69a6 Mon Sep 17 00:00:00 2001 From: jean-philippe bempel Date: Wed, 26 Feb 2025 13:46:19 +0100 Subject: [PATCH 2/2] log instead of re-throw bump jarsToScanQueue capacity --- .../java/com/datadog/debugger/symbol/SymbolAggregator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/SymbolAggregator.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/SymbolAggregator.java index 04705ed1ca6..ccae4e0916e 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/SymbolAggregator.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/SymbolAggregator.java @@ -47,7 +47,7 @@ public class SymbolAggregator { private int totalClasses; // ClassNameTrie is not thread safe, All accesses must be protected by a lock private final ClassNameTrie.Builder loadedClasses = new ClassNameTrie.Builder(); - private final Queue jarsToScanQueue = new ArrayBlockingQueue<>(64); + private final Queue jarsToScanQueue = new ArrayBlockingQueue<>(128); private final Set alreadyScannedJars = ConcurrentHashMap.newKeySet(); public SymbolAggregator( @@ -224,7 +224,7 @@ private void scanDirectory( alreadyScannedJars.add(jarPath.toString()); } catch (IOException e) { symDBReport.addIOException(jarPath.toString(), e); - throw new RuntimeException(e); + LOGGER.debug("Exception during scanning directory: {}", jarPath, e); } }