Skip to content

Convert all logging to structured message templates - #349

Merged
lahma merged 27 commits into
mainfrom
log-messages
Aug 9, 2026
Merged

Convert all logging to structured message templates#349
lahma merged 27 commits into
mainfrom
log-messages

Conversation

@lahma

@lahma lahma commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Closes #265.

All ~890 ILogger call sites across src/, test/ and examples/ now use constant message templates with PascalCase named placeholders instead of string interpolation, concatenation or string.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

  • Latent crash fix: ~97 sites passed dynamically built strings as the template. Text containing braces (SpEL literals {1,2,3}, ODBC escapes {call sp}, Velocity ${var}) was parsed as a malformed template. Pass-through adapters (SimpleLoggingAdvice, NVelocity CommonsLoggingLogSystem, NMS NmsTrace, FailFastProblemReporter, …) now log via a constant "{Message}" template; sites whose message was assembled next to the call got real templates.
  • Credential leak fix: AdoPlatformTransactionManager/ConnectionUtils no longer log ConnectionString (the concern raised in Log messages use concatenated strings #265).
  • Exception handling: sites that flattened exceptions into message text now use the exception-first overloads; two examples that passed the exception as an unused template argument are fixed; redundant (Exception) casts removed.
  • Regression guard: CA2254/CA2253/CA2017/CA1727 are now build errors (EnableNETAnalyzers + AnalysisMode=None + editorconfig opt-in), so non-constant templates can't come back.
  • MEL.Abstractions floor 2.1.0 → 8.0.0: 2.1.x built a formatter for any message containing { even with zero args (render-time FormatException) and had no template caching. NUnitAspEx-hosted test AppDomains needed System.Memory/Unsafe binding redirects in their web.configs as a consequence.
  • 348 now-redundant IsEnabled guards removed (templates defer formatting); guards kept where argument evaluation is expensive, levels mismatch (verbosity gates), or contracts pin behavior (SimpleLoggingAdvice).
  • Spring.Messaging.Ems also got its four orphaned ILog references fixed (the project did not compile since the ILog alias removal).

Rendered-output changes (documented in BreakingChanges.txt)

  • Invariant culture instead of current culture for argument formatting
  • null args render as (null)
  • Exceptions render via the sink's exception slot instead of inline text
  • A few collection args render as comma-separated lists
  • Connection strings redacted as above

Verification

  • Full solution rebuild with the four CA rules as errors: 0 warnings, 0 errors
  • All locally runnable suites green on net8.0 + net462 (Spring.Web.Tests 202/202 incl. hosted-AppDomain fixtures); the one local failure is the machine-specific FormatUsingDefaults date-format quirk
  • The out-of-order string.Format sites ({2}…{0}…{1}) were argument-reordered and hand-verified
  • Spring.Messaging.Ems is 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
  • Rendered text was kept byte-identical except where listed above; the two test fixtures that assert rendered log output pass unchanged

🤖 Generated with Claude Code

lahma and others added 27 commits August 9, 2026 11:37
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>
@lahma
lahma merged commit 54334e9 into main Aug 9, 2026
3 checks passed
@lahma
lahma deleted the log-messages branch August 9, 2026 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Log messages use concatenated strings

1 participant