Add ILogger API with interceptors, carriers, and unified dispatch#25
Merged
Add ILogger API with interceptors, carriers, and unified dispatch#25
Conversation
Prototypes validating the new ILogger API approach: - LoggerPrototype: InterpolatedStringHandler dual-buffer (text+JSON), CallerArgumentExpression, typed JSON, short-circuit, string overloads - RefStructTest: ref struct implementing IDisposable and custom interfaces, allows ref struct generic constraint - ConditionalTest: [Conditional] removes entire call site including handler construction and side effects - InterceptorConditionalTest: [Conditional] on interceptor does NOT work, must be on original method; new .NET 10 interceptor API validated - WhenInterceptorTest: When(false)->NullLogger->handler disabled flow with real source generator interceptors Design summary (prototype/summary.md) covers all decisions, the Quarry-inspired carrier chain pattern, outstanding questions, and deliberate exclusions.
Resolved 18 issues from API design and systems engineering reviews: Critical fixes: - Remove phantom LogChain type; all chains use ILogger + generated carriers - Scoped() only adds path segment, no persistent structured properties - Full ILogger interface declaration with all user-callable methods - Re-entrant logging returns NullLogger (carrier _inUse guard) - Add Prototype Limitations section noting alloc gaps Consistency fixes: - Unified Dispatch via DispatchInfo ref struct - Handler naming: LogInformationHandler (consistent with method name) - Handler constructor overload for Exception parameter - TimingOperation implements ILogger for timed+logging ergonomics - GetJsonWritten() guarded with _jsonFinalized boolean - PathSegment uses Volatile.Write/Interlocked (ARM64 safe, behind flag) - When() allowed at any chain position (LSMITH012 dropped) - RateLimited(N) added as chain method - Supplemental compilation note corrected - Disabled-path cost table: Direct ~3-5ns, Chained ~8-15ns, Static 0ns - Generator chain detection must use SemanticModel (requirement noted) - sink.AsLogger() removed from migration path - Resolved Decisions table added for traceability
16 issues resolved from second review cycle: Critical: - Re-entrancy code fixed to return NullLogger (was contradicting decision) - Chain methods (Sampled/RateLimited/Tagged) get => this defaults - [Conditional]+ref validated, noted in doc Medium: - Scoped() is now extension method returning concrete LogScope struct (plain string param, no handler, no boxing) - Static tier Dispatch calls updated to use DispatchInfo - PathSegment TOCTOU race documented as accepted trade-off - Multi-tag deferred; single Tagged(string) only for v2 Low: - logger.Info() -> logger.Information() naming fix - NullLogger gets static sentinel LoggerContext - GetJsonWritten guard marked as production-only fix - Sampling counter uses (uint) cast for overflow correctness - Scoped() no longer uses handler (plain string avoids silent discard) - LogScope/TimingOperation added to Core Types - DispatchInfo sync consumption documented in limitations - TimingOperation noted on ILogger interface as extension method
9 issues resolved from third review cycle:
Must-fix:
- Terminal methods now show full default implementations (Debug pattern,
all levels identical). Carriers/NullLogger only need Context property.
- Static Log class methods get [InterpolatedStringHandlerArgument("logger")]
on handler parameters (required for compiler to pass ILogger to handler ctor)
- Carrier _inUse released on handler.IsEnabled early-return path (prevents
permanent carrier lock on dynamic level change race)
Minor:
- NullLogger fully specified: sentinel context, no-op PathSegment, CreateChild
returns Instance, Scoped() creates no-op LogScope
- TimingOperation method surface declared: Complete/Fail/TimeStep/Dispose
- Stale "multi-tag support" changed to "single-tag filtering"
- PathSegment/Segment naming delegation explained
- Chained disabled cost notes scaling with interpolation hole count
- CreateChild gets default implementation (=> NullLogger.Instance)
All 5 remaining questions now have final decisions: - Handler types: Separate per level (LogDebugHandler, etc.) wrapping shared LogHandlerCore. Clean signatures, no hidden params. - DPanic: Dedicated method on ILogger, dispatches at Error level, throws InvalidOperationException in DEBUG builds only. - Event IDs: FNV-1a hash of concatenated template literal parts. Same template = same ID. Variable renames don't affect ID. - Config inheritance: Children share parent SinkSet by reference. MinimumLevel overridable per child. Parent reconfig affects children. - Chain detection: SemanticModel resolution required at each chain step to confirm ILogger receiver. Syntax name matching alone rejected. No outstanding questions remain. Design summary is implementation-ready.
Consolidate the dispatch infrastructure as the foundation for the ILogger rework. DispatchInfo carries all log data (text, JSON, path, tag, caller info) through a single ref struct. ILogSink.Write now takes in DispatchInfo instead of separate LogEntry + ReadOnlySpan<byte> parameters. - Add DispatchInfo ref struct with Utf8Message/Utf8Json/Utf8Path spans - Unify ILogSink and IStructuredLogSink into single Write(in DispatchInfo) - Remove LogEntry, LogScope (AsyncLocal), IStructuredLogSink, WriteProperties - Update all sinks (Console, File, Stream, Buffered, Debug, Null, Recording) - Update DefaultLogFormatter with path (|) and tag (#) support - Simplify SinkSet (no text/structured split), LogManager.Dispatch - Update generator to emit DispatchInfo construction instead of LogEntry - Update Extensions.Logging bridge (BeginScope returns no-op disposable) - Update samples, benchmarks, and all tests (260 passing)
Save session state (workflow.md, handoff.md) for resumption. Phase 1 complete, 260 tests passing, Phase 2 next.
…paths LoggerContext becomes the central dispatch hub that fills in category, timestamp, thread info, and path from context state. LogManager gains GetLogger()/GetLogger<T>() factory methods with ConcurrentDictionary caching. PathNode provides linked-list hierarchical paths with version-based UTF-8 caching and volatile/interlocked thread safety.
… TimingOperation ILogger interface provides terminal methods (Trace through Critical) with default implementations, chain methods (When/Sampled/Tagged), and hierarchy support. LogManager.GetLogger() now returns ILogger via LoggerInstance wrapper. NullLogger singleton short-circuits all operations. LogScope and TimingOperation are struct-based ILogger wrappers for scoped paths and timed operations. Resolves ILogger naming conflicts with MEL in bridge and benchmark projects.
LogHandlerCore ref struct writes both UTF-8 text and structured JSON simultaneously during string interpolation. Per-level handler wrappers (LogTraceHandler through LogCriticalHandler) short-circuit at construction when level is disabled. ILogger gains handler-based terminal method overloads using [InterpolatedStringHandlerArgument] for zero-overhead disabled path. JIT-specialized WriteJsonProperty avoids boxing for primitive types (int, double, bool, string, DateTime, Guid, etc.).
Static Log class provides [Conditional] Trace/Debug methods for zero-cost compile-time stripping. Generator now emits a static LoggerContext field per class, lazily initialized from LogManager.GetLogger().Context. Standard mode methods dispatch through LoggerContext (which fills timestamp, thread info, category, path) instead of LogManager directly. Generator also emits inline Utf8JsonWriter code for structured JSON output of message parameters. Explicit sink mode retains direct sink dispatch with full DispatchInfo.
Implements RegisterImplementationSourceOutput pipeline that intercepts ILogger terminal and chain method calls. Direct calls get caller info and FNV-1a event IDs baked in at compile time. Chain calls (When, Tagged, Sampled, RateLimited) use per-shape carrier types with ThreadStatic pooling. Roslyn upgraded to 4.12.0 for GetInterceptableLocation API. New files: - Interception/ChainAnalyzer.cs: walks syntax backward from terminals - Interception/InterceptorEmitter.cs: emits interceptor methods - Interception/CarrierEmitter.cs: emits per-chain-shape carrier classes - Interception/InterceptorCallSite.cs, InterceptorChain.cs: models - InterceptorTests.cs: 19 new tests 385 tests passing (252 runtime + 125 generator + 8 MEL bridge).
…anup - AddLogsmith(IServiceCollection) registers ILogger and ILogger<T> - LoggerOfT<T> open generic for typed DI resolution - Fix chain terminal interceptor: extract context before returning carrier - Remove dead StructuredPathEmitter (replaced by inline JSON in Phase 5) - 20 new integration tests (DI resolution, chain dispatch, caller info, [LogMessage]+ILogger coexistence, all log levels, tagged chains) 405 tests passing (267 runtime + 125 generator + 13 MEL bridge).
… tests Review remediation (A+B items): - Fix DefaultLogFormatter: compute dynamic span size instead of fixed 512 - Fix MEL bridge: heap fallback for messages >4096 UTF-8 bytes, no truncation - Implement LSMITH013: detect chain methods stored in variables, emit warning - Add runtime integration tests for Sampled/RateLimited chain interceptors - Add sink-throws-continue-to-next-sink test 410 tests passing (270 runtime + 127 generator + 13 MEL bridge).
DJGosnell
added a commit
that referenced
this pull request
Apr 5, 2026
* Add ILogger rework prototypes and design summary
Prototypes validating the new ILogger API approach:
- LoggerPrototype: InterpolatedStringHandler dual-buffer (text+JSON),
CallerArgumentExpression, typed JSON, short-circuit, string overloads
- RefStructTest: ref struct implementing IDisposable and custom interfaces,
allows ref struct generic constraint
- ConditionalTest: [Conditional] removes entire call site including
handler construction and side effects
- InterceptorConditionalTest: [Conditional] on interceptor does NOT work,
must be on original method; new .NET 10 interceptor API validated
- WhenInterceptorTest: When(false)->NullLogger->handler disabled flow
with real source generator interceptors
Design summary (prototype/summary.md) covers all decisions, the
Quarry-inspired carrier chain pattern, outstanding questions, and
deliberate exclusions.
* Address review findings in design summary
Resolved 18 issues from API design and systems engineering reviews:
Critical fixes:
- Remove phantom LogChain type; all chains use ILogger + generated carriers
- Scoped() only adds path segment, no persistent structured properties
- Full ILogger interface declaration with all user-callable methods
- Re-entrant logging returns NullLogger (carrier _inUse guard)
- Add Prototype Limitations section noting alloc gaps
Consistency fixes:
- Unified Dispatch via DispatchInfo ref struct
- Handler naming: LogInformationHandler (consistent with method name)
- Handler constructor overload for Exception parameter
- TimingOperation implements ILogger for timed+logging ergonomics
- GetJsonWritten() guarded with _jsonFinalized boolean
- PathSegment uses Volatile.Write/Interlocked (ARM64 safe, behind flag)
- When() allowed at any chain position (LSMITH012 dropped)
- RateLimited(N) added as chain method
- Supplemental compilation note corrected
- Disabled-path cost table: Direct ~3-5ns, Chained ~8-15ns, Static 0ns
- Generator chain detection must use SemanticModel (requirement noted)
- sink.AsLogger() removed from migration path
- Resolved Decisions table added for traceability
* Address round 2 review findings in design summary
16 issues resolved from second review cycle:
Critical:
- Re-entrancy code fixed to return NullLogger (was contradicting decision)
- Chain methods (Sampled/RateLimited/Tagged) get => this defaults
- [Conditional]+ref validated, noted in doc
Medium:
- Scoped() is now extension method returning concrete LogScope struct
(plain string param, no handler, no boxing)
- Static tier Dispatch calls updated to use DispatchInfo
- PathSegment TOCTOU race documented as accepted trade-off
- Multi-tag deferred; single Tagged(string) only for v2
Low:
- logger.Info() -> logger.Information() naming fix
- NullLogger gets static sentinel LoggerContext
- GetJsonWritten guard marked as production-only fix
- Sampling counter uses (uint) cast for overflow correctness
- Scoped() no longer uses handler (plain string avoids silent discard)
- LogScope/TimingOperation added to Core Types
- DispatchInfo sync consumption documented in limitations
- TimingOperation noted on ILogger interface as extension method
* Address round 3 review findings in design summary
9 issues resolved from third review cycle:
Must-fix:
- Terminal methods now show full default implementations (Debug pattern,
all levels identical). Carriers/NullLogger only need Context property.
- Static Log class methods get [InterpolatedStringHandlerArgument("logger")]
on handler parameters (required for compiler to pass ILogger to handler ctor)
- Carrier _inUse released on handler.IsEnabled early-return path (prevents
permanent carrier lock on dynamic level change race)
Minor:
- NullLogger fully specified: sentinel context, no-op PathSegment, CreateChild
returns Instance, Scoped() creates no-op LogScope
- TimingOperation method surface declared: Complete/Fail/TimeStep/Dispose
- Stale "multi-tag support" changed to "single-tag filtering"
- PathSegment/Segment naming delegation explained
- Chained disabled cost notes scaling with interpolation hole count
- CreateChild gets default implementation (=> NullLogger.Instance)
* Resolve all outstanding design questions
All 5 remaining questions now have final decisions:
- Handler types: Separate per level (LogDebugHandler, etc.) wrapping
shared LogHandlerCore. Clean signatures, no hidden params.
- DPanic: Dedicated method on ILogger, dispatches at Error level,
throws InvalidOperationException in DEBUG builds only.
- Event IDs: FNV-1a hash of concatenated template literal parts.
Same template = same ID. Variable renames don't affect ID.
- Config inheritance: Children share parent SinkSet by reference.
MinimumLevel overridable per child. Parent reconfig affects children.
- Chain detection: SemanticModel resolution required at each chain
step to confirm ILogger receiver. Syntax name matching alone rejected.
No outstanding questions remain. Design summary is implementation-ready.
* Replace LogEntry with DispatchInfo ref struct and unify sink interface
Consolidate the dispatch infrastructure as the foundation for the ILogger
rework. DispatchInfo carries all log data (text, JSON, path, tag, caller
info) through a single ref struct. ILogSink.Write now takes in DispatchInfo
instead of separate LogEntry + ReadOnlySpan<byte> parameters.
- Add DispatchInfo ref struct with Utf8Message/Utf8Json/Utf8Path spans
- Unify ILogSink and IStructuredLogSink into single Write(in DispatchInfo)
- Remove LogEntry, LogScope (AsyncLocal), IStructuredLogSink, WriteProperties
- Update all sinks (Console, File, Stream, Buffered, Debug, Null, Recording)
- Update DefaultLogFormatter with path (|) and tag (#) support
- Simplify SinkSet (no text/structured split), LogManager.Dispatch
- Update generator to emit DispatchInfo construction instead of LogEntry
- Update Extensions.Logging bridge (BeginScope returns no-op disposable)
- Update samples, benchmarks, and all tests (260 passing)
* Suspend workflow after Phase 1 completion
Save session state (workflow.md, handoff.md) for resumption.
Phase 1 complete, 260 tests passing, Phase 2 next.
* Add LoggerContext as central dispatch hub with PathNode hierarchical paths
LoggerContext becomes the central dispatch hub that fills in category,
timestamp, thread info, and path from context state. LogManager gains
GetLogger()/GetLogger<T>() factory methods with ConcurrentDictionary
caching. PathNode provides linked-list hierarchical paths with
version-based UTF-8 caching and volatile/interlocked thread safety.
* Add ILogger interface with default methods, NullLogger, LogScope, and TimingOperation
ILogger interface provides terminal methods (Trace through Critical) with
default implementations, chain methods (When/Sampled/Tagged), and hierarchy
support. LogManager.GetLogger() now returns ILogger via LoggerInstance
wrapper. NullLogger singleton short-circuits all operations. LogScope and
TimingOperation are struct-based ILogger wrappers for scoped paths and
timed operations. Resolves ILogger naming conflicts with MEL in bridge
and benchmark projects.
* Add dual-buffer InterpolatedStringHandler infrastructure
LogHandlerCore ref struct writes both UTF-8 text and structured JSON
simultaneously during string interpolation. Per-level handler wrappers
(LogTraceHandler through LogCriticalHandler) short-circuit at construction
when level is disabled. ILogger gains handler-based terminal method
overloads using [InterpolatedStringHandlerArgument] for zero-overhead
disabled path. JIT-specialized WriteJsonProperty avoids boxing for
primitive types (int, double, bool, string, DateTime, Guid, etc.).
* Add static Log class and adapt generator for LoggerContext dispatch
Static Log class provides [Conditional] Trace/Debug methods for zero-cost
compile-time stripping. Generator now emits a static LoggerContext field
per class, lazily initialized from LogManager.GetLogger().Context. Standard
mode methods dispatch through LoggerContext (which fills timestamp, thread
info, category, path) instead of LogManager directly. Generator also emits
inline Utf8JsonWriter code for structured JSON output of message parameters.
Explicit sink mode retains direct sink dispatch with full DispatchInfo.
* Update workflow and handoff for session 2 suspend
* Add Generator Stage 2 interceptors with chain carrier support
Implements RegisterImplementationSourceOutput pipeline that intercepts
ILogger terminal and chain method calls. Direct calls get caller info
and FNV-1a event IDs baked in at compile time. Chain calls (When,
Tagged, Sampled, RateLimited) use per-shape carrier types with
ThreadStatic pooling. Roslyn upgraded to 4.12.0 for
GetInterceptableLocation API.
New files:
- Interception/ChainAnalyzer.cs: walks syntax backward from terminals
- Interception/InterceptorEmitter.cs: emits interceptor methods
- Interception/CarrierEmitter.cs: emits per-chain-shape carrier classes
- Interception/InterceptorCallSite.cs, InterceptorChain.cs: models
- InterceptorTests.cs: 19 new tests
385 tests passing (252 runtime + 125 generator + 8 MEL bridge).
* Add DI integration, end-to-end tests, fix chain carrier dispatch, cleanup
- AddLogsmith(IServiceCollection) registers ILogger and ILogger<T>
- LoggerOfT<T> open generic for typed DI resolution
- Fix chain terminal interceptor: extract context before returning carrier
- Remove dead StructuredPathEmitter (replaced by inline JSON in Phase 5)
- 20 new integration tests (DI resolution, chain dispatch, caller info,
[LogMessage]+ILogger coexistence, all log levels, tagged chains)
405 tests passing (267 runtime + 125 generator + 13 MEL bridge).
* Address review findings: formatter bounds, MEL truncation, LSMITH013, tests
Review remediation (A+B items):
- Fix DefaultLogFormatter: compute dynamic span size instead of fixed 512
- Fix MEL bridge: heap fallback for messages >4096 UTF-8 bytes, no truncation
- Implement LSMITH013: detect chain methods stored in variables, emit warning
- Add runtime integration tests for Sampled/RateLimited chain interceptors
- Add sink-throws-continue-to-next-sink test
410 tests passing (270 runtime + 127 generator + 13 MEL bridge).
* Update workflow phase to REMEDIATE
* Remove session artifacts before merge
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
1 participant
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.
Summary
Adds a new ILogger API alongside the existing
[LogMessage]attribute pattern. Both share the same unified sink/dispatch system viaDispatchInfoandLoggerContext.Reason for Change
The existing
[LogMessage]attribute pattern excels at high-performance compile-time logging but requires boilerplate (partial methods, attributes). The new ILogger API provides an ergonomic alternative using C# interpolated strings with compile-time optimizations via source generator interceptors, while keeping the same zero-allocation goals.Impact
ILogger,ILogger<T>,LoggerContext,NullLogger,LogScope,TimingOperation, handlers, staticLogclassAddLogsmith(IServiceCollection)for native Logsmith DILogEntry→DispatchInfo,ILogSink.Writesignature change,IStructuredLogSinkremoved,LogScope(AsyncLocal) replaced with struct-based versionPlan items implemented as specified
Deviations from plan implemented
Logsmith.Extensions.Logging(not core) to avoid adding DI dependency to core packageStructuredPathEmitterreplaced by inline JSON inMethodEmitter(same outcome, different structure)DPanicmethod deferred to API surface gaps: DPanic, Log overloads, structured timing, Conditional tests #24Gaps in original plan implemented
LSMITH013diagnostic: detects broken chains (chain method results stored in variables)Migration Steps
LogEntrywithDispatchInfoin custom sinksILogSink.Write(in LogEntry, ReadOnlySpan<byte>)→Write(in DispatchInfo)ILogFormattermethods to acceptDispatchInfoIStructuredLogSinkwith unifiedILogSink(structured data viainfo.Utf8Json)LogScope.Push()/LogScope.Currentwithlogger.Scoped("segment")<InterceptorsNamespaces>$(InterceptorsNamespaces);Logsmith.Generated</InterceptorsNamespaces>for ProjectReference consumersPerformance Considerations
Security Considerations
Breaking Changes