[Metrics] New components to track HTTP failures and exceptions. (#3928)

* New components to track http failures and generic errors. * Emitting metrics signals for every failed request. * Emiting metrics when exceptions are logged. * Cover one more scenario = errorslogger with no exceptions * Fix use of diferrent counters.
microsoft · Jun 19, 2024 · 414f8b3 · 414f8b3
1 parent 4bea202
commit 414f8b3
Show file tree

Hide file tree

Showing 14 changed files with 295 additions and 28 deletions.
diff --git a/src/Microsoft.Health.Fhir.Core/Logging/Metrics/DefaultFailureMetricHandler.cs b/src/Microsoft.Health.Fhir.Core/Logging/Metrics/DefaultFailureMetricHandler.cs
@@ -0,0 +1,44 @@
+// -------------------------------------------------------------------------------------------------
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
+// -------------------------------------------------------------------------------------------------
+
+using System.Collections.Generic;
+using System.Diagnostics.Metrics;
+using EnsureThat;
+
+namespace Microsoft.Health.Fhir.Core.Logging.Metrics
+{
+    public sealed class DefaultFailureMetricHandler : BaseMeterMetricHandler, IFailureMetricHandler
+    {
+        private readonly Counter<long> _counterExceptions;
+        private readonly Counter<long> _counterHttpFailures;
+
+        public DefaultFailureMetricHandler(IMeterFactory meterFactory)
+            : base(meterFactory)
+        {
+            _counterExceptions = MetricMeter.CreateCounter<long>("Failures.Exceptions");
+            _counterHttpFailures = MetricMeter.CreateCounter<long>("Failures.HttpFailures");
+        }
+
+        public void EmitException(IExceptionMetricNotification notification)
+        {
+            EnsureArg.IsNotNull(notification, nameof(notification));
+
+            _counterExceptions.Add(
+                1,
+                KeyValuePair.Create<string, object>("OperationName", notification.OperationName),
+                KeyValuePair.Create<string, object>("Severity", notification.Severity),
+                KeyValuePair.Create<string, object>("ExceptionType", notification.ExceptionType));
+        }
+
+        public void EmitHttpFailure(IHttpFailureMetricNotification notification)
+        {
+            EnsureArg.IsNotNull(notification, nameof(notification));
+
+            _counterHttpFailures.Add(
+                1,
+                KeyValuePair.Create<string, object>("OperationName", notification.OperationName));
+        }
+    }
+}
diff --git a/src/Microsoft.Health.Fhir.Core/Logging/Metrics/ExceptionMetricNotification.cs b/src/Microsoft.Health.Fhir.Core/Logging/Metrics/ExceptionMetricNotification.cs
@@ -0,0 +1,16 @@
+// -------------------------------------------------------------------------------------------------
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
+// -------------------------------------------------------------------------------------------------
+
+namespace Microsoft.Health.Fhir.Core.Logging.Metrics
+{
+    public sealed class ExceptionMetricNotification : IExceptionMetricNotification
+    {
+        public string OperationName { get; set; }
+
+        public string ExceptionType { get; set; }
+
+        public string Severity { get; set; }
+    }
+}
diff --git a/src/Microsoft.Health.Fhir.Core/Logging/Metrics/HttpErrorMetricNotification.cs b/src/Microsoft.Health.Fhir.Core/Logging/Metrics/HttpErrorMetricNotification.cs
@@ -0,0 +1,12 @@
+// -------------------------------------------------------------------------------------------------
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
+// -------------------------------------------------------------------------------------------------
+
+namespace Microsoft.Health.Fhir.Core.Logging.Metrics
+{
+    public sealed class HttpErrorMetricNotification : IHttpFailureMetricNotification
+    {
+        public string OperationName { get; set; }
+    }
+}
diff --git a/src/Microsoft.Health.Fhir.Core/Logging/Metrics/IExceptionMetricNotification.cs b/src/Microsoft.Health.Fhir.Core/Logging/Metrics/IExceptionMetricNotification.cs
@@ -0,0 +1,16 @@
+// -------------------------------------------------------------------------------------------------
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
+// -------------------------------------------------------------------------------------------------
+
+namespace Microsoft.Health.Fhir.Core.Logging.Metrics
+{
+    public interface IExceptionMetricNotification
+    {
+        string OperationName { get; set; }
+
+        string ExceptionType { get; set; }
+
+        string Severity { get; set; }
+    }
+}
diff --git a/src/Microsoft.Health.Fhir.Core/Logging/Metrics/IFailureMetricHandler.cs b/src/Microsoft.Health.Fhir.Core/Logging/Metrics/IFailureMetricHandler.cs
@@ -0,0 +1,14 @@
+// -------------------------------------------------------------------------------------------------
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
+// -------------------------------------------------------------------------------------------------
+
+namespace Microsoft.Health.Fhir.Core.Logging.Metrics
+{
+    public interface IFailureMetricHandler
+    {
+        void EmitHttpFailure(IHttpFailureMetricNotification notification);
+
+        void EmitException(IExceptionMetricNotification notification);
+    }
+}
diff --git a/src/Microsoft.Health.Fhir.Core/Logging/Metrics/IHttpFailureMetricNotification.cs b/src/Microsoft.Health.Fhir.Core/Logging/Metrics/IHttpFailureMetricNotification.cs
@@ -0,0 +1,12 @@
+// -------------------------------------------------------------------------------------------------
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
+// -------------------------------------------------------------------------------------------------
+
+namespace Microsoft.Health.Fhir.Core.Logging.Metrics
+{
+    public interface IHttpFailureMetricNotification
+    {
+        string OperationName { get; set; }
+    }
+}
diff --git a/...soft.Health.Fhir.Shared.Api.UnitTests/Features/Exceptions/BaseExceptionMiddlewareTests.cs b/...soft.Health.Fhir.Shared.Api.UnitTests/Features/Exceptions/BaseExceptionMiddlewareTests.cs
@@ -14,6 +14,7 @@
 using Microsoft.Health.Fhir.Api.Features.Exceptions;
 using Microsoft.Health.Fhir.Api.Features.Formatters;
 using Microsoft.Health.Fhir.Core.Features.Context;
+using Microsoft.Health.Fhir.Core.Logging.Metrics;
 using Microsoft.Health.Fhir.Tests.Common;
 using Microsoft.Health.Test.Utilities;
 using NSubstitute;
@@ -51,7 +52,9 @@ public BaseExceptionMiddlewareTests()
         [InlineData("The MetadataAddress or Authority must use HTTPS unless disabled for development by setting RequireHttpsMetadata=false.", "The security configuration requires the authority to be set to an https address.")]
         public async Task GivenAnHttpContextWithException_WhenExecutingBaseExceptionMiddleware_TheResponseShouldBeOperationOutcome(string exceptionMessage, string diagnosticMessage)
         {
-            var baseExceptionMiddleware = CreateBaseExceptionMiddleware(innerHttpContext => throw new Exception(exceptionMessage));
+            IFailureMetricHandler failureMetricHandler = Substitute.For<IFailureMetricHandler>();
+
+            var baseExceptionMiddleware = CreateBaseExceptionMiddleware(innerHttpContext => throw new Exception(exceptionMessage), failureMetricHandler);
 
             baseExceptionMiddleware.ExecuteResultAsync(Arg.Any<HttpContext>(), Arg.Any<IActionResult>()).Returns(Task.CompletedTask);
 
@@ -64,23 +67,34 @@ await baseExceptionMiddleware
                     Arg.Is<OperationOutcomeResult>(x => x.StatusCode == HttpStatusCode.InternalServerError &&
                                             x.Result.Id == _correlationId &&
                                             x.Result.Issue[0].Diagnostics == diagnosticMessage));
+
+            failureMetricHandler.Received(1).EmitHttpFailure(Arg.Any<IHttpFailureMetricNotification>());
         }
 
         [Fact]
         public async Task GivenAnHttpContextWithNoException_WhenExecutingBaseExceptionMiddleware_TheResponseShouldBeEmpty()
         {
-            var baseExceptionMiddleware = CreateBaseExceptionMiddleware(innerHttpContext => Task.CompletedTask);
+            IFailureMetricHandler failureMetricHandler = Substitute.For<IFailureMetricHandler>();
+
+            var baseExceptionMiddleware = CreateBaseExceptionMiddleware(innerHttpContext => Task.CompletedTask, failureMetricHandler);
 
             await baseExceptionMiddleware.Invoke(_context);
 
             Assert.Equal(200, _context.Response.StatusCode);
             Assert.Null(_context.Response.ContentType);
             Assert.Equal(0, _context.Response.Body.Length);
+
+            failureMetricHandler.Received(0).EmitHttpFailure(Arg.Any<IHttpFailureMetricNotification>());
         }
 
