Make agent index generation stable irrespective of filesystem iteration order (#11889)

refactor(agent): Make index generation immune to filesystem unordered iteration

`AgentJarIndex.IndexGenerator` used `Files.walkFileTree` traversal order
as stable input. But directory entry order is explicitly unspecified,
and in CI, the folder creation in `included/` depends on when the
parallel Gradle task are finished.

This causes a different index file to be generated, and invalidates the
input of the `shadowJar`. Note the index remains correct, however this
behavior changes the prefix IDs in the index file.

This happens because `Files.walkFileTree` rely under the hood on
`UnixFileSystemProvider.newDirectoryStream` which uses `readdir(3)` C
api which asks the file system via syscall, in particular we can read:

> The order in which filenames are read by successive calls to
> readdir() depends on the filesystem implementation; it is unlikely
> that the names will be sorted in any fashion.

https://www.man7.org/linux/man-pages/man3/readdir.3.html
https://man7.org/linux/man-pages/man2/getdents.2.html

The tool `find` has the same behavior.
https://sources.debian.org/src/findutils/4.10.0-3/gl/lib/fts.c/#L1444

test(agent): Replace awkward stream and for loop, with a pure stream variant

refactor(agent): Remove normalization for windows

Originally, normalization was introduced so that the index would be same
across OS, but it's not really improtant for windows.

refactor(agent): Remove unused `Comparator` import

Merge branch 'master' into bdu/non-determistic-index-file-walking

Co-authored-by: brice.dutheil <brice.dutheil@datadoghq.com>

Author: bric3

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/AgentJarIndex.java

@@ -7,18 +7,17 @@
 import java.io.DataOutputStream;
 import java.io.File;
 import java.io.IOException;
-import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
+import java.nio.file.LinkOption;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.nio.file.SimpleFileVisitor;
-import java.nio.file.attribute.BasicFileAttributes;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.jar.JarFile;
+import java.util.stream.Stream;
 import java.util.zip.ZipEntry;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -90,7 +89,7 @@ public static AgentJarIndex readIndex(JarFile agentJar) {
    * Generates an index from the contents of the 'build/resources' directory that makes up the agent
    * jar.
    */
-  static class IndexGenerator extends SimpleFileVisitor<Path> {
+  static class IndexGenerator {
     private static final Set<String> ignoredFileNames =
         new HashSet<>(Arrays.asList("MANIFEST.MF", "NOTICE", "LICENSE.renamed"));
 
@@ -102,17 +101,31 @@ static class IndexGenerator extends SimpleFileVisitor<Path> {
     private final List<String> collectedEntryKeys = new ArrayList<>();
     private final List<Integer> collectedPrefixIds = new ArrayList<>();
 
-    private Path prefixRoot;
-    private int prefixId;
-
     IndexGenerator(Path resourcesDir) {
       this.resourcesDir = resourcesDir;
 
       prefixTrie.put("datadog.*", 0);
     }
 
     void buildIndex() throws IOException {
-      Files.walkFileTree(resourcesDir, this);
+      Set<String> seen = new HashSet<>();
+      try (Stream<Path> paths = Files.walk(resourcesDir)) {
+        paths
+            .filter(path -> Files.isRegularFile(path, LinkOption.NOFOLLOW_LINKS))
+            .map(resourcesDir::relativize)
+            .sorted()
+            .filter(entry -> entry.getNameCount() >= 2)
+            .forEach(
+                entry -> {
+                  String prefix = entry.getName(0) + "/";
+                  int prefixId = prefixIdFor(prefix);
+                  String entryKey = computeEntryKey(entry.subpath(1, entry.getNameCount()));
+                  if (null != entryKey && seen.add(prefixId + "\0" + entryKey)) {
+                    collectedEntryKeys.add(entryKey);
+                    collectedPrefixIds.add(prefixId);
+                  }
+                });
+      }
 
       for (int i = 0; i < collectedEntryKeys.size(); i++) {
         prefixTrie.put(collectedEntryKeys.get(i), collectedPrefixIds.get(i));
@@ -152,42 +165,13 @@ private String getPrefix(int prefixId) {
       return prefixes.get(prefixId - 1);
     }
 
-    @Override
-    public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) {
-      if (dir.getParent().equals(resourcesDir)) {
-        String prefix = dir.getFileName() + "/";
-        prefixRoot = dir;
-        prefixId = 1 + prefixes.indexOf(prefix);
-        if (prefixId < 1) {
-          prefixes.add(prefix);
-          prefixId = prefixes.size();
-        }
-      }
-      return FileVisitResult.CONTINUE;
-    }
-
-    @Override
-    public FileVisitResult postVisitDirectory(Path dir, IOException exc) {
-      if (dir.equals(prefixRoot)) {
-        prefixRoot = null;
-      }
-      return FileVisitResult.CONTINUE;
-    }
-
-    @Override
-    public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
-      if (null != prefixRoot) {
-        String entryKey = computeEntryKey(prefixRoot.relativize(file));
-        if (null != entryKey) {
-          collectedEntryKeys.add(entryKey);
-          collectedPrefixIds.add(prefixId);
-          if (entryKey.endsWith("*")) {
-            // optimization: wildcard will match everything under here so can skip
-            return FileVisitResult.SKIP_SIBLINGS;
-          }
-        }
+    private int prefixIdFor(String prefix) {
+      int prefixId = 1 + prefixes.indexOf(prefix);
+      if (prefixId < 1) {
+        prefixes.add(prefix);
+        prefixId = prefixes.size();
       }
-      return FileVisitResult.CONTINUE;
+      return prefixId;
     }
 
     static String computeEntryKey(Path path) {

dd-java-agent/agent-bootstrap/src/test/java/datadog/trace/bootstrap/AgentJarIndexTest.java

@@ -1,13 +1,18 @@
 package datadog.trace.bootstrap;
 
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 
+import java.io.BufferedInputStream;
+import java.io.DataInputStream;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.List;
 import java.util.jar.JarFile;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipOutputStream;
@@ -103,6 +108,25 @@ private static void createFile(Path root, String relativePath) throws IOExceptio
     Files.createFile(file);
   }
 
+  private static Path writeIndex(Path resourcesDir, Path indexFile) throws IOException {
+    AgentJarIndex.IndexGenerator generator = new AgentJarIndex.IndexGenerator(resourcesDir);
+    generator.buildIndex();
+    generator.writeIndex(indexFile);
+    return indexFile;
+  }
+
+  private static List<String> readPrefixes(Path indexFile) throws IOException {
+    try (DataInputStream in =
+        new DataInputStream(new BufferedInputStream(Files.newInputStream(indexFile)))) {
+      int prefixCount = in.readInt();
+      String[] prefixes = new String[prefixCount];
+      for (int i = 0; i < prefixCount; i++) {
+        prefixes[i] = in.readUTF();
+      }
+      return Arrays.asList(prefixes);
+    }
+  }
+
   @Test
   void classEntryNameResolvesBootstrapClassToTopLevel(@TempDir Path tempDir) throws Exception {
     Path resources = tempDir.resolve("resources");
@@ -199,6 +223,52 @@ void multiplePrefixesAreIndexedIndependently(@TempDir Path tempDir) throws Excep
         index.classEntryName("com.datadoghq.stats.StatsClient"));
   }
 
+  @Test
+  void buildIndexWritesPrefixesInSortedOrder(@TempDir Path tempDir) throws Exception {
+    Path resources = tempDir.resolve("resources");
+    createFile(resources, "metrics/com/datadoghq/stats/StatsClient.classdata");
+    createFile(resources, "inst/datadog/trace/instrumentation/servlet/ServletAdvice.classdata");
+    createFile(resources, "appsec/com/datadog/appsec/Event.classdata");
+
+    Path indexFile = writeIndex(resources, tempDir.resolve("dd-java-agent.index"));
+
+    assertEquals(Arrays.asList("appsec/", "inst/", "metrics/"), readPrefixes(indexFile));
+  }
+
+  @Test
+  void buildIndexIgnoresTopLevelFilesWhenCollectingPrefixes(@TempDir Path tempDir)
+      throws Exception {
+    Path resources = tempDir.resolve("resources");
+    createFile(resources, "root-resource.properties");
+    createFile(resources, "inst/datadog/trace/instrumentation/servlet/ServletAdvice.classdata");
+
+    Path indexFile = writeIndex(resources, tempDir.resolve("dd-java-agent.index"));
+
+    assertEquals(Arrays.asList("inst/"), readPrefixes(indexFile));
+  }
+
+  @Test
+  void buildIndexIsIndependentOfDirectoryCreationOrder(@TempDir Path tempDir) throws Exception {
+    Path buildResources = tempDir.resolve("build-resources");
+    createFile(buildResources, "appsec/com/datadog/appsec/Event.classdata");
+    createFile(buildResources, "ci-visibility/com/datadog/ci/Visibility.classdata");
+    createFile(buildResources, "cws-tls/com/datadog/cws/Tls.classdata");
+    createFile(
+        buildResources, "inst/datadog/trace/instrumentation/servlet/ServletAdvice.classdata");
+
+    Path deployResources = tempDir.resolve("deploy-resources");
+    createFile(deployResources, "cws-tls/com/datadog/cws/Tls.classdata");
+    createFile(deployResources, "ci-visibility/com/datadog/ci/Visibility.classdata");
+    createFile(deployResources, "appsec/com/datadog/appsec/Event.classdata");
+    createFile(
+        deployResources, "inst/datadog/trace/instrumentation/servlet/ServletAdvice.classdata");
+
+    Path buildIndex = writeIndex(buildResources, tempDir.resolve("build-dd-java-agent.index"));
+    Path deployIndex = writeIndex(deployResources, tempDir.resolve("deploy-dd-java-agent.index"));
+
+    assertArrayEquals(Files.readAllBytes(buildIndex), Files.readAllBytes(deployIndex));
+  }
+
   @Test
   void buildIndexWithEmptyResourcesDirProducesWorkingIndex(@TempDir Path tempDir) throws Exception {
     Path resources = tempDir.resolve("resources");