Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add concurrency tests with Coyote to CI #4879

Merged
merged 46 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
3d25657
skeleton
Yun-Ting Sep 22, 2023
b776160
test
Yun-Ting Sep 27, 2023
9d2982a
Merge branch 'main' into yunl/addCoyoteCI_3
Yun-Ting Sep 27, 2023
a7ff1ac
update
Yun-Ting Sep 27, 2023
0a95eb7
split tests and cleanup
Yun-Ting Sep 27, 2023
22eb5dd
yikes
Yun-Ting Sep 27, 2023
b9b4db7
Merge branch 'main' into yunl/addCoyoteCI_3
Yun-Ting Sep 28, 2023
1da2417
space
Yun-Ting Sep 28, 2023
a18602b
Merge branch 'main' into yunl/addCoyoteCI_3
Yun-Ting Oct 5, 2023
5b96500
cleanups and net8
Yun-Ting Oct 5, 2023
c2f8650
Merge branch 'main' into yunl/addCoyoteCI_3
Yun-Ting Oct 5, 2023
cc152e7
comment
Yun-Ting Oct 5, 2023
7ce1dc4
Merge branch 'yunl/addCoyoteCI_3' of https://github.com/Yun-Ting/open…
Yun-Ting Oct 5, 2023
2bea31c
test
Yun-Ting Oct 6, 2023
3a69758
Merge branch 'main' into yunl/addCoyoteCI_3
Yun-Ting Oct 6, 2023
30e582f
Merge branch 'main' into yunl/addCoyoteCI_3
Yun-Ting Oct 6, 2023
67d51d0
use 7 for now
Yun-Ting Oct 6, 2023
f11877e
cleanups
Yun-Ting Oct 6, 2023
c48df7f
update shell
Yun-Ting Oct 6, 2023
f47ed19
cleanups
Yun-Ting Oct 6, 2023
fe21daf
,
Yun-Ting Oct 6, 2023
e979325
Merge from main.
CodeBlanch Oct 20, 2023
76ce40f
Added TracerConcurrencyTest.
CodeBlanch Oct 20, 2023
24f13bf
Merge branch 'main' into yunl/addCoyoteCI_3
CodeBlanch Oct 20, 2023
2d7777d
Tweaks.
CodeBlanch Oct 20, 2023
cfa87fc
Tweak.
CodeBlanch Oct 20, 2023
cbce63d
Tweak.
CodeBlanch Oct 20, 2023
e203f2f
Tweak.
CodeBlanch Oct 20, 2023
06d10de
Introduce race condition.
CodeBlanch Oct 20, 2023
bff40bc
Tweaks.
CodeBlanch Oct 20, 2023
e25754c
Tweak.
CodeBlanch Oct 20, 2023
15021b4
Revert race condition.
CodeBlanch Oct 20, 2023
3309c8e
Merge branch 'main' into yunl/addCoyoteCI_3
Yun-Ting Oct 31, 2023
80f2fc9
md workflow
Yun-Ting Oct 31, 2023
57d1b95
composition
Yun-Ting Oct 31, 2023
dfc8f80
sanity and sln
Yun-Ting Oct 31, 2023
1601fa2
fix
Yun-Ting Oct 31, 2023
f24b08e
comment
Yun-Ting Oct 31, 2023
ace5edf
Merge branch 'main' into yunl/addCoyoteCI_3
Yun-Ting Nov 1, 2023
b1015ce
merge main
Yun-Ting Nov 15, 2023
f7afd78
fixed error NU1605: OpenTelemetry.Tests -> Microsoft.Coyote 1.7.10 -…
Yun-Ting Nov 16, 2023
9d98916
Merge branch 'main' into yunl/addCoyoteCI_3
Yun-Ting Nov 16, 2023
af6e0fe
Merge branch 'main' into yunl/addCoyoteCI_3
Yun-Ting Nov 17, 2023
0388f74
comment
Yun-Ting Nov 17, 2023
7dca448
Merge branch 'yunl/addCoyoteCI_3' of https://github.com/Yun-Ting/open…
Yun-Ting Nov 17, 2023
583b7d6
ci
Yun-Ting Nov 17, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .github/workflows/ci-concurrency-md.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Syntax: https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions
# See also: https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/troubleshooting-required-status-checks#handling-skipped-but-required-checks

# Description: This workflow exists to unblock documentation-only PRs.

# IMPORTANT: This workflow MUST use the same 'name' and 'matrix' as the non -md workflow.


name: Coyote Concurrency Tests

on:
push:
branches: [ 'main*' ]
paths-ignore:
- '**.md'
pull_request:
branches: [ 'main*' ]
paths:
- '**.md'

jobs:
coyote-concurrency-tests:

