Skip to content

Commit

Permalink
CosmosClient: Fixes missing Trace when converting HTTP Timeout to 503 (…
Browse files Browse the repository at this point in the history 
…#3866)

* Added tracing when converting HTTP Timeout to 503

* Fixed tracing when converting HTTP Timeout to 503

* Resolved PR comments

* Using ITrace as part of ClientSideRequestStatisticsTraceDatum

* Refactoring

* Test update

* Unit tests fix
  • Loading branch information
@rinatmini
rinatmini authored Jun 1, 2023
1 parent 327422b commit 3e3bbe5
Show file tree
Hide file tree
Showing 15 changed files with 121 additions and 78 deletions.
2 changes: 1 addition & 1 deletion Microsoft.Azure.Cosmos/src/GatewayAccountReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ await this.cosmosAuthorization.AddAuthorizationHeaderAsync(

using (ITrace trace = Trace.GetRootTrace("Account Read", TraceComponent.Transport, TraceLevel.Info))
{
IClientSideRequestStatistics stats = new ClientSideRequestStatisticsTraceDatum(DateTime.UtcNow, trace.Summary);
IClientSideRequestStatistics stats = new ClientSideRequestStatisticsTraceDatum(DateTime.UtcNow, trace);

try
{
Expand Down
2 changes: 1 addition & 1 deletion Microsoft.Azure.Cosmos/src/Handler/TransportHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ internal async Task<ResponseMessage> ProcessMessageAsync(

DocumentServiceRequest serviceRequest = request.ToDocumentServiceRequest();

ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum = new ClientSideRequestStatisticsTraceDatum(DateTime.UtcNow, request.Trace.Summary);
ClientSideRequestStatisticsTraceDatum clientSideRequestStatisticsTraceDatum = new ClientSideRequestStatisticsTraceDatum(DateTime.UtcNow, request.Trace);
serviceRequest.RequestContext.ClientRequestStatistics = clientSideRequestStatisticsTraceDatum;

//TODO: extrace auth into a separate handler
Expand Down
4 changes: 4 additions & 0 deletions Microsoft.Azure.Cosmos/src/HttpClient/CosmosHttpClientCore.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ namespace Microsoft.Azure.Cosmos
using System.Threading.Tasks;
using Microsoft.Azure.Cosmos.Core.Trace;
using Microsoft.Azure.Cosmos.Resource.CosmosExceptions;
using Microsoft.Azure.Cosmos.Tracing;
using Microsoft.Azure.Cosmos.Tracing.TraceData;
using Microsoft.Azure.Documents;
using Microsoft.Azure.Documents.Collections;
Expand Down Expand Up @@ -361,9 +362,11 @@ private async Task<HttpResponseMessage> SendHttpHelperAsync(
}
catch (Exception e)
{
ITrace trace = NoOpTrace.Singleton;
if (clientSideRequestStatistics is ClientSideRequestStatisticsTraceDatum datum)
{
datum.RecordHttpException(requestMessage, e, resourceType, requestStartTime);
trace = datum.Trace;
}
bool isOutOfRetries = CosmosHttpClientCore.IsOutOfRetries(timeoutPolicy, startDateTimeUtc, timeoutEnumerator);

Expand Down Expand Up @@ -394,6 +397,7 @@ private async Task<HttpResponseMessage> SendHttpHelperAsync(
ActivityId = System.Diagnostics.Trace.CorrelationManager.ActivityId.ToString(),
SubStatusCode = SubStatusCodes.TransportGenerated503
},
trace: trace,
innerException: e);
}

Expand Down
2 changes: 1 addition & 1 deletion Microsoft.Azure.Cosmos/src/Routing/ClientCollectionCache.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ private async Task<ContainerProperties> ReadCollectionAsync(
{
headers.XDate = Rfc1123DateTimeCache.UtcNow();

request.RequestContext.ClientRequestStatistics = clientSideRequestStatistics ?? new ClientSideRequestStatisticsTraceDatum(DateTime.UtcNow, trace.Summary);
request.RequestContext.ClientRequestStatistics = clientSideRequestStatistics ?? new ClientSideRequestStatisticsTraceDatum(DateTime.UtcNow, trace);
if (clientSideRequestStatistics == null)
{
childTrace.AddDatum(
Expand Down
2 changes: 1 addition & 1 deletion Microsoft.Azure.Cosmos/src/Routing/PartitionKeyRangeCache.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ private async Task<DocumentServiceResponse> ExecutePartitionKeyRangeReadChangeFe
}

request.Headers[HttpConstants.HttpHeaders.Authorization] = authorizationToken;
request.RequestContext.ClientRequestStatistics = clientSideRequestStatistics ?? new ClientSideRequestStatisticsTraceDatum(DateTime.UtcNow, trace.Summary);
request.RequestContext.ClientRequestStatistics = clientSideRequestStatistics ?? new ClientSideRequestStatisticsTraceDatum(DateTime.UtcNow, trace);
if (clientSideRequestStatistics == null)
{
childTrace.AddDatum("Client Side Request Stats", request.RequestContext.ClientRequestStatistics);
Expand Down
13 changes: 8 additions & 5 deletions Microsoft.Azure.Cosmos/src/Tracing/TraceData/ClientSideRequestStatisticsTraceDatum.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,8 @@ internal sealed class ClientSideRequestStatisticsTraceDatum : TraceDatum, IClien
private IReadOnlyList<StoreResponseStatistics> shallowCopyOfStoreResponseStatistics = null;
private IReadOnlyList<HttpResponseStatistics> shallowCopyOfHttpResponseStatistics = null;
private SystemUsageHistory systemUsageHistory = null;
public TraceSummary TraceSummary = null;

public ClientSideRequestStatisticsTraceDatum(DateTime startTime, TraceSummary summary)
public ClientSideRequestStatisticsTraceDatum(DateTime startTime, ITrace trace)
{
this.RequestStartTimeUtc = startTime;
this.RequestEndTimeUtc = null;
Expand All @@ -45,7 +44,7 @@ public ClientSideRequestStatisticsTraceDatum(DateTime startTime, TraceSummary su
this.FailedReplicas = new HashSet<TransportAddressUri>();
this.RegionsContacted = new HashSet<(string, Uri)>();
this.httpResponseStatistics = new List<HttpResponseStatistics>();
this.TraceSummary = summary;
this.Trace = trace;
}

public DateTime RequestStartTimeUtc { get; }
Expand Down Expand Up @@ -75,6 +74,10 @@ public IReadOnlyDictionary<string, AddressResolutionStatistics> EndpointToAddres

public HashSet<(string, Uri)> RegionsContacted { get; }

public ITrace Trace { get; private set; }

public TraceSummary TraceSummary => this.Trace?.Summary;

public IReadOnlyList<StoreResponseStatistics> StoreResponseStatisticsList
{
get
Expand Down Expand Up @@ -502,11 +505,11 @@ public HttpResponseStatistics(
if (responseMessage != null)
{
Headers headers = new Headers(GatewayStoreClient.ExtractResponseHeaders(responseMessage));
this.ActivityId = headers.ActivityId ?? Trace.CorrelationManager.ActivityId.ToString();
this.ActivityId = headers.ActivityId ?? System.Diagnostics.Trace.CorrelationManager.ActivityId.ToString();
}
else
{
this.ActivityId = Trace.CorrelationManager.ActivityId.ToString();
this.ActivityId = System.Diagnostics.Trace.CorrelationManager.ActivityId.ToString();
}
}

Expand Down
2 changes: 1 addition & 1 deletion ...rosoft.Azure.Cosmos.Performance.Tests/Benchmarks/RegionContactedInDiagnosticsBenchmark.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ private ITrace CreateTestTraceTree()

private TraceDatum GetDatumObject(string regionName1, string regionName2 = null)
{
ClientSideRequestStatisticsTraceDatum datum = new ClientSideRequestStatisticsTraceDatum(DateTime.UtcNow, new TraceSummary());
ClientSideRequestStatisticsTraceDatum datum = new ClientSideRequestStatisticsTraceDatum(DateTime.UtcNow, Trace.GetRootTrace(nameof(RegionContactedInDiagnosticsBenchmark)));
Uri uri1 = new Uri("http://someUri1.com");
datum.RegionsContacted.Add((regionName1, uri1));
if (regionName2 != null)
Expand Down
6 changes: 3 additions & 3 deletions ...osoft.Azure.Cosmos.Tests/BaselineTest/TestBaseline/TraceWriterBaselineTests.TraceData.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@
TraceForBaselineTesting rootTrace;
using (rootTrace = TraceForBaselineTesting.GetRootTrace())
{
ClientSideRequestStatisticsTraceDatum datum = new ClientSideRequestStatisticsTraceDatum(DateTime.MinValue, new TraceSummary());
ClientSideRequestStatisticsTraceDatum datum = new ClientSideRequestStatisticsTraceDatum(DateTime.MinValue, rootTrace);
TransportAddressUri uri1 = new TransportAddressUri(new Uri("http://someUri1.com"));
TransportAddressUri uri2 = new TransportAddressUri(new Uri("http://someUri2.com"));
Expand Down Expand Up @@ -405,7 +405,7 @@
TraceForBaselineTesting rootTrace;
using (rootTrace = TraceForBaselineTesting.GetRootTrace())
{
ClientSideRequestStatisticsTraceDatum datum = new ClientSideRequestStatisticsTraceDatum(DateTime.MinValue, new TraceSummary());
ClientSideRequestStatisticsTraceDatum datum = new ClientSideRequestStatisticsTraceDatum(DateTime.MinValue, rootTrace);
datum.ContactedReplicas.Add(default);
TraceWriterBaselineTests.GetPrivateField<Dictionary<string, AddressResolutionStatistics>>(datum, "endpointToAddressResolutionStats").Add("asdf", default);
Expand Down Expand Up @@ -546,7 +546,7 @@
TraceForBaselineTesting rootTrace;
using (rootTrace = TraceForBaselineTesting.GetRootTrace())
{
ClientSideRequestStatisticsTraceDatum datum = new ClientSideRequestStatisticsTraceDatum(DateTime.MinValue, new TraceSummary());
ClientSideRequestStatisticsTraceDatum datum = new ClientSideRequestStatisticsTraceDatum(DateTime.MinValue, rootTrace);
TraceWriterBaselineTests.SetEndRequestTime(datum,DateTime.MaxValue);
HttpResponseStatistics httpResponseStatistics = new HttpResponseStatistics(
Expand Down
Loading

0 comments on commit 3e3bbe5

Please sign in to comment.