Convert all logging to structured message templates - #349
Merged
Conversation
Four field declarations still used the ILog alias that was removed from src/ in the Common.Logging -> MEL migration (07049eb), leaving the project uncompilable for --build-ems users. Replace with ILogger. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
Turns on .NET analyzers (AnalysisMode=None) with only the four logging template rules opted in: CA2254 (static templates), CA2253 (no numeric placeholders), CA2017 (arity), CA1727 (PascalCase placeholders). WarningsNotAsErrors keeps them non-fatal while call sites are converted to message templates (#265); the escape hatch is removed once the conversion is complete. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
Aligns with the sibling MEL 8.0.0 entries. On 2.1.x, FormattedLogValues
builds a formatter for any message containing '{' even with zero
arguments, turning dynamic log messages with braces (SpEL literals,
ODBC escapes, Velocity syntax) into render-time FormatExceptions; 3.0+
adds the empty-args guard and template caching.
MEL 3+ also dispatches Log<FormattedLogValues> (struct TState) instead
of Log<object>, so the FakeItEasy helper in SimpleLoggingAdviceTests now
matches log calls untyped.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
- AssemblyTypeScanner: numeric {0} placeholder becomes {FilteredAssemblies}
- ScannedGenericObjectDefinition: {ComponentNAme} typo, and the second
placeholder is named {Definition} to match its ToString() argument
- AbstractAutowireCapableObjectFactory: two templates missing the
closing quote around '{ObjectName}'
Part of #265.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
Sites that concatenated the exception into the message now use the
exception-first Log* overloads, so sinks receive the structured
exception (stack trace rendering moves from message text to the
exception slot). The DisposableObjectAdapter level-dependent if/else
blocks collapse into single exception-first calls.
Also fixes the two examples that passed the exception as an unused
template argument (LogError("msg", e)), which silently dropped it.
Part of #265.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
Pass-through log sinks (SimpleLoggingAdvice.WriteToLog, NVelocity
CommonsLoggingLogSystem, Apache NMS NmsTrace, FailFastProblemReporter)
and other sites whose message text is built at runtime now log via a
constant "{Message}" template instead of passing the dynamic string as
the template. Dynamic text containing braces (SpEL literals, SQL escape
syntax, Velocity ${var}) is no longer parsed as a template, and each
call site now caches a single template formatter.
Where the dynamic string was assembled next to the call, the call gets
a real template instead (SpEL exception handlers, CachingConnectionFactory,
SimpleAdoTestUtils, PropertyResourceConfigurer, AbstractApplicationContext).
Rendered output is unchanged.
Part of #265.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
The hosted ASP.NET test domain does not inherit the test assembly's auto-generated binding redirects. MEL 8.x pulls System.Memory and System.Runtime.CompilerServices.Unsafe on net462, which fail to load in that domain without redirects. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
Locally-built message strings passed to Log* calls become real
templates where the string was assembled next to the call (the local is
deleted), or a "{Message}" template where the string is also thrown or
comes from an overridable member. Rendered output is unchanged except
where the exception object moves from message text to the exception
slot.
Part of #265.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
AdoPlatformTransactionManager and ConnectionUtils logged IDbConnection.ConnectionString at Debug level, which can leak credentials - the concern raised in #265. The messages now identify the connection object only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
AbstractApplicationContext, AbstractAutowireCapableObjectFactory and
AbstractObjectFactory: interpolation, concatenation and string.Format
messages become constant templates with PascalCase placeholders.
Out-of-order string.Format indexes ({2}...{0}...{1}) had their argument
lists reordered to match placeholder appearance order, and redundant
CultureInfo.InvariantCulture arguments were dropped (MEL formats
invariantly). Rendered output is unchanged; one missing apostrophe
around '{InitMethodName}' was fixed.
Part of #265.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
Part of #265. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
One deliberate rendering change: DataSetFill table names (string[]) now render as a comma-separated list instead of System.String[]. Part of #265. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
Part of #265. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
Remaining Spring.Core files: interpolation, concatenation and string.Format messages become constant templates. Constant holes (ObjectDefinitionConstants members) stay in template text via const concatenation; dead System.Globalization usings removed where the culture argument was the last use. Part of #265. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
Part of #265. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
Sync and async twins share placeholder names for identical messages. Part of #265. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
Part of #265. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
…lates Part of #265. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
The WebApplicationContextTests root hit the same System.Memory/Unsafe load failure once its logging path started using message templates; LocalResourceManagerTests and PageHandlerFactoryTests get the same treatment preemptively. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
…emplates Testing.NUnit and Testing.Microsoft twins share placeholder names, as do the two SpringResourceLoader copies. One rendering change: Velocity resource loader paths (string[]) now render as a comma-separated list. Part of #265. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
Placeholder names match the Spring.Messaging.Nms twins. This project is excluded from the solution build (needs TIBCO.EMS.dll), so these edits are verified by review and placeholder/argument arity audit only. Part of #265. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
The Spring.Calculator CommonLoggingAroundAdvice pass-through logs via a
{Message} template. Test fixtures asserting rendered log output were
not touched.
Part of #265.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
Leftovers from the Common.Logging migration; every argument's static type already derives from System.Exception. Also converts the one log call the sweep missed (a UTF-16-encoded test file). Part of #265. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
MEL defers formatting, so a guard around a single template call with cheap arguments (locals, fields, simple property reads) buys nothing. 348 such guards removed. Guards kept where they still pay for themselves: expensive argument evaluation (string building, GetType/ ToString/helper calls, new expressions), cached-boolean guards, level-mismatch verbosity gates, multi-statement bodies, and the documented SimpleLoggingAdvice/NmsTrace contracts. Part of #265. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
Removes the WarningsNotAsErrors escape hatch: CA2254 (static templates), CA2253 (no numeric placeholders), CA2017 (arity) and CA1727 (PascalCase placeholders) now fail the build via TreatWarningsAsErrors. The full solution rebuild is clean, closing #265. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
- Remove the dead Common.Logging package pin (nothing references it) - Remove the inert WarningsAsErrors=True properties (the property takes a list of warning IDs; TreatWarningsAsErrors does the real work) - Deduplicate the ILog global-using ItemGroup in test props - BreakingChanges.txt: document the MEL switch, structured-template rendering changes, connection-string redaction and the MEL 8.0 floor - AGENTS.md: logging conventions and enforcing analyzers Closes #265. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.com>
ContextUtil.TransactionId/ActivityId throw when no COM+ context is active (as with the test service-domain adapter), so the IsEnabled guard must prevent argument evaluation, not just formatting. Fixes the net462 Spring.Data.Tests failures on CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Marko Lahma <marko.lahma@gmail.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.
Closes #265.
All ~890
ILoggercall sites acrosssrc/,test/andexamples/now use constant message templates with PascalCase named placeholders instead of string interpolation, concatenation orstring.Format. Structured sinks (NLog${message:raw=true}, Serilog, etc.) get real templates and named properties, and message-template-based categorization works for Spring logs.Highlights
{1,2,3}, ODBC escapes{call sp}, Velocity${var}) was parsed as a malformed template. Pass-through adapters (SimpleLoggingAdvice, NVelocityCommonsLoggingLogSystem, NMSNmsTrace,FailFastProblemReporter, …) now log via a constant"{Message}"template; sites whose message was assembled next to the call got real templates.AdoPlatformTransactionManager/ConnectionUtilsno longer logConnectionString(the concern raised in Log messages use concatenated strings #265).(Exception)casts removed.EnableNETAnalyzers+AnalysisMode=None+ editorconfig opt-in), so non-constant templates can't come back.{even with zero args (render-timeFormatException) and had no template caching. NUnitAspEx-hosted test AppDomains neededSystem.Memory/Unsafebinding redirects in theirweb.configs as a consequence.IsEnabledguards removed (templates defer formatting); guards kept where argument evaluation is expensive, levels mismatch (verbosity gates), or contracts pin behavior (SimpleLoggingAdvice).Spring.Messaging.Emsalso got its four orphanedILogreferences fixed (the project did not compile since theILogalias removal).Rendered-output changes (documented in BreakingChanges.txt)
nullargs render as(null)Verification
Spring.Web.Tests202/202 incl. hosted-AppDomain fixtures); the one local failure is the machine-specificFormatUsingDefaultsdate-format quirkstring.Formatsites ({2}…{0}…{1}) were argument-reordered and hand-verifiedSpring.Messaging.Emsis excluded from the solution build (needs TIBCO.EMS.dll), so its conversions are verified by review plus a placeholder/argument arity lint (59/59 clean) — CI cannot compile this project🤖 Generated with Claude Code