From 35d6e23390a5b39b5ed2d5ce01f40a02b814c98d Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 8 Oct 2026 16:45:41 +0100 Subject: [PATCH] Fix duplicate LoggerMessage event ids; add a guard test Three source-generated log events shared an EventId with another event: 116 and 117 (maintenance notifications vs. slot-map/undialled-server events, landed a day apart on parallel branches) and 99 (the two TLS "connection established" overloads, with different templates). LogErrorConnectionFailed had no explicit id at all and was generated as 0. Renumber the newer side of each pair (124, 125, 126) and give the connection-failed event 127. Add LoggerEventIdTests, which reflects over LoggerExtensions and fails if any [LoggerMessage] lacks an explicit EventId or if any id is used twice, reporting every problem at once and logging the next free id. --- src/StackExchange.Redis/LoggerExtensions.cs | 7 ++- .../LoggerEventIdTests.cs | 60 +++++++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 tests/StackExchange.Redis.Tests/LoggerEventIdTests.cs diff --git a/src/StackExchange.Redis/LoggerExtensions.cs b/src/StackExchange.Redis/LoggerExtensions.cs index 69e117927..6617f3df6 100644 --- a/src/StackExchange.Redis/LoggerExtensions.cs +++ b/src/StackExchange.Redis/LoggerExtensions.cs @@ -71,6 +71,7 @@ internal static void LogWithThreadPoolStats(this ILogger? log, string message) // awkwardly they read. If a name genuinely must change, pin the old one via the attribute's EventName. [LoggerMessage( Level = LogLevel.Error, + EventId = 127, Message = "Connection failed: {EndPoint} ({ConnectionType}, {FailureType}): {ErrorMessage}")] internal static partial void LogErrorConnectionFailed(this ILogger logger, Exception? exception, EndPointLogValue endPoint, ConnectionType connectionType, ConnectionFailureType failureType, string errorMessage); @@ -768,7 +769,7 @@ internal static void LogWithThreadPoolStats(this ILogger? log, string message) // established" arrived under one id on netfx and another on modern TFMs, for no gain to anyone. [LoggerMessage( Level = LogLevel.Information, - EventId = 99, + EventId = 126, Message = "TLS connection established successfully using protocol: {SslProtocol}, cipher suite: {CipherSuite}")] internal static partial void LogInformationTLSConnectionEstablished(this ILogger logger, System.Security.Authentication.SslProtocols sslProtocol, System.Net.Security.TlsCipherSuite cipherSuite); #endif @@ -781,13 +782,13 @@ internal static void LogWithThreadPoolStats(this ILogger? log, string message) [LoggerMessage( Level = LogLevel.Information, - EventId = 116, + EventId = 124, Message = "{Server}: Requesting maintenance notifications ({Mode})")] internal static partial void LogInformationRequestingMaintenanceNotifications(this ILogger logger, ServerEndPointLogValue server, MaintenanceNotificationMode mode); [LoggerMessage( Level = LogLevel.Information, - EventId = 117, + EventId = 125, Message = "{Server}: Maintenance notifications accepted")] internal static partial void LogInformationMaintenanceNotificationsAccepted(this ILogger logger, ServerEndPointLogValue server); diff --git a/tests/StackExchange.Redis.Tests/LoggerEventIdTests.cs b/tests/StackExchange.Redis.Tests/LoggerEventIdTests.cs new file mode 100644 index 000000000..4c014a735 --- /dev/null +++ b/tests/StackExchange.Redis.Tests/LoggerEventIdTests.cs @@ -0,0 +1,60 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using Microsoft.Extensions.Logging; +using Xunit; + +namespace StackExchange.Redis.Tests; + +/// +/// Guards the source-generated events in LoggerExtensions: +/// every event must carry an explicit EventId, and no two events may share one. Event ids are +/// what consumers filter and alert on, so a clash silently merges unrelated events downstream; this +/// has happened when parallel branches both took "the next free number". +/// +public class LoggerEventIdTests(ITestOutputHelper output) : TestBase(output) +{ + [Fact] + public void EventIdsAreExplicitAndUnique() + { + var events = ( + from method in typeof(LoggerExtensions).GetMethods(BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic) + let attrib = method.GetCustomAttribute() + where attrib is not null + orderby attrib.EventId, method.Name + select (Method: method, Attrib: attrib)).ToList(); + + Assert.NotEmpty(events); // if reflection stops finding them, the test is vacuous + + var problems = new List(); + + // LoggerMessageAttribute.EventId defaults to -1 when not specified; the generator then emits 0, + // which is indistinguishable from "unknown" to a consumer + foreach (var (method, attrib) in events.Where(e => e.Attrib.EventId < 0)) + { + problems.Add($"{Describe(method)} has no explicit EventId"); + } + + foreach (var group in events.Where(e => e.Attrib.EventId >= 0).GroupBy(e => e.Attrib.EventId).Where(g => g.Count() > 1)) + { + problems.Add($"EventId {group.Key} is used {group.Count()} times: {string.Join(", ", group.Select(e => Describe(e.Method)))}"); + } + + int max = events.Max(e => e.Attrib.EventId); + Log($"{events.Count} events; highest EventId {max}; next free EventId {max + 1}"); + foreach (var problem in problems) + { + Log(problem); + } + + Assert.True(problems.Count == 0, string.Join(Environment.NewLine, problems)); + } + + private static string Describe(MethodInfo method) + { + // skip the "this ILogger" parameter; show the rest so overloads are distinguishable + var args = method.GetParameters().Skip(1).Select(p => p.ParameterType.Name); + return $"{method.Name}({string.Join(", ", args)})"; + } +}