Skip to content

Commit d55fe02

Browse files
nikita-tkachenko-datadognikita-tkachenko-datadog
committed
Implement fallback to file-level granularity for impacted tests detection
1 parent 5ddb07b commit d55fe02

File tree

7 files changed

+171
-28
lines changed

7 files changed

+171
-28
lines changed

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

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

910
public interface ConfigurationApi {
1011

@@ -31,6 +32,11 @@ public Map<String, Collection<TestIdentifier>> getKnownTestsByModule(
3132
TracerEnvironment tracerEnvironment) {
3233
return Collections.emptyMap();
3334
}
35+
36+
@Override
37+
public Set<String> getChangedFiles(TracerEnvironment tracerEnvironment) {
38+
return Collections.emptySet();
39+
}
3440
};
3541

3642
CiVisibilitySettings getSettings(TracerEnvironment tracerEnvironment) throws IOException;
@@ -42,4 +48,6 @@ Map<String, Collection<TestIdentifier>> getFlakyTestsByModule(TracerEnvironment
4248

4349
Map<String, Collection<TestIdentifier>> getKnownTestsByModule(TracerEnvironment tracerEnvironment)
4450
throws IOException;
51+
52+
Set<String> getChangedFiles(TracerEnvironment tracerEnvironment) throws IOException;
4553
}

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import java.util.HashSet;
3131
import java.util.List;
3232
import java.util.Map;
33+
import java.util.Set;
3334
import java.util.UUID;
3435
import java.util.function.Supplier;
3536
import javax.annotation.Nullable;
@@ -47,6 +48,7 @@ public class ConfigurationApiImpl implements ConfigurationApi {
4748

4849
private static final String SETTINGS_URI = "libraries/tests/services/setting";
4950
private static final String SKIPPABLE_TESTS_URI = "ci/tests/skippable";
51+
private static final String CHANGED_FILES_URI = "ci/tests/diffs";
5052
private static final String FLAKY_TESTS_URI = "ci/libraries/tests/flaky";
5153
private static final String KNOWN_TESTS_URI = "ci/libraries/tests";
5254

@@ -58,6 +60,7 @@ public class ConfigurationApiImpl implements ConfigurationApi {
5860
private final JsonAdapter<EnvelopeDto<CiVisibilitySettings>> settingsResponseAdapter;
5961
private final JsonAdapter<MultiEnvelopeDto<TestIdentifierJson>> testIdentifiersResponseAdapter;
6062
private final JsonAdapter<EnvelopeDto<KnownTestsDto>> testFullNamesResponseAdapter;
63+
private final JsonAdapter<EnvelopeDto<ChangedFilesDto>> changedFilesResponseAdapter;
6164

6265
public ConfigurationApiImpl(BackendApi backendApi, CiVisibilityMetricCollector metricCollector) {
6366
this(backendApi, metricCollector, () -> UUID.randomUUID().toString());
@@ -98,6 +101,11 @@ public ConfigurationApiImpl(BackendApi backendApi, CiVisibilityMetricCollector m
98101
Types.newParameterizedTypeWithOwner(
99102
ConfigurationApiImpl.class, EnvelopeDto.class, KnownTestsDto.class);
100103
testFullNamesResponseAdapter = moshi.adapter(testFullNamesResponseType);
104+
105+
ParameterizedType changedFilesResponseAdapterType =
106+
Types.newParameterizedTypeWithOwner(
107+
ConfigurationApiImpl.class, EnvelopeDto.class, ChangedFilesDto.class);
108+
changedFilesResponseAdapter = moshi.adapter(changedFilesResponseAdapterType);
101109
}
102110

103111
@Override
@@ -285,6 +293,37 @@ private Map<String, Collection<TestIdentifier>> parseTestIdentifiers(KnownTestsD
285293
return testIdentifiers;
286294
}
287295

296+
@Override
297+
public Set<String> getChangedFiles(TracerEnvironment tracerEnvironment) throws IOException {
298+
OkHttpUtils.CustomListener telemetryListener =
299+
new TelemetryListener.Builder(metricCollector)
300+
.requestCount(CiVisibilityCountMetric.IMPACTED_TESTS_DETECTION_REQUEST)
301+
.requestErrors(CiVisibilityCountMetric.IMPACTED_TESTS_DETECTION_REQUEST_ERRORS)
302+
.requestDuration(CiVisibilityDistributionMetric.IMPACTED_TESTS_DETECTION_REQUEST_MS)
303+
.responseBytes(CiVisibilityDistributionMetric.IMPACTED_TESTS_DETECTION_RESPONSE_BYTES)
304+
.build();
305+
306+
String uuid = uuidGenerator.get();
307+
EnvelopeDto<TracerEnvironment> request =
308+
new EnvelopeDto<>(new DataDto<>(uuid, "ci_app_tests_diffs_request", tracerEnvironment));
309+
String json = requestAdapter.toJson(request);
310+
RequestBody requestBody = RequestBody.create(JSON, json);
311+
ChangedFilesDto changedFiles =
312+
backendApi.post(
313+
CHANGED_FILES_URI,
314+
requestBody,
315+
is ->
316+
changedFilesResponseAdapter.fromJson(Okio.buffer(Okio.source(is))).data.attributes,
317+
telemetryListener,
318+
false);
319+
320+
LOGGER.debug("Received {} changed files", changedFiles.files.size());
321+
metricCollector.add(
322+
CiVisibilityDistributionMetric.IMPACTED_TESTS_DETECTION_RESPONSE_FILES,
323+
changedFiles.files.size());
324+
return changedFiles.files;
325+
}
326+
288327
private static final class EnvelopeDto<T> {
289328
private final DataDto<T> data;
290329

@@ -372,4 +411,12 @@ private KnownTestsDto(Map<String, Map<String, List<String>>> tests) {
372411
this.tests = tests;
373412
}
374413
}
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+
}
375422
}

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

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@
99
import datadog.trace.api.git.GitInfoProvider;
1010
import datadog.trace.civisibility.ci.PullRequestInfo;
1111
import datadog.trace.civisibility.diff.Diff;
12+
import datadog.trace.civisibility.diff.FileDiff;
1213
import datadog.trace.civisibility.git.tree.GitClient;
1314
import datadog.trace.civisibility.git.tree.GitDataUploader;
15+
import java.io.IOException;
1416
import java.nio.file.Path;
1517
import java.nio.file.Paths;
1618
import java.util.BitSet;
@@ -173,7 +175,7 @@ private TracerEnvironment buildTracerEnvironment(
173175
moduleNames.addAll(knownTestsByModule.keySet());
174176
}
175177

176-
Diff pullRequestDiff = getPullRequestDiff(impactedTestsDetectionEnabled);
178+
Diff pullRequestDiff = getPullRequestDiff(impactedTestsDetectionEnabled, tracerEnvironment);
177179

178180
Map<String, ExecutionSettings> settingsByModule = new HashMap<>();
179181
for (String moduleName : moduleNames) {
@@ -300,24 +302,33 @@ private Map<String, Collection<TestIdentifier>> getKnownTestsByModule(
300302
}
301303

302304
@NotNull
303-
private Diff getPullRequestDiff(boolean impactedTestsDetectionEnabled) {
304-
if (repositoryRoot == null || !impactedTestsDetectionEnabled) {
305+
private Diff getPullRequestDiff(
306+
boolean impactedTestsDetectionEnabled, TracerEnvironment tracerEnvironment) {
307+
if (!impactedTestsDetectionEnabled) {
305308
return Diff.EMPTY;
306309
}
307-
// FIXME nikita: add file-based granularity fallback (+ telemetry)
308-
// FIXME nikita: add integration/smoke tests
310+
309311
try {
310-
GitClient gitClient = gitClientFactory.create(repositoryRoot);
311-
return gitClient.getGitDiff(
312-
pullRequestInfo.getPullRequestBaseBranchSha(), pullRequestInfo.getGitCommitHeadSha());
312+
if (repositoryRoot != null && pullRequestInfo.isNotEmpty()) {
313+
GitClient gitClient = gitClientFactory.create(repositoryRoot);
314+
return gitClient.getGitDiff(
315+
pullRequestInfo.getPullRequestBaseBranchSha(), pullRequestInfo.getGitCommitHeadSha());
316+
}
313317

314318
} catch (InterruptedException e) {
315319
LOGGER.error("Interrupted while getting git diff for PR: {}", pullRequestInfo, e);
316320
Thread.currentThread().interrupt();
317-
return Diff.EMPTY;
318321

319322
} catch (Exception e) {
320323
LOGGER.error("Could not get git diff for PR: {}", pullRequestInfo, e);
324+
}
325+
326+
try {
327+
// falling back to file-level granularity
328+
return new FileDiff(configurationApi.getChangedFiles(tracerEnvironment));
329+
330+
} catch (IOException e) {
331+
LOGGER.error("Could not get changed files from backend", e);
321332
return Diff.EMPTY;
322333
}
323334
}

‎dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/config/ConfigurationApiImplTest.groovy‎

Lines changed: 76 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ class ConfigurationApiImplTest extends Specification {
6767

6868
def response = response
6969
if (expectedRequest) {
70-
def requestBody = ('{' +
70+
def responseBody = ('{' +
7171
' "data": {' +
7272
' "type": "ci_app_tracers_test_service_settings",' +
7373
' "id": "uuid",' +
@@ -95,10 +95,10 @@ class ConfigurationApiImplTest extends Specification {
9595
def gzipSupported = header != null && header.contains("gzip")
9696
if (gzipSupported) {
9797
response.addHeader("Content-Encoding", "gzip")
98-
requestBody = gzip(requestBody)
98+
responseBody = gzip(responseBody)
9999
}
100100

101-
response.status(200).send(requestBody)
101+
response.status(200).send(responseBody)
102102
} else {
103103
response.status(400).send()
104104
}
@@ -187,7 +187,7 @@ class ConfigurationApiImplTest extends Specification {
187187
}
188188

189189
def response = response
190-
def requestBody = """
190+
def responseBody = """
191191
{
192192
"data": [
193193
${tests.join(',')}
@@ -207,10 +207,10 @@ class ConfigurationApiImplTest extends Specification {
207207
def gzipSupported = header != null && header.contains("gzip")
208208
if (gzipSupported) {
209209
response.addHeader("Content-Encoding", "gzip")
210-
requestBody = gzip(requestBody)
210+
responseBody = gzip(responseBody)
211211
}
212212

213-
response.status(200).send(requestBody)
213+
response.status(200).send(responseBody)
214214
}
215215

216216
prefix("/api/v2/ci/libraries/tests/flaky") {
@@ -296,7 +296,7 @@ class ConfigurationApiImplTest extends Specification {
296296
}
297297

298298
def response = response
299-
def requestBody = """
299+
def responseBody = """
300300
{
301301
"data": [
302302
${tests.join(',')}
@@ -316,10 +316,10 @@ class ConfigurationApiImplTest extends Specification {
316316
def gzipSupported = header != null && header.contains("gzip")
317317
if (gzipSupported) {
318318
response.addHeader("Content-Encoding", "gzip")
319-
requestBody = gzip(requestBody)
319+
responseBody = gzip(responseBody)
320320
}
321321

322-
response.status(200).send(requestBody)
322+
response.status(200).send(responseBody)
323323
}
324324

325325
prefix("/api/v2/ci/libraries/tests") {
@@ -354,7 +354,7 @@ class ConfigurationApiImplTest extends Specification {
354354

355355
def response = response
356356
if (expectedRequest) {
357-
def requestBody = ("""
357+
def responseBody = ("""
358358
{
359359
"data": {
360360
"id": "9p1jTQLXB8g",
@@ -387,14 +387,62 @@ class ConfigurationApiImplTest extends Specification {
387387
def gzipSupported = header != null && header.contains("gzip")
388388
if (gzipSupported) {
389389
response.addHeader("Content-Encoding", "gzip")
390-
requestBody = gzip(requestBody)
390+
responseBody = gzip(responseBody)
391391
}
392392

393-
response.status(200).send(requestBody)
393+
response.status(200).send(responseBody)
394394
} else {
395395
response.status(400).send()
396396
}
397397
}
398+
399+
prefix("/api/v2/ci/tests/diffs") {
400+
def requestJson = moshi.adapter(Map).fromJson(new String(request.body))
401+
402+
// assert request contents
403+
def requestData = requestJson['data']
404+
if (requestData['type'] != "ci_app_tests_diffs_request") {
405+
response.status(400).send()
406+
return
407+
}
408+
409+
def requestAttrs = requestData['attributes']
410+
if (requestAttrs['service'] != "foo"
411+
|| requestAttrs['env'] != "foo_env"
412+
|| requestAttrs['repository_url'] != "https://github.com/DataDog/foo"
413+
|| requestAttrs['branch'] != "prod"
414+
|| requestAttrs['sha'] != "d64185e45d1722ab3a53c45be47accae"
415+
) {
416+
response.status(400).send()
417+
return
418+
}
419+
420+
def response = response
421+
def responseBody = """
422+
{
423+
"data": {
424+
"type": "ci_app_tests_diffs_response",
425+
"id": "<some-hash>",
426+
"attributes": {
427+
"base_sha": "ef733331f7cee9b1c89d82df87942d8606edf3f7",
428+
"files": [
429+
"domains/ci-app/apps/apis/rapid-ci-app/internal/itrapihttp/api.go",
430+
"domains/ci-app/apps/apis/rapid-ci-app/internal/itrapihttp/api_test.go"
431+
]
432+
}
433+
}
434+
}
435+
""".bytes
436+
437+
def header = request.getHeader("Accept-Encoding")
438+
def gzipSupported = header != null && header.contains("gzip")
439+
if (gzipSupported) {
440+
response.addHeader("Content-Encoding", "gzip")
441+
responseBody = gzip(responseBody)
442+
}
443+
444+
response.status(200).send(responseBody)
445+
}
398446
}
399447
}
400448

@@ -566,6 +614,22 @@ class ConfigurationApiImplTest extends Specification {
566614
givenIntakeApi(true) | "intake, response compression enabled"
567615
}
568616

617+
def "test changed files request"() {
618+
given:
619+
def tracerEnvironment = givenTracerEnvironment()
620+
621+
when:
622+
def api = givenIntakeApi(true)
623+
def configurationApi = new ConfigurationApiImpl(api, Stub(CiVisibilityMetricCollector), () -> "1234")
624+
def files = configurationApi.getChangedFiles(tracerEnvironment)
625+
626+
then:
627+
files == new HashSet([
628+
"domains/ci-app/apps/apis/rapid-ci-app/internal/itrapihttp/api.go",
629+
"domains/ci-app/apps/apis/rapid-ci-app/internal/itrapihttp/api_test.go"
630+
])
631+
}
632+
569633
private BackendApi givenEvpProxy(boolean responseCompression) {
570634
String traceId = "a-trace-id"
571635
HttpUrl proxyUrl = HttpUrl.get(intakeServer.address)

‎dd-java-agent/agent-ci-visibility/src/testFixtures/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ import java.nio.file.Files
5656
import java.nio.file.Path
5757
import java.nio.file.Paths
5858

59+
// FIXME nikita: add integration/smoke tests
5960
abstract class CiVisibilityInstrumentationTest extends AgentTestRunner {
6061

6162
static String dummyModule
@@ -121,7 +122,7 @@ abstract class CiVisibilityInstrumentationTest extends AgentTestRunner {
121122
false,
122123
itrEnabled,
123124
flakyRetryEnabled,
124-
false, // FIXME nikita: add instrumentation tests for impacted tests detection
125+
false,
125126
earlyFlakinessDetectionSettings,
126127
itrEnabled ? "itrCorrelationId" : null,
127128
skippableTestsWithMetadata,
@@ -205,8 +206,8 @@ abstract class CiVisibilityInstrumentationTest extends AgentTestRunner {
205206
TestFrameworkSession testSession = testFrameworkSessionFactory.startSession(dummyModule, component, null)
206207
TestFrameworkModule testModule = testSession.testModuleStart(dummyModule, null)
207208
new TestEventsHandlerImpl(metricCollector, testSession, testModule,
208-
suiteStore != null ? suiteStore : new ConcurrentHashMapContextStore<>(),
209-
testStore != null ? testStore : new ConcurrentHashMapContextStore<>())
209+
suiteStore != null ? suiteStore : new ConcurrentHashMapContextStore<>(),
210+
testStore != null ? testStore : new ConcurrentHashMapContextStore<>())
210211
}
211212
}
212213

‎internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/CiVisibilityCountMetric.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,13 @@ public enum CiVisibilityCountMetric {
131131
EFD_REQUEST_ERRORS("early_flake_detection.request_errors", ErrorType.class, StatusCode.class),
132132
/** The number of requests sent to the flaky tests endpoint */
133133
FLAKY_TESTS_REQUEST("flaky_tests.request", RequestCompressed.class),
134-
/** The number of known tests requests sent to the flaky tests that errored */
135-
FLAKY_TESTS_REQUEST_ERRORS("flaky_tests.request_errors", ErrorType.class, StatusCode.class);
134+
/** The number of tests requests sent to the flaky tests endpoint that errored */
135+
FLAKY_TESTS_REQUEST_ERRORS("flaky_tests.request_errors", ErrorType.class, StatusCode.class),
136+
/** The number of requests sent to the changed files endpoint */
137+
IMPACTED_TESTS_DETECTION_REQUEST("impacted_tests_detection.request", RequestCompressed.class),
138+
/** The number of tests requests sent to the changed files endpoint that errored */
139+
IMPACTED_TESTS_DETECTION_REQUEST_ERRORS(
140+
"impacted_tests_detection.request_errors", ErrorType.class, StatusCode.class);
136141

137142
// need a "holder" class, as accessing static fields from enum constructors is illegal
138143
static class IndexHolder {

0 commit comments

Comments
 (0)