IDiagnosticContext.Add() -> IDiagnosticContext.Set(); more small tweaks

Author: nblumhardt

src/Serilog.Extensions.Hosting/Extensions/Hosting/AmbientDiagnosticContextCollector.cs

@@ -1,4 +1,18 @@
-using System;
+// Copyright 2019 Serilog Contributors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+using System;
 using System.Threading;
 
 namespace Serilog.Extensions.Hosting

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

@@ -13,14 +13,15 @@
 // limitations under the License.
 
 using System;
+using System.Threading;
 
 namespace Serilog.Extensions.Hosting
 {
     /// <summary>
-    /// Implements an ambient/async-local diagnostic context. Consumers should
-    /// use <see cref="IDiagnosticContext"/> in preference to this concrete type.
+    /// Implements an ambient diagnostic context using <see cref="AsyncLocal{T}"/>.
     /// </summary>
-    public class DiagnosticContext : IDiagnosticContext
+    /// <remarks>Consumers should use <see cref="IDiagnosticContext"/> to set context properties.</remarks>
+    public sealed class DiagnosticContext : IDiagnosticContext
     {
         readonly ILogger _logger;
 
@@ -34,25 +35,25 @@ public DiagnosticContext(ILogger logger)
         }
 
         /// <summary>
-        /// Start collecting properties to associate with the current async context. This will replace
+        /// Start collecting properties to associate with the current diagnostic context. This will replace
         /// the active collector, if any.
         /// </summary>
-        /// <returns>A collector that will receive events added in the current async context.</returns>
+        /// <returns>A collector that will receive properties added in the current diagnostic context.</returns>
         public DiagnosticContextCollector BeginCollection()
         {
             return AmbientDiagnosticContextCollector.Begin();
         }
 
-        /// <inheritdoc cref="IDiagnosticContext.Add"/>
-        public void Add(string propertyName, object value, bool destructureObjects = false)
+        /// <inheritdoc cref="IDiagnosticContext.Set"/>
+        public void Set(string propertyName, object value, bool destructureObjects = false)
         {
             if (propertyName == null) throw new ArgumentNullException(nameof(propertyName));
 
             var collector = AmbientDiagnosticContextCollector.Current;
             if (collector != null && 
                 (_logger ?? Log.Logger).BindProperty(propertyName, value, destructureObjects, out var property))
             {
-                collector.Add(property);
+                collector.AddOrUpdate(property);
             }
         }
     }

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

@@ -9,26 +9,42 @@ namespace Serilog.Extensions.Hosting
     /// </summary>
     public sealed class DiagnosticContextCollector : IDisposable
     {
-        readonly AmbientDiagnosticContextCollector _ambientCollector;
+        readonly IDisposable _chainedDisposable;
         readonly object _propertiesLock = new object();
         List<LogEventProperty> _properties = new List<LogEventProperty>();
 
-        internal DiagnosticContextCollector(AmbientDiagnosticContextCollector ambientCollector)
+        /// <summary>
+        /// Construct a <see cref="DiagnosticContextCollector"/>.
+        /// </summary>
+        /// <param name="chainedDisposable">An object that will be disposed to signal completion/disposal of
+        /// the collector.</param>
+        public DiagnosticContextCollector(IDisposable chainedDisposable)
         {
-            _ambientCollector = ambientCollector ?? throw new ArgumentNullException(nameof(ambientCollector));
+            _chainedDisposable = chainedDisposable ?? throw new ArgumentNullException(nameof(chainedDisposable));
         }
 
         /// <summary>
         /// Add the property to the context.
         /// </summary>
         /// <param name="property">The property to add.</param>
-        public void Add(LogEventProperty property)
+        public void AddOrUpdate(LogEventProperty property)
         {
             if (property == null) throw new ArgumentNullException(nameof(property));
 
             lock (_propertiesLock)
             {
-                _properties?.Add(property);
+                if (_properties == null) return;
+
+                for (var i = 0; i < _properties.Count; ++i)
+                {
+                    if (_properties[i].Name == property.Name)
+                    {
+                        _properties[i] = property;
+                        return;
+                    }
+                }
+
+                _properties.Add(property);
             }
         }
 
@@ -53,7 +69,7 @@ public bool TryComplete(out List<LogEventProperty> properties)
         /// <inheritdoc/>
         public void Dispose()
         {
-            _ambientCollector.Dispose();
+            _chainedDisposable.Dispose();
         }
     }
 }

src/Serilog.Extensions.Hosting/IDiagnosticContext.cs

@@ -20,12 +20,13 @@ namespace Serilog
     public interface IDiagnosticContext
     {
         /// <summary>
-        /// Add the specified property to the request's diagnostic payload.
+        /// Set the specified property on the current diagnostic context. The property will be collected
+        /// and attached to the event emitted at the completion of the context.
         /// </summary>
         /// <param name="propertyName">The name of the property. Must be non-empty.</param>
         /// <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 Add(string propertyName, object value, bool destructureObjects = false); 
+        void Set(string propertyName, object value, bool destructureObjects = false); 
     }
 }

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

@@ -1,17 +1,21 @@
 using System;
+using System.Linq;
 using System.Threading.Tasks;
+using Serilog.Events;
 using Serilog.Extensions.Hosting.Tests.Support;
 using Xunit;
 
+// ReSharper disable PossibleNullReferenceException
+
 namespace Serilog.Extensions.Hosting.Tests
 {
     public class DiagnosticContextTests
     {
         [Fact]
-        public void AddIsSafeWhenNoContextIsActive()
+        public void SetIsSafeWhenNoContextIsActive()
         {
             var dc = new DiagnosticContext(Some.Logger());
-            dc.Add(Some.String("name"), Some.Int32());
+            dc.Set(Some.String("name"), Some.Int32());
         }
 
         [Fact]
@@ -20,19 +24,34 @@ public async Task PropertiesAreCollectedInAnActiveContext()
             var dc = new DiagnosticContext(Some.Logger());
 
             var collector = dc.BeginCollection();
-            dc.Add(Some.String("name"), Some.Int32());
+            dc.Set(Some.String("first"), Some.Int32());
             await Task.Delay(TimeSpan.FromMilliseconds(10));
-            dc.Add(Some.String("name"), Some.Int32());
+            dc.Set(Some.String("second"), Some.Int32());
 
             Assert.True(collector.TryComplete(out var properties));
-            Assert.Equal(2, properties.Count);
+            Assert.Equal(2, properties.Count());
 
             Assert.False(collector.TryComplete(out _));
 
             collector.Dispose();
 
-            dc.Add(Some.String("name"), Some.Int32());
+            dc.Set(Some.String("third"), Some.Int32());
             Assert.False(collector.TryComplete(out _));
         }
+
+        [Fact]
+        public void ExistingPropertiesCanBeUpdated()
+        {
+            var dc = new DiagnosticContext(Some.Logger());
+
+            var collector = dc.BeginCollection();
+            dc.Set("name", 10);
+            dc.Set("name", 20);
+
+            Assert.True(collector.TryComplete(out var properties));
+            var prop = Assert.Single(properties);
+            var scalar = Assert.IsType<ScalarValue>(prop.Value);
+            Assert.Equal(20, scalar.Value);
+        }
     }
 }