Change defaults: do not replace TraceIdentifier (#33)

* By default, do not replace `TraceIdentifier` * Remove `sealed` from tests
wdolek · Aug 30, 2020 · cd0f0c1 · cd0f0c1
1 parent 8123a86
commit cd0f0c1
Show file tree

Hide file tree

Showing 8 changed files with 41 additions and 42 deletions.
diff --git a/docs/configuration.md b/docs/configuration.md
@@ -8,9 +8,9 @@ By default, Correlator is configured following way:
   - `X-Request-Id`
 - By default, when there are multiple correlation headers sent, it is not guaranteed which one is going to be read
 - When request correlation is missing or has empty value, new correlation ID is generated in form of GUID
-- Correlation ID is forwarded to subsequent requests via `X-Correlation-Id` (when using `CorrelatorHttpMessageHandler`)
+- Correlation ID is forwarded to subsequent requests as `X-Correlation-Id` (when using `CorrelatorHttpMessageHandler`)
 - Correlation ID is not set to HTTP response headers
-- Correlation ID replaces [`HttpContext.TraceIdentifier`](https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.http.httpcontext.traceidentifier)
+- Correlation ID does not replace [`HttpContext.TraceIdentifier`](https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.http.httpcontext.traceidentifier)
 - Correlation ID is not added to logger scope
 
 To adjust setting, use `AddDefaultCorrelator` or `AddCorrelator` overload:
@@ -36,8 +36,8 @@ services.AddDefaultCorrelator(
         // - when correlation ID not received and generated, sending it as "X-Correlation-Id"
         correlatorOptions.Forward = PropagationSettings.KeepIncomingHeaderName();
 
-        // don't write correlation ID to HttpContext.TraceIdentifier
-        ReplaceTraceIdentifier = false,
+        // replace `HttpContext.TraceIdentifier`
+        ReplaceTraceIdentifier = true,
 
         // create logging scope with given key
         LoggingScope = LoggingScopeSettings.IncludeLoggingScope("Correlation"),
@@ -140,10 +140,12 @@ Either:
 - `NoScope`: logging scope is not created
 - `IncludeLoggingScope(string)`: logging scope is created, correlation ID is saved with provided key
 
+Use scope for structured logging.
+
 ```csharp
 // no logging scope
 correlatorOptions.LoggingScope = LoggingScopeSettings.NoScope;
 
-// logging scope with: correlation = <Correlation ID>
-correlatorOptions.LoggingScope = LoggingScopeSettings.IncludeLoggingScope("correlation");
+// logging scope with: Correlation = <Correlation ID>
+correlatorOptions.LoggingScope = LoggingScopeSettings.IncludeLoggingScope("Correlation");
 ```
diff --git a/src/W4k.AspNetCore.Correlator/Http/HttpHeaders.cs b/src/W4k.AspNetCore.Correlator/Http/HttpHeaders.cs
@@ -6,21 +6,18 @@
     public static class HttpHeaders
     {
         /// <summary>
-        /// <c>X-CorrelationId</c> header name.
+        /// <c>X-Correlation-Id</c> header name.
         /// </summary>
         public static readonly string CorrelationId = "X-Correlation-Id";
 
         /// <summary>
-        /// <c>X-RequestId</c> header name.
+        /// <c>X-Request-Id</c> header name.
         /// </summary>
         public static readonly string RequestId = "X-Request-Id";
 
         /// <summary>
         /// <c>Request-Id</c> header name.
         /// </summary>
-        /// <remarks>
-        /// Used by ASP.NET Core 2.x.
-        /// </remarks>
         public static readonly string AspNetRequestId = "Request-Id";
     }
 }
diff --git a/src/W4k.AspNetCore.Correlator/Options/CorrelatorOptions.cs b/src/W4k.AspNetCore.Correlator/Options/CorrelatorOptions.cs
@@ -6,7 +6,7 @@
 namespace W4k.AspNetCore.Correlator.Options
 {
     /// <summary>
-    /// Correlator (middleware) options.
+    /// Correlator options.
     /// </summary>
     public sealed class CorrelatorOptions
     {
@@ -42,7 +42,7 @@ public sealed class CorrelatorOptions
         /// <summary>
         /// Gets or sets a value indicating whether <see cref="HttpContext.TraceIdentifier"/> is replaced by correlation ID or left intact.
         /// </summary>
-        public bool ReplaceTraceIdentifier { get; set; } = true;
+        public bool ReplaceTraceIdentifier { get; set; } = false;
 
         /// <summary>
         /// Gets or sets logging scope settings.

diff --git a/test/W4k.AspNetCore.Correlator.IntegrationTests/ConcurrentCorrelatorTests.cs b/test/W4k.AspNetCore.Correlator.IntegrationTests/ConcurrentCorrelatorTests.cs
@@ -9,7 +9,7 @@
 
 namespace W4k.AspNetCore.Correlator
 {
-    public sealed class ConcurrentCorrelatorTests : CorrelatorTestsBase<CustomOptionsStartup>
+    public class ConcurrentCorrelatorTests : CorrelatorTestsBase<CustomOptionsStartup>
     {
         private readonly ITestOutputHelper _output;
         private readonly int _concurrency;

diff --git a/test/W4k.AspNetCore.Correlator.IntegrationTests/CustomCorrelatorTests.cs b/test/W4k.AspNetCore.Correlator.IntegrationTests/CustomCorrelatorTests.cs
@@ -7,7 +7,7 @@
 
 namespace W4k.AspNetCore.Correlator
 {
-    public sealed class CustomCorrelatorTests : CorrelatorTestsBase<CustomOptionsStartup>
+    public class CustomCorrelatorTests : CorrelatorTestsBase<CustomOptionsStartup>
     {
         [Fact]
         public async Task CorrelationIdReadFromRequest()
@@ -30,22 +30,5 @@ public async Task CorrelationIdReadFromRequest()
             string correlationId = await response.Content.ReadAsStringAsync();
             Assert.Equal("123", correlationId);
         }
-
-        [Fact]
-        public async Task CorrelationIdNotFound()
-        {
-            // arrange
-            var request = new HttpRequestMessage(HttpMethod.Get, "/");
-            request.Headers.Add("X-Dummy-Correlation-Id", "123");
-
-            // act
-            HttpResponseMessage response = await Client.SendAsync(request, CancellationToken.None);
-
-            // assert
-            Assert.False(response.Headers.Contains("X-Dummy-Correlation-Id"));
-
-            string correlationId = await response.Content.ReadAsStringAsync();
-            Assert.Equal("", correlationId);
-        }
     }
 }
diff --git a/test/W4k.AspNetCore.Correlator.IntegrationTests/DefaultCorrelatorTests.cs b/test/W4k.AspNetCore.Correlator.IntegrationTests/DefaultCorrelatorTests.cs
@@ -7,7 +7,7 @@
 
 namespace W4k.AspNetCore.Correlator
 {
-    public sealed class DefaultCorrelatorTests : CorrelatorTestsBase<DefaultOptionsStartup>
+    public class DefaultCorrelatorTests : CorrelatorTestsBase<DefaultOptionsStartup>
     {
         [Theory]
         [InlineData("Request-Id")]
@@ -42,5 +42,23 @@ public async Task CorrelationIdGenerated()
             Assert.NotEmpty(correlationId);
             Assert.True(Guid.TryParse(correlationId, out Guid _));
         }
+
+        [Fact]
+        public async Task CorrelationIdNotFound()
+        {
+            // arrange
+            var request = new HttpRequestMessage(HttpMethod.Get, "/");
+            request.Headers.Add("X-Dummy-Correlation-Id", "123");
+
+            // act
+            HttpResponseMessage response = await Client.SendAsync(request, CancellationToken.None);
+
+            // assert
+            Assert.False(response.Headers.Contains("X-Dummy-Correlation-Id"));
+
+            string correlationId = await response.Content.ReadAsStringAsync();
+            Assert.NotEqual("123", correlationId);
+            Assert.True(Guid.TryParse(correlationId, out Guid _));
+        }
     }
 }
diff --git a/test/W4k.AspNetCore.Correlator.IntegrationTests/Startup/CustomOptionsStartup.cs b/test/W4k.AspNetCore.Correlator.IntegrationTests/Startup/CustomOptionsStartup.cs
@@ -2,7 +2,6 @@
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.DependencyInjection;
-using W4k.AspNetCore.Correlator.Context;
 using W4k.AspNetCore.Correlator.Extensions;
 using W4k.AspNetCore.Correlator.Extensions.DependencyInjection;
 using W4k.AspNetCore.Correlator.Options;
@@ -26,19 +25,19 @@ public void ConfigureServices(IServiceCollection services) =>
                     // do not emit correlation ID
                     o.Emit = PropagationSettings.KeepIncomingHeaderName();
 
-                    // don't overwrite `HttpContext.TraceIdentifier` by correlation ID
-                    o.ReplaceTraceIdentifier = false;
+                    // overwrite `HttpContext.TraceIdentifier` by correlation ID
+                    o.ReplaceTraceIdentifier = true;
 
                     // enable logging scope containing correlation ID
-                    o.LoggingScope = LoggingScopeSettings.IncludeLoggingScope("correlation");
+                    o.LoggingScope = LoggingScopeSettings.IncludeLoggingScope("Correlation");
                 });
 
-        public void Configure(IApplicationBuilder app, ICorrelationContextAccessor correlationContextAccessor)
+        public void Configure(IApplicationBuilder app)
         {
             app.UseCorrelator();
             app.Use(async (context, next) =>
             {
-                var correlationId = correlationContextAccessor.CorrelationContext.CorrelationId;
+                var correlationId = context.TraceIdentifier;
 
                 context.Response.Headers.Add("Content-Type", "text/plain");
                 await context.Response.WriteAsync(correlationId);

diff --git a/test/W4k.AspNetCore.Correlator.IntegrationTests/Startup/DefaultOptionsStartup.cs b/test/W4k.AspNetCore.Correlator.IntegrationTests/Startup/DefaultOptionsStartup.cs
@@ -2,6 +2,7 @@
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.DependencyInjection;
+using W4k.AspNetCore.Correlator.Context;
 using W4k.AspNetCore.Correlator.Extensions;
 using W4k.AspNetCore.Correlator.Extensions.DependencyInjection;
 
@@ -14,13 +15,12 @@ public void ConfigureServices(IServiceCollection services)
             services.AddDefaultCorrelator();
         }
 
-        public void Configure(IApplicationBuilder app)
+        public void Configure(IApplicationBuilder app, ICorrelationContextAccessor correlationContextAccessor)
         {
             app.UseCorrelator();
             app.Use(async (context, next) =>
             {
-                // by default, correlation ID is written to `HttpContext.TraceIdentifier`
-                var correlationId = context.TraceIdentifier;
+                var correlationId = correlationContextAccessor.CorrelationContext.CorrelationId;
 
                 context.Response.Headers.Add("Content-Type", "text/plain");
                 await context.Response.WriteAsync(correlationId);