diff --git a/build/common.props b/build/common.props index afa45088b8..2c3160a332 100644 --- a/build/common.props +++ b/build/common.props @@ -5,7 +5,7 @@ latest 4 27 - 5 + 7 0 diff --git a/src/WebJobs.Script.WebHost/Diagnostics/DiagnosticEventNullRepository.cs b/src/WebJobs.Script.WebHost/Diagnostics/DiagnosticEventNullRepository.cs new file mode 100644 index 0000000000..c1034fc001 --- /dev/null +++ b/src/WebJobs.Script.WebHost/Diagnostics/DiagnosticEventNullRepository.cs @@ -0,0 +1,20 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +using System; +using System.Collections.Concurrent; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Azure.Cosmos.Table; +using Microsoft.Azure.WebJobs.Host.Executors; +using Microsoft.Azure.WebJobs.Script.WebHost.Helpers; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging; + +namespace Microsoft.Azure.WebJobs.Script.WebHost.Diagnostics +{ + public class DiagnosticEventNullRepository : IDiagnosticEventRepository + { + public void WriteDiagnosticEvent(DateTime timestamp, string errorCode, LogLevel level, string message, string helpLink, Exception exception) { } + } +} \ No newline at end of file diff --git a/src/WebJobs.Script.WebHost/Diagnostics/DiagnosticEventRepositoryFactory.cs b/src/WebJobs.Script.WebHost/Diagnostics/DiagnosticEventRepositoryFactory.cs index 179c7a955c..3b269256d0 100644 --- a/src/WebJobs.Script.WebHost/Diagnostics/DiagnosticEventRepositoryFactory.cs +++ b/src/WebJobs.Script.WebHost/Diagnostics/DiagnosticEventRepositoryFactory.cs @@ -2,24 +2,37 @@ // Licensed under the MIT License. See License.txt in the project root for license information. using System; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; namespace Microsoft.Azure.WebJobs.Script.WebHost.Diagnostics { public class DiagnosticEventRepositoryFactory : IDiagnosticEventRepositoryFactory { private readonly IServiceProvider _serviceProvider; + private readonly IConfiguration _configuration; - public DiagnosticEventRepositoryFactory(IServiceProvider serviceProvider) + public DiagnosticEventRepositoryFactory(IServiceProvider serviceProvider, IConfiguration configuration) { _serviceProvider = serviceProvider; + _configuration = configuration; } public IDiagnosticEventRepository Create() { // Using this to break ciruclar dependency for ILoggers. Typically you cannot log errors within the logging pipeline because it creates infinte loop. // However in this case that loop is broken because of the filtering in the DiagnosticEventLogger - return _serviceProvider.GetRequiredService(); + + string storageConnectionString = _configuration.GetWebJobsConnectionString(ConnectionStringNames.Storage); + if (string.IsNullOrEmpty(storageConnectionString)) + { + return new DiagnosticEventNullRepository(); + } + else + { + return _serviceProvider.GetRequiredService(); + } } } } \ No newline at end of file diff --git a/src/WebJobs.Script.WebHost/Diagnostics/DiagnosticEventTableStorageRepository.cs b/src/WebJobs.Script.WebHost/Diagnostics/DiagnosticEventTableStorageRepository.cs index 4f6c4693a2..80507b63ba 100644 --- a/src/WebJobs.Script.WebHost/Diagnostics/DiagnosticEventTableStorageRepository.cs +++ b/src/WebJobs.Script.WebHost/Diagnostics/DiagnosticEventTableStorageRepository.cs @@ -53,16 +53,16 @@ internal CloudTableClient TableClient if (!_environment.IsPlaceholderModeEnabled() && _tableClient == null) { string storageConnectionString = _configuration.GetWebJobsConnectionString(ConnectionStringNames.Storage); - if (string.IsNullOrEmpty(storageConnectionString)) - { - _logger.LogError("Azure Storage connection string is empty or invalid. Unable to write diagnostic events."); - } - - if (CloudStorageAccount.TryParse(storageConnectionString, out CloudStorageAccount account)) + if (!string.IsNullOrEmpty(storageConnectionString) + && CloudStorageAccount.TryParse(storageConnectionString, out CloudStorageAccount account)) { var tableClientConfig = new TableClientConfiguration(); _tableClient = new CloudTableClient(account.TableStorageUri, account.Credentials, tableClientConfig); } + else + { + _logger.LogError("Azure Storage connection string is empty or invalid. Unable to write diagnostic events."); + } } return _tableClient; @@ -124,10 +124,17 @@ internal virtual async Task FlushLogs(CloudTable table = null) return; } - table = table ?? GetDiagnosticEventsTable(); - try { + table = table ?? GetDiagnosticEventsTable(); + + if (table == null) + { + _logger.LogError("Unable to get table reference. Aborting write operation"); + StopTimer(); + return; + } + bool tableCreated = await TableStorageHelpers.CreateIfNotExistsAsync(table, _tableCreationRetries); if (tableCreated) { @@ -136,11 +143,13 @@ internal virtual async Task FlushLogs(CloudTable table = null) } catch (Exception ex) { - _logger.LogError(ex, $"Unable to create table '{table.Name}' after {_tableCreationRetries} retries. Aborting write operation {ex}"); - + _logger.LogError(ex, $"Unable to get table reference or create table. Aborting write operation."); + return; + } + finally + { // Clearing the memory cache to avoid memory build up. _events.Clear(); - return; } // Assigning a new empty directory to reset the event count in the new duration window. @@ -174,9 +183,8 @@ internal async Task ExecuteBatchAsync(ConcurrentDictionary [Fact] public async Task FlushLogs_LogsErrorAndClearsEvents_WhenTableCreatingFails() { - // Clear events by flush logs if table creation attempts fail + // Arrange IEnvironment testEnvironment = new TestEnvironment(); testEnvironment.SetEnvironmentVariable(EnvironmentSettingNames.AzureWebsitePlaceholderMode, "0"); - DiagnosticEventTableStorageRepository repository = - new DiagnosticEventTableStorageRepository(_configuration, _hostIdProvider, testEnvironment, _logger); + var testData = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "AzureWebJobsStorage", null } + }; - var tableClient = repository.TableClient; - var cloudTable = tableClient.GetTableReference("aa"); + var configuration = new ConfigurationBuilder() + .AddEnvironmentVariables() + .AddInMemoryCollection(testData) + .Build(); - repository.WriteDiagnosticEvent(DateTime.UtcNow, "eh1", LogLevel.Information, "This is the message", "https://fwlink/", new Exception("exception message")); + DiagnosticEventTableStorageRepository repository = + new DiagnosticEventTableStorageRepository(configuration, _hostIdProvider, testEnvironment, _logger); - Assert.Equal(1, repository.Events.Values.Count()); + // Act + repository.WriteDiagnosticEvent(DateTime.UtcNow, "eh1", LogLevel.Information, "This is the message", "https://fwlink/", new Exception("exception message")); + await repository.FlushLogs(); - await repository.FlushLogs(cloudTable); + // Assert + var logMessage = _loggerProvider.GetAllLogMessages().SingleOrDefault(m => m.FormattedMessage.Contains("Unable to get table reference")); + Assert.NotNull(logMessage); + var messagePresent = _loggerProvider.GetAllLogMessages().Any(m => m.FormattedMessage.Contains("Azure Storage connection string is empty or invalid. Unable to write diagnostic events.")); + Assert.True(messagePresent); + Assert.Equal(0, repository.Events.Values.Count()); - var logMessage = _loggerProvider.GetAllLogMessages()[0]; - Assert.True(logMessage.FormattedMessage.StartsWith("Unable to create table 'aa'")); } [Fact]