XenApi: Removed the default value for Xenserver User Agent

[rashmib-1112]

Machine Creation Service (MCS) is now responsible for setting the user agent value that includes the MCS version for XenServer users via the JSON RPC client.

Previously, the default value "XenAPI/{Helper.APIVersionString(API_Version.LATEST)}" defined here was overriding the MCS version user agent value provided by the client. To avoid this, MCS now sets an explicit user agent value, and the client passes that value through to this component.

From XenServer’s perspective, the calls appear generically as {{XenAPI/2.15 }}, making it difficult to identify the source.
This change contains additional details to tell XenServer the version in a format: MCS/{DaaS/CVAD_Version}

[lindig]

This needs a review by @kc284. I support the idea that any client needs to provide an identity that will transfer into logs and would support a mechanism where no default is provided such that a client must do this.

Is it correct that a client-provided UA currently is overridden? That seems like a bug to me and the code should have checked for the presence of a value before overriding it.

[edwintorok]

What happens if someone uses our SDK (samples) as it is, and doesn't override the user-agent at all?
Will that mean that we send no user-agent? That'll equally make it difficult to identify where requests are coming from.

I think a User-Agent must always be supplied, either by the SDK (as a fallback) or by the application using the SDK.

[rashmib-1112]

I just spoke with one of my seniors and he suggested we add the user agent as a property with a get function named deafultUserAgent. And when and if we get the value from the client, we will be assigning that (the new version), similarly, if we don't (like probably in the case you mentioned), we keep it as the default UserAgent value.
Would that be fine? @edwintorok

[edwintorok]

I just spoke with one of my seniors and he suggested we add the user agent as a property with a get function named deafultUserAgent. And when and if we get the value from the client, we will be assigning that (the new version), similarly, if we don't (like probably in the case you mentioned), we keep it as the default UserAgent value. Would that be fine? @edwintorok

I'm not sure what the best mechanism would be for overriding the user-agent, I'll let the maintainer @kc284 comment on that. IIRC XenCenter already overrides the user-agent, so would probably be good to use a mechanism here that both XenCenter and other users of this SDK can use.

[lindig]

The updated logic seems correct to me: use the default UA when the client has not provided a UA.

[rashmib-1112]

Made a small change by introducing a configurable static UserAgent with a private backing field.
This will lead to a single place to configure the UserAgent, honoring values from externally created JsonRpcClient instances while preserving a sensible default.
CC: @lindig @edwintorok @kc284

[kc284]

I had an existing ticket for this (CP-48452) and the way I was planning to fix it was somewhat different. The static UserAgent was sloppy (so was the static Proxy causing tickets like CP-54003), so I was planning to replace both with Properties. Public fields are also not a good design. Here is the diff of how I think we should fix it. Note that the change is breaking (but it might be ok since the next SDK release bumps the major digit 2025.xx =>2026.yy), so please review it and apply it to this PR (alternatively I can raise a new one).
xen-api-13-04-16.patch

[lindig]

diff --git forkSrcPrefix/ocaml/sdk-gen/csharp/autogen/src/Session.cs forkDstPrefix/ocaml/sdk-gen/csharp/autogen/src/Session.cs
index 5d999136833d96eed23dec2e2f9fae2d326dbbf7..5fafbae3cbcff39771f3ce05f00df7ab913f6ad1 100644
--- forkSrcPrefix/ocaml/sdk-gen/csharp/autogen/src/Session.cs
+++ forkDstPrefix/ocaml/sdk-gen/csharp/autogen/src/Session.cs
@@ -46,27 +46,21 @@ namespace XenAPI
         public const int STANDARD_TIMEOUT = 24 * 60 * 60 * 1000;
 
         /// <summary>
-        /// This string is used as the HTTP UserAgent for each request.
+        /// The default HTTP UserAgent for each request.
         /// </summary>