-        private BaseExceptionMiddleware CreateBaseExceptionMiddleware(RequestDelegate nextDelegate)
+        private BaseExceptionMiddleware CreateBaseExceptionMiddleware(RequestDelegate nextDelegate, IFailureMetricHandler failureMetricHandler)
         {
-            return Substitute.ForPartsOf<BaseExceptionMiddleware>(nextDelegate, NullLogger<BaseExceptionMiddleware>.Instance, _fhirRequestContextAccessor, _formatParametersValidator);
+            return Substitute.ForPartsOf<BaseExceptionMiddleware>(
+                nextDelegate,
+                NullLogger<BaseExceptionMiddleware>.Instance,
+                failureMetricHandler,
+                _fhirRequestContextAccessor,
+                _formatParametersValidator);
         }
     }
 }
diff --git a/src/Microsoft.Health.Fhir.Shared.Api/Extensions/HttpRequestExtension.cs b/src/Microsoft.Health.Fhir.Shared.Api/Extensions/HttpRequestExtension.cs
@@ -0,0 +1,45 @@
+// -------------------------------------------------------------------------------------------------
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
+// -------------------------------------------------------------------------------------------------
+
+using System;
+using System.Linq;
+using Microsoft.AspNetCore.Http;
+using Microsoft.Health.Fhir.Core.Features.Telemetry;
+
+namespace Microsoft.Health.Fhir.Api.Extensions
+{
+    public static class HttpRequestExtension
+    {
+        public static string GetOperationName(this HttpRequest request, bool includeRouteValues = true)
+        {
+            if (request != null)
+            {
+                var name = request.Path.Value;
+                if (request.RouteValues != null
+                    && request.RouteValues.TryGetValue(KnownHttpRequestProperties.RouteValueAction, out var action)
+                    && request.RouteValues.TryGetValue(KnownHttpRequestProperties.RouteValueController, out var controller))
+                {
+                    name = $"{controller}/{action}";
+
+                    if (includeRouteValues)
+                    {
+                        var parameterArray = request.RouteValues.Keys?.Where(
+                            k => k.Contains(KnownHttpRequestProperties.RouteValueParameterSuffix, StringComparison.OrdinalIgnoreCase))
+                            .OrderBy(k => k, StringComparer.OrdinalIgnoreCase)
+                            .ToArray();
+                        if (parameterArray != null && parameterArray.Any())
+                        {
+                            name += $" [{string.Join("/", parameterArray)}]";
+                        }
+                    }
+                }
+
+                return $"{request.Method} {name}".Trim();
+            }
+
+            return null;
+        }
+    }
+}
diff --git a/src/Microsoft.Health.Fhir.Shared.Api/Features/Exceptions/BaseExceptionMiddleware.cs b/src/Microsoft.Health.Fhir.Shared.Api/Features/Exceptions/BaseExceptionMiddleware.cs
@@ -14,10 +14,12 @@
 using Microsoft.Extensions.Logging;
 using Microsoft.Health.Abstractions.Exceptions;
 using Microsoft.Health.Core.Features.Context;
