From a619d53ea704f2e359b7f811ed440ba704516c80 Mon Sep 17 00:00:00 2001 From: Pan Shao Date: Mon, 30 May 2022 13:40:35 +0800 Subject: [PATCH 01/15] Add scope of waitForCompletion in LRO --- .../src/Shared/OperationInternalBase.cs | 28 +++++++++++++------ .../src/Shared/OperationInternalOfT.cs | 2 +- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs index e4d7d6edea0f..bb8fe7993498 100644 --- a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs +++ b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs @@ -16,6 +16,7 @@ internal abstract class OperationInternalBase { private readonly ClientDiagnostics _diagnostics; private readonly string _updateStatusScopeName; + private readonly string _waitForCompletionScopeName; private readonly IReadOnlyDictionary? _scopeAttributes; private readonly DelayStrategy? _fallbackStrategy; private readonly AsyncLockWithValue _responseLock; @@ -24,6 +25,7 @@ protected OperationInternalBase(Response rawResponse) { _diagnostics = new ClientDiagnostics(ClientOptions.Default); _updateStatusScopeName = string.Empty; + _waitForCompletionScopeName = string.Empty; _scopeAttributes = default; _fallbackStrategy = default; _responseLock = new AsyncLockWithValue(rawResponse); @@ -33,6 +35,7 @@ protected OperationInternalBase(ClientDiagnostics clientDiagnostics, string oper { _diagnostics = clientDiagnostics; _updateStatusScopeName = $"{operationTypeName}.UpdateStatus"; + _waitForCompletionScopeName = $"{operationTypeName}.WaitForCompletion"; _scopeAttributes = scopeAttributes?.ToDictionary(kvp => kvp.Key, kvp => kvp.Value); _fallbackStrategy = fallbackStrategy; _responseLock = new AsyncLockWithValue(); @@ -188,20 +191,29 @@ private async ValueTask WaitForCompletionResponseAsync(bool async, Tim return lockOrValue.Value; } - var poller = new OperationPoller(_fallbackStrategy); - var response = async - ? await poller.WaitForCompletionResponseAsync(this, pollingInterval, cancellationToken).ConfigureAwait(false) - : poller.WaitForCompletionResponse(this, pollingInterval, cancellationToken); + using var scope = CreateScope(false); + try + { + var poller = new OperationPoller(_fallbackStrategy); + var response = async + ? await poller.WaitForCompletionResponseAsync(this, pollingInterval, cancellationToken).ConfigureAwait(false) + : poller.WaitForCompletionResponse(this, pollingInterval, cancellationToken); - lockOrValue.SetValue(response); - return response; + lockOrValue.SetValue(response); + return response; + } + catch (Exception e) + { + scope.Failed(e); + throw; + } } protected abstract ValueTask UpdateStatusAsync(bool async, CancellationToken cancellationToken); - protected DiagnosticScope CreateScope() + protected DiagnosticScope CreateScope(bool isUpdateStatus) { - DiagnosticScope scope = _diagnostics.CreateScope(_updateStatusScopeName); + DiagnosticScope scope = _diagnostics.CreateScope(isUpdateStatus ? _updateStatusScopeName : _waitForCompletionScopeName); if (_scopeAttributes != null) { diff --git a/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs b/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs index 5328f8b6ee14..1512515c4d33 100644 --- a/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs +++ b/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs @@ -260,7 +260,7 @@ protected override async ValueTask UpdateStatusAsync(bool async, Cance return GetResponseFromState(asyncLock.Value); } - using var scope = CreateScope(); + using var scope = CreateScope(true); try { var state = await _operation.UpdateStateAsync(async, cancellationToken).ConfigureAwait(false); From 4515f0ef645499ef1f62bb625b1ad9509c2b0c7d Mon Sep 17 00:00:00 2001 From: Pan Shao Date: Thu, 2 Jun 2022 14:57:04 +0800 Subject: [PATCH 02/15] Change flag to string --- .../src/Shared/OperationInternalBase.cs | 16 +++++++++++----- .../src/Shared/OperationInternalOfT.cs | 2 +- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs index bb8fe7993498..46984b8df469 100644 --- a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs +++ b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs @@ -15,12 +15,13 @@ namespace Azure.Core internal abstract class OperationInternalBase { private readonly ClientDiagnostics _diagnostics; - private readonly string _updateStatusScopeName; - private readonly string _waitForCompletionScopeName; private readonly IReadOnlyDictionary? _scopeAttributes; private readonly DelayStrategy? _fallbackStrategy; private readonly AsyncLockWithValue _responseLock; + protected readonly string _updateStatusScopeName; + private readonly string _waitForCompletionScopeName; + protected OperationInternalBase(Response rawResponse) { _diagnostics = new ClientDiagnostics(ClientOptions.Default); @@ -191,7 +192,7 @@ private async ValueTask WaitForCompletionResponseAsync(bool async, Tim return lockOrValue.Value; } - using var scope = CreateScope(false); + using var scope = CreateScope(_waitForCompletionScopeName); try { var poller = new OperationPoller(_fallbackStrategy); @@ -211,9 +212,14 @@ private async ValueTask WaitForCompletionResponseAsync(bool async, Tim protected abstract ValueTask UpdateStatusAsync(bool async, CancellationToken cancellationToken); - protected DiagnosticScope CreateScope(bool isUpdateStatus) + protected DiagnosticScope CreateScope(string scopeName) { - DiagnosticScope scope = _diagnostics.CreateScope(isUpdateStatus ? _updateStatusScopeName : _waitForCompletionScopeName); + if (scopeName != _updateStatusScopeName && scopeName != _waitForCompletionScopeName) + { + throw new NotSupportedException($"The scope name is not supported. Allowed value: {_updateStatusScopeName}, {_waitForCompletionScopeName}"); + } + + DiagnosticScope scope = _diagnostics.CreateScope(scopeName); if (_scopeAttributes != null) { diff --git a/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs b/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs index 1512515c4d33..f30699357263 100644 --- a/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs +++ b/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs @@ -260,7 +260,7 @@ protected override async ValueTask UpdateStatusAsync(bool async, Cance return GetResponseFromState(asyncLock.Value); } - using var scope = CreateScope(true); + using var scope = CreateScope(_updateStatusScopeName); try { var state = await _operation.UpdateStateAsync(async, cancellationToken).ConfigureAwait(false); From 55b5fb82ec83fb82bb98a802c907fb475f63290e Mon Sep 17 00:00:00 2001 From: pshao25 <97225342+pshao25@users.noreply.github.com> Date: Mon, 6 Jun 2022 14:41:05 +0800 Subject: [PATCH 03/15] Resolve comments --- .../src/Shared/OperationInternalBase.cs | 16 ++++------------ .../src/Shared/OperationInternalOfT.cs | 2 +- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs index 46984b8df469..ebc852b6592c 100644 --- a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs +++ b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs @@ -19,14 +19,12 @@ internal abstract class OperationInternalBase private readonly DelayStrategy? _fallbackStrategy; private readonly AsyncLockWithValue _responseLock; - protected readonly string _updateStatusScopeName; - private readonly string _waitForCompletionScopeName; + protected readonly string _operationTypeName; protected OperationInternalBase(Response rawResponse) { _diagnostics = new ClientDiagnostics(ClientOptions.Default); - _updateStatusScopeName = string.Empty; - _waitForCompletionScopeName = string.Empty; + _operationTypeName = string.Empty; _scopeAttributes = default; _fallbackStrategy = default; _responseLock = new AsyncLockWithValue(rawResponse); @@ -35,8 +33,7 @@ protected OperationInternalBase(Response rawResponse) protected OperationInternalBase(ClientDiagnostics clientDiagnostics, string operationTypeName, IEnumerable>? scopeAttributes = null, DelayStrategy? fallbackStrategy = null) { _diagnostics = clientDiagnostics; - _updateStatusScopeName = $"{operationTypeName}.UpdateStatus"; - _waitForCompletionScopeName = $"{operationTypeName}.WaitForCompletion"; + _operationTypeName = operationTypeName; _scopeAttributes = scopeAttributes?.ToDictionary(kvp => kvp.Key, kvp => kvp.Value); _fallbackStrategy = fallbackStrategy; _responseLock = new AsyncLockWithValue(); @@ -192,7 +189,7 @@ private async ValueTask WaitForCompletionResponseAsync(bool async, Tim return lockOrValue.Value; } - using var scope = CreateScope(_waitForCompletionScopeName); + using var scope = CreateScope($"{_operationTypeName}.WaitForCompletionResponse"); try { var poller = new OperationPoller(_fallbackStrategy); @@ -214,11 +211,6 @@ private async ValueTask WaitForCompletionResponseAsync(bool async, Tim protected DiagnosticScope CreateScope(string scopeName) { - if (scopeName != _updateStatusScopeName && scopeName != _waitForCompletionScopeName) - { - throw new NotSupportedException($"The scope name is not supported. Allowed value: {_updateStatusScopeName}, {_waitForCompletionScopeName}"); - } - DiagnosticScope scope = _diagnostics.CreateScope(scopeName); if (_scopeAttributes != null) diff --git a/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs b/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs index f30699357263..4ee3ce6e31fc 100644 --- a/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs +++ b/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs @@ -260,7 +260,7 @@ protected override async ValueTask UpdateStatusAsync(bool async, Cance return GetResponseFromState(asyncLock.Value); } - using var scope = CreateScope(_updateStatusScopeName); + using var scope = CreateScope($"{_operationTypeName}.UpdateStatus"); try { var state = await _operation.UpdateStateAsync(async, cancellationToken).ConfigureAwait(false); From 96b69bb0a936cb599d1e1c28d25f8bd63ebd4a77 Mon Sep 17 00:00:00 2001 From: pshao25 <97225342+pshao25@users.noreply.github.com> Date: Tue, 7 Jun 2022 14:44:16 +0800 Subject: [PATCH 04/15] Resolve comments --- .../src/Shared/OperationInternalBase.cs | 2 +- .../src/Shared/OperationInternalOfT.cs | 2 +- .../tests/OperationInternalOfTTests.cs | 42 +++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs index ebc852b6592c..c8c9ac4d724c 100644 --- a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs +++ b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs @@ -189,7 +189,7 @@ private async ValueTask WaitForCompletionResponseAsync(bool async, Tim return lockOrValue.Value; } - using var scope = CreateScope($"{_operationTypeName}.WaitForCompletionResponse"); + using var scope = CreateScope(string.IsNullOrEmpty(_operationTypeName) ? string.Empty : $"{_operationTypeName}.WaitForCompletionResponse"); try { var poller = new OperationPoller(_fallbackStrategy); diff --git a/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs b/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs index 4ee3ce6e31fc..2dd220581849 100644 --- a/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs +++ b/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs @@ -260,7 +260,7 @@ protected override async ValueTask UpdateStatusAsync(bool async, Cance return GetResponseFromState(asyncLock.Value); } - using var scope = CreateScope($"{_operationTypeName}.UpdateStatus"); + using var scope = CreateScope(string.IsNullOrEmpty(_operationTypeName) ? string.Empty : $"{_operationTypeName}.UpdateStatus"); try { var state = await _operation.UpdateStateAsync(async, cancellationToken).ConfigureAwait(false); diff --git a/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs b/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs index 663bc58c958c..372b4ec239d3 100644 --- a/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs +++ b/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs @@ -148,6 +148,19 @@ public async Task UpdateStatusCreatesDiagnosticScope([Values(true, false)] bool testListener.AssertScope($"{expectedTypeName}.UpdateStatus", expectedAttributes); } + [Test] + public async Task UpdateStatusNotCreateDiagnosticScope([Values(true, false)] bool async) + { + using ClientDiagnosticListener testListener = new(DiagnosticNamespace); + + var operationInternal = OperationInternal.Succeeded(InitialResponse, 1); + _ = async + ? await operationInternal.UpdateStatusAsync(CancellationToken.None) + : operationInternal.UpdateStatus(CancellationToken.None); + + CollectionAssert.IsEmpty(testListener.Scopes); + } + [Test] public async Task UpdateStatusSetsFailedScopeWhenOperationFails([Values(true, false)] bool async) { @@ -205,6 +218,35 @@ public async Task UpdateStatusPassesTheCancellationTokenToUpdateState([Values(tr Assert.AreEqual(originalToken, passedToken); } + [Test] + public async Task WaitForCompletionResponseCreatesDiagnosticScope([Values(true, false)] bool async, [Values(null, "CustomTypeName")] string operationTypeName) + { + using ClientDiagnosticListener testListener = new(DiagnosticNamespace); + + string expectedTypeName = operationTypeName ?? nameof(TestOperation); + KeyValuePair[] expectedAttributes = { new("key1", "value1"), new("key2", "value2") }; + var operationInternal = new OperationInternal(ClientDiagnostics, TestOperation.Succeeded(1), InitialResponse, operationTypeName, expectedAttributes); + + _ = async + ? await operationInternal.WaitForCompletionResponseAsync(CancellationToken.None) + : operationInternal.WaitForCompletionResponse(CancellationToken.None); + + testListener.AssertScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); + } + + [Test] + public async Task WaitForCompletionResponseNotCreateDiagnosticScope([Values(true, false)] bool async) + { + using ClientDiagnosticListener testListener = new(DiagnosticNamespace); + + var operationInternal = OperationInternal.Succeeded(InitialResponse, 1); + _ = async + ? await operationInternal.WaitForCompletionResponseAsync(CancellationToken.None) + : operationInternal.WaitForCompletionResponse(CancellationToken.None); + + CollectionAssert.IsEmpty(testListener.Scopes); + } + [Test] public async Task WaitForCompletionCallsUntilOperationCompletes([Values(true, false)] bool useDefaultPollingInterval) { From eb9fcf321c09beb4754581d160fdbde2d077f9bd Mon Sep 17 00:00:00 2001 From: pshao25 <97225342+pshao25@users.noreply.github.com> Date: Wed, 8 Jun 2022 15:58:19 +0800 Subject: [PATCH 05/15] Add tests --- .../tests/OperationInternalOfTTests.cs | 29 +++++++++++++ .../tests/OperationInternalTests.cs | 42 +++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs b/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs index 372b4ec239d3..8f00f45126cd 100644 --- a/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs +++ b/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs @@ -234,6 +234,22 @@ public async Task WaitForCompletionResponseCreatesDiagnosticScope([Values(true, testListener.AssertScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); } + [Test] + public async Task WaitForCompletionCreatesDiagnosticScope([Values(true, false)] bool async, [Values(null, "CustomTypeName")] string operationTypeName) + { + using ClientDiagnosticListener testListener = new(DiagnosticNamespace); + + string expectedTypeName = operationTypeName ?? nameof(TestOperation); + KeyValuePair[] expectedAttributes = { new("key1", "value1"), new("key2", "value2") }; + var operationInternal = new OperationInternal(ClientDiagnostics, TestOperation.Succeeded(1), InitialResponse, operationTypeName, expectedAttributes); + + _ = async + ? await operationInternal.WaitForCompletionAsync(CancellationToken.None) + : operationInternal.WaitForCompletion(CancellationToken.None); + + testListener.AssertScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); + } + [Test] public async Task WaitForCompletionResponseNotCreateDiagnosticScope([Values(true, false)] bool async) { @@ -247,6 +263,19 @@ public async Task WaitForCompletionResponseNotCreateDiagnosticScope([Values(true CollectionAssert.IsEmpty(testListener.Scopes); } + [Test] + public async Task WaitForCompletionNotCreateDiagnosticScope([Values(true, false)] bool async) + { + using ClientDiagnosticListener testListener = new(DiagnosticNamespace); + + var operationInternal = OperationInternal.Succeeded(InitialResponse, 1); + _ = async + ? await operationInternal.WaitForCompletionAsync(CancellationToken.None) + : operationInternal.WaitForCompletion(CancellationToken.None); + + CollectionAssert.IsEmpty(testListener.Scopes); + } + [Test] public async Task WaitForCompletionCallsUntilOperationCompletes([Values(true, false)] bool useDefaultPollingInterval) { diff --git a/sdk/core/Azure.Core/tests/OperationInternalTests.cs b/sdk/core/Azure.Core/tests/OperationInternalTests.cs index a2eb7298cfca..35869c056307 100644 --- a/sdk/core/Azure.Core/tests/OperationInternalTests.cs +++ b/sdk/core/Azure.Core/tests/OperationInternalTests.cs @@ -130,6 +130,19 @@ public async Task UpdateStatusCreatesDiagnosticScope([Values(true, false)] bool testListener.AssertScope($"{expectedTypeName}.UpdateStatus", expectedAttributes); } + [Test] + public async Task UpdateStatusNotCreateDiagnosticScope([Values(true, false)] bool async) + { + using ClientDiagnosticListener testListener = new(DiagnosticNamespace); + + var operationInternal = OperationInternal.Succeeded(InitialResponse); + _ = async + ? await operationInternal.UpdateStatusAsync(CancellationToken.None) + : operationInternal.UpdateStatus(CancellationToken.None); + + CollectionAssert.IsEmpty(testListener.Scopes); + } + [Test] public async Task UpdateStatusSetsFailedScopeWhenOperationFails([Values(true, false)] bool async) { @@ -187,6 +200,35 @@ public async Task UpdateStatusPassesTheCancellationTokenToUpdateState([Values(tr Assert.AreEqual(originalToken, passedToken); } + [Test] + public async Task WaitForCompletionResponseCreatesDiagnosticScope([Values(true, false)] bool async, [Values(null, "CustomTypeName")] string operationTypeName) + { + using ClientDiagnosticListener testListener = new(DiagnosticNamespace); + + string expectedTypeName = operationTypeName ?? nameof(TestOperation); + KeyValuePair[] expectedAttributes = { new("key1", "value1"), new("key2", "value2") }; + var operationInternal = new OperationInternal(ClientDiagnostics, TestOperation.Succeeded(), InitialResponse, operationTypeName, expectedAttributes); + + _ = async + ? await operationInternal.WaitForCompletionResponseAsync(CancellationToken.None) + : operationInternal.WaitForCompletionResponse(CancellationToken.None); + + testListener.AssertScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); + } + + [Test] + public async Task WaitForCompletionResponseNotCreateDiagnosticScope([Values(true, false)] bool async) + { + using ClientDiagnosticListener testListener = new(DiagnosticNamespace); + + var operationInternal = OperationInternal.Succeeded(InitialResponse); + _ = async + ? await operationInternal.WaitForCompletionResponseAsync(CancellationToken.None) + : operationInternal.WaitForCompletionResponse(CancellationToken.None); + + CollectionAssert.IsEmpty(testListener.Scopes); + } + [Test] public async Task WaitForCompletionCallsUntilOperationCompletes([Values(true, false)] bool useDefaultPollingInterval) { From 29d6ca305d53b361ec6900c74793ab3efdd52e20 Mon Sep 17 00:00:00 2001 From: pshao25 <97225342+pshao25@users.noreply.github.com> Date: Thu, 9 Jun 2022 17:10:13 +0800 Subject: [PATCH 06/15] Resolve comments --- .../src/Shared/OperationInternalBase.cs | 18 ++++++--- .../src/Shared/OperationInternalOfT.cs | 40 ++++++++++++------- .../tests/OperationInternalOfTTests.cs | 4 +- 3 files changed, 40 insertions(+), 22 deletions(-) diff --git a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs index c8c9ac4d724c..a7f0e6e633e1 100644 --- a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs +++ b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs @@ -19,12 +19,13 @@ internal abstract class OperationInternalBase private readonly DelayStrategy? _fallbackStrategy; private readonly AsyncLockWithValue _responseLock; - protected readonly string _operationTypeName; + private readonly string? _waitForCompletionResponseScopeName; + protected readonly string? _updateStatusScopeName; + protected readonly string? _waitForCompletionScopeName; protected OperationInternalBase(Response rawResponse) { _diagnostics = new ClientDiagnostics(ClientOptions.Default); - _operationTypeName = string.Empty; _scopeAttributes = default; _fallbackStrategy = default; _responseLock = new AsyncLockWithValue(rawResponse); @@ -33,7 +34,9 @@ protected OperationInternalBase(Response rawResponse) protected OperationInternalBase(ClientDiagnostics clientDiagnostics, string operationTypeName, IEnumerable>? scopeAttributes = null, DelayStrategy? fallbackStrategy = null) { _diagnostics = clientDiagnostics; - _operationTypeName = operationTypeName; + _updateStatusScopeName = $"{operationTypeName}.UpdateStatus"; + _waitForCompletionResponseScopeName = $"{operationTypeName}.WaitForCompletionResponse"; + _waitForCompletionScopeName = $"{operationTypeName}.WaitForCompletion"; _scopeAttributes = scopeAttributes?.ToDictionary(kvp => kvp.Key, kvp => kvp.Value); _fallbackStrategy = fallbackStrategy; _responseLock = new AsyncLockWithValue(); @@ -179,7 +182,7 @@ public Response WaitForCompletionResponse(CancellationToken cancellationToken) public Response WaitForCompletionResponse(TimeSpan pollingInterval, CancellationToken cancellationToken) => WaitForCompletionResponseAsync(async: false, pollingInterval, cancellationToken).EnsureCompleted(); - private async ValueTask WaitForCompletionResponseAsync(bool async, TimeSpan? pollingInterval, CancellationToken cancellationToken) + protected async ValueTask WaitForCompletionResponseAsync(bool async, TimeSpan? pollingInterval, CancellationToken cancellationToken) { // If _responseLock has the value, lockOrValue will contain that value, and no lock is acquired. // If _responseLock doesn't have the value, GetLockOrValueAsync will acquire the lock that will be released when lockOrValue is disposed @@ -189,7 +192,7 @@ private async ValueTask WaitForCompletionResponseAsync(bool async, Tim return lockOrValue.Value; } - using var scope = CreateScope(string.IsNullOrEmpty(_operationTypeName) ? string.Empty : $"{_operationTypeName}.WaitForCompletionResponse"); + using var scope = CreateScope(_waitForCompletionResponseScopeName!); try { var poller = new OperationPoller(_fallbackStrategy); @@ -228,5 +231,10 @@ protected DiagnosticScope CreateScope(string scopeName) protected async ValueTask CreateException(bool async, Response response) => async ? await _diagnostics.CreateRequestFailedExceptionAsync(response).ConfigureAwait(false) : _diagnostics.CreateRequestFailedException(response); + + protected bool TryGetResponseValue(out Response? value) + { + return _responseLock.TryGetValue(out value); + } } } diff --git a/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs b/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs index 2dd220581849..ea9fa60843ff 100644 --- a/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs +++ b/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs @@ -174,10 +174,7 @@ public T Value /// The last HTTP response received from the server, including the final result of the long-running operation. /// Thrown if there's been any issues during the connection, or if the operation has completed with failures. public async ValueTask> WaitForCompletionAsync(CancellationToken cancellationToken) - { - var rawResponse = await WaitForCompletionResponseAsync(cancellationToken).ConfigureAwait(false); - return Response.FromValue(Value, rawResponse); - } + => await WaitForCompletionAsync(async: true, null, cancellationToken).ConfigureAwait(false); /// /// Periodically calls until the long-running operation completes. The interval @@ -198,10 +195,7 @@ public async ValueTask> WaitForCompletionAsync(CancellationToken can /// The last HTTP response received from the server, including the final result of the long-running operation. /// Thrown if there's been any issues during the connection, or if the operation has completed with failures. public async ValueTask> WaitForCompletionAsync(TimeSpan pollingInterval, CancellationToken cancellationToken) - { - var rawResponse = await WaitForCompletionResponseAsync(pollingInterval, cancellationToken).ConfigureAwait(false); - return Response.FromValue(Value, rawResponse); - } + => await WaitForCompletionAsync(async: true, pollingInterval, cancellationToken).ConfigureAwait(false); /// /// Periodically calls until the long-running operation completes. @@ -220,10 +214,7 @@ public async ValueTask> WaitForCompletionAsync(TimeSpan pollingInter /// The last HTTP response received from the server, including the final result of the long-running operation. /// Thrown if there's been any issues during the connection, or if the operation has completed with failures. public Response WaitForCompletion(CancellationToken cancellationToken) - { - var rawResponse = WaitForCompletionResponse(cancellationToken); - return Response.FromValue(Value, rawResponse); - } + => WaitForCompletionAsync(async: false, null, cancellationToken).EnsureCompleted(); /// /// Periodically calls until the long-running operation completes. The interval @@ -244,9 +235,28 @@ public Response WaitForCompletion(CancellationToken cancellationToken) /// The last HTTP response received from the server, including the final result of the long-running operation. /// Thrown if there's been any issues during the connection, or if the operation has completed with failures. public Response WaitForCompletion(TimeSpan pollingInterval, CancellationToken cancellationToken) + => WaitForCompletionAsync(async: false, pollingInterval, cancellationToken).EnsureCompleted(); + + private async ValueTask> WaitForCompletionAsync(bool async, TimeSpan? pollingInterval, CancellationToken cancellationToken) { - var rawResponse = WaitForCompletionResponse(pollingInterval, cancellationToken); - return Response.FromValue(Value, rawResponse); + if (TryGetResponseValue(out var rawResponse)) + { + return Response.FromValue(Value, rawResponse!); + } + + using var scope = CreateScope(_waitForCompletionScopeName!); + try + { + rawResponse = async + ? await WaitForCompletionResponseAsync(async: true, pollingInterval, cancellationToken).ConfigureAwait(false) + : WaitForCompletionResponseAsync(async: false, pollingInterval, cancellationToken).EnsureCompleted(); + return Response.FromValue(Value, rawResponse); + } + catch (Exception e) + { + scope.Failed(e); + throw; + } } protected override async ValueTask UpdateStatusAsync(bool async, CancellationToken cancellationToken) @@ -260,7 +270,7 @@ protected override async ValueTask UpdateStatusAsync(bool async, Cance return GetResponseFromState(asyncLock.Value); } - using var scope = CreateScope(string.IsNullOrEmpty(_operationTypeName) ? string.Empty : $"{_operationTypeName}.UpdateStatus"); + using var scope = CreateScope(_updateStatusScopeName!); try { var state = await _operation.UpdateStateAsync(async, cancellationToken).ConfigureAwait(false); diff --git a/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs b/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs index 8f00f45126cd..db1a8faab7cf 100644 --- a/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs +++ b/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs @@ -17,7 +17,7 @@ namespace Azure.Core.Tests public class OperationInternalOfTTests { private static readonly string DiagnosticNamespace = "Azure.Core.Tests"; - private static readonly ClientDiagnostics ClientDiagnostics = new(new TestClientOptions()); + private static readonly ClientDiagnostics ClientDiagnostics = new(new TestClientOptions(), true); private static readonly RequestFailedException OriginalException = new(""); private static readonly StackOverflowException CustomException = new(); private static readonly MockResponse InitialResponse = new(200); @@ -247,7 +247,7 @@ public async Task WaitForCompletionCreatesDiagnosticScope([Values(true, false)] ? await operationInternal.WaitForCompletionAsync(CancellationToken.None) : operationInternal.WaitForCompletion(CancellationToken.None); - testListener.AssertScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); + testListener.AssertScope($"{expectedTypeName}.WaitForCompletion", expectedAttributes); } [Test] From ea748355105ead2208946cc4159e3ce78eaafce2 Mon Sep 17 00:00:00 2001 From: pshao25 <97225342+pshao25@users.noreply.github.com> Date: Fri, 10 Jun 2022 14:17:36 +0800 Subject: [PATCH 07/15] Fix tests --- sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs | 6 ++++-- sdk/core/Azure.Core/tests/OperationInternalTests.cs | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs b/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs index db1a8faab7cf..34d5c9595674 100644 --- a/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs +++ b/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs @@ -231,7 +231,8 @@ public async Task WaitForCompletionResponseCreatesDiagnosticScope([Values(true, ? await operationInternal.WaitForCompletionResponseAsync(CancellationToken.None) : operationInternal.WaitForCompletionResponse(CancellationToken.None); - testListener.AssertScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); + testListener.AssertAndRemoveScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); + CollectionAssert.IsEmpty(testListener.Scopes); } [Test] @@ -247,7 +248,8 @@ public async Task WaitForCompletionCreatesDiagnosticScope([Values(true, false)] ? await operationInternal.WaitForCompletionAsync(CancellationToken.None) : operationInternal.WaitForCompletion(CancellationToken.None); - testListener.AssertScope($"{expectedTypeName}.WaitForCompletion", expectedAttributes); + testListener.AssertAndRemoveScope($"{expectedTypeName}.WaitForCompletion", expectedAttributes); + CollectionAssert.IsEmpty(testListener.Scopes); } [Test] diff --git a/sdk/core/Azure.Core/tests/OperationInternalTests.cs b/sdk/core/Azure.Core/tests/OperationInternalTests.cs index 35869c056307..d34c25ca59f2 100644 --- a/sdk/core/Azure.Core/tests/OperationInternalTests.cs +++ b/sdk/core/Azure.Core/tests/OperationInternalTests.cs @@ -213,7 +213,8 @@ public async Task WaitForCompletionResponseCreatesDiagnosticScope([Values(true, ? await operationInternal.WaitForCompletionResponseAsync(CancellationToken.None) : operationInternal.WaitForCompletionResponse(CancellationToken.None); - testListener.AssertScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); + testListener.AssertAndRemoveScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); + CollectionAssert.IsEmpty(testListener.Scopes); } [Test] From 0bb67016e8431cc81312d84f46016553fc262699 Mon Sep 17 00:00:00 2001 From: pshao25 <97225342+pshao25@users.noreply.github.com> Date: Mon, 13 Jun 2022 13:39:04 +0800 Subject: [PATCH 08/15] Fix test --- sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs | 6 ++++++ sdk/core/Azure.Core/tests/OperationInternalTests.cs | 3 +++ 2 files changed, 9 insertions(+) diff --git a/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs b/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs index 34d5c9595674..64918d749347 100644 --- a/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs +++ b/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs @@ -231,8 +231,11 @@ public async Task WaitForCompletionResponseCreatesDiagnosticScope([Values(true, ? await operationInternal.WaitForCompletionResponseAsync(CancellationToken.None) : operationInternal.WaitForCompletionResponse(CancellationToken.None); + testListener.AssertScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); +#if NET5_0_OR_GREATER testListener.AssertAndRemoveScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); CollectionAssert.IsEmpty(testListener.Scopes); +#endif } [Test] @@ -248,8 +251,11 @@ public async Task WaitForCompletionCreatesDiagnosticScope([Values(true, false)] ? await operationInternal.WaitForCompletionAsync(CancellationToken.None) : operationInternal.WaitForCompletion(CancellationToken.None); + testListener.AssertScope($"{expectedTypeName}.WaitForCompletion", expectedAttributes); +#if NET5_0_OR_GREATER testListener.AssertAndRemoveScope($"{expectedTypeName}.WaitForCompletion", expectedAttributes); CollectionAssert.IsEmpty(testListener.Scopes); +#endif } [Test] diff --git a/sdk/core/Azure.Core/tests/OperationInternalTests.cs b/sdk/core/Azure.Core/tests/OperationInternalTests.cs index d34c25ca59f2..f36a39949699 100644 --- a/sdk/core/Azure.Core/tests/OperationInternalTests.cs +++ b/sdk/core/Azure.Core/tests/OperationInternalTests.cs @@ -213,8 +213,11 @@ public async Task WaitForCompletionResponseCreatesDiagnosticScope([Values(true, ? await operationInternal.WaitForCompletionResponseAsync(CancellationToken.None) : operationInternal.WaitForCompletionResponse(CancellationToken.None); + testListener.AssertScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); +#if NET5_0_OR_GREATER testListener.AssertAndRemoveScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); CollectionAssert.IsEmpty(testListener.Scopes); +#endif } [Test] From a3fa16f6a05f5dbb578f2657a8b5f3f3c0ea70dd Mon Sep 17 00:00:00 2001 From: pshao25 <97225342+pshao25@users.noreply.github.com> Date: Mon, 13 Jun 2022 13:42:21 +0800 Subject: [PATCH 09/15] Fix test --- sdk/core/Azure.Core/tests/OperationInternalTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/Azure.Core/tests/OperationInternalTests.cs b/sdk/core/Azure.Core/tests/OperationInternalTests.cs index f36a39949699..43878302cb7b 100644 --- a/sdk/core/Azure.Core/tests/OperationInternalTests.cs +++ b/sdk/core/Azure.Core/tests/OperationInternalTests.cs @@ -18,7 +18,7 @@ namespace Azure.Core.Tests public class OperationInternalTests { private static readonly string DiagnosticNamespace = "Azure.Core.Tests"; - private static readonly ClientDiagnostics ClientDiagnostics = new(new TestClientOptions()); + private static readonly ClientDiagnostics ClientDiagnostics = new(new TestClientOptions(), true); private static readonly RequestFailedException OriginalException = new(""); private static readonly StackOverflowException CustomException = new(); private static readonly MockResponse InitialResponse = new(200); From e83f355152177dd0368bc8d258a81ed2eaf25bb9 Mon Sep 17 00:00:00 2001 From: pshao25 <97225342+pshao25@users.noreply.github.com> Date: Thu, 16 Jun 2022 17:02:04 +0800 Subject: [PATCH 10/15] Resolve comments --- .../tests/OperationInternalOfTTests.cs | 24 ++++++++++++------- .../tests/OperationInternalTests.cs | 13 ++++++---- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs b/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs index 64918d749347..bd041ea82c3e 100644 --- a/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs +++ b/sdk/core/Azure.Core/tests/OperationInternalOfTTests.cs @@ -17,7 +17,7 @@ namespace Azure.Core.Tests public class OperationInternalOfTTests { private static readonly string DiagnosticNamespace = "Azure.Core.Tests"; - private static readonly ClientDiagnostics ClientDiagnostics = new(new TestClientOptions(), true); + private static readonly ClientDiagnostics ClientDiagnostics = new(new TestClientOptions()); private static readonly RequestFailedException OriginalException = new(""); private static readonly StackOverflowException CustomException = new(); private static readonly MockResponse InitialResponse = new(200); @@ -219,13 +219,13 @@ public async Task UpdateStatusPassesTheCancellationTokenToUpdateState([Values(tr } [Test] - public async Task WaitForCompletionResponseCreatesDiagnosticScope([Values(true, false)] bool async, [Values(null, "CustomTypeName")] string operationTypeName) + public async Task WaitForCompletionResponseCreatesDiagnosticScope([Values(true, false)] bool async, [Values(null, "CustomTypeName")] string operationTypeName, [Values(true, false)] bool suppressNestedClientActivities) { using ClientDiagnosticListener testListener = new(DiagnosticNamespace); string expectedTypeName = operationTypeName ?? nameof(TestOperation); KeyValuePair[] expectedAttributes = { new("key1", "value1"), new("key2", "value2") }; - var operationInternal = new OperationInternal(ClientDiagnostics, TestOperation.Succeeded(1), InitialResponse, operationTypeName, expectedAttributes); + var operationInternal = new OperationInternal(new(new TestClientOptions(), suppressNestedClientActivities), TestOperation.Succeeded(1), InitialResponse, operationTypeName, expectedAttributes); _ = async ? await operationInternal.WaitForCompletionResponseAsync(CancellationToken.None) @@ -233,19 +233,22 @@ public async Task WaitForCompletionResponseCreatesDiagnosticScope([Values(true, testListener.AssertScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); #if NET5_0_OR_GREATER - testListener.AssertAndRemoveScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); - CollectionAssert.IsEmpty(testListener.Scopes); + if (suppressNestedClientActivities) + { + testListener.AssertAndRemoveScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); + CollectionAssert.IsEmpty(testListener.Scopes); + } #endif } [Test] - public async Task WaitForCompletionCreatesDiagnosticScope([Values(true, false)] bool async, [Values(null, "CustomTypeName")] string operationTypeName) + public async Task WaitForCompletionCreatesDiagnosticScope([Values(true, false)] bool async, [Values(null, "CustomTypeName")] string operationTypeName, [Values(true, false)] bool suppressNestedClientActivities) { using ClientDiagnosticListener testListener = new(DiagnosticNamespace); string expectedTypeName = operationTypeName ?? nameof(TestOperation); KeyValuePair[] expectedAttributes = { new("key1", "value1"), new("key2", "value2") }; - var operationInternal = new OperationInternal(ClientDiagnostics, TestOperation.Succeeded(1), InitialResponse, operationTypeName, expectedAttributes); + var operationInternal = new OperationInternal(new(new TestClientOptions(), suppressNestedClientActivities), TestOperation.Succeeded(1), InitialResponse, operationTypeName, expectedAttributes); _ = async ? await operationInternal.WaitForCompletionAsync(CancellationToken.None) @@ -253,8 +256,11 @@ public async Task WaitForCompletionCreatesDiagnosticScope([Values(true, false)] testListener.AssertScope($"{expectedTypeName}.WaitForCompletion", expectedAttributes); #if NET5_0_OR_GREATER - testListener.AssertAndRemoveScope($"{expectedTypeName}.WaitForCompletion", expectedAttributes); - CollectionAssert.IsEmpty(testListener.Scopes); + if (suppressNestedClientActivities) + { + testListener.AssertAndRemoveScope($"{expectedTypeName}.WaitForCompletion", expectedAttributes); + CollectionAssert.IsEmpty(testListener.Scopes); + } #endif } diff --git a/sdk/core/Azure.Core/tests/OperationInternalTests.cs b/sdk/core/Azure.Core/tests/OperationInternalTests.cs index 43878302cb7b..3e8e2f11c12e 100644 --- a/sdk/core/Azure.Core/tests/OperationInternalTests.cs +++ b/sdk/core/Azure.Core/tests/OperationInternalTests.cs @@ -18,7 +18,7 @@ namespace Azure.Core.Tests public class OperationInternalTests { private static readonly string DiagnosticNamespace = "Azure.Core.Tests"; - private static readonly ClientDiagnostics ClientDiagnostics = new(new TestClientOptions(), true); + private static readonly ClientDiagnostics ClientDiagnostics = new(new TestClientOptions()); private static readonly RequestFailedException OriginalException = new(""); private static readonly StackOverflowException CustomException = new(); private static readonly MockResponse InitialResponse = new(200); @@ -201,13 +201,13 @@ public async Task UpdateStatusPassesTheCancellationTokenToUpdateState([Values(tr } [Test] - public async Task WaitForCompletionResponseCreatesDiagnosticScope([Values(true, false)] bool async, [Values(null, "CustomTypeName")] string operationTypeName) + public async Task WaitForCompletionResponseCreatesDiagnosticScope([Values(true, false)] bool async, [Values(null, "CustomTypeName")] string operationTypeName, [Values(true, false)] bool suppressNestedClientActivities) { using ClientDiagnosticListener testListener = new(DiagnosticNamespace); string expectedTypeName = operationTypeName ?? nameof(TestOperation); KeyValuePair[] expectedAttributes = { new("key1", "value1"), new("key2", "value2") }; - var operationInternal = new OperationInternal(ClientDiagnostics, TestOperation.Succeeded(), InitialResponse, operationTypeName, expectedAttributes); + var operationInternal = new OperationInternal(new(new TestClientOptions(), suppressNestedClientActivities), TestOperation.Succeeded(), InitialResponse, operationTypeName, expectedAttributes); _ = async ? await operationInternal.WaitForCompletionResponseAsync(CancellationToken.None) @@ -215,8 +215,11 @@ public async Task WaitForCompletionResponseCreatesDiagnosticScope([Values(true, testListener.AssertScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); #if NET5_0_OR_GREATER - testListener.AssertAndRemoveScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); - CollectionAssert.IsEmpty(testListener.Scopes); + if (suppressNestedClientActivities) + { + testListener.AssertAndRemoveScope($"{expectedTypeName}.WaitForCompletionResponse", expectedAttributes); + CollectionAssert.IsEmpty(testListener.Scopes); + } #endif } From f572bcf0b3391d22d66a5599f18aea6f5592e53f Mon Sep 17 00:00:00 2001 From: pshao25 <97225342+pshao25@users.noreply.github.com> Date: Mon, 20 Jun 2022 11:02:44 +0800 Subject: [PATCH 11/15] Resolve comments --- sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs index a7f0e6e633e1..45a62b110b04 100644 --- a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs +++ b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs @@ -34,8 +34,8 @@ protected OperationInternalBase(Response rawResponse) protected OperationInternalBase(ClientDiagnostics clientDiagnostics, string operationTypeName, IEnumerable>? scopeAttributes = null, DelayStrategy? fallbackStrategy = null) { _diagnostics = clientDiagnostics; - _updateStatusScopeName = $"{operationTypeName}.UpdateStatus"; - _waitForCompletionResponseScopeName = $"{operationTypeName}.WaitForCompletionResponse"; + _updateStatusScopeName = $"{operationTypeName}.{nameof(UpdateStatus)}"; + _waitForCompletionResponseScopeName = $"{operationTypeName}.{nameof(WaitForCompletionResponse)}"; _waitForCompletionScopeName = $"{operationTypeName}.WaitForCompletion"; _scopeAttributes = scopeAttributes?.ToDictionary(kvp => kvp.Key, kvp => kvp.Value); _fallbackStrategy = fallbackStrategy; From 102dbed668938f47b92a33c0a72c61c02e329489 Mon Sep 17 00:00:00 2001 From: pshao25 <97225342+pshao25@users.noreply.github.com> Date: Mon, 20 Jun 2022 17:30:19 +0800 Subject: [PATCH 12/15] Make scope name not nullable. --- .../Azure.Core/src/Shared/OperationInternalBase.cs | 11 +++++++---- .../Azure.Core/src/Shared/OperationInternalOfT.cs | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs index 45a62b110b04..0be615a9306b 100644 --- a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs +++ b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs @@ -19,13 +19,16 @@ internal abstract class OperationInternalBase private readonly DelayStrategy? _fallbackStrategy; private readonly AsyncLockWithValue _responseLock; - private readonly string? _waitForCompletionResponseScopeName; - protected readonly string? _updateStatusScopeName; - protected readonly string? _waitForCompletionScopeName; + private readonly string _waitForCompletionResponseScopeName; + protected readonly string _updateStatusScopeName; + protected readonly string _waitForCompletionScopeName; protected OperationInternalBase(Response rawResponse) { _diagnostics = new ClientDiagnostics(ClientOptions.Default); + _updateStatusScopeName = string.Empty; + _waitForCompletionResponseScopeName = string.Empty; + _waitForCompletionScopeName = string.Empty; _scopeAttributes = default; _fallbackStrategy = default; _responseLock = new AsyncLockWithValue(rawResponse); @@ -192,7 +195,7 @@ protected async ValueTask WaitForCompletionResponseAsync(bool async, T return lockOrValue.Value; } - using var scope = CreateScope(_waitForCompletionResponseScopeName!); + using var scope = CreateScope(_waitForCompletionResponseScopeName); try { var poller = new OperationPoller(_fallbackStrategy); diff --git a/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs b/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs index ea9fa60843ff..10ee7cad8a60 100644 --- a/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs +++ b/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs @@ -244,7 +244,7 @@ private async ValueTask> WaitForCompletionAsync(bool async, TimeSpan return Response.FromValue(Value, rawResponse!); } - using var scope = CreateScope(_waitForCompletionScopeName!); + using var scope = CreateScope(_waitForCompletionScopeName); try { rawResponse = async @@ -270,7 +270,7 @@ protected override async ValueTask UpdateStatusAsync(bool async, Cance return GetResponseFromState(asyncLock.Value); } - using var scope = CreateScope(_updateStatusScopeName!); + using var scope = CreateScope(_updateStatusScopeName); try { var state = await _operation.UpdateStateAsync(async, cancellationToken).ConfigureAwait(false); From a4eb221652cd8e510aceceb7ffa105914ae160fb Mon Sep 17 00:00:00 2001 From: pshao25 <97225342+pshao25@users.noreply.github.com> Date: Tue, 21 Jun 2022 09:59:58 +0800 Subject: [PATCH 13/15] Resolve comments --- .../src/Shared/OperationInternalBase.cs | 12 +++++----- .../src/Shared/OperationInternalOfT.cs | 22 ++++--------------- 2 files changed, 10 insertions(+), 24 deletions(-) diff --git a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs index 0be615a9306b..f57d19282501 100644 --- a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs +++ b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs @@ -121,7 +121,7 @@ public Response UpdateStatus(CancellationToken cancellationToken) => /// The last HTTP response received from the server, including the final result of the long-running operation. /// Thrown if there's been any issues during the connection, or if the operation has completed with failures. public async ValueTask WaitForCompletionResponseAsync(CancellationToken cancellationToken) - => await WaitForCompletionResponseAsync(async: true, null, cancellationToken).ConfigureAwait(false); + => await WaitForCompletionResponseAsync(async: true, null, _waitForCompletionResponseScopeName, cancellationToken).ConfigureAwait(false); /// /// Periodically calls until the long-running operation completes. The interval @@ -142,7 +142,7 @@ public async ValueTask WaitForCompletionResponseAsync(CancellationToke /// The last HTTP response received from the server, including the final result of the long-running operation. /// Thrown if there's been any issues during the connection, or if the operation has completed with failures. public async ValueTask WaitForCompletionResponseAsync(TimeSpan pollingInterval, CancellationToken cancellationToken) - => await WaitForCompletionResponseAsync(async: true, pollingInterval, cancellationToken).ConfigureAwait(false); + => await WaitForCompletionResponseAsync(async: true, pollingInterval, _waitForCompletionResponseScopeName, cancellationToken).ConfigureAwait(false); /// /// Periodically calls until the long-running operation completes. @@ -162,7 +162,7 @@ public async ValueTask WaitForCompletionResponseAsync(TimeSpan polling /// The last HTTP response received from the server, including the final result of the long-running operation. /// Thrown if there's been any issues during the connection, or if the operation has completed with failures. public Response WaitForCompletionResponse(CancellationToken cancellationToken) - => WaitForCompletionResponseAsync(async: false, null, cancellationToken).EnsureCompleted(); + => WaitForCompletionResponseAsync(async: false, null, _waitForCompletionResponseScopeName, cancellationToken).EnsureCompleted(); /// /// Periodically calls until the long-running operation completes. The interval @@ -183,9 +183,9 @@ public Response WaitForCompletionResponse(CancellationToken cancellationToken) /// The last HTTP response received from the server, including the final result of the long-running operation. /// Thrown if there's been any issues during the connection, or if the operation has completed with failures. public Response WaitForCompletionResponse(TimeSpan pollingInterval, CancellationToken cancellationToken) - => WaitForCompletionResponseAsync(async: false, pollingInterval, cancellationToken).EnsureCompleted(); + => WaitForCompletionResponseAsync(async: false, pollingInterval, _waitForCompletionResponseScopeName, cancellationToken).EnsureCompleted(); - protected async ValueTask WaitForCompletionResponseAsync(bool async, TimeSpan? pollingInterval, CancellationToken cancellationToken) + protected async ValueTask WaitForCompletionResponseAsync(bool async, TimeSpan? pollingInterval, string scopeName, CancellationToken cancellationToken) { // If _responseLock has the value, lockOrValue will contain that value, and no lock is acquired. // If _responseLock doesn't have the value, GetLockOrValueAsync will acquire the lock that will be released when lockOrValue is disposed @@ -195,7 +195,7 @@ protected async ValueTask WaitForCompletionResponseAsync(bool async, T return lockOrValue.Value; } - using var scope = CreateScope(_waitForCompletionResponseScopeName); + using var scope = CreateScope(scopeName); try { var poller = new OperationPoller(_fallbackStrategy); diff --git a/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs b/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs index 10ee7cad8a60..e3aef98c5961 100644 --- a/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs +++ b/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs @@ -239,24 +239,10 @@ public Response WaitForCompletion(TimeSpan pollingInterval, CancellationToken private async ValueTask> WaitForCompletionAsync(bool async, TimeSpan? pollingInterval, CancellationToken cancellationToken) { - if (TryGetResponseValue(out var rawResponse)) - { - return Response.FromValue(Value, rawResponse!); - } - - using var scope = CreateScope(_waitForCompletionScopeName); - try - { - rawResponse = async - ? await WaitForCompletionResponseAsync(async: true, pollingInterval, cancellationToken).ConfigureAwait(false) - : WaitForCompletionResponseAsync(async: false, pollingInterval, cancellationToken).EnsureCompleted(); - return Response.FromValue(Value, rawResponse); - } - catch (Exception e) - { - scope.Failed(e); - throw; - } + var rawResponse = async + ? await WaitForCompletionResponseAsync(async: true, pollingInterval, _waitForCompletionScopeName, cancellationToken).ConfigureAwait(false) + : WaitForCompletionResponseAsync(async: false, pollingInterval, _waitForCompletionScopeName, cancellationToken).EnsureCompleted(); + return Response.FromValue(Value, rawResponse); } protected override async ValueTask UpdateStatusAsync(bool async, CancellationToken cancellationToken) From 610a2419440477943febea7e3424d2a0e2b720d2 Mon Sep 17 00:00:00 2001 From: pshao25 <97225342+pshao25@users.noreply.github.com> Date: Tue, 21 Jun 2022 14:38:46 +0800 Subject: [PATCH 14/15] Remove useless method --- sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs index f57d19282501..c0bf47075cd9 100644 --- a/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs +++ b/sdk/core/Azure.Core/src/Shared/OperationInternalBase.cs @@ -234,10 +234,5 @@ protected DiagnosticScope CreateScope(string scopeName) protected async ValueTask CreateException(bool async, Response response) => async ? await _diagnostics.CreateRequestFailedExceptionAsync(response).ConfigureAwait(false) : _diagnostics.CreateRequestFailedException(response); - - protected bool TryGetResponseValue(out Response? value) - { - return _responseLock.TryGetValue(out value); - } } } From 339e43377b1ab384eb7c28126777ddace7bfe63b Mon Sep 17 00:00:00 2001 From: pshao25 <97225342+pshao25@users.noreply.github.com> Date: Wed, 22 Jun 2022 07:40:25 +0800 Subject: [PATCH 15/15] Resolve comment --- sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs b/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs index e3aef98c5961..3319a8d8c380 100644 --- a/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs +++ b/sdk/core/Azure.Core/src/Shared/OperationInternalOfT.cs @@ -239,9 +239,7 @@ public Response WaitForCompletion(TimeSpan pollingInterval, CancellationToken private async ValueTask> WaitForCompletionAsync(bool async, TimeSpan? pollingInterval, CancellationToken cancellationToken) { - var rawResponse = async - ? await WaitForCompletionResponseAsync(async: true, pollingInterval, _waitForCompletionScopeName, cancellationToken).ConfigureAwait(false) - : WaitForCompletionResponseAsync(async: false, pollingInterval, _waitForCompletionScopeName, cancellationToken).EnsureCompleted(); + var rawResponse = await WaitForCompletionResponseAsync(async, pollingInterval, _waitForCompletionScopeName, cancellationToken).ConfigureAwait(false); return Response.FromValue(Value, rawResponse); }