-        public static string UserAgent = $"XenAPI/{Helper.APIVersionString(API_Version.LATEST)}";
-
-        /// <summary>
-        /// If null, no proxy is used, otherwise this proxy is used for each request.
-        /// </summary>
-        public static IWebProxy Proxy = null;
-
-        public API_Version APIVersion = API_Version.UNKNOWN;
-
-        public object Tag;
+        public static readonly string DefaultUserAgent = $"XenAPI/{Helper.APIVersionString(API_Version.LATEST)}";
 
         #region Constructors
 
+        /// <exception cref="ArgumentNullException">Thrown if 'client' is null</exception>
         public Session(JsonRpcClient client)
         {
+            if (client == null)
+                throw new ArgumentNullException(nameof(client));
+
             client.Timeout = STANDARD_TIMEOUT;
             client.KeepAlive = true;
-            client.UserAgent = UserAgent;
-            client.WebProxy = Proxy;
+            client.UserAgent = DefaultUserAgent;
             client.AllowAutoRedirect = true;
             JsonRpcClient = client;
         }
@@ -230,6 +224,19 @@ namespace XenAPI
 
         #region Properties
 
+        /// <summary>
+        /// The WebProxy to use for each HTTP request.
+        /// </summary>
+        public IWebProxy Proxy
+        {
+            get => JsonRpcClient.WebProxy;
+            set => JsonRpcClient.WebProxy = value;
+        }
+
+        public API_Version APIVersion { get; private set;  } = API_Version.UNKNOWN;
+
+        public object Tag { get; set; }
+
         /// <summary>
         /// Retrieves the current users details from the UserDetails map. These values are only updated when a new session is created.
         /// </summary>
@@ -239,15 +246,25 @@ namespace XenAPI
 
         public string Url => JsonRpcClient.Url;
 
+        /// <summary>
+        /// The UserAgent to use for each HTTP request. If set to null or empty
+        /// the DefaultUserAgent will be used.
+        /// </summary>
+        public string UserAgent
+        {
+            get => JsonRpcClient.UserAgent;
+            set => JsonRpcClient.UserAgent = string.IsNullOrEmpty(value) ? DefaultUserAgent : value;
+        }
+
         public string ConnectionGroupName
         {
-            get => JsonRpcClient?.ConnectionGroupName;
+            get => JsonRpcClient.ConnectionGroupName;
             set => JsonRpcClient.ConnectionGroupName = value;
         }
 
         public int Timeout
         {
-            get => JsonRpcClient?.Timeout ?? STANDARD_TIMEOUT;
+            get => JsonRpcClient.Timeout;
             set => JsonRpcClient.Timeout = value;
         }
 

[kc284]

Actually, scrap that, it has issues too. Here is a reworked patch:
xencenter-14-10-53.patch

diff --git forkSrcPrefix/XenModel/XenAPI/Session.cs forkDstPrefix/XenModel/XenAPI/Session.cs
index d271beb66bb1cf5632e7af6a134cbceb31d4e575..c7fd5a25b5caabfa8551a8ed70acd5fe5514a2f9 100644
--- forkSrcPrefix/XenModel/XenAPI/Session.cs
+++ forkDstPrefix/XenModel/XenAPI/Session.cs
@@ -46,29 +46,16 @@ namespace XenAPI
         public const int STANDARD_TIMEOUT = 24 * 60 * 60 * 1000;
 
         /// <summary>
-        /// This string is used as the HTTP UserAgent for each request.
+        /// The default HTTP UserAgent for each request.
         /// </summary>
-        public static string UserAgent = $"XenAPI/{Helper.APIVersionString(API_Version.LATEST)}";
-
-        /// <summary>
-        /// If null, no proxy is used, otherwise this proxy is used for each request.
-        /// </summary>
-        public static IWebProxy Proxy = null;
-
-        public API_Version APIVersion = API_Version.UNKNOWN;
-
-        public object Tag;
+        public static readonly string DefaultUserAgent = $"XenAPI/{Helper.APIVersionString(API_Version.LATEST)}";
 
         #region Constructors
 
