diff --git a/src/Microsoft.Health.Fhir.Api/Features/Health/StorageInitializedHealthCheck.cs b/src/Microsoft.Health.Fhir.Api/Features/Health/StorageInitializedHealthCheck.cs index 9b90d7d316..30b50a42b5 100644 --- a/src/Microsoft.Health.Fhir.Api/Features/Health/StorageInitializedHealthCheck.cs +++ b/src/Microsoft.Health.Fhir.Api/Features/Health/StorageInitializedHealthCheck.cs @@ -9,10 +9,9 @@ using EnsureThat; using MediatR; using Microsoft.Extensions.Diagnostics.HealthChecks; -using Microsoft.Health.Core; +using Microsoft.Health.Fhir.Core.Data; using Microsoft.Health.Fhir.Core.Extensions; using Microsoft.Health.Fhir.Core.Messages.Search; -using Microsoft.Health.Fhir.Core.Messages.Storage; namespace Microsoft.Health.Fhir.Api.Features.Health { @@ -45,8 +44,9 @@ public Task CheckHealthAsync(HealthCheckContext context, Canc } // Check if customer-managed key (CMK) is properly set. - var isCMKProperlySet = _databaseStatusReporter.IsCustomerManagerKeyProperlySetAsync(cancellationToken).GetAwaiter().GetResult(); - if (!isCMKProperlySet) + var isCMKProperlySet = _databaseStatusReporter.IsCustomerManagedKeyProperlySetAsync(cancellationToken).GetAwaiter().GetResult(); + DatabaseAvailability databaseAvailability = _databaseStatusReporter.GetDatabaseAvailability(); + if (!isCMKProperlySet || databaseAvailability == DatabaseAvailability.DegradedByCustomerManagedKey) { return Task.FromResult(new HealthCheckResult(HealthStatus.Degraded, DegradedCMKMessage)); } diff --git a/src/Microsoft.Health.Fhir.Core/Data/BaseDatabaseStatusReporter.cs b/src/Microsoft.Health.Fhir.Core/Data/BaseDatabaseStatusReporter.cs new file mode 100644 index 0000000000..f822ccc457 --- /dev/null +++ b/src/Microsoft.Health.Fhir.Core/Data/BaseDatabaseStatusReporter.cs @@ -0,0 +1,37 @@ +// ------------------------------------------------------------------------------------------------- +// 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.Threading; +using System.Threading.Tasks; + +namespace Microsoft.Health.Fhir.Core.Data +{ + public abstract class BaseDatabaseStatusReporter : IDatabaseStatusReporter + { + private long _databaseAvailability; + + protected BaseDatabaseStatusReporter() + { + _databaseAvailability = (long)DatabaseAvailability.Available; + } + + public abstract bool IsCustomerManagedKeyException(Exception exception); + + public abstract Task IsCustomerManagedKeyProperlySetAsync(CancellationToken cancellationToken); + +#pragma warning disable CA1024 // Use properties where appropriate. Justification: Prefer to keep this as a method for consistency with the other methods in this interface. + public DatabaseAvailability GetDatabaseAvailability() + { + return (DatabaseAvailability)Interlocked.Read(ref _databaseAvailability); + } +#pragma warning restore CA1024 + + public void ReportDatabaseAvailability(DatabaseAvailability availability) + { + Interlocked.Exchange(ref _databaseAvailability, (long)availability); + } + } +} diff --git a/src/Microsoft.Health.Fhir.Core/Data/DatabaseAvailability.cs b/src/Microsoft.Health.Fhir.Core/Data/DatabaseAvailability.cs new file mode 100644 index 0000000000..abe2031b8e --- /dev/null +++ b/src/Microsoft.Health.Fhir.Core/Data/DatabaseAvailability.cs @@ -0,0 +1,13 @@ +// ------------------------------------------------------------------------------------------------- +// 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.Data +{ + public enum DatabaseAvailability + { + Available, + DegradedByCustomerManagedKey, + } +} diff --git a/src/Microsoft.Health.Fhir.Api/Features/Health/IDatabaseStatusReporter.cs b/src/Microsoft.Health.Fhir.Core/Data/IDatabaseStatusReporter.cs similarity index 73% rename from src/Microsoft.Health.Fhir.Api/Features/Health/IDatabaseStatusReporter.cs rename to src/Microsoft.Health.Fhir.Core/Data/IDatabaseStatusReporter.cs index 1a4f3f906d..dab5518d2b 100644 --- a/src/Microsoft.Health.Fhir.Api/Features/Health/IDatabaseStatusReporter.cs +++ b/src/Microsoft.Health.Fhir.Core/Data/IDatabaseStatusReporter.cs @@ -3,10 +3,11 @@ // Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. // ------------------------------------------------------------------------------------------------- +using System; using System.Threading; using System.Threading.Tasks; -namespace Microsoft.Health.Fhir.Api.Features.Health +namespace Microsoft.Health.Fhir.Core.Data { /// /// Provides functionality to report the status of Data Store. @@ -18,6 +19,12 @@ public interface IDatabaseStatusReporter /// /// A token to cancel the operation. /// A task that returns true if the key is healthy; otherwise, false. - Task IsCustomerManagerKeyProperlySetAsync(CancellationToken cancellationToken); + Task IsCustomerManagedKeyProperlySetAsync(CancellationToken cancellationToken); + + bool IsCustomerManagedKeyException(Exception exception); + + DatabaseAvailability GetDatabaseAvailability(); + + void ReportDatabaseAvailability(DatabaseAvailability availability); } } diff --git a/src/Microsoft.Health.Fhir.CosmosDb.UnitTests/Features/Health/CosmosDbStatusReporterTests.cs b/src/Microsoft.Health.Fhir.CosmosDb.UnitTests/Features/Health/CosmosDbStatusReporterTests.cs index 2b6bddd097..634e36b0da 100644 --- a/src/Microsoft.Health.Fhir.CosmosDb.UnitTests/Features/Health/CosmosDbStatusReporterTests.cs +++ b/src/Microsoft.Health.Fhir.CosmosDb.UnitTests/Features/Health/CosmosDbStatusReporterTests.cs @@ -25,7 +25,7 @@ public async Task GivenHealthyCustomerKeyHealth_WhenIsCustomerManagerKeyProperly var reporter = new CosmosDbStatusReporter(); // Act - bool result = await reporter.IsCustomerManagerKeyProperlySetAsync(CancellationToken.None); + bool result = await reporter.IsCustomerManagedKeyProperlySetAsync(CancellationToken.None); // Assert Assert.True(result); diff --git a/src/Microsoft.Health.Fhir.CosmosDb/Features/Health/CosmosDbHealthCheck.cs b/src/Microsoft.Health.Fhir.CosmosDb/Features/Health/CosmosDbHealthCheck.cs index c9f334dfd1..b9b82c7110 100644 --- a/src/Microsoft.Health.Fhir.CosmosDb/Features/Health/CosmosDbHealthCheck.cs +++ b/src/Microsoft.Health.Fhir.CosmosDb/Features/Health/CosmosDbHealthCheck.cs @@ -151,7 +151,7 @@ void LogWithDetails(LogLevel logLevel, Exception ex, string message, IEnumerable [attempt, ex.GetType().Name]); } } - catch (CosmosException ex) when (ex.IsCmkClientError()) + catch (CosmosException ex) when (ex.IsCustomerManagedKeyException()) { // Handling CMK errors. LogWithDetails( diff --git a/src/Microsoft.Health.Fhir.CosmosDb/Features/Health/CosmosDbStatusReporter.cs b/src/Microsoft.Health.Fhir.CosmosDb/Features/Health/CosmosDbStatusReporter.cs index 52811040af..73b370fb56 100644 --- a/src/Microsoft.Health.Fhir.CosmosDb/Features/Health/CosmosDbStatusReporter.cs +++ b/src/Microsoft.Health.Fhir.CosmosDb/Features/Health/CosmosDbStatusReporter.cs @@ -3,23 +3,33 @@ // Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. // ------------------------------------------------------------------------------------------------- +using System; using System.Threading; using System.Threading.Tasks; -using Microsoft.Health.Fhir.Api.Features.Health; +using Microsoft.Azure.Cosmos; +using Microsoft.Health.Fhir.Core.Data; +using Microsoft.Health.Fhir.CosmosDb.Features.Storage; namespace Microsoft.Health.Fhir.CosmosDb.Features.Health { /// - /// Cosmos DB implementation of . - /// Always returns healthy status without performing any checks. + /// Cosmos DB implementation of . /// - public class CosmosDbStatusReporter : IDatabaseStatusReporter + public class CosmosDbStatusReporter : BaseDatabaseStatusReporter { - /// - public Task IsCustomerManagerKeyProperlySetAsync(CancellationToken cancellationToken = default) + public CosmosDbStatusReporter() + : base() { - // [WI] to implement: https://microsofthealth.visualstudio.com/Health/_workitems/edit/166817 - return Task.FromResult(true); + } + + public override bool IsCustomerManagedKeyException(Exception exception) + { + return exception is CosmosException cdbe && cdbe.IsCustomerManagedKeyException(); + } + + public override Task IsCustomerManagedKeyProperlySetAsync(CancellationToken cancellationToken = default) + { + return Task.FromResult(GetDatabaseAvailability() != DatabaseAvailability.DegradedByCustomerManagedKey); } } } diff --git a/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/CosmosContainerProvider.cs b/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/CosmosContainerProvider.cs index e6219c6d64..0b52adcbbf 100644 --- a/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/CosmosContainerProvider.cs +++ b/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/CosmosContainerProvider.cs @@ -17,6 +17,7 @@ using Microsoft.Health.Abstractions.Exceptions; using Microsoft.Health.Core; using Microsoft.Health.Extensions.DependencyInjection; +using Microsoft.Health.Fhir.Core.Data; using Microsoft.Health.Fhir.Core.Messages.Storage; using Microsoft.Health.Fhir.CosmosDb.Core.Configs; using Microsoft.Health.Fhir.CosmosDb.Core.Features.Storage; @@ -33,6 +34,7 @@ public class CosmosContainerProvider : IHostedService, IRequireInitializationOnF private const int CollectionSettingsVersion = 3; private readonly ILogger _logger; private readonly IMediator _mediator; + private readonly IDatabaseStatusReporter _databaseStatusReporter; private readonly ICosmosDbDistributedLockFactory _distributedLockFactory; private Lazy _container; private readonly RetryableInitializationOperation _initializationOperation; @@ -50,6 +52,7 @@ public CosmosContainerProvider( IMediator mediator, IEnumerable collectionInitializers, ICosmosDbDistributedLockFactory distributedLockFactory, + IDatabaseStatusReporter databaseStatusReporter, ICosmosClientTestProvider cosmosClientTestProvider) { EnsureArg.IsNotNull(cosmosDataStoreConfiguration, nameof(cosmosDataStoreConfiguration)); @@ -61,11 +64,13 @@ public CosmosContainerProvider( EnsureArg.IsNotNull(collectionDataUpdater, nameof(collectionDataUpdater)); EnsureArg.IsNotNull(retryPolicyFactory, nameof(retryPolicyFactory)); EnsureArg.IsNotNull(mediator, nameof(mediator)); + EnsureArg.IsNotNull(databaseStatusReporter, nameof(databaseStatusReporter)); EnsureArg.IsNotNull(distributedLockFactory, nameof(distributedLockFactory)); _logger = logger; _mediator = mediator; _distributedLockFactory = distributedLockFactory; + _databaseStatusReporter = databaseStatusReporter; string collectionId = collectionConfiguration.Get(Constants.CollectionConfigurationName).CollectionId; _client = cosmosClientInitializer.CreateCosmosClient(cosmosDataStoreConfiguration); @@ -163,6 +168,12 @@ private async Task InitializeDataStoreAsync( { LogLevel logLevel = LogLevel.Critical; _logger.Log(logLevel, ex, "Cosmos DB Database {DatabaseId} Initialization has failed.", cosmosDataStoreConfiguration.DatabaseId); + + if (_databaseStatusReporter.IsCustomerManagedKeyException(ex)) + { + _databaseStatusReporter.ReportDatabaseAvailability(DatabaseAvailability.DegradedByCustomerManagedKey); + } + throw; } } diff --git a/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/CosmosExceptionExtensions.cs b/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/CosmosExceptionExtensions.cs index c3f5175926..796c5dd1af 100644 --- a/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/CosmosExceptionExtensions.cs +++ b/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/CosmosExceptionExtensions.cs @@ -61,7 +61,7 @@ public static class CosmosExceptionExtensions /// /// The exception object /// True iff the error is due to client CMK setting. - public static bool IsCmkClientError(this CosmosException exception) + public static bool IsCustomerManagedKeyException(this CosmosException exception) { return exception.StatusCode == HttpStatusCode.Forbidden && Enum.IsDefined(typeof(KnownCosmosDbCmkSubStatusValueClientIssue), exception.SubStatusCode); } diff --git a/src/Microsoft.Health.Fhir.CosmosDb/Registration/FhirServerBuilderCosmosDbRegistrationExtensions.cs b/src/Microsoft.Health.Fhir.CosmosDb/Registration/FhirServerBuilderCosmosDbRegistrationExtensions.cs index 1f7dc8929d..77691d033e 100644 --- a/src/Microsoft.Health.Fhir.CosmosDb/Registration/FhirServerBuilderCosmosDbRegistrationExtensions.cs +++ b/src/Microsoft.Health.Fhir.CosmosDb/Registration/FhirServerBuilderCosmosDbRegistrationExtensions.cs @@ -14,7 +14,6 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Microsoft.Health.Extensions.DependencyInjection; -using Microsoft.Health.Fhir.Api.Features.Health; using Microsoft.Health.Fhir.Core.Extensions; using Microsoft.Health.Fhir.Core.Features.Operations; using Microsoft.Health.Fhir.Core.Features.Operations.Export; diff --git a/src/Microsoft.Health.Fhir.R4.Core.UnitTests/Features/Validation/ServerProvideProfileValidationTests.cs b/src/Microsoft.Health.Fhir.R4.Core.UnitTests/Features/Validation/ServerProvideProfileValidationTests.cs index 5d0835a027..f1246086f2 100644 --- a/src/Microsoft.Health.Fhir.R4.Core.UnitTests/Features/Validation/ServerProvideProfileValidationTests.cs +++ b/src/Microsoft.Health.Fhir.R4.Core.UnitTests/Features/Validation/ServerProvideProfileValidationTests.cs @@ -15,6 +15,7 @@ using Microsoft.Extensions.Options; using Microsoft.Health.Extensions.DependencyInjection; using Microsoft.Health.Fhir.Core.Configs; +using Microsoft.Health.Fhir.Core.Data; using Microsoft.Health.Fhir.Core.Extensions; using Microsoft.Health.Fhir.Core.Features.Persistence; using Microsoft.Health.Fhir.Core.Features.Search; @@ -61,6 +62,7 @@ public ServerProvideProfileValidationTests() _options, _mediator, _hostApplicationLifetime, + Substitute.For(), NullLogger.Instance); } diff --git a/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Features/Health/StorageInitializedHealthCheckTests.cs b/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Features/Health/StorageInitializedHealthCheckTests.cs index f739e694fa..649e1d3bd5 100644 --- a/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Features/Health/StorageInitializedHealthCheckTests.cs +++ b/src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Features/Health/StorageInitializedHealthCheckTests.cs @@ -7,9 +7,9 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Diagnostics.HealthChecks; +using Microsoft.Health.Fhir.Core.Data; using Microsoft.Health.Fhir.Core.Extensions; using Microsoft.Health.Fhir.Core.Messages.Search; -using Microsoft.Health.Fhir.Core.Messages.Storage; using Microsoft.Health.Fhir.Tests.Common; using Microsoft.Health.Test.Utilities; using NSubstitute; @@ -27,7 +27,8 @@ public class StorageInitializedHealthCheckTests public StorageInitializedHealthCheckTests() { _databaseStatusReporter = Substitute.For(); - _databaseStatusReporter.IsCustomerManagerKeyProperlySetAsync(Arg.Any()).Returns(Task.FromResult(true)); + _databaseStatusReporter.IsCustomerManagedKeyProperlySetAsync(Arg.Any()).Returns(Task.FromResult(true)); + _databaseStatusReporter.GetDatabaseAvailability().Returns(DatabaseAvailability.Available); _sut = new StorageInitializedHealthCheck(_databaseStatusReporter); } @@ -53,6 +54,10 @@ public async Task GivenStorageInitializedHealthCheck_WhenCheckHealthAsync_ThenCh { using (Mock.Property(() => ClockResolver.TimeProvider, new Microsoft.Extensions.Time.Testing.FakeTimeProvider(DateTimeOffset.Now.AddMinutes(5).AddSeconds(1)))) { + // Arrange + _databaseStatusReporter.IsCustomerManagedKeyProperlySetAsync(Arg.Any()).Returns(Task.FromResult(true)); + _databaseStatusReporter.GetDatabaseAvailability().Returns(DatabaseAvailability.Available); + HealthCheckResult result = await _sut.CheckHealthAsync(new HealthCheckContext(), CancellationToken.None); Assert.Equal(HealthStatus.Unhealthy, result.Status); @@ -66,11 +71,30 @@ public async Task GivenUnhealthyCMK_WhenCheckHealthAsyncAfter5Minutes_ThenChange using (Mock.Property(() => ClockResolver.TimeProvider, new Microsoft.Extensions.Time.Testing.FakeTimeProvider(DateTimeOffset.Now.AddMinutes(5).AddSeconds(1)))) { // Arrange - _databaseStatusReporter.IsCustomerManagerKeyProperlySetAsync(Arg.Any()).Returns(Task.FromResult(false)); + _databaseStatusReporter.IsCustomerManagedKeyProperlySetAsync(Arg.Any()).Returns(Task.FromResult(false)); + _databaseStatusReporter.GetDatabaseAvailability().Returns(DatabaseAvailability.Available); HealthCheckResult result = await _sut.CheckHealthAsync(new HealthCheckContext(), CancellationToken.None); Assert.Equal(HealthStatus.Degraded, result.Status); Assert.Contains("The health of the store has degraded. Customer-managed key is not properly set.", result.Description); } } + + [Fact] + public async Task GivenDatabaseNotAvailable_WhenCheckHealthAsync_ThenReturnsDegraded() + { + using (Mock.Property(() => ClockResolver.TimeProvider, new Microsoft.Extensions.Time.Testing.FakeTimeProvider(DateTimeOffset.Now.AddMinutes(5).AddSeconds(1)))) + { + // Arrange + _databaseStatusReporter.IsCustomerManagedKeyProperlySetAsync(Arg.Any()).Returns(Task.FromResult(true)); + _databaseStatusReporter.GetDatabaseAvailability().Returns(DatabaseAvailability.DegradedByCustomerManagedKey); + + // Act + HealthCheckResult result = await _sut.CheckHealthAsync(new HealthCheckContext(), CancellationToken.None); + + // Assert + Assert.Equal(HealthStatus.Degraded, result.Status); + Assert.Contains("The health of the store has degraded. Customer-managed key is not properly set.", result.Description); + } + } } diff --git a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Validation/ServerProvideProfileValidationTests.cs b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Validation/ServerProvideProfileValidationTests.cs index 84d5d45b6d..026534a0b6 100644 --- a/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Validation/ServerProvideProfileValidationTests.cs +++ b/src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Validation/ServerProvideProfileValidationTests.cs @@ -15,6 +15,7 @@ using Microsoft.Extensions.Options; using Microsoft.Health.Extensions.DependencyInjection; using Microsoft.Health.Fhir.Core.Configs; +using Microsoft.Health.Fhir.Core.Data; using Microsoft.Health.Fhir.Core.Extensions; using Microsoft.Health.Fhir.Core.Features.Persistence; using Microsoft.Health.Fhir.Core.Features.Search; @@ -63,6 +64,7 @@ public ServerProvideProfileValidationTests() _options, _mediator, _hostApplicationLifetime, + Substitute.For(), NullLogger.Instance); } diff --git a/src/Microsoft.Health.Fhir.Shared.Core/Features/Validation/ServerProvideProfileValidation.cs b/src/Microsoft.Health.Fhir.Shared.Core/Features/Validation/ServerProvideProfileValidation.cs index 8f7ebb7582..c26438eb1a 100644 --- a/src/Microsoft.Health.Fhir.Shared.Core/Features/Validation/ServerProvideProfileValidation.cs +++ b/src/Microsoft.Health.Fhir.Shared.Core/Features/Validation/ServerProvideProfileValidation.cs @@ -24,6 +24,8 @@ using Microsoft.Health.Core.Extensions; using Microsoft.Health.Extensions.DependencyInjection; using Microsoft.Health.Fhir.Core.Configs; +using Microsoft.Health.Fhir.Core.Data; +using Microsoft.Health.Fhir.Core.Exceptions; using Microsoft.Health.Fhir.Core.Extensions; using Microsoft.Health.Fhir.Core.Features.Search; using Microsoft.Health.Fhir.Core.Features.Storage; @@ -48,6 +50,7 @@ public sealed class ServerProvideProfileValidation : IProvideProfilesForValidati private readonly ValidateOperationConfiguration _validateOperationConfig; private readonly FhirMemoryCache _resourcesByUri; private readonly IMediator _mediator; + private readonly IDatabaseStatusReporter _databaseStatusReporter; private bool _isDisposed = false; private string _mostRecentProfileHash = null; @@ -61,6 +64,7 @@ public ServerProvideProfileValidation( IOptions options, IMediator mediator, IHostApplicationLifetime hostApplicationLifetime, + IDatabaseStatusReporter databaseStatusReporter, ILogger logger) { EnsureArg.IsNotNull(searchServiceFactory, nameof(searchServiceFactory)); @@ -68,11 +72,13 @@ public ServerProvideProfileValidation( EnsureArg.IsNotNull(mediator, nameof(mediator)); EnsureArg.IsNotNull(logger, nameof(logger)); EnsureArg.IsNotNull(hostApplicationLifetime, nameof(hostApplicationLifetime)); + EnsureArg.IsNotNull(databaseStatusReporter, nameof(databaseStatusReporter)); _searchServiceFactory = searchServiceFactory; _expirationTime = DateTime.UtcNow; _validateOperationConfig = options.Value; _mediator = mediator; + _databaseStatusReporter = databaseStatusReporter; _logger = logger; _resourcesByUri = new FhirMemoryCache( @@ -346,7 +352,15 @@ private async Task BackgroundLoop(CancellationToken cancellationToken) } catch (Exception ex) { - _logger.LogError(ex, "Profiles: Background profile status task failed. Elapsed time: {ElapsedTime}ms", stopwatch.ElapsedMilliseconds); + if (_databaseStatusReporter.IsCustomerManagedKeyException(ex)) + { + _databaseStatusReporter.ReportDatabaseAvailability(DatabaseAvailability.DegradedByCustomerManagedKey); + _logger.LogWarning(ex, "Profiles: Background profile status task failed due to customer managed key issue. Database availability set as degraded. Elapsed time: {ElapsedTime}ms", stopwatch.ElapsedMilliseconds); + } + else + { + _logger.LogError(ex, "Profiles: Background profile status task failed. Elapsed time: {ElapsedTime}ms", stopwatch.ElapsedMilliseconds); + } } await Task.Delay(TimeSpan.FromSeconds(_validateOperationConfig.BackgroundProfileStatusCheckIntervalInSeconds), cancellationToken); diff --git a/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/ExceptionExtensionTests.cs b/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/ExceptionExtensionTests.cs index e719246558..a0f9d0c4fc 100644 --- a/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/ExceptionExtensionTests.cs +++ b/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/ExceptionExtensionTests.cs @@ -4,8 +4,10 @@ // ------------------------------------------------------------------------------------------------- using System; +using Microsoft.Data.SqlClient; using Microsoft.Health.Fhir.SqlServer.Features; using Microsoft.Health.Fhir.Tests.Common; +using Microsoft.Health.SqlServer.Features.Storage; using Microsoft.Health.Test.Utilities; using Xunit; @@ -187,5 +189,47 @@ public void GivenExceptionWithRemoteProcedureCallError_WhenIsRetriable_ThenRetur Assert.True(result); } + + [Theory] + [InlineData(SqlErrorCodes.KeyVaultCriticalError)] + [InlineData(SqlErrorCodes.KeyVaultEncounteredError)] + [InlineData(SqlErrorCodes.KeyVaultErrorObtainingInfo)] + public void GivenSqlExceptionWithCmkErrorNumber_WhenCheckingIsCustomerManagedKeyException_ThenReturnsTrue(int sqlCmkErrorCode) + { + SqlException exception = SqlExceptionUtils.CreateSqlException(sqlCmkErrorCode); // CMK error code + Assert.True(exception.IsCustomerManagedKeyException()); + } + + [Fact] + public void GivenSqlExceptionWithDifferentErrorNumber_WhenCheckingIsCustomerManagedKeyException_ThenReturnsFalse() + { + SqlException exception = SqlExceptionUtils.CreateSqlException(1205); // Deadlock error + Assert.False(exception.IsCustomerManagedKeyException()); + } + + [Fact] + public void GivenNullException_WhenCheckingIsCustomerManagedKeyException_ThenReturnsFalse() + { + Exception exception = null; + Assert.False(exception.IsCustomerManagedKeyException()); + } + + [Fact] + public void GivenNonSqlException_WhenCheckingIsCustomerManagedKeyException_ThenReturnsFalse() + { + var exception = new InvalidOperationException("Not a SQL exception"); + Assert.False(exception.IsCustomerManagedKeyException()); + } + + [Theory] + [InlineData("(..) is not accessible due to Azure Key Vault critical error (...)")] + [InlineData("(...) is not accessible due to Azure Key Vault encountered an error (...)")] + [InlineData("(...) is not accessible due to Azure Key Vault error obtaining information (...)")] + + public void GivenSqlExceptionWithKeyVaultErrorMessage_WhenCheckingIsCustomerManagedKeyException_ThenReturnsTrue(string errorMessage) + { + var exception = SqlExceptionUtils.CreateSqlException("Azure Key Vault critical error", new InvalidOperationException(errorMessage)); + Assert.True(exception.IsCustomerManagedKeyException()); + } } } diff --git a/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Health/SqlStatusReporterTest.cs b/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Health/SqlStatusReporterTest.cs index 0f02679936..d4b37c98bf 100644 --- a/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Health/SqlStatusReporterTest.cs +++ b/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Health/SqlStatusReporterTest.cs @@ -37,7 +37,7 @@ public async Task GivenHealthyCustomerKeyHealth_WhenIsCustomerManagerKeyProperly var reporter = new SqlStatusReporter(_customerKeyHealthCache); // Act - bool result = await reporter.IsCustomerManagerKeyProperlySetAsync(CancellationToken.None); + bool result = await reporter.IsCustomerManagedKeyProperlySetAsync(CancellationToken.None); // Assert Assert.True(result); @@ -57,7 +57,7 @@ public async Task GivenUnhealthyCustomerKeyHealth_WhenIsCustomerManagerKeyProper var reporter = new SqlStatusReporter(_customerKeyHealthCache); // Act - bool result = await reporter.IsCustomerManagerKeyProperlySetAsync(CancellationToken.None); + bool result = await reporter.IsCustomerManagedKeyProperlySetAsync(CancellationToken.None); // Assert Assert.False(result); diff --git a/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Storage/SqlExceptionActionProcessorTests.cs b/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Storage/SqlExceptionActionProcessorTests.cs index e577bf33b0..275cceb18c 100644 --- a/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Storage/SqlExceptionActionProcessorTests.cs +++ b/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Storage/SqlExceptionActionProcessorTests.cs @@ -4,14 +4,9 @@ // ------------------------------------------------------------------------------------------------- using System; -using System.Data; -using System.Data.Common; using System.Data.SqlTypes; -using System.Linq; -using System.Reflection; using System.Threading; using System.Threading.Tasks; -using Antlr4.Runtime.Tree; using Microsoft.Data.SqlClient; using Microsoft.Extensions.Logging; using Microsoft.Health.Fhir.Core.Exceptions; @@ -64,7 +59,7 @@ public async Task GivenSqlTruncateException_WhenExecuting_ThenResourceSqlTruncat public async Task GivenSqlException_WhenExecuting_ThenSpecificExceptionIsThrown(int errorCode, Type expectedExceptionType) { // Arrange - var sqlException = CreateSqlException(errorCode); + var sqlException = SqlExceptionUtils.CreateSqlException(errorCode); var processor = new SqlExceptionActionProcessor(_mockLogger); // Act & Assert @@ -85,7 +80,7 @@ await Assert.ThrowsAsync(expectedExceptionType, () => public async Task GivenSqlExceptionWithCmkError_WhenExecuting_ThenCustomerManagedKeyExceptionIsThrown(int errorCode) { // Arrange - var sqlException = CreateSqlException(errorCode); + var sqlException = SqlExceptionUtils.CreateSqlException(errorCode); var processor = new SqlExceptionActionProcessor(_mockLogger); // Act & Assert @@ -104,7 +99,7 @@ public async Task GivenSqlExceptionWithCmkError_WhenExecuting_ThenCustomerManage public async Task GivenSqlExceptionWithUnhandledError_WhenExecuting_ThenResourceSqlExceptionIsThrown() { // Arrange - var sqlException = CreateSqlException(99999); + var sqlException = SqlExceptionUtils.CreateSqlException(99999); var processor = new SqlExceptionActionProcessor(_mockLogger); // Act & Assert @@ -118,60 +113,5 @@ public async Task GivenSqlExceptionWithUnhandledError_WhenExecuting_ThenResource sqlException, $"A {nameof(SqlException)} occurred while executing request"); } - - /// - /// Creates an SqlException with specific error number. - /// This is required as SQLException has no public constructor and cannot be mocked easily. - /// Useful for testing system errors that can't be generated by a user query. - /// - /// Sql exception number - /// sql exception - private static SqlException CreateSqlException(int number) - { - // Create SqlError using reflection - var error = Create( - number, // infoNumber - (byte)0, // errorState - (byte)0, // errorClass - string.Empty, // server - string.Empty, // message - string.Empty, // procedure - 0, // lineNumber - null); // exception - - // Create SqlErrorCollection using reflection - var errorCollection = Create(); - - // Add the error to the collection - typeof(SqlErrorCollection) - .GetMethod("Add", BindingFlags.NonPublic | BindingFlags.Instance) - ?.Invoke(errorCollection, new object[] { error }); - - // Create SqlException using the CreateException static method - var exception = typeof(SqlException) - .GetMethod( - "CreateException", - BindingFlags.NonPublic | BindingFlags.Static, - null, - CallingConventions.ExplicitThis, - new[] { typeof(SqlErrorCollection), typeof(string) }, - new ParameterModifier[] { }) - ?.Invoke(null, new object[] { errorCollection, "7.0.0" }) as SqlException; - - return exception ?? throw new InvalidOperationException("Could not create SqlException"); - } - - private static T Create(params object[] parameters) - { - var constructors = typeof(T).GetConstructors(BindingFlags.NonPublic | BindingFlags.Instance); - var constructor = constructors.FirstOrDefault(ctor => ctor.GetParameters().Length == parameters.Length); - - if (constructor == null) - { - throw new InvalidOperationException($"Could not find constructor for {typeof(T).Name} with {parameters.Length} parameters"); - } - - return (T)constructor.Invoke(parameters); - } } } diff --git a/src/Microsoft.Health.Fhir.SqlServer.UnitTests/SqlExceptionUtils.cs b/src/Microsoft.Health.Fhir.SqlServer.UnitTests/SqlExceptionUtils.cs new file mode 100644 index 0000000000..9a6e03a1db --- /dev/null +++ b/src/Microsoft.Health.Fhir.SqlServer.UnitTests/SqlExceptionUtils.cs @@ -0,0 +1,100 @@ +// ------------------------------------------------------------------------------------------------- +// 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 System.Reflection; +using Microsoft.Data.SqlClient; + +namespace Microsoft.Health.Fhir.SqlServer.UnitTests +{ + internal static class SqlExceptionUtils + { + private const string ReflectionErrorMessage = + "Failed to create SqlException via reflection. This may indicate a breaking change in " + + "Microsoft.Data.SqlClient. Check the internal constructor signatures of SqlException, " + + "SqlErrorCollection, and SqlError classes."; + + /// + /// Creates an SqlException with specific error number. + /// This is required as SQLException has no public constructor and cannot be mocked easily. + /// Useful for testing system errors that can't be generated by a user query. + /// + /// Sql exception number + /// sql exception + public static SqlException CreateSqlException(int number) + { + // Create SqlError using reflection + var error = Create( + number, // infoNumber + (byte)0, // errorState + (byte)0, // errorClass + string.Empty, // server + string.Empty, // message + string.Empty, // procedure + 0, // lineNumber + null); // exception + + // Create SqlErrorCollection using reflection + var errorCollection = Create(); + + // Add the error to the collection + typeof(SqlErrorCollection) + .GetMethod("Add", BindingFlags.NonPublic | BindingFlags.Instance) + ?.Invoke(errorCollection, new object[] { error }); + + // Create SqlException using the CreateException static method + var exception = typeof(SqlException) + .GetMethod( + "CreateException", + BindingFlags.NonPublic | BindingFlags.Static, + null, + CallingConventions.ExplicitThis, + new[] { typeof(SqlErrorCollection), typeof(string) }, + new ParameterModifier[] { }) + ?.Invoke(null, new object[] { errorCollection, "7.0.0" }) as SqlException; + + return exception ?? throw new InvalidOperationException("Could not create SqlException"); + } + + /// + /// Creates a SqlException with the specified message and inner exception. + /// + /// The exception message. + /// The inner exception. + /// A SqlException instance for testing. + /// Thrown when reflection fails due to internal API changes. + public static SqlException CreateSqlException(string message, Exception innerException) + { + var errorCollection = Activator.CreateInstance( + typeof(SqlErrorCollection), + nonPublic: true) as SqlErrorCollection + ?? throw new InvalidOperationException($"{ReflectionErrorMessage} SqlErrorCollection constructor not found."); + + var sqlException = Activator.CreateInstance( + typeof(SqlException), + BindingFlags.NonPublic | BindingFlags.Instance, + binder: null, + args: new object[] { message, errorCollection, innerException, Guid.NewGuid() }, + culture: null) as SqlException + ?? throw new InvalidOperationException($"{ReflectionErrorMessage} SqlException constructor not found."); + + return sqlException; + } + + private static T Create(params object[] parameters) + { + var constructors = typeof(T).GetConstructors(BindingFlags.NonPublic | BindingFlags.Instance); + var constructor = constructors.FirstOrDefault(ctor => ctor.GetParameters().Length == parameters.Length); + + if (constructor == null) + { + throw new InvalidOperationException($"Could not find constructor for {typeof(T).Name} with {parameters.Length} parameters"); + } + + return (T)constructor.Invoke(parameters); + } + } +} diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/ExceptionExtension.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/ExceptionExtension.cs index d907d13e2d..ab00ec3d51 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/ExceptionExtension.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/ExceptionExtension.cs @@ -4,11 +4,46 @@ // ------------------------------------------------------------------------------------------------- using System; using Microsoft.Data.SqlClient; +using Microsoft.Health.SqlServer.Features.Storage; namespace Microsoft.Health.Fhir.SqlServer.Features { public static class ExceptionExtension { + private static readonly int[] _sqlCustomerManagedKeyErrorCodes = new int[] + { + SqlErrorCodes.KeyVaultCriticalError, + SqlErrorCodes.KeyVaultEncounteredError, + SqlErrorCodes.KeyVaultErrorObtainingInfo, + }; + + public static bool IsCustomerManagedKeyException(this Exception exception) + { + if (exception == null) + { + return false; + } + + // Handles SQL Exceptions. + SqlException sqlException = exception as SqlException; + if (sqlException != null && _sqlCustomerManagedKeyErrorCodes.Contains(sqlException.Number)) + { + return true; + } + + // Handles non-SQL Exceptions. + string exceptionAsString = exception.ToString(); + + if (exceptionAsString.Contains("is not accessible due to Azure Key Vault critical error", StringComparison.OrdinalIgnoreCase) || + exceptionAsString.Contains("is not accessible due to Azure Key Vault encountered an error", StringComparison.OrdinalIgnoreCase) || + exceptionAsString.Contains("is not accessible due to Azure Key Vault error obtaining information", StringComparison.OrdinalIgnoreCase)) + { + return true; + } + + return false; + } + public static bool IsRetriable(this Exception e) { var str = e.ToString().ToLowerInvariant(); diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Health/SqlStatusReporter.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Health/SqlStatusReporter.cs index a5600bdfa8..4a323b50db 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Health/SqlStatusReporter.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Health/SqlStatusReporter.cs @@ -9,23 +9,29 @@ using EnsureThat; using Microsoft.Health.Core.Features.Health; using Microsoft.Health.Encryption.Customer.Health; -using Microsoft.Health.Fhir.Api.Features.Health; +using Microsoft.Health.Fhir.Core.Data; namespace Microsoft.Health.Fhir.SqlServer.Features.Health { /// /// SQL implementation of using a ValueCache of CustomerKeyHealth. /// - public class SqlStatusReporter : IDatabaseStatusReporter + public class SqlStatusReporter : BaseDatabaseStatusReporter { private readonly ValueCache _customerKeyHealthCache; public SqlStatusReporter(ValueCache customerKeyHealthCache) + : base() { _customerKeyHealthCache = EnsureArg.IsNotNull(customerKeyHealthCache, nameof(customerKeyHealthCache)); } - public async Task IsCustomerManagerKeyProperlySetAsync(CancellationToken cancellationToken = default) + public override bool IsCustomerManagedKeyException(Exception exception) + { + return exception.IsCustomerManagedKeyException(); + } + + public override async Task IsCustomerManagedKeyProperlySetAsync(CancellationToken cancellationToken = default) { // Check Customer-Managed Key Health - CMK CustomerKeyHealth customerKeyHealth = await IsCustomerManagedKeyHealthyAsync(cancellationToken); diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlExceptionExtensions.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlExceptionExtensions.cs index 95ee99f490..ab5abb676b 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlExceptionExtensions.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlExceptionExtensions.cs @@ -3,10 +3,12 @@ // Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. // ------------------------------------------------------------------------------------------------- +using System; using System.Collections.Generic; using EnsureThat; using Microsoft.Data.SqlClient; using Microsoft.Health.SqlServer.Features.Storage; +using Microsoft.SqlServer.Management.Smo; namespace Microsoft.Health.Fhir.SqlServer.Features.Storage { diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Watchdogs/JobMonitorWatchdog.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Watchdogs/JobMonitorWatchdog.cs index d1ac294a51..f210203dee 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Watchdogs/JobMonitorWatchdog.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Watchdogs/JobMonitorWatchdog.cs @@ -12,6 +12,7 @@ using MediatR; using Microsoft.Data.SqlClient; using Microsoft.Extensions.Logging; +using Microsoft.Health.Fhir.Core.Data; using Microsoft.Health.Fhir.Core.Extensions; using Microsoft.Health.Fhir.Core.Features.Operations; using Microsoft.Health.Fhir.Core.Features.Operations.JobMonitor.Messages; @@ -30,16 +31,19 @@ internal sealed class JobMonitorWatchdog : Watchdog internal const int StaleQueueWarningThresholdSeconds = 600; private readonly ISqlRetryService _sqlRetryService; + private readonly IDatabaseStatusReporter _databaseStatusReporter; private readonly IMediator _mediator; private readonly ILogger _logger; public JobMonitorWatchdog( ISqlRetryService sqlRetryService, ILogger logger, + IDatabaseStatusReporter databaseStatusReporter, IMediator mediator) : base(sqlRetryService, logger) { _sqlRetryService = EnsureArg.IsNotNull(sqlRetryService, nameof(sqlRetryService)); + _databaseStatusReporter = EnsureArg.IsNotNull(databaseStatusReporter, nameof(databaseStatusReporter)); _mediator = EnsureArg.IsNotNull(mediator, nameof(mediator)); _logger = EnsureArg.IsNotNull(logger, nameof(logger)); } @@ -96,9 +100,18 @@ protected override async Task RunWorkAsync(CancellationToken cancellationToken) } catch (Exception ex) { - // Publish is the last statement of the try, so a failure publishes nothing this cycle - // (all-or-nothing, per ADR 2605). Rethrow so FhirTimer records the failing state. - _logger.LogError(ex, "JobMonitorWatchdog: queue metrics were not refreshed this cycle."); + if (_databaseStatusReporter.IsCustomerManagedKeyException(ex)) + { + _databaseStatusReporter.ReportDatabaseAvailability(DatabaseAvailability.DegradedByCustomerManagedKey); + _logger.LogError(ex, "JobMonitorWatchdog: queue metrics were not refreshed this cycle due a CMK error. Database availability set as degraded."); + } + else + { + // Publish is the last statement of the try, so a failure publishes nothing this cycle + // (all-or-nothing, per ADR 2605). Rethrow so FhirTimer records the failing state. + _logger.LogError(ex, "JobMonitorWatchdog: queue metrics were not refreshed this cycle."); + } + throw; } } diff --git a/src/Microsoft.Health.Fhir.SqlServer/Registration/FhirServerBuilderSqlServerRegistrationExtensions.cs b/src/Microsoft.Health.Fhir.SqlServer/Registration/FhirServerBuilderSqlServerRegistrationExtensions.cs index 7577774871..cdfcc426a0 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Registration/FhirServerBuilderSqlServerRegistrationExtensions.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Registration/FhirServerBuilderSqlServerRegistrationExtensions.cs @@ -16,7 +16,6 @@ using Microsoft.Health.Core.Features.Health; using Microsoft.Health.Encryption.Customer.Health; using Microsoft.Health.Extensions.DependencyInjection; -using Microsoft.Health.Fhir.Api.Features.Health; using Microsoft.Health.Fhir.Core.Configs; using Microsoft.Health.Fhir.Core.Extensions; using Microsoft.Health.Fhir.Core.Features.Operations.JobMonitor; diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerWatchdogTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerWatchdogTests.cs index 27965df9fe..f08ae8f36d 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerWatchdogTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerWatchdogTests.cs @@ -15,6 +15,7 @@ using Microsoft.Extensions.Primitives; using Microsoft.Health.Core.Features.Context; using Microsoft.Health.Core.Features.Security; +using Microsoft.Health.Fhir.Core.Data; using Microsoft.Health.Fhir.Core.Extensions; using Microsoft.Health.Fhir.Core.Features.Compartment; using Microsoft.Health.Fhir.Core.Features.Context; @@ -807,9 +808,12 @@ public async Task JobMonitorWatchdog_RunningJobInOneQueue_DoesNotZeroStaleCreate private async Task RunJobMonitorWatchdogUntilPublishAsync(IMediator mediator, Func isPublished) { + var databaseStatusReporter = Substitute.For(); + var wd = new JobMonitorWatchdog( _fixture.SqlRetryService, XUnitLogger.Create(_testOutputHelper), + databaseStatusReporter, mediator) { PeriodSec = 1,