Skip to content

Commit 5e0e4ed

Browse files
nikita-tkachenko-datadognikita-tkachenko-datadog
committed
Use commit SHA from backend diff response if no PR info is available + extract a no-op git client implementation to avoid spamming logs with exceptions in envs where git executable is not available
1 parent d6c4567 commit 5e0e4ed

File tree

15 files changed

+1026
-645
lines changed

15 files changed

+1026
-645
lines changed

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

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@
88
import datadog.communication.ddagent.SharedCommunicationObjects;
99
import datadog.communication.http.HttpRetryPolicy;
1010
import datadog.communication.http.OkHttpUtils;
11+
import datadog.communication.util.IOUtils;
1112
import datadog.trace.api.Config;
13+
import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric;
1214
import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
15+
import datadog.trace.api.civisibility.telemetry.tag.Command;
1316
import datadog.trace.api.git.GitInfoProvider;
1417
import datadog.trace.civisibility.ci.CIProviderInfoFactory;
1518
import datadog.trace.civisibility.ci.env.CiEnvironment;
@@ -22,24 +25,28 @@
2225
import datadog.trace.civisibility.git.CIProviderGitInfoBuilder;
2326
import datadog.trace.civisibility.git.GitClientGitInfoBuilder;
2427
import datadog.trace.civisibility.git.tree.GitClient;
28+
import datadog.trace.civisibility.git.tree.NoOpGitClient;
29+
import datadog.trace.civisibility.git.tree.ShellGitClient;
2530
import datadog.trace.civisibility.ipc.SignalClient;
2631
import datadog.trace.civisibility.source.BestEffortLinesResolver;
2732
import datadog.trace.civisibility.source.ByteCodeLinesResolver;
2833
import datadog.trace.civisibility.source.CompilerAidedLinesResolver;
2934
import datadog.trace.civisibility.source.LinesResolver;
3035
import datadog.trace.civisibility.source.index.*;
36+
import datadog.trace.civisibility.utils.ShellCommandExecutor;
37+
import java.io.File;
3138
import java.lang.reflect.Type;
3239
import java.net.InetSocketAddress;
3340
import java.nio.file.FileSystem;
3441
import java.nio.file.FileSystems;
3542
import java.nio.file.Path;
3643
import java.util.Collections;
3744
import java.util.Map;
45+
import javax.annotation.Nonnull;
3846
import javax.annotation.Nullable;
3947
import okhttp3.HttpUrl;
4048
import okhttp3.OkHttpClient;
4149
import okhttp3.Request;
42-
import org.jetbrains.annotations.NotNull;
4350
import org.slf4j.Logger;
4451
import org.slf4j.LoggerFactory;
4552

@@ -78,7 +85,7 @@ public class CiVisibilityServices {
7885
this.backendApi =
7986
new BackendApiFactory(config, sco).createBackendApi(BackendApiFactory.Intake.API);
8087
this.jvmInfoFactory = new CachingJvmInfoFactory(config, new JvmInfoFactoryImpl());
81-
this.gitClientFactory = new GitClient.Factory(config, metricCollector);
88+
this.gitClientFactory = buildGitClientFactory(config, metricCollector);
8289

8390
CiEnvironment environment = buildCiEnvironment(config, sco);
8491
this.ciProviderInfoFactory = new CIProviderInfoFactory(config, environment);
@@ -111,7 +118,27 @@ public class CiVisibilityServices {
111118
}
112119
}
113120