+using Microsoft.Health.Fhir.Api.Extensions;
 using Microsoft.Health.Fhir.Api.Features.ActionResults;
 using Microsoft.Health.Fhir.Api.Features.ContentTypes;
 using Microsoft.Health.Fhir.Api.Features.Formatters;
 using Microsoft.Health.Fhir.Core.Features.Context;
+using Microsoft.Health.Fhir.Core.Logging.Metrics;
 using Task = System.Threading.Tasks.Task;
 
 namespace Microsoft.Health.Fhir.Api.Features.Exceptions
@@ -26,28 +28,33 @@ public class BaseExceptionMiddleware
     {
         private readonly RequestDelegate _next;
         private readonly ILogger<BaseExceptionMiddleware> _logger;
+        private readonly IFailureMetricHandler _failureMetricHandler;
         private readonly RequestContextAccessor<IFhirRequestContext> _fhirRequestContextAccessor;
         private readonly IFormatParametersValidator _parametersValidator;
 
         public BaseExceptionMiddleware(
             RequestDelegate next,
             ILogger<BaseExceptionMiddleware> logger,
+            IFailureMetricHandler failureMetricHandler,
             RequestContextAccessor<IFhirRequestContext> fhirRequestContextAccessor,
             IFormatParametersValidator parametersValidator)
         {
             EnsureArg.IsNotNull(next, nameof(next));
             EnsureArg.IsNotNull(logger, nameof(logger));
+            EnsureArg.IsNotNull(failureMetricHandler, nameof(failureMetricHandler));
             EnsureArg.IsNotNull(fhirRequestContextAccessor, nameof(fhirRequestContextAccessor));
             EnsureArg.IsNotNull(parametersValidator, nameof(parametersValidator));
 
             _next = next;
             _logger = logger;
+            _failureMetricHandler = failureMetricHandler;
             _fhirRequestContextAccessor = fhirRequestContextAccessor;
             _parametersValidator = parametersValidator;
         }
 
         public async Task Invoke(HttpContext context)
         {
+            bool doesOperationOutcomeHaveError = false;
             try
             {
                 await _next(context);
@@ -105,13 +112,30 @@ public async Task Invoke(HttpContext context)
                     operationOutcome,
                     exception is ServiceUnavailableException ? HttpStatusCode.ServiceUnavailable : HttpStatusCode.InternalServerError);
 
+                doesOperationOutcomeHaveError = true;
+
                 await ExecuteResultAsync(context, result);
             }
+            finally
+            {
+                EmitHttpFailureMetricInCaseOfError(context, doesOperationOutcomeHaveError);
+            }
         }
 
         protected internal virtual async Task ExecuteResultAsync(HttpContext context, IActionResult result)
         {
             await result.ExecuteResultAsync(new ActionContext { HttpContext = context });
         }
