Fix SimpleLoggingAdvice configuration breakage from the MS logging migration - #346
Merged
Conversation
- Accept legacy Common.Logging level names (All, Info, Warn, Fatal, Off) when converting strings to Microsoft.Extensions.Logging.LogLevel via a new LogLevelConverter registered in TypeConverterRegistry - Resolve named advice loggers lazily so a LogManager.LoggerFactory assigned after the advice was constructed still takes effect (also keeps a deserialized static-mode advice in static mode) - Use the concrete advice type as the default log category instead of always AbstractLoggingAdvice Fixes #341 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #341.
The Common.Logging → Microsoft.Extensions.Logging migration (fda46d9, #267) broke
SimpleLoggingAdvicein two ways for anyone upgrading from ≤3.0.2:<property name="LogLevel" value="Info"/>throwsTypeMismatchException, because MEL'sLogLevelhasInformation/Warning/Critical/Nonewhere Common.Logging hadInfo/Warn/Fatal/Off/All. The reference docs and example configs still use the old spellings.AbstractLoggingAdviceresolved its static logger eagerly in the constructor. WithLogManager.LoggerFactorytypically still unset while the container builds the advice, that cachedNullLogger.Instanceforever — assigning the factory afterwards had no effect. The default log category was also alwaysAbstractLoggingAdvice(aMethodBase.GetCurrentMethod().DeclaringTypeaccident), never the concrete advice type, so category-based filter rules didn't match.Changes
LogLevelConverter(Spring.Core, registered inTypeConverterRegistry): accepts the legacy Common.Logging names case-insensitively —All→Trace,Info→Information,Warn→Warning,Fatal→Critical,Off→None— and delegates everything else toEnumConverter. Pre-3.0 XML configs (and the shipped reference docs/examples usingvalue="Info") work again.LogExceptionHandler.LogLevelgains the same support for free.AbstractLoggingAdvice:SetDefaultLoggernow only stores the category name; the logger is resolved throughLogManagerper invocation (MEL'sLoggerFactorycaches loggers by category, so this matches the cost profile dynamic mode always had). ALogManager.LoggerFactoryassigned any time before the first intercepted call now takes effect. This also keeps a deserialized static-mode advice in static mode (defaultLoggeris[NonSerialized]; the name survives).Spring.Aspects.Logging.SimpleLoggingAdvice), consistent with what theUseDynamicLogger = falsesetter path already did and with Java Spring'sgetClass()behavior.Behavior changes to be aware of
Spring.Aspects.Logging.AbstractLoggingAdvice(accidental) to the concrete advice type's full name. Filter rules keyed on the old name need adjusting.defaultLoggerfield now strictly means "explicitly supplied logger"; in named-logger mode it staysnulland resolution goes throughLogManagerper invocation.Note for the issue reporter
The other half of #341 — no output even with
Information— is expected in 3.x untilSpring.LogManager.LoggerFactoryis assigned (Common.Logging's app.config auto-configuration is gone). E.g. for log4net:LogManager.LoggerFactory = LoggerFactory.Create(b => b.AddLog4Net());. With this PR a factory assigned even after the context is built is picked up.Follow-up (not in this PR)
LogExceptionHandlerbuilds a SpEL expression#log.Information(...)etc. — those instance methods don't exist on MELILogger(they'reLogXxxextension methods), so its log action silently fails; needs a separate fix.Verification
TypeConversionUtilsend-to-end, lateLoggerFactorywiring, default category, and anXmlObjectFactoryrepro of the exact config from the issue.dotnet build Spring.Net.slnclean; full Spring.Aop.Tests and Spring.Core.Tests suites green on net8.0 and net462 (only known-local-env failureFormatUsingDefaultsunrelated to this change).🤖 Generated with Claude Code