Add logging to M.E.Caching.StackExchangeRedis (#41769)

src/Caching/StackExchangeRedis/src/Microsoft.Extensions.Caching.StackExchangeRedis.csproj

@@ -12,6 +12,7 @@
 
   <ItemGroup>
     <Reference Include="Microsoft.Extensions.Caching.Abstractions" />
+    <Reference Include="Microsoft.Extensions.Logging.Abstractions" />
     <Reference Include="Microsoft.Extensions.Options" />
     <Reference Include="StackExchange.Redis" />
   </ItemGroup>

src/Caching/StackExchangeRedis/src/RedisCache.Log.cs

@@ -0,0 +1,16 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using Microsoft.Extensions.Logging;
+
+namespace Microsoft.Extensions.Caching.StackExchangeRedis;
+
+public partial class RedisCache
+{
+    private static partial class Log
+    {
+        [LoggerMessage(1, LogLevel.Warning, "Could not determine the Redis server version. Falling back to use HMSET command instead of HSET.", EventName = "CouldNotDetermineServerVersion")]
+        public static partial void CouldNotDetermineServerVersion(ILogger logger, Exception exception);
+    }
+}

src/Caching/StackExchangeRedis/src/RedisCache.cs

@@ -7,6 +7,7 @@ using System.Diagnostics.CodeAnalysis;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Extensions.Caching.Distributed;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 using StackExchange.Redis;
 
@@ -16,7 +17,7 @@ namespace Microsoft.Extensions.Caching.StackExchangeRedis;
 /// Distributed cache implementation using Redis.
 /// <para>Uses <c>StackExchange.Redis</c> as the Redis client.</para>
 /// </summary>
-public class RedisCache : IDistributedCache, IDisposable
+public partial class RedisCache : IDistributedCache, IDisposable
 {
     // -- Explanation of why two kinds of SetScript are used --
     // * Redis 2.0 had HSET key field value for setting individual hash fields,
@@ -58,6 +59,7 @@ public class RedisCache : IDistributedCache, IDisposable
 
     private readonly RedisCacheOptions _options;
     private readonly string _instance;
+    private readonly ILogger _logger;
 
     private readonly SemaphoreSlim _connectionLock = new SemaphoreSlim(initialCount: 1, maxCount: 1);
 
@@ -66,13 +68,29 @@ public class RedisCache : IDistributedCache, IDisposable
     /// </summary>
     /// <param name="optionsAccessor">The configuration options.</param>
     public RedisCache(IOptions<RedisCacheOptions> optionsAccessor)
+        : this(optionsAccessor, Logging.Abstractions.NullLoggerFactory.Instance.CreateLogger<RedisCache>())
+    {
+    }
+
+    /// <summary>
+    /// Initializes a new instance of <see cref="RedisCache"/>.
+    /// </summary>
+    /// <param name="optionsAccessor">The configuration options.</param>
+    /// <param name="logger">The logger.</param>
+    internal RedisCache(IOptions<RedisCacheOptions> optionsAccessor, ILogger logger)
     {
         if (optionsAccessor == null)
         {
             throw new ArgumentNullException(nameof(optionsAccessor));
         }
 
+        if (logger == null)
+        {
+            throw new ArgumentNullException(nameof(logger));
+        }
+
         _options = optionsAccessor.Value;
+        _logger = logger;
 
         // This allows partitioning a single backend cache for use with multiple apps/services.
         _instance = _options.InstanceName ?? string.Empty;
@@ -303,8 +321,10 @@ public class RedisCache : IDistributedCache, IDisposable
                 }
             }
         }
