From 3156f4946be8bae0a34464eb195fb31270fa6646 Mon Sep 17 00:00:00 2001 From: stijnmoreels <9039753+stijnmoreels@users.noreply.github.com> Date: Mon, 2 May 2022 10:11:38 +0200 Subject: [PATCH 1/2] fix: remove invalid exn handling --- .../Extensions/IServiceCollectionExtensions.cs | 13 ------------- .../Extensions/IServiceCollectionExtensions.cs | 13 ------------- 2 files changed, 26 deletions(-) diff --git a/src/Arcus.BackgroundJobs.AzureActiveDirectory/Extensions/IServiceCollectionExtensions.cs b/src/Arcus.BackgroundJobs.AzureActiveDirectory/Extensions/IServiceCollectionExtensions.cs index a0ff375..96fd36a 100644 --- a/src/Arcus.BackgroundJobs.AzureActiveDirectory/Extensions/IServiceCollectionExtensions.cs +++ b/src/Arcus.BackgroundJobs.AzureActiveDirectory/Extensions/IServiceCollectionExtensions.cs @@ -1,9 +1,6 @@ using System; -using System.Linq; -using System.Threading.Tasks; using Arcus.BackgroundJobs.AzureActiveDirectory; using GuardNet; -using Microsoft.Extensions.Logging; // ReSharper disable once CheckNamespace namespace Microsoft.Extensions.DependencyInjection @@ -36,17 +33,7 @@ public static IServiceCollection AddClientSecretExpirationJob( options.SetUserOptions(additionalOptions); }); - builder.UnobservedTaskExceptionHandler = (sender, args) => UnobservedExceptionHandler(args, services); }); } - - private static void UnobservedExceptionHandler(UnobservedTaskExceptionEventArgs eventArgs, IServiceCollection services) - { - ServiceDescriptor logger = services.FirstOrDefault(service => service.ServiceType == typeof(ILogger)); - var loggerInstance = (ILogger) logger?.ImplementationInstance; - - loggerInstance?.LogCritical(eventArgs.Exception, "Unhandled exception in job {JobName}", nameof(ClientSecretExpirationJob)); - eventArgs.SetObserved(); - } } } diff --git a/src/Arcus.BackgroundJobs.Databricks/Extensions/IServiceCollectionExtensions.cs b/src/Arcus.BackgroundJobs.Databricks/Extensions/IServiceCollectionExtensions.cs index cbc52a8..c69dfe1 100644 --- a/src/Arcus.BackgroundJobs.Databricks/Extensions/IServiceCollectionExtensions.cs +++ b/src/Arcus.BackgroundJobs.Databricks/Extensions/IServiceCollectionExtensions.cs @@ -1,9 +1,6 @@ using System; -using System.Linq; -using System.Threading.Tasks; using Arcus.BackgroundJobs.Databricks; using GuardNet; -using Microsoft.Extensions.Logging; // ReSharper disable once CheckNamespace namespace Microsoft.Extensions.DependencyInjection @@ -45,17 +42,7 @@ public static IServiceCollection AddDatabricksJobMetricsJob( options.TokenSecretKey = tokenSecretKey; options.SetUserOptions(additionalOptions); }); - builder.UnobservedTaskExceptionHandler = (sender, args) => UnobservedExceptionHandler(args, services); }); } - - private static void UnobservedExceptionHandler(UnobservedTaskExceptionEventArgs eventArgs, IServiceCollection services) - { - ServiceDescriptor logger = services.FirstOrDefault(service => service.ServiceType == typeof(ILogger)); - var loggerInstance = (ILogger) logger?.ImplementationInstance; - - loggerInstance?.LogCritical(eventArgs.Exception, "Unhandled exception in job {JobName}", nameof(DatabricksJobMetricsJob)); - eventArgs.SetObserved(); - } } } From 29bd10e6739a5c77e0a5a9e019fedd0a644b96bf Mon Sep 17 00:00:00 2001 From: stijnmoreels <9039753+stijnmoreels@users.noreply.github.com> Date: Wed, 4 May 2022 10:56:14 +0200 Subject: [PATCH 2/2] pr-fix: update w custom exn handling strategy --- .../ClientSecretExpirationJob.cs | 64 ++++++++++++------- .../DatabricksJobMetricsJob.cs | 23 ++++--- 2 files changed, 55 insertions(+), 32 deletions(-) diff --git a/src/Arcus.BackgroundJobs.AzureActiveDirectory/ClientSecretExpirationJob.cs b/src/Arcus.BackgroundJobs.AzureActiveDirectory/ClientSecretExpirationJob.cs index 1376252..4ea145a 100644 --- a/src/Arcus.BackgroundJobs.AzureActiveDirectory/ClientSecretExpirationJob.cs +++ b/src/Arcus.BackgroundJobs.AzureActiveDirectory/ClientSecretExpirationJob.cs @@ -70,36 +70,52 @@ public ClientSecretExpirationJob( /// public async Task ExecuteAsync(CancellationToken stoppingToken) { - _logger.LogTrace("Executing {Name}", nameof(ClientSecretExpirationJob)); - var graphServiceClient = new GraphServiceClient(new DefaultAzureCredential()); - _logger.LogTrace("Token retrieved, getting a list of applications with expired or about to expire secrets."); - - var clientSecretExpirationInfoProvider = new ClientSecretExpirationInfoProvider(graphServiceClient, _logger); - IEnumerable applications = - await clientSecretExpirationInfoProvider.GetApplicationsWithPotentialExpiredSecrets(_options.UserOptions.ExpirationThreshold); - - foreach (AzureApplication application in applications) + try { - var telemetryContext = new Dictionary(); - telemetryContext.Add("KeyId", application.KeyId); - telemetryContext.Add("ApplicationName", application.Name); - telemetryContext.Add("RemainingValidDays", application.RemainingValidDays); + _logger.LogTrace("Executing {Name}", nameof(ClientSecretExpirationJob)); + var graphServiceClient = new GraphServiceClient(new DefaultAzureCredential()); + _logger.LogTrace("Token retrieved, getting a list of applications with expired or about to expire secrets"); - var eventType = ClientSecretExpirationEventType.ClientSecretAboutToExpire; - if (application.RemainingValidDays < 0) - { - eventType = ClientSecretExpirationEventType.ClientSecretExpired; - _logger.LogEvent($"The secret {application.KeyId} for Azure Active Directory application {application.Name} has expired.", telemetryContext); - } - else + var clientSecretExpirationInfoProvider = new ClientSecretExpirationInfoProvider(graphServiceClient, _logger); + IEnumerable applications = + await clientSecretExpirationInfoProvider.GetApplicationsWithPotentialExpiredSecrets(_options.UserOptions.ExpirationThreshold); + + foreach (AzureApplication application in applications) { - _logger.LogEvent($"The secret {application.KeyId} for Azure Active Directory application {application.Name} will expire within {application.RemainingValidDays} days.", telemetryContext); + ClientSecretExpirationEventType eventType = DetermineExpirationEventType(application); + + CloudEvent @event = _options.UserOptions.CreateEvent(application, eventType, _options.UserOptions.EventUri); + await _eventGridPublisher.PublishAsync(@event); } + _logger.LogTrace("Executing {Name} finished", nameof(ClientSecretExpirationJob)); + } + catch (Exception exception) + { + _logger.LogCritical(exception, "Could not correctly publish Azure EventGrid events for potential expired client secrets in the Azure Active Directory due to an exception"); + } + } + + private ClientSecretExpirationEventType DetermineExpirationEventType(AzureApplication application) + { + var telemetryContext = new Dictionary + { + { "KeyId", application.KeyId }, + { "ApplicationName", application.Name }, + { "RemainingValidDays", application.RemainingValidDays } + }; - CloudEvent @event = _options.UserOptions.CreateEvent(application, eventType, _options.UserOptions.EventUri); - await _eventGridPublisher.PublishAsync(@event); + var eventType = ClientSecretExpirationEventType.ClientSecretAboutToExpire; + if (application.RemainingValidDays < 0) + { + eventType = ClientSecretExpirationEventType.ClientSecretExpired; + _logger.LogEvent($"The secret {application.KeyId} for Azure Active Directory application {application.Name} has expired.", telemetryContext); + } + else + { + _logger.LogEvent($"The secret {application.KeyId} for Azure Active Directory application {application.Name} will expire within {application.RemainingValidDays} days.", telemetryContext); } - _logger.LogTrace("Executing {Name} finished", nameof(ClientSecretExpirationJob)); + + return eventType; } } } diff --git a/src/Arcus.BackgroundJobs.Databricks/DatabricksJobMetricsJob.cs b/src/Arcus.BackgroundJobs.Databricks/DatabricksJobMetricsJob.cs index bfb40e7..24c3805 100644 --- a/src/Arcus.BackgroundJobs.Databricks/DatabricksJobMetricsJob.cs +++ b/src/Arcus.BackgroundJobs.Databricks/DatabricksJobMetricsJob.cs @@ -65,17 +65,24 @@ public DatabricksJobMetricsJob( /// public async Task ExecuteAsync(CancellationToken stoppingToken) { - using (DatabricksClient client = await _options.CreateDatabricksClientAsync(_secretProvider)) - using (var databricksInfoProvider = new DatabricksInfoProvider(client, _logger)) + try { - (DateTimeOffset start, DateTimeOffset end) = _options.DetermineNextTimeWindow(); + using (DatabricksClient client = await _options.CreateDatabricksClientAsync(_secretProvider)) + using (var databricksInfoProvider = new DatabricksInfoProvider(client, _logger)) + { + (DateTimeOffset start, DateTimeOffset end) = _options.DetermineNextTimeWindow(); - _logger.LogInformation( - "Job monitor for Databricks is starting at {TriggerTime} for time windows {WindowStart} - {WindowEnd}", - DateTimeOffset.UtcNow, start, end); + _logger.LogInformation( + "Job monitor for Databricks is starting at {TriggerTime} for time windows {WindowStart} - {WindowEnd}", + DateTimeOffset.UtcNow, start, end); - string metricName = _options.UserOptions.MetricName; - await databricksInfoProvider.MeasureJobOutcomesAsync(metricName, start, end); + string metricName = _options.UserOptions.MetricName; + await databricksInfoProvider.MeasureJobOutcomesAsync(metricName, start, end); + } + } + catch (Exception exception) + { + _logger.LogCritical(exception, "Could not measure the finished Databricks jobs due to an exception"); } } }