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

Avoid redundant DiagnosticEvents error message #10395

Merged
merged 2 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions src/WebJobs.Script.WebHost/Diagnostics/DiagnosticEventLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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();
}
}

Expand Down Expand Up @@ -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>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,9 @@ public void FlushLogs()
{
Events.Clear();
}

public bool IsEnabled()
{
return true;
}
}