-        catch (NotSupportedException)
+        catch (NotSupportedException ex)
         {
+            Log.CouldNotDetermineServerVersion(_logger, ex);
+
             // The GetServer call may not be supported with some configurations, in which
             // case let's also fall back to using the older command.
             _setScript = SetScriptPreExtendedSetCommand;

src/Caching/StackExchangeRedis/src/RedisCacheImpl.cs

@@ -0,0 +1,20 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
+
+namespace Microsoft.Extensions.Caching.StackExchangeRedis;
+
+internal sealed class RedisCacheImpl : RedisCache
+{
+    public RedisCacheImpl(IOptions<RedisCacheOptions> optionsAccessor, ILogger<RedisCache> logger)
+        : base(optionsAccessor, logger)
+    {
+    }
+
+    public RedisCacheImpl(IOptions<RedisCacheOptions> optionsAccessor)
+        : base(optionsAccessor)
+    {
+    }
+}

src/Caching/StackExchangeRedis/src/StackExchangeRedisCacheServiceCollectionExtensions.cs

@@ -32,8 +32,9 @@ public static class StackExchangeRedisCacheServiceCollectionExtensions
         }
 
         services.AddOptions();
+
         services.Configure(setupAction);
-        services.Add(ServiceDescriptor.Singleton<IDistributedCache, RedisCache>());
+        services.Add(ServiceDescriptor.Singleton<IDistributedCache, RedisCacheImpl>());
 
         return services;
     }

src/Caching/StackExchangeRedis/test/CacheServiceExtensionsTests.cs

@@ -4,6 +4,8 @@
 using System.Linq;
 using Microsoft.Extensions.Caching.Distributed;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Abstractions;
 using Moq;
 using Xunit;
 
@@ -44,7 +46,7 @@ public class CacheServiceExtensionsTests
 
         Assert.NotNull(distributedCache);
         Assert.Equal(ServiceLifetime.Scoped, distributedCache.Lifetime);
-        Assert.IsType<RedisCache>(serviceProvider.GetRequiredService<IDistributedCache>());
+        Assert.IsAssignableFrom<RedisCache>(serviceProvider.GetRequiredService<IDistributedCache>());
     }
 
     [Fact]
@@ -54,4 +56,69 @@ public class CacheServiceExtensionsTests
 
         Assert.Same(services, services.AddStackExchangeRedisCache(_ => { }));
     }
+
+    [Fact]
+    public void AddStackExchangeRedisCache_IDistributedCacheWithoutLoggingCanBeResolved()
+    {
+        // Arrange
+        var services = new ServiceCollection();
+
+        // Act
+        services.AddStackExchangeRedisCache(options => { });
+
+        // Assert
+        using var serviceProvider = services.BuildServiceProvider();
+        var distributedCache = serviceProvider.GetRequiredService<IDistributedCache>();
+
+        Assert.NotNull(distributedCache);
+    }
+
+    [Fact]
+    public void AddStackExchangeRedisCache_IDistributedCacheWithLoggingCanBeResolved()
+    {
+        // Arrange
+        var services = new ServiceCollection();
+
+        // Act
+        services.AddStackExchangeRedisCache(options => { });
+        services.AddLogging();
+
+        // Assert
+        using var serviceProvider = services.BuildServiceProvider();
+        var distributedCache = serviceProvider.GetRequiredService<IDistributedCache>();
+
+        Assert.NotNull(distributedCache);
+    }
+
+    [Fact]
+    public void AddStackExchangeRedisCache_UsesLoggerFactoryAlreadyRegisteredWithServiceCollection()
+    {
+        // Arrange
+        var services = new ServiceCollection();
+        services.AddScoped(typeof(IDistributedCache), sp => Mock.Of<IDistributedCache>());
+
+        var loggerFactory = new Mock<ILoggerFactory>();
+
+        loggerFactory
+            .Setup(lf => lf.CreateLogger(It.IsAny<string>()))
+            .Returns((string name) => NullLoggerFactory.Instance.CreateLogger(name))
+            .Verifiable();
+
+        services.AddScoped(typeof(ILoggerFactory), _ => loggerFactory.Object);
+
+        // Act
+        services.AddLogging();
+        services.AddStackExchangeRedisCache(options => { });
+
+        // Assert
+        var serviceProvider = services.BuildServiceProvider();
+
+        var distributedCache = services.FirstOrDefault(desc => desc.ServiceType == typeof(IDistributedCache));
+
+        Assert.NotNull(distributedCache);
+        Assert.Equal(ServiceLifetime.Scoped, distributedCache.Lifetime);
+        Assert.IsAssignableFrom<RedisCache>(serviceProvider.GetRequiredService<IDistributedCache>());
+
+        loggerFactory.Verify();
+    }
 }