+
+        private void EmitHttpFailureMetricInCaseOfError(HttpContext context, bool doesOperationOutcomeHaveError)
+        {
+            if (context.Response.StatusCode >= 500 || doesOperationOutcomeHaveError)
+            {
+                string operationName = context.Request.GetOperationName(includeRouteValues: false);
+
+                _failureMetricHandler.EmitHttpFailure(
+                    new HttpErrorMetricNotification() { OperationName = operationName });
+            }
+        }
     }
 }
diff --git a/src/Microsoft.Health.Fhir.Shared.Api/Microsoft.Health.Fhir.Shared.Api.projitems b/src/Microsoft.Health.Fhir.Shared.Api/Microsoft.Health.Fhir.Shared.Api.projitems
@@ -20,6 +20,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Controllers\FhirController.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Controllers\SearchParameterController.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Controllers\ValidateController.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Extensions\HttpRequestExtension.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Features\ActionConstraints\ConditionalConstraintAttribute.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Features\ActionResults\FhirResult.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Features\ActionResults\MemberMatchResult.cs" />

diff --git a/src/Microsoft.Health.Fhir.Shared.Api/Registration/FhirServerServiceCollectionExtensions.cs b/src/Microsoft.Health.Fhir.Shared.Api/Registration/FhirServerServiceCollectionExtensions.cs
@@ -166,6 +166,7 @@ private static void AddMetricEmitter(IServiceCollection services)
             services.TryAddSingleton<IBundleMetricHandler, DefaultBundleMetricHandler>();
             services.TryAddSingleton<ICrudMetricHandler, DefaultCrudMetricHandler>();
             services.TryAddSingleton<ISearchMetricHandler, DefaultSearchMetricHandler>();
+            services.TryAddSingleton<IFailureMetricHandler, DefaultFailureMetricHandler>();
         }
 
         private class FhirServerBuilder : IFhirServerBuilder