Skip to content

Interceptor for AmbientLoggerFactory - #91

Merged
nickrandolph merged 5 commits into
masterfrom
dev/nr/loggerinterceptor
Apr 9, 2026
Merged

Interceptor for AmbientLoggerFactory#91
nickrandolph merged 5 commits into
masterfrom
dev/nr/loggerinterceptor

Conversation

@nickrandolph

Copy link
Copy Markdown
Contributor

Add the ability to intercept when the AmbientLoggerFactory is set by an app

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a new extension point to Uno.Extensions.LogExtensionPoint allowing hosts to intercept and potentially wrap/replace the AmbientLoggerFactory whenever an app sets it, along with a new MSTest suite validating the expected behaviors.

Changes:

  • Added a configurable RegisterFactoryInterceptor(...) hook invoked when AmbientLoggerFactory is set.
  • Updated the AmbientLoggerFactory setter to apply the interceptor result before storing.
  • Added LogExtensionPointTests and updated the tests project to reference the singleton logging extensions project.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/Uno.Core.Tests/Uno.Core.Tests.csproj Adds project reference needed to test the new singleton logging extension behavior.
src/Uno.Core.Tests/LogExtensionPointTests.cs Introduces new tests covering interceptor registration and AmbientLoggerFactory behavior.
src/Uno.Core.Extensions.Logging.Singleton/LogExtensionPoint.cs Adds interceptor API and wires it into the ambient factory setter.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Uno.Core.Extensions.Logging.Singleton/LogExtensionPoint.cs
Comment thread src/Uno.Core.Extensions.Logging.Singleton/LogExtensionPoint.cs Outdated
Comment thread src/Uno.Core.Tests/LogExtensionPointTests.cs Outdated
@nickrandolph

Copy link
Copy Markdown
Contributor Author

@copilot apply changes based on the comments in this thread

Copilot AI commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

@copilot apply changes based on the comments in this thread

Applied all changes from the review threads in commit 04bd533:

  • Thread 1 (multiple interceptor invocations): switched the setter from Transactional.Update to Volatile.Read/Volatile.Write, so the interceptor runs exactly once per assignment.
  • Thread 2 (ambiguous contract): changed the interceptor delegate to Func<ILoggerFactory?, ILoggerFactory, ILoggerFactory>? — both proposed and the return type are non-nullable, enforced at compile time.
  • Thread 3 (inaccurate test): replaced RegisterFactoryInterceptor_InterceptorCanReturnNull_FallsBackToGetFactory with RegisterFactoryInterceptor_InterceptorMustReturnNonNull (validates the non-null contract) and added AmbientLoggerFactory_Set_InterceptorRunsOnceEvenUnderContention to validate the single-invocation behavior.

Comment thread src/Uno.Core.Extensions.Logging.Singleton/LogExtensionPoint.cs Outdated
…eptor and factory

Agent-Logs-Url: https://github.com/unoplatform/Uno.Core/sessions/40c9fb54-ff2a-43e8-9838-f09b4e5333ca

Co-authored-by: nickrandolph <1614057+nickrandolph@users.noreply.github.com>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ nickrandolph
❌ Copilot
You have signed the CLA already but the status is still pending? Let us recheck it.

@nickrandolph
nickrandolph merged commit 5df33dd into master Apr 9, 2026
4 of 5 checks passed
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.

5 participants