Skip to content

Commit 7a4b0dd

Browse files
j82wj82w
authored
Diagnostics: Fix ToString() to not grow exponentially with retries (#1539)
Diagnostics: Fix ToString() to not grow exponentially with retries (#1539)
1 parent acf24c5 commit 7a4b0dd

7 files changed

Lines changed: 147 additions & 26 deletions

File tree

‎Microsoft.Azure.Cosmos/src/Diagnostics/CosmosClientSideRequestStatistics.cs‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ namespace Microsoft.Azure.Cosmos
1616

1717
internal sealed class CosmosClientSideRequestStatistics : CosmosDiagnosticsInternal, IClientSideRequestStatistics
1818
{
19+
public const string DefaultToStringMessage = "Please see CosmosDiagnostics";
1920
private readonly object lockObject = new object();
2021

2122
public CosmosClientSideRequestStatistics(CosmosDiagnosticsContext diagnosticsContext = null)
@@ -151,20 +152,24 @@ public void RecordAddressResolutionEnd(string identifier)
151152
}
152153
}
153154

155+
/// <summary>
156+
/// The new Cosmos Exception always includes the diagnostics and the
157+
/// document client exception message. Some of the older document client exceptions
158+
/// include the request statistics in the message causing a circle reference.
159+
/// This always returns empty string to prevent the circle reference which
160+
/// would cause the diagnostic string to grow exponentially.
161+
/// </summary>
154162
public override string ToString()
155163
{
156-
// This is required for the older IClientSideRequestStatistics
157-
// Capture the entire diagnostic context in the toString to avoid losing any information
158-
// for any APIs using the older interface.
159-
return this.DiagnosticsContext.ToString();
164+
return DefaultToStringMessage;
160165
}
161166

167+
/// <summary>
168+
/// Please see ToString() documentation
169+
/// </summary>
162170
public void AppendToBuilder(StringBuilder stringBuilder)
163171
{
164-
// This is required for the older IClientSideRequestStatistics
165-
// Capture the entire diagnostic context in the toString to avoid losing any information
166-
// for any APIs using the older interface.
167-
stringBuilder.Append(this.DiagnosticsContext.ToString());
172+
stringBuilder.Append(DefaultToStringMessage);
168173
}
169174

170175
public override void Accept(CosmosDiagnosticsInternalVisitor visitor)

‎Microsoft.Azure.Cosmos/src/Resource/CosmosExceptions/CosmosException.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,7 @@ private string ToStringHelper(
236236

237237
if (this.Diagnostics != null)
238238
{
239+
stringBuilder.Append("--- Cosmos Diagnostics ---");
239240
stringBuilder.Append(this.Diagnostics);
240241
}
241242

‎Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosDiagnosticsTests.cs‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ namespace Microsoft.Azure.Cosmos.SDK.EmulatorTests
1616
using System.Linq;
1717
using System.Net;
1818
using System.Runtime.CompilerServices;
19+
using System.Text.RegularExpressions;
1920
using System.Threading;
2021
using System.Threading.Tasks;
2122

@@ -129,6 +130,88 @@ public async Task PointOperationRequestTimeoutDiagnostic(bool disableDiagnostics
129130
}
130131
}
131132

133+
[TestMethod]
134+
public async Task PointOperationForbiddenDiagnostic()
135+
{
136+
List<int> stringLength = new List<int>();
137+
foreach (int maxCount in new int[] { 1, 2, 4 })
138+
{
139+
int count = 0;
140+
List<(string, string)> activityIdAndErrorMessage = new List<(string, string)>(maxCount);
141+
Guid transportExceptionActivityId = Guid.NewGuid();
142+
string transportErrorMessage = $"TransportErrorMessage{Guid.NewGuid()}";
143+
144+
Action<Uri, Documents.ResourceOperation, Documents.DocumentServiceRequest> interceptor =
145+
(uri, operation, request) =>
146+
{
147+
if (request.ResourceType == Documents.ResourceType.Document)
148+
{
149+
if (count >= maxCount)
150+
{
151+
TransportClientHelper.ThrowTransportExceptionOnItemOperation(
152+
uri,
153+
operation,
154+
request,
155+
transportExceptionActivityId,
156+
transportErrorMessage);
157+
}
158+
159+
count++;
160+
string activityId = Guid.NewGuid().ToString();
161+
string errorMessage = $"Error{Guid.NewGuid()}";
162+
163+
activityIdAndErrorMessage.Add((activityId, errorMessage));
164+
TransportClientHelper.ThrowForbiddendExceptionOnItemOperation(
165+
uri,
166+
request,
167+
activityId,
168+
errorMessage);
169+
}
170+
};
171+
172+
Container containerWithTransportException = TransportClientHelper.GetContainerWithIntercepter(
173+
this.database.Id,
174+
this.Container.Id,
175+
interceptor);
176+
//Checking point operation diagnostics on typed operations
177+
ToDoActivity testItem = ToDoActivity.CreateRandomToDoActivity();
178+
179+
try
180+
{
181+
ItemResponse<ToDoActivity> createResponse = await containerWithTransportException.CreateItemAsync<ToDoActivity>(
182+
item: testItem);
183+
Assert.Fail("Should have thrown a request timeout exception");
184+
}
185+
catch (CosmosException ce) when (ce.StatusCode == System.Net.HttpStatusCode.RequestTimeout)
186+
{
187+
string exceptionToString = ce.ToString();
188+
Assert.IsNotNull(exceptionToString);
189+
stringLength.Add(exceptionToString.Length);
190+
191+
// Request timeout info will be in the exception message and in the diagnostic info.
192+
Assert.AreEqual(2, Regex.Matches(exceptionToString, transportExceptionActivityId.ToString()).Count);
193+
Assert.AreEqual(2, Regex.Matches(exceptionToString, transportErrorMessage).Count);
194+
195+
// Check to make sure the diagnostics does not include duplicate info.
196+
foreach ((string activityId, string errorMessage) in activityIdAndErrorMessage)
197+
{
198+
Assert.AreEqual(1, Regex.Matches(exceptionToString, activityId).Count);
199+
Assert.AreEqual(1, Regex.Matches(exceptionToString, errorMessage).Count);
200+
}
201+
}
202+
}
203+
204+
// Check if the exception message is not growing exponentially
205+
Assert.IsTrue(stringLength.Count > 2);
206+
for (int i = 0; i < stringLength.Count - 1; i++)
207+
{
208+
int currLength = stringLength[i];
209+
int nextLength = stringLength[i + 1];
210+
Assert.IsTrue( nextLength < currLength * 2,
211+
$"The diagnostic string is growing faster than linear. Length: {currLength}, Next Length: {nextLength}");
212+
}
213+
}
214+
132215
[TestMethod]
133216
[DataRow(true)]
134217
[DataRow(false)]

‎Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosItemTests.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1667,7 +1667,9 @@ public async Task VerifySessionNotFoundStatistics()
16671667
}
16681668
catch (CosmosException cosmosException)
16691669
{
1670-
Assert.IsTrue(cosmosException.Message.Contains("StorePhysicalAddress"), cosmosException.Message);
1670+
Assert.IsTrue(cosmosException.Message.Contains("The read session is not available for the input session token."), cosmosException.Message);
1671+
string exception = cosmosException.ToString();
1672+
Assert.IsTrue(exception.Contains("StorePhysicalAddress"), exception);
16711673
}
16721674
}
16731675
finally

