diff --git a/.github/workflows/ci-concurrency-md.yml b/.github/workflows/ci-concurrency-md.yml new file mode 100644 index 00000000000..f6898c7f9b2 --- /dev/null +++ b/.github/workflows/ci-concurrency-md.yml @@ -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"' diff --git a/.github/workflows/ci-concurrency.yml b/.github/workflows/ci-concurrency.yml new file mode 100644 index 00000000000..82060ae39f2 --- /dev/null +++ b/.github/workflows/ci-concurrency.yml @@ -0,0 +1,41 @@ +name: Coyote Concurrency Tests + +on: + push: + branches: [ 'main*' ] + paths-ignore: + - '**.md' + 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.*' diff --git a/.gitignore b/.gitignore index af409279810..e06229e460f 100644 --- a/.gitignore +++ b/.gitignore @@ -348,3 +348,6 @@ ASALocalRun/ # Tempo files tempo-data/ + +# Coyote Rewrite Files +rewrite.coyote.json diff --git a/OpenTelemetry.sln b/OpenTelemetry.sln index 78fbafb7463..337b5953028 100644 --- a/OpenTelemetry.sln +++ b/OpenTelemetry.sln @@ -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 @@ -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 @@ -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 @@ -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}" @@ -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}" @@ -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 diff --git a/build/test-threadSafety.ps1 b/build/test-threadSafety.ps1 new file mode 100644 index 00000000000..6694870b6b4 --- /dev/null +++ b/build/test-threadSafety.ps1 @@ -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." +$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 + diff --git a/src/OpenTelemetry.Api/Trace/TracerProvider.cs b/src/OpenTelemetry.Api/Trace/TracerProvider.cs index 50d033cae51..f293b843c62 100644 --- a/src/OpenTelemetry.Api/Trace/TracerProvider.cs +++ b/src/OpenTelemetry.Api/Trace/TracerProvider.cs @@ -28,7 +28,7 @@ namespace OpenTelemetry.Trace; /// public class TracerProvider : BaseProvider { - private ConcurrentDictionary? tracers = new(); + internal ConcurrentDictionary? Tracers = new(); /// /// Initializes a new instance of the class. @@ -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. @@ -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. @@ -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) @@ -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; diff --git a/test/Directory.Packages.props b/test/Directory.Packages.props index 91bbe459e06..575224321a8 100644 --- a/test/Directory.Packages.props +++ b/test/Directory.Packages.props @@ -1,7 +1,8 @@ - + + diff --git a/test/OpenTelemetry.Api.Tests/OpenTelemetry.Api.Tests.csproj b/test/OpenTelemetry.Api.Tests/OpenTelemetry.Api.Tests.csproj index 3b5edd93cb3..31c6c8eb3db 100644 --- a/test/OpenTelemetry.Api.Tests/OpenTelemetry.Api.Tests.csproj +++ b/test/OpenTelemetry.Api.Tests/OpenTelemetry.Api.Tests.csproj @@ -14,6 +14,7 @@ + @@ -25,5 +26,6 @@ runtime; build; native; contentfiles; analyzers + diff --git a/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs b/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs index c38cdb46ec9..3dfde198017 100644 --- a/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs +++ b/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs @@ -15,17 +15,22 @@ // 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"); } @@ -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 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; @@ -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); + } } diff --git a/test/OpenTelemetry.Tests/Concurrency/MetricsConcurrencyTests.cs b/test/OpenTelemetry.Tests/Concurrency/MetricsConcurrencyTests.cs new file mode 100644 index 00000000000..65eb44c8145 --- /dev/null +++ b/test/OpenTelemetry.Tests/Concurrency/MetricsConcurrencyTests.cs @@ -0,0 +1,70 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using Microsoft.Coyote; +using Microsoft.Coyote.SystematicTesting; +using OpenTelemetry.Metrics.Tests; +using Xunit; +using Xunit.Abstractions; + +namespace OpenTelemetry.Tests.Concurrency; + +public class MetricsConcurrencyTests +{ + private readonly ITestOutputHelper output; + private readonly AggregatorTests aggregatorTests; + + public MetricsConcurrencyTests(ITestOutputHelper output) + { + this.output = output; + this.aggregatorTests = new AggregatorTests(); + } + + [SkipUnlessEnvVarFoundFact("OTEL_RUN_COYOTE_TESTS")] + [Trait("CategoryName", "CoyoteConcurrencyTests")] + public void MultithreadedLongHistogramTestConcurrencyTest() + { + var config = Configuration.Create() + .WithTestingIterations(100) + .WithMemoryAccessRaceCheckingEnabled(true); + + var test = TestingEngine.Create(config, this.aggregatorTests.MultiThreadedHistogramUpdateAndSnapShotTest); + + 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.MultithreadedLongHistogramTestConcurrencyTest)}_CoyoteOutput", out var reportPaths)) + { + foreach (var reportPath in reportPaths) + { + this.output.WriteLine($"Execution Report: {reportPath}"); + } + } + + if (test.TryEmitCoverageReports(dir, $"{nameof(this.MultithreadedLongHistogramTestConcurrencyTest)}_CoyoteOutput", out reportPaths)) + { + foreach (var reportPath in reportPaths) + { + this.output.WriteLine($"Coverage report: {reportPath}"); + } + } + + Assert.Equal(0, test.TestReport.NumOfFoundBugs); + } +} diff --git a/test/OpenTelemetry.Tests/OpenTelemetry.Tests.csproj b/test/OpenTelemetry.Tests/OpenTelemetry.Tests.csproj index d8ad801f561..e7b5c8a03e7 100644 --- a/test/OpenTelemetry.Tests/OpenTelemetry.Tests.csproj +++ b/test/OpenTelemetry.Tests/OpenTelemetry.Tests.csproj @@ -36,5 +36,6 @@ runtime; build; native; contentfiles; analyzers +