+        /// <exception cref="ArgumentNullException">Thrown if 'client' is null</exception>
         public Session(JsonRpcClient client)
         {
-            client.Timeout = STANDARD_TIMEOUT;
-            client.KeepAlive = true;
-            client.UserAgent = UserAgent;
-            client.WebProxy = Proxy;
-            client.AllowAutoRedirect = true;
-            JsonRpcClient = client;
+            JsonRpcClient = client ?? throw new ArgumentNullException(nameof(client));
         }
 
         public Session(string url) :
@@ -230,6 +217,19 @@ namespace XenAPI
 
         #region Properties
 
+        /// <summary>
+        /// The WebProxy to use for each HTTP request.
+        /// </summary>
+        public IWebProxy Proxy
+        {
+            get => JsonRpcClient.WebProxy;
+            set => JsonRpcClient.WebProxy = value;
+        }
+
+        public API_Version APIVersion { get; private set;  } = API_Version.UNKNOWN;
+
+        public object Tag { get; set; }
+
         /// <summary>
         /// Retrieves the current users details from the UserDetails map. These values are only updated when a new session is created.
         /// </summary>
@@ -239,15 +239,24 @@ namespace XenAPI
 
         public string Url => JsonRpcClient.Url;
 
+        /// <summary>
+        /// The UserAgent to use for each HTTP request. If set to null or empty the DefaultUserAgent will be used.
+        /// </summary>
+        public string UserAgent
+        {
+            get => JsonRpcClient.UserAgent;
+            set => JsonRpcClient.UserAgent = value;
+        }
+
         public string ConnectionGroupName
         {
-            get => JsonRpcClient?.ConnectionGroupName;
+            get => JsonRpcClient.ConnectionGroupName;
             set => JsonRpcClient.ConnectionGroupName = value;
         }
 
         public int Timeout
         {
-            get => JsonRpcClient?.Timeout ?? STANDARD_TIMEOUT;
+            get => JsonRpcClient.Timeout;
             set => JsonRpcClient.Timeout = value;
         }
 
diff --git forkSrcPrefix/XenModel/XenAPI/JsonRpc.cs forkDstPrefix/XenModel/XenAPI/JsonRpc.cs
index 45fcdd6befa8b6f4c5acc2e16d62d51779ed2847..848d4e253348fba60ba7f227db1219791c4fc573 100755
--- forkSrcPrefix/XenModel/XenAPI/JsonRpc.cs
+++ forkDstPrefix/XenModel/XenAPI/JsonRpc.cs
@@ -163,6 +163,7 @@ namespace XenAPI
     public partial class JsonRpcClient
     {
         private int _globalId;
+        private string _userAgent;
 
 #if (NET8_0_OR_GREATER)
         private static readonly Type ClassType = typeof(JsonRpcClient);
@@ -205,6 +206,10 @@ namespace XenAPI
             Url = baseUrl;
             JsonRpcUrl = new Uri(new Uri(baseUrl), "/jsonrpc").ToString();
             JsonRpcVersion = JsonRpcVersion.v1;
+            Timeout = Session.STANDARD_TIMEOUT;
+            UserAgent = Session.DefaultUserAgent;
+            KeepAlive = true;
+            AllowAutoRedirect = true;
         }
 
         /// <summary>
@@ -215,7 +220,13 @@ namespace XenAPI
         public event Action<string> RequestEvent;
 
         public JsonRpcVersion JsonRpcVersion { get; set; }
-        public string UserAgent { get; set; }
+
+        public string UserAgent
+        {
+            get => _userAgent;
+            set => _userAgent = string.IsNullOrEmpty(value) ? Session.DefaultUserAgent : value;
+        }
+
         public bool KeepAlive { get; set; }
         public IWebProxy WebProxy { get; set; }
         public int Timeout { get; set; }