Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BREAKING CHANGE] DefaultTracing: Removes DefaultTraceListener to disable tracing by default #2926

Merged
merged 12 commits into from
Dec 10, 2021
68 changes: 67 additions & 1 deletion Microsoft.Azure.Cosmos/src/CosmosClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ namespace Microsoft.Azure.Cosmos
{
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Net;
using System.Text;
using System.Threading;
Expand Down Expand Up @@ -96,6 +97,8 @@ namespace Microsoft.Azure.Cosmos
/// <seealso href="https://docs.microsoft.com/azure/cosmos-db/request-units">Request Units</seealso>
public class CosmosClient : IDisposable
{
private static readonly object defaultTraceLockObject = new object();

private readonly string DatabaseRootUri = Paths.Databases_Root;
private ConsistencyLevel? accountConsistencyLevel;
private bool isDisposed = false;
Expand All @@ -119,6 +122,15 @@ static CosmosClient()
// NOTE: Native ServiceInteropWrapper.AssembliesExist has appsettings dependency which are proofed for CTL (native dll entry) scenarios.
// Revert of this depends on handling such in direct assembly
ServiceInteropWrapper.AssembliesExist = new Lazy<bool>(() => true);

Microsoft.Azure.Cosmos.Core.Trace.DefaultTrace.InitEventListener();

// If a debugger is not attached remove the DefaultTraceListener.
// DefaultTraceListener can cause lock contention leading to availability issues
if (!Debugger.IsAttached)
j82w marked this conversation as resolved.
Show resolved Hide resolved
{
CosmosClient.RemoveDefaultTraceListener();
j82w marked this conversation as resolved.
Show resolved Hide resolved
}
j82w marked this conversation as resolved.
Show resolved Hide resolved
}

/// <summary>
Expand Down Expand Up @@ -257,7 +269,7 @@ internal CosmosClient(
this.ClientContext = ClientContextCore.Create(
this,
clientOptions);

this.ClientConfigurationTraceDatum = new ClientConfigurationTraceDatum(this.ClientContext, DateTime.UtcNow);
}

Expand Down Expand Up @@ -1033,6 +1045,60 @@ public virtual Task<ResponseMessage> CreateDatabaseStreamAsync(
});
}

/// <summary>
j82w marked this conversation as resolved.
Show resolved Hide resolved
/// The DefaultTraceListener is removed from the CosmosClient's TraceSource unless
/// a debugger is attached.This prevents possible lock contention which can lead to
/// availability issues by requests waiting on the locks.
/// </summary>
/// <remarks>
/// This is enabled by default when Debugger.IsAttached is true. This makes it
/// easier to troubleshoot issues while debugging in Visual Studio.
/// </remarks>
public static void AddDefaultTraceListener()
j82w marked this conversation as resolved.
Show resolved Hide resolved
{
lock (CosmosClient.defaultTraceLockObject)
{
if (Core.Trace.DefaultTrace.TraceSource.Listeners.Count > 0)
{
foreach (object traceListnerObject in Core.Trace.DefaultTrace.TraceSource.Listeners)
{
// The TraceSource already has the default trace listener
if (traceListnerObject is DefaultTraceListener)
{
return;
}
}
}

Microsoft.Azure.Cosmos.Core.Trace.DefaultTrace.TraceSource.Listeners.Add(new DefaultTraceListener());
}
}

internal static void RemoveDefaultTraceListener()
{
lock (CosmosClient.defaultTraceLockObject)
{
if (Core.Trace.DefaultTrace.TraceSource.Listeners.Count > 0)
{
List<DefaultTraceListener> removeDefaultTraceListeners = new List<DefaultTraceListener>();
foreach (object traceListnerObject in Core.Trace.DefaultTrace.TraceSource.Listeners)
{
// The TraceSource already has the default trace listener
if (traceListnerObject is DefaultTraceListener defaultTraceListener)
{
removeDefaultTraceListeners.Add(defaultTraceListener);
}
}

// Remove all the default trace listeners
foreach (DefaultTraceListener defaultTraceListener in removeDefaultTraceListeners)
{
Core.Trace.DefaultTrace.TraceSource.Listeners.Remove(defaultTraceListener);
}
}
}
}

