Skip to content

Commit

Permalink
Merge branch 'main' into alanwest/fix-activity-http-route
Browse files Browse the repository at this point in the history
  • Loading branch information
utpilla authored Nov 17, 2023
2 parents 8ac2cf5 + 2a228f9 commit dbb9e37
Show file tree
Hide file tree
Showing 25 changed files with 1,162 additions and 545 deletions.
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'
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."
$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

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
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,6 @@ private static void AddResponseTags(HttpWebResponse response, Activity activity)
activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode));
}

activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)response.StatusCode));

try
{
TracingOptions.EnrichWithHttpWebResponse?.Invoke(activity, response);
Expand Down Expand Up @@ -243,45 +241,15 @@ private static string GetErrorType(Exception exception)
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void AddExceptionTags(Exception exception, Activity activity, out HttpStatusCode? statusCode, out Version protocolVersion)
private static void AddExceptionEvent(Exception exception, Activity activity)
{
Debug.Assert(activity != null, "Activity must not be null");

statusCode = null;
protocolVersion = null;

if (!activity.IsAllDataRequested)
{
return;
}

ActivityStatusCode status;

if (exception is WebException wexc && wexc.Response is HttpWebResponse response)
{
statusCode = response.StatusCode;
protocolVersion = response.ProtocolVersion;

if (tracingEmitOldAttributes)
{
activity.SetTag(SemanticConventions.AttributeHttpStatusCode, (int)statusCode);
}

if (tracingEmitNewAttributes)
{
activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(protocolVersion));
activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, (int)statusCode);
}

status = SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)statusCode);
}
else
{
status = ActivityStatusCode.Error;
}

activity.SetStatus(status);

if (TracingOptions.RecordException)
{
activity.RecordException(exception);
Expand Down Expand Up @@ -401,6 +369,7 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC
HttpStatusCode? httpStatusCode = null;
string errorType = null;
Version protocolVersion = null;
ActivityStatusCode activityStatus = ActivityStatusCode.Unset;

// Activity may be null if we are not tracing in these cases:
// 1. No listeners
Expand All @@ -417,14 +386,30 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC
if (result is Exception ex)
{
errorType = GetErrorType(ex);
if (activity != null)
{
AddExceptionTags(ex, activity, out httpStatusCode, out protocolVersion);
}
else if (ex is WebException wexc && wexc.Response is HttpWebResponse response)
if (ex is WebException wexc && wexc.Response is HttpWebResponse response)
{
httpStatusCode = response.StatusCode;
protocolVersion = response.ProtocolVersion;
activityStatus = SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)response.StatusCode);
if (activityStatus == ActivityStatusCode.Error)
{
// override the errorType to statusCode for failures.
errorType = TelemetryHelper.GetStatusCodeString(response.StatusCode);
}

if (activity != null)
{
AddResponseTags(response, activity);
AddExceptionEvent(ex, activity);
}
}
else
{
activityStatus = ActivityStatusCode.Error;
if (activity != null)
{
AddExceptionEvent(ex, activity);
}
}
}
else
Expand Down Expand Up @@ -465,9 +450,11 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC
protocolVersion = response.ProtocolVersion;
}

if (SpanHelper.ResolveSpanStatusForHttpStatusCode(ActivityKind.Client, (int)httpStatusCode.Value) == ActivityStatusCode.Error)
activityStatus = SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)httpStatusCode.Value);

if (activityStatus == ActivityStatusCode.Error)
{
// override the errorType to statusCode for failures.
// set the errorType to statusCode for failures.
errorType = TelemetryHelper.GetStatusCodeString(httpStatusCode.Value);
}
}
Expand All @@ -477,9 +464,13 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC
HttpInstrumentationEventSource.Log.FailedProcessResult(ex);
}

if (tracingEmitNewAttributes && errorType != null)
if (activity != null && activity.IsAllDataRequested)
{
activity?.SetTag(SemanticConventions.AttributeErrorType, errorType);
activity.SetStatus(activityStatus);
if (tracingEmitNewAttributes && errorType != null)
{
activity.SetTag(SemanticConventions.AttributeErrorType, errorType);
}
}

activity?.Stop();
Expand Down
7 changes: 7 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@
`8.0.0`.
([#5051](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5051))

* Revert the default behavior of Metrics SDK for Delta aggregation. It would not
reclaim unused Metric Points by default. You can enable the SDK to reclaim
unused Metric Points by setting the environment variable
`OTEL_DOTNET_EXPERIMENTAL_METRICS_RECLAIM_UNUSED_METRIC_POINTS` to `true`
before setting up the `MeterProvider`.
([#5052](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5052))

## 1.7.0-alpha.1

Released 2023-Oct-16
Expand Down
8 changes: 6 additions & 2 deletions src/OpenTelemetry/Metrics/AggregatorStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ namespace OpenTelemetry.Metrics;
internal sealed class AggregatorStore
{
internal readonly bool OutputDelta;
internal readonly bool ShouldReclaimUnusedMetricPoints;
internal long DroppedMeasurements = 0;

private static readonly string MetricPointCapHitFixMessage = "Consider opting in for the experimental SDK feature to emit all the throttled metrics under the overflow attribute by setting env variable OTEL_DOTNET_EXPERIMENTAL_METRICS_EMIT_OVERFLOW_ATTRIBUTE = true. You could also modify instrumentation to reduce the number of unique key/value pair combinations. Or use Views to drop unwanted tags. Or use MeterProviderBuilder.SetMaxMetricPointsPerMetricStream to set higher limit.";
Expand Down Expand Up @@ -81,6 +82,7 @@ internal AggregatorStore(
AggregationTemporality temporality,
int maxMetricPoints,
bool emitOverflowAttribute,
bool shouldReclaimUnusedMetricPoints,
ExemplarFilter? exemplarFilter = null)
{
this.name = metricStreamIdentity.InstrumentName;
Expand Down Expand Up @@ -122,7 +124,9 @@ internal AggregatorStore(
reservedMetricPointsCount++;
}

if (this.OutputDelta)
this.ShouldReclaimUnusedMetricPoints = shouldReclaimUnusedMetricPoints;

if (this.OutputDelta && shouldReclaimUnusedMetricPoints)
{
this.availableMetricPoints = new Queue<int>(maxMetricPoints - reservedMetricPointsCount);

Expand Down Expand Up @@ -181,7 +185,7 @@ internal int Snapshot()
this.batchSize = 0;
if (this.OutputDelta)
{
if (this.reclaimMetricPoints)
if (this.ShouldReclaimUnusedMetricPoints && this.reclaimMetricPoints)
{
this.SnapshotDeltaWithMetricPointReclaim();
}
Expand Down
Loading

0 comments on commit dbb9e37

Please sign in to comment.