diff --git a/Directory.Packages.props b/Directory.Packages.props index 3ec7add..7f92c0c 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -3,29 +3,29 @@ true - + - - - - + + + + - - + + all runtime; build; native; contentfiles; analyzers; buildtransitive - + - + @@ -39,12 +39,12 @@ - - - - + + + + - + @@ -55,29 +55,29 @@ - - + + - - - - + + + + all runtime; build; native; contentfiles; analyzers; buildtransitive - - + + - - - + + + - - + + @@ -86,17 +86,17 @@ - + - + - - - - + + + + \ No newline at end of file diff --git a/PiBox.Hosting/Abstractions/src/PiBox.Hosting.Abstractions/Middlewares/EnrichRequestMetricsMiddleware.cs b/PiBox.Hosting/Abstractions/src/PiBox.Hosting.Abstractions/Middlewares/EnrichRequestMetricsMiddleware.cs new file mode 100644 index 0000000..dee8023 --- /dev/null +++ b/PiBox.Hosting/Abstractions/src/PiBox.Hosting.Abstractions/Middlewares/EnrichRequestMetricsMiddleware.cs @@ -0,0 +1,26 @@ +using Chronos.Abstractions; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; +using PiBox.Hosting.Abstractions.Attributes; + +namespace PiBox.Hosting.Abstractions.Middlewares +{ + [Middleware] + public sealed class EnrichRequestMetricsMiddleware : ApiMiddleware + { + public EnrichRequestMetricsMiddleware(RequestDelegate next, IDateTimeProvider dateTimeProvider) : base(next, + dateTimeProvider) + { + } + + public override Task Invoke(HttpContext context) + { + var tagsFeature = context.Features.Get(); + if (tagsFeature == null) return Next.Invoke(context); + var authorizedParty = context.User.Claims.SingleOrDefault(x => x.Type == "azp")?.Value; + tagsFeature.Tags.Add(new KeyValuePair("azp", authorizedParty ?? "")); + + return Next.Invoke(context); + } + } +} diff --git a/PiBox.Hosting/Abstractions/test/PiBox.Hosting.Abstractions.Tests/Middlewares/EnrichRequestMetricsMiddlewareTests.cs b/PiBox.Hosting/Abstractions/test/PiBox.Hosting.Abstractions.Tests/Middlewares/EnrichRequestMetricsMiddlewareTests.cs new file mode 100644 index 0000000..c52eee3 --- /dev/null +++ b/PiBox.Hosting/Abstractions/test/PiBox.Hosting.Abstractions.Tests/Middlewares/EnrichRequestMetricsMiddlewareTests.cs @@ -0,0 +1,86 @@ +using System.Security.Claims; +using Chronos.Abstractions; +using FluentAssertions; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; +using NSubstitute; +using NUnit.Framework; +using PiBox.Hosting.Abstractions.Middlewares; + +namespace PiBox.Hosting.Abstractions.Tests.Middlewares +{ + public class EnrichRequestMetricsMiddlewareTests + { + private readonly IDateTimeProvider _dateTimeProvider = Substitute.For(); + + private static HttpContext GetContext() + { + return new DefaultHttpContext { Response = { Body = new MemoryStream() } }; + } + + [SetUp] + public void Setup() + { + _dateTimeProvider.UtcNow.Returns(new DateTime(2020, 1, 1)); + } + + [Test] + public async Task ExistingAzpClaimShouldAddTagWithUserIdToMetrics() + { + var httpMetricsTagsFeature = Substitute.For(); + httpMetricsTagsFeature.Tags.Returns(new List>()); + var middleware = new EnrichRequestMetricsMiddleware(x => + { + x.Response.StatusCode = 200; + return Task.CompletedTask; + }, _dateTimeProvider); + var context = GetContext(); + + context.User = new ClaimsPrincipal(new ClaimsIdentity(new List() { new Claim("azp", "userid1") })); + context.Features.Set(httpMetricsTagsFeature); + + await middleware.Invoke(context); + var metrics = context.Features.Get(); + metrics.Tags.Should().Contain(x => x.Key == "azp" && x.Value.ToString() == "userid1"); + } + + [Test] + public async Task NonExistingAzpClaimShouldAddTagWithEmptyValueToMetrics() + { + var httpMetricsTagsFeature = Substitute.For(); + httpMetricsTagsFeature.Tags.Returns(new List>()); + var middleware = new EnrichRequestMetricsMiddleware(x => + { + x.Response.StatusCode = 200; + return Task.CompletedTask; + }, _dateTimeProvider); + var context = GetContext(); + + context.User = new ClaimsPrincipal(new ClaimsIdentity(new List() { new Claim("azp1", "userid1") })); + context.Features.Set(httpMetricsTagsFeature); + + await middleware.Invoke(context); + var metrics = context.Features.Get(); + metrics.Tags.Should().Contain(x => x.Key == "azp" && x.Value.ToString() == ""); + } + + [Test] + public async Task NoHttpMetricsTagsFeatureShouldNotAddMetricTagsToResponse() + { + var httpMetricsTagsFeature = Substitute.For(); + httpMetricsTagsFeature.Tags.Returns(new List>()); + var middleware = new EnrichRequestMetricsMiddleware(x => + { + x.Response.StatusCode = 200; + return Task.CompletedTask; + }, _dateTimeProvider); + var context = GetContext(); + + context.User = new ClaimsPrincipal(new ClaimsIdentity(new List() { new Claim("azp1", "userid1") })); + + await middleware.Invoke(context); + var metrics = context.Features.Get(); + metrics.Should().BeNull(); + } + } +} diff --git a/PiBox.Hosting/WebHost/src/PiBox.Hosting.WebHost/Configurators/ServiceConfigurator.cs b/PiBox.Hosting/WebHost/src/PiBox.Hosting.WebHost/Configurators/ServiceConfigurator.cs index 18a26d7..19c1f94 100644 --- a/PiBox.Hosting/WebHost/src/PiBox.Hosting.WebHost/Configurators/ServiceConfigurator.cs +++ b/PiBox.Hosting/WebHost/src/PiBox.Hosting.WebHost/Configurators/ServiceConfigurator.cs @@ -7,7 +7,6 @@ using Chronos.Abstractions; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Cors.Infrastructure; -using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.HttpOverrides; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.ModelBinding; @@ -126,14 +125,7 @@ internal void ConfigureMetrics() resource.AddService(serviceName, serviceVersion: entryAssembly.GetName().Version?.ToString() ?? "unknown", serviceInstanceId: Environment.MachineName); }) - .AddAspNetCoreInstrumentation(options => - { - options.Enrich = (string metricName, HttpContext context, ref TagList tags) => - { - var authorizedParty = context.User.Claims.SingleOrDefault(x => x.Type == "azp")?.Value; - tags.Add("azp", authorizedParty); - }; - }) + .AddAspNetCoreInstrumentation() .AddHttpClientInstrumentation() .AddRuntimeInstrumentation() .AddProcessInstrumentation() diff --git a/PiBox.Hosting/WebHost/test/PiBox.Hosting.WebHost.Tests/Logging/RequestLoggingTests.cs b/PiBox.Hosting/WebHost/test/PiBox.Hosting.WebHost.Tests/Logging/RequestLoggingTests.cs index d73b8bd..607f9f2 100644 --- a/PiBox.Hosting/WebHost/test/PiBox.Hosting.WebHost.Tests/Logging/RequestLoggingTests.cs +++ b/PiBox.Hosting/WebHost/test/PiBox.Hosting.WebHost.Tests/Logging/RequestLoggingTests.cs @@ -1,3 +1,4 @@ +using FluentAssertions; using Microsoft.AspNetCore.Http; using NUnit.Framework; using PiBox.Hosting.WebHost.Logging; @@ -17,7 +18,7 @@ public void TestDetermineLoggingIsSettingTheLevelToErrorForTheGivenPathsStatusco StructuredLoggingExtensions.DetermineRequestLogLevel(new[] { "/metrics-text" })(httpContext, 0, null); - Assert.AreEqual(LogEventLevel.Error, logLevel); + LogEventLevel.Error.Should().Be(logLevel); } [Test] @@ -31,7 +32,7 @@ public void TestDetermineLoggingIsSettingTheLevelToErrorForTheGivenPathsExceptio StructuredLoggingExtensions.DetermineRequestLogLevel(new[] { "/metrics-text" })(httpContext, 0, new Exception("test")); - Assert.AreEqual(LogEventLevel.Error, logLevel); + LogEventLevel.Error.Should().Be(logLevel); } [Test] @@ -45,7 +46,7 @@ public void TestDetermineLoggingIsExcludingTheGivenPaths() StructuredLoggingExtensions.DetermineRequestLogLevel(new[] { "/MeTRICS", "/metrics" })(httpContext, 0, null); - Assert.AreEqual(LogEventLevel.Verbose, logLevel); + LogEventLevel.Verbose.Should().Be(logLevel); } [Test] @@ -58,7 +59,7 @@ public void TestDetermineLoggingIsExcludingTheGivenWildcardPaths() StructuredLoggingExtensions.DetermineRequestLogLevel(new[] { "/MeTRICS", "/metrics*" })( httpContext, 0, null); - Assert.AreEqual(LogEventLevel.Verbose, logLevel); + LogEventLevel.Verbose.Should().Be(logLevel); } [Test] @@ -71,7 +72,7 @@ public void TestDetermineLoggingIsNotExcludingTheGivenWildcardPaths() var logLevel = StructuredLoggingExtensions.DetermineRequestLogLevel(new[] { "/hangfire*" })(httpContext, 0, null); - Assert.AreEqual(LogEventLevel.Information, logLevel); + LogEventLevel.Information.Should().Be(logLevel); } [TestCase("/metrics-text", new[] { "/MeTRICS", "/metrics" }, 499, null)] @@ -86,7 +87,7 @@ public void TestDetermineLoggingIsNotExcludingTheGivenPaths(string actualPath, I var logLevel = StructuredLoggingExtensions.DetermineRequestLogLevel(paths)(httpContext, 0, exception); - Assert.AreEqual(LogEventLevel.Information, logLevel); + LogEventLevel.Information.Should().Be(logLevel); } } }