Merge pull request #58 from serilog/dev

5.0.0 Release

Author: nblumhardt

Build.ps1

@@ -11,7 +11,7 @@ if(Test-Path .\artifacts) {
 
 $branch = @{ $true = $env:APPVEYOR_REPO_BRANCH; $false = $(git symbolic-ref --short -q HEAD) }[$env:APPVEYOR_REPO_BRANCH -ne $NULL];
 $revision = @{ $true = "{0:00000}" -f [convert]::ToInt32("0" + $env:APPVEYOR_BUILD_NUMBER, 10); $false = "local" }[$env:APPVEYOR_BUILD_NUMBER -ne $NULL];
-$suffix = @{ $true = ""; $false = "$($branch.Substring(0, [math]::Min(10,$branch.Length)))-$revision"}[$branch -eq "master" -and $revision -ne "local"]
+$suffix = @{ $true = ""; $false = "$($branch.Substring(0, [math]::Min(10,$branch.Length)))-$revision"}[$branch -eq "main" -and $revision -ne "local"]
 
 echo "build: Version suffix is $suffix"
 

appveyor.yml

@@ -1,6 +1,6 @@
 version: '{build}'
 skip_tags: true
-image: Visual Studio 2019
+image: Visual Studio 2022
 test: off
 build_script:
 - ps: ./Build.ps1
@@ -15,11 +15,11 @@ deploy:
   api_key:
     secure: Q65rY+zaFWOhs8a9IVSaX4Go5HNcIlEXjEFWMB83Y325WE9aXzi0xzDDc0/fJDzk
   on:
-    branch: /^(master|dev)$/
+    branch: /^(main|dev)$/
 - provider: GitHub
   auth_token:
     secure: p4LpVhBKxGS5WqucHxFQ5c7C8cP74kbNB0Z8k9Oxx/PMaDQ1+ibmoexNqVU5ZlmX
   artifact: /Serilog.*\.nupkg/
   tag: v$(appveyor_build_version)
   on:
-    branch: master
+    branch: main

global.json

@@ -1,7 +1,7 @@
 {
   "sdk": {
     "allowPrerelease": false,
-    "version": "5.0.102",
+    "version": "6.0.300",
     "rollForward": "latestFeature"
   }
 }

src/Serilog.Extensions.Hosting/Extensions/Hosting/DiagnosticContext.cs

@@ -56,5 +56,12 @@ public void Set(string propertyName, object value, bool destructureObjects = fal
                 collector.AddOrUpdate(property);
             }
         }
+
+        /// <inheritdoc cref="IDiagnosticContext.SetException"/>
+        public void SetException(Exception exception)
+        {
+            var collector = AmbientDiagnosticContextCollector.Current;
+            collector?.SetException(exception);
+        }
     }
 }

src/Serilog.Extensions.Hosting/Extensions/Hosting/DiagnosticContextCollector.cs

@@ -11,6 +11,7 @@ public sealed class DiagnosticContextCollector : IDisposable
     {
         readonly IDisposable _chainedDisposable;
         readonly object _propertiesLock = new object();
+        Exception _exception;
         Dictionary<string, LogEventProperty> _properties = new Dictionary<string, LogEventProperty>();
 
         /// <summary>
@@ -38,19 +39,60 @@ public void AddOrUpdate(LogEventProperty property)
             }
         }
 
+        /// <summary>
+        /// Set the exception associated with the current diagnostic context.
+        /// </summary>
+        /// <example>
+        /// Passing an exception to the diagnostic context is useful when unhandled exceptions are handled before reaching Serilog's
+        /// RequestLoggingMiddleware. One example is using https://www.nuget.org/packages/Hellang.Middleware.ProblemDetails to transform
+        /// exceptions to ProblemDetails responses.
+        /// </example>
+        /// <remarks>
+        /// If an unhandled exception reaches Serilog's RequestLoggingMiddleware, then the unhandled exception takes precedence.<br/>
+        /// If <c>null</c> is given, it clears any previously assigned exception.
+        /// </remarks>
+        /// <param name="exception">The exception to log.</param>
+        public void SetException(Exception exception)
+        {
+            lock (_propertiesLock)
+            {
+                if (_properties == null) return;
+                _exception = exception;
+            }
+        }
+
         /// <summary>
         /// Complete the context and retrieve the properties added to it, if any. This will
         /// stop collection and remove the collector from the original execution context and
         /// any of its children.
         /// </summary>
         /// <param name="properties">The collected properties, or null if no collection is active.</param>
         /// <returns>True if properties could be collected.</returns>
