Distributed Tracing: Fixes dependency failure on appinsights (#4098)

* first draft * refactor * fix tests * fixed condition
Azure · Sep 26, 2023 · 08e7f54 · 08e7f54
1 parent 9f8d848
commit 08e7f54
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 43 deletions.
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
@@ -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;
 
     /// <summary>
     /// 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,12 @@ public void MarkFailed(Exception exception)
                     this.scope.AddAttribute(OpenTelemetryAttributeKeys.ExceptionMessage, exception.Message);
                 }
 
-                this.scope.Failed(exception);
+                if (exception is not CosmosException || (exception is CosmosException cosmosException
+                            && !DiagnosticsFilterHelper
+                                    .IsSuccessfulResponse(cosmosException.StatusCode, cosmosException.SubStatusCode)))
+                {
+                    this.scope.Failed(exception);
+                }
             }
         }
 
@@ -205,7 +211,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;
@@ -228,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
@@ -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,49 @@ 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))
+                {
+                    Assert.AreEqual(ActivityStatusCode.Error, activity.Status);
+                }
             }
         }
 

diff --git a/....Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Telemetry/DiagnosticsFilterHelperTest.cs b/....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()}");
-
         }
 
     }