[in-proc] Avoid redundant DiagnosticEvents error message (#10396)

Author: RohitRanjanMS

src/WebJobs.Script.WebHost/Diagnostics/DiagnosticEventLogger.cs

@@ -54,20 +54,17 @@ private bool IsDiagnosticEvent(IDictionary<string, object> state)
 
         public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
         {
-            if (!IsEnabled(logLevel))
+            // Exit early if logging is not enabled or the state is not a diagnostic event
+            if (!IsEnabled(logLevel) ||
+                (_diagnosticEventRepository != null && !_diagnosticEventRepository.IsEnabled()) ||
+                !(state is IDictionary<string, object> stateInfo && IsDiagnosticEvent(stateInfo)))
             {
                 return;
             }
 
-            if (state is IDictionary<string, object> stateInfo && IsDiagnosticEvent(stateInfo))
-            {
-                string message = formatter(state, exception);
-                if (_diagnosticEventRepository == null)
-                {
-                    _diagnosticEventRepository = _diagnosticEventRepositoryFactory.Create();
-                }
-                _diagnosticEventRepository.WriteDiagnosticEvent(DateTime.UtcNow, stateInfo[ScriptConstants.ErrorCodeKey].ToString(), logLevel, message, stateInfo[ScriptConstants.HelpLinkKey].ToString(), exception);
-            }
+            string message = formatter(state, exception);
+            _diagnosticEventRepository ??= _diagnosticEventRepositoryFactory.Create();
+            _diagnosticEventRepository.WriteDiagnosticEvent(DateTime.UtcNow, stateInfo[ScriptConstants.ErrorCodeKey].ToString(), logLevel, message, stateInfo[ScriptConstants.HelpLinkKey].ToString(), exception);
         }
 
         public void Dispose()

src/WebJobs.Script.WebHost/Diagnostics/DiagnosticEventNullRepository.cs

@@ -2,19 +2,17 @@
 // 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) { }
+
+        public bool IsEnabled()
+        {
+            return false;
+        }
     }
 }

src/WebJobs.Script.WebHost/Diagnostics/DiagnosticEventTableStorageRepository.cs

@@ -39,6 +39,7 @@ public class DiagnosticEventTableStorageRepository : IDiagnosticEventRepository,
         private bool _disposed = false;
         private bool _purged = false;
         private string _tableName;
+        private bool _isEnabled = true;
 
         internal DiagnosticEventTableStorageRepository(IConfiguration configuration, IHostIdProvider hostIdProvider, IEnvironment environment, IScriptHostManager scriptHostManager,
             ILogger<DiagnosticEventTableStorageRepository> logger, int logFlushInterval)
@@ -66,10 +67,16 @@ internal TableServiceClient TableClient
                     try
                     {
                         _tableClient = new TableServiceClient(storageConnectionString);
+
+                        // The TableServiceClient only verifies the format of the connection string.
+                        // To ensure the storage account exists and supports Table storage, validate the connection string by retrieving the properties of the table service.
+                        _ = _tableClient.GetProperties();
                     }
                     catch (Exception ex)
                     {
-                        _logger.LogError(ex, "Azure Storage connection string is empty or invalid. Unable to write diagnostic events.");
+                        _logger.LogError(ex, "The Azure Storage connection string is either empty or invalid. Unable to record diagnostic events, so the diagnostic logging service is being stopped.");
+                        _isEnabled = false;
+                        StopTimer();
                     }
                 }
 
@@ -269,6 +276,11 @@ public void WriteDiagnosticEvent(DateTime timestamp, string errorCode, LogLevel
             }
         }
 
+        public bool IsEnabled()
+        {
+            return _isEnabled;
+        }
+
         private bool IsPrimaryHost()
         {
             var primaryHostStateProvider = _serviceProvider?.GetService<IPrimaryHostStateProvider>();

src/WebJobs.Script.WebHost/Diagnostics/IDiagnosticEventRepository.cs

@@ -2,14 +2,14 @@
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
 using System;
-using System.Collections.Generic;
-using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
 
 namespace Microsoft.Azure.WebJobs.Script.WebHost.Diagnostics
 {
     public interface IDiagnosticEventRepository
     {
         void WriteDiagnosticEvent(DateTime timestamp, string errorCode, LogLevel level, string message, string helpLink, Exception exception);
+
+        bool IsEnabled();
     }
 }

test/WebJobs.Script.Tests.Integration/Diagnostics/DiagnosticEventTableStorageRepositoryTests.cs

@@ -153,11 +153,15 @@ public void GetDiagnosticEventsTable_LogsError_StorageConnectionStringIsNotPrese
             var configuration = new ConfigurationBuilder().Build();
             DiagnosticEventTableStorageRepository repository =
                 new DiagnosticEventTableStorageRepository(configuration, _hostIdProvider, testEnvironment, _scriptHostMock.Object, _logger);
+
+            // The repository should be initially enabled and then disabled after TableServiceClient failure.
+            Assert.True(repository.IsEnabled());
             DateTime dateTime = new DateTime(2021, 1, 1);
             var cloudTable = repository.GetDiagnosticEventsTable(dateTime);
             Assert.Null(cloudTable);
             var messages = _loggerProvider.GetAllLogMessages();
-            Assert.Equal(messages[0].FormattedMessage, "Azure Storage connection string is empty or invalid. Unable to write diagnostic events.");
+            Assert.Equal(messages[0].FormattedMessage, "The Azure Storage connection string is either empty or invalid. Unable to record diagnostic events, so the diagnostic logging service is being stopped.");
+            Assert.False(repository.IsEnabled());
         }
 
         [Fact]
@@ -274,7 +278,7 @@ public async Task FlushLogs_LogsErrorAndClearsEvents_WhenTableCreatingFails()
             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."));
+            var messagePresent = _loggerProvider.GetAllLogMessages().Any(m => m.FormattedMessage.Contains("The Azure Storage connection string is either empty or invalid. Unable to record diagnostic events, so the diagnostic logging service is being stopped."));
             Assert.True(messagePresent);
 
             Assert.Equal(0, repository.Events.Values.Count());

test/WebJobs.Script.Tests.Shared/TestDiagnosticEventRepository.cs

@@ -35,4 +35,9 @@ public void FlushLogs()
     {
         Events.Clear();
     }
+
+    public bool IsEnabled()
+    {
+        return true;
+    }
 }