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.

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

feat(notification): misc logging, types, fixes #6004

Merged
merged 9 commits into from
Nov 15, 2024
4 changes: 2 additions & 2 deletions packages/core/src/notifications/activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ export async function activate(
const controller = new NotificationsController(panelNode)
const engine = new RuleEngine(await getRuleContext(context, initialState))

void controller.pollForStartUp(engine)
void controller.pollForEmergencies(engine)
await controller.pollForStartUp(engine)
await controller.pollForEmergencies(engine)

globals.clock.setInterval(async () => {
const ruleContext = await getRuleContext(context, await authStateFn())
Expand Down
10 changes: 4 additions & 6 deletions packages/core/src/notifications/controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,22 @@ import { telemetry } from '../shared/telemetry/telemetry'
export class NotificationsController {
public static readonly suggestedPollIntervalMs = 1000 * 60 * 10 // 10 minutes

public readonly storageKey: globalKey

/** Internal memory state that is written to global state upon modification. */
private readonly state: NotificationsState

static #instance: NotificationsController | undefined

constructor(
private readonly notificationsNode: NotificationsNode,
private readonly fetcher: NotificationFetcher = new RemoteFetcher()
private readonly fetcher: NotificationFetcher = new RemoteFetcher(),
public readonly storageKey: globalKey = 'aws.notifications'
) {
if (!NotificationsController.#instance) {
// Register on first creation only.
registerDismissCommand()
}
NotificationsController.#instance = this

this.storageKey = 'aws.notifications'
this.state = globals.globalState.tryGet<NotificationsState>(this.storageKey, NotificationsStateConstructor, {
startUp: {},
emergency: {},
Expand Down Expand Up @@ -94,7 +92,7 @@ export class NotificationsController {
ruleEngine.shouldDisplayNotification(n)
)

NotificationsNode.instance.setNotifications(startUp, emergency)
await NotificationsNode.instance.setNotifications(startUp, emergency)

// Emergency notifications can't be dismissed, but if the user minimizes the panel then
// we don't want to focus it each time we set the notification nodes.
Expand Down Expand Up @@ -128,7 +126,7 @@ export class NotificationsController {
this.state.dismissed.push(notificationId)
await this.writeState()

NotificationsNode.instance.dismissStartUpNotification(notificationId)
await NotificationsNode.instance.dismissStartUpNotification(notificationId)
}

/**
Expand Down
88 changes: 64 additions & 24 deletions packages/core/src/notifications/panelNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,31 @@
*/

import * as vscode from 'vscode'
import * as nls from 'vscode-nls'
import { ResourceTreeDataProvider, TreeNode } from '../shared/treeview/resourceTreeDataProvider'
import { Command, Commands } from '../shared/vscode/commands2'
import { Icon, IconPath, getIcon } from '../shared/icons'
import { Icon, getIcon } from '../shared/icons'
import { contextKey, setContext } from '../shared/vscode/setContext'
import { NotificationType, ToolkitNotification, getNotificationTelemetryId } from './types'
import { NotificationType, OnReceiveType, ToolkitNotification, getNotificationTelemetryId } from './types'
import { ToolkitError } from '../shared/errors'
import { isAmazonQ } from '../shared/extensionUtilities'
import { getLogger } from '../shared/logger/logger'
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 localize = nls.loadMessageBundle()
const logger = getLogger('notifications')
justinmk3 marked this conversation as resolved.
Show resolved Hide resolved

/**
* Controls the "Notifications" side panel/tree in each extension. It takes purely UX actions
* and does not determine what notifications to dispaly or how to fetch and store them.
*/
export class NotificationsNode implements TreeNode {
public static readonly title = localize('AWS.notifications.title', 'Notifications')

public readonly id = 'notifications'
public readonly resource = this
public provider?: ResourceTreeDataProvider
Expand All @@ -34,6 +41,7 @@ export class NotificationsNode implements TreeNode {
private readonly showContextStr: contextKey
private readonly startUpNodeContext: string
private readonly emergencyNodeContext: string
private view: vscode.TreeView<TreeNode> | undefined

static #instance: NotificationsNode

Expand Down Expand Up @@ -62,31 +70,49 @@ export class NotificationsNode implements TreeNode {
}

public getTreeItem() {
const item = new vscode.TreeItem('Notifications')
const item = new vscode.TreeItem(NotificationsNode.title)
item.collapsibleState = vscode.TreeItemCollapsibleState.Collapsed
item.contextValue = 'notifications'

return item
}

public refresh(): void {
const hasNotifications = this.startUpNotifications.length > 0 || this.emergencyNotifications.length > 0
void setContext(this.showContextStr, hasNotifications)
public refresh() {
const totalNotifications = this.notificationCount()
if (this.view) {
if (totalNotifications > 0) {
this.view.badge = {
tooltip: `${totalNotifications} notification${totalNotifications > 1 ? 's' : ''}`,
value: totalNotifications,
}
this.view.title = `${NotificationsNode.title} (${totalNotifications})`
} else {
this.view.badge = undefined
this.view.title = NotificationsNode.title
}
} else {
logger.warn('NotificationsNode was refreshed but the view was not initialized!')
}

this.provider?.refresh()
return setContext(this.showContextStr, totalNotifications > 0)
}

public getChildren() {
const buildNode = (n: ToolkitNotification, type: NotificationType) => {
const icon: Icon | IconPath =
type === 'startUp'
? getIcon('vscode-question')
: { ...getIcon('vscode-alert'), color: new vscode.ThemeColor('errorForeground') }
const icon: Icon =
type === 'emergency'
? Object.assign(getIcon('vscode-alert') as Icon, {
color: new vscode.ThemeColor('errorForeground'),
})
: (getIcon('vscode-question') as Icon)

const title = n.uiRenderInstructions.content['en-US'].title
return this.openNotificationCmd.build(n).asTreeNode({
label: n.uiRenderInstructions.content['en-US'].title,
label: title,
tooltip: title,
iconPath: icon,
contextValue: type === 'startUp' ? this.startUpNodeContext : this.emergencyNodeContext,
tooltip: 'Click to open',
})
}

Expand All @@ -100,10 +126,10 @@ export class NotificationsNode implements TreeNode {
* Sets the current list of notifications. Nodes are generated for each notification.
* No other processing is done, see NotificationController.
*/
public setNotifications(startUp: ToolkitNotification[], emergency: ToolkitNotification[]) {
public async setNotifications(startUp: ToolkitNotification[], emergency: ToolkitNotification[]) {
this.startUpNotifications = startUp
this.emergencyNotifications = emergency
this.refresh()
await this.refresh()
}

/**
Expand All @@ -112,9 +138,9 @@ export class NotificationsNode implements TreeNode {
*
* Only dismisses startup notifications.
*/
public dismissStartUpNotification(id: string) {
public async dismissStartUpNotification(id: string) {
this.startUpNotifications = this.startUpNotifications.filter((n) => n.id !== id)
this.refresh()
await this.refresh()
}

/**
Expand All @@ -124,6 +150,10 @@ export class NotificationsNode implements TreeNode {
return vscode.commands.executeCommand(this.focusCmdStr)
}

private notificationCount() {
return this.startUpNotifications.length + this.emergencyNotifications.length
}

/**
* Fired when a notification is clicked on in the panel. It will run any rendering
* instructions included in the notification. See {@link ToolkitNotification.uiRenderInstructions}.
Expand All @@ -132,23 +162,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(
Expand All @@ -165,7 +195,11 @@ export class NotificationsNode implements TreeNode {
* Can be either a blocking modal or a bottom-right corner toast
* Handles the button click actions based on the button type.
*/
private showInformationWindow(notification: ToolkitNotification, type: string = 'toast', passive: boolean = false) {
private showInformationWindow(
notification: ToolkitNotification,
type: OnReceiveType = 'toast',
passive: boolean = false
) {
const isModal = type === 'modal'

// modal has to have defined actions (buttons)
Expand Down Expand Up @@ -203,7 +237,10 @@ export class NotificationsNode implements TreeNode {
)
break
case 'updateAndReload':
await this.updateAndReload(notification.displayIf.extensionId)
// Give things time to finish executing.
globals.clock.setTimeout(() => {
return this.updateAndReload(notification.displayIf.extensionId)
}, 1000)
Copy link
Contributor

@justinmk3 justinmk3 Nov 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there state that can be checked & retried via waitUntil ?

Copy link
Contributor Author

@hayemaxi hayemaxi Nov 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some arbitrary amount of telemetry, e.g. a ui_click and the run block it is currently in. Maybe we could check this telemetry state. A tradeoff between complexity <> and potential (unlikely) race conditions.

No, there is an arbitrary amount of async telemetry. It needs to wait for this block to complete at least. Then there may be other telemetry depending on how this notification was invoked (e.g. ui_click)

break
case 'openUrl':
if (selectedButton.url) {
Expand All @@ -228,7 +265,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')
}
Expand Down Expand Up @@ -258,14 +299,13 @@ export class NotificationsNode implements TreeNode {
}

registerView(context: vscode.ExtensionContext) {
const view = registerToolView(
this.view = registerToolView(
{
nodes: [this],
view: isAmazonQ() ? 'aws.amazonq.notifications' : 'aws.toolkit.notifications',
refreshCommands: [(provider: ResourceTreeDataProvider) => this.registerProvider(provider)],
},
context
)
view.message = `New feature announcements and emergency notifications for ${isAmazonQ() ? 'Amazon Q' : 'AWS Toolkit'} will appear here.`
}
}
33 changes: 26 additions & 7 deletions packages/core/src/notifications/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
*
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -134,8 +155,6 @@ export class RuleEngine {
return hasAnyOfExpected(this.context.authStates)
case 'AuthScopes':
return isEqualSetToExpected(this.context.authScopes)
case 'InstalledExtensions':
return isSuperSetOfExpected(this.context.installedExtensions)
case 'ActiveExtensions':
return isSuperSetOfExpected(this.context.activeExtensions)
default:
Expand Down Expand Up @@ -169,16 +188,16 @@ export async function getRuleContext(context: vscode.ExtensionContext, authState
computeEnv: await getComputeEnvType(),
authTypes: [...new Set(authTypes)],
authScopes: authState.authScopes ? authState.authScopes?.split(',') : [],
installedExtensions: vscode.extensions.all.map((e) => e.id),
activeExtensions: vscode.extensions.all.filter((e) => e.isActive).map((e) => e.id),

// Toolkit (and eventually Q?) may have multiple connections with different regions and states.
// However, this granularity does not seem useful at this time- only the active connection is considered.
authRegions: authState.awsRegion ? [authState.awsRegion] : [],
authStates: [authState.authStatus],
}
const { activeExtensions, installedExtensions, ...loggableRuleContext } = ruleContext
getLogger('notifications').debug('getRuleContext() determined rule context: %O', loggableRuleContext)

const { activeExtensions, ...loggableRuleContext } = ruleContext
logger.debug('getRuleContext() determined rule context: %O', loggableRuleContext)

return ruleContext
}
Loading
Loading