fix: remove invalid exn handling (#132)

* fix: remove invalid exn handling * pr-fix: update w custom exn handling strategy
arcus-azure · May 9, 2022 · 84f75db · 84f75db
1 parent a9bbcd1
commit 84f75db
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 58 deletions.
diff --git a/src/Arcus.BackgroundJobs.AzureActiveDirectory/ClientSecretExpirationJob.cs b/src/Arcus.BackgroundJobs.AzureActiveDirectory/ClientSecretExpirationJob.cs
@@ -70,36 +70,52 @@ public ClientSecretExpirationJob(
         /// </returns>
         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<AzureApplication> applications = 
-                await clientSecretExpirationInfoProvider.GetApplicationsWithPotentialExpiredSecrets(_options.UserOptions.ExpirationThreshold);
-
-            foreach (AzureApplication application in applications)
+            try
             {
-                var telemetryContext = new Dictionary<string, object>();
-                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<AzureApplication> 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<string, object>
+            {
+                { "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.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/DatabricksJobMetricsJob.cs b/src/Arcus.BackgroundJobs.Databricks/DatabricksJobMetricsJob.cs
@@ -65,17 +65,24 @@ public DatabricksJobMetricsJob(
         /// </returns>
         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");
             }
         }
     }

diff --git 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();
-        }
     }
 }