Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit c88f535

Browse files
Fix root coverage packages calculation in large repositories (DataDog#6317)
1 parent d1eda60 commit c88f535

File tree

7 files changed

+61
-29
lines changed

7 files changed

+61
-29
lines changed

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilitySystem.java

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -177,10 +177,17 @@ private static DDBuildSystemSession.Factory buildSystemSessionFactory(
177177
ResourceResolver resourceResolver =
178178
new ConventionBasedResourceResolver(
179179
fileSystem, config.getCiVisibilityResourceFolderNames());
180-
RepoIndexBuilder indexBuilder =
181-
new RepoIndexBuilder(repoRoot, packageResolver, resourceResolver, fileSystem);
182180

183-
SourcePathResolver sourcePathResolver = getSourcePathResolver(repoRoot, indexBuilder);
181+
// scanning only the project folder and not the entire repo to
182+
// save time and to properly determine root packages for coverage instrumentation
183+
// (when there are multiple projects in the repo, root package resolution cannot establish
184+
// common roots)
185+
String sourcePathResolutionRoot = projectRoot.toString();
186+
RepoIndexBuilder indexBuilder =
187+
new RepoIndexBuilder(
188+
config, sourcePathResolutionRoot, packageResolver, resourceResolver, fileSystem);
189+
SourcePathResolver sourcePathResolver =
190+
getSourcePathResolver(sourcePathResolutionRoot, indexBuilder);
184191
Codeowners codeowners = getCodeowners(repoRoot);
185192

186193
MethodLinesResolver methodLinesResolver =
@@ -307,7 +314,7 @@ private static DDTestFrameworkSession.Factory testFrameworkSessionFactory(
307314
new ConventionBasedResourceResolver(
308315
fileSystem, config.getCiVisibilityResourceFolderNames());
309316
RepoIndexProvider indexProvider =
310-
new RepoIndexBuilder(repoRoot, packageResolver, resourceResolver, fileSystem);
317+
new RepoIndexBuilder(config, repoRoot, packageResolver, resourceResolver, fileSystem);
311318

312319
BackendApi backendApi = backendApiFactory.createBackendApi();
313320
GitDataUploader gitDataUploader =
@@ -389,7 +396,7 @@ private static CIVisibility.SessionFactory apiSessionFactory(
389396
new ConventionBasedResourceResolver(
390397
fileSystem, config.getCiVisibilityResourceFolderNames());
391398
RepoIndexProvider indexProvider =
392-
new RepoIndexBuilder(repoRoot, packageResolver, resourceResolver, fileSystem);
399+
new RepoIndexBuilder(config, repoRoot, packageResolver, resourceResolver, fileSystem);
393400
SourcePathResolver sourcePathResolver = getSourcePathResolver(repoRoot, indexProvider);
394401

395402
return new DDTestSessionImpl(

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/PackageTree.java

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package datadog.trace.civisibility.source.index;
22

3+
import datadog.trace.api.Config;
34
import java.nio.file.Path;
45
import java.util.ArrayDeque;
56
import java.util.ArrayList;
@@ -18,10 +19,10 @@
1819
* packages a/b, a/b/c, a/b/d, all of which contain classes, only a/b will be retained - a/b/c and
1920
* a/b/d will be discarded as redundant).
2021
*
21-
* <p>The length of the list is limited by {@link #MAX_CHILDREN_LEAVES}. If there are more packages
22-
* present than the limit, coarsening will happen (for instance, if there are packages a/b/c, a/b/d,
23-
* b/c/d, b/c/e/f and the limit is 2, the packages will be coarsened and a/b, b/c will be retained
24-
* as the result).
22+
* <p>The limit on the length of the list is configurable. If there are more packages present than
23+
* the limit, coarsening will happen (for instance, if there are packages a/b/c, a/b/d, b/c/d,
24+
* b/c/e/f and the limit is 2, the packages will be coarsened and a/b, b/c will be retained as the
25+
* result).
2526
*
2627
* <p>The intent is to be as specific as possible without exceeding the limit, so coarsening applies
2728
* first to the "longest" package names (not in terms of the actual string length, but in terms of
@@ -30,10 +31,14 @@
3031
*/
3132
public class PackageTree {
3233

33-
private static final int MAX_CHILDREN_LEAVES = 25;
34-
3534
private final Node root = new Node(null, "");
3635

36+
private final int rootPackagesLimit;
37+
38+
public PackageTree(Config config) {
39+
rootPackagesLimit = config.getCiVisibilityCoverageRootPackagesLimit();
40+
}
41+
3742
void add(Path packagePath) {
3843
if (packagePath.toString().isEmpty()) {
3944
return;
@@ -44,14 +49,14 @@ void add(Path packagePath) {
4449
List<String> asList() {
4550
truncateIfNeeded(root);
4651

47-
List<String> childrenPackages = new ArrayList<>(MAX_CHILDREN_LEAVES);
52+
List<String> childrenPackages = new ArrayList<>(rootPackagesLimit);
4853
for (Node child : root.children.values()) {
4954
child.stringify(childrenPackages, "");
5055
}
5156
return childrenPackages;
5257
}
5358

54-
private staticvoid truncateIfNeeded(Node root) {
59+
private void truncateIfNeeded(Node root) {
5560
Deque<List<Node>> nodesByDepth = new ArrayDeque<>();
5661

5762
List<Node> current = Collections.singletonList(root);
@@ -73,7 +78,7 @@ private static void truncateIfNeeded(Node root) {
7378
nodes.sort(Comparator.comparingInt(node -> -node.leafChildren));
7479

7580
for (Node node : nodes) {
76-
if (root.leafChildren <= MAX_CHILDREN_LEAVES) {
81+
if (root.leafChildren <= rootPackagesLimit) {
7782
// stop as soon as we have truncated enough, even if it's mid-level
7883
return;
7984
} else {

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndexBuilder.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package datadog.trace.civisibility.source.index;
22

3+
import datadog.trace.api.Config;
34
import datadog.trace.util.ClassNameTrie;
45
import java.io.File;
56
import java.io.IOException;
@@ -20,6 +21,7 @@ public class RepoIndexBuilder implements RepoIndexProvider {
2021

2122
private static final Logger log = LoggerFactory.getLogger(RepoIndexBuilder.class);
2223

24+
private final Config config;
2325
private final String repoRoot;
2426
private final PackageResolver packageResolver;
2527
private final ResourceResolver resourceResolver;
@@ -29,10 +31,12 @@ public class RepoIndexBuilder implements RepoIndexProvider {
2931
private volatile RepoIndex index;
3032

3133
public RepoIndexBuilder(
34+
Config config,
3235
String repoRoot,
3336
PackageResolver packageResolver,
3437
ResourceResolver resourceResolver,
3538
FileSystem fileSystem) {
39+
this.config = config;
3640
this.repoRoot = repoRoot;
3741
this.packageResolver = packageResolver;
3842
this.resourceResolver = resourceResolver;
@@ -60,7 +64,7 @@ private RepoIndex doGetIndex() {
6064

6165
Path repoRootPath = fileSystem.getPath(repoRoot);
6266
RepoIndexingFileVisitor repoIndexingFileVisitor =
63-
new RepoIndexingFileVisitor(packageResolver, resourceResolver, repoRootPath);
67+
new RepoIndexingFileVisitor(config, packageResolver, resourceResolver, repoRootPath);
6468

6569
long startTime = System.currentTimeMillis();
6670
try {
@@ -100,13 +104,16 @@ private static final class RepoIndexingFileVisitor implements FileVisitor<Path>
100104
private final Path repoRoot;
101105

102106
private RepoIndexingFileVisitor(
103-
PackageResolver packageResolver, ResourceResolver resourceResolver, Path repoRoot) {
107+
Config config,
108+
PackageResolver packageResolver,
109+
ResourceResolver resourceResolver,
110+
Path repoRoot) {
104111
this.packageResolver = packageResolver;
105112
this.resourceResolver = resourceResolver;
106113
this.repoRoot = repoRoot;
107114
trieBuilder = new ClassNameTrie.Builder();
108115
sourceRoots = new LinkedHashSet<>();
109-
packageTree = new PackageTree();
116+
packageTree = new PackageTree(config);
110117
indexingStats = new RepoIndexingStats();
111118
}
112119

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndexSourcePathResolver.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,14 @@ public RepoIndexSourcePathResolver(String repoRoot, RepoIndexProvider indexProvi
2020
}
2121

2222
RepoIndexSourcePathResolver(
23+
Config config,
2324
String repoRoot,
2425
PackageResolver packageResolver,
2526
ResourceResolver resourceResolver,
2627
FileSystem fileSystem) {
2728
this.repoRoot = repoRoot;
2829
this.indexProvider =
29-
new RepoIndexBuilder(repoRoot, packageResolver, resourceResolver, fileSystem);
30+
new RepoIndexBuilder(config, repoRoot, packageResolver, resourceResolver, fileSystem);
3031
}
3132

3233
@Nullable

‎dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/index/RepoIndexSourcePathResolverTest.groovy

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package datadog.trace.civisibility.source.index
22

33
import com.google.common.jimfs.Configuration
44
import com.google.common.jimfs.Jimfs
5+
import datadog.trace.api.Config
56
import groovy.transform.PackageScope
67
import spock.lang.Specification
78

@@ -10,6 +11,7 @@ import java.nio.file.Path
1011

1112
class RepoIndexSourcePathResolverTest extends Specification {
1213

14+
def config = Stub(Config)
1315
def packageResolver = Stub(PackageResolver)
1416
def resourceResolver = Stub(ResourceResolver)
1517
def fileSystem = Jimfs.newFileSystem(Configuration.unix())
@@ -20,7 +22,7 @@ class RepoIndexSourcePathResolverTest extends Specification {
2022
def expectedSourcePath = givenSourceFile(RepoIndexSourcePathResolverTest, repoRoot + "/src")
2123

2224
when:
23-
def sourcePathResolver = new RepoIndexSourcePathResolver(repoRoot, packageResolver, resourceResolver, fileSystem)
25+
def sourcePathResolver = new RepoIndexSourcePathResolver(config, repoRoot, packageResolver, resourceResolver, fileSystem)
2426

2527
then:
2628
sourcePathResolver.getSourcePath(RepoIndexSourcePathResolverTest) == expectedSourcePath
@@ -31,7 +33,7 @@ class RepoIndexSourcePathResolverTest extends Specification {
3133
def expectedSourcePath = givenSourceFile(RepoIndexSourcePathResolverTest, repoRoot + "/src")
3234

3335
when:
34-
def sourcePathResolver = new RepoIndexSourcePathResolver(repoRoot, packageResolver, resourceResolver, fileSystem)
36+
def sourcePathResolver = new RepoIndexSourcePathResolver(config, repoRoot, packageResolver, resourceResolver, fileSystem)
3537

3638
then:
3739
sourcePathResolver.getSourcePath(InnerClass) == expectedSourcePath
@@ -42,7 +44,7 @@ class RepoIndexSourcePathResolverTest extends Specification {
4244
def expectedSourcePath = givenSourceFile(RepoIndexSourcePathResolverTest, repoRoot + "/src")
4345

4446
when:
45-
def sourcePathResolver = new RepoIndexSourcePathResolver(repoRoot, packageResolver, resourceResolver, fileSystem)
47+
def sourcePathResolver = new RepoIndexSourcePathResolver(config, repoRoot, packageResolver, resourceResolver, fileSystem)
4648

4749
then:
4850
sourcePathResolver.getSourcePath(InnerClass.NestedInnerClass) == expectedSourcePath
@@ -53,7 +55,7 @@ class RepoIndexSourcePathResolverTest extends Specification {
5355
def expectedSourcePath = givenSourceFile(RepoIndexSourcePathResolverTest, repoRoot + "/src")
5456

5557
when:
56-
def sourcePathResolver = new RepoIndexSourcePathResolver(repoRoot, packageResolver, resourceResolver, fileSystem)
58+
def sourcePathResolver = new RepoIndexSourcePathResolver(config, repoRoot, packageResolver, resourceResolver, fileSystem)
5759
def r = new Runnable() {
5860
void run() {}
5961
}
@@ -67,7 +69,7 @@ class RepoIndexSourcePathResolverTest extends Specification {
6769
def expectedSourcePath = givenSourceFile(RepoIndexSourcePathResolverTest, repoRoot + "/src")
6870

6971
when:
70-
def sourcePathResolver = new RepoIndexSourcePathResolver(repoRoot, packageResolver, resourceResolver, fileSystem)
72+
def sourcePathResolver = new RepoIndexSourcePathResolver(config, repoRoot, packageResolver, resourceResolver, fileSystem)
7173

7274
then:
7375
sourcePathResolver.getSourcePath(PackagePrivateClass) == expectedSourcePath
@@ -78,7 +80,7 @@ class RepoIndexSourcePathResolverTest extends Specification {
7880
def expectedSourcePath = givenSourceFile(RepoIndexSourcePathResolverTest, repoRoot + "/src")
7981

8082
when:
81-
def sourcePathResolver = new RepoIndexSourcePathResolver(repoRoot, packageResolver, resourceResolver, fileSystem)
83+
def sourcePathResolver = new RepoIndexSourcePathResolver(config, repoRoot, packageResolver, resourceResolver, fileSystem)
8284

8385
then:
8486
sourcePathResolver.getSourcePath(PackagePrivateClass.NestedIntoPackagePrivateClass) == expectedSourcePath
@@ -89,7 +91,7 @@ class RepoIndexSourcePathResolverTest extends Specification {
8991
def expectedSourcePath = givenSourceFile(RepoIndexSourcePathResolverTest, repoRoot + "/src")
9092

9193
when:
92-
def sourcePathResolver = new RepoIndexSourcePathResolver(repoRoot, packageResolver, resourceResolver, fileSystem)
94+
def sourcePathResolver = new RepoIndexSourcePathResolver(config, repoRoot, packageResolver, resourceResolver, fileSystem)
9395

9496
then:
9597
sourcePathResolver.getSourcePath(PublicClassWhoseNameDoesNotCorrespondToFileName) == expectedSourcePath
@@ -99,7 +101,7 @@ class RepoIndexSourcePathResolverTest extends Specification {
99101
setup:
100102

101103
when:
102-
def sourcePathResolver = new RepoIndexSourcePathResolver(repoRoot, packageResolver, resourceResolver, fileSystem)
104+
def sourcePathResolver = new RepoIndexSourcePathResolver(config, repoRoot, packageResolver, resourceResolver, fileSystem)
103105

104106
then:
105107
sourcePathResolver.getSourcePath(RepoIndexSourcePathResolver) == null
@@ -109,7 +111,7 @@ class RepoIndexSourcePathResolverTest extends Specification {
109111
setup:
110112

111113
when:
112-
def sourcePathResolver = new RepoIndexSourcePathResolver(repoRoot, packageResolver, resourceResolver, fileSystem)
114+
def sourcePathResolver = new RepoIndexSourcePathResolver(config, repoRoot, packageResolver, resourceResolver, fileSystem)
113115

114116
then:
115117
sourcePathResolver.getSourcePath(PackagePrivateClass) == null
@@ -124,7 +126,7 @@ class RepoIndexSourcePathResolverTest extends Specification {
124126
Files.write(classPath, "STUB CLASS BODY".getBytes())
125127

126128
when:
127-
def sourcePathResolver = new RepoIndexSourcePathResolver(repoRoot, packageResolver, resourceResolver, fileSystem)
129+
def sourcePathResolver = new RepoIndexSourcePathResolver(config, repoRoot, packageResolver, resourceResolver, fileSystem)
128130

129131
then:
130132
sourcePathResolver.getSourcePath(RepoIndexSourcePathResolverTest) == null
@@ -138,7 +140,7 @@ class RepoIndexSourcePathResolverTest extends Specification {
138140
givenRepoFile(fileSystem.getPath(repoRoot, "README.md"))
139141

140142
when:
141-
def sourcePathResolver = new RepoIndexSourcePathResolver(repoRoot, packageResolver, resourceResolver, fileSystem)
143+
def sourcePathResolver = new RepoIndexSourcePathResolver(config, repoRoot, packageResolver, resourceResolver, fileSystem)
142144

143145
then:
144146
sourcePathResolver.getSourcePath(RepoIndexSourcePathResolverTest) == expectedSourcePathOne

‎dd-trace-api/src/main/java/datadog/trace/api/config/CiVisibilityConfig.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ public final class CiVisibilityConfig {
6262
public static final String CIVISIBILITY_JVM_INFO_CACHE_SIZE = "civisibility.jvm.info.cache.size";
6363
public static final String CIVISIBILITY_COVERAGE_SEGMENTS_ENABLED =
6464
"civisibility.coverage.segments.enabled";
65+
public static final String CIVISIBILITY_COVERAGE_ROOT_PACKAGES_LIMIT =
66+
"civisibility.coverage.root.packages.limit";
6567
public static final String CIVISIBILITY_INJECTED_TRACER_VERSION =
6668
"civisibility.injected.tracer.version";
6769
public static final String CIVISIBILITY_RESOURCE_FOLDER_NAMES =

‎internal-api/src/main/java/datadog/trace/api/Config.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@
153153
import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_CODE_COVERAGE_REPORT_DUMP_DIR;
154154
import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_COMPILER_PLUGIN_AUTO_CONFIGURATION_ENABLED;
155155
import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_COMPILER_PLUGIN_VERSION;
156+
import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_COVERAGE_ROOT_PACKAGES_LIMIT;
156157
import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_COVERAGE_SEGMENTS_ENABLED;
157158
import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_DEBUG_PORT;
158159
import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_GIT_COMMAND_TIMEOUT_MILLIS;
@@ -736,6 +737,7 @@ static class HostNameHolder {
736737
private final int ciVisibilityModuleExecutionSettingsCacheSize;
737738
private final int ciVisibilityJvmInfoCacheSize;
738739
private final boolean ciVisibilityCoverageSegmentsEnabled;
740+
private final int ciVisibilityCoverageRootPackagesLimit;
739741
private final String ciVisibilityInjectedTracerVersion;
740742
private final List<String> ciVisibilityResourceFolderNames;
741743

@@ -1690,6 +1692,8 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins
16901692
ciVisibilityJvmInfoCacheSize = configProvider.getInteger(CIVISIBILITY_JVM_INFO_CACHE_SIZE, 8);
16911693
ciVisibilityCoverageSegmentsEnabled =
16921694
configProvider.getBoolean(CIVISIBILITY_COVERAGE_SEGMENTS_ENABLED, false);
1695+
ciVisibilityCoverageRootPackagesLimit =
1696+
configProvider.getInteger(CIVISIBILITY_COVERAGE_ROOT_PACKAGES_LIMIT, 30);
16931697
ciVisibilityInjectedTracerVersion =
16941698
configProvider.getString(CIVISIBILITY_INJECTED_TRACER_VERSION);
16951699
ciVisibilityResourceFolderNames =
@@ -2834,6 +2838,10 @@ public boolean isCiVisibilityCoverageSegmentsEnabled() {
28342838
return ciVisibilityCoverageSegmentsEnabled;
28352839
}
28362840

2841+
public int getCiVisibilityCoverageRootPackagesLimit() {
2842+
return ciVisibilityCoverageRootPackagesLimit;
2843+
}
2844+
28372845
public String getCiVisibilityInjectedTracerVersion() {
28382846
return ciVisibilityInjectedTracerVersion;
28392847
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /