[SG-419] Added logging to mobile push notifications (#2332)

* Added logging to push notifications.

* Added additional logging for testing push notifications.

* Removed package lock changes.

* Removed package lock changes.

* Renamed the property and added a description.

* Undid changes to LoggerFactory.

* Removed filter on Microsoft library logging.

Co-authored-by: Todd Martin <>

src/Core/Services/Implementations/BaseIdentityClientService.cs

@@ -54,16 +54,19 @@ public abstract class BaseIdentityClientService : IDisposable
 
     protected async Task<TResult> SendAsync<TRequest, TResult>(HttpMethod method, string path, TRequest requestModel)
     {
+        var fullRequestPath = string.Concat(Client.BaseAddress, path);
+
         var tokenStateResponse = await HandleTokenStateAsync();
         if (!tokenStateResponse)
         {
+            _logger.LogError("Unable to send {method} request to {requestUri} because an access token was unable to be obtained", method.Method, fullRequestPath);
             return default;
         }
 
         var message = new TokenHttpRequestMessage(requestModel, AccessToken)
         {
             Method = method,
-            RequestUri = new Uri(string.Concat(Client.BaseAddress, path))
+            RequestUri = new Uri(fullRequestPath)
         };
         try
         {
@@ -120,7 +123,7 @@ public abstract class BaseIdentityClientService : IDisposable
 
         if (!response.IsSuccessStatusCode)
         {
-            _logger.LogInformation("Unsuccessful token response with status code {StatusCode}", response.StatusCode);
+            _logger.LogInformation("Unsuccessful token response from {identity} for client {clientId} with status code {StatusCode}", IdentityClient.BaseAddress, _identityClientId, response.StatusCode);
 
             if (response.StatusCode == HttpStatusCode.BadRequest)
             {

src/Core/Services/Implementations/MultiServicePushNotificationService.cs

@@ -44,7 +44,7 @@ public class MultiServicePushNotificationService : IPushNotificationService
             if (CoreHelpers.SettingHasValue(globalSettings.NotificationHub.ConnectionString))
             {
                 _services.Add(new NotificationHubPushNotificationService(installationDeviceRepository,
-                    globalSettings, httpContextAccessor));
+                    globalSettings, httpContextAccessor, hubLogger));
             }
             if (CoreHelpers.SettingHasValue(globalSettings.Notifications?.ConnectionString))
             {

src/Core/Services/Implementations/NotificationHubPushNotificationService.cs

@@ -9,6 +9,7 @@ using Bit.Core.Repositories;
 using Bit.Core.Settings;
 using Microsoft.AspNetCore.Http;
 using Microsoft.Azure.NotificationHubs;
+using Microsoft.Extensions.Logging;
 
 namespace Bit.Core.Services;
 
@@ -17,20 +18,23 @@ public class NotificationHubPushNotificationService : IPushNotificationService
     private readonly IInstallationDeviceRepository _installationDeviceRepository;
     private readonly GlobalSettings _globalSettings;
     private readonly IHttpContextAccessor _httpContextAccessor;
-
     private NotificationHubClient _client = null;
+    private ILogger _logger;
 
     public NotificationHubPushNotificationService(
         IInstallationDeviceRepository installationDeviceRepository,
         GlobalSettings globalSettings,
-        IHttpContextAccessor httpContextAccessor)
+        IHttpContextAccessor httpContextAccessor,
+        ILogger<NotificationsApiPushNotificationService> logger)
     {
         _installationDeviceRepository = installationDeviceRepository;
         _globalSettings = globalSettings;
         _httpContextAccessor = httpContextAccessor;
         _client = NotificationHubClient.CreateClientFromConnectionString(
             _globalSettings.NotificationHub.ConnectionString,
-            _globalSettings.NotificationHub.HubName);
+            _globalSettings.NotificationHub.HubName,
+            _globalSettings.NotificationHub.EnableSendTracing);
+        _logger = logger;
     }
 
     public async Task PushSyncCipherCreateAsync(Cipher cipher, IEnumerable<Guid> collectionIds)
@@ -244,12 +248,17 @@ public class NotificationHubPushNotificationService : IPushNotificationService
 
     private async Task SendPayloadAsync(string tag, PushType type, object payload)
     {
-        await _client.SendTemplateNotificationAsync(
+        var outcome = await _client.SendTemplateNotificationAsync(
             new Dictionary<string, string>
             {
                 { "type",  ((byte)type).ToString() },
                 { "payload", JsonSerializer.Serialize(payload) }
             }, tag);
+        if (_globalSettings.NotificationHub.EnableSendTracing)
+        {
+            _logger.LogInformation("Azure Notification Hub Tracking ID: {id} | {type} push notification with {success} successes and {failure} failures with a payload of {@payload} and result of {@results}",
+                outcome.TrackingId, type, outcome.Success, outcome.Failure, payload, outcome.Results);
+        }
     }
 
     private string SanitizeTagInput(string input)

src/Core/Settings/GlobalSettings.cs

@@ -412,6 +412,12 @@ public class GlobalSettings : IGlobalSettings
             set => _connectionString = value.Trim('"');
         }
         public string HubName { get; set; }
+
+        /// <summary>
+        /// Enables TestSend on the Azure Notification Hub, which allows tracing of the request through the hub and to the platform-specific push notification service (PNS).
+        /// Enabling this will result in delayed responses because the Hub must wait on delivery to the PNS.  This should ONLY be enabled in a non-production environment, as results are throttled.
+        /// </summary>
+        public bool EnableSendTracing { get; set; } = false;
     }
 
     public class YubicoSettings

test/Core.Test/Services/NotificationHubPushNotificationServiceTests.cs

@@ -2,6 +2,7 @@
 using Bit.Core.Services;
 using Bit.Core.Settings;
 using Microsoft.AspNetCore.Http;
+using Microsoft.Extensions.Logging;
 using NSubstitute;
 using Xunit;
 
@@ -14,17 +15,20 @@ public class NotificationHubPushNotificationServiceTests
     private readonly IInstallationDeviceRepository _installationDeviceRepository;
     private readonly GlobalSettings _globalSettings;
     private readonly IHttpContextAccessor _httpContextAccessor;
+    private readonly ILogger<NotificationsApiPushNotificationService> _logger;
 
     public NotificationHubPushNotificationServiceTests()
     {
         _installationDeviceRepository = Substitute.For<IInstallationDeviceRepository>();
         _globalSettings = new GlobalSettings();
         _httpContextAccessor = Substitute.For<IHttpContextAccessor>();
+        _logger = Substitute.For<ILogger<NotificationsApiPushNotificationService>>();
 
         _sut = new NotificationHubPushNotificationService(
             _installationDeviceRepository,
             _globalSettings,
-            _httpContextAccessor
+            _httpContextAccessor,
+            _logger
         );
     }