Skip to content

Commit d50b687

Browse files
JoshLove-msftseanmcc-msftabhipsaMisra
authored
Check for duplicate scopes (#23601)
* Check for duplicate scopes * Fix duplicate check * Remove IsDuplicate * Traverse all ancestors * tryget * Fixed most duplicate Diagnostic Scopes in Data Lake * Fixed most duplicate Diagnostic Scopes in Blobs * Fix Event Grid tests * Remove interpolate * Be smarter * fix(digitaltwins): Remove duplicate DiagnosticScope initialization Co-authored-by: Sean McCullough <seanmcc@microsoft.com> Co-authored-by: Abhipsa Misra <abhipsa.misra@microsoft.com>
1 parent 7b5422c commit d50b687

File tree

9 files changed

+362
-693
lines changed

9 files changed

+362
-693
lines changed

‎sdk/core/Azure.Core.TestFramework/src/ClientDiagnosticListener.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,21 @@ public void Dispose()
141141

142142
foreach (ProducedDiagnosticScope producedDiagnosticScope in Scopes)
143143
{
144+
var activity = producedDiagnosticScope.Activity;
145+
var operationName = activity.OperationName;
146+
// traverse the activities and check for duplicates among ancestors
147+
while (activity != null)
148+
{
149+
if (operationName == activity.Parent?.OperationName)
150+
{
151+
// Throw this exception lazily on Dispose, rather than when the scope is started, so that we don't trigger a bunch of other
152+
// erroneous exceptions relating to scopes not being completed/started that hide the actual issue
153+
throw new InvalidOperationException($"A scope has already started for event '{producedDiagnosticScope.Name}'");
154+
}
155+
156+
activity = activity.Parent;
157+
}
158+
144159
if (!producedDiagnosticScope.IsCompleted)
145160
{
146161
throw new InvalidOperationException($"'{producedDiagnosticScope.Name}' scope is not completed");

‎sdk/core/Azure.Core/tests/ClientTestBaseDiagnosticScopeTests.cs‎

Lines changed: 85 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,29 @@ public void ThrowsWhenNoDiagnosticScope()
2626
StringAssert.Contains("Expected some diagnostic scopes to be created, found none", ex.Message);
2727
}
2828

29+
[Test]
30+
public void ThrowsWhenDuplicateDiagnosticScope_DirectAncestor()
31+
{
32+
InvalidDiagnosticScopeTestClient client = InstrumentClient(new InvalidDiagnosticScopeTestClient());
33+
InvalidOperationException ex = Assert.ThrowsAsync<InvalidOperationException>(async () => await client.DuplicateScopeDirectAncestorAsync());
34+
StringAssert.Contains($"A scope has already started for event '{typeof(InvalidDiagnosticScopeTestClient).Name}.{nameof(client.DuplicateScopeDirectAncestor)}'", ex.Message);
35+
}
36+
37+
[Test]
38+
public void ThrowsWhenDuplicateDiagnosticScope_Ancestor()
39+
{
40+
InvalidDiagnosticScopeTestClient client = InstrumentClient(new InvalidDiagnosticScopeTestClient());
41+
InvalidOperationException ex = Assert.ThrowsAsync<InvalidOperationException>(async () => await client.DuplicateScopeAncestorAsync());
42+
StringAssert.Contains($"A scope has already started for event '{typeof(InvalidDiagnosticScopeTestClient).Name}.{nameof(client.DuplicateScopeAncestor)}'", ex.Message);
43+
}
44+
45+
[Test]
46+
public async Task DoesNotThrowWhenDuplicateDiagnosticScopeProperlyDisposed()
47+
{
48+
InvalidDiagnosticScopeTestClient client = InstrumentClient(new InvalidDiagnosticScopeTestClient());
49+
await client.DuplicateScopeProperlyDisposedAsync();
50+
}
51+
2952
[Test]
3053
public void ThrowsWhenNoDiagnosticScopeInsidePageable()
3154
{
@@ -74,11 +97,17 @@ public async Task DoesNotThrowForCorrectDiagnosticScope()
7497

7598
public class InvalidDiagnosticScopeTestClient
7699
{
77-
private void FireScope(string method)
100+
private DiagnosticScope CreateScope(string method)
78101
{
79102
DiagnosticScopeFactory clientDiagnostics = new DiagnosticScopeFactory("Azure.Core.Tests", "random", true);
80103
string activityName = $"{typeof(InvalidDiagnosticScopeTestClient).Name}.{method}";
81104
DiagnosticScope scope = clientDiagnostics.CreateScope(activityName);
105+
return scope;
106+
}
107+
108+
private void CreateAndFireScope(string method)
109+
{
110+
DiagnosticScope scope = CreateScope(method);
82111
scope.Start();
83112
scope.Dispose();
84113
}
@@ -95,47 +124,92 @@ public virtual bool NoScope()
95124
return true;
96125
}
97126

127+
public virtual Task<bool> DuplicateScopeDirectAncestorAsync()
128+
{
129+
return Task.FromResult(DuplicateScopeDirectAncestor());
130+
}
131+
132+
public virtual bool DuplicateScopeDirectAncestor()
133+
{
134+
using DiagnosticScope scope1 = CreateScope(nameof(DuplicateScopeDirectAncestor));
135+
scope1.Start();
136+
using DiagnosticScope scope2 = CreateScope(nameof(DuplicateScopeDirectAncestor));
137+
scope2.Start();
138+
return true;
139+
}
140+
141+
public virtual Task<bool> DuplicateScopeAncestorAsync()
142+
{
143+
return Task.FromResult(DuplicateScopeAncestor());
144+
}
145+
146+
public virtual bool DuplicateScopeAncestor()
147+
{
148+
using DiagnosticScope scope1 = CreateScope(nameof(DuplicateScopeAncestor));
149+
scope1.Start();
150+
using DiagnosticScope scope2 = CreateScope(nameof(CorrectScope));
151+
scope2.Start();
152+
using DiagnosticScope scope3 = CreateScope(nameof(DuplicateScopeAncestor));
153+
scope3.Start();
154+
return true;
155+
}
156+
157+
public virtual Task<bool> DuplicateScopeProperlyDisposedAsync()
158+
{
159+
return Task.FromResult(DuplicateScopeProperlyDisposed());
160+
}
161+
162+
public virtual bool DuplicateScopeProperlyDisposed()
163+
{
164+
DiagnosticScope scope1 = CreateScope(nameof(DuplicateScopeProperlyDisposed));
165+
scope1.Start();
166+
scope1.Dispose();
167+
using DiagnosticScope scope2 = CreateScope(nameof(DuplicateScopeProperlyDisposed));
168+
scope2.Start();
169+
return true;
170+
}
171+
98172
public virtual Task<bool> WrongScopeAsync()
99173
{
100-
FireScope("DoesNotExist");
174+
CreateAndFireScope("DoesNotExist");
101175
return Task.FromResult(true);
102176
}
103177

104178
public virtual bool WrongScope()
105179
{
106-
FireScope("DoesNotExist");
180+
CreateAndFireScope("DoesNotExist");
107181
return true;
108182
}
109183

110184
public virtual Task<bool> CorrectScopeAsync()
111185
{
112-
FireScope(nameof(CorrectScope));
186+
CreateAndFireScope(nameof(CorrectScope));
113187
return Task.FromResult(true);
114188
}
115189

116190
public virtual bool CorrectScope()
117191
{
118-
FireScope(nameof(CorrectScope));
192+
CreateAndFireScope(nameof(CorrectScope));
119193
return true;
120194
}
121195

122196
[ForwardsClientCalls]
123197
public virtual Task<bool> ForwardsAsync()
124198
{
125-
FireScope(nameof(CorrectScope));
199+
CreateAndFireScope(nameof(CorrectScope));
126200
return Task.FromResult(true);
127201
}
128202

129203
[ForwardsClientCalls]
130204
public virtual bool Forwards()
131205
{
132-
FireScope(nameof(CorrectScope));
206+
CreateAndFireScope(nameof(CorrectScope));
133207
return true;
134208
}
135209

136210
public virtual AsyncPageable<int> GetPageableNoPageableScopesAsync()
137211
{
138-
FireScope(nameof(GetPageableNoPageableScopesAsync));
212+
CreateAndFireScope(nameof(GetPageableNoPageableScopesAsync));
139213

140214
return PageResponseEnumerator.CreateAsyncEnumerable(s =>
141215
{
@@ -150,7 +224,7 @@ public virtual AsyncPageable<int> GetPageableNoPageableScopesAsync()
150224

151225
public virtual Pageable<int> GetPageableNoPageableScopes()
152226
{
153-
FireScope(nameof(GetPageableNoPageableScopes));
227+
CreateAndFireScope(nameof(GetPageableNoPageableScopes));
154228

155229
return PageResponseEnumerator.CreateEnumerable(s =>
156230
{
@@ -167,7 +241,7 @@ public virtual AsyncPageable<int> GetPageableValidScopesAsync()
167241
{
168242
return PageResponseEnumerator.CreateAsyncEnumerable(s =>
169243
{
170-
FireScope(nameof(GetPageableValidScopes));
244+
CreateAndFireScope(nameof(GetPageableValidScopes));
171245

172246
if (s == null)
173247
{
@@ -182,7 +256,7 @@ public virtual Pageable<int> GetPageableValidScopes()
182256
{
183257
return PageResponseEnumerator.CreateEnumerable(s =>
184258
{
185-
FireScope(nameof(GetPageableValidScopes));
259+
CreateAndFireScope(nameof(GetPageableValidScopes));
186260

187261
if (s == null)
188262
{

‎sdk/digitaltwins/Azure.DigitalTwins.Core/src/Customized/DigitalTwinModelsRestClient.cs‎

Lines changed: 48 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -21,37 +21,27 @@ internal async Task<Response<IReadOnlyList<DigitalTwinsModelData>>> AddAsync(
2121
CreateModelsOptions digitalTwinModelsAddOptions = null,
2222
CancellationToken cancellationToken = default)
2323
{
24-
using DiagnosticScope scope = _clientDiagnostics.CreateScope("DigitalTwinModelsClient.Add");
25-
scope.Start();
26-
try
27-
{
28-
using HttpMessage message = CreateAddRequest(models, digitalTwinModelsAddOptions);
29-
await _pipeline.SendAsync(message, cancellationToken).ConfigureAwait(false);
30-
switch (message.Response.Status)
31-
{
32-
case 200:
33-
case 201:
24+
using HttpMessage message = CreateAddRequest(models, digitalTwinModelsAddOptions);
25+
await _pipeline.SendAsync(message, cancellationToken).ConfigureAwait(false);
26+
switch (message.Response.Status)
27+
{
28+
case 200:
29+
case 201:
30+
{
31+
IReadOnlyList<DigitalTwinsModelData> value = default;
32+
using JsonDocument document = await JsonDocument
33+
.ParseAsync(message.Response.ContentStream, default, cancellationToken)
34+
.ConfigureAwait(false);
35+
List<DigitalTwinsModelData> array = new List<DigitalTwinsModelData>(document.RootElement.GetArrayLength());
36+
foreach (JsonElement item in document.RootElement.EnumerateArray())
3437
{
35-
IReadOnlyList<DigitalTwinsModelData> value = default;
36-
using JsonDocument document = await JsonDocument
37-
.ParseAsync(message.Response.ContentStream, default, cancellationToken)
38-
.ConfigureAwait(false);
39-
List<DigitalTwinsModelData> array = new List<DigitalTwinsModelData>(document.RootElement.GetArrayLength());
40-
foreach (JsonElement item in document.RootElement.EnumerateArray())
41-
{
42-
array.Add(DigitalTwinsModelData.DeserializeDigitalTwinsModelData(item));
43-
}
44-
value = array;
45-
return Response.FromValue(value, message.Response);
38+
array.Add(DigitalTwinsModelData.DeserializeDigitalTwinsModelData(item));
4639
}
47-
default:
48-
throw await _clientDiagnostics.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false);
49-
}
50-
}
51-
catch (Exception ex)
52-
{
53-
scope.Failed(ex);
54-
throw;
40+
value = array;
41+
return Response.FromValue(value, message.Response);
42+
}
43+
default:
44+
throw await _clientDiagnostics.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false);
5545
}
5646
}
5747

@@ -62,35 +52,25 @@ internal Response<IReadOnlyList<DigitalTwinsModelData>> Add(
6252
CreateModelsOptions digitalTwinModelsAddOptions = null,
6353
CancellationToken cancellationToken = default)
6454
{
65-
using DiagnosticScope scope = _clientDiagnostics.CreateScope("DigitalTwinModelsClient.Add");
66-
scope.Start();
67-
try
68-
{
69-
using HttpMessage message = CreateAddRequest(models, digitalTwinModelsAddOptions);
70-
_pipeline.Send(message, cancellationToken);
71-
switch (message.Response.Status)
72-
{
73-
case 200:
74-
case 201:
55+
using HttpMessage message = CreateAddRequest(models, digitalTwinModelsAddOptions);
56+
_pipeline.Send(message, cancellationToken);
57+
switch (message.Response.Status)
58+
{
59+
case 200:
60+
case 201:
61+
{
62+
IReadOnlyList<DigitalTwinsModelData> value = default;
63+
using var document = JsonDocument.Parse(message.Response.ContentStream);
64+
List<DigitalTwinsModelData> array = new List<DigitalTwinsModelData>(document.RootElement.GetArrayLength());
65+
foreach (JsonElement item in document.RootElement.EnumerateArray())
7566
{
76-
IReadOnlyList<DigitalTwinsModelData> value = default;
77-
using var document = JsonDocument.Parse(message.Response.ContentStream);
78-
List<DigitalTwinsModelData> array = new List<DigitalTwinsModelData>(document.RootElement.GetArrayLength());
79-
foreach (JsonElement item in document.RootElement.EnumerateArray())
80-
{
81-
array.Add(DigitalTwinsModelData.DeserializeDigitalTwinsModelData(item));
82-
}
83-
value = array;
84-
return Response.FromValue(value, message.Response);
67+
array.Add(DigitalTwinsModelData.DeserializeDigitalTwinsModelData(item));
8568
}
86-
default:
87-
throw _clientDiagnostics.CreateRequestFailedException(message.Response);
88-
}
89-
}
90-
catch (Exception ex)
91-
{
92-
scope.Failed(ex);
93-
throw;
69+
value = array;
70+
return Response.FromValue(value, message.Response);
71+
}
72+
default:
73+
throw _clientDiagnostics.CreateRequestFailedException(message.Response);
9474
}
9575
}
9676

@@ -111,23 +91,13 @@ internal async Task<Response> UpdateAsync(
11191
throw new ArgumentNullException(nameof(modelUpdates));
11292
}
11393

114-
using DiagnosticScope scope = _clientDiagnostics.CreateScope("DigitalTwinModelsClient.Update");
115-
scope.Start();
116-
try
94+
using HttpMessage message = CreateUpdateRequest(id, modelUpdates, digitalTwinModelsUpdateOptions);
95+
await _pipeline.SendAsync(message, cancellationToken).ConfigureAwait(false);
96+
return message.Response.Status switch
11797
{
118-
using HttpMessage message = CreateUpdateRequest(id, modelUpdates, digitalTwinModelsUpdateOptions);
119-
await _pipeline.SendAsync(message, cancellationToken).ConfigureAwait(false);
120-
return message.Response.Status switch
121-
{
122-
204 => message.Response,
123-
_ => throw await _clientDiagnostics.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false),
124-
};
125-
}
126-
catch (Exception e)
127-
{
128-
scope.Failed(e);
129-
throw;
130-
}
98+
204 => message.Response,
99+
_ => throw await _clientDiagnostics.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false),
100+
};
131101
}
132102

133103
// The modelUpdates parameter needs to be changed from IEnumerable<object> to IEnumerable<string>
@@ -147,23 +117,13 @@ internal Response Update(
147117
throw new ArgumentNullException(nameof(modelUpdates));
148118
}
149119

150-
using DiagnosticScope scope = _clientDiagnostics.CreateScope("DigitalTwinModelsClient.Update");
151-
scope.Start();
152-
try
120+
using HttpMessage message = CreateUpdateRequest(id, modelUpdates, digitalTwinModelsUpdateOptions);
121+
_pipeline.Send(message, cancellationToken);
122+
return message.Response.Status switch
153123
{
154-
using HttpMessage message = CreateUpdateRequest(id, modelUpdates, digitalTwinModelsUpdateOptions);
155-
_pipeline.Send(message, cancellationToken);
156-
return message.Response.Status switch
157-
{
158-
204 => message.Response,
159-
_ => throw _clientDiagnostics.CreateRequestFailedException(message.Response),
160-
};
161-
}
162-
catch (Exception e)
163-
{
164-
scope.Failed(e);
165-
throw;
166-
}
124+
204 => message.Response,
125+
_ => throw _clientDiagnostics.CreateRequestFailedException(message.Response),
126+
};
167127
}
168128

169129
// The strings are already json, so we do not want them to be serialized.

0 commit comments

Comments
 (0)