Fix and improve logging (#580)

Resolve #579.
dotnet · Jan 30, 2023 · 7571e9a · 7571e9a
1 parent dd84559
commit 7571e9a
Show file tree

Hide file tree

Showing 8 changed files with 100 additions and 10 deletions.
diff --git a/Directory.Packages.props b/Directory.Packages.props
@@ -8,6 +8,8 @@
     <PackageVersion Include="Azure.Security.KeyVault.Certificates" Version="4.4.0" />
     <PackageVersion Include="AzureSign.Core" Version="4.0.1" />
     <PackageVersion Include="coverlet.collector" Version="3.1.2" />
+    <PackageVersion Include="Microsoft.Extensions.Configuration.EnvironmentVariables" Version="7.0.0" />
+    <PackageVersion Include="Microsoft.Extensions.Configuration.Json" Version="7.0.0" />
     <PackageVersion Include="Microsoft.Extensions.FileSystemGlobbing" Version="7.0.0" />
     <PackageVersion Include="Microsoft.Extensions.Logging" Version="7.0.0" />
     <PackageVersion Include="Microsoft.Extensions.Logging.Console" Version="7.0.0" />

diff --git a/src/Sign.Core/KeyVault/KeyVaultService.cs b/src/Sign.Core/KeyVault/KeyVaultService.cs
@@ -2,21 +2,32 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE.txt file in the project root for more information.
 
+using System.Diagnostics;
 using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
 using Azure;
 using Azure.Core;
 using Azure.Security.KeyVault.Certificates;
+using Microsoft.Extensions.Logging;
 using RSAKeyVaultProvider;
 
 namespace Sign.Core
 {
     internal sealed class KeyVaultService : IKeyVaultService
     {
         private Uri? _keyVaultUrl;
+        private readonly ILogger<IKeyVaultService> _logger;
         private Task<KeyVaultCertificateWithPolicy>? _task;
         private TokenCredential? _tokenCredential;
 
+        // Dependency injection requires a public constructor.
+        public KeyVaultService(ILogger<IKeyVaultService> logger)
+        {
+            ArgumentNullException.ThrowIfNull(logger, nameof(logger));
+
+            _logger = logger;
+        }
+
         public async Task<X509Certificate2> GetCertificateAsync()
         {
             ThrowIfUninitialized();
@@ -54,15 +65,21 @@ public void Initialize(Uri keyVaultUrl, TokenCredential tokenCredential, string
             _task = GetKeyVaultCertificateAsync(keyVaultUrl, tokenCredential, certificateName);
         }
 
-        private static async Task<KeyVaultCertificateWithPolicy> GetKeyVaultCertificateAsync(
+        private async Task<KeyVaultCertificateWithPolicy> GetKeyVaultCertificateAsync(
             Uri keyVaultUrl,
             TokenCredential tokenCredential,
             string certificateName)
         {
+            Stopwatch stopwatch = Stopwatch.StartNew();
+
+            _logger.LogTrace(Resources.FetchingCertificate);
+
             CertificateClient client = new(keyVaultUrl, tokenCredential);
             Response<KeyVaultCertificateWithPolicy>? response =
                 await client.GetCertificateAsync(certificateName).ConfigureAwait(false);
 
+            _logger.LogTrace(Resources.FetchedCertificate, stopwatch.Elapsed.TotalMilliseconds);
+
             return response.Value;
         }
 

diff --git a/src/Sign.Core/Resources.Designer.cs b/src/Sign.Core/Resources.Designer.cs
diff --git a/src/Sign.Core/Resources.resx b/src/Sign.Core/Resources.resx
@@ -166,6 +166,13 @@
     <value>An exception occurred while attempting to delete directory {path}.</value>
     <comment>Do not localize value inside {}.</comment>
   </data>
+  <data name="FetchedCertificate" xml:space="preserve">
+    <value>Fetched certificate. [{milliseconds} ms]</value>
+    <comment>Do not localize value inside {}.</comment>
+  </data>
+  <data name="FetchingCertificate" xml:space="preserve">
+    <value>Fetching certificate from Azure Key Vault.</value>
+  </data>
   <data name="OpeningContainer" xml:space="preserve">
     <value>Extracting container {ContainerFilePath} to {DirectoryPath}.</value>
     <comment>Do not localize value inside {}.</comment>

diff --git a/src/Sign.Core/ServiceProvider.cs b/src/Sign.Core/ServiceProvider.cs
@@ -2,8 +2,10 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE.txt file in the project root for more information.
 
+using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Configuration;
 
 namespace Sign.Core
 {
@@ -27,11 +29,21 @@ public ServiceProvider(IServiceProvider serviceProvider)
         internal static ServiceProvider CreateDefault(LogLevel logLevel = LogLevel.Information)
         {
             IServiceCollection services = new ServiceCollection();
+            IConfigurationBuilder configurationBuilder = new ConfigurationBuilder();
+            AppRootDirectoryLocator locator = new();
+
+            configurationBuilder.SetBasePath(locator.Directory.FullName)
+                .AddJsonFile("appsettings.json", optional: true, reloadOnChange: false)
+                .AddEnvironmentVariables();
+
+            IConfiguration configuration = configurationBuilder.Build();
+            IConfigurationSection loggingSection = configuration.GetSection("Logging");
 
             services.AddLogging(builder =>
             {
-                builder.AddConsole();
-                builder.SetMinimumLevel(logLevel);
+                builder.SetMinimumLevel(logLevel)
+                    .AddConfiguration(loggingSection)
+                    .AddConsole();
             });
 
             services.AddSingleton<IAppRootDirectoryLocator, AppRootDirectoryLocator>();

diff --git a/src/Sign.Core/Sign.Core.csproj b/src/Sign.Core/Sign.Core.csproj
@@ -9,6 +9,8 @@
     <PackageReference Include="Azure.Identity" />
     <PackageReference Include="Azure.Security.KeyVault.Certificates" PrivateAssets="analyzers;build;compile;contentfiles" />
     <PackageReference Include="AzureSign.Core" PrivateAssets="analyzers;build;compile;contentfiles" />
+    <PackageReference Include="Microsoft.Extensions.Configuration.EnvironmentVariables" />
+    <PackageReference Include="Microsoft.Extensions.Configuration.Json" />
     <PackageReference Include="Microsoft.Extensions.FileSystemGlobbing" />
     <PackageReference Include="Microsoft.Extensions.Logging" />
     <PackageReference Include="Microsoft.Extensions.Logging.Console" PrivateAssets="analyzers;build;compile;contentfiles" />

diff --git a/src/Sign.Core/Signer.cs b/src/Sign.Core/Signer.cs
@@ -16,13 +16,16 @@ namespace Sign.Core
     internal sealed class Signer : ISigner
     {
         private readonly IServiceProvider _serviceProvider;
+        private readonly ILogger<ISigner> _logger;
 
         // Dependency injection requires a public constructor.
-        public Signer(IServiceProvider serviceProvider)
+        public Signer(IServiceProvider serviceProvider, ILogger<ISigner> logger)
         {
             ArgumentNullException.ThrowIfNull(serviceProvider, nameof(serviceProvider));
+            ArgumentNullException.ThrowIfNull(logger, nameof(logger));
 
             _serviceProvider = serviceProvider;
+            _logger = logger;
         }
 
         public async Task<int> SignAsync(
@@ -42,7 +45,6 @@ public async Task<int> SignAsync(
             string certificateName)
         {
             IAggregatingSignatureProvider signatureProvider = _serviceProvider.GetRequiredService<IAggregatingSignatureProvider>();
-            ILogger<Signer> logger = _serviceProvider.GetRequiredService<ILogger<Signer>>();
             IDirectoryService directoryService = _serviceProvider.GetRequiredService<IDirectoryService>();
             ParallelOptions parallelOptions = new() { MaxDegreeOfParallelism = maxConcurrency };
 
@@ -128,7 +130,7 @@ await Parallel.ForEachAsync(inputFiles, parallelOptions, async (input, token) =>
 
                     //Do action
 
-                    logger.LogInformation(Resources.SubmittingFileForSigning, input.FullName);
+                    _logger.LogInformation(Resources.SubmittingFileForSigning, input.FullName);
 
                     // this might have two files, one containing the file list
                     // The first will be the package and the second is the filter
@@ -142,7 +144,7 @@ await Parallel.ForEachAsync(inputFiles, parallelOptions, async (input, token) =>
                             inputFileName = Path.ChangeExtension(inputFileName, input.Extension);
                         }
 
-                        logger.LogInformation(Resources.SignAsyncCalled, input.FullName, inputFileName);
+                        _logger.LogInformation(Resources.SignAsyncCalled, input.FullName, inputFileName);
 
                         if (input.Length > 0)
                         {
@@ -156,18 +158,18 @@ await Parallel.ForEachAsync(inputFiles, parallelOptions, async (input, token) =>
                         fi.CopyTo(output.FullName, overwrite: true);
                     }
 
-                    logger.LogInformation(Resources.SigningSucceededWithTimeElapsed, output.FullName, sw.ElapsedMilliseconds);
+                    _logger.LogInformation(Resources.SigningSucceededWithTimeElapsed, output.FullName, sw.ElapsedMilliseconds);
                 });
 
             }
             catch (AuthenticationException e)
             {
-                logger.LogError(e, e.Message);
+                _logger.LogError(e, e.Message);
                 return ExitCode.Failed;
             }
             catch (Exception e)
             {
-                logger.LogError(e, e.Message);
+                _logger.LogError(e, e.Message);
                 return ExitCode.Failed;
             }
 

diff --git a/test/Sign.Core.Test/SignerTests.cs b/test/Sign.Core.Test/SignerTests.cs
@@ -0,0 +1,30 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE.txt file in the project root for more information.
+
+using Microsoft.Extensions.Logging;
+using Moq;
+
+namespace Sign.Core.Test
+{
+    public class SignerTests
+    {
+        [Fact]
+        public void Constructor_WhenServiceProviderIsNull_Throws()
+        {
+            ArgumentNullException exception = Assert.Throws<ArgumentNullException>(
+                () => new Signer(serviceProvider: null!, Mock.Of<ILogger<ISigner>>()));
+
+            Assert.Equal("serviceProvider", exception.ParamName);
+        }
+
+        [Fact]
+        public void Constructor_WhenLoggerIsNull_Throws()
+        {
+            ArgumentNullException exception = Assert.Throws<ArgumentNullException>(
+                () => new Signer(Mock.Of<IServiceProvider>(), logger: null!));
+
+            Assert.Equal("logger", exception.ParamName);
+        }
+    }
+}