add LibraryName at ConfigurationOptions level (#2502)

* add LibraryName at ConfigurationOptions level * add PR number * add GetProvider * remove `libname` config-string; move docs to code-only section * remove s_DefaultProvider * Update docs/ReleaseNotes.md --------- Co-authored-by: Nick Craver <[email protected]>
StackExchange · Jul 7, 2023 · ce9506b · ce9506b
1 parent cd2dbb4
commit ce9506b
Show file tree

Hide file tree

Showing 8 changed files with 54 additions and 15 deletions.
diff --git a/docs/Configuration.md b/docs/Configuration.md
@@ -97,7 +97,7 @@ The `ConfigurationOptions` object has a wide range of properties, all of which a
 | tiebreaker={string}    | `TieBreaker`           | `__Booksleeve_TieBreak`      | Key to use for selecting a server in an ambiguous primary scenario                                        |
 | version={string}       | `DefaultVersion`       | (`4.0` in Azure, else `2.0`) | Redis version level (useful when the server does not make this available)                                 |
 | tunnel={string}        | `Tunnel`               | `null`                       | Tunnel for connections (use `http:{proxy url}` for "connect"-based proxy server)                          |
-| setlib={bool}          | `SetClientLibrary`     | `true`                       | Whether to attempt to use `CLIENT SETINFO` to set the lib name/version on the connection                  |
+| setlib={bool}          | `SetClientLibrary`     | `true`                       | Whether to attempt to use `CLIENT SETINFO` to set the library name/version on the connection              |
 
 Additional code-only options:
 - ReconnectRetryPolicy (`IReconnectRetryPolicy`) - Default: `ReconnectRetryPolicy = ExponentialRetry(ConnectTimeout / 2);`
@@ -115,6 +115,8 @@ Additional code-only options:
 - HeartbeatInterval - Default: `1000ms`
   - Allows running the heartbeat more often which importantly includes timeout evaluation for async commands. For example if you have a 50ms async command timeout, we're only actually checking it during the heartbeat (once per second by default), so it's possible 50-1050ms pass _before we notice it timed out_. If you want more fidelity in that check and to observe that a server failed faster, you can lower this to run the heartbeat more often to achieve that. 
   - **Note: heartbeats are not free and that's why the default is 1 second. There is additional overhead to running this more often simply because it does some work each time it fires.**
+- LibraryName - Default: `SE.Redis` (unless a `DefaultOptionsProvider` specifies otherwise)
+  - The library name to use with `CLIENT SETINFO` when setting the library name/version on the connection
 
 Tokens in the configuration string are comma-separated; any without an `=` sign are assumed to be redis server endpoints. Endpoints without an explicit port will use 6379 if ssl is not enabled, and 6380 if ssl is enabled.
 Tokens starting with `$` are taken to represent command maps, for example: `$config=cfg`.

diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md
@@ -11,6 +11,8 @@ Current package versions:
 - Change: Target net6.0 instead of net5.0, since net5.0 is end of life. ([#2497 by eerhardt](https://github.com/StackExchange/StackExchange.Redis/pull/2497))
 - Fix: Fix nullability annotation of IConnectionMultiplexer.RegisterProfiler ([#2494 by eerhardt](https://github.com/StackExchange/StackExchange.Redis/pull/2494))
 - Add: `Timer.ActiveCount` under `POOL` in timeout messages on .NET 6+ to help diagnose timer overload affecting timeout evaluations ([#2500 by NickCraver](https://github.com/StackExchange/StackExchange.Redis/pull/2500))
+- Add: `LibraryName` configuration option; allows the library name to be controlled at the individual options level (in addition to the existing controls in `DefaultOptionsProvider`) ([#2502 by mgravell](https://github.com/StackExchange/StackExchange.Redis/pull/2502))
+- Add: `DefaultOptionsProvider.GetProvider` allows lookup of provider by endpoint ([#2502 by mgravell](https://github.com/StackExchange/StackExchange.Redis/pull/2502))
 
 ## 2.6.116
 

diff --git a/src/StackExchange.Redis/Configuration/AzureOptionsProvider.cs b/src/StackExchange.Redis/Configuration/AzureOptionsProvider.cs
@@ -1,8 +1,7 @@
-using System;
-using System.Collections.Generic;
+using StackExchange.Redis.Maintenance;
+using System;
 using System.Net;
 using System.Threading.Tasks;
-using StackExchange.Redis.Maintenance;
 
 namespace StackExchange.Redis.Configuration
 {

diff --git a/src/StackExchange.Redis/Configuration/DefaultOptionsProvider.cs b/src/StackExchange.Redis/Configuration/DefaultOptionsProvider.cs
@@ -52,7 +52,7 @@ public static void AddProvider(DefaultOptionsProvider provider)
         /// <summary>
         /// Gets a provider for the given endpoints, falling back to <see cref="DefaultOptionsProvider"/> if nothing more specific is found.
         /// </summary>
-        internal static Func<EndPointCollection, DefaultOptionsProvider> GetForEndpoints { get; } = (endpoints) =>
+        public static DefaultOptionsProvider GetProvider(EndPointCollection endpoints)
         {
             foreach (var provider in KnownProviders)
             {
@@ -65,8 +65,23 @@ public static void AddProvider(DefaultOptionsProvider provider)
                 }
             }
 
-            return new DefaultOptionsProvider();
-        };
+            return new DefaultOptionsProvider(); // no memoize; allow mutability concerns (also impacts subclasses, but: pragmatism)
+        }
+
+        /// <summary>
+        /// Gets a provider for a given endpoints, falling back to <see cref="DefaultOptionsProvider"/> if nothing more specific is found.
+        /// </summary>
+        public static DefaultOptionsProvider GetProvider(EndPoint endpoint)
+        {
+            foreach (var provider in KnownProviders)
+            {
+                if (provider.IsMatch(endpoint))
+                {
+                    return provider;
+                }
+            }
+            return new DefaultOptionsProvider(); // no memoize; allow mutability concerns (also impacts subclasses, but: pragmatism)
+        }
 
         /// <summary>
         /// Gets or sets whether connect/configuration timeouts should be explicitly notified via a TimeoutException.

diff --git a/src/StackExchange.Redis/ConfigurationOptions.cs b/src/StackExchange.Redis/ConfigurationOptions.cs
@@ -180,7 +180,7 @@ public static string TryNormalize(string value)
         /// </summary>
         public DefaultOptionsProvider Defaults
         {
-            get => defaultOptions ??= DefaultOptionsProvider.GetForEndpoints(EndPoints);
+            get => defaultOptions ??= DefaultOptionsProvider.GetProvider(EndPoints);
             set => defaultOptions = value;
         }
 
@@ -233,14 +233,23 @@ public bool UseSsl
         }
 
         /// <summary>
-        /// Gets or sets whether the library should identify itself by library-name/version when possible
+        /// Gets or sets whether the library should identify itself by library-name/version when possible.
         /// </summary>
         public bool SetClientLibrary
         {
             get => setClientLibrary ?? Defaults.SetClientLibrary;
             set => setClientLibrary = value;
         }
 
+
+        /// <summary>
+        /// Gets or sets the library name to use for CLIENT SETINFO lib-name calls to Redis during handshake.
+        /// Defaults to "SE.Redis".
+        /// </summary>
+        /// <remarks>If the value is null, empty or whitespace, then the value from the options-provideer is used;
+        /// to disable the library name feature, use <see cref="SetClientLibrary"/> instead.</remarks>
+        public string? LibraryName { get; set; }
+
         /// <summary>
         /// Automatically encodes and decodes channels.
         /// </summary>
@@ -671,6 +680,7 @@ public static ConfigurationOptions Parse(string configuration, bool ignoreUnknow
 #endif
             Tunnel = Tunnel,
             setClientLibrary = setClientLibrary,
+            LibraryName = LibraryName,
         };
 
         /// <summary>

diff --git a/src/StackExchange.Redis/PublicAPI/PublicAPI.Shipped.txt b/src/StackExchange.Redis/PublicAPI/PublicAPI.Shipped.txt
@@ -243,6 +243,8 @@ StackExchange.Redis.ConfigurationOptions.IncludePerformanceCountersInExceptions.
 StackExchange.Redis.ConfigurationOptions.IncludePerformanceCountersInExceptions.set -> void
 StackExchange.Redis.ConfigurationOptions.KeepAlive.get -> int
 StackExchange.Redis.ConfigurationOptions.KeepAlive.set -> void
+StackExchange.Redis.ConfigurationOptions.LibraryName.get -> string?
+StackExchange.Redis.ConfigurationOptions.LibraryName.set -> void
 StackExchange.Redis.ConfigurationOptions.Password.get -> string?
 StackExchange.Redis.ConfigurationOptions.Password.set -> void
 StackExchange.Redis.ConfigurationOptions.PreserveAsyncOrder.get -> bool
@@ -1604,6 +1606,8 @@ static StackExchange.Redis.Condition.StringLengthLessThan(StackExchange.Redis.Re
 static StackExchange.Redis.Condition.StringNotEqual(StackExchange.Redis.RedisKey key, StackExchange.Redis.RedisValue value) -> StackExchange.Redis.Condition!
 static StackExchange.Redis.Configuration.DefaultOptionsProvider.AddProvider(StackExchange.Redis.Configuration.DefaultOptionsProvider! provider) -> void
 static StackExchange.Redis.Configuration.DefaultOptionsProvider.ComputerName.get -> string!
+static StackExchange.Redis.Configuration.DefaultOptionsProvider.GetProvider(StackExchange.Redis.EndPointCollection! endpoints) -> StackExchange.Redis.Configuration.DefaultOptionsProvider!
+static StackExchange.Redis.Configuration.DefaultOptionsProvider.GetProvider(System.Net.EndPoint! endpoint) -> StackExchange.Redis.Configuration.DefaultOptionsProvider!
 static StackExchange.Redis.Configuration.DefaultOptionsProvider.LibraryVersion.get -> string!
 static StackExchange.Redis.ConfigurationOptions.Parse(string! configuration) -> StackExchange.Redis.ConfigurationOptions!
 static StackExchange.Redis.ConfigurationOptions.Parse(string! configuration, bool ignoreUnknown) -> StackExchange.Redis.ConfigurationOptions!

diff --git a/src/StackExchange.Redis/ServerEndPoint.cs b/src/StackExchange.Redis/ServerEndPoint.cs
@@ -898,8 +898,9 @@ private async Task HandshakeAsync(PhysicalConnection connection, LogProxy? log)
             }
             Message msg;
             // Note that we need "" (not null) for password in the case of 'nopass' logins
-            string? user = Multiplexer.RawConfig.User;
-            string password = Multiplexer.RawConfig.Password ?? "";
+            var config = Multiplexer.RawConfig;
+            string? user = config.User;
+            string password = config.Password ?? "";
             if (!string.IsNullOrWhiteSpace(user))
             {
                 log?.WriteLine($"{Format.ToString(this)}: Authenticating (user/password)");
@@ -929,13 +930,19 @@ private async Task HandshakeAsync(PhysicalConnection connection, LogProxy? log)
                         await WriteDirectOrQueueFireAndForgetAsync(connection, msg, ResultProcessor.DemandOK).ForAwait();
                     }
                 }
-                if (Multiplexer.RawConfig.SetClientLibrary)
+                if (config.SetClientLibrary)
                 {
                     // note that this is a relatively new feature, but usually we won't know the
                     // server version, so we will use this speculatively and hope for the best
                     log?.WriteLine($"{Format.ToString(this)}: Setting client lib/ver");
 
-                    var libName = Multiplexer.RawConfig.Defaults.LibraryName;
+                    var libName = config.LibraryName;
+                    if (string.IsNullOrWhiteSpace(libName))
+                    {
+                        // defer to provider if missing (note re null vs blank; if caller wants to disable
+                        // it, they should set SetClientLibrary to false, not set the name to empty string)
+                        libName = config.Defaults.LibraryName;
+                    }
                     if (!string.IsNullOrWhiteSpace(libName))
                     {
                         msg = Message.Create(-1, CommandFlags.FireAndForget, RedisCommand.CLIENT,

diff --git a/tests/StackExchange.Redis.Tests/DefaultOptionsTests.cs b/tests/StackExchange.Redis.Tests/DefaultOptionsTests.cs
@@ -52,11 +52,11 @@ public void IsMatchOnDomain()
         DefaultOptionsProvider.AddProvider(new TestOptionsProvider(".testdomain"));
 
         var epc = new EndPointCollection(new List<EndPoint>() { new DnsEndPoint("local.testdomain", 0) });
-        var provider = DefaultOptionsProvider.GetForEndpoints(epc);
+        var provider = DefaultOptionsProvider.GetProvider(epc);
         Assert.IsType<TestOptionsProvider>(provider);
 
         epc = new EndPointCollection(new List<EndPoint>() { new DnsEndPoint("local.nottestdomain", 0) });
-        provider = DefaultOptionsProvider.GetForEndpoints(epc);
+        provider = DefaultOptionsProvider.GetProvider(epc);
         Assert.IsType<DefaultOptionsProvider>(provider);
     }