+        /// <seealso cref="IDiagnosticContext.Set"/>
+        [Obsolete("Replaced by TryComplete(out IEnumerable<LogEventProperty> properties, out Exception exception).")]
         public bool TryComplete(out IEnumerable<LogEventProperty> properties)
+        {
+            return TryComplete(out properties, out _);
+        }
+
+        /// <summary>
+        /// Complete the context and retrieve the properties and exception added to it, if any. This will
+        /// stop collection and remove the collector from the original execution context and
+        /// any of its children.
+        /// </summary>
+        /// <param name="properties">The collected properties, or null if no collection is active.</param>
+        /// <param name="exception">The collected exception, or null if none has been collected or if no collection is active.</param>
+        /// <returns>True if properties could be collected.</returns>
+        /// <seealso cref="IDiagnosticContext.Set"/>
+        /// <seealso cref="Serilog.IDiagnosticContext.SetException"/>
+        public bool TryComplete(out IEnumerable<LogEventProperty> properties, out Exception exception)
         {
             lock (_propertiesLock)
             {
                 properties = _properties?.Values;
+                exception = _exception;
                 _properties = null;
+                _exception = null;
                 Dispose();
                 return properties != null;
             }

src/Serilog.Extensions.Hosting/Extensions/Hosting/ReloadableLogger.cs

@@ -368,9 +368,13 @@ public bool BindProperty(string propertyName, object value, bool destructureObje
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         (ILogger, bool) UpdateForCaller(ILogger root, ILogger cached, IReloadableLogger caller, out ILogger newRoot, out ILogger newCached, out bool frozen)
         {
+            // Synchronization on `_sync` is not required in this method; it will be called without a lock
+            // if `_frozen` and under a lock if not.
+            
             if (_frozen)
             {
-                // If we're frozen, then the caller hasn't observed this yet and should update.
+                // If we're frozen, then the caller hasn't observed this yet and should update. We could optimize a little here
+                // and only signal an update if the cached logger is stale (as per the next condition below).
                 newRoot = _logger;
                 newCached = caller.ReloadLogger();
                 frozen = true;
@@ -384,7 +388,7 @@ public bool BindProperty(string propertyName, object value, bool destructureObje
                 frozen = false;
                 return (cached, false);
             }
-            
+        
             newRoot = _logger;
             newCached = caller.ReloadLogger();
             frozen = false;

src/Serilog.Extensions.Hosting/IDiagnosticContext.cs

@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+using System;
+
 namespace Serilog
 {
     /// <summary>
@@ -27,6 +29,17 @@ public interface IDiagnosticContext
         /// <param name="value">The property value.</param>
         /// <param name="destructureObjects">If true, the value will be serialized as structured
         /// data if possible; if false, the object will be recorded as a scalar or simple array.</param>
-        void Set(string propertyName, object value, bool destructureObjects = false); 
+        void Set(string propertyName, object value, bool destructureObjects = false);
+
+        /// <summary>
+        /// Set the specified exception on the current diagnostic context.
+        /// </summary>
+        /// <remarks>
+        /// This method is useful when unhandled exceptions do not reach <c>Serilog.AspNetCore.RequestLoggingMiddleware</c>,
+        /// such as when using <a href="https://www.nuget.org/packages/Hellang.Middleware.ProblemDetails">Hellang.Middleware.ProblemDetails</a>
+        /// to transform exceptions to ProblemDetails responses.
+        /// </remarks>
+        /// <param name="exception">The exception to log. If <c>null</c> is given, it clears any previously assigned exception.</param>
+        void SetException(Exception exception);
     }
 }

src/Serilog.Extensions.Hosting/Serilog.Extensions.Hosting.csproj

@@ -2,7 +2,7 @@
 
   <PropertyGroup>
     <Description>Serilog support for .NET Core logging in hosted services</Description>
-    <VersionPrefix>4.2.0</VersionPrefix>
+    <VersionPrefix>5.0.0</VersionPrefix>
     <Authors>Microsoft;Serilog Contributors</Authors>
     <TargetFrameworks>netstandard2.0;netstandard2.1</TargetFrameworks>
     <LangVersion>8</LangVersion>

src/Serilog.Extensions.Hosting/SerilogHostBuilderExtensions.cs

@@ -80,7 +80,13 @@ public static IHostBuilder UseSerilog(
                     collection.AddSingleton<ILoggerFactory>(services => new SerilogLoggerFactory(logger, dispose));
                 }
 
-                ConfigureServices(collection, logger);
+                if (logger != null)
+                {
+                    // This won't (and shouldn't) take ownership of the logger. 
+                    collection.AddSingleton(logger);
+                }
+                bool useRegisteredLogger = logger != null;
+                ConfigureDiagnosticContext(collection, useRegisteredLogger);
             });
 
             return builder;
@@ -221,32 +227,26 @@ public static IHostBuilder UseSerilog(
 
                     return factory;
                 });
