Tracing: http semconv update (#39617)

* Adopt HTTP semconv 1.23.0

sdk/core/Azure.Core/CHANGELOG.md

@@ -6,6 +6,8 @@
 
 ### Breaking Changes
 
+- Updated tracing attributes names to conform to OpenTelemetry semantic conventions version to 1.23.0.
+
 ### Bugs Fixed
 
 ### Other Changes

sdk/core/Azure.Core/src/Pipeline/Internal/RequestActivityPolicy.cs

@@ -62,27 +62,28 @@ private async ValueTask ProcessAsync(HttpMessage message, ReadOnlyMemory<HttpPip
             using var scope = CreateDiagnosticScope(message);
 
             bool isActivitySourceEnabled = IsActivitySourceEnabled;
+            scope.SetDisplayName(message.Request.Method.Method);
 
-            scope.AddAttribute("http.method", message.Request.Method.Method);
-            scope.AddAttribute("http.url", _sanitizer.SanitizeUrl(message.Request.Uri.ToString()));
+            scope.AddAttribute(isActivitySourceEnabled ? "http.request.method" : "http.method", message.Request.Method.Method);
+            scope.AddAttribute(isActivitySourceEnabled ? "url.full" : "http.url", _sanitizer.SanitizeUrl(message.Request.Uri.ToString()));
             scope.AddAttribute(isActivitySourceEnabled ? "az.client_request_id": "requestId", message.Request.ClientRequestId);
+            if (message.RetryNumber > 0)
+            {
+                scope.AddIntegerAttribute("http.request.resend_count", message.RetryNumber);
+            }
 
             if (isActivitySourceEnabled && message.Request.Uri.Host is string host)
             {
-                scope.AddAttribute("net.peer.name", host);
-                int port = message.Request.Uri.Port;
-                if (port != 443)
-                {
-                    scope.AddIntegerAttribute("net.peer.port", port);
-                }
+                scope.AddAttribute("server.address", host);
+                scope.AddIntegerAttribute("server.port", message.Request.Uri.Port);
             }
 
             if (_resourceProviderNamespace != null)
             {
                 scope.AddAttribute("az.namespace", _resourceProviderNamespace);
             }
 
-            if (message.Request.Headers.TryGetValue("User-Agent", out string? userAgent))
+            if (!isActivitySourceEnabled && message.Request.Headers.TryGetValue("User-Agent", out string? userAgent))
             {
                 scope.AddAttribute("http.user_agent", userAgent);
             }
@@ -106,13 +107,14 @@ private async ValueTask ProcessAsync(HttpMessage message, ReadOnlyMemory<HttpPip
                 throw;
             }
 
+            string statusCodeStr = message.Response.Status.ToString(CultureInfo.InvariantCulture);
             if (isActivitySourceEnabled)
             {
-                scope.AddIntegerAttribute("http.status_code", message.Response.Status);
+                scope.AddIntegerAttribute("http.response.status_code", message.Response.Status);
             }
             else
             {
-                scope.AddAttribute("http.status_code", message.Response.Status, static i => i.ToString(CultureInfo.InvariantCulture));
+                scope.AddAttribute("http.status_code", statusCodeStr);
             }
 
             if (message.Response.Headers.RequestId is string serviceRequestId)
@@ -123,10 +125,17 @@ private async ValueTask ProcessAsync(HttpMessage message, ReadOnlyMemory<HttpPip
 
             if (message.Response.IsError)
             {
-                scope.AddAttribute("otel.status_code", "ERROR");
-                scope.Failed();
+                if (isActivitySourceEnabled)
+                {
+                    scope.AddAttribute("error.type", statusCodeStr);
+                }
+                else
+                {
+                    scope.AddAttribute("otel.status_code", "ERROR");
+                }
+                scope.Failed(statusCodeStr);
             }
-            else
+            else if (!isActivitySourceEnabled)
             {
                 // Set the status to UNSET so the AppInsights doesn't try to infer it from the status code
                 scope.AddAttribute("otel.status_code",  "UNSET");

sdk/core/Azure.Core/src/Shared/DiagnosticScope.cs

@@ -10,6 +10,7 @@
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Linq.Expressions;
+using System.Net.Http;
 using System.Reflection;
 
 namespace Azure.Core.Pipeline
@@ -18,7 +19,7 @@ namespace Azure.Core.Pipeline
     {
         private const string AzureSdkScopeLabel = "az.sdk.scope";
         internal const string OpenTelemetrySchemaAttribute = "az.schema_url";
-        internal const string OpenTelemetrySchemaVersion = "https://opentelemetry.io/schemas/1.17.0";
+        internal const string OpenTelemetrySchemaVersion = "https://opentelemetry.io/schemas/1.23.0";
         private static readonly object AzureSdkScopeValue = bool.TrueString;
 
         private readonly ActivityAdapter? _activityAdapter;
@@ -140,9 +141,28 @@ public void Dispose()
         [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026", Justification = "The Exception being passed into this method has public properties preserved on the inner method MarkFailed." +
             "The public property System.Exception.TargetSite.get is not compatible with trimming and produces a warning when preserving all public properties. Since we do not use this property, and" +
             "neither does Application Insights, we can suppress the warning coming from the inner method.")]
-        public void Failed(Exception? exception = default)
+        public void Failed(Exception exception)
         {
-            _activityAdapter?.MarkFailed(exception);
+            if (exception is RequestFailedException requestFailedException)
+            {
+                // TODO (limolkova) when we start targeting .NET 8 we should put
+                // requestFailedException.InnerException.HttpRequestError into error.type
+
+                _activityAdapter?.MarkFailed(exception, requestFailedException.ErrorCode);
+            }
+            else
+            {
+                _activityAdapter?.MarkFailed(exception, null);
+            }
+        }
+
+        /// <summary>
+        /// Marks the scope as failed with low-cardinality error.type attribute.
+        /// </summary>
+        /// <param name="errorCode">Error code to associate with the failed scope.</param>
+        public void Failed(string errorCode)
+        {
+            _activityAdapter?.MarkFailed((Exception?)null, errorCode);
         }
 
 #if NETCOREAPP2_1
@@ -500,7 +520,7 @@ public void SetStartTime(DateTime startTime)
             }
 
             [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026", Justification = "The Exception being passed into this method has the commonly used properties being preserved with DynamicallyAccessedMemberTypes.")]
-            public void MarkFailed<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T>(T exception)
+            public void MarkFailed<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T>(T? exception, string? errorCode)
             {
                 if (exception != null)
                 {
@@ -514,6 +534,12 @@ public void SetStartTime(DateTime startTime)
                 }
 #endif
 #if NET6_0_OR_GREATER // SetStatus is only defined in NET 6 or greater
+                if (errorCode == null && exception != null)
+                {
+                    errorCode = exception.GetType().FullName;
+                }
+
+                _currentActivity?.AddTag("error.type", errorCode ?? "_OTHER");
                 _currentActivity?.SetStatus(ActivityStatusCode.Error, exception?.ToString());
 #endif
             }

sdk/core/Azure.Core/tests/ClientDiagnosticsTests.Net50.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
+using System.Net.Http;
 using Azure.Core.Pipeline;
 using Azure.Core.TestFramework;
 using NUnit.Framework;
@@ -531,6 +532,63 @@ public void OpenTelemetryCompatibilityWithCustomSampler()
             Assert.AreEqual(4, activeActivityCounts); // 1 activity will be dropped due to sampler logic
         }
 
+        [Test]
+        [NonParallelizable]
+        public void FailedStopsActivityAndWritesErrorTypeException()
+        {
+            using var _ = SetAppConfigSwitch();
+
+            using var testListener = new TestActivitySourceListener("Azure.Clients");
+            DiagnosticScopeFactory clientDiagnostics = new DiagnosticScopeFactory("Azure.Clients", "Microsoft.Azure.Core.Cool.Tests", true, false);
+
+            using DiagnosticScope scope = clientDiagnostics.CreateScope("ActivityName");
+            scope.Start();
+            scope.Failed(new ArgumentException());
+
+            Activity activity = testListener.AssertAndRemoveActivity("ActivityName");
+            Assert.IsEmpty(activity.Events);
+            CollectionAssert.Contains(activity.Tags, new KeyValuePair<string, string>("error.type", typeof(ArgumentException).FullName));
+            Assert.AreEqual(ActivityStatusCode.Error, activity.Status);
+        }
+
+        [Test]
+        [NonParallelizable]
+        public void FailedStopsActivityAndWritesErrorTypeRequestException()
+        {
+            using var _ = SetAppConfigSwitch();
+
+            using var testListener = new TestActivitySourceListener("Azure.Clients");
+            DiagnosticScopeFactory clientDiagnostics = new DiagnosticScopeFactory("Azure.Clients", "Microsoft.Azure.Core.Cool.Tests", true, false);
+
+            using DiagnosticScope scope = clientDiagnostics.CreateScope("ActivityName");
+            scope.Start();
+            scope.Failed(new RequestFailedException(400, "error", "errorCode", new HttpRequestException()));
+
+            Activity activity = testListener.AssertAndRemoveActivity("ActivityName");
+            Assert.IsEmpty(activity.Events);
+            CollectionAssert.Contains(activity.Tags, new KeyValuePair<string, string>("error.type", "errorCode"));
+            Assert.AreEqual(ActivityStatusCode.Error, activity.Status);
+        }
+
+        [Test]
+        [NonParallelizable]
+        public void FailedStopsActivityAndWritesErrorTypeString()
+        {
+            using var _ = SetAppConfigSwitch();
+
+            using var testListener = new TestActivitySourceListener("Azure.Clients");
+            DiagnosticScopeFactory clientDiagnostics = new DiagnosticScopeFactory("Azure.Clients", "Microsoft.Azure.Core.Cool.Tests", true, false);
+
+            using DiagnosticScope scope = clientDiagnostics.CreateScope("ActivityName");
+            scope.Start();
+            scope.Failed("errorCode");
+
+            Activity activity = testListener.AssertAndRemoveActivity("ActivityName");
+            Assert.IsEmpty(activity.Events);
+            CollectionAssert.Contains(activity.Tags, new KeyValuePair<string, string>("error.type", "errorCode"));
+            Assert.AreEqual(ActivityStatusCode.Error, activity.Status);
+        }
+
         private class CustomSampler : Sampler
         {
             public override SamplingResult ShouldSample(in SamplingParameters samplingParameters)

sdk/core/Azure.Core/tests/RequestActivityPolicyTests.cs

@@ -5,9 +5,13 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
+using System.Net.Http;
+using System.Net.Sockets;
+using System.Threading;
 using System.Threading.Tasks;
 using Azure.Core.Pipeline;
 using Azure.Core.TestFramework;
+using Microsoft.AspNetCore.Http;
 using NUnit.Framework;
 
 namespace Azure.Core.Tests
@@ -322,7 +326,7 @@ public async Task ActivitySourceActivityStartedOnRequest(int? port)
                 Task<Response> requestTask = SendRequestAsync(mockTransport, request =>
                 {
                     request.Method = RequestMethod.Get;
-                    request.Uri.Reset(new Uri("http://example.com/path"));
+                    request.Uri.Reset(new Uri("https://example.com/path"));
                     if (port != null)
                     {
                         request.Uri.Port = port.Value;
@@ -336,26 +340,22 @@ public async Task ActivitySourceActivityStartedOnRequest(int? port)
 
                 await requestTask;
 
-                Assert.AreEqual(activity, testListener.Activities.Single());
+                Assert.AreSame(activity, testListener.Activities.Single());
+                Assert.AreEqual("GET", activity.DisplayName);
 
-                CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, int>("http.status_code", 201));
-                CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, string>("http.url", url));
-                CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, string>("http.method", "GET"));
-                CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, string>("http.user_agent", "agent"));
+                Assert.AreEqual(ActivityKind.Client, activity.Kind);
+                CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, int>("http.response.status_code", 201));
+                CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, string>("url.full", url));
+                CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, string>("http.request.method", "GET"));
+                Assert.IsEmpty(activity.Tags.Where(kvp => kvp.Key == "http.user_agent"));
+                Assert.IsEmpty(activity.Tags.Where(kvp => kvp.Key == "requestId"));
+                Assert.IsEmpty(activity.Tags.Where(kvp => kvp.Key == "serviceRequestId"));
 