internal virtual async Task<ConsistencyLevel> GetAccountConsistencyLevelAsync()
{
if (!this.accountConsistencyLevel.HasValue)
Expand Down
2 changes: 0 additions & 2 deletions Microsoft.Azure.Cosmos/src/DocumentClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -646,8 +646,6 @@ internal virtual void Initialize(Uri serviceEndpoint,
throw new ArgumentNullException("serviceEndpoint");
}

DefaultTrace.InitEventListener();

this.queryPartitionProvider = new AsyncLazy<QueryPartitionProvider>(async () =>
{
await this.EnsureValidClientAsync(NoOpTrace.Singleton);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2184,6 +2184,11 @@
"Attributes": [],
"MethodInfo": "[Void .ctor(System.String, System.String, Microsoft.Azure.Cosmos.CosmosClientOptions), Void .ctor(System.String, System.String, Microsoft.Azure.Cosmos.CosmosClientOptions)]"
},
"Void AddDefaultTraceListener()": {
j82w marked this conversation as resolved.
Show resolved Hide resolved
"Type": "Method",
"Attributes": [],
"MethodInfo": "Void AddDefaultTraceListener();IsAbstract:False;IsStatic:True;IsVirtual:False;IsGenericMethod:False;IsConstructor:False;IsFinal:False;"
},
"Void Dispose()": {
"Type": "Method",
"Attributes": [],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
//------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
//------------------------------------------------------------
namespace Microsoft.Azure.Cosmos.Tests
{
using System;
using System.Diagnostics;
using System.Net.Http;
using System.Reflection;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Azure.Cosmos.Core.Trace;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Moq;

[TestClass]
public class DefaultTracingTests
{
[TestMethod]
public void DefaultTracingDisabledByDefault()
{
// Used to just force the CosmosClient static ctor to get called
Assert.IsTrue(CosmosClient.numberOfClientsCreated >= 0);

if (Debugger.IsAttached)
{
Assert.IsTrue(this.DefaultTraceHasDefaulTraceListener());
}
else
{
Assert.IsFalse(this.DefaultTraceHasDefaulTraceListener());
}
}

[TestMethod]
public void DefaultTracingEnableTest()
{
Assert.IsFalse(this.DefaultTraceHasDefaulTraceListener());
CosmosClient.AddDefaultTraceListener();
Assert.IsTrue(this.DefaultTraceHasDefaulTraceListener());
CosmosClient.RemoveDefaultTraceListener();
Assert.IsFalse(this.DefaultTraceHasDefaulTraceListener());
}

private bool DefaultTraceHasDefaulTraceListener()
{
if (DefaultTrace.TraceSource.Listeners.Count == 0)
{
return false;
}

foreach (TraceListener listener in DefaultTrace.TraceSource.Listeners)
{
if (listener is DefaultTraceListener)
{
return true;
}
}

DefaultTrace.TraceSource.Listeners.Clear();
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ void TraceHandler(string message)
{
Assert.IsFalse(failedToResolve, "Failure to resolve should happen only once.");
failedToResolve = true;
didNotRetryMessage = message.Substring(failedToResolveMessage.Length).Split('\n')[0];
didNotRetryMessage = message[failedToResolveMessage.Length..].Split('\n')[0];
}

if (failedToResolve && message.Contains("NOT be retried") && message.Contains(didNotRetryMessage))
Expand All @@ -71,24 +71,36 @@ void TraceHandler(string message)
Console.WriteLine(message);
}

DefaultTrace.TraceSource.Listeners.Add(new TestTraceListener { Callback = TraceHandler });
TestTraceListener testTraceListener = new TestTraceListener { Callback = TraceHandler };
DefaultTrace.TraceSource.Listeners.Add(testTraceListener);
DefaultTrace.InitEventListener();

try
{
DocumentClient myclient = new DocumentClient(new Uri(accountEndpoint), "base64encodedurl",
new ConnectionPolicy
{
});
try
{
DocumentClient myclient = new DocumentClient(new Uri(accountEndpoint), "base64encodedurl",
new ConnectionPolicy
{
});

await myclient.OpenAsync();
await myclient.OpenAsync();
}
catch
{
}

DefaultTrace.TraceSource.Flush();

// it should fail fast and not into the retry logic.
Assert.IsTrue(failedToResolve, "OpenAsync did not fail to resolve. No matching trace was received.");
Assert.IsTrue(didNotRetry, "OpenAsync did not fail without retrying. No matching trace was received.");
}
catch
finally
{

DefaultTrace.TraceSource.Listeners.Remove(testTraceListener);
}

// it should fail fast and not into the retry logic.
Assert.IsTrue(failedToResolve, "OpenAsync did not fail to resolve. No matching trace was received.");
Assert.IsTrue(didNotRetry, "OpenAsync did not fail without retrying. No matching trace was received.");
}

/// <summary>
Expand Down