[SDTEST-2993] Switch to paginated endpoint for retrieving known tests#8380
Draft
calvinbayer wants to merge 3 commits intomasterfrom
Draft
[SDTEST-2993] Switch to paginated endpoint for retrieving known tests#8380calvinbayer wants to merge 3 commits intomasterfrom
calvinbayer wants to merge 3 commits intomasterfrom
Conversation
The known tests endpoint now returns results page by page instead of in
a single bulk response. This reduces load on the backend storage layer
and avoids timeouts for repositories with large numbers of tests.
The client sends page_info:{} on the first request to signal pagination
support, then follows cursor-based pagination until has_next is false.
Results are merged locally across all pages. A 10,000-page safety valve
prevents runaway loops. Backward compatible with non-paginated responses.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
calvinbayer
added
area:ci-visibility
type:enhancement
- Change KnownTestsSuites value type from string[] to List<string> and use AddRange instead of allocating new arrays for merging - Extract pageResponse.Value and pageInfo.Value.Cursor to local vars - Use StringUtil.IsNullOrEmpty instead of string.IsNullOrEmpty - Rename TestsJsonPages to KnownTestsJsonPages in MockData - Return collected data instead of default when hitting max pages limit Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
calvinbayer
commented
Mar 26, 2026
| } | ||
| else | ||
| { | ||
| var merged = new string[existingTests.Length + suiteEntry.Value.Length]; |
Author
There was a problem hiding this comment.
(from @tonyredondo on #8379)
oh for each suite in different pages it will allocate new string arrays, I think it will be better change the type to a List and just call AddRange from here.
calvinbayer
commented
Mar 26, 2026
| Tests = tests; | ||
| } | ||
|
|
||
| public sealed class KnownTestsSuites : Dictionary<string, string[]?> |
Author
There was a problem hiding this comment.
(from @tonyredondo on #8379)
here, that string[] can be changed to List<string>
calvinbayer
commented
Mar 26, 2026
Comment on lines
+84
to
+87
| MergeKnownTests(ref aggregateTests, pageResponse.Value.Tests); | ||
|
|
||
| // Check pagination | ||
| var pageInfo = pageResponse.Value.PageInfo; |
Author
There was a problem hiding this comment.
(from @tonyredondo on #8379)
nit, you can create a local variable to store pageResponse.Value and reuse it.
calvinbayer
commented
Mar 26, 2026
Comment on lines
+94
to
+100
| if (string.IsNullOrEmpty(pageInfo.Value.Cursor)) | ||
| { | ||
| Log.Warning<int>("TestOptimizationClient: Known tests response has has_next=true but no cursor on page {PageNumber}. Aborting pagination.", pageNumber); | ||
| return default; | ||
| } | ||
|
|
||
| pageState = pageInfo.Value.Cursor; |
Author
There was a problem hiding this comment.
(from @tonyredondo on #8379)
nit: same here pageInfo.Value.Cursor is used twice here, we can declare a local var to store it in the stack a reuse it.
Also, we prefer StringUtil.IsNullOrEmpty instead of string.IsNullOrEmpty (not the first time in this block of code)
calvinbayer
commented
Mar 26, 2026
| /// pages sequentially instead of <see cref="TestsJson"/>. Each entry must be a complete | ||
| /// JSON response including page_info with cursor/has_next. | ||
| /// </summary> | ||
| public readonly string[]? TestsJsonPages; |
Author
There was a problem hiding this comment.
(from @tonyredondo on #8379)
the name of the variable should be KnownTestsJsonPages right?
calvinbayer
commented
Mar 26, 2026
Comment on lines
+104
to
108
| if (pageNumber >= MaxKnownTestsPages) | ||
| { | ||
| Log.Warning<int>("TestOptimizationClient: Known tests pagination exceeded maximum of {MaxPages} pages. Aborting.", MaxKnownTestsPages); | ||
| return default; | ||
| } |
Author
There was a problem hiding this comment.
(from @tonyredondo on #8379)
doing all the serialization, deserialization network request work for 10000 pages to throw it all here. Feels like a waste of resources.
BenchmarksBenchmark execution time: 2026-03-29 20:59:26 Comparing candidate commit af1697b in PR branch Found 7 performance improvements and 9 performance regressions! Performance is the same for 255 metrics, 17 unstable metrics.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8380) and master. ✅ No regressions detected - check the details below Full Metrics ComparisonFakeDbCommand
HttpMessageHandler
Comparison explanationExecution-time benchmarks measure the whole time it takes to execute a program, and are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are highlighted in **red**. The following thresholds were used for comparing the execution times:
Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard. Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph). Duration chartsFakeDbCommand (.NET Framework 4.8)gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8380) - mean (72ms) : 69, 74
master - mean (72ms) : 69, 74
section Bailout
This PR (8380) - mean (76ms) : 74, 78
master - mean (76ms) : 74, 78
section CallTarget+Inlining+NGEN
This PR (8380) - mean (1,071ms) : 1034, 1108
master - mean (1,085ms) : 1026, 1144
FakeDbCommand (.NET Core 3.1)gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8380) - mean (112ms) : 108, 115
master - mean (113ms) : 110, 116
section Bailout
This PR (8380) - mean (113ms) : 110, 116
master - mean (114ms) : 112, 117
section CallTarget+Inlining+NGEN
This PR (8380) - mean (791ms) : 767, 814
master - mean (796ms) : 777, 815
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8380) - mean (99ms) : 96, 102
master - mean (100ms) : 96, 103
section Bailout
This PR (8380) - mean (100ms) : 97, 103
master - mean (100ms) : 97, 102
section CallTarget+Inlining+NGEN
This PR (8380) - mean (949ms) : 909, 990
master - mean (946ms) : 900, 992
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8380) - mean (98ms) : 94, 102
master - mean (98ms) : 95, 102
section Bailout
This PR (8380) - mean (99ms) : 97, 102
master - mean (99ms) : 97, 101
section CallTarget+Inlining+NGEN
This PR (8380) - mean (831ms) : 798, 863
master - mean (831ms) : 801, 862
HttpMessageHandler (.NET Framework 4.8)gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8380) - mean (193ms) : 189, 197
master - mean (193ms) : 189, 197
section Bailout
This PR (8380) - mean (197ms) : 193, 201
master - mean (197ms) : 193, 201
section CallTarget+Inlining+NGEN
This PR (8380) - mean (1,159ms) : 1103, 1216
master - mean (1,163ms) : 1108, 1217
HttpMessageHandler (.NET Core 3.1)gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8380) - mean (278ms) : 272, 284
master - mean (278ms) : 272, 284
section Bailout
This PR (8380) - mean (279ms) : 274, 284
master - mean (279ms) : 274, 283
section CallTarget+Inlining+NGEN
This PR (8380) - mean (949ms) : 919, 978
master - mean (951ms) : 923, 978
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8380) - mean (270ms) : 265, 276
master - mean (271ms) : 265, 277
section Bailout
This PR (8380) - mean (270ms) : 265, 275
master - mean (271ms) : 267, 276
section CallTarget+Inlining+NGEN
This PR (8380) - mean (1,153ms) : 1116, 1191
master - mean (1,152ms) : 1113, 1190
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8380) - mean (270ms) : 263, 276
master - mean (269ms) : 264, 275
section Bailout
This PR (8380) - mean (268ms) : 264, 273
master - mean (270ms) : 264, 276
section CallTarget+Inlining+NGEN
This PR (8380) - mean (1,035ms) : 997, 1072
master - mean (1,036ms) : 992, 1081
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
MergeKnownTests returned early without initializing the aggregate dictionary when a page had Count: 0. This left aggregate as null, causing KnownTestsFeature.Enabled to return false and preventing EFD from activating. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
api/v2/ci/libraries/tests) as part of SDTEST-2993page_infoin requests and followshas_next/cursorpagination, deep-merging test results across all pagespage_infoare treated as single-page (existing behavior)Changes
tracer/src/Datadog.Trace/Ci/Net/TestOptimizationClient.GetKnownTestsAsync.cs— Core pagination loop withMergeKnownTests(), newPageInfoRequest/PageInfoResponse/KnownTestsPageResponseDTOstracer/test/.../CI/TestingFrameworkEvpTest.cs— Mock server supports paginated response mode viaTestsJsonPagesarraytracer/test/.../CI/XUnitEvpTests.cs— Newefd_with_test_bypass_paginatedtest scenariotracer/test/snapshots/XUnitEvpTests.EarlyFlakeDetection_efd_with_test_bypass_paginated.verified.txt— Snapshot for paginated scenarioTest plan
🤖 Generated with Claude Code