-                CollectionAssert.DoesNotContain(activity.TagObjects, new KeyValuePair<string, string>("requestId", clientRequestId));
                 CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, string>("az.client_request_id", clientRequestId));
-
-                CollectionAssert.DoesNotContain(activity.TagObjects, new KeyValuePair<string, string>("serviceRequestId", "server request id"));
                 CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, string>("az.service_request_id", "server request id"));
 
-                CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, string>("net.peer.name", "example.com"));
-
-                if (port is null or 443)
-                    CollectionAssert.DoesNotContain(activity.TagObjects, new KeyValuePair<string, int>("net.peer.port", 443));
-                else
-                    CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, int>("net.peer.port", port.Value));
-
+                CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, string>("server.address", "example.com"));
+                CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, object>("server.port", port ?? 443));
                 CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, string>("az.namespace", "Microsoft.Azure.Core.Cool.Tests"));
             }
             finally
@@ -392,7 +392,7 @@ public async Task HttpActivityNeverSuppressed()
                 await requestTask;
 
                 Assert.AreEqual(1, testListener.Activities.Count);
-                CollectionAssert.Contains(testListener.Activities.Single().TagObjects, new KeyValuePair<string, int>("http.status_code", 201));
+                CollectionAssert.Contains(testListener.Activities.Single().TagObjects, new KeyValuePair<string, int>("http.response.status_code", 201));
             }
             finally
             {
@@ -405,23 +405,20 @@ public async Task HttpActivityNeverSuppressed()
         public void ActivityShouldBeStoppedWhenTransportThrowsActivitySource()
         {
             using var _ = SetAppConfigSwitch();
-            ActivityIdFormat previousFormat = Activity.DefaultIdFormat;
-            Activity.DefaultIdFormat = ActivityIdFormat.W3C;
-
-            Exception exception = new Exception("Test exception");
+            HttpRequestException exception = new HttpRequestException("Test exception");
             using var clientListener = new TestActivitySourceListener("Azure.Core.Http");
 
             MockTransport mockTransport = CreateMockTransport(_ => throw exception);
 
-            Assert.ThrowsAsync<Exception>(async () => await SendRequestAsync(mockTransport, request =>
+            Assert.ThrowsAsync<HttpRequestException>(async () => await SendRequestAsync(mockTransport, request =>
             {
                 request.Method = RequestMethod.Get;
                 request.Uri.Reset(new Uri("http://example.com"));
-                request.Headers.Add("User-Agent", "agent");
             }, s_enabledPolicy));
 
             var activity = clientListener.AssertAndRemoveActivity("Azure.Core.Http.Request");
 
+            CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, string>("error.type", "System.Net.Http.HttpRequestException"));
             Assert.AreEqual(ActivityStatusCode.Error, activity.Status);
             StringAssert.Contains("Test exception", activity.StatusDescription);
         }
@@ -431,8 +428,6 @@ public void ActivityShouldBeStoppedWhenTransportThrowsActivitySource()
         public async Task ActivityMarkedAsErrorForErrorResponseActivitySource()
         {
             using var _ = SetAppConfigSwitch();
-            ActivityIdFormat previousFormat = Activity.DefaultIdFormat;
-            Activity.DefaultIdFormat = ActivityIdFormat.W3C;
             using var clientListener = new TestActivitySourceListener("Azure.Core.Http");
 
             MockTransport mockTransport = CreateMockTransport(_ =>
@@ -445,9 +440,40 @@ public async Task ActivityMarkedAsErrorForErrorResponseActivitySource()
 
             var activity = clientListener.AssertAndRemoveActivity("Azure.Core.Http.Request");
 
-            CollectionAssert.Contains(activity.Tags, new KeyValuePair<string, string>("otel.status_code", "ERROR"));
+            CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, string>("error.type", "500"));
+            Assert.AreEqual(ActivityStatusCode.Error, activity.Status);
+            Assert.IsNull(activity.StatusDescription);
+            Assert.IsEmpty(activity.TagObjects.Where(t => t.Key == "otel.status_code"));
+        }
+
+        [Test]
+        [NonParallelizable]
+        public async Task ActivityHasHttpResendCountOnRetries()
+        {
+            using var _ = SetAppConfigSwitch();
+            Activity.DefaultIdFormat = ActivityIdFormat.W3C;
+            using var clientListener = new TestActivitySourceListener("Azure.Core.Http");
+
+            MockTransport mockTransport = CreateMockTransport(_ =>
+            {
+                MockResponse mockResponse = new MockResponse(500);
+                return mockResponse;
+            });
+
+            await SendRequestAsync(mockTransport, message =>
+            {
+                message.Request.Method = RequestMethod.Get;
+                message.RetryNumber = 42;
+                message.Request.Uri.Reset(new Uri("http://example.com"));
+            }, s_enabledPolicy);
+
+            var activity = clientListener.AssertAndRemoveActivity("Azure.Core.Http.Request");
+
+            CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, object>("http.request.resend_count", 42));
+            CollectionAssert.Contains(activity.TagObjects, new KeyValuePair<string, string>("error.type", "500"));
             Assert.AreEqual(ActivityStatusCode.Error, activity.Status);
             Assert.IsNull(activity.StatusDescription);
+            Assert.IsEmpty(activity.TagObjects.Where(t => t.Key == "otel.status_code"));
         }
 #endif
     }