From 5314e4410444c379afc6273269fa2b4d93a9febc Mon Sep 17 00:00:00 2001 From: Laurynas Ruskys Date: Wed, 27 Sep 2023 17:32:05 +0300 Subject: [PATCH 01/12] Make logging scope key configurable --- src/Correlate.Core/CorrelationManager.cs | 15 +++++++++++---- .../CorrelationManagerOptions.cs | 12 ++++++++++++ .../Extensions/LoggerExtensions.cs | 10 ++++++---- src/Correlate.Core/RootActivity.cs | 7 +++++-- .../IServiceCollectionExtensions.cs | 18 ++++++++++++++++++ .../CorrelationManagerTests.cs | 10 ++++++++-- 6 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 src/Correlate.Core/CorrelationManagerOptions.cs diff --git a/src/Correlate.Core/CorrelationManager.cs b/src/Correlate.Core/CorrelationManager.cs index 8dfa3f8..518651d 100644 --- a/src/Correlate.Core/CorrelationManager.cs +++ b/src/Correlate.Core/CorrelationManager.cs @@ -1,5 +1,6 @@ using System.Diagnostics; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; namespace Correlate; @@ -13,6 +14,7 @@ public class CorrelationManager : IAsyncCorrelationManager, ICorrelationManager, private readonly ICorrelationIdFactory _correlationIdFactory; private readonly DiagnosticListener? _diagnosticListener; private readonly ILogger _logger; + private readonly CorrelationManagerOptions _options; /// /// Initializes a new instance of the class. @@ -21,18 +23,21 @@ public class CorrelationManager : IAsyncCorrelationManager, ICorrelationManager, /// The correlation id factory used to generate a new correlation id per context. /// The correlation context accessor. /// The logger. + /// The configuration options. public CorrelationManager ( ICorrelationContextFactory correlationContextFactory, ICorrelationIdFactory correlationIdFactory, ICorrelationContextAccessor correlationContextAccessor, - ILogger logger + ILogger logger, + IOptions options ) { _correlationContextFactory = correlationContextFactory ?? throw new ArgumentNullException(nameof(correlationContextFactory)); _correlationIdFactory = correlationIdFactory ?? throw new ArgumentNullException(nameof(correlationIdFactory)); _correlationContextAccessor = correlationContextAccessor ?? throw new ArgumentNullException(nameof(correlationContextAccessor)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _options = options.Value; } /// @@ -43,14 +48,16 @@ ILogger logger /// The correlation context accessor. /// The logger. /// The diagnostics listener to run activities on. + /// The configuration options. public CorrelationManager ( ICorrelationContextFactory correlationContextFactory, ICorrelationIdFactory correlationIdFactory, ICorrelationContextAccessor correlationContextAccessor, ILogger logger, - DiagnosticListener diagnosticListener - ) : this(correlationContextFactory, correlationIdFactory, correlationContextAccessor, logger) + DiagnosticListener diagnosticListener, + IOptions options + ) : this(correlationContextFactory, correlationIdFactory, correlationContextAccessor, logger, options) { _diagnosticListener = diagnosticListener ?? throw new ArgumentNullException(nameof(diagnosticListener)); } @@ -61,7 +68,7 @@ DiagnosticListener diagnosticListener /// The correlated activity. public IActivity CreateActivity() { - return new RootActivity(_correlationContextFactory, _logger, _diagnosticListener); + return new RootActivity(_correlationContextFactory, _logger, _diagnosticListener, _options); } /// diff --git a/src/Correlate.Core/CorrelationManagerOptions.cs b/src/Correlate.Core/CorrelationManagerOptions.cs new file mode 100644 index 0000000..eefea83 --- /dev/null +++ b/src/Correlate.Core/CorrelationManagerOptions.cs @@ -0,0 +1,12 @@ +namespace Correlate; + +/// +/// The options class for configuring . +/// +public class CorrelationManagerOptions +{ + /// + /// The scope key that will be used for adding correlation ID to log context. Default is CorrelationId. + /// + public string LoggingScopeKey { get; set; } = CorrelateConstants.CorrelationIdKey; +} \ No newline at end of file diff --git a/src/Correlate.Core/Extensions/LoggerExtensions.cs b/src/Correlate.Core/Extensions/LoggerExtensions.cs index e11631b..66e2443 100644 --- a/src/Correlate.Core/Extensions/LoggerExtensions.cs +++ b/src/Correlate.Core/Extensions/LoggerExtensions.cs @@ -5,17 +5,19 @@ namespace Correlate.Extensions; internal static class LoggerExtensions { - public static IDisposable? BeginCorrelatedScope(this ILogger logger, string correlationId) + public static IDisposable? BeginCorrelatedScope(this ILogger logger, string scopeKey, string correlationId) { - return logger.BeginScope(new CorrelatedLogScope(correlationId)); + return logger.BeginScope(new CorrelatedLogScope(scopeKey, correlationId)); } private sealed class CorrelatedLogScope : IReadOnlyList> { + private readonly string _scopeKey; private readonly string _correlationId; - public CorrelatedLogScope(string correlationId) + public CorrelatedLogScope(string scopeKey, string correlationId) { + _scopeKey = scopeKey; _correlationId = correlationId; } @@ -41,7 +43,7 @@ public KeyValuePair this[int index] { if (index == 0) { - return new KeyValuePair(CorrelateConstants.CorrelationIdKey, _correlationId); + return new KeyValuePair(_scopeKey, _correlationId); } throw new ArgumentOutOfRangeException(nameof(index)); diff --git a/src/Correlate.Core/RootActivity.cs b/src/Correlate.Core/RootActivity.cs index 6eb4d0a..f53f41b 100644 --- a/src/Correlate.Core/RootActivity.cs +++ b/src/Correlate.Core/RootActivity.cs @@ -8,6 +8,7 @@ internal class RootActivity : IActivity { private readonly ICorrelationContextFactory _correlationContextFactory; private readonly DiagnosticListener? _diagnosticListener; + private readonly CorrelationManagerOptions _options; private readonly ILogger _logger; private IDisposable? _logScope; @@ -15,11 +16,13 @@ public RootActivity ( ICorrelationContextFactory correlationContextFactory, ILogger logger, - DiagnosticListener? diagnosticListener) + DiagnosticListener? diagnosticListener, + CorrelationManagerOptions options) { _correlationContextFactory = correlationContextFactory ?? throw new ArgumentNullException(nameof(correlationContextFactory)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _diagnosticListener = diagnosticListener; + _options = options; } /// @@ -49,7 +52,7 @@ public CorrelationContext Start(string correlationId) if (isLoggingEnabled) { - _logScope = _logger.BeginCorrelatedScope(correlationId); + _logScope = _logger.BeginCorrelatedScope(_options.LoggingScopeKey, correlationId); } return context; diff --git a/src/Correlate.DependencyInjection/IServiceCollectionExtensions.cs b/src/Correlate.DependencyInjection/IServiceCollectionExtensions.cs index a5afdbe..c40bb8a 100644 --- a/src/Correlate.DependencyInjection/IServiceCollectionExtensions.cs +++ b/src/Correlate.DependencyInjection/IServiceCollectionExtensions.cs @@ -9,6 +9,24 @@ namespace Correlate.DependencyInjection; // ReSharper disable once InconsistentNaming public static class IServiceCollectionExtensions { + /// + /// Adds services required for using correlation. + /// + /// The to add the services to. + /// The so that additional calls can be chained. + /// The callback to customize defaults. + /// + public static IServiceCollection AddCorrelate(this IServiceCollection services, Action configure) + { + services + .AddOptions() + .Configure(configure); + + services.AddCorrelate(); + + return services; + } + /// /// Adds services required for using correlation. /// diff --git a/test/Correlate.Core.Tests/CorrelationManagerTests.cs b/test/Correlate.Core.Tests/CorrelationManagerTests.cs index 436a48d..dc28cc6 100644 --- a/test/Correlate.Core.Tests/CorrelationManagerTests.cs +++ b/test/Correlate.Core.Tests/CorrelationManagerTests.cs @@ -1,7 +1,9 @@ using System.Collections; +using System.Diagnostics; using Correlate.Testing; using Correlate.Testing.TestCases; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using Serilog; using Serilog.Core; using Serilog.Extensions.Logging; @@ -15,6 +17,7 @@ public class CorrelationManagerTests : IDisposable private readonly CorrelationContextAccessor _correlationContextAccessor; private readonly ICorrelationIdFactory _correlationIdFactoryMock; private readonly ILogger _logger; + private readonly IOptions _options; private readonly SerilogLoggerProvider _logProvider; private readonly CorrelationManager _sut; @@ -33,12 +36,14 @@ protected CorrelationManagerTests() _logProvider = new SerilogLoggerProvider(serilogLogger); _logger = new TestLogger(_logProvider.CreateLogger(nameof(CorrelationManager))); + _options = Options.Create(new CorrelationManagerOptions()); _sut = new CorrelationManager( new CorrelationContextFactory(_correlationContextAccessor), _correlationIdFactoryMock, _correlationContextAccessor, - _logger + _logger, + _options ); } @@ -674,7 +679,8 @@ public static IEnumerable NullArgumentTestCases() Substitute.For(), Substitute.For(), Substitute.For(), - Substitute.For>() + Substitute.For>(), + Substitute.For>() ); static Task CorrelatedTask() From 476790d108163c9f4936a94fa4b03188b53acab3 Mon Sep 17 00:00:00 2001 From: Laurynas Ruskys Date: Wed, 27 Sep 2023 17:32:17 +0300 Subject: [PATCH 02/12] Fix documentation --- .../IServiceCollectionExtensions.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Correlate.DependencyInjection/IServiceCollectionExtensions.cs b/src/Correlate.DependencyInjection/IServiceCollectionExtensions.cs index c40bb8a..b1dc648 100644 --- a/src/Correlate.DependencyInjection/IServiceCollectionExtensions.cs +++ b/src/Correlate.DependencyInjection/IServiceCollectionExtensions.cs @@ -13,9 +13,8 @@ public static class IServiceCollectionExtensions /// Adds services required for using correlation. /// /// The to add the services to. - /// The so that additional calls can be chained. /// The callback to customize defaults. - /// + /// The so that additional calls can be chained. public static IServiceCollection AddCorrelate(this IServiceCollection services, Action configure) { services From cd29fea75d35c4a65773dd58b809474083d71e83 Mon Sep 17 00:00:00 2001 From: Laurynas Ruskys Date: Wed, 27 Sep 2023 17:32:24 +0300 Subject: [PATCH 03/12] Add a comment to deconfuse what that test is validating --- .../AspNetCore/CorrelateFeatureTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Correlate.AspNetCore.Tests/AspNetCore/CorrelateFeatureTests.cs b/test/Correlate.AspNetCore.Tests/AspNetCore/CorrelateFeatureTests.cs index eaa15a9..7b72de7 100644 --- a/test/Correlate.AspNetCore.Tests/AspNetCore/CorrelateFeatureTests.cs +++ b/test/Correlate.AspNetCore.Tests/AspNetCore/CorrelateFeatureTests.cs @@ -231,6 +231,7 @@ public async Task When_correlating_has_started_it_should_create_logScope_with_co .ContainSingle(le => le.MessageTemplate.Text.StartsWith("Setting response header")) .Which.Properties .Should() + // this tests the {CorrelationId} from log message template in CorrelateFeature.LogRequestHeaderFound, not the one from log scope added by IActivityFactory.CreateActivity .ContainSingle(p => p.Key == expectedLogProperty) .Which.Value .Should() From dfcacbcbb30e7d98fafe6a75c187a30930b9b3cf Mon Sep 17 00:00:00 2001 From: Laurynas Ruskys Date: Wed, 27 Sep 2023 17:32:31 +0300 Subject: [PATCH 04/12] Allow setting LoggingScopeKey in all use cases --- .../AspNetCore/CorrelateOptions.cs | 2 +- .../ServiceCollectionExtensions.cs | 15 +++++++++++++-- src/Correlate.Core/Http/CorrelateClientOptions.cs | 2 +- .../IHttpClientBuilderExtensions.cs | 8 ++++++++ 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/Correlate.AspNetCore/AspNetCore/CorrelateOptions.cs b/src/Correlate.AspNetCore/AspNetCore/CorrelateOptions.cs index 43336c3..3c09a48 100644 --- a/src/Correlate.AspNetCore/AspNetCore/CorrelateOptions.cs +++ b/src/Correlate.AspNetCore/AspNetCore/CorrelateOptions.cs @@ -5,7 +5,7 @@ namespace Correlate.AspNetCore; /// /// Options for handling correlation id on incoming requests. /// -public sealed class CorrelateOptions +public sealed class CorrelateOptions: CorrelationManagerOptions { private static readonly string[] DefaultRequestHeaders = { CorrelationHttpHeaders.CorrelationId }; diff --git a/src/Correlate.AspNetCore/DependencyInjection/ServiceCollectionExtensions.cs b/src/Correlate.AspNetCore/DependencyInjection/ServiceCollectionExtensions.cs index ebf374f..08c69bf 100644 --- a/src/Correlate.AspNetCore/DependencyInjection/ServiceCollectionExtensions.cs +++ b/src/Correlate.AspNetCore/DependencyInjection/ServiceCollectionExtensions.cs @@ -1,5 +1,6 @@ using Correlate.AspNetCore; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; namespace Correlate.DependencyInjection; @@ -16,8 +17,18 @@ public static class ServiceCollectionExtensions /// The so that additional calls can be chained. public static IServiceCollection AddCorrelate(this IServiceCollection services, Action configureOptions) { - return services - .Configure(configureOptions) + services + .Configure(configureOptions); + + services.AddOptions() + .Configure((CorrelationManagerOptions cmo, IOptions co) => + { + cmo.LoggingScopeKey = co.Value.LoggingScopeKey; + }); + + services .AddCorrelate(); + + return services; } } diff --git a/src/Correlate.Core/Http/CorrelateClientOptions.cs b/src/Correlate.Core/Http/CorrelateClientOptions.cs index a896c25..7a79930 100644 --- a/src/Correlate.Core/Http/CorrelateClientOptions.cs +++ b/src/Correlate.Core/Http/CorrelateClientOptions.cs @@ -3,7 +3,7 @@ /// /// Client options for adding correlation id to outgoing requests. /// -public class CorrelateClientOptions +public class CorrelateClientOptions : CorrelationManagerOptions { /// /// Gets or sets the request header to set the correlation id in for outgoing requests. Default 'X-Correlation-ID'. diff --git a/src/Correlate.DependencyInjection/IHttpClientBuilderExtensions.cs b/src/Correlate.DependencyInjection/IHttpClientBuilderExtensions.cs index 524f605..f8873d4 100644 --- a/src/Correlate.DependencyInjection/IHttpClientBuilderExtensions.cs +++ b/src/Correlate.DependencyInjection/IHttpClientBuilderExtensions.cs @@ -35,6 +35,14 @@ public static IHttpClientBuilder CorrelateRequests(this IHttpClientBuilder build throw new ArgumentNullException(nameof(builder)); } + builder.Services.AddOptions() + .Configure((CorrelationManagerOptions cmo) => + { + var options = new CorrelateClientOptions(); + configureOptions(options); + cmo.LoggingScopeKey = options.LoggingScopeKey; + }); + builder.Services.AddCorrelate(); builder.Services.TryAddTransient(); From 09a85d496657b87f3720f3329d1af2f7265db535 Mon Sep 17 00:00:00 2001 From: Laurynas Ruskys Date: Wed, 27 Sep 2023 17:32:40 +0300 Subject: [PATCH 05/12] Set correlation id to exception data using the same property name as in logs --- src/Correlate.Core/CorrelationManager.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Correlate.Core/CorrelationManager.cs b/src/Correlate.Core/CorrelationManager.cs index 518651d..93dbd96 100644 --- a/src/Correlate.Core/CorrelationManager.cs +++ b/src/Correlate.Core/CorrelationManager.cs @@ -212,12 +212,12 @@ private T Execute(string? correlationId, Func correlatedFunc, OnException? } } - private static bool HandlesException(OnException? onException, CorrelationContext correlationContext, Exception ex, out T result) + private bool HandlesException(OnException? onException, CorrelationContext correlationContext, Exception ex, out T result) { - if (!ex.Data.Contains(CorrelateConstants.CorrelationIdKey)) + if (!ex.Data.Contains(_options.LoggingScopeKey)) { // Because we're about to lose context scope, enrich exception with correlation id for reference by calling code. - ex.Data.Add(CorrelateConstants.CorrelationIdKey, correlationContext.CorrelationId); + ex.Data.Add(_options.LoggingScopeKey, correlationContext.CorrelationId); } if (onException is not null) From 6cb2651a3439b50d4dc5d54ed78422f38555192c Mon Sep 17 00:00:00 2001 From: Laurynas Ruskys Date: Wed, 27 Sep 2023 17:32:49 +0300 Subject: [PATCH 06/12] Add some variance into tests to validate custom logging scope key --- .../CorrelationManagerTests.cs | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/test/Correlate.Core.Tests/CorrelationManagerTests.cs b/test/Correlate.Core.Tests/CorrelationManagerTests.cs index dc28cc6..7cf417b 100644 --- a/test/Correlate.Core.Tests/CorrelationManagerTests.cs +++ b/test/Correlate.Core.Tests/CorrelationManagerTests.cs @@ -1,5 +1,4 @@ using System.Collections; -using System.Diagnostics; using Correlate.Testing; using Correlate.Testing.TestCases; using Microsoft.Extensions.Logging; @@ -21,7 +20,7 @@ public class CorrelationManagerTests : IDisposable private readonly SerilogLoggerProvider _logProvider; private readonly CorrelationManager _sut; - protected CorrelationManagerTests() + protected CorrelationManagerTests(CorrelationManagerOptions options) { _correlationContextAccessor = new CorrelationContextAccessor(); @@ -36,7 +35,7 @@ protected CorrelationManagerTests() _logProvider = new SerilogLoggerProvider(serilogLogger); _logger = new TestLogger(_logProvider.CreateLogger(nameof(CorrelationManager))); - _options = Options.Create(new CorrelationManagerOptions()); + _options = Options.Create(options); _sut = new CorrelationManager( new CorrelationContextFactory(_correlationContextAccessor), @@ -55,6 +54,13 @@ public void Dispose() public class Async : CorrelationManagerTests { + public Async() : base(new() + { + LoggingScopeKey = "ActivityId" + }) + { + } + [Fact] public async Task Given_a_task_should_run_task_inside_correlated_context() { @@ -148,7 +154,7 @@ await _sut.CorrelateAsync(() => var logEvents = TestCorrelator.GetLogEventsFromCurrentContext().ToList(); logEvents.Should() .HaveCount(3) - .And.ContainSingle(ev => ev.MessageTemplate.Text == "Message with correlation id." && ev.Properties.ContainsKey("CorrelationId")); + .And.ContainSingle(ev => ev.MessageTemplate.Text == "Message with correlation id." && ev.Properties.ContainsKey("ActivityId")); } } @@ -236,7 +242,7 @@ Task ThrowingTask() .Cast() .ToDictionary(kvp => kvp.Key, kvp => kvp.Value) .Should() - .ContainKey(CorrelateConstants.CorrelationIdKey) + .ContainKey("ActivityId") .WhoseValue.Should() .Be(GeneratedCorrelationId); } @@ -366,6 +372,10 @@ await _sut.CorrelateAsync(innerContextId, public class Sync : CorrelationManagerTests { + public Sync() : base(new()) + { + } + [Fact] public void Given_a_action_should_run_action_inside_correlated_context() { From 914256d8def24d3245fcb895968e56a37a364174 Mon Sep 17 00:00:00 2001 From: Laurynas Ruskys Date: Tue, 3 Oct 2023 17:07:49 +0300 Subject: [PATCH 07/12] Revert to using `CorrelationId` in exception data --- src/Correlate.Core/CorrelationManager.cs | 4 ++-- test/Correlate.Core.Tests/CorrelationManagerTests.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Correlate.Core/CorrelationManager.cs b/src/Correlate.Core/CorrelationManager.cs index 93dbd96..1247fa6 100644 --- a/src/Correlate.Core/CorrelationManager.cs +++ b/src/Correlate.Core/CorrelationManager.cs @@ -214,10 +214,10 @@ private T Execute(string? correlationId, Func correlatedFunc, OnException? private bool HandlesException(OnException? onException, CorrelationContext correlationContext, Exception ex, out T result) { - if (!ex.Data.Contains(_options.LoggingScopeKey)) + if (!ex.Data.Contains(CorrelateConstants.CorrelationIdKey)) { // Because we're about to lose context scope, enrich exception with correlation id for reference by calling code. - ex.Data.Add(_options.LoggingScopeKey, correlationContext.CorrelationId); + ex.Data.Add(CorrelateConstants.CorrelationIdKey, correlationContext.CorrelationId); } if (onException is not null) diff --git a/test/Correlate.Core.Tests/CorrelationManagerTests.cs b/test/Correlate.Core.Tests/CorrelationManagerTests.cs index 7cf417b..983e5b1 100644 --- a/test/Correlate.Core.Tests/CorrelationManagerTests.cs +++ b/test/Correlate.Core.Tests/CorrelationManagerTests.cs @@ -242,7 +242,7 @@ Task ThrowingTask() .Cast() .ToDictionary(kvp => kvp.Key, kvp => kvp.Value) .Should() - .ContainKey("ActivityId") + .ContainKey(CorrelateConstants.CorrelationIdKey) .WhoseValue.Should() .Be(GeneratedCorrelationId); } From 48b8509acb50902091c5cb7188485996e8b977fd Mon Sep 17 00:00:00 2001 From: Laurynas Ruskys Date: Tue, 3 Oct 2023 17:20:58 +0300 Subject: [PATCH 08/12] Follow the original constructor chaining pattern to keep backwards compatibility --- src/Correlate.Core/CorrelationManager.cs | 31 ++++++++++++++----- .../CorrelationManagerTests.cs | 7 +++-- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/Correlate.Core/CorrelationManager.cs b/src/Correlate.Core/CorrelationManager.cs index 1247fa6..00403b7 100644 --- a/src/Correlate.Core/CorrelationManager.cs +++ b/src/Correlate.Core/CorrelationManager.cs @@ -14,7 +14,7 @@ public class CorrelationManager : IAsyncCorrelationManager, ICorrelationManager, private readonly ICorrelationIdFactory _correlationIdFactory; private readonly DiagnosticListener? _diagnosticListener; private readonly ILogger _logger; - private readonly CorrelationManagerOptions _options; + private readonly CorrelationManagerOptions _options = new CorrelationManagerOptions(); /// /// Initializes a new instance of the class. @@ -23,21 +23,38 @@ public class CorrelationManager : IAsyncCorrelationManager, ICorrelationManager, /// The correlation id factory used to generate a new correlation id per context. /// The correlation context accessor. /// The logger. - /// The configuration options. public CorrelationManager ( ICorrelationContextFactory correlationContextFactory, ICorrelationIdFactory correlationIdFactory, ICorrelationContextAccessor correlationContextAccessor, - ILogger logger, - IOptions options + ILogger logger ) { _correlationContextFactory = correlationContextFactory ?? throw new ArgumentNullException(nameof(correlationContextFactory)); _correlationIdFactory = correlationIdFactory ?? throw new ArgumentNullException(nameof(correlationIdFactory)); _correlationContextAccessor = correlationContextAccessor ?? throw new ArgumentNullException(nameof(correlationContextAccessor)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); - _options = options.Value; + } + + /// + /// Initializes a new instance of the class. + /// + /// The correlation context factory used to create new contexts. + /// The correlation id factory used to generate a new correlation id per context. + /// The correlation context accessor. + /// The logger. + /// The diagnostics listener to run activities on. + public CorrelationManager + ( + ICorrelationContextFactory correlationContextFactory, + ICorrelationIdFactory correlationIdFactory, + ICorrelationContextAccessor correlationContextAccessor, + ILogger logger, + DiagnosticListener diagnosticListener + ) : this(correlationContextFactory, correlationIdFactory, correlationContextAccessor, logger) + { + _diagnosticListener = diagnosticListener ?? throw new ArgumentNullException(nameof(diagnosticListener)); } /// @@ -57,9 +74,9 @@ public CorrelationManager ILogger logger, DiagnosticListener diagnosticListener, IOptions options - ) : this(correlationContextFactory, correlationIdFactory, correlationContextAccessor, logger, options) + ) : this(correlationContextFactory, correlationIdFactory, correlationContextAccessor, logger, diagnosticListener) { - _diagnosticListener = diagnosticListener ?? throw new ArgumentNullException(nameof(diagnosticListener)); + _options = options?.Value ?? throw new ArgumentNullException(nameof(options)); } /// diff --git a/test/Correlate.Core.Tests/CorrelationManagerTests.cs b/test/Correlate.Core.Tests/CorrelationManagerTests.cs index 983e5b1..1bab845 100644 --- a/test/Correlate.Core.Tests/CorrelationManagerTests.cs +++ b/test/Correlate.Core.Tests/CorrelationManagerTests.cs @@ -1,4 +1,5 @@ using System.Collections; +using System.Diagnostics; using Correlate.Testing; using Correlate.Testing.TestCases; using Microsoft.Extensions.Logging; @@ -16,6 +17,7 @@ public class CorrelationManagerTests : IDisposable private readonly CorrelationContextAccessor _correlationContextAccessor; private readonly ICorrelationIdFactory _correlationIdFactoryMock; private readonly ILogger _logger; + private readonly DiagnosticListener _diagnosticListener; private readonly IOptions _options; private readonly SerilogLoggerProvider _logProvider; private readonly CorrelationManager _sut; @@ -35,6 +37,7 @@ protected CorrelationManagerTests(CorrelationManagerOptions options) _logProvider = new SerilogLoggerProvider(serilogLogger); _logger = new TestLogger(_logProvider.CreateLogger(nameof(CorrelationManager))); + _diagnosticListener = new DiagnosticListener("test"); _options = Options.Create(options); _sut = new CorrelationManager( @@ -42,6 +45,7 @@ protected CorrelationManagerTests(CorrelationManagerOptions options) _correlationIdFactoryMock, _correlationContextAccessor, _logger, + _diagnosticListener, _options ); } @@ -689,8 +693,7 @@ public static IEnumerable NullArgumentTestCases() Substitute.For(), Substitute.For(), Substitute.For(), - Substitute.For>(), - Substitute.For>() + Substitute.For>() ); static Task CorrelatedTask() From 969ab18a4b186fbb74ba21d42ac34cd40f444bf1 Mon Sep 17 00:00:00 2001 From: Laurynas Ruskys Date: Wed, 4 Oct 2023 08:58:02 +0300 Subject: [PATCH 09/12] Do not change global options when registering HttpClient correlation handler --- src/Correlate.Core/Http/CorrelateClientOptions.cs | 2 +- .../IHttpClientBuilderExtensions.cs | 8 -------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/Correlate.Core/Http/CorrelateClientOptions.cs b/src/Correlate.Core/Http/CorrelateClientOptions.cs index 7a79930..a896c25 100644 --- a/src/Correlate.Core/Http/CorrelateClientOptions.cs +++ b/src/Correlate.Core/Http/CorrelateClientOptions.cs @@ -3,7 +3,7 @@ /// /// Client options for adding correlation id to outgoing requests. /// -public class CorrelateClientOptions : CorrelationManagerOptions +public class CorrelateClientOptions { /// /// Gets or sets the request header to set the correlation id in for outgoing requests. Default 'X-Correlation-ID'. diff --git a/src/Correlate.DependencyInjection/IHttpClientBuilderExtensions.cs b/src/Correlate.DependencyInjection/IHttpClientBuilderExtensions.cs index f8873d4..524f605 100644 --- a/src/Correlate.DependencyInjection/IHttpClientBuilderExtensions.cs +++ b/src/Correlate.DependencyInjection/IHttpClientBuilderExtensions.cs @@ -35,14 +35,6 @@ public static IHttpClientBuilder CorrelateRequests(this IHttpClientBuilder build throw new ArgumentNullException(nameof(builder)); } - builder.Services.AddOptions() - .Configure((CorrelationManagerOptions cmo) => - { - var options = new CorrelateClientOptions(); - configureOptions(options); - cmo.LoggingScopeKey = options.LoggingScopeKey; - }); - builder.Services.AddCorrelate(); builder.Services.TryAddTransient(); From bc6eac85a878503f8acd1a764c9656f4673fa324 Mon Sep 17 00:00:00 2001 From: Laurynas Ruskys Date: Wed, 4 Oct 2023 09:48:10 +0300 Subject: [PATCH 10/12] Set `HandlesException` back to `static` --- src/Correlate.Core/CorrelationManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Correlate.Core/CorrelationManager.cs b/src/Correlate.Core/CorrelationManager.cs index 00403b7..6bffe69 100644 --- a/src/Correlate.Core/CorrelationManager.cs +++ b/src/Correlate.Core/CorrelationManager.cs @@ -229,7 +229,7 @@ private T Execute(string? correlationId, Func correlatedFunc, OnException? } } - private bool HandlesException(OnException? onException, CorrelationContext correlationContext, Exception ex, out T result) + private static bool HandlesException(OnException? onException, CorrelationContext correlationContext, Exception ex, out T result) { if (!ex.Data.Contains(CorrelateConstants.CorrelationIdKey)) { From 9050885e54f9043393d5ef7c190c09a54ed09a52 Mon Sep 17 00:00:00 2001 From: Laurynas Ruskys Date: Thu, 5 Oct 2023 08:59:52 +0300 Subject: [PATCH 11/12] Add tests for AddCorrelate overload --- .../IServiceCollectionExtensionsTests.cs | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/test/Correlate.DependencyInjection.Tests/IServiceCollectionExtensionsTests.cs b/test/Correlate.DependencyInjection.Tests/IServiceCollectionExtensionsTests.cs index 06db37b..bfe2605 100644 --- a/test/Correlate.DependencyInjection.Tests/IServiceCollectionExtensionsTests.cs +++ b/test/Correlate.DependencyInjection.Tests/IServiceCollectionExtensionsTests.cs @@ -10,15 +10,20 @@ public class When_adding_correlate_to_container private readonly IServiceCollection _services; private readonly IServiceProvider _sut; - public When_adding_correlate_to_container() + protected When_adding_correlate_to_container(IServiceCollection services) { - _services = new ServiceCollection() - .AddLogging() - .AddCorrelate(); + _services = services; _sut = _services.BuildServiceProvider(); } + public When_adding_correlate_to_container() + : this(new ServiceCollection() + .AddLogging() + .AddCorrelate()) + { + } + [Theory] [ClassData(typeof(ExpectedRegistrations))] public void It_should_resolve(ExpectedRegistration registration) @@ -58,3 +63,13 @@ protected virtual IEnumerable TestCases() } } } + +public class When_adding_correlate_to_container_with_options : When_adding_correlate_to_container +{ + public When_adding_correlate_to_container_with_options() + : base(new ServiceCollection() + .AddLogging() + .AddCorrelate(o => o.LoggingScopeKey = "CustomLoggingScopeKey")) + { + } +} \ No newline at end of file From 2f57541f585b361edcc92cc759013f9a9ae81378 Mon Sep 17 00:00:00 2001 From: Laurynas Ruskys Date: Thu, 5 Oct 2023 09:17:22 +0300 Subject: [PATCH 12/12] Use full overload in tests to cover all constructors --- test/Correlate.Core.Tests/CorrelationManagerTests.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/Correlate.Core.Tests/CorrelationManagerTests.cs b/test/Correlate.Core.Tests/CorrelationManagerTests.cs index 1bab845..422d007 100644 --- a/test/Correlate.Core.Tests/CorrelationManagerTests.cs +++ b/test/Correlate.Core.Tests/CorrelationManagerTests.cs @@ -693,7 +693,9 @@ public static IEnumerable NullArgumentTestCases() Substitute.For(), Substitute.For(), Substitute.For(), - Substitute.For>() + Substitute.For>(), + Substitute.For("test"), + Options.Create(new CorrelationManagerOptions()) ); static Task CorrelatedTask()