From 74c28e489c506cb2fdc5f338e3bd80aaa62219f6 Mon Sep 17 00:00:00 2001 From: Sourabh Jain Date: Thu, 21 Sep 2023 16:15:50 +0530 Subject: [PATCH 1/4] first draft --- .../OpenTelemetry/OpenTelemetryCoreRecorder.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/OpenTelemetryCoreRecorder.cs b/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/OpenTelemetryCoreRecorder.cs index 2ef929ca8c..460025a830 100644 --- a/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/OpenTelemetryCoreRecorder.cs +++ b/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/OpenTelemetryCoreRecorder.cs @@ -8,6 +8,7 @@ namespace Microsoft.Azure.Cosmos.Telemetry using System.Collections.Generic; using System.Diagnostics; using global::Azure.Core; + using Microsoft.Azure.Cosmos.Telemetry.Diagnostics; /// /// This class is used to add information in an Activity tags ref. https://github.com/Azure/azure-cosmos-dotnet-v3/issues/3058 @@ -177,7 +178,14 @@ public void MarkFailed(Exception exception) this.scope.AddAttribute(OpenTelemetryAttributeKeys.ExceptionMessage, exception.Message); } - this.scope.Failed(exception); + if (DiagnosticsFilterHelper.IsSuccessfulResponse(this.response)) + { + this.scope.Dispose(); + } + else + { + this.scope.Failed(exception); + } } } From 88bf919b4a57ca07f17f414c9224f8b893eef4f5 Mon Sep 17 00:00:00 2001 From: Sourabh Jain Date: Fri, 22 Sep 2023 21:46:09 +0530 Subject: [PATCH 2/4] refactor --- .../OpenTelemetry/CosmosDbEventSource.cs | 2 +- .../Filters/DiagnosticsFilterHelper.cs | 16 ++--- .../OpenTelemetryCoreRecorder.cs | 11 ++- .../Tracing/AssertActivity.cs | 69 +++++++++++-------- .../Telemetry/DiagnosticsFilterHelperTest.cs | 3 +- 5 files changed, 58 insertions(+), 43 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/CosmosDbEventSource.cs b/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/CosmosDbEventSource.cs index 9e8028c0d8..8898cea96d 100644 --- a/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/CosmosDbEventSource.cs +++ b/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/CosmosDbEventSource.cs @@ -36,7 +36,7 @@ public static void RecordDiagnosticsForRequests( OpenTelemetryAttributes response) { if (!DiagnosticsFilterHelper.IsSuccessfulResponse( - response: response) && CosmosDbEventSource.IsEnabled(EventLevel.Warning)) + response.StatusCode, response.SubStatusCode) && CosmosDbEventSource.IsEnabled(EventLevel.Warning)) { CosmosDbEventSource.Singleton.FailedRequest(response.Diagnostics.ToString()); } diff --git a/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/Filters/DiagnosticsFilterHelper.cs b/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/Filters/DiagnosticsFilterHelper.cs index a5ca0215cb..d48392bb3e 100644 --- a/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/Filters/DiagnosticsFilterHelper.cs +++ b/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/Filters/DiagnosticsFilterHelper.cs @@ -5,8 +5,8 @@ namespace Microsoft.Azure.Cosmos.Telemetry.Diagnostics { using System; + using System.Net; using Documents; - using static Antlr4.Runtime.TokenStreamRewriter; internal static class DiagnosticsFilterHelper { @@ -39,13 +39,13 @@ public static bool IsLatencyThresholdCrossed( /// Check if response HTTP status code is returning successful /// /// true or false - public static bool IsSuccessfulResponse(OpenTelemetryAttributes response) - { - return response.StatusCode.IsSuccess() - || (response.StatusCode == System.Net.HttpStatusCode.NotFound && response.SubStatusCode == 0) - || (response.StatusCode == System.Net.HttpStatusCode.NotModified && response.SubStatusCode == 0) - || (response.StatusCode == System.Net.HttpStatusCode.Conflict && response.SubStatusCode == 0) - || (response.StatusCode == System.Net.HttpStatusCode.PreconditionFailed && response.SubStatusCode == 0); + public static bool IsSuccessfulResponse(HttpStatusCode statusCode, int substatusCode) + { + return statusCode.IsSuccess() + || (statusCode == System.Net.HttpStatusCode.NotFound && substatusCode == 0) + || (statusCode == System.Net.HttpStatusCode.NotModified && substatusCode == 0) + || (statusCode == System.Net.HttpStatusCode.Conflict && substatusCode == 0) + || (statusCode == System.Net.HttpStatusCode.PreconditionFailed && substatusCode == 0); } /// diff --git a/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/OpenTelemetryCoreRecorder.cs b/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/OpenTelemetryCoreRecorder.cs index 460025a830..507d3d097d 100644 --- a/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/OpenTelemetryCoreRecorder.cs +++ b/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/OpenTelemetryCoreRecorder.cs @@ -178,15 +178,22 @@ public void MarkFailed(Exception exception) this.scope.AddAttribute(OpenTelemetryAttributeKeys.ExceptionMessage, exception.Message); } - if (DiagnosticsFilterHelper.IsSuccessfulResponse(this.response)) + if (exception is CosmosException cosmosException + && DiagnosticsFilterHelper + .IsSuccessfulResponse(cosmosException.StatusCode, cosmosException.SubStatusCode)) { this.scope.Dispose(); } else { + Console.WriteLine(exception); this.scope.Failed(exception); } } + else + { + this.activity?.Stop(); + } } /// @@ -213,7 +220,7 @@ internal static bool IsExceptionRegistered(Exception exception, DiagnosticScope public void Dispose() { - if (this.scope.IsEnabled) + if (this.IsEnabled) { Documents.OperationType operationType = (this.response == null || this.response?.OperationType == Documents.OperationType.Invalid) ? this.operationType : this.response.OperationType; diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/AssertActivity.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/AssertActivity.cs index 75153d3b88..c08c768779 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/AssertActivity.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/AssertActivity.cs @@ -8,8 +8,9 @@ namespace Microsoft.Azure.Cosmos.Tracing using System.Collections.Generic; using System.Diagnostics; using System.Linq; - using global::Azure; + using System.Net; using Microsoft.Azure.Cosmos.Telemetry; + using Microsoft.Azure.Cosmos.Telemetry.Diagnostics; using Microsoft.Azure.Cosmos.Tests; using Microsoft.VisualStudio.TestTools.UnitTesting; using Newtonsoft.Json; @@ -32,35 +33,35 @@ public static void IsValidOperationActivity(Activity activity) } IList expectedTags = new List - { - "az.namespace", - "az.schema_url", - "kind", - "db.system", - "db.name", - "db.operation", - "net.peer.name", - "db.cosmosdb.client_id", - "db.cosmosdb.machine_id", - "user_agent.original", - "db.cosmosdb.connection_mode", - "db.cosmosdb.operation_type", - "db.cosmosdb.container", - "db.cosmosdb.request_content_length_bytes", - "db.cosmosdb.response_content_length_bytes", - "db.cosmosdb.status_code", - "db.cosmosdb.sub_status_code", - "db.cosmosdb.request_charge", - "db.cosmosdb.regions_contacted", - "db.cosmosdb.retry_count", - "db.cosmosdb.item_count", - "db.cosmosdb.request_diagnostics", - "exception.type", - "exception.message", - "exception.stacktrace", - "db.cosmosdb.activity_id", - "db.cosmosdb.correlated_activity_id" - }; + { + "az.namespace", + "az.schema_url", + "kind", + "db.system", + "db.name", + "db.operation", + "net.peer.name", + "db.cosmosdb.client_id", + "db.cosmosdb.machine_id", + "user_agent.original", + "db.cosmosdb.connection_mode", + "db.cosmosdb.operation_type", + "db.cosmosdb.container", + "db.cosmosdb.request_content_length_bytes", + "db.cosmosdb.response_content_length_bytes", + "db.cosmosdb.status_code", + "db.cosmosdb.sub_status_code", + "db.cosmosdb.request_charge", + "db.cosmosdb.regions_contacted", + "db.cosmosdb.retry_count", + "db.cosmosdb.item_count", + "db.cosmosdb.request_diagnostics", + "exception.type", + "exception.message", + "exception.stacktrace", + "db.cosmosdb.activity_id", + "db.cosmosdb.correlated_activity_id" + }; foreach (KeyValuePair actualTag in activity.Tags) { @@ -68,6 +69,14 @@ public static void IsValidOperationActivity(Activity activity) AssertActivity.AssertDatabaseAndContainerName(activity.OperationName, actualTag); } + + HttpStatusCode statusCode = (HttpStatusCode)Convert.ToInt32(activity.GetTagItem("db.cosmosdb.status_code")); + int subStatusCode = Convert.ToInt32(activity.GetTagItem("db.cosmosdb.sub_status_code")); + if (!DiagnosticsFilterHelper.IsSuccessfulResponse(statusCode, subStatusCode)) + { + Console.WriteLine(statusCode + " " + subStatusCode); + Assert.AreEqual(ActivityStatusCode.Error, activity.Status); + } } } diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Telemetry/DiagnosticsFilterHelperTest.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Telemetry/DiagnosticsFilterHelperTest.cs index e345394265..5e32b26da1 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Telemetry/DiagnosticsFilterHelperTest.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Telemetry/DiagnosticsFilterHelperTest.cs @@ -71,11 +71,10 @@ public void CheckReturnTrueOnFailedStatusCode() Assert.IsTrue( !DiagnosticsFilterHelper - .IsSuccessfulResponse(response), + .IsSuccessfulResponse(response.StatusCode, response.SubStatusCode), $" Response time is {response.Diagnostics.GetClientElapsedTime().Milliseconds}ms " + $"and Configured threshold value is {distributedTracingOptions.LatencyThresholdForDiagnosticEvent.Value.Milliseconds}ms " + $"and Is response Success : {response.StatusCode.IsSuccess()}"); - } } From 84c32b539a632540ae1fee1fa748fa865bb78704 Mon Sep 17 00:00:00 2001 From: Sourabh Jain Date: Fri, 22 Sep 2023 23:46:14 +0530 Subject: [PATCH 3/4] fix tests --- .../OpenTelemetry/OpenTelemetryCoreRecorder.cs | 16 ++++++---------- .../Tracing/AssertActivity.cs | 1 - 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/OpenTelemetryCoreRecorder.cs b/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/OpenTelemetryCoreRecorder.cs index 507d3d097d..6b5d167ade 100644 --- a/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/OpenTelemetryCoreRecorder.cs +++ b/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/OpenTelemetryCoreRecorder.cs @@ -179,21 +179,12 @@ public void MarkFailed(Exception exception) } if (exception is CosmosException cosmosException - && DiagnosticsFilterHelper + && !DiagnosticsFilterHelper .IsSuccessfulResponse(cosmosException.StatusCode, cosmosException.SubStatusCode)) { - this.scope.Dispose(); - } - else - { - Console.WriteLine(exception); this.scope.Failed(exception); } } - else - { - this.activity?.Stop(); - } } /// @@ -243,6 +234,11 @@ Documents.OperationType operationType this.scope.AddAttribute(OpenTelemetryAttributeKeys.Region, ClientTelemetryHelper.GetContactedRegions(this.response.Diagnostics.GetContactedRegions())); CosmosDbEventSource.RecordDiagnosticsForRequests(this.config, operationType, this.response); } + + if (!DiagnosticsFilterHelper.IsSuccessfulResponse(this.response.StatusCode, this.response.SubStatusCode)) + { + this.scope.Failed(); + } } this.scope.Dispose(); diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/AssertActivity.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/AssertActivity.cs index c08c768779..9b9be4479a 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/AssertActivity.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Tracing/AssertActivity.cs @@ -74,7 +74,6 @@ public static void IsValidOperationActivity(Activity activity) int subStatusCode = Convert.ToInt32(activity.GetTagItem("db.cosmosdb.sub_status_code")); if (!DiagnosticsFilterHelper.IsSuccessfulResponse(statusCode, subStatusCode)) { - Console.WriteLine(statusCode + " " + subStatusCode); Assert.AreEqual(ActivityStatusCode.Error, activity.Status); } } From 98146ef13384a3e5fde1493751876bc561c06ae3 Mon Sep 17 00:00:00 2001 From: Sourabh Jain Date: Mon, 25 Sep 2023 07:09:43 +0530 Subject: [PATCH 4/4] fixed condition --- .../src/Telemetry/OpenTelemetry/OpenTelemetryCoreRecorder.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/OpenTelemetryCoreRecorder.cs b/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/OpenTelemetryCoreRecorder.cs index 6b5d167ade..ced441d368 100644 --- a/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/OpenTelemetryCoreRecorder.cs +++ b/Microsoft.Azure.Cosmos/src/Telemetry/OpenTelemetry/OpenTelemetryCoreRecorder.cs @@ -178,9 +178,9 @@ public void MarkFailed(Exception exception) this.scope.AddAttribute(OpenTelemetryAttributeKeys.ExceptionMessage, exception.Message); } - if (exception is CosmosException cosmosException + if (exception is not CosmosException || (exception is CosmosException cosmosException && !DiagnosticsFilterHelper - .IsSuccessfulResponse(cosmosException.StatusCode, cosmosException.SubStatusCode)) + .IsSuccessfulResponse(cosmosException.StatusCode, cosmosException.SubStatusCode))) { this.scope.Failed(exception); }