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.

Sign up for GitHub

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

[in-proc] Add support for identity-based connections to Diagnostic Events #10442

Merged
merged 1 commit into from
Oct 19, 2024
Merged

[in-proc] Add support for identity-based connections to Diagnostic Events #10442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion release_notes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@
- Update Node.js Worker Version to [3.10.1](https://github.com/Azure/azure-functions-nodejs-worker/releases/tag/v3.10.1)
- Implement host configuration property to allow configuration of the metadata provider timeout period (#10526)
- The value can be set via `metadataProviderTimeout` in host.json and defaults to "00:00:30" (30 seconds).
- For logic apps, unless configured via the host.json, the timeout is disabled by default.
- For logic apps, unless configured via the host.json, the timeout is disabled by default.
- Added support for identity-based connections to Diagnostic Events (#10438)
8 changes: 4 additions & 4 deletions src/WebJobs.Script.WebHost/Diagnostics/DiagnosticEventRepositoryFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;

namespace Microsoft.Azure.WebJobs.Script.WebHost.Diagnostics
{
Expand All @@ -21,11 +20,12 @@ public DiagnosticEventRepositoryFactory(IServiceProvider serviceProvider, IConfi

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.
// Using this to break circular dependency for ILoggers. Typically you cannot log errors within the logging pipeline because it creates infinite loop.
// However in this case that loop is broken because of the filtering in the DiagnosticEventLogger

string storageConnectionString = _configuration.GetWebJobsConnectionString(ConnectionStringNames.Storage);
if (string.IsNullOrEmpty(storageConnectionString))
IConfigurationSection storageConnectionSection = _configuration.GetWebJobsConnectionSection(ConnectionStringNames.Storage);

if (storageConnectionSection is null || !storageConnectionSection.Exists())
{
return new DiagnosticEventNullRepository();
}
Expand Down
36 changes: 11 additions & 25 deletions src/WebJobs.Script.WebHost/Diagnostics/DiagnosticEventTableStorageRepository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
using Microsoft.Azure.WebJobs.Hosting;
using Microsoft.Azure.WebJobs.Logging;
using Microsoft.Azure.WebJobs.Script.WebHost.Helpers;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;

Expand All @@ -25,9 +24,9 @@ public class DiagnosticEventTableStorageRepository : IDiagnosticEventRepository,
private const int TableCreationMaxRetryCount = 5;

private readonly Timer _flushLogsTimer;
private readonly IConfiguration _configuration;
private readonly IHostIdProvider _hostIdProvider;
private readonly IEnvironment _environment;
private readonly IAzureTableStorageProvider _azureTableStorageProvider;
private readonly ILogger<DiagnosticEventTableStorageRepository> _logger;
private readonly IServiceProvider _serviceProvider;
private readonly object _syncLock = new object();
Expand All @@ -41,43 +40,30 @@ public class DiagnosticEventTableStorageRepository : IDiagnosticEventRepository,
private string _tableName;
private bool _isEnabled = true;

internal DiagnosticEventTableStorageRepository(IConfiguration configuration, IHostIdProvider hostIdProvider, IEnvironment environment, IScriptHostManager scriptHostManager,
ILogger<DiagnosticEventTableStorageRepository> logger, int logFlushInterval)
internal DiagnosticEventTableStorageRepository(IHostIdProvider hostIdProvider, IEnvironment environment, IScriptHostManager scriptHostManager,
IAzureTableStorageProvider azureTableStorageProvider, ILogger<DiagnosticEventTableStorageRepository> logger, int logFlushInterval)
{
_configuration = configuration;
_hostIdProvider = hostIdProvider;
_environment = environment;
_serviceProvider = scriptHostManager as IServiceProvider;
_logger = logger;
_flushLogsTimer = new Timer(OnFlushLogs, null, logFlushInterval, logFlushInterval);
_azureTableStorageProvider = azureTableStorageProvider;
}

public DiagnosticEventTableStorageRepository(IConfiguration configuration, IHostIdProvider hostIdProvider, IEnvironment environment, IScriptHostManager scriptHost,
ILogger<DiagnosticEventTableStorageRepository> logger)
: this(configuration, hostIdProvider, environment, scriptHost, logger, LogFlushInterval) { }
public DiagnosticEventTableStorageRepository(IHostIdProvider hostIdProvider, IEnvironment environment, IScriptHostManager scriptHost,
IAzureTableStorageProvider azureTableStorageProvider, ILogger<DiagnosticEventTableStorageRepository> logger)
: this(hostIdProvider, environment, scriptHost, azureTableStorageProvider, logger, LogFlushInterval) { }

internal TableServiceClient TableClient
{
get
{
if (!_environment.IsPlaceholderModeEnabled() && _tableClient == null)
if (!_environment.IsPlaceholderModeEnabled() && _tableClient == null && !_azureTableStorageProvider.TryCreateHostingTableServiceClient(out _tableClient))
{
string storageConnectionString = _configuration.GetWebJobsConnectionString(ConnectionStringNames.Storage);

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, "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();
}
_logger.LogWarning("An error occurred initializing the Table Storage Client. We are unable to record diagnostic events, so the diagnostic logging service is being stopped.");
_isEnabled = false;
StopTimer();
}

return _tableClient;
Expand Down
37 changes: 21 additions & 16 deletions ...ebJobs.Script.Tests.Integration/Diagnostics/DiagnosticEventTableStorageRepositoryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ public class DiagnosticEventTableStorageRepositoryTests
private const string TestHostId = "testhostid";
private readonly IHostIdProvider _hostIdProvider;
private readonly TestLoggerProvider _loggerProvider;
private IConfiguration _configuration;
private readonly IAzureTableStorageProvider _azureTableStorageProvider;

private ILogger<DiagnosticEventTableStorageRepository> _logger;
private Mock<IScriptHostManager> _scriptHostMock;

public DiagnosticEventTableStorageRepositoryTests()
{
_configuration = new ConfigurationBuilder().AddEnvironmentVariables().Build();
_hostIdProvider = new FixedHostIdProvider(TestHostId);

var mockPrimaryHostStateProvider = new Mock<IPrimaryHostStateProvider>(MockBehavior.Strict);
Expand All @@ -46,6 +46,9 @@ public DiagnosticEventTableStorageRepositoryTests()
ILoggerFactory loggerFactory = new LoggerFactory();
loggerFactory.AddProvider(_loggerProvider);
_logger = loggerFactory.CreateLogger<DiagnosticEventTableStorageRepository>();

var configuration = new ConfigurationBuilder().AddEnvironmentVariables().Build();
_azureTableStorageProvider = TestHelpers.GetAzureTableStorageProvider(configuration);
}

[Fact]
Expand All @@ -54,7 +57,7 @@ public async Task TimerFlush_CalledOnExpectedInterval()
IEnvironment testEnvironment = new TestEnvironment();
int flushInterval = 10;
Mock<DiagnosticEventTableStorageRepository> mockDiagnosticEventTableStorageRepository = new Mock<DiagnosticEventTableStorageRepository>
(_configuration, _hostIdProvider, testEnvironment, _scriptHostMock.Object, NullLogger<DiagnosticEventTableStorageRepository>.Instance, flushInterval);
(_hostIdProvider, testEnvironment, _scriptHostMock.Object, _azureTableStorageProvider, NullLogger<DiagnosticEventTableStorageRepository>.Instance, flushInterval);

DiagnosticEventTableStorageRepository repository = mockDiagnosticEventTableStorageRepository.Object;

Expand All @@ -76,7 +79,7 @@ public void WriteDiagnostic_LogsError_WhenHostIdNotSet()
IEnvironment testEnvironment = new TestEnvironment();

DiagnosticEventTableStorageRepository repository =
new DiagnosticEventTableStorageRepository(_configuration, null, testEnvironment, _scriptHostMock.Object, _logger);
new DiagnosticEventTableStorageRepository(null, testEnvironment, _scriptHostMock.Object, _azureTableStorageProvider, _logger);

repository.WriteDiagnosticEvent(DateTime.UtcNow, "eh1", LogLevel.Information, "This is the message", "https://fwlink/", new Exception("exception message"));

Expand All @@ -91,7 +94,7 @@ public async Task WriteDiagnostic_HasTheCorrectHitCount_WhenCalledFromMultipleTh
testEnvironment.SetEnvironmentVariable(EnvironmentSettingNames.AzureWebsitePlaceholderMode, "0");

DiagnosticEventTableStorageRepository repository =
new DiagnosticEventTableStorageRepository(_configuration, _hostIdProvider, testEnvironment, _scriptHostMock.Object, _logger);
new DiagnosticEventTableStorageRepository(_hostIdProvider, testEnvironment, _scriptHostMock.Object, _azureTableStorageProvider, _logger);

var eventTask1 = Task.Run(() =>
{
Expand Down Expand Up @@ -136,7 +139,7 @@ public void GetDiagnosticEventsTable_ReturnsExpectedValue_WhenSpecialized()
testEnvironment.SetEnvironmentVariable(EnvironmentSettingNames.AzureWebsitePlaceholderMode, "0");

DiagnosticEventTableStorageRepository repository =
new DiagnosticEventTableStorageRepository(_configuration, _hostIdProvider, testEnvironment, _scriptHostMock.Object, _logger);
new DiagnosticEventTableStorageRepository(_hostIdProvider, testEnvironment, _scriptHostMock.Object, _azureTableStorageProvider, _logger);
DateTime dateTime = new DateTime(2021, 1, 1);
var cloudTable = repository.GetDiagnosticEventsTable(dateTime);
Assert.NotNull(cloudTable);
Expand All @@ -151,16 +154,17 @@ public void GetDiagnosticEventsTable_LogsError_StorageConnectionStringIsNotPrese
testEnvironment.SetEnvironmentVariable(EnvironmentSettingNames.AzureWebsitePlaceholderMode, "0");

var configuration = new ConfigurationBuilder().Build();
var localStorageProvider = TestHelpers.GetAzureTableStorageProvider(configuration);
DiagnosticEventTableStorageRepository repository =
new DiagnosticEventTableStorageRepository(configuration, _hostIdProvider, testEnvironment, _scriptHostMock.Object, _logger);
new DiagnosticEventTableStorageRepository(_hostIdProvider, testEnvironment, _scriptHostMock.Object, localStorageProvider, _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, "The Azure Storage connection string is either empty or invalid. Unable to record diagnostic events, so the diagnostic logging service is being stopped.");
Assert.Equal(messages[0].FormattedMessage, "An error occurred initializing the Table Storage Client. We are unable to record diagnostic events, so the diagnostic logging service is being stopped.");
Assert.False(repository.IsEnabled());
}

Expand All @@ -171,7 +175,7 @@ public async Task QueueBackgroundDiagnosticsEventsTablePurge_PurgesTables()
testEnvironment.SetEnvironmentVariable(EnvironmentSettingNames.AzureWebsitePlaceholderMode, "0");

DiagnosticEventTableStorageRepository repository =
new DiagnosticEventTableStorageRepository(_configuration, _hostIdProvider, testEnvironment, _scriptHostMock.Object, _logger);
new DiagnosticEventTableStorageRepository(_hostIdProvider, testEnvironment, _scriptHostMock.Object, _azureTableStorageProvider, _logger);

// delete any existing non-current diagnostics events tables
string tablePrefix = DiagnosticEventTableStorageRepository.TableNamePrefix;
Expand Down Expand Up @@ -212,7 +216,7 @@ public async Task QueueBackgroundDiagnosticsEventsTablePurge_PurgesOnlyDiagnosti
testEnvironment.SetEnvironmentVariable(EnvironmentSettingNames.AzureWebsitePlaceholderMode, "0");

DiagnosticEventTableStorageRepository repository =
new DiagnosticEventTableStorageRepository(_configuration, _hostIdProvider, testEnvironment, _scriptHostMock.Object, _logger);
new DiagnosticEventTableStorageRepository(_hostIdProvider, testEnvironment, _scriptHostMock.Object, _azureTableStorageProvider, _logger);

// delete any existing non-current diagnostics events tables
string tablePrefix = DiagnosticEventTableStorageRepository.TableNamePrefix;
Expand Down Expand Up @@ -267,8 +271,9 @@ public async Task FlushLogs_LogsErrorAndClearsEvents_WhenTableCreatingFails()
.AddInMemoryCollection(testData)
.Build();

var localStorageProvider = TestHelpers.GetAzureTableStorageProvider(configuration);
DiagnosticEventTableStorageRepository repository =
new DiagnosticEventTableStorageRepository(configuration, _hostIdProvider, testEnvironment, _scriptHostMock.Object, _logger);
new DiagnosticEventTableStorageRepository(_hostIdProvider, testEnvironment, _scriptHostMock.Object, localStorageProvider, _logger);

// Act
repository.WriteDiagnosticEvent(DateTime.UtcNow, "eh1", LogLevel.Information, "This is the message", "https://fwlink/", new Exception("exception message"));
Expand All @@ -278,7 +283,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("The Azure Storage connection string is either empty or invalid. Unable to record diagnostic events, so the diagnostic logging service is being stopped."));
var messagePresent = _loggerProvider.GetAllLogMessages().Any(m => m.FormattedMessage.Contains("An error occurred initializing the Table Storage Client. We are unable to record diagnostic events, so the diagnostic logging service is being stopped."));
Assert.True(messagePresent);

Assert.Equal(0, repository.Events.Values.Count());
Expand All @@ -302,7 +307,7 @@ public async Task FlushLogs_OnPrimaryHost_PurgesPreviousEventVersionTables(strin
scriptHostMock.As<IServiceProvider>().Setup(m => m.GetService(typeof(IPrimaryHostStateProvider))).Returns(mockPrimaryHostStateProvider.Object);

DiagnosticEventTableStorageRepository repository =
new DiagnosticEventTableStorageRepository(_configuration, _hostIdProvider, testEnvironment, scriptHostMock.Object, _logger);
new DiagnosticEventTableStorageRepository(_hostIdProvider, testEnvironment, scriptHostMock.Object, _azureTableStorageProvider, _logger);

// delete existing tables
string tablePrefix = DiagnosticEventTableStorageRepository.TableNamePrefix;
Expand Down Expand Up @@ -352,7 +357,7 @@ public async Task ExecuteBatchAsync_WritesToTableStorage()
testEnvironment.SetEnvironmentVariable(EnvironmentSettingNames.AzureWebsitePlaceholderMode, "0");

DiagnosticEventTableStorageRepository repository =
new DiagnosticEventTableStorageRepository(_configuration, _hostIdProvider, testEnvironment, _scriptHostMock.Object, _logger);
new DiagnosticEventTableStorageRepository(_hostIdProvider, testEnvironment, _scriptHostMock.Object, _azureTableStorageProvider, _logger);

var table = repository.GetDiagnosticEventsTable();
await TableStorageHelpers.CreateIfNotExistsAsync(table, repository.TableClient, 2);
Expand All @@ -375,7 +380,7 @@ public async Task FlushLogs_WritesToTableStorage()
testEnvironment.SetEnvironmentVariable(EnvironmentSettingNames.AzureWebsitePlaceholderMode, "0");

DiagnosticEventTableStorageRepository repository =
new DiagnosticEventTableStorageRepository(_configuration, _hostIdProvider, testEnvironment, _scriptHostMock.Object, _logger);
new DiagnosticEventTableStorageRepository(_hostIdProvider, testEnvironment, _scriptHostMock.Object, _azureTableStorageProvider, _logger);

var table = repository.GetDiagnosticEventsTable();
await TableStorageHelpers.CreateIfNotExistsAsync(table, repository.TableClient, 2);
Expand All @@ -397,7 +402,7 @@ public async Task ExecuteBatchAsync_LogsError()
testEnvironment.SetEnvironmentVariable(EnvironmentSettingNames.AzureWebsitePlaceholderMode, "0");

DiagnosticEventTableStorageRepository repository =
new DiagnosticEventTableStorageRepository(_configuration, _hostIdProvider, testEnvironment, _scriptHostMock.Object, _logger);
new DiagnosticEventTableStorageRepository(_hostIdProvider, testEnvironment, _scriptHostMock.Object, _azureTableStorageProvider, _logger);

var tableClient = repository.TableClient;
var table = tableClient.GetTableClient("aa");
Expand Down
Loading