[Test Optimization] ci: git caching + env improvements#8072
Merged
tonyredondo merged 8 commits intomasterfrom Feb 3, 2026
Merged
[Test Optimization] ci: git caching + env improvements#8072tonyredondo merged 8 commits intomasterfrom
tonyredondo merged 8 commits intomasterfrom
Conversation
This was referenced Jan 15, 2026
BenchmarksBenchmark execution time: 2026-02-03 10:08:16 Comparing candidate commit 9debeb5 in PR branch Found 7 performance improvements and 9 performance regressions! Performance is the same for 160 metrics, 16 unstable metrics. scenario:Benchmarks.Trace.ActivityBenchmark.StartStopWithChild net6.0
scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleMoreComplexBody netcoreapp3.1
scenario:Benchmarks.Trace.Asm.AppSecEncoderBenchmark.EncodeLegacyArgs net6.0
scenario:Benchmarks.Trace.Asm.AppSecEncoderBenchmark.EncodeLegacyArgs netcoreapp3.1
scenario:Benchmarks.Trace.AspNetCoreBenchmark.SendRequest net6.0
scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces net472
scenario:Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces netcoreapp3.1
scenario:Benchmarks.Trace.CharSliceBenchmark.OriginalCharSlice net6.0
scenario:Benchmarks.Trace.ElasticsearchBenchmark.CallElasticsearch net6.0
scenario:Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync netcoreapp3.1
scenario:Benchmarks.Trace.Log4netBenchmark.EnrichedLog netcoreapp3.1
scenario:Benchmarks.Trace.NLogBenchmark.EnrichedLog net6.0
scenario:Benchmarks.Trace.SpanBenchmark.StartFinishSpan net6.0
scenario:Benchmarks.Trace.SpanBenchmark.StartFinishSpan netcoreapp3.1
|
d68d0a9 to
de740b1
Compare
50fc18c to
2018124
Compare
This comment has been minimized.
This comment has been minimized.
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8072) 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 (8072) - mean (72ms) : 70, 75
master - mean (74ms) : 70, 77
section Bailout
This PR (8072) - mean (78ms) : 75, 81
master - mean (78ms) : 77, 79
section CallTarget+Inlining+NGEN
This PR (8072) - mean (1,055ms) : 1015, 1095
master - mean (1,066ms) : 1011, 1120
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 (8072) - mean (114ms) : 110, 119
master - mean (116ms) : 113, 119
section Bailout
This PR (8072) - mean (115ms) : 112, 117
master - mean (117ms) : 114, 120
section CallTarget+Inlining+NGEN
This PR (8072) - mean (788ms) : 736, 841
master - mean (788ms) : 730, 845
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8072) - mean (100ms) : 97, 104
master - mean (103ms) : 99, 107
section Bailout
This PR (8072) - mean (102ms) : 98, 105
master - mean (104ms) : 101, 106
section CallTarget+Inlining+NGEN
This PR (8072) - mean (745ms) : 678, 813
master - mean (747ms) : 675, 818
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8072) - mean (99ms) : 96, 102
master - mean (101ms) : 98, 103
section Bailout
This PR (8072) - mean (101ms) : 98, 103
master - mean (102ms) : 100, 105
section CallTarget+Inlining+NGEN
This PR (8072) - mean (678ms) : 647, 708
master - mean (685ms) : 666, 705
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 (8072) - mean (198ms) : 191, 204
master - mean (197ms) : 192, 203
section Bailout
This PR (8072) - mean (200ms) : 197, 204
master - mean (203ms) : 196, 209
section CallTarget+Inlining+NGEN
This PR (8072) - mean (1,162ms) : 1101, 1223
master - mean (1,171ms) : 1096, 1246
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 (8072) - mean (284ms) : 277, 291
master - mean (284ms) : 277, 291
section Bailout
This PR (8072) - mean (285ms) : 279, 290
master - mean (286ms) : 279, 292
section CallTarget+Inlining+NGEN
This PR (8072) - mean (963ms) : 908, 1018
master - mean (962ms) : 915, 1009
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8072) - mean (278ms) : 270, 286
master - mean (277ms) : 271, 283
section Bailout
This PR (8072) - mean (277ms) : 272, 283
master - mean (278ms) : 270, 286
section CallTarget+Inlining+NGEN
This PR (8072) - mean (947ms) : 884, 1009
master - mean (941ms) : 873, 1008
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8072) - mean (278ms) : 271, 285
master - mean (277ms) : 271, 283
section Bailout
This PR (8072) - mean (277ms) : 270, 284
master - mean (279ms) : 270, 287
section CallTarget+Inlining+NGEN
This PR (8072) - mean (856ms) : 831, 881
master - mean (855ms) : 828, 881
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
de740b1 to
a92777c
Compare
b98a4d3 to
e77015c
Compare
a92777c to
d0595bf
Compare
e77015c to
8fae54a
Compare
d0595bf to
7216d32
Compare
8fae54a to
33077cb
Compare
tonyredondo
commented
Jan 16, 2026
Comment on lines
+113
to
+123
| if (gitInfo.Errors.Count > 0) | ||
| { | ||
| var sb = StringBuilderCache.Acquire(); | ||
| sb.AppendLine(); | ||
| foreach (var err in gitInfo.Errors) | ||
| { | ||
| sb.AppendLine(" Error: " + err); | ||
| } | ||
|
|
||
| Log.Warning("CIEnvironmentValues: Errors detected in the local gitInfo: {Errors}", StringBuilderCache.GetStringAndRelease(sb)); | ||
| } |
Member
Author
There was a problem hiding this comment.
Before this we were swallowing all the git errors when extracting the local git info (without any ci provider info)
tonyredondo
commented
Jan 16, 2026
Comment on lines
+125
to
+140
| if (Log.IsEnabled(LogEventLevel.Debug)) | ||
| { | ||
| var sb = StringBuilderCache.Acquire(); | ||
| sb.AppendLine(); | ||
| var values = ValueProvider.GetValues(); | ||
| foreach (var field in typeof(CIEnvironmentValues.Constants).GetFields()) | ||
| { | ||
| var fieldName = field.GetValue(null) as string; | ||
| if (!StringUtil.IsNullOrEmpty(fieldName) && values.TryGetValue<string>(fieldName, out var value)) | ||
| { | ||
| sb.AppendFormat("\t{0}={1}{2}", fieldName, value == string.Empty ? "(empty)" : $"\"{value}\"", Environment.NewLine); | ||
| } | ||
| } | ||
|
|
||
| Log.Debug("CIEnvironmentValues: Values detected:{Values}", StringBuilderCache.GetStringAndRelease(sb)); | ||
| } |
Member
Author
There was a problem hiding this comment.
To help to debug issues in the CI providers parser here we dump the environment variables defined in the Contants class related to the CI Providers environment variables.
tonyredondo
commented
Jan 16, 2026
Comment on lines
-35
to
-45
| // Ensure we have permissions to read the git directory | ||
| var safeDirectory = ProcessHelpers.RunCommand( | ||
| new ProcessHelpers.Command( | ||
| cmd: "git", | ||
| arguments: $"config --global --add safe.directory {gitDirectory.FullName}", | ||
| workingDirectory: gitDirectory.FullName, | ||
| useWhereIsIfFileNotFound: true)); | ||
| if (safeDirectory?.ExitCode != 0) | ||
| { | ||
| localGitInfo.Errors.Add($"Error setting safe.directory: {safeDirectory?.Error}"); | ||
| } |
Member
Author
There was a problem hiding this comment.
we no longer need to modify the git global configuration, we will use the -c safe.directory option on each git command call.
tonyredondo
commented
Jan 16, 2026
| } | ||
|
|
||
| // Get the repository URL | ||
| var repositoryOutput = ProcessHelpers.RunCommand( |
Member
Author
There was a problem hiding this comment.
we already have a helper to call git commands
tonyredondo
commented
Jan 16, 2026
Comment on lines
+128
to
+135
| var baseDirectory = AppContext.BaseDirectory; | ||
| if (!baseDirectory.Contains("/dotnet/sdk") && !string.IsNullOrWhiteSpace(RuntimeFolder) && !baseDirectory.Contains(RuntimeFolder)) | ||
| { | ||
| if (!File.Exists(Path.Combine(baseDirectory, DatadogTraceToolsRunnerAssembly))) | ||
| { | ||
| searchPaths.Add(baseDirectory); | ||
| } | ||
| } |
Member
Author
There was a problem hiding this comment.
for the testhost process the base directory is the dotnet sdk, so we need to check that before considering it as a search path for the git metadata.
tonyredondo
commented
Jan 16, 2026
Comment on lines
+46
to
+86
| if (useCache && string.IsNullOrEmpty(input)) | ||
| { | ||
| string runId; | ||
| if (TestOptimization.Instance is TestOptimization { } tOpt) | ||
| { | ||
| runId = tOpt.EnsureRunId(workingDirectory); | ||
| } | ||
| else | ||
| { | ||
| runId = TestOptimization.Instance.RunId; | ||
| } | ||
|
|
||
| var cacheFolder = Path.Combine(workingDirectory, ".dd", runId, "git"); | ||
| try | ||
| { | ||
| if (!Directory.Exists(cacheFolder)) | ||
| { | ||
| Directory.CreateDirectory(cacheFolder); | ||
| } | ||
|
|
||
| lock (Hasher) | ||
| { | ||
| var hash = Hasher.ComputeHash(Encoding.UTF8.GetBytes(arguments)); | ||
| cacheKey = Path.Combine(cacheFolder, BitConverter.ToString(hash).ToLowerInvariant() + ".json"); | ||
| } | ||
|
|
||
| if (File.Exists(cacheKey)) | ||
| { | ||
| var jsonValue = File.ReadAllText(cacheKey); | ||
| if (JsonConvert.DeserializeObject<ProcessHelpers.CommandOutput>(jsonValue) is { } cachedOutput) | ||
| { | ||
| cachedOutput.Cached = true; | ||
| return cachedOutput; | ||
| } | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Log.Warning(ex, "Error in the git cache."); | ||
| } | ||
| } |
Member
Author
There was a problem hiding this comment.
this prepares the cache folder for the git commands, so we avoid running the same git commands over and over again from different processes (that's the issue with test optimization, involves multiple processes per run)
tonyredondo
commented
Jan 16, 2026
| try | ||
| { | ||
| var sw = System.Diagnostics.Stopwatch.StartNew(); | ||
| arguments = $"-c safe.directory={workingDirectory} {arguments}"; |
Member
Author
There was a problem hiding this comment.
here's the -c safe.directory to avoid changing the global git configuration.
tonyredondo
commented
Jan 16, 2026
Member
Author
There was a problem hiding this comment.
we need to make this serializable for the git commands cache
tonyredondo
commented
Jan 16, 2026
| Directory.Exists(testItem.GitFolderPath).Should().BeTrue(); | ||
|
|
||
| // Let's try with the git info provider based on manual parsing of the git folder | ||
| if (!ManualParserGitInfoProvider.Instance.TryGetFrom(testItem.GitFolderPath, out var gitInfo)) |
Member
Author
There was a problem hiding this comment.
we are removing the ManualParser provider, because we now always need the git command, so it doesn't worth having a manual .git folder parser.
anmarchenko
approved these changes
Jan 16, 2026
7216d32 to
4a70c7a
Compare
07b09e0 to
566cf1c
Compare
4a70c7a to
b3116fd
Compare
566cf1c to
87c7fd2
Compare
b3116fd to
643a4d8
Compare
87c7fd2 to
2f8135c
Compare
643a4d8 to
36cf4ce
Compare
eee4289 to
882dbe0
Compare
bouwkast
approved these changes
Jan 28, 2026
tonyredondo
added a commit
that referenced
this pull request
Jan 30, 2026
## Summary of changes - Add FixedSizeArrayPool and logger overloads to reduce per-log allocations. - Add RefStopwatch/StopwatchHelpers/CodeDuration and apply timing to TraceClock/TracerManagerFactory/Instrumentation and CI processors. - Lower init logs to Debug for CI protocol writer/sender and processors. ## Reason for change Reduce allocations and improve timing diagnostics in hot paths. ## Implementation details - New utility helpers in `Datadog.Trace.Util` for allocation-free timing and pooling. - Logging changes to avoid params array allocations in common log paths. - Timing/log-level tweaks in CI protocol writer/sender and trace processors. ## Test coverage CI passes then all changes are good. ## Other details - Stacked PRs (current marked): - **CURRENT**: PR1 #8070 - PR2 #8071 - PR3 #8072 - PR4 #8073
36cf4ce to
1e2d8dd
Compare
882dbe0 to
2131bb0
Compare
tonyredondo
added a commit
that referenced
this pull request
Feb 3, 2026
## Summary of changes - Introduce Test Optimization RunId and EnsureRunId with cache folder lifecycle. - Add `_DD_INTERNAL_TOPT_RUNID` to config mapping/docs and regenerate keys. - Update CI integration tests to set/print RunId for deterministic runs. - Adjust test helper thread naming for clearer CI debugging. ## Reason for change Provide a stable RunId before caching work that depends on it and keep configuration in sync. ## Implementation details - Add RunId/EnsureRunId to `ITestOptimization`/`TestOptimization` with environment propagation. - Update configuration mapping and generated CI visibility keys. - Update CI integration tests for RunId coverage. ## Test coverage CI passes then all changes are good. ## Other details - Stacked PRs (current marked): - PR1 #8070 - **CURRENT**: PR2 #8071 - PR3 #8072 - PR4 #8073
Base automatically changed from
tony/testoptimization-performance-work-pr2
to
master
February 3, 2026 09:23
b8c4942 to
9debeb5
Compare
tonyredondo
added a commit
that referenced
this pull request
Feb 3, 2026
## Summary of changes - Add cached/file Test Optimization clients and update client wrappers. - Reduce feature init overhead and adjust background initialization. - Improve runner logging and CI workspace cache usage. ## Reason for change Reduce repeated HTTP calls and improve initialization performance for CI visibility features. ## Implementation details - New cached/file client implementations for disk/memory caching. - Feature creation updated to reduce dependencies on client calls. - CodeDuration instrumentation added to CI initialization paths. ## Test coverage CI passes then all changes are good. ## Other details - Stacked PRs (current marked): - PR1 #8070 - PR2 #8071 - PR3 #8072 - **CURRENT**: PR4 #8073
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
3 participants
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 of changes
JIRA: SDTEST-3226
Reason for change
Reduce git command overhead and improve visibility into CI git metadata collection.
Implementation details
Test coverage
CI passes then all changes are good.
Other details