Include resource namespace in diagnostics scope (#9655)

Azure · Jan 28, 2020 · d63cf3c · d63cf3c
1 parent bce1ab2
commit d63cf3c
Show file tree

Hide file tree

Showing 34 changed files with 250 additions and 74 deletions.
diff --git a/sdk/appconfiguration/Azure.Data.AppConfiguration/src/Azure.Data.AppConfiguration.csproj b/sdk/appconfiguration/Azure.Data.AppConfiguration/src/Azure.Data.AppConfiguration.csproj
@@ -20,6 +20,7 @@
 
   <ItemGroup>
     <Compile Include="$(AzureCoreSharedSources)Argument.cs" />
+    <Compile Include="$(AzureCoreSharedSources)AzureResourceProviderNamespaceAttribute.cs" />
     <Compile Include="$(AzureCoreSharedSources)ConnectionString.cs" />
     <Compile Include="$(AzureCoreSharedSources)ArrayBufferWriter.cs" />
     <Compile Include="$(AzureCoreSharedSources)ClientDiagnostics.cs" />

diff --git a/sdk/appconfiguration/Azure.Data.AppConfiguration/src/Properties/AssemblyInfo.cs b/sdk/appconfiguration/Azure.Data.AppConfiguration/src/Properties/AssemblyInfo.cs
@@ -7,3 +7,4 @@
 
 // See https://github.com/castleproject/Core/blob/master/src/Castle.Core/Core/Internal/InternalsVisible.cs for values
 [assembly: InternalsVisibleTo("DynamicProxyGenAssembly2, PublicKey=0024000004800000940000000602000000240000525341310004000001000100c547cac37abd99c8db225ef2f6c8a3602f3b3606cc9891605d02baa56104f4cfc0734aa39b93bf7852f7d9266654753cc297e7d2edfe0bac1cdcf9f717241550e0a7b191195b7667bb4f64bcb8e2121380fd1d9d46ad2d92d2d15605093924cceaf74c4861eff62abf69b9291ed0a340e113be11e6a7d3113e92484cf7045cc7")]
+[assembly: Azure.Core.AzureResourceProviderNamespace("Microsoft.AppConfiguration")]
diff --git a/sdk/core/Azure.Core/src/Azure.Core.csproj b/sdk/core/Azure.Core/src/Azure.Core.csproj
@@ -21,13 +21,13 @@
   </ItemGroup>
 
   <ItemGroup>
-    <Compile Remove="Shared\*.cs"/>
-    <Compile Include="Shared\Argument.cs"/>
-    <Compile Include="Shared\EventSourceEventFormatting.cs"/>
-    <Compile Include="Shared\HashCodeBuilder.cs"/>
-    <Compile Include="Shared\NullableAttributes.cs"/>
-    <Compile Include="Shared\ContentTypeUtilities.cs"/>
-    <Compile Include="Shared\ClientDiagnostics.cs"/>
-    <Compile Include="Shared\DiagnosticScope.cs"/>
+    <Compile Remove="Shared\*.cs" />
+    <Compile Include="Shared\Argument.cs" />
+    <Compile Include="Shared\EventSourceEventFormatting.cs" />
+    <Compile Include="Shared\HashCodeBuilder.cs" />
+    <Compile Include="Shared\NullableAttributes.cs" />
+    <Compile Include="Shared\ContentTypeUtilities.cs" />
+    <Compile Include="Shared\ClientDiagnostics.cs" />
+    <Compile Include="Shared\DiagnosticScope.cs" />
   </ItemGroup>
 </Project>
diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs
@@ -75,7 +75,7 @@ public static HttpPipeline Build(ClientOptions options, HttpPipelinePolicy[] per
 
             policies.Add(BufferResponsePolicy.Shared);
 
-            policies.Add(new RequestActivityPolicy(isDistributedTracingEnabled));
+            policies.Add(new RequestActivityPolicy(isDistributedTracingEnabled, ClientDiagnostics.GetResourceProviderNamespace(options.GetType().Assembly)));
 
             policies.RemoveAll(policy => policy == null);
 

diff --git a/sdk/core/Azure.Core/src/Pipeline/Internal/RequestActivityPolicy.cs b/sdk/core/Azure.Core/src/Pipeline/Internal/RequestActivityPolicy.cs
@@ -11,16 +11,18 @@ namespace Azure.Core.Pipeline
     internal class RequestActivityPolicy : HttpPipelinePolicy
     {
         private readonly bool _isDistributedTracingEnabled;
+        private readonly string? _resourceProviderNamespace;
 
         private const string TraceParentHeaderName = "traceparent";
         private const string TraceStateHeaderName = "tracestate";
         private const string RequestIdHeaderName = "Request-Id";
 
         private static readonly DiagnosticListener s_diagnosticSource = new DiagnosticListener("Azure.Core");
 
-        public RequestActivityPolicy(bool isDistributedTracingEnabled)
+        public RequestActivityPolicy(bool isDistributedTracingEnabled, string? resourceProviderNamespace)
         {
             _isDistributedTracingEnabled = isDistributedTracingEnabled;
+            _resourceProviderNamespace = resourceProviderNamespace;
         }
 
         public override ValueTask ProcessAsync(HttpMessage message, ReadOnlyMemory<HttpPipelinePolicy> pipeline)
@@ -62,6 +64,11 @@ private async ValueTask ProcessAsync(HttpMessage message, ReadOnlyMemory<HttpPip
             activity.AddTag("requestId", message.Request.ClientRequestId);
             activity.AddTag("kind", "client");
 
+            if (_resourceProviderNamespace != null)
+            {
+                activity.AddTag("az.namespace", _resourceProviderNamespace);
+            }
+
             if (message.Request.Headers.TryGetValue("User-Agent", out string? userAgent))
             {
                 activity.AddTag("http.user_agent", userAgent);
@@ -78,26 +85,30 @@ private async ValueTask ProcessAsync(HttpMessage message, ReadOnlyMemory<HttpPip
                 activity.Start();
             }
 
-
-            if (isAsync)
+            try
             {
-                await ProcessNextAsync(message, pipeline, true).ConfigureAwait(false);
-            }
-            else
-            {
-                ProcessNextAsync(message, pipeline, false).EnsureCompleted();
-            }
-
-            activity.AddTag("http.status_code", message.Response.Status.ToString(CultureInfo.InvariantCulture));
-            activity.AddTag("serviceRequestId", message.Response.Headers.RequestId);
+                if (isAsync)
+                {
+                    await ProcessNextAsync(message, pipeline, true).ConfigureAwait(false);
+                }
+                else
+                {
+                    ProcessNextAsync(message, pipeline, false).EnsureCompleted();
+                }
 
-            if (diagnosticSourceActivityEnabled)
-            {
-                s_diagnosticSource.StopActivity(activity, message);
+                activity.AddTag("http.status_code", message.Response.Status.ToString(CultureInfo.InvariantCulture));
+                activity.AddTag("serviceRequestId", message.Response.Headers.RequestId);
             }
-            else
+            finally
             {
-                activity.Stop();
+                if (diagnosticSourceActivityEnabled)
+                {
+                    s_diagnosticSource.StopActivity(activity, message);
+                }
+                else
+                {
+                    activity.Stop();
+                }
             }
         }
 

diff --git a/sdk/core/Azure.Core/src/Shared/AzureResourceProviderNamespaceAttribute.cs b/sdk/core/Azure.Core/src/Shared/AzureResourceProviderNamespaceAttribute.cs
@@ -0,0 +1,22 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System;
+
+namespace Azure.Core
+{
+    /// <summary>
+    /// This attribute should be set on all client assemblies with value of one of the resource providers
+    /// from the https://docs.microsoft.com/en-us/azure/azure-resource-manager/management/azure-services-resource-providers list.
+    /// </summary>
+    [AttributeUsage(AttributeTargets.Assembly, AllowMultiple = false)]
+    internal class AzureResourceProviderNamespaceAttribute : Attribute
+    {
+        public string ResourceProviderNamespace { get; }
+
+        public AzureResourceProviderNamespaceAttribute(string resourceProviderNamespace)
+        {
+            ResourceProviderNamespace = resourceProviderNamespace;
+        }
+    }
+}
diff --git a/sdk/core/Azure.Core/src/Shared/ClientDiagnostics.cs b/sdk/core/Azure.Core/src/Shared/ClientDiagnostics.cs
@@ -1,7 +1,9 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+using System;
 using System.Diagnostics;
+using System.Reflection;
 
 #nullable enable
 
@@ -11,20 +13,25 @@ namespace Azure.Core.Pipeline
     internal sealed class ClientDiagnostics
 #pragma warning restore CA1001 // Types that own disposable fields should be disposable
     {
+        private readonly string? _resourceProviderNamespace;
         private readonly DiagnosticListener? _source;
 
         public bool IsActivityEnabled { get;  }
 
-        public ClientDiagnostics(string clientNamespace, bool isActivityEnabled)
+        public ClientDiagnostics(string clientNamespace, string? resourceProviderNamespace, bool isActivityEnabled)
         {
+            _resourceProviderNamespace = resourceProviderNamespace;
             IsActivityEnabled = isActivityEnabled;
             if (IsActivityEnabled)
             {
                 _source = new DiagnosticListener(clientNamespace);
             }
         }
 
-        public ClientDiagnostics(ClientOptions options) : this(options.GetType().Namespace, options.Diagnostics.IsDistributedTracingEnabled)
+        public ClientDiagnostics(ClientOptions options) : this(
+            options.GetType().Namespace,
+            GetResourceProviderNamespace(options.GetType().Assembly),
+            options.Diagnostics.IsDistributedTracingEnabled)
         {
         }
 
@@ -34,7 +41,28 @@ public DiagnosticScope CreateScope(string name)
             {
                 return default;
             }
-            return new DiagnosticScope(name, _source);
+            var scope = new DiagnosticScope(name, _source);
+
+            if (_resourceProviderNamespace != null)
+            {
+                scope.AddAttribute("az.namespace", _resourceProviderNamespace);
+            }
+            return scope;
+        }
+
+        internal static string? GetResourceProviderNamespace(Assembly assembly)
+        {
+            foreach (var customAttribute in assembly.GetCustomAttributes(true))
+            {
+                // Weak bind internal shared type
+                var attributeType = customAttribute.GetType();
+                if (attributeType.Name == "AzureResourceProviderNamespaceAttribute")
+                {
+                    return attributeType.GetProperty("ResourceProviderNamespace")?.GetValue(customAttribute) as string;
+                }
+            }
+
+            return null;
         }
     }
 }
diff --git a/sdk/core/Azure.Core/tests/Azure.Core.Tests.csproj b/sdk/core/Azure.Core/tests/Azure.Core.Tests.csproj
@@ -29,6 +29,7 @@
     <Compile Include="..\src\Shared\NoBodyResponse{T}.cs" />
     <Compile Include="..\src\Shared\ResponseExceptionExtensions.cs" />
     <Compile Include="..\src\Shared\OperationHelpers.cs" />
+    <Compile Include="..\src\Shared\AzureResourceProviderNamespaceAttribute.cs" />
   </ItemGroup>
 
 </Project>
diff --git a/sdk/core/Azure.Core/tests/ClientDiagnosticsTests.cs b/sdk/core/Azure.Core/tests/ClientDiagnosticsTests.cs
@@ -5,10 +5,12 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
-using Azure.Core.Diagnostics;
+using Azure.Core;
 using Azure.Core.Pipeline;
 using NUnit.Framework;
 
+[assembly:AzureResourceProviderNamespace("Microsoft.Azure.Core.Cool.Tests")]
+
 namespace Azure.Core.Tests
 {
     public class ClientDiagnosticsTests
@@ -18,7 +20,7 @@ public void CreatesActivityWithNameAndTags()
         {
 
             using var testListener = new TestDiagnosticListener("Azure.Clients");
-            ClientDiagnostics clientDiagnostics = new ClientDiagnostics("Azure.Clients", true);
+            ClientDiagnostics clientDiagnostics = new ClientDiagnostics("Azure.Clients", "Microsoft.Azure.Core.Cool.Tests", true);
 
             DiagnosticScope scope = clientDiagnostics.CreateScope("ActivityName");
 
@@ -44,13 +46,40 @@ public void CreatesActivityWithNameAndTags()
             CollectionAssert.Contains(activity.Tags, new KeyValuePair<string, string>("Attribute1", "Value1"));
             CollectionAssert.Contains(activity.Tags, new KeyValuePair<string, string>("Attribute2", "2"));
             CollectionAssert.Contains(activity.Tags, new KeyValuePair<string, string>("Attribute3", "3"));
+            CollectionAssert.Contains(activity.Tags, new KeyValuePair<string, string>("az.namespace", "Microsoft.Azure.Core.Cool.Tests"));
+        }
+
+        [Test]
+        public void ResourceNameIsOptional()
+        {
+
+            using var testListener = new TestDiagnosticListener("Azure.Clients");
+            ClientDiagnostics clientDiagnostics = new ClientDiagnostics("Azure.Clients", null, true);
+
+            DiagnosticScope scope = clientDiagnostics.CreateScope("ActivityName");
+            scope.Start();
+
+            (string Key, object Value, DiagnosticListener) startEvent = testListener.Events.Dequeue();
+
+            Activity activity = Activity.Current;
+
+            scope.Dispose();
+
+            (string Key, object Value, DiagnosticListener) stopEvent = testListener.Events.Dequeue();
+
+            Assert.Null(Activity.Current);
+            Assert.AreEqual("ActivityName.Start", startEvent.Key);
+            Assert.AreEqual("ActivityName.Stop", stopEvent.Key);
+
+            Assert.AreEqual(ActivityIdFormat.W3C, activity.IdFormat);
+            CollectionAssert.IsEmpty(activity.Tags);
         }
 
         [Test]
         public void AddLinkCallsPassesLinksAsPartOfStartPayload()
         {
             using var testListener = new TestDiagnosticListener("Azure.Clients");
-            ClientDiagnostics clientDiagnostics = new ClientDiagnostics("Azure.Clients", true);
+            ClientDiagnostics clientDiagnostics = new ClientDiagnostics("Azure.Clients",  "Microsoft.Azure.Core.Cool.Tests",true);
 
             DiagnosticScope scope = clientDiagnostics.CreateScope("ActivityName");
 
@@ -91,7 +120,7 @@ public void AddLinkCallsPassesLinksAsPartOfStartPayload()
         public void FailedStopsActivityAndWritesExceptionEvent()
         {
             using var testListener = new TestDiagnosticListener("Azure.Clients");
-            ClientDiagnostics clientDiagnostics = new ClientDiagnostics("Azure.Clients", true);
+            ClientDiagnostics clientDiagnostics = new ClientDiagnostics("Azure.Clients",  "Microsoft.Azure.Core.Cool.Tests", true);
 
             DiagnosticScope scope = clientDiagnostics.CreateScope("ActivityName");
 
@@ -120,18 +149,25 @@ public void FailedStopsActivityAndWritesExceptionEvent()
 
             CollectionAssert.Contains(activity.Tags, new KeyValuePair<string, string>("Attribute1", "Value1"));
             CollectionAssert.Contains(activity.Tags, new KeyValuePair<string, string>("Attribute2", "2"));
+            CollectionAssert.Contains(activity.Tags, new KeyValuePair<string, string>("az.namespace", "Microsoft.Azure.Core.Cool.Tests"));
         }
 
         [Test]
         public void NoopsWhenDisabled()
         {
-            ClientDiagnostics clientDiagnostics = new ClientDiagnostics("Azure.Clients", false);
+            ClientDiagnostics clientDiagnostics = new ClientDiagnostics("Azure.Clients",  "Microsoft.Azure.Core.Cool.Tests", false);
             DiagnosticScope scope = clientDiagnostics.CreateScope("");
 
             scope.AddAttribute("Attribute1", "Value1");
             scope.AddAttribute("Attribute2", 2, i => i.ToString());
             scope.Failed(new Exception());
             scope.Dispose();
         }
+
+        [Test]
+        public void GetResourceProviderNamespaceReturnsAttributeValue()
+        {
+            Assert.AreEqual("Microsoft.Azure.Core.Cool.Tests", ClientDiagnostics.GetResourceProviderNamespace(GetType().Assembly));
+        }
     }
 }
diff --git a/sdk/core/Azure.Core/tests/ClientTestBaseDiagnosticScopeTests.cs b/sdk/core/Azure.Core/tests/ClientTestBaseDiagnosticScopeTests.cs
@@ -23,7 +23,7 @@ public void ThrowsWhenNoDiagnosticScope()
         {
             InvalidDiagnosticScopeTestClient client = InstrumentClient(new InvalidDiagnosticScopeTestClient());
             InvalidOperationException ex = Assert.ThrowsAsync<InvalidOperationException>(async () => await client.NoScopeAsync());
-            StringAssert.Contains("Expected some diagnostic event to fire", ex.Message);
+            StringAssert.Contains("Expected some diagnostic scopes to be created, found none", ex.Message);
         }
 
         [Test]
@@ -57,7 +57,7 @@ public class InvalidDiagnosticScopeTestClient
         {
             private void FireScope(string method)
             {
-                ClientDiagnostics clientDiagnostics = new ClientDiagnostics("Azure.Core.Tests", true);
+                ClientDiagnostics clientDiagnostics = new ClientDiagnostics("Azure.Core.Tests", "random", true);
                 string activityName = $"{typeof(InvalidDiagnosticScopeTestClient).Name}.{method}";
                 DiagnosticScope scope = clientDiagnostics.CreateScope(activityName);
                 scope.Start();
Original file line number	Diff line number	Diff line change
Expand Up		@@ -7,3 +7,4 @@

		// See https://github.com/castleproject/Core/blob/master/src/Castle.Core/Core/Internal/InternalsVisible.cs for values
		[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2, PublicKey=0024000004800000940000000602000000240000525341310004000001000100c547cac37abd99c8db225ef2f6c8a3602f3b3606cc9891605d02baa56104f4cfc0734aa39b93bf7852f7d9266654753cc297e7d2edfe0bac1cdcf9f717241550e0a7b191195b7667bb4f64bcb8e2121380fd1d9d46ad2d92d2d15605093924cceaf74c4861eff62abf69b9291ed0a340e113be11e6a7d3113e92484cf7045cc7")]
		[assembly: Azure.Core.AzureResourceProviderNamespace("Microsoft.AppConfiguration")]