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 ILogger support for TryRefreshAsync in .NET Core package #273

Merged
merged 10 commits into from
Sep 8, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public static IServiceCollection AddAzureAppConfiguration(this IServiceCollectio
throw new ArgumentNullException(nameof(services));
}

services.AddLogging();
services.AddSingleton<IConfigurationRefresherProvider, AzureAppConfigurationRefresherProvider>();
return services;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.Extensions.Configuration.AzureAppConfiguration.Extensions;
using Microsoft.Extensions.Configuration.AzureAppConfiguration.FeatureManagement;
using Microsoft.Extensions.Configuration.AzureAppConfiguration.Models;
using Microsoft.Extensions.Logging;
avanigupta marked this conversation as resolved.
Show resolved Hide resolved
using System;
using System.Collections.Generic;
using System.Linq;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Microsoft.Extensions.Configuration.AzureAppConfiguration.Extensions;
using Microsoft.Extensions.Configuration.AzureAppConfiguration.FeatureManagement;
using Microsoft.Extensions.Configuration.AzureAppConfiguration.Models;
using Microsoft.Extensions.Logging;
using System;
using System.Collections.Generic;
using System.Diagnostics;
Expand All @@ -18,7 +19,6 @@

namespace Microsoft.Extensions.Configuration.AzureAppConfiguration
{

internal class AzureAppConfigurationProvider : ConfigurationProvider, IConfigurationRefresher
{
private bool _optional;
Expand All @@ -41,6 +41,8 @@ internal class AzureAppConfigurationProvider : ConfigurationProvider, IConfigura

// To avoid concurrent network operations, this flag is used to achieve synchronization between multiple threads.
private int _networkOperationsInProgress = 0;
private ILogger _logger;
private ILoggerFactory _loggerFactory;

public Uri AppConfigurationEndpoint
{
Expand Down Expand Up @@ -68,6 +70,19 @@ public Uri AppConfigurationEndpoint
}
}

public ILoggerFactory LoggerFactory
{
get
{
return _loggerFactory;
}
set
zhenlan marked this conversation as resolved.
Show resolved Hide resolved
{
_loggerFactory = value;
_logger = _loggerFactory?.CreateLogger(LoggingConstants.AppConfigRefreshLogCategory);
}
}

public AzureAppConfigurationProvider(ConfigurationClient client, AzureAppConfigurationOptions options, bool optional)
{
_client = client ?? throw new ArgumentNullException(nameof(client));
Expand Down Expand Up @@ -182,11 +197,31 @@ public async Task<bool> TryRefreshAsync()
{
await RefreshAsync().ConfigureAwait(false);
}
catch (Exception e) when (
e is KeyVaultReferenceException ||
e is RequestFailedException ||
((e as AggregateException)?.InnerExceptions?.All(e => e is RequestFailedException) ?? false) ||
e is OperationCanceledException)
catch (RequestFailedException e)
{
if (e.Status == (int)HttpStatusCode.Unauthorized || e.Status == (int)HttpStatusCode.Forbidden)
avanigupta marked this conversation as resolved.
Show resolved Hide resolved
{
_logger?.LogError(e, "Refresh operation failed due to an authentication error.");
zhenlan marked this conversation as resolved.
Show resolved Hide resolved
}
zhenlan marked this conversation as resolved.
Show resolved Hide resolved

return false;
}
catch (AggregateException e) when (e?.InnerExceptions?.All(e => e is RequestFailedException) ?? false)
{
if (e.InnerExceptions.Any(exception => (exception is RequestFailedException ex)
&& (ex.Status == (int)HttpStatusCode.Unauthorized || ex.Status == (int)HttpStatusCode.Forbidden)))
{
_logger?.LogError(e, "Refresh operation failed due to an authentication error.");
}
zhenlan marked this conversation as resolved.
Show resolved Hide resolved

return false;
}
catch (KeyVaultReferenceException e)
{
_logger?.LogError(e, "Refresh operation failed while resolving a Key Vault reference.");
return false;
}
catch (OperationCanceledException)
{
return false;
avanigupta marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
//
using Microsoft.Extensions.Logging;
using System;
using System.Threading.Tasks;

Expand All @@ -11,6 +12,19 @@ internal class AzureAppConfigurationRefresher : IConfigurationRefresher
private AzureAppConfigurationProvider _provider = null;

public Uri AppConfigurationEndpoint { get; private set; } = null;

public ILoggerFactory LoggerFactory {
get
{
ThrowIfNullProvider(nameof(LoggerFactory));
return _provider.LoggerFactory;
}
set
{
ThrowIfNullProvider(nameof(LoggerFactory));
_provider.LoggerFactory = value;
}
}

public void SetProvider(AzureAppConfigurationProvider provider)
{
Expand All @@ -20,7 +34,7 @@ public void SetProvider(AzureAppConfigurationProvider provider)

public async Task RefreshAsync()
{
ThrowIfNullProvider();
ThrowIfNullProvider(nameof(RefreshAsync));
await _provider.RefreshAsync().ConfigureAwait(false);
}

Expand All @@ -36,15 +50,15 @@ public async Task<bool> TryRefreshAsync()

public void SetDirty(TimeSpan? maxDelay)
{
ThrowIfNullProvider();
ThrowIfNullProvider(nameof(SetDirty));
_provider.SetDirty(maxDelay);
}

private void ThrowIfNullProvider()
private void ThrowIfNullProvider(string operation)
{
if (_provider == null)
{
throw new InvalidOperationException("ConfigurationBuilder.Build() must be called before this operation can be performed.");
throw new InvalidOperationException($"ConfigurationBuilder.Build() must be called before {operation} can be accessed.");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
//
using Microsoft.Extensions.Logging;
using System;
using System.Collections.Generic;
using System.Linq;
Expand All @@ -11,7 +12,7 @@ internal class AzureAppConfigurationRefresherProvider : IConfigurationRefresherP
{
public IEnumerable<IConfigurationRefresher> Refreshers { get; }

public AzureAppConfigurationRefresherProvider(IConfiguration configuration)
public AzureAppConfigurationRefresherProvider(IConfiguration configuration, ILoggerFactory _loggerFactory)
{
var configurationRoot = configuration as IConfigurationRoot;
var refreshers = new List<IConfigurationRefresher>();
Expand All @@ -22,6 +23,12 @@ public AzureAppConfigurationRefresherProvider(IConfiguration configuration)
{
if (provider is IConfigurationRefresher refresher)
{
// Use _loggerFactory only if LoggerFactory hasn't been set in AzureAppConfigurationOptions
if (refresher.LoggerFactory == null)
{
refresher.LoggerFactory = _loggerFactory;
}

refreshers.Add(refresher);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
//
namespace Microsoft.Extensions.Configuration.AzureAppConfiguration
{
internal class LoggingConstants
{
public const string AppConfigRefreshLogCategory = "Microsoft.Extensions.Configuration.AzureAppConfiguration.Refresh";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license.
//
using Azure;
using Microsoft.Extensions.Logging;
using System;
using System.Threading.Tasks;

Expand All @@ -17,6 +18,11 @@ public interface IConfigurationRefresher
/// </summary>
Uri AppConfigurationEndpoint { get; }

/// <summary>
/// An <see cref="ILoggerFactory"/> for creating a logger to log errors.
/// </summary>
ILoggerFactory LoggerFactory { get; set; }
avanigupta marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Refreshes the data from App Configuration asynchronously.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
<PackageReference Include="Azure.Security.KeyVault.Secrets" Version="4.0.1" />
<PackageReference Include="Microsoft.Extensions.Configuration" Version="3.1.18" />
<PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="3.1.18" />
<PackageReference Include="Microsoft.Extensions.Logging" Version="3.1.18" />
<PackageReference Include="System.Text.Json" Version="4.6.0" />
</ItemGroup>

Expand Down