feat(notification): misc logging, types, fixes

packages/core/src/notifications/panelNode.ts

@@ -16,6 +16,9 @@ import { registerToolView } from '../awsexplorer/activationShared'
 import { readonlyDocument } from '../shared/utilities/textDocumentUtilities'
 import { openUrl } from '../shared/utilities/vsCodeUtils'
 import { telemetry } from '../shared/telemetry/telemetry'
+import { globals } from '../shared'
+
+const logger = getLogger('notifications')
 
 /**
  * Controls the "Notifications" side panel/tree in each extension. It takes purely UX actions
@@ -132,23 +135,23 @@ export class NotificationsNode implements TreeNode {
         switch (notification.uiRenderInstructions.onClick.type) {
             case 'modal':
                 // Render blocking modal
-                getLogger('notifications').verbose(`rendering modal for notificaiton: ${notification.id} ...`)
+                logger.verbose(`rendering modal for notificaiton: ${notification.id} ...`)
                 await this.showInformationWindow(notification, 'modal', false)
                 break
             case 'openUrl':
                 // Show open url option
                 if (!notification.uiRenderInstructions.onClick.url) {
                     throw new ToolkitError('No url provided for onclick open url')
                 }
-                getLogger('notifications').verbose(`opening url for notification: ${notification.id} ...`)
+                logger.verbose(`opening url for notification: ${notification.id} ...`)
                 await openUrl(
                     vscode.Uri.parse(notification.uiRenderInstructions.onClick.url),
                     getNotificationTelemetryId(notification)
                 )
                 break
             case 'openTextDocument':
                 // Display read-only txt document
-                getLogger('notifications').verbose(`showing txt document for notification: ${notification.id} ...`)
+                logger.verbose(`showing txt document for notification: ${notification.id} ...`)
                 await telemetry.toolkit_invokeAction.run(async () => {
                     telemetry.record({ source: getNotificationTelemetryId(notification), action: 'openTxt' })
                     await readonlyDocument.show(
@@ -207,7 +210,10 @@ export class NotificationsNode implements TreeNode {
                                     )
                                     break
                                 case 'updateAndReload':
-                                    await this.updateAndReload(notification.displayIf.extensionId)
+                                    // Give things time to finish executing.
+                                    globals.clock.setTimeout(() => {
+                                        void this.updateAndReload(notification.displayIf.extensionId)
+                                    }, 1000)
                                     break
                                 case 'openUrl':
                                     if (selectedButton.url) {
@@ -232,7 +238,11 @@ export class NotificationsNode implements TreeNode {
     }
 
     private async updateAndReload(id: string) {
-        getLogger('notifications').verbose('Updating and reloading the extension...')
+        logger.verbose('Updating and reloading the extension...')
+
+        // Publish pending telemetry before it is lost to the window reload.
+        await globals.telemetry.flushRecords()
+
         await vscode.commands.executeCommand('workbench.extensions.installExtension', id)
         await vscode.commands.executeCommand('workbench.action.reloadWindow')
     }

packages/core/src/notifications/rules.ts

@@ -11,6 +11,7 @@ import { getComputeEnvType, getOperatingSystem } from '../shared/telemetry/util'
 import { AuthFormId } from '../login/webview/vue/types'
 import { getLogger } from '../shared/logger/logger'
 
+const logger = getLogger('notifications')
 /**
  * Evaluates if a given version fits into the parameters specified by a notification, e.g:
  *
@@ -72,31 +73,51 @@ export class RuleEngine {
     constructor(private readonly context: RuleContext) {}
 
     public shouldDisplayNotification(payload: ToolkitNotification) {
-        return this.evaluate(payload.displayIf)
+        return this.evaluate(payload.id, payload.displayIf)
     }
 
-    private evaluate(condition: DisplayIf): boolean {
+    private evaluate(id: string, condition: DisplayIf): boolean {
         const currentExt = globals.context.extension.id
         if (condition.extensionId !== currentExt) {
+            logger.verbose(
+                'notification id: (%s) did NOT pass extension id check, actual ext id: (%s), expected ext id: (%s)',
+                id,
+                currentExt,
+                condition.extensionId
+            )
             return false
         }
 
         if (condition.ideVersion) {
             if (!isValidVersion(this.context.ideVersion, condition.ideVersion)) {
+                logger.verbose(
+                    'notification id: (%s) did NOT pass IDE version check, actual version: (%s), expected version: (%s)',
+                    id,
+                    this.context.ideVersion,
+                    condition.ideVersion
+                )
                 return false
             }
         }
         if (condition.extensionVersion) {
             if (!isValidVersion(this.context.extensionVersion, condition.extensionVersion)) {
+                logger.verbose(
+                    'notification id: (%s) did NOT pass extension version check, actual ext version: (%s), expected ext version: (%s)',
+                    id,
+                    this.context.extensionVersion,
+                    condition.extensionVersion
+                )
                 return false
             }
         }
 
         if (condition.additionalCriteria) {
             for (const criteria of condition.additionalCriteria) {
                 if (!this.evaluateRule(criteria)) {
+                    logger.verbose('notification id: (%s) did NOT pass criteria check: %O', id, criteria)
                     return false
                 }
+                logger.debug('notification id: (%s) passed criteria check: %O', id, criteria)
             }
         }
 
@@ -176,7 +197,7 @@ export async function getRuleContext(context: vscode.ExtensionContext, authState
     }
 
     const { activeExtensions, ...loggableRuleContext } = ruleContext
-    getLogger('notifications').debug('getRuleContext() determined rule context: %O', loggableRuleContext)
+    logger.debug('getRuleContext() determined rule context: %O', loggableRuleContext)
 
     return ruleContext
 }

packages/core/src/shared/telemetry/telemetryService.ts

@@ -195,9 +195,11 @@ export class DefaultTelemetryService {
     }
 
     /**
-     * Publish metrics to the Telemetry Service.
+     * Publish metrics to the Telemetry Service. Usually it will automatically flush recent events
+     * on a regular interval. This should not be used unless you are interrupting this interval,
+     * e.g. via a forced window reload.
      */
