refactor

sourabh1007 · sourabh1007 · commit 88bf919b4a57 · 2023-09-22T21:59:53.000+05:30
diff --git 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
@@ -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
         /// </summary>
         /// <returns>true or false</returns>
-        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);
         }
 
         /// <summary>
diff --git 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();
+            }
         }
 
         /// <summary>
@@ -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
@@ -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,42 +33,50 @@ public static void IsValidOperationActivity(Activity activity)
                 }
 
                 IList<string> expectedTags = new List<string>
-            {
-                 "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<string, string> actualTag in activity.Tags)
                 {
                     Assert.IsTrue(expectedTags.Contains(actualTag.Key), $"{actualTag.Key} is not allowed for {activity.OperationName}");
 
                     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
@@ -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()}");
-
         }
 
     }

Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@ public static void RecordDiagnosticsForRequests(`
`36`	`36`	`OpenTelemetryAttributes response)`
`37`	`37`	`{`
`38`	`38`	`if (!DiagnosticsFilterHelper.IsSuccessfulResponse(`
`39`		`- response: response) && CosmosDbEventSource.IsEnabled(EventLevel.Warning))`
	`39`	`+ response.StatusCode, response.SubStatusCode) && CosmosDbEventSource.IsEnabled(EventLevel.Warning))`
`40`	`40`	`{`
`41`	`41`	`CosmosDbEventSource.Singleton.FailedRequest(response.Diagnostics.ToString());`
`42`	`42`	`}`
Original file line number	Diff line number	Diff line change
`@@ -178,15 +178,22 @@ public void MarkFailed(Exception exception)`
`178`	`178`	`this.scope.AddAttribute(OpenTelemetryAttributeKeys.ExceptionMessage, exception.Message);`
`179`	`179`	`}`
`180`	`180`
`181`		`- if (DiagnosticsFilterHelper.IsSuccessfulResponse(this.response))`
	`181`	`+ if (exception is CosmosException cosmosException`
	`182`	`+ && DiagnosticsFilterHelper`
	`183`	`+ .IsSuccessfulResponse(cosmosException.StatusCode, cosmosException.SubStatusCode))`
`182`	`184`	`{`
`183`	`185`	`this.scope.Dispose();`
`184`	`186`	`}`
`185`	`187`	`else`
`186`	`188`	`{`
	`189`	`+ Console.WriteLine(exception);`
`187`	`190`	`this.scope.Failed(exception);`
`188`	`191`	`}`
`189`	`192`	`}`
	`193`	`+ else`
	`194`	`+ {`
	`195`	`+ this.activity?.Stop();`
	`196`	`+ }`
`190`	`197`	`}`
`191`	`198`
`192`	`199`	`/// <summary>`
`@@ -213,7 +220,7 @@ internal static bool IsExceptionRegistered(Exception exception, DiagnosticScope`
`213`	`220`
`214`	`221`	`public void Dispose()`
`215`	`222`	`{`
`216`		`- if (this.scope.IsEnabled)`
	`223`	`+ if (this.IsEnabled)`
`217`	`224`	`{`
`218`	`225`	`Documents.OperationType operationType`
`219`	`226`	`= (this.response == null \|\| this.response?.OperationType == Documents.OperationType.Invalid) ? this.operationType : this.response.OperationType;`
Original file line number	Diff line number	Diff line change
`@@ -71,11 +71,10 @@ public void CheckReturnTrueOnFailedStatusCode()`
`71`	`71`
`72`	`72`	`Assert.IsTrue(`
`73`	`73`	`!DiagnosticsFilterHelper`
`74`		`- .IsSuccessfulResponse(response),`
	`74`	`+ .IsSuccessfulResponse(response.StatusCode, response.SubStatusCode),`
`75`	`75`	`$" Response time is {response.Diagnostics.GetClientElapsedTime().Milliseconds}ms " +`
`76`	`76`	`$"and Configured threshold value is {distributedTracingOptions.LatencyThresholdForDiagnosticEvent.Value.Milliseconds}ms " +`
`77`	`77`	`$"and Is response Success : {response.StatusCode.IsSuccess()}");`
`78`		`-`
`79`	`78`	`}`
`80`	`79`
`81`	`80`	`}`