‎Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/TransportWrapperTests.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,9 @@ private void ValidateTransportException(ResponseMessage responseMessage)
125125
{
126126
Assert.AreEqual(HttpStatusCode.ServiceUnavailable, responseMessage.StatusCode);
127127
string message = responseMessage.ErrorMessage;
128-
Assert.AreEqual(responseMessage.ErrorMessage, responseMessage.CosmosException.Message);
129-
Assert.IsTrue(message.Contains("TransportException: A client transport error occurred: The connection failed"), "StoreResult Exception is missing");
128+
Assert.AreEqual(message, responseMessage.CosmosException.Message);
129+
Assert.IsTrue(message.Contains("ServiceUnavailable (503); Substatus: 0; ActivityId:"));
130+
Assert.IsTrue(message.Contains("Reason: (Message: Channel is closed"), "Should contain exception message");
130131
string diagnostics = responseMessage.Diagnostics.ToString();
131132
Assert.IsNotNull(diagnostics);
132133
Assert.IsTrue(diagnostics.Contains("TransportException: A client transport error occurred: The connection failed"));

‎Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Utils/TransportClientHelper.cs‎

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,12 @@ namespace Microsoft.Azure.Cosmos.SDK.EmulatorTests
77
using System;
88
using System.Collections.Generic;
99
using System.Diagnostics;
10+
using System.Globalization;
1011
using System.Text;
1112
using System.Threading.Tasks;
1213
using Microsoft.Azure.Cosmos.SDK.EmulatorTests;
1314
using Microsoft.Azure.Documents;
14-
15+
using Microsoft.Azure.Documents.Collections;
1516

1617
internal static class TransportClientHelper
1718
{
@@ -20,24 +21,35 @@ internal static Container GetContainerWithItemTransportException(
2021
string containerId,
2122
Guid activityId,
2223
string transportExceptionSourceDescription)
24+
{
25+
return GetContainerWithIntercepter(
26+
databaseId,
27+
containerId,
28+
(uri, resourceOperation, request) => TransportClientHelper.ThrowTransportExceptionOnItemOperation(
29+
uri,
30+
resourceOperation,
31+
request,
32+
activityId,
33+
transportExceptionSourceDescription));
34+
}
35+
36+
internal static Container GetContainerWithIntercepter(
37+
string databaseId,
38+
string containerId,
39+
Action<Uri, ResourceOperation, DocumentServiceRequest> interceptor)
2340
{
2441
CosmosClient clientWithIntercepter = TestCommon.CreateCosmosClient(
2542
builder =>
2643
{
2744
builder.WithTransportClientHandlerFactory(transportClient => new TransportClientWrapper(
2845
transportClient,
29-
(uri, resourceOperation, request) => TransportClientHelper.ThrowTransportExceptionOnItemOperation(
30-
uri,
31-
resourceOperation,
32-
request,
33-
activityId,
34-
transportExceptionSourceDescription)));
46+
interceptor));
3547
});
3648

3749
return clientWithIntercepter.GetContainer(databaseId, containerId);
3850
}
3951

40-
private static void ThrowTransportExceptionOnItemOperation(
52+
public static void ThrowTransportExceptionOnItemOperation(
4153
Uri physicalAddress,
4254
ResourceOperation resourceOperation,
4355
DocumentServiceRequest request,
@@ -60,6 +72,27 @@ private static void ThrowTransportExceptionOnItemOperation(
6072
}
6173
}
6274

75+
public static void ThrowForbiddendExceptionOnItemOperation(
76+
Uri physicalAddress,
77+
DocumentServiceRequest request,
78+
string activityId,
79+
string errorMessage)
80+
{
81+
if (request.ResourceType == ResourceType.Document)
82+
{
83+
DictionaryNameValueCollection headers = new DictionaryNameValueCollection();
84+
headers.Add(HttpConstants.HttpHeaders.ActivityId, activityId.ToString());
85+
headers.Add(WFConstants.BackendHeaders.SubStatus, ((int)SubStatusCodes.WriteForbidden).ToString(CultureInfo.InvariantCulture));
86+
87+
ForbiddenException forbiddenException = new ForbiddenException(
88+
errorMessage,
89+
headers,
90+
physicalAddress);
91+
92+
throw forbiddenException;
93+
}
94+
}
95+
6396
private sealed class TransportClientWrapper : TransportClient
6497
{
6598
private readonly TransportClient baseClient;

‎Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosDiagnosticsUnitTests.cs‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -126,14 +126,12 @@ public void ValidateClientSideRequestStatisticsToString()
126126

127127
CosmosClientSideRequestStatistics clientSideRequestStatistics = new CosmosClientSideRequestStatistics(diagnosticsContext);
128128
string noInfo = clientSideRequestStatistics.ToString();
129-
Assert.IsNotNull(noInfo);
129+
Assert.AreEqual("Please see CosmosDiagnostics", noInfo);
130130

131131
StringBuilder stringBuilder = new StringBuilder();
132132
clientSideRequestStatistics.AppendToBuilder(stringBuilder);
133133
string noInfoStringBuilder = stringBuilder.ToString();
134-
Assert.IsNotNull(noInfoStringBuilder);
135-
136-
Assert.AreEqual(noInfo, noInfoStringBuilder);
134+
Assert.AreEqual("Please see CosmosDiagnostics", noInfo);
137135

138136
string id = clientSideRequestStatistics.RecordAddressResolutionStart(new Uri("https://testuri"));
139137
clientSideRequestStatistics.RecordAddressResolutionEnd(id);
@@ -168,9 +166,7 @@ public void ValidateClientSideRequestStatisticsToString()
168166
usingLocalLSN: true));
169167

170168
string statistics = clientSideRequestStatistics.ToString();
171-
Assert.IsTrue(statistics.Contains("\"UserAgent\":\"cosmos-netstandard-sdk"));
172-
Assert.IsTrue(statistics.Contains("UsingLocalLSN: True, TransportException: null"));
173-
Assert.IsTrue(statistics.Contains("AddressResolutionStatistics\",\"StartTimeUtc"));
169+
Assert.AreEqual("Please see CosmosDiagnostics", statistics);
174170
}
175171

176172

0 commit comments

Comments
 (0)