-                
-                // Null is passed here because we've already (lazily) registered `ILogger`
-                ConfigureServices(collection, null);
+
+                ConfigureDiagnosticContext(collection, preserveStaticLogger);
             });
 
             return builder;
         }
 
-        static void ConfigureServices(IServiceCollection collection, ILogger logger)
+        static void ConfigureDiagnosticContext(IServiceCollection collection, bool useRegisteredLogger)
         {
             if (collection == null) throw new ArgumentNullException(nameof(collection));
 
-            if (logger != null)
-            {
-                // This won't (and shouldn't) take ownership of the logger. 
-                collection.AddSingleton(logger);
-            }
-
-            // Registered to provide two services...
-            var diagnosticContext = new DiagnosticContext(logger);
-
+            // Registered to provide two services...            
             // Consumed by e.g. middleware
-            collection.AddSingleton(diagnosticContext);
-
+            collection.AddSingleton(services =>
+            {
+                ILogger logger = useRegisteredLogger ? services.GetRequiredService<RegisteredLogger>().Logger : null;
+                return new DiagnosticContext(logger);
+            });
             // Consumed by user code
-            collection.AddSingleton<IDiagnosticContext>(diagnosticContext);
+            collection.AddSingleton<IDiagnosticContext>(services => services.GetRequiredService<DiagnosticContext>());
         }
     }
 }

test/Serilog.Extensions.Hosting.Tests/DiagnosticContextTests.cs

@@ -18,6 +18,13 @@ public void SetIsSafeWhenNoContextIsActive()
             dc.Set(Some.String("name"), Some.Int32());
         }
 
+        [Fact]
+        public void SetExceptionIsSafeWhenNoContextIsActive()
+        {
+            var dc = new DiagnosticContext(Some.Logger());
+            dc.SetException(new Exception("test"));
+        }
+
         [Fact]
         public async Task PropertiesAreCollectedInAnActiveContext()
         {
@@ -39,6 +46,48 @@ public async Task PropertiesAreCollectedInAnActiveContext()
             Assert.False(collector.TryComplete(out _));
         }
 
+        [Fact]
+        public void ExceptionIsCollectedInAnActiveContext()
+        {
+            var dc = new DiagnosticContext(Some.Logger());
+            var collector = dc.BeginCollection();
+
+            var setException = new Exception("before collect");
+            dc.SetException(setException);
+
+            Assert.True(collector.TryComplete(out _, out var collectedException));
+            Assert.Same(setException, collectedException);
+        }
+
+        [Fact]
+        public void ExceptionIsNotCollectedAfterTryComplete()
+        {
+            var dc = new DiagnosticContext(Some.Logger());
+            var collector = dc.BeginCollection();
+            collector.TryComplete(out _, out _);
+            dc.SetException(new Exception(Some.String("after collect")));
+
+            var tryComplete2 = collector.TryComplete(out _, out var collectedException2);
+
+            Assert.False(tryComplete2);
+            Assert.Null(collectedException2);
+        }
+
+        [Fact]
+        public void ExceptionIsNotCollectedAfterDispose()
+        {
+            var dc = new DiagnosticContext(Some.Logger());
+            var collector = dc.BeginCollection();
+            collector.Dispose();
+
+            dc.SetException(new Exception("after dispose"));
+
+            var tryComplete = collector.TryComplete(out _, out var collectedException);
+
+            Assert.True(tryComplete);
+            Assert.Null(collectedException);
+        }
+
         [Fact]
         public void ExistingPropertiesCanBeUpdated()
         {
@@ -53,5 +102,18 @@ public void ExistingPropertiesCanBeUpdated()
             var scalar = Assert.IsType<ScalarValue>(prop.Value);
             Assert.Equal(20, scalar.Value);
         }
+
+        [Fact]
+        public void ExistingExceptionCanBeUpdated()
+        {
+            var dc = new DiagnosticContext(Some.Logger());
+            var collector = dc.BeginCollection();
+
+            dc.SetException(new Exception("ex1"));
+            dc.SetException(new Exception("ex2"));
+
+            Assert.True(collector.TryComplete(out _, out var collectedException));
+            Assert.Equal("ex2", collectedException.Message);
+        }
     }
 }