Skip to content

Commit bd03e78

Browse files
tonyredondotonyredondo
committed
[CI Visibility] Harden global coverage lifecycle correctness
1 parent bfd8734 commit bd03e78

49 files changed

Lines changed: 746 additions & 675 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎tracer/src/Datadog.Trace.Tools.Runner/RunCiCommand.cs‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
using System;
77
using System.CommandLine;
88
using System.CommandLine.Invocation;
9-
using System.Diagnostics;
109
using System.Linq;
1110
using System.Threading.Tasks;
1211
using Datadog.Trace.Ci.Tags;
@@ -37,10 +36,6 @@ public RunCiCommand(ApplicationContext applicationContext)
3736
this.SetHandler(ExecuteAsync);
3837
}
3938

40-
internal static TimeSpan? ProcessTimeoutForTests { get; set; }
41-
42-
internal static Action<ProcessStartInfo> ProcessStartObserverForTests { get; set; }
43-
4439
private async Task ExecuteAsync(InvocationContext context)
4540
{
4641
var args = _runSettings.Command.GetValue(context);
@@ -93,14 +88,10 @@ private async Task ExecuteAsync(InvocationContext context)
9388
processInfo.ArgumentList.Add(arg);
9489
}
9590

96-
ProcessStartObserverForTests?.Invoke(processInfo);
97-
9891
Log.Debug("RunCiCommand.FileName: '{FileName}'", processInfo.FileName);
9992
Log.Debug("RunCiCommand.Arguments: '{Arguments}'", string.Join(" ", processInfo.ArgumentList));
10093
Log.Debug("RunCiCommand.WorkingDirectory: '{WorkingDirectory}'", processInfo.WorkingDirectory);
101-
var exitCode = ProcessTimeoutForTests is { } processTimeout
102-
? Utils.RunProcess(processInfo, _applicationContext.TokenSource.Token, processTimeout).ExitCode
103-
: Utils.RunProcess(processInfo, _applicationContext.TokenSource.Token);
94+
var exitCode = Utils.RunProcess(processInfo, _applicationContext.TokenSource.Token);
10495
Log.Debug<int>("RunCiCommand.ExitCode: {Value}", exitCode);
10596

10697
if (!initResults.TestSkippingEnabled)

‎tracer/src/Datadog.Trace.Tools.Runner/RunProcessResult.cs‎

Lines changed: 0 additions & 31 deletions
This file was deleted.

‎tracer/src/Datadog.Trace.Tools.Runner/Utils.cs‎

Lines changed: 5 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -284,32 +284,13 @@ public static ProcessStartInfo GetProcessStartInfo(string filename, string curre
284284
}
285285

286286
public static int RunProcess(ProcessStartInfo startInfo, CancellationToken cancellationToken)
287-
=> RunProcessCore(startInfo, cancellationToken, timeout: null, stopwatch: null).ExitCode;
288-
289-
internal static RunProcessResult RunProcess(ProcessStartInfo startInfo, CancellationToken cancellationToken, TimeSpan timeout)
290-
{
291-
if (timeout < TimeSpan.Zero)
292-
{
293-
throw new ArgumentOutOfRangeException(nameof(timeout));
294-
}
295-
296-
return RunProcessCore(startInfo, cancellationToken, timeout, Stopwatch.StartNew());
297-
}
298-
299-
private static RunProcessResult RunProcessCore(
300-
ProcessStartInfo startInfo,
301-
CancellationToken cancellationToken,
302-
TimeSpan? timeout,
303-
Stopwatch stopwatch)
304287
{
305-
var rootProcessId = -1;
306288
try
307289
{
308290
using var childProcess = new Process();
309291
childProcess.StartInfo = startInfo;
310292
childProcess.EnableRaisingEvents = true;
311293
childProcess.Start();
312-
rootProcessId = childProcess.Id;
313294

314295
using var ctr = cancellationToken.Register(
315296
() =>
@@ -324,66 +305,8 @@ private static RunProcessResult RunProcessCore(
324305
}
325306
});
326307

327-
if (timeout is null)
328-
{
329-
childProcess.WaitForExit();
330-
return new RunProcessResult(
331-
cancellationToken.IsCancellationRequested ? 1 : childProcess.ExitCode,
332-
timedOut: false,
333-
rootProcessId,
334-
treeKillAttempted: false,
335-
treeKillSucceeded: false,
336-
reaped: true);
337-
}
338-
339-
var remaining = timeout.Value - stopwatch.Elapsed;
340-
var waitMilliseconds = remaining <= TimeSpan.Zero
341-
? 0
342-
: (int)Math.Min(int.MaxValue, Math.Ceiling(remaining.TotalMilliseconds));
343-
if (childProcess.WaitForExit(waitMilliseconds))
344-
{
345-
return new RunProcessResult(
346-
cancellationToken.IsCancellationRequested ? 1 : childProcess.ExitCode,
347-
timedOut: false,
348-
rootProcessId,
349-
treeKillAttempted: false,
350-
treeKillSucceeded: false,
351-
reaped: true);
352-
}
353-
354-
Log.Error<int>("RunProcess: Process {ProcessId} exceeded its test timeout and will be terminated.", rootProcessId);
355-
var treeKillSucceeded = false;
356-
try
357-
{
358-
#if NET5_0_OR_GREATER
359-
childProcess.Kill(entireProcessTree: true);
360-
#else
361-
childProcess.Kill();
362-
#endif
363-
treeKillSucceeded = true;
364-
}
365-
catch (Exception ex)
366-
{
367-
Log.Warning<int>(ex, "RunProcess: Failed to terminate timed-out process {ProcessId}.", rootProcessId);
368-
}
369-
370-
var reaped = false;
371-
try
372-
{
373-
reaped = childProcess.WaitForExit((int)TimeSpan.FromSeconds(30).TotalMilliseconds);
374-
}
375-
catch (Exception ex)
376-
{
377-
Log.Warning<int>(ex, "RunProcess: Failed while reaping timed-out process {ProcessId}.", rootProcessId);
378-
}
379-
380-
return new RunProcessResult(
381-
exitCode: 1,
382-
timedOut: true,
383-
rootProcessId,
384-
treeKillAttempted: true,
385-
treeKillSucceeded,
386-
reaped);
308+
childProcess.WaitForExit();
309+
return cancellationToken.IsCancellationRequested ? 1 : childProcess.ExitCode;
387310
}
388311
catch (System.ComponentModel.Win32Exception win32Exception)
389312
{
@@ -406,7 +329,7 @@ private static RunProcessResult RunProcessCore(
406329
if (File.Exists(path))
407330
{
408331
startInfo.FileName = path;
409-
return RunProcessCore(startInfo, cancellationToken, timeout, stopwatch);
332+
return RunProcess(startInfo, cancellationToken);
410333
}
411334
}
412335
}
@@ -421,7 +344,7 @@ private static RunProcessResult RunProcessCore(
421344
File.Exists(processPath))
422345
{
423346
startInfo.FileName = processPath;
424-
return RunProcessCore(startInfo, cancellationToken, timeout, stopwatch);
347+
return RunProcess(startInfo, cancellationToken);
425348
}
426349
}
427350
}
@@ -433,13 +356,7 @@ private static RunProcessResult RunProcessCore(
433356
AnsiConsole.WriteException(ex);
434357
}
435358

436-
return new RunProcessResult(
437-
exitCode: 1,
438-
timedOut: false,
439-
rootProcessId,
440-
treeKillAttempted: false,
441-
treeKillSucceeded: false,
442-
reaped: rootProcessId < 0);
359+
return 1;
443360
}
444361

445362
public static string[] SplitArgs(string command, bool keepQuote = false)

‎tracer/src/Datadog.Trace/Ci/Coverage/CoverageContextContainer.cs‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using System.Runtime.ExceptionServices;
1212
using System.Threading;
1313
using Datadog.Trace.Ci.Coverage.Metadata;
14+
using Datadog.Trace.Util;
1415

1516
namespace Datadog.Trace.Ci.Coverage;
1617

@@ -31,9 +32,9 @@ public CoverageContextContainer(object? state = null, ModuleValue.BufferKind buf
3132

3233
public object? State { get; set; }
3334

34-
internal bool IsClosed => Volatile.Read(ref _closed) != 0;
35+
public bool IsClosed => Volatile.Read(ref _closed) != 0;
3536

36-
internal ModuleValue? GetModuleValue(Module module)
37+
public ModuleValue? GetModuleValue(Module module)
3738
{
3839
if (IsClosed)
3940
{
@@ -56,7 +57,7 @@ public CoverageContextContainer(object? state = null, ModuleValue.BufferKind buf
5657
}
5758
}
5859

59-
internal bool TryGetOrAddModuleValue(
60+
public bool TryGetOrAddModuleValue(
6061
ModuleCoverageMetadata metadata,
6162
Module module,
6263
int rawByteLength,
@@ -100,7 +101,7 @@ internal bool TryGetOrAddModuleValue(
100101
}
101102
}
102103

103-
internal bool TryCloseAndGetModules(out IReadOnlyList<ModuleValue> modules)
104+
public bool TryCloseAndGetModules(out IReadOnlyList<ModuleValue> modules)
104105
{
105106
lock (_gate)
106107
{
@@ -117,20 +118,20 @@ internal bool TryCloseAndGetModules(out IReadOnlyList<ModuleValue> modules)
117118
}
118119
}
119120

120-
internal ModuleValue[] SnapshotModules(int maximumModules = int.MaxValue)
121+
public ModuleValue[] SnapshotModules(int maximumModules = int.MaxValue)
121122
{
122123
lock (_gate)
123124
{
124125
if (_modules.Count > maximumModules)
125126
{
126-
throw new InvalidOperationException("The global coverage fallback contains too many modules.");
127+
ThrowHelper.ThrowInvalidOperationException("The global coverage fallback contains too many modules.");
127128
}
128129

129130
return _modules.Count == 0 ? Array.Empty<ModuleValue>() : _modules.ToArray();
130131
}
131132
}
132133

133-
internal void Clear() => Dispose();
134+
public void Clear() => Dispose();
134135

135136
public void Dispose()
136137
{

‎tracer/src/Datadog.Trace/Ci/Coverage/CoverageEventHandler.cs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
using Datadog.Trace.Ci.Telemetry;
1313
using Datadog.Trace.Telemetry;
1414
using Datadog.Trace.Telemetry.Metrics;
15+
using Datadog.Trace.Util;
1516

1617
namespace Datadog.Trace.Ci.Coverage;
1718

@@ -21,11 +22,11 @@ internal abstract class CoverageEventHandler
2122
private readonly CoverageContextContainer _globalContainer = new(bufferKind: ModuleValue.BufferKind.GlobalFallback);
2223
private readonly CoverageContextDiagnostics _contextDiagnostics = new();
2324

24-
internal CoverageContextContainer? Container => _asyncContext.Value;
25+
public CoverageContextContainer? Container => _asyncContext.Value;
2526

26-
internal CoverageContextContainer GlobalContainer => _globalContainer;
27+
public CoverageContextContainer GlobalContainer => _globalContainer;
2728

28-
internal CoverageContextDiagnosticSnapshot ContextDiagnostics => _contextDiagnostics.GetSnapshot();
29+
public CoverageContextDiagnosticSnapshot ContextDiagnostics => _contextDiagnostics.GetSnapshot();
2930

3031
[MethodImpl(MethodImplOptions.AggressiveInlining)]
3132
public CoverageSessionHandle StartSession(string? testingFramework = null)
@@ -81,7 +82,7 @@ public CoverageSessionHandle StartSession(string? testingFramework = null)
8182

8283
if (!ReferenceEquals(handle.Owner, this))
8384
{
84-
throw new InvalidOperationException("The coverage session handle belongs to another handler.");
85+
ThrowHelper.ThrowInvalidOperationException("The coverage session handle belongs to another handler.");
8586
}
8687

8788
var context = handle.Context!;
@@ -120,7 +121,7 @@ public CoverageSessionHandle StartSession(string? testingFramework = null)
120121
}
121122
}
122123

123-
internal void AbortSession(CoverageSessionHandle handle, GlobalCoverageFailureReason reason)
124+
public void AbortSession(CoverageSessionHandle handle, GlobalCoverageFailureReason reason)
124125
{
125126
try
126127
{
@@ -157,7 +158,7 @@ internal void AbortSession(CoverageSessionHandle handle, GlobalCoverageFailureRe
157158
}
158159
}
159160

160-
internal void MarkProbeDataIncomplete(GlobalCoverageFailureReason reason)
161+
public void MarkProbeDataIncomplete(GlobalCoverageFailureReason reason)
161162
{
162163
try
163164
{

‎tracer/src/Datadog.Trace/Ci/Coverage/CoverageReporter.cs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,15 @@ internal static CoverageEventHandler Handler
3131
[MethodImpl(MethodImplOptions.AggressiveInlining)]
3232
get => LazyInitializer.EnsureInitialized(ref _handler, static () => CreateDefaultHandler(TestOptimization.Instance.Settings))!;
3333
[MethodImpl(MethodImplOptions.AggressiveInlining)]
34-
set => Volatile.Write(ref _handler, value ?? throw new ArgumentNullException(nameof(value)));
34+
set
35+
{
36+
if (value is null)
37+
{
38+
ThrowHelper.ThrowArgumentNullException(nameof(value));
39+
}
40+
41+
Volatile.Write(ref _handler, value);
42+
}
3543
}
3644

3745
internal static CoverageContextContainer? Container => Handler.Container;
@@ -55,7 +63,7 @@ internal static CoverageEventHandler CreateDefaultHandler(TestOptimizationSettin
5563
{
5664
if (settings is null)
5765
{
58-
throw new ArgumentNullException(nameof(settings));
66+
ThrowHelper.ThrowArgumentNullException(nameof(settings));
5967
}
6068

6169
return settings.TestsSkippingEnabled == true && StringUtil.IsNullOrWhiteSpace(settings.CodeCoveragePath)

‎tracer/src/Datadog.Trace/Ci/Coverage/CoverageReporter`1.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ private static ModuleValue GetOrCreateGlobalModuleValue(CoverageEventHandler han
9898
ModuleMemorySize,
9999
out var module) || module is null)
100100
{
101-
throw new InvalidOperationException("The global coverage context is unexpectedly closed.");
101+
ThrowHelper.ThrowInvalidOperationException("The global coverage context is unexpectedly closed.");
102102
}
103103

104104
Interlocked.CompareExchange(ref _globalModuleValue, module, null);

‎tracer/src/Datadog.Trace/Ci/Coverage/CoverageSessionHandle.cs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ namespace Datadog.Trace.Ci.Coverage;
99

1010
internal sealed class CoverageSessionHandle
1111
{
12-
internal static readonly CoverageSessionHandle Invalid = new();
12+
public static readonly CoverageSessionHandle Invalid = new();
1313

1414
private CoverageSessionHandle()
1515
{
@@ -18,22 +18,22 @@ private CoverageSessionHandle()
1818
Admission = CoverageContextAdmission.Noop;
1919
}
2020

21-
internal CoverageSessionHandle(CoverageEventHandler owner, CoverageContextContainer context, CoverageContextAdmission admission)
21+
public CoverageSessionHandle(CoverageEventHandler owner, CoverageContextContainer context, CoverageContextAdmission admission)
2222
{
2323
Owner = owner;
2424
Context = context;
2525
Admission = admission;
2626
}
2727

28-
internal CoverageEventHandler? Owner { get; }
28+
public CoverageEventHandler? Owner { get; }
2929

30-
internal CoverageContextContainer? Context { get; }
30+
public CoverageContextContainer? Context { get; }
3131

32-
internal CoverageContextAdmission Admission { get; }
32+
public CoverageContextAdmission Admission { get; }
3333

34-
internal bool IsValid => Owner is not null && Context is not null;
34+
public bool IsValid => Owner is not null && Context is not null;
3535

36-
internal void AbortIncomplete(GlobalCoverageFailureReason reason)
36+
public void AbortIncomplete(GlobalCoverageFailureReason reason)
3737
{
3838
Owner?.AbortSession(this, reason);
3939
}

0 commit comments

Comments
 (0)