Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new health check for improper behavior #3818

Merged
merged 7 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build/jobs/provision-deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ jobs:

$additionalProperties["SqlServer__DeleteAllDataOnStartup"] = "false"
$additionalProperties["SqlServer__AllowDatabaseCreation"] = "true"
$additionalProperties["CosmosDb__InitialDatabaseThroughput"] = 1500
$additionalProperties["FhirServer__Operations__Import__PollingFrequencyInSeconds"] = 1
$additionalProperties["FhirServer__Operations__Export__PollingFrequencyInSeconds"] = 1
$additionalProperties["ASPNETCORE_FORWARDEDHEADERS_ENABLED"] = "true"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using System.Threading;
using System.Threading.Tasks;
using MediatR;
using Microsoft.Extensions.Diagnostics.HealthChecks;
using Microsoft.Health.Fhir.Core.Features.Health;

namespace Microsoft.Health.Fhir.Api.Features.Health
{
public class ImproperBehaviorHealthCheck : IHealthCheck, INotificationHandler<ImproperBehaviorNotification>
{
private bool _isHealthy = true;
private string _message = string.Empty;

public Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default)
{
if (_isHealthy)
{
return Task.FromResult(HealthCheckResult.Healthy());
}

return Task.FromResult(new HealthCheckResult(HealthStatus.Unhealthy, "Improper server behavior has been detected." + _message));
}

public Task Handle(ImproperBehaviorNotification notification, CancellationToken cancellationToken)
{
_isHealthy = false;
_message += " " + notification.Message;
return Task.CompletedTask;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using System;

namespace Microsoft.Health.Fhir.Core.Exceptions
{
public class InitializationException : Exception
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this inherit from FhirException?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so because it isn't related to FHIR, it is related to the server startup.

{
public InitializationException(string message)
: base(message)
{
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using Microsoft.Health.Fhir.Core.Exceptions;
using Microsoft.Health.Fhir.Core.Extensions;
using Microsoft.Health.Fhir.Core.Features.Definition.BundleWrappers;
using Microsoft.Health.Fhir.Core.Features.Health;
using Microsoft.Health.Fhir.Core.Features.Operations;
using Microsoft.Health.Fhir.Core.Features.Search;
using Microsoft.Health.Fhir.Core.Features.Search.Expressions;
Expand All @@ -40,6 +41,8 @@
private readonly IScopeProvider<ISearchService> _searchServiceFactory;
private readonly ILogger _logger;

private bool _initialized = false;

public SearchParameterDefinitionManager(
IModelInfoProvider modelInfoProvider,
IMediator mediator,
Expand Down Expand Up @@ -83,13 +86,31 @@

public async Task EnsureInitializedAsync(CancellationToken cancellationToken)
{
await LoadSearchParamsFromDataStore(cancellationToken);
try
{
_initialized = true;
await LoadSearchParamsFromDataStore(cancellationToken);

await _mediator.Publish(new SearchParameterDefinitionManagerInitialized(), cancellationToken);
await _mediator.Publish(new SearchParameterDefinitionManagerInitialized(), cancellationToken);
}
catch
{
_initialized = false;
}
Comment on lines +96 to +99

Check notice

Code scanning / CodeQL

Generic catch clause Note

Generic catch clause.
}

public void EnsureInitialized()
{
if (!_initialized)
{
throw new InitializationException("Failed to initialize search parameters");
}
}

public IEnumerable<SearchParameterInfo> GetSearchParameters(string resourceType)
{
EnsureInitialized();

if (TypeLookup.TryGetValue(resourceType, out ConcurrentDictionary<string, SearchParameterInfo> value))
{
return value.Values;
Expand All @@ -100,6 +121,8 @@

public SearchParameterInfo GetSearchParameter(string resourceType, string code)
{
EnsureInitialized();

if (TypeLookup.TryGetValue(resourceType, out ConcurrentDictionary<string, SearchParameterInfo> lookup) &&
lookup.TryGetValue(code, out SearchParameterInfo searchParameter))
{
Expand All @@ -111,6 +134,7 @@

public bool TryGetSearchParameter(string resourceType, string code, out SearchParameterInfo searchParameter)
{
EnsureInitialized();
searchParameter = null;

return TypeLookup.TryGetValue(resourceType, out ConcurrentDictionary<string, SearchParameterInfo> searchParameters) &&
Expand All @@ -119,6 +143,7 @@

public SearchParameterInfo GetSearchParameter(string definitionUri)
{
EnsureInitialized();
if (UrlLookup.TryGetValue(definitionUri, out SearchParameterInfo value))
{
return value;
Expand All @@ -129,11 +154,13 @@

public bool TryGetSearchParameter(string definitionUri, out SearchParameterInfo value)
{
EnsureInitialized();
return UrlLookup.TryGetValue(definitionUri, out value);
}

public string GetSearchParameterHashForResourceType(string resourceType)
{
EnsureInitialized();
EnsureArg.IsNotNullOrWhiteSpace(resourceType, nameof(resourceType));

if (_resourceTypeSearchParameterHashMap.TryGetValue(resourceType, out string hash))
Expand Down Expand Up @@ -214,15 +241,45 @@

public async Task Handle(SearchParametersUpdatedNotification notification, CancellationToken cancellationToken)
{
_logger.LogInformation("SearchParameterDefinitionManager: Search parameters updated");
CalculateSearchParameterHash();
await _mediator.Publish(new RebuildCapabilityStatement(RebuildPart.SearchParameter), cancellationToken);
var retry = 0;
while (retry < 3)
{
try
{
_logger.LogInformation("SearchParameterDefinitionManager: Search parameters updated");
CalculateSearchParameterHash();
await _mediator.Publish(new RebuildCapabilityStatement(RebuildPart.SearchParameter), cancellationToken);
return;
}
catch (Exception ex)
{
_logger.LogError(ex, $"Error calculating search parameter hash. Retry {retry}");
retry++;
}
Comment on lines +254 to +258

Check notice

Code scanning / CodeQL

Generic catch clause Note

Generic catch clause.
}

await _mediator.Publish(new ImproperBehaviorNotification("Error calculating search parameter hash"), cancellationToken);
}

public async Task Handle(StorageInitializedNotification notification, CancellationToken cancellationToken)
{
_logger.LogInformation("SearchParameterDefinitionManager: Storage initialized");
await EnsureInitializedAsync(cancellationToken);
var retry = 0;
while (retry < 3)
{
try
{
_logger.LogInformation("SearchParameterDefinitionManager: Storage initialized");
await EnsureInitializedAsync(cancellationToken);
return;
}
catch (Exception ex)
{
_logger.LogError(ex, $"Error initializing search parameters. Retry {retry}");
retry++;
}
Comment on lines +275 to +279

Check notice

Code scanning / CodeQL

Generic catch clause Note

Generic catch clause.
}

await _mediator.Publish(new ImproperBehaviorNotification("Error initializing search parameters"), cancellationToken);
}

private async Task LoadSearchParamsFromDataStore(CancellationToken cancellationToken)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using EnsureThat;
using MediatR;

namespace Microsoft.Health.Fhir.Core.Features.Health
{
public class ImproperBehaviorNotification : INotification
{
public ImproperBehaviorNotification(string message)
{
EnsureArg.IsNotNull(message, nameof(message));

Message = message;
}

public string Message { get; }
}
}
13 changes: 13 additions & 0 deletions src/Microsoft.Health.Fhir.Shared.Api/Modules/FhirModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,22 @@
using Microsoft.AspNetCore.Mvc.Formatters;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Diagnostics.HealthChecks;
using Microsoft.Extensions.Options;
using Microsoft.Health.Core.Features.Context;
using Microsoft.Health.Core.Features.Security;
using Microsoft.Health.Extensions.DependencyInjection;
using Microsoft.Health.Fhir.Api.Features.Context;
using Microsoft.Health.Fhir.Api.Features.Filters;
using Microsoft.Health.Fhir.Api.Features.Formatters;
using Microsoft.Health.Fhir.Api.Features.Health;
using Microsoft.Health.Fhir.Api.Features.Resources;
using Microsoft.Health.Fhir.Api.Features.Resources.Bundle;
using Microsoft.Health.Fhir.Core.Extensions;
using Microsoft.Health.Fhir.Core.Features;
using Microsoft.Health.Fhir.Core.Features.Conformance;
using Microsoft.Health.Fhir.Core.Features.Context;
using Microsoft.Health.Fhir.Core.Features.Health;
using Microsoft.Health.Fhir.Core.Features.Operations;
using Microsoft.Health.Fhir.Core.Features.Persistence;
using Microsoft.Health.Fhir.Core.Features.Security;
Expand Down Expand Up @@ -163,6 +166,16 @@ ResourceElement SetMetadata(Resource resource, string versionId, DateTimeOffset
// Register a router for Bundle requests.
services.AddSingleton<IRouter, BundleRouter>();

// Registers a health check for improper behavior
services.RemoveServiceTypeExact<ImproperBehaviorHealthCheck, INotificationHandler<ImproperBehaviorNotification>>()
.Add<ImproperBehaviorHealthCheck>()
.Singleton()
.AsSelf()
.AsService<IHealthCheck>()
.AsService<INotificationHandler<ImproperBehaviorNotification>>();

services.AddHealthChecks().AddCheck<ImproperBehaviorHealthCheck>(name: "BehaviorHealthCheck");

services.AddLazy();
services.AddScoped();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ public class SearchOptionsFactory : ISearchOptionsFactory
private readonly ExpressionAccessControl _expressionAccess;
private readonly ISearchParameterDefinitionManager _searchParameterDefinitionManager;
private readonly ILogger _logger;
private readonly SearchParameterInfo _resourceTypeSearchParameter;
private readonly CoreFeatureConfiguration _featureConfiguration;
private SearchParameterInfo _resourceTypeSearchParameter;
private readonly HashSet<string> _queryHintParameterNames = new() { KnownQueryParameterNames.GlobalEndSurrogateId, KnownQueryParameterNames.EndSurrogateId, KnownQueryParameterNames.GlobalStartSurrogateId, KnownQueryParameterNames.StartSurrogateId };

public SearchOptionsFactory(
Expand All @@ -66,8 +66,19 @@ public SearchOptionsFactory(
_searchParameterDefinitionManager = searchParameterDefinitionManagerResolver();
_logger = logger;
_featureConfiguration = featureConfiguration.Value;
}

_resourceTypeSearchParameter = _searchParameterDefinitionManager.GetSearchParameter(ResourceType.Resource.ToString(), SearchParameterNames.ResourceType);
private SearchParameterInfo ResourceTypeSearchParameter
{
get
{
if (_resourceTypeSearchParameter == null)
{
_resourceTypeSearchParameter = _searchParameterDefinitionManager.GetSearchParameter(ResourceType.Resource.ToString(), SearchParameterNames.ResourceType);
}

return _resourceTypeSearchParameter;
}
}

public SearchOptions Create(string resourceType, IReadOnlyList<Tuple<string, string>> queryParameters, bool isAsyncOperation = false, ResourceVersionType resourceVersionTypes = ResourceVersionType.Latest, bool onlyIds = false)
Expand Down Expand Up @@ -313,7 +324,7 @@ public SearchOptions Create(
throw new ResourceNotSupportedException(resourceType);
}

searchExpressions.Add(Expression.SearchParameter(_resourceTypeSearchParameter, Expression.StringEquals(FieldName.TokenCode, null, resourceType, false)));
searchExpressions.Add(Expression.SearchParameter(ResourceTypeSearchParameter, Expression.StringEquals(FieldName.TokenCode, null, resourceType, false)));
}

CheckFineGrainedAccessControl(searchExpressions);
Expand Down Expand Up @@ -676,11 +687,11 @@ private void CheckFineGrainedAccessControl(List<Expression> searchExpressions)
{
if (clinicalScopeResources.Any())
{
searchExpressions.Add(Expression.SearchParameter(_resourceTypeSearchParameter, Expression.In(FieldName.TokenCode, null, clinicalScopeResources)));
searchExpressions.Add(Expression.SearchParameter(ResourceTypeSearchParameter, Expression.In(FieldName.TokenCode, null, clinicalScopeResources)));
}
else // block all queries
{
searchExpressions.Add(Expression.SearchParameter(_resourceTypeSearchParameter, Expression.StringEquals(FieldName.TokenCode, null, "none", false)));
searchExpressions.Add(Expression.SearchParameter(ResourceTypeSearchParameter, Expression.StringEquals(FieldName.TokenCode, null, "none", false)));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ internal class PartitionEliminationRewriter : SqlExpressionRewriterWithInitialCo
{
private readonly ISqlServerFhirModel _model;
private readonly SchemaInformation _schemaInformation;
private readonly SearchParameterInfo _resourceTypeSearchParameter;
private readonly ISearchParameterDefinitionManager.SearchableSearchParameterDefinitionManagerResolver _searchParameterDefinitionManagerResolver;
private SearchParameterInfo _resourceTypeSearchParameter;
private SearchParameterExpression _allTypesExpression;

public PartitionEliminationRewriter(
Expand All @@ -42,7 +43,20 @@ public PartitionEliminationRewriter(

_model = model;
_schemaInformation = schemaInformation;
_resourceTypeSearchParameter = searchParameterDefinitionManagerResolver.Invoke().GetSearchParameter(KnownResourceTypes.Resource, SearchParameterNames.ResourceType);
_searchParameterDefinitionManagerResolver = searchParameterDefinitionManagerResolver;
}

private SearchParameterInfo ResourceTypeSearchParameter
{
get
{
if (_resourceTypeSearchParameter == null)
{
_resourceTypeSearchParameter = _searchParameterDefinitionManagerResolver.Invoke().GetSearchParameter(KnownResourceTypes.Resource, SearchParameterNames.ResourceType);
}

return _resourceTypeSearchParameter;
}
}

private SearchParameterExpression GetAllTypesExpression()
Expand All @@ -58,7 +72,7 @@ private SearchParameterExpression GetAllTypesExpression()
resourceTypes[i] = _model.GetResourceTypeName(typeId);
}

_allTypesExpression = Expression.SearchParameter(_resourceTypeSearchParameter, Expression.In(FieldName.TokenCode, null, resourceTypes));
_allTypesExpression = Expression.SearchParameter(ResourceTypeSearchParameter, Expression.In(FieldName.TokenCode, null, resourceTypes));

return _allTypesExpression;
}
Expand All @@ -83,7 +97,7 @@ public override Expression VisitSqlRoot(SqlRootExpression expression, object con
{
primaryKeyValueIndex = i;
}
else if (ReferenceEquals(parameter, _resourceTypeSearchParameter))
else if (ReferenceEquals(parameter, ResourceTypeSearchParameter))
{
hasTypeRestriction = true;
}
Expand All @@ -96,7 +110,7 @@ public override Expression VisitSqlRoot(SqlRootExpression expression, object con
{
for (var i = 0; i < expression.SearchParamTableExpressions.Count; i++)
{
if (expression.SearchParamTableExpressions[i].ToString().Contains(_resourceTypeSearchParameter.Code, StringComparison.Ordinal))
if (expression.SearchParamTableExpressions[i].ToString().Contains(ResourceTypeSearchParameter.Code, StringComparison.Ordinal))
{
needTypeRestriction = true;
}
Expand Down
Loading
Loading