114-
@NotNull
121+
private static GitClient.Factory buildGitClientFactory(
122+
Config config, CiVisibilityMetricCollector metricCollector) {
123+
try {
124+
ShellCommandExecutor shellCommandExecutor =
125+
new ShellCommandExecutor(new File("."), config.getCiVisibilityGitCommandTimeoutMillis());
126+
String gitVersion = shellCommandExecutor.executeCommand(IOUtils::readFully, "git", "version");
127+
logger.debug("Detected git executable version {}", gitVersion);
128+
return new ShellGitClient.Factory(config, metricCollector);
129+
130+
} catch (Exception e) {
131+
metricCollector.add(
132+
CiVisibilityCountMetric.GIT_COMMAND_ERRORS,
133+
1,
134+
Command.OTHER,
135+
ShellCommandExecutor.getExitCode(e));
136+
logger.info("No git executable detected, some features will not be available");
137+
return r -> NoOpGitClient.INSTANCE;
138+
}
139+
}
140+
141+
@Nonnull
115142
private static CiEnvironment buildCiEnvironment(Config config, SharedCommunicationObjects sco) {
116143
String remoteEnvVarsProviderUrl = config.getCiVisibilityRemoteEnvVarsProviderUrl();
117144
if (remoteEnvVarsProviderUrl != null) {

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package datadog.trace.civisibility.config;
2+
3+
import com.squareup.moshi.Json;
4+
import java.util.Collections;
5+
import java.util.Set;
6+
7+
public class ChangedFiles {
8+
9+
public static final ChangedFiles EMPTY = new ChangedFiles(null, Collections.emptySet());
10+
11+
@Json(name = "base_sha")
12+
private final String baseSha;
13+
14+
private final Set<String> files;
15+
16+
ChangedFiles(String baseSha, Set<String> files) {
17+
this.baseSha = baseSha;
18+
this.files = files;
19+
}
20+
21+
public String getBaseSha() {
22+
return baseSha;
23+
}
24+
25+
public Set<String> getFiles() {
26+
return files;
27+
}
28+
}

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import java.util.Collection;
66
import java.util.Collections;
77
import java.util.Map;
8-
import java.util.Set;
98

109
public interface ConfigurationApi {
1110

@@ -34,8 +33,8 @@ public Map<String, Collection<TestIdentifier>> getKnownTestsByModule(
3433
}
3534

3635
@Override
37-
public Set<String> getChangedFiles(TracerEnvironment tracerEnvironment) {
38-
return Collections.emptySet();
36+
public ChangedFiles getChangedFiles(TracerEnvironment tracerEnvironment) {
37+
return ChangedFiles.EMPTY;
3938
}
4039
};
4140

@@ -49,5 +48,5 @@ Map<String, Collection<TestIdentifier>> getFlakyTestsByModule(TracerEnvironment
4948
Map<String, Collection<TestIdentifier>> getKnownTestsByModule(TracerEnvironment tracerEnvironment)
5049
throws IOException;
5150

52-
Set<String> getChangedFiles(TracerEnvironment tracerEnvironment) throws IOException;
51+
ChangedFiles getChangedFiles(TracerEnvironment tracerEnvironment) throws IOException;
5352
}

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

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import java.util.HashSet;
3131
import java.util.List;
3232
import java.util.Map;
33-
import java.util.Set;
3433
import java.util.UUID;
3534
import java.util.function.Supplier;
3635
import javax.annotation.Nullable;
@@ -60,7 +59,7 @@ public class ConfigurationApiImpl implements ConfigurationApi {
6059
private final JsonAdapter<EnvelopeDto<CiVisibilitySettings>> settingsResponseAdapter;
6160
private final JsonAdapter<MultiEnvelopeDto<TestIdentifierJson>> testIdentifiersResponseAdapter;
6261
private final JsonAdapter<EnvelopeDto<KnownTestsDto>> testFullNamesResponseAdapter;
63-
private final JsonAdapter<EnvelopeDto<ChangedFilesDto>> changedFilesResponseAdapter;
62+
private final JsonAdapter<EnvelopeDto<ChangedFiles>> changedFilesResponseAdapter;
6463

6564
public ConfigurationApiImpl(BackendApi backendApi, CiVisibilityMetricCollector metricCollector) {
6665
this(backendApi, metricCollector, () -> UUID.randomUUID().toString());
@@ -104,7 +103,7 @@ public ConfigurationApiImpl(BackendApi backendApi, CiVisibilityMetricCollector m
104103

105104
ParameterizedType changedFilesResponseAdapterType =
106105
Types.newParameterizedTypeWithOwner(
107-
ConfigurationApiImpl.class, EnvelopeDto.class, ChangedFilesDto.class);
106+
ConfigurationApiImpl.class, EnvelopeDto.class, ChangedFiles.class);
108107
changedFilesResponseAdapter = moshi.adapter(changedFilesResponseAdapterType);
109108
}
110109

@@ -294,7 +293,7 @@ private Map<String, Collection<TestIdentifier>> parseTestIdentifiers(KnownTestsD
294293
}
295294

296295
@Override
297-
public Set<String> getChangedFiles(TracerEnvironment tracerEnvironment) throws IOException {
296+
public ChangedFiles getChangedFiles(TracerEnvironment tracerEnvironment) throws IOException {
298297
OkHttpUtils.CustomListener telemetryListener =
299298
new TelemetryListener.Builder(metricCollector)
300299
.requestCount(CiVisibilityCountMetric.IMPACTED_TESTS_DETECTION_REQUEST)
@@ -308,7 +307,7 @@ public Set<String> getChangedFiles(TracerEnvironment tracerEnvironment) throws I
308307
new EnvelopeDto<>(new DataDto<>(uuid, "ci_app_tests_diffs_request", tracerEnvironment));
309308
String json = requestAdapter.toJson(request);
310309
RequestBody requestBody = RequestBody.create(JSON, json);
311-
ChangedFilesDto changedFiles =
310+
ChangedFiles changedFiles =
312311
backendApi.post(
313312
CHANGED_FILES_URI,
314313
requestBody,
@@ -317,11 +316,11 @@ public Set<String> getChangedFiles(TracerEnvironment tracerEnvironment) throws I
317316
telemetryListener,
318317
false);
319318

320-
LOGGER.debug("Received {} changed files", changedFiles.files.size());
319+
int filesCount = changedFiles.getFiles().size();
320+
LOGGER.debug("Received {} changed files", filesCount);
321321
metricCollector.add(
322-
CiVisibilityDistributionMetric.IMPACTED_TESTS_DETECTION_RESPONSE_FILES,
323-
changedFiles.files.size());
324-
return changedFiles.files;
322+
CiVisibilityDistributionMetric.IMPACTED_TESTS_DETECTION_RESPONSE_FILES, filesCount);
323+
return changedFiles;
325324
}
326325

327326
private static final class EnvelopeDto<T> {
@@ -411,12 +410,4 @@ private KnownTestsDto(Map<String, Map<String, List<String>>> tests) {
411410
this.tests = tests;
412411
}
413412
}
414-
415-
private static final class ChangedFilesDto {
416-
private final Set<String> files;
417-
418-
private ChangedFilesDto(Set<String> files) {
419-
this.files = files;
420-
}
421-
}
422413
}

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

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import datadog.trace.civisibility.git.tree.GitClient;
1414
import datadog.trace.civisibility.git.tree.GitDataUploader;
1515
import datadog.trace.civisibility.git.tree.GitRepoUnshallow;
16-
import java.io.IOException;
1716
import java.nio.file.Path;
1817
import java.nio.file.Paths;
1918
import java.util.BitSet;
@@ -26,7 +25,6 @@
2625
import java.util.concurrent.TimeUnit;
2726
import javax.annotation.Nonnull;
2827
import javax.annotation.Nullable;
29-
import org.jetbrains.annotations.NotNull;
3028
import org.slf4j.Logger;
3129
import org.slf4j.LoggerFactory;
3230

@@ -113,7 +111,7 @@ private TracerEnvironment buildTracerEnvironment(
113111
.build();
114112
}
115113

116-
private @NotNull Map<String, ExecutionSettings> create(TracerEnvironment tracerEnvironment) {
114+
private @Nonnull Map<String, ExecutionSettings> create(TracerEnvironment tracerEnvironment) {
117115
CiVisibilitySettings ciVisibilitySettings = getCiVisibilitySettings(tracerEnvironment);
118116

119117
boolean itrEnabled = isItrEnabled(ciVisibilitySettings);
@@ -305,19 +303,24 @@ private Map<String, Collection<TestIdentifier>> getKnownTestsByModule(
305303
}
306304
}
307305

308-
@NotNull
306+
@Nonnull
309307
private Diff getPullRequestDiff(
310308
boolean impactedTestsDetectionEnabled, TracerEnvironment tracerEnvironment) {
311309
if (!impactedTestsDetectionEnabled) {
312310
return Diff.EMPTY;
313311
}
314312

315313
try {
316-
if (repositoryRoot != null && pullRequestInfo.isNotEmpty()) {
314+
if (repositoryRoot != null) {
317315
// ensure repo is not shallow before attempting to get git diff
318316
gitRepoUnshallow.unshallow();
319-
return gitClient.getGitDiff(
320-
pullRequestInfo.getPullRequestBaseBranchSha(), pullRequestInfo.getGitCommitHeadSha());
317+
Diff diff =
318+
gitClient.getGitDiff(
319+
pullRequestInfo.getPullRequestBaseBranchSha(),
320+
pullRequestInfo.getGitCommitHeadSha());
321+
if (diff != null) {
322+
return diff;
323+
}
321324
}
322325

323326
} catch (InterruptedException e) {
@@ -329,12 +332,29 @@ private Diff getPullRequestDiff(
329332
}
330333

331334
try {
335+
ChangedFiles changedFiles = configurationApi.getChangedFiles(tracerEnvironment);
336+
337+
// attempting to use base SHA returned by the backend to calculate git diff
338+
if (repositoryRoot != null) {
339+
// ensure repo is not shallow before attempting to get git diff
340+
gitRepoUnshallow.unshallow();
341+
Diff diff = gitClient.getGitDiff(changedFiles.getBaseSha(), tracerEnvironment.getSha());
342+
if (diff != null) {
343+
return diff;
344+
}
345+
}
346+
332347
// falling back to file-level granularity
333-
return new FileDiff(configurationApi.getChangedFiles(tracerEnvironment));
348+
return new FileDiff(changedFiles.getFiles());
334349

335-
} catch (IOException e) {
336-
LOGGER.error("Could not get changed files from backend", e);
337-
return Diff.EMPTY;
350+
} catch (InterruptedException e) {
351+
LOGGER.error("Interrupted while getting git diff for: {}", tracerEnvironment, e);
352+
Thread.currentThread().interrupt();
353+
354+
} catch (Exception e) {
355+
LOGGER.error("Could not get git diff for: {}", tracerEnvironment, e);
338356
}
357+
358+
return Diff.EMPTY;
339359
}
340360
}

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,40 @@ private TracerEnvironment(
3636
this.configurations = configurations;
3737
}
3838

39+
public String getSha() {
40+
return sha;
41+
}
42+
3943
public Configurations getConfigurations() {
4044
return configurations;
4145
}
4246

47+
@Override
48+
public String toString() {
49+
return "TracerEnvironment{"
50+
+ "service='"
51+
+ service
52+
+ '\''
53+
+ ", env='"
54+
+ env
55+
+ '\''
56+
+ ", repositoryUrl='"
57+
+ repositoryUrl
58+
+ '\''
59+
+ ", branch='"
60+
+ branch
61+
+ '\''
62+
+ ", sha='"
63+
+ sha
64+
+ '\''
65+
+ ", testLevel='"
66+
+ testLevel
67+
+ '\''
68+
+ ", configurations="
69+
+ configurations
70+
+ '}';
71+
}
72+
4373
public static Builder builder() {
4474
return new Builder();
4575
}

0 commit comments

Comments
 (0)