strategy:
fail-fast: false # ensures the entire test matrix is run, even if one permutation fails
matrix:
os: [ windows-latest, ubuntu-latest ]
version: [ net8.0 ]
project: [ OpenTelemetry.Tests, OpenTelemetry.Api.Tests ]

runs-on: ${{ matrix.os }}
steps:
- run: 'echo "No build required"'
41 changes: 41 additions & 0 deletions .github/workflows/ci-concurrency.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
name: Coyote Concurrency Tests

on:
push:
branches: [ 'main*' ]
paths-ignore:
- '**.md'
Yun-Ting marked this conversation as resolved.
Show resolved Hide resolved
pull_request:
branches: [ 'main*' ]
paths-ignore:
- '**.md'

jobs:
coyote-concurrency-tests:

strategy:
fail-fast: false # ensures the entire test matrix is run, even if one permutation fails
matrix:
os: [ windows-latest, ubuntu-latest ]
version: [ net8.0 ]
project: [ OpenTelemetry.Tests, OpenTelemetry.Api.Tests ]

runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0 # fetching all

- name: Setup dotnet
uses: actions/setup-dotnet@v3

- name: Run Coyote Tests
shell: pwsh
run: .\build\test-threadSafety.ps1 -testProjectName ${{ matrix.project }} -targetFramework ${{ matrix.version }}

- name: Publish Artifacts
if: always() && !cancelled()
uses: actions/upload-artifact@v3
with:
name: ${{ matrix.os }}-${{ matrix.project }}-${{ matrix.version }}-coyoteoutput
path: '**/*_CoyoteOutput.*'
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -348,3 +348,6 @@ ASALocalRun/

# Tempo files
tempo-data/

