From cee6a1abb948c9752a2b0c20e0a2936aab7652a9 Mon Sep 17 00:00:00 2001 From: Nicholas Blumhardt Date: Wed, 26 Jun 2019 10:51:27 +1000 Subject: [PATCH 1/4] Make DiagnosticContextCollector thread-safe --- .../Hosting/DiagnosticContextCollector.cs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/Serilog.Extensions.Hosting/Extensions/Hosting/DiagnosticContextCollector.cs b/src/Serilog.Extensions.Hosting/Extensions/Hosting/DiagnosticContextCollector.cs index dd92af7..11bab25 100644 --- a/src/Serilog.Extensions.Hosting/Extensions/Hosting/DiagnosticContextCollector.cs +++ b/src/Serilog.Extensions.Hosting/Extensions/Hosting/DiagnosticContextCollector.cs @@ -10,6 +10,7 @@ namespace Serilog.Extensions.Hosting public sealed class DiagnosticContextCollector : IDisposable { readonly AmbientDiagnosticContextCollector _ambientCollector; + readonly object _propertiesLock = new object(); List _properties = new List(); internal DiagnosticContextCollector(AmbientDiagnosticContextCollector ambientCollector) @@ -24,7 +25,11 @@ internal DiagnosticContextCollector(AmbientDiagnosticContextCollector ambientCol public void Add(LogEventProperty property) { if (property == null) throw new ArgumentNullException(nameof(property)); - _properties?.Add(property); + + lock (_propertiesLock) + { + _properties?.Add(property); + } } /// @@ -36,10 +41,13 @@ public void Add(LogEventProperty property) /// True if properties could be collected. public bool TryComplete(out List properties) { - properties = _properties; - _properties = null; - Dispose(); - return properties != null; + lock (_propertiesLock) + { + properties = _properties; + _properties = null; + Dispose(); + return properties != null; + } } /// From 3a5f0795c3639d6ea45341b61dce6079216a031a Mon Sep 17 00:00:00 2001 From: Nicholas Blumhardt Date: Wed, 26 Jun 2019 14:25:07 +1000 Subject: [PATCH 2/4] `IDiagnosticContext.Add()` -> `IDiagnosticContext.Set()`; more small tweaks --- .../AmbientDiagnosticContextCollector.cs | 16 +++++++++- .../Extensions/Hosting/DiagnosticContext.cs | 17 +++++----- .../Hosting/DiagnosticContextCollector.cs | 28 +++++++++++++---- .../IDiagnosticContext.cs | 5 +-- .../DiagnosticContextTests.cs | 31 +++++++++++++++---- 5 files changed, 74 insertions(+), 23 deletions(-) diff --git a/src/Serilog.Extensions.Hosting/Extensions/Hosting/AmbientDiagnosticContextCollector.cs b/src/Serilog.Extensions.Hosting/Extensions/Hosting/AmbientDiagnosticContextCollector.cs index acc1ccd..68010ee 100644 --- a/src/Serilog.Extensions.Hosting/Extensions/Hosting/AmbientDiagnosticContextCollector.cs +++ b/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 diff --git a/src/Serilog.Extensions.Hosting/Extensions/Hosting/DiagnosticContext.cs b/src/Serilog.Extensions.Hosting/Extensions/Hosting/DiagnosticContext.cs index 0e5e602..7c12c19 100644 --- a/src/Serilog.Extensions.Hosting/Extensions/Hosting/DiagnosticContext.cs +++ b/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 { /// - /// Implements an ambient/async-local diagnostic context. Consumers should - /// use in preference to this concrete type. + /// Implements an ambient diagnostic context using . /// - public class DiagnosticContext : IDiagnosticContext + /// Consumers should use to set context properties. + public sealed class DiagnosticContext : IDiagnosticContext { readonly ILogger _logger; @@ -34,17 +35,17 @@ public DiagnosticContext(ILogger logger) } /// - /// 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. /// - /// A collector that will receive events added in the current async context. + /// A collector that will receive properties added in the current diagnostic context. public DiagnosticContextCollector BeginCollection() { return AmbientDiagnosticContextCollector.Begin(); } - /// - public void Add(string propertyName, object value, bool destructureObjects = false) + /// + public void Set(string propertyName, object value, bool destructureObjects = false) { if (propertyName == null) throw new ArgumentNullException(nameof(propertyName)); @@ -52,7 +53,7 @@ public void Add(string propertyName, object value, bool destructureObjects = fal if (collector != null && (_logger ?? Log.Logger).BindProperty(propertyName, value, destructureObjects, out var property)) { - collector.Add(property); + collector.AddOrUpdate(property); } } } diff --git a/src/Serilog.Extensions.Hosting/Extensions/Hosting/DiagnosticContextCollector.cs b/src/Serilog.Extensions.Hosting/Extensions/Hosting/DiagnosticContextCollector.cs index 11bab25..5ebf216 100644 --- a/src/Serilog.Extensions.Hosting/Extensions/Hosting/DiagnosticContextCollector.cs +++ b/src/Serilog.Extensions.Hosting/Extensions/Hosting/DiagnosticContextCollector.cs @@ -9,26 +9,42 @@ namespace Serilog.Extensions.Hosting /// public sealed class DiagnosticContextCollector : IDisposable { - readonly AmbientDiagnosticContextCollector _ambientCollector; + readonly IDisposable _chainedDisposable; readonly object _propertiesLock = new object(); List _properties = new List(); - internal DiagnosticContextCollector(AmbientDiagnosticContextCollector ambientCollector) + /// + /// Construct a . + /// + /// An object that will be disposed to signal completion/disposal of + /// the collector. + public DiagnosticContextCollector(IDisposable chainedDisposable) { - _ambientCollector = ambientCollector ?? throw new ArgumentNullException(nameof(ambientCollector)); + _chainedDisposable = chainedDisposable ?? throw new ArgumentNullException(nameof(chainedDisposable)); } /// /// Add the property to the context. /// /// The property to add. - 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 properties) /// public void Dispose() { - _ambientCollector.Dispose(); + _chainedDisposable.Dispose(); } } } diff --git a/src/Serilog.Extensions.Hosting/IDiagnosticContext.cs b/src/Serilog.Extensions.Hosting/IDiagnosticContext.cs index 5b472c4..468fa84 100644 --- a/src/Serilog.Extensions.Hosting/IDiagnosticContext.cs +++ b/src/Serilog.Extensions.Hosting/IDiagnosticContext.cs @@ -20,12 +20,13 @@ namespace Serilog public interface IDiagnosticContext { /// - /// 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. /// /// The name of the property. Must be non-empty. /// The property value. /// 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. - void Add(string propertyName, object value, bool destructureObjects = false); + void Set(string propertyName, object value, bool destructureObjects = false); } } diff --git a/test/Serilog.Extensions.Hosting.Tests/DiagnosticContextTests.cs b/test/Serilog.Extensions.Hosting.Tests/DiagnosticContextTests.cs index 834492c..c32f9ea 100644 --- a/test/Serilog.Extensions.Hosting.Tests/DiagnosticContextTests.cs +++ b/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(prop.Value); + Assert.Equal(20, scalar.Value); + } } } From df57744a852f96eb1af3c70808d4effa64fd0b2e Mon Sep 17 00:00:00 2001 From: Nicholas Blumhardt Date: Wed, 26 Jun 2019 14:35:55 +1000 Subject: [PATCH 3/4] Probably not paranoid here to trade some actual performance for better worst-case execution time --- .../Hosting/DiagnosticContextCollector.cs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/Serilog.Extensions.Hosting/Extensions/Hosting/DiagnosticContextCollector.cs b/src/Serilog.Extensions.Hosting/Extensions/Hosting/DiagnosticContextCollector.cs index 5ebf216..d1aba65 100644 --- a/src/Serilog.Extensions.Hosting/Extensions/Hosting/DiagnosticContextCollector.cs +++ b/src/Serilog.Extensions.Hosting/Extensions/Hosting/DiagnosticContextCollector.cs @@ -11,7 +11,7 @@ public sealed class DiagnosticContextCollector : IDisposable { readonly IDisposable _chainedDisposable; readonly object _propertiesLock = new object(); - List _properties = new List(); + Dictionary _properties = new Dictionary(); /// /// Construct a . @@ -34,17 +34,7 @@ public void AddOrUpdate(LogEventProperty property) lock (_propertiesLock) { 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); + _properties[property.Name] = property; } } @@ -55,11 +45,11 @@ public void AddOrUpdate(LogEventProperty property) /// /// The collected properties, or null if no collection is active. /// True if properties could be collected. - public bool TryComplete(out List properties) + public bool TryComplete(out IEnumerable properties) { lock (_propertiesLock) { - properties = _properties; + properties = _properties?.Values; _properties = null; Dispose(); return properties != null; From 55bb2a572ffb503bd81e0749ba7c56f31e552719 Mon Sep 17 00:00:00 2001 From: Nicholas Blumhardt Date: Wed, 26 Jun 2019 14:51:32 +1000 Subject: [PATCH 4/4] README trivia --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f99b283..259ae32 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Serilog.Extensions.Hosting [![Build status](https://ci.appveyor.com/api/projects/status/ue4s7htjwj88fulh?svg=true)](https://ci.appveyor.com/project/serilog/serilog-extensions-hosting) [![NuGet Version](http://img.shields.io/nuget/v/Serilog.Extensions.Hosting.svg?style=flat)](https://www.nuget.org/packages/Serilog.Extensions.Hosting/) -Serilog logging for Microsoft.Extensions.Hosting . This package routes Microsoft.Extensions.Hosting log messages through Serilog, so you can get information about the framework's internal operations logged to the same Serilog sinks as your application events. +Serilog logging for _Microsoft.Extensions.Hosting_. This package routes framework log messages through Serilog, so you can get information about the framework's internal operations written to the same Serilog sinks as your application events. ### Instructions