-    private async flushRecords(): Promise<void> {
+    public async flushRecords(): Promise<void> {
         if (this.telemetryEnabled) {
             await this._flushRecords()
         }

packages/core/src/test/notifications/rendering.test.ts

@@ -5,24 +5,35 @@
 
 import * as vscode from 'vscode'
 import * as sinon from 'sinon'
+import * as FakeTimers from '@sinonjs/fake-timers'
 import assert from 'assert'
 import { ToolkitNotification } from '../../notifications/types'
 import { panelNode } from './controller.test'
 import { getTestWindow } from '../shared/vscode/window'
 import * as VsCodeUtils from '../../shared/utilities/vsCodeUtils'
-import { assertTextEditorContains } from '../testUtil'
+import { assertTextEditorContains, installFakeClock } from '../testUtil'
 
 describe('Notifications Rendering', function () {
     let sandbox: sinon.SinonSandbox
+    let clock: FakeTimers.InstalledClock
+
+    before(function () {
+        clock = installFakeClock()
+    })
 
     beforeEach(function () {
         sandbox = sinon.createSandbox()
     })
 
     afterEach(function () {
+        clock.reset()
         sandbox.restore()
     })
 
+    after(function () {
+        clock.uninstall()
+    })
+
     // util to test txt pop-up under different senarios
     async function verifyTxtNotification(notification: ToolkitNotification) {
         const expectedContent = notification.uiRenderInstructions.content['en-US'].description
@@ -95,6 +106,9 @@ describe('Notifications Rendering', function () {
         const notification = getModalNotification()
         await panelNode.openNotification(notification)
 
+        // Update and Reload is put on a timer so that other methods (e.g. telemetry) can finish.
+        await clock.tickAsync(2000)
+
         assert.ok(excuteCommandStub.calledWith('workbench.extensions.installExtension', 'aws.toolkit.fake.extension'))
         assert.ok(excuteCommandStub.calledWith('workbench.action.reloadWindow'))
     })