# Coyote Rewrite Files
rewrite.coyote.json
12 changes: 8 additions & 4 deletions OpenTelemetry.sln
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,9 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
ProjectSection(SolutionItems) = preProject
.dockerignore = .dockerignore
.editorconfig = .editorconfig
.gitignore = .gitignore
.github\workflows\ci-concurrency.yml = .github\workflows\ci-concurrency.yml
CONTRIBUTING.md = CONTRIBUTING.md
Directory.Packages.props = Directory.Packages.props
test\Directory.Packages.props = test\Directory.Packages.props
examples\Directory.Packages.props = examples\Directory.Packages.props
docs\Directory.Packages.props = docs\Directory.Packages.props
global.json = global.json
LICENSE = LICENSE
NuGet.config = NuGet.config
Expand Down Expand Up @@ -45,6 +43,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "build", "build", "{7CB2F02E
build\RELEASING.md = build\RELEASING.md
build\stylecop.json = build\stylecop.json
build\test-aot-compatibility.ps1 = build\test-aot-compatibility.ps1
build\test-threadSafety.ps1 = build\test-threadSafety.ps1
build\xunit.runner.json = build\xunit.runner.json
EndProjectSection
EndProject
Expand Down Expand Up @@ -93,6 +92,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "workflows", "workflows", "{
ProjectSection(SolutionItems) = preProject
.github\workflows\ci-aot-md.yml = .github\workflows\ci-aot-md.yml
.github\workflows\ci-aot.yml = .github\workflows\ci-aot.yml
.github\workflows\ci-concurrency.yml = .github\workflows\ci-concurrency.yml
.github\workflows\ci-concurrency-md.yml = .github\workflows\ci-concurrency-md.yml
.github\workflows\ci-instrumentation-libraries-md.yml = .github\workflows\ci-instrumentation-libraries-md.yml
.github\workflows\ci-instrumentation-libraries.yml = .github\workflows\ci-instrumentation-libraries.yml
.github\workflows\ci-md.yml = .github\workflows\ci-md.yml
Expand Down Expand Up @@ -121,6 +122,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test", "test", "{D2E73927-5
ProjectSection(SolutionItems) = preProject
test\Directory.Build.props = test\Directory.Build.props
test\Directory.Build.targets = test\Directory.Build.targets
test\Directory.Packages.props = test\Directory.Packages.props
EndProjectSection
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "OpenTelemetry.Instrumentation.Grpc.Tests", "test\OpenTelemetry.Instrumentation.Grpc.Tests\OpenTelemetry.Instrumentation.Grpc.Tests.csproj", "{305E9DFD-E73B-4A28-8769-795C25551020}"
Expand All @@ -146,6 +148,7 @@ EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "examples", "examples", "{2C7DD1DA-C229-4D9E-9AF0-BCD5CD3E4948}"
ProjectSection(SolutionItems) = preProject
examples\Directory.Build.props = examples\Directory.Build.props
examples\Directory.Packages.props = examples\Directory.Packages.props
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "trace", "trace", "{5B7FB835-3FFF-4BC2-99C5-A5B5FAE3C818}"
Expand Down Expand Up @@ -176,6 +179,7 @@ EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "docs", "docs", "{CB401DF1-FF5C-4055-886E-1183E832B2D6}"
ProjectSection(SolutionItems) = preProject
docs\Directory.Build.props = docs\Directory.Build.props
docs\Directory.Packages.props = docs\Directory.Packages.props
docs\docfx.json = docs\docfx.json
docs\toc.yml = docs\toc.yml
EndProjectSection
Expand Down
34 changes: 34 additions & 0 deletions build/test-threadSafety.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
param(
[Parameter()][string]$coyoteVersion="1.7.10",
[Parameter(Mandatory=$true)][string]$testProjectName,
[Parameter(Mandatory=$true)][string]$targetFramework,
[Parameter()][string]$categoryName="CoyoteConcurrencyTests",
[Parameter()][string]$configuration="Release"
)

$env:OTEL_RUN_COYOTE_TESTS = 'true'

$rootDirectory = Split-Path $PSScriptRoot -Parent

Write-Host "Install Coyote CLI."
dotnet tool install --global Microsoft.Coyote.CLI

Write-Host "Build $testProjectName project."
dotnet build "$rootDirectory/test/$testProjectName/$testProjectName.csproj" --configuration $configuration

$artifactsPath = Join-Path $rootDirectory "test/$testProjectName/bin/$configuration/$targetFramework"

Write-Host "Generate Coyote rewriting options JSON file."
Yun-Ting marked this conversation as resolved.
Show resolved Hide resolved
$assemblies = Get-ChildItem $artifactsPath -Filter OpenTelemetry*.dll | ForEach-Object {$_.Name}

$RewriteOptionsJson = @{}
[void]$RewriteOptionsJson.Add("AssembliesPath", $artifactsPath)
[void]$RewriteOptionsJson.Add("Assemblies", $assemblies)
$RewriteOptionsJson | ConvertTo-Json -Compress | Set-Content -Path "$rootDirectory/test/$testProjectName/rewrite.coyote.json"

Write-Host "Run Coyote rewrite."
coyote rewrite "$rootDirectory/test/$testProjectName/rewrite.coyote.json"

Write-Host "Execute re-written binary."
dotnet test "$artifactsPath/$testProjectName.dll" --framework $targetFramework --filter CategoryName=$categoryName

2 changes: 1 addition & 1 deletion examples/Directory.Packages.props
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<Project>
<Import Project="$([MSBuild]::GetPathOfFileAbove(Directory.Packages.props, $(MSBuildThisFileDirectory)..))" />
<ItemGroup>
<PackageVersion Update="System.Text.Json" Version="6.0.5" />
<PackageVersion Update="System.Text.Json" Version="7.0.1" />
Yun-Ting marked this conversation as resolved.
Show resolved Hide resolved
</ItemGroup>
</Project>
10 changes: 5 additions & 5 deletions src/OpenTelemetry.Api/Trace/TracerProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace OpenTelemetry.Trace;
/// </summary>
public class TracerProvider : BaseProvider
{
private ConcurrentDictionary<TracerKey, Tracer>? tracers = new();
internal ConcurrentDictionary<TracerKey, Tracer>? Tracers = new();

/// <summary>
/// Initializes a new instance of the <see cref="TracerProvider"/> class.
Expand All @@ -55,7 +55,7 @@ public Tracer GetTracer(
string name,
string? version = null)
{
var tracers = this.tracers;
var tracers = this.Tracers;
if (tracers == null)
{
// Note: Returns a no-op Tracer once dispose has been called.
Expand All @@ -68,7 +68,7 @@ public Tracer GetTracer(
{
lock (tracers)
{
if (this.tracers == null)
if (this.Tracers == null)
{
// Note: We check here for a race with Dispose and return a
// no-op Tracer in that case.
Expand All @@ -93,7 +93,7 @@ protected override void Dispose(bool disposing)
{
if (disposing)
{
var tracers = Interlocked.CompareExchange(ref this.tracers, null, this.tracers);
var tracers = Interlocked.CompareExchange(ref this.Tracers, null, this.Tracers);
if (tracers != null)
{
lock (tracers)
Expand All @@ -114,7 +114,7 @@ protected override void Dispose(bool disposing)
base.Dispose(disposing);
}

private readonly record struct TracerKey
internal readonly record struct TracerKey
{
public readonly string Name;
public readonly string? Version;
Expand Down
3 changes: 2 additions & 1 deletion test/Directory.Packages.props
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
<Project>
<Import Project="$([MSBuild]::GetPathOfFileAbove(Directory.Packages.props, $(MSBuildThisFileDirectory)..))" />
<ItemGroup>
<PackageVersion Update="System.Text.Json" Version="6.0.5" />
<PackageVersion Update="System.Text.Json" Version="7.0.1" />
<PackageVersion Include="System.Runtime.InteropServices.RuntimeInformation" Version="4.3.0" />
<PackageVersion Include="Microsoft.Coyote" Version="1.7.10" />
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
</ItemGroup>
</Project>
2 changes: 2 additions & 0 deletions test/OpenTelemetry.Api.Tests/OpenTelemetry.Api.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

<ItemGroup>
<Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Shared\EventSourceTestHelper.cs" Link="Includes\EventSourceTestHelper.cs" />
<Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Shared\SkipUnlessEnvVarFoundFactAttribute.cs" Link="Includes\SkipUnlessEnvVarFoundFactAttribute.cs" />
<Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Shared\TestEventListener.cs" Link="Includes\TestEventListener.cs" />
<Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Shared\Utils.cs" Link="Includes\Utils.cs" />
</ItemGroup>
Expand All @@ -25,5 +26,6 @@
<PackageReference Include="xunit.runner.visualstudio" PrivateAssets="All">
<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
</PackageReference>
<PackageReference Include="Microsoft.Coyote" />
</ItemGroup>
</Project>
101 changes: 99 additions & 2 deletions test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,22 @@
// </copyright>

using System.Diagnostics;
using Microsoft.Coyote;
using Microsoft.Coyote.SystematicTesting;
using OpenTelemetry.Tests;
using Xunit;
using Xunit.Abstractions;

namespace OpenTelemetry.Trace.Tests;

public class TracerTest : IDisposable
{
// TODO: This is only a basic test. This must cover the entire shim API scenarios.
private readonly ITestOutputHelper output;
private readonly Tracer tracer;

public TracerTest()
public TracerTest(ITestOutputHelper output)
{
this.output = output;
this.tracer = TracerProvider.Default.GetTracer("tracername", "tracerversion");
}

Expand Down Expand Up @@ -309,6 +314,91 @@ public void TracerBecomesNoopWhenParentProviderIsDisposedTest()
Assert.False(span3.IsRecording);
}

[SkipUnlessEnvVarFoundFact("OTEL_RUN_COYOTE_TESTS")]
[Trait("CategoryName", "CoyoteConcurrencyTests")]
public void TracerConcurrencyTest()
{
var config = Configuration.Create()
.WithTestingIterations(100)
.WithMemoryAccessRaceCheckingEnabled(true);

var test = TestingEngine.Create(config, InnerTest);

test.Run();

this.output.WriteLine(test.GetReport());
this.output.WriteLine($"Bugs, if any: {string.Join("\n", test.TestReport.BugReports)}");

var dir = Directory.GetCurrentDirectory();
if (test.TryEmitReports(dir, $"{nameof(this.TracerConcurrencyTest)}_CoyoteOutput", out IEnumerable<string> reportPaths))
{
foreach (var reportPath in reportPaths)
{
this.output.WriteLine($"Execution Report: {reportPath}");
}
}

if (test.TryEmitCoverageReports(dir, $"{nameof(this.TracerConcurrencyTest)}_CoyoteOutput", out reportPaths))
{
foreach (var reportPath in reportPaths)
{
this.output.WriteLine($"Coverage report: {reportPath}");
}
}

Assert.Equal(0, test.TestReport.NumOfFoundBugs);

static void InnerTest()
{
var testTracerProvider = new TestTracerProvider
{
ExpectedNumberOfThreads = Math.Max(1, Environment.ProcessorCount / 2),
};

var tracers = testTracerProvider.Tracers;

Assert.NotNull(tracers);

Thread[] getTracerThreads = new Thread[testTracerProvider.ExpectedNumberOfThreads];
for (int i = 0; i < testTracerProvider.ExpectedNumberOfThreads; i++)
{
getTracerThreads[i] = new Thread((object state) =>
{
var testTracerProvider = state as TestTracerProvider;

var id = Interlocked.Increment(ref testTracerProvider.NumberOfThreads);
var name = $"Tracer{id}";

if (id == testTracerProvider.ExpectedNumberOfThreads)
{
testTracerProvider.StartHandle.Set();
}
else
{
testTracerProvider.StartHandle.WaitOne();
}

var tracer = testTracerProvider.GetTracer(name);

Assert.NotNull(tracer);
});

getTracerThreads[i].Start(testTracerProvider);
}

testTracerProvider.StartHandle.WaitOne();

testTracerProvider.Dispose();

foreach (var getTracerThread in getTracerThreads)
{
getTracerThread.Join();
}

Assert.Empty(tracers);
}
}

public void Dispose()
{
Activity.Current = null;
Expand All @@ -319,4 +409,11 @@ private static bool IsNoopSpan(TelemetrySpan span)
{
return span.Activity == null;
}

private sealed class TestTracerProvider : TracerProvider
{
public int ExpectedNumberOfThreads;
public int NumberOfThreads;
public EventWaitHandle StartHandle = new ManualResetEvent(false);
}
}
Loading