UserAgentValue arch board feedback (#27364)

* API for setting a dynamic UserAgent string
Azure · Mar 8, 2022 · 0915f13 · 0915f13
1 parent 26fd2e3
commit 0915f13
Show file tree

Hide file tree

Showing 13 changed files with 147 additions and 91 deletions.
diff --git a/sdk/core/Azure.Core/api/Azure.Core.net461.cs b/sdk/core/Azure.Core/api/Azure.Core.net461.cs
@@ -652,6 +652,14 @@ public StatusCodeClassifier(System.ReadOnlySpan<ushort> successStatusCodes) { }
         public override bool IsErrorResponse(Azure.Core.HttpMessage message) { throw null; }
     }
     public delegate System.Threading.Tasks.Task SyncAsyncEventHandler<T>(T e) where T : Azure.SyncAsyncEventArgs;
+    public partial class TelemetryDetails
+    {
+        public TelemetryDetails(System.Reflection.Assembly assembly, string? applicationId = null) { }
+        public string? ApplicationId { get { throw null; } }
+        public System.Reflection.Assembly Assembly { get { throw null; } }
+        public void Apply(Azure.Core.HttpMessage message) { }
+        public override string ToString() { throw null; }
+    }
     public abstract partial class TokenCredential
     {
         protected TokenCredential() { }
@@ -902,10 +910,6 @@ public void Dispose() { }
         public override void Process(Azure.Core.HttpMessage message) { }
         public override System.Threading.Tasks.ValueTask ProcessAsync(Azure.Core.HttpMessage message) { throw null; }
     }
-    public static partial class HttpMessageExtensions
-    {
-        public static void SetUserAgentString(this Azure.Core.HttpMessage message, Azure.Core.Pipeline.UserAgentValue userAgentValue) { }
-    }
     public partial class HttpPipeline
     {
         public HttpPipeline(Azure.Core.Pipeline.HttpPipelineTransport transport, Azure.Core.Pipeline.HttpPipelinePolicy[]? policies = null, Azure.Core.ResponseClassifier? responseClassifier = null) { }
@@ -962,12 +966,6 @@ public ServerCertificateCustomValidationArgs(System.Security.Cryptography.X509Ce
         public System.Security.Cryptography.X509Certificates.X509Chain? CertificateAuthorityChain { get { throw null; } }
         public System.Net.Security.SslPolicyErrors SslPolicyErrors { get { throw null; } }
     }
-    public partial class UserAgentValue
-    {
-        public UserAgentValue(System.Type type, string? applicationId = null) { }
-        public static Azure.Core.Pipeline.UserAgentValue FromType<T>(string? applicationId = null) { throw null; }
-        public override string ToString() { throw null; }
-    }
 }
 namespace Azure.Core.Serialization
 {

diff --git a/sdk/core/Azure.Core/api/Azure.Core.net5.0.cs b/sdk/core/Azure.Core/api/Azure.Core.net5.0.cs
@@ -652,6 +652,14 @@ public StatusCodeClassifier(System.ReadOnlySpan<ushort> successStatusCodes) { }
         public override bool IsErrorResponse(Azure.Core.HttpMessage message) { throw null; }
     }
     public delegate System.Threading.Tasks.Task SyncAsyncEventHandler<T>(T e) where T : Azure.SyncAsyncEventArgs;
+    public partial class TelemetryDetails
+    {
+        public TelemetryDetails(System.Reflection.Assembly assembly, string? applicationId = null) { }
+        public string? ApplicationId { get { throw null; } }
+        public System.Reflection.Assembly Assembly { get { throw null; } }
+        public void Apply(Azure.Core.HttpMessage message) { }
+        public override string ToString() { throw null; }
+    }
     public abstract partial class TokenCredential
     {
         protected TokenCredential() { }
@@ -902,10 +910,6 @@ public void Dispose() { }
         public override void Process(Azure.Core.HttpMessage message) { }
         public override System.Threading.Tasks.ValueTask ProcessAsync(Azure.Core.HttpMessage message) { throw null; }
     }
-    public static partial class HttpMessageExtensions
-    {
-        public static void SetUserAgentString(this Azure.Core.HttpMessage message, Azure.Core.Pipeline.UserAgentValue userAgentValue) { }
-    }
     public partial class HttpPipeline
     {
         public HttpPipeline(Azure.Core.Pipeline.HttpPipelineTransport transport, Azure.Core.Pipeline.HttpPipelinePolicy[]? policies = null, Azure.Core.ResponseClassifier? responseClassifier = null) { }
@@ -962,12 +966,6 @@ public ServerCertificateCustomValidationArgs(System.Security.Cryptography.X509Ce
         public System.Security.Cryptography.X509Certificates.X509Chain? CertificateAuthorityChain { get { throw null; } }
         public System.Net.Security.SslPolicyErrors SslPolicyErrors { get { throw null; } }
     }
-    public partial class UserAgentValue
-    {
-        public UserAgentValue(System.Type type, string? applicationId = null) { }
-        public static Azure.Core.Pipeline.UserAgentValue FromType<T>(string? applicationId = null) { throw null; }
-        public override string ToString() { throw null; }
-    }
 }
 namespace Azure.Core.Serialization
 {

diff --git a/sdk/core/Azure.Core/api/Azure.Core.netcoreapp2.1.cs b/sdk/core/Azure.Core/api/Azure.Core.netcoreapp2.1.cs
@@ -652,6 +652,14 @@ public StatusCodeClassifier(System.ReadOnlySpan<ushort> successStatusCodes) { }
         public override bool IsErrorResponse(Azure.Core.HttpMessage message) { throw null; }
     }
     public delegate System.Threading.Tasks.Task SyncAsyncEventHandler<T>(T e) where T : Azure.SyncAsyncEventArgs;
+    public partial class TelemetryDetails
+    {
+        public TelemetryDetails(System.Reflection.Assembly assembly, string? applicationId = null) { }
+        public string? ApplicationId { get { throw null; } }
+        public System.Reflection.Assembly Assembly { get { throw null; } }
+        public void Apply(Azure.Core.HttpMessage message) { }
+        public override string ToString() { throw null; }
+    }
     public abstract partial class TokenCredential
     {
         protected TokenCredential() { }
@@ -902,10 +910,6 @@ public void Dispose() { }
         public override void Process(Azure.Core.HttpMessage message) { }
         public override System.Threading.Tasks.ValueTask ProcessAsync(Azure.Core.HttpMessage message) { throw null; }
     }
-    public static partial class HttpMessageExtensions
-    {
-        public static void SetUserAgentString(this Azure.Core.HttpMessage message, Azure.Core.Pipeline.UserAgentValue userAgentValue) { }
-    }
     public partial class HttpPipeline
     {
         public HttpPipeline(Azure.Core.Pipeline.HttpPipelineTransport transport, Azure.Core.Pipeline.HttpPipelinePolicy[]? policies = null, Azure.Core.ResponseClassifier? responseClassifier = null) { }
@@ -962,12 +966,6 @@ public ServerCertificateCustomValidationArgs(System.Security.Cryptography.X509Ce
         public System.Security.Cryptography.X509Certificates.X509Chain? CertificateAuthorityChain { get { throw null; } }
         public System.Net.Security.SslPolicyErrors SslPolicyErrors { get { throw null; } }
     }
-    public partial class UserAgentValue
-    {
-        public UserAgentValue(System.Type type, string? applicationId = null) { }
-        public static Azure.Core.Pipeline.UserAgentValue FromType<T>(string? applicationId = null) { throw null; }
-        public override string ToString() { throw null; }
-    }
 }
 namespace Azure.Core.Serialization
 {

diff --git a/sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs b/sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs
@@ -652,6 +652,14 @@ public StatusCodeClassifier(System.ReadOnlySpan<ushort> successStatusCodes) { }
         public override bool IsErrorResponse(Azure.Core.HttpMessage message) { throw null; }
     }
     public delegate System.Threading.Tasks.Task SyncAsyncEventHandler<T>(T e) where T : Azure.SyncAsyncEventArgs;
+    public partial class TelemetryDetails
+    {
+        public TelemetryDetails(System.Reflection.Assembly assembly, string? applicationId = null) { }
+        public string? ApplicationId { get { throw null; } }
+        public System.Reflection.Assembly Assembly { get { throw null; } }
+        public void Apply(Azure.Core.HttpMessage message) { }
+        public override string ToString() { throw null; }
+    }
     public abstract partial class TokenCredential
     {
         protected TokenCredential() { }
@@ -902,10 +910,6 @@ public void Dispose() { }
         public override void Process(Azure.Core.HttpMessage message) { }
         public override System.Threading.Tasks.ValueTask ProcessAsync(Azure.Core.HttpMessage message) { throw null; }
     }
-    public static partial class HttpMessageExtensions
-    {
-        public static void SetUserAgentString(this Azure.Core.HttpMessage message, Azure.Core.Pipeline.UserAgentValue userAgentValue) { }
-    }
     public partial class HttpPipeline
     {
         public HttpPipeline(Azure.Core.Pipeline.HttpPipelineTransport transport, Azure.Core.Pipeline.HttpPipelinePolicy[]? policies = null, Azure.Core.ResponseClassifier? responseClassifier = null) { }
@@ -962,12 +966,6 @@ public ServerCertificateCustomValidationArgs(System.Security.Cryptography.X509Ce
         public System.Security.Cryptography.X509Certificates.X509Chain? CertificateAuthorityChain { get { throw null; } }
         public System.Net.Security.SslPolicyErrors SslPolicyErrors { get { throw null; } }
     }
-    public partial class UserAgentValue
-    {
-        public UserAgentValue(System.Type type, string? applicationId = null) { }
-        public static Azure.Core.Pipeline.UserAgentValue FromType<T>(string? applicationId = null) { throw null; }
-        public override string ToString() { throw null; }
-    }
 }
 namespace Azure.Core.Serialization
 {

diff --git a/sdk/core/Azure.Core/src/HttpMessage.cs b/sdk/core/Azure.Core/src/HttpMessage.cs
@@ -17,7 +17,7 @@ public sealed class HttpMessage : IDisposable
         /// <summary>
         /// This dictionary is keyed with <c>Type</c> for a couple of reasons. Primarily, it allows values to be stored such that even if the accessor methods
         /// become public, storing values keyed by internal types make them inaccessible to other assemblies. This protects internal values from being overwritten
-        /// by external code. See the <see cref="UserAgentValue"/> and <see cref="UserAgentValueKey"/> types for an example of this usage.
+        /// by external code. See the <see cref="TelemetryDetails"/> and <see cref="UserAgentValueKey"/> types for an example of this usage.
         /// </summary>
         private Dictionary<Type, object>? _typeProperties;
 

diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpMessageExtensions.cs b/sdk/core/Azure.Core/src/Pipeline/HttpMessageExtensions.cs
diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs
@@ -187,7 +187,8 @@ void AddNonNullPolicies(HttpPipelinePolicy[] policiesToAdd)
         // internal for testing
         internal static TelemetryPolicy CreateTelemetryPolicy(ClientOptions options)
         {
-            var userAgentValue = new UserAgentValue(options.GetType(), options.Diagnostics.ApplicationId);
+            var type = options.GetType();
+            var userAgentValue = new TelemetryDetails(type.Assembly, options.Diagnostics.ApplicationId);
             return new TelemetryPolicy(userAgentValue);
         }
     }

diff --git a/sdk/core/Azure.Core/src/Pipeline/Internal/TelemetryPolicy.cs b/sdk/core/Azure.Core/src/Pipeline/Internal/TelemetryPolicy.cs
@@ -7,9 +7,9 @@ internal class TelemetryPolicy : HttpPipelineSynchronousPolicy
     {
         private readonly string _defaultHeader;
 
-        public TelemetryPolicy(UserAgentValue userAgentValue)
+        public TelemetryPolicy(TelemetryDetails telemetryDetails)
         {
-            _defaultHeader = userAgentValue.ToString();
+            _defaultHeader = telemetryDetails.ToString();
         }
 
         public override void OnSendingRequest(HttpMessage message)

diff --git a/sdk/core/Azure.Core/src/Pipeline/Internal/UserAgentValueKey.cs b/sdk/core/Azure.Core/src/Pipeline/Internal/UserAgentValueKey.cs
@@ -4,7 +4,7 @@
 namespace Azure.Core.Pipeline
 {
     /// <summary>
-    /// Class that serves as the key for <see cref="UserAgentValue"/> UserAgent strings on <see cref="HttpMessage"/>.
+    /// Class that serves as the key for <see cref="TelemetryDetails"/> UserAgent strings on <see cref="HttpMessage"/>.
     /// </summary>
     internal class UserAgentValueKey { }
 }
diff --git a/...Azure.Core/src/Pipeline/UserAgentValue.cs → sdk/core/Azure.Core/src/TelemetryDetails.cs b/...Azure.Core/src/Pipeline/UserAgentValue.cs → sdk/core/Azure.Core/src/TelemetryDetails.cs
@@ -2,47 +2,60 @@
 // Licensed under the MIT License.
 
 using System;
+using System.ComponentModel;
 using System.Reflection;
 using System.Runtime.InteropServices;
+using Azure.Core.Pipeline;
 
-namespace Azure.Core.Pipeline
+namespace Azure.Core
 {
     /// <summary>
-    /// Information about the package to be included in UserAgent telemetry
+    /// Details about the package to be included in UserAgent telemetry
     /// </summary>
-    public class UserAgentValue
+    public class TelemetryDetails
     {
-        private string _userAgent;
+        private const int MaxApplicationIdLength = 24;
+        private readonly string _userAgent;
 
         /// <summary>
-        /// Initialize an instance of <see cref="UserAgentValue"/> by extracting the name and version information from the <see cref="Assembly"/> associated with the <paramref name="type"/>.
+        /// The package type represented by this <see cref="TelemetryDetails"/> instance.
         /// </summary>
-        /// <param name="type">The <see cref="Type"/> used to generate the package name and version information for the <see cref="UserAgentValue"/> value.</param>
-        /// <param name="applicationId">An optional value to be prepended to the <see cref="UserAgentValue"/>.
+        public Assembly Assembly { get; }
+
+        /// <summary>
+        /// The value of the applicationId used to initialize this <see cref="TelemetryDetails"/> instance.
+        /// </summary>
+        public string? ApplicationId { get; }
+
+        /// <summary>
+        /// Initialize an instance of <see cref="TelemetryDetails"/> by extracting the name and version information from the <see cref="System.Reflection.Assembly"/> associated with the <paramref name="assembly"/>.
+        /// </summary>
+        /// <param name="assembly">The <see cref="System.Reflection.Assembly"/> used to generate the package name and version information for the <see cref="TelemetryDetails"/> value.</param>
+        /// <param name="applicationId">An optional value to be prepended to the <see cref="TelemetryDetails"/>.
         /// This value overrides the behavior of the <see cref="DiagnosticsOptions.ApplicationId"/> property for the <see cref="HttpMessage"/> it is applied to.</param>
-        public UserAgentValue(Type type, string? applicationId = null)
+        public TelemetryDetails(Assembly assembly, string? applicationId = null)
         {
-            var assembly = Assembly.GetAssembly(type);
-            if (assembly == null) throw new ArgumentException($"The type parameter {type.FullName} does not have a valid Assembly");
+            Argument.AssertNotNull(assembly, nameof(assembly));
+            if ( applicationId?.Length > MaxApplicationIdLength)
+            {
+                throw new ArgumentOutOfRangeException(nameof(applicationId), $"{nameof(applicationId)} must be shorter than {MaxApplicationIdLength + 1} characters");
+            }
+
+            Assembly = assembly;
+            ApplicationId = applicationId;
             _userAgent = GenerateUserAgentString(assembly, applicationId);
         }
 
         /// <summary>
-        /// Creates an instance of a <see cref="UserAgentValue"/> based on the Type provided.
+        /// Sets the package name and version portion of the UserAgent telemetry value for the context of the <paramref name="message"/>
+        /// Note: If <see cref="DiagnosticsOptions.IsTelemetryEnabled"/> is false, this value is never used.
         /// </summary>
-        /// <param name="applicationId"></param>
-        /// <typeparam name="T">The type contained by the Assembly used to generate package name and version information.</typeparam>
-        /// <returns></returns>
-        public static UserAgentValue FromType<T>(string? applicationId = null)
+        /// <param name="message">The <see cref="HttpMessage"/> that will use this <see cref="TelemetryDetails"/>.</param>
+        public void Apply(HttpMessage message)
         {
-            return new UserAgentValue(typeof(T), applicationId);
+            message.SetInternalProperty(typeof(UserAgentValueKey), ToString());
         }
 
-        /// <summary>
-        /// Returns a formatted UserAgent string
-        /// </summary>
-        public override string ToString() => _userAgent;
-
         internal static string GenerateUserAgentString(Assembly clientAssembly, string? applicationId = null)
         {
             const string PackagePrefix = "Azure.";
@@ -73,5 +86,10 @@ internal static string GenerateUserAgentString(Assembly clientAssembly, string?
                 ? $"{applicationId} azsdk-net-{assemblyName}/{version} {platformInformation}"
                 : $"azsdk-net-{assemblyName}/{version} {platformInformation}";
         }
+
+        /// <summary>
+        /// The properly formatted UserAgent string based on this <see cref="TelemetryDetails"/> instance.
+        /// </summary>
+        public override string ToString() => _userAgent;
     }
 }
diff --git a/sdk/core/Azure.Core/tests/HttpPipelineBuilderTest.cs b/sdk/core/Azure.Core/tests/HttpPipelineBuilderTest.cs
@@ -137,7 +137,8 @@ public async Task UsesAssemblyNameAndInformationalVersionForTelemetryPolicySetti
             HttpPipeline pipeline = HttpPipelineBuilder.Build(options);
 
             var message = pipeline.CreateMessage();
-            message.SetUserAgentString(UserAgentValue.FromType<string>());
+            var userAgent = new TelemetryDetails(typeof(string).Assembly);
+            userAgent.Apply(message);
             using Request request = message.Request;
             request.Method = RequestMethod.Get;
             request.Uri.Reset(new Uri("http://example.com"));

diff --git a/sdk/core/Azure.Core/tests/TelemetryDetailsTests.cs b/sdk/core/Azure.Core/tests/TelemetryDetailsTests.cs
@@ -0,0 +1,66 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System.Reflection;
+using System.Runtime.InteropServices;
+using Azure.Core.Pipeline;
+using Azure.Core.TestFramework;
+using NUnit.Framework;
+
+namespace Azure.Core.Tests
+{
+    public class TelemetryDetailsTests
+    {
+        private const string appId = "MyApplicationId";
+
+        [Test]
+        public void CtorInitializesValue([Values(null, appId)] string applicationId)
+        {
+            var target = new TelemetryDetails(typeof(TelemetryDetailsTests).Assembly, applicationId);
+
+            var assembly = Assembly.GetAssembly(GetType());
+            AssemblyInformationalVersionAttribute versionAttribute = assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>();
+            string version = versionAttribute.InformationalVersion;
+            int hashSeparator = version.IndexOfOrdinal('+');
+            if (hashSeparator != -1)
+            {
+                version = version.Substring(0, hashSeparator);
+            }
+            if (applicationId == null)
+            {
+                Assert.AreEqual(
+                    target.ToString(),
+                    $"azsdk-net-Core.Tests/{version} ({RuntimeInformation.FrameworkDescription}; {RuntimeInformation.OSDescription})");
+            }
+            else
+            {
+                Assert.AreEqual(
+                    target.ToString(),
+                    $"{applicationId} azsdk-net-Core.Tests/{version} ({RuntimeInformation.FrameworkDescription}; {RuntimeInformation.OSDescription})");
+            }
+        }
+
+        [Test]
+        public void CtorPopulatesProperties([Values(null, appId)] string applicationId)
+        {
+            var target = new TelemetryDetails(typeof(TelemetryDetailsTests).Assembly, applicationId);
+
+            Assert.AreEqual(typeof(TelemetryDetailsTests).Assembly, target.Assembly);
+            Assert.AreEqual(applicationId, target.ApplicationId);
+        }
+
+        [Test]
+        public void AppliesToMessage()
+        {
+            var target = new TelemetryDetails(typeof(TelemetryDetailsTests).Assembly);
+            var message = new HttpMessage(new MockRequest(), ResponseClassifier.Shared);
+
+            target.Apply(message);
+
+            message.TryGetInternalProperty(typeof(UserAgentValueKey), out var obj);
+            string actualValue = obj as string;
+
+            Assert.AreEqual(target.ToString(), actualValue);
+        }
+    }
+}