Skip to content

Commit

Permalink
[Internal] Performance: Fixes exception serialization when tracing is…
Browse files Browse the repository at this point in the history
… not enabled (#1999)

* ToString only if tracing is enabled

* tests
  • Loading branch information
ealsur committed Nov 13, 2020
1 parent 64f2ff8 commit 91ed21c
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 4 deletions.
7 changes: 3 additions & 4 deletions Microsoft.Azure.Cosmos/src/ResourceThrottleRetryPolicy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,16 @@ private Task<ShouldRetryResult> ShouldRetryInternalAsync(TimeSpan? retryAfter)
}
}

private string GetExceptionMessage(Exception exception)
private object GetExceptionMessage(Exception exception)
{
DocumentClientException dce = exception as DocumentClientException;
if (dce != null && dce.StatusCode != null && (int)dce.StatusCode < (int)StatusCodes.InternalServerError)
if (exception is DocumentClientException dce && dce.StatusCode != null && (int)dce.StatusCode < (int)StatusCodes.InternalServerError)
{
// for client related errors, don't print out the whole call stack.
// simply return the message to prevent CPU overhead on ToString()
return exception.Message;
}

return exception.ToString();
return exception;
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
//------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
//------------------------------------------------------------

namespace Microsoft.Azure.Cosmos.Tests
{
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Threading.Tasks;
using Microsoft.Azure.Cosmos.Core.Trace;
using Microsoft.VisualStudio.TestTools.UnitTesting;

[TestClass]
public class ResourceThrottleRetryPolicyTests
{
private readonly List<TraceListener> existingListener = new List<TraceListener>();
private SourceSwitch existingSourceSwitch;

[TestInitialize]
public void CaptureCurrentTraceConfiguration()
{
foreach (TraceListener listener in DefaultTrace.TraceSource.Listeners)
{
this.existingListener.Add(listener);
}

DefaultTrace.TraceSource.Listeners.Clear();
this.existingSourceSwitch = DefaultTrace.TraceSource.Switch;
}

[TestCleanup]
public void ResetTraceConfiguration()
{
DefaultTrace.TraceSource.Listeners.Clear();
foreach (TraceListener listener in this.existingListener)
{
DefaultTrace.TraceSource.Listeners.Add(listener);
}

DefaultTrace.TraceSource.Switch = this.existingSourceSwitch;
}

[TestMethod]
public async Task DoesNotSerializeExceptionOnTracingDisabled()
{
// No listeners
ResourceThrottleRetryPolicy policy = new ResourceThrottleRetryPolicy(0);
CustomException exception = new CustomException();
await policy.ShouldRetryAsync(exception, default);
Assert.AreEqual(0, exception.ToStringCount, "Exception was serialized");
}

[TestMethod]
public async Task DoesSerializeExceptionOnTracingEnabled()
{
// Let the default trace listener
DefaultTrace.TraceSource.Switch = new SourceSwitch("ClientSwitch", "Error");
DefaultTrace.TraceSource.Listeners.Add(new DefaultTraceListener());
ResourceThrottleRetryPolicy policy = new ResourceThrottleRetryPolicy(0);
CustomException exception = new CustomException();
await policy.ShouldRetryAsync(exception, default);
Assert.AreEqual(1, exception.ToStringCount, "Exception was not serialized");
}

private class CustomException : Exception
{
public int ToStringCount { get; private set; } = 0;

public override string ToString()
{
++this.ToStringCount;
return string.Empty;
}
}
}
}

0 comments on commit 91ed21c

Please sign in to comment.