From 142d1feb9338d30ebe6779fbf287666d0b92dcd9 Mon Sep 17 00:00:00 2001 From: Erli Suo Date: Fri, 27 Jan 2023 11:59:07 -0800 Subject: [PATCH 01/13] Init check in --- ...tyStatusTelemetry.sendingToSendFailed.html | 71 +++++++++++++ ...vityStatusTelemetry.sendingToSendFailed.js | 6 ++ ...usTelemetry.sendingToSendFailedToSent.html | 99 +++++++++++++++++++ ...atusTelemetry.sendingToSendFailedToSent.js | 6 ++ ...activityStatusTelemetry.sendingToSent.html | 61 ++++++++++++ .../activityStatusTelemetry.sendingToSent.js | 6 ++ packages/api/src/hooks/Composer.tsx | 5 +- .../ActivitySendStatusTelemetryComposer.tsx | 35 +++++++ 8 files changed, 288 insertions(+), 1 deletion(-) create mode 100644 __tests__/html/activityStatusTelemetry.sendingToSendFailed.html create mode 100644 __tests__/html/activityStatusTelemetry.sendingToSendFailed.js create mode 100644 __tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.html create mode 100644 __tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.js create mode 100644 __tests__/html/activityStatusTelemetry.sendingToSent.html create mode 100644 __tests__/html/activityStatusTelemetry.sendingToSent.js create mode 100644 packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx diff --git a/__tests__/html/activityStatusTelemetry.sendingToSendFailed.html b/__tests__/html/activityStatusTelemetry.sendingToSendFailed.html new file mode 100644 index 0000000000..157780592a --- /dev/null +++ b/__tests__/html/activityStatusTelemetry.sendingToSendFailed.html @@ -0,0 +1,71 @@ + + + + + + + + + +
+ + + diff --git a/__tests__/html/activityStatusTelemetry.sendingToSendFailed.js b/__tests__/html/activityStatusTelemetry.sendingToSendFailed.js new file mode 100644 index 0000000000..e17c27c967 --- /dev/null +++ b/__tests__/html/activityStatusTelemetry.sendingToSendFailed.js @@ -0,0 +1,6 @@ +/** @jest-environment ./packages/test/harness/src/host/jest/WebDriverEnvironment.js */ + +describe('ActivityStatusTelemetry', () => { + test('activity status telemetry logged when activity status changed from "sending" to "send failed"', () => + runHTML('activityStatusTelemetry.sendingToSendFailed.html')); +}); diff --git a/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.html b/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.html new file mode 100644 index 0000000000..c916ab444f --- /dev/null +++ b/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.html @@ -0,0 +1,99 @@ + + + + + + + + + +
+ + + diff --git a/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.js b/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.js new file mode 100644 index 0000000000..021b06b47f --- /dev/null +++ b/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.js @@ -0,0 +1,6 @@ +/** @jest-environment ./packages/test/harness/src/host/jest/WebDriverEnvironment.js */ + +describe('ActivityStatusTelemetry', () => { + test('activity status telemetry logged when activity status changed from "sending" to "send failed" to "sent"', () => + runHTML('activityStatusTelemetry.sendingToSendFailedToSent.html')); +}); diff --git a/__tests__/html/activityStatusTelemetry.sendingToSent.html b/__tests__/html/activityStatusTelemetry.sendingToSent.html new file mode 100644 index 0000000000..2217906894 --- /dev/null +++ b/__tests__/html/activityStatusTelemetry.sendingToSent.html @@ -0,0 +1,61 @@ + + + + + + + + + +
+ + + diff --git a/__tests__/html/activityStatusTelemetry.sendingToSent.js b/__tests__/html/activityStatusTelemetry.sendingToSent.js new file mode 100644 index 0000000000..6985040614 --- /dev/null +++ b/__tests__/html/activityStatusTelemetry.sendingToSent.js @@ -0,0 +1,6 @@ +/** @jest-environment ./packages/test/harness/src/host/jest/WebDriverEnvironment.js */ + +describe('ActivityStatusTelemetry', () => { + test('activity status telemetry logged when activity status changed from "sending" to "sent"', () => + runHTML('activityStatusTelemetry.sendingToSent.html')); +}); diff --git a/packages/api/src/hooks/Composer.tsx b/packages/api/src/hooks/Composer.tsx index 69846813c6..a136d1cae5 100644 --- a/packages/api/src/hooks/Composer.tsx +++ b/packages/api/src/hooks/Composer.tsx @@ -36,6 +36,7 @@ import ActivityAcknowledgementComposer from '../providers/ActivityAcknowledgemen import ActivityKeyerComposer from '../providers/ActivityKeyer/ActivityKeyerComposer'; import ActivityMiddleware from '../types/ActivityMiddleware'; import ActivitySendStatusComposer from '../providers/ActivitySendStatus/ActivitySendStatusComposer'; +import ActivitySendStatusTelemetryComposer from '../providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer'; import AttachmentForScreenReaderMiddleware from '../types/AttachmentForScreenReaderMiddleware'; import AttachmentMiddleware from '../types/AttachmentMiddleware'; import AvatarMiddleware from '../types/AvatarMiddleware'; @@ -611,7 +612,9 @@ const ComposerCore: FC = ({ return ( - {typeof children === 'function' ? children(context) : children} + + {typeof children === 'function' ? children(context) : children} + {onTelemetry && } diff --git a/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx b/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx new file mode 100644 index 0000000000..603b54ccae --- /dev/null +++ b/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx @@ -0,0 +1,35 @@ +// import React, { FC, PropsWithChildren, ReactNode } from 'react'; + +import { useGetActivityByKey, useSendStatusByActivityKey, useTrackEvent } from '../../hooks'; + +import usePrevious from '../../hooks/internal/usePrevious'; + +const ActivitySendStatusTelemetryComposer: React.FC<{ children?: any }> = ({ children }) => { + const [activityToSendStatusMap] = useSendStatusByActivityKey(); + const previousActivityToSendStatusMap = usePrevious(activityToSendStatusMap); + const getActivityByKey = useGetActivityByKey(); + const trackEvent = useTrackEvent(); + + if (activityToSendStatusMap && previousActivityToSendStatusMap) { + const allActivityKeys = activityToSendStatusMap.keys(); + + for (const key of allActivityKeys) { + const currentStatus = activityToSendStatusMap.get(key); + const previousStatus = previousActivityToSendStatusMap.get(key); + + if (!!currentStatus && !!previousStatus && currentStatus !== previousStatus) { + const activity = getActivityByKey(key); + const telemetryPayload = { + currentStatus: currentStatus.toString(), + previousStatus: previousStatus.toString(), + clientActivityID: activity?.channelData?.clientActivityID, + type: activity?.type?.toString() + }; + trackEvent && trackEvent('send-status:change', telemetryPayload); + } + } + } + return children; +}; + +export default ActivitySendStatusTelemetryComposer; From d358db1bd151860621996408953fd8e1dcb5e5e5 Mon Sep 17 00:00:00 2001 From: Erli Suo Date: Fri, 27 Jan 2023 15:17:25 -0800 Subject: [PATCH 02/13] updating composer structure --- CHANGELOG.md | 4 ++++ packages/api/src/hooks/Composer.tsx | 5 ++--- .../ActivitySendStatusTelemetryComposer.tsx | 9 ++++----- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a19a73286..bf6caad2f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added +- Added function to emit status change telemetry event for activities. PR [#4627](https://github.com/microsoft/BotFramework-WebChat/pull/4627) [@Erli-ms](https://github.com/Erli-ms) + +### Added + - Added ability for developers to customize Web Chat by extending the default UI without having to re-implement existing components. [@dawolff-ms](https://github.com/dawolff-ms) in PR [#4539](https://github.com/microsoft/BotFramework-WebChat/pull/4539) ### Fixed diff --git a/packages/api/src/hooks/Composer.tsx b/packages/api/src/hooks/Composer.tsx index a136d1cae5..38a42aee2f 100644 --- a/packages/api/src/hooks/Composer.tsx +++ b/packages/api/src/hooks/Composer.tsx @@ -612,9 +612,8 @@ const ComposerCore: FC = ({ return ( - - {typeof children === 'function' ? children(context) : children} - + {typeof children === 'function' ? children(context) : children} + {onTelemetry && } diff --git a/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx b/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx index 603b54ccae..78e2bca98b 100644 --- a/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx +++ b/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx @@ -1,10 +1,8 @@ -// import React, { FC, PropsWithChildren, ReactNode } from 'react'; - import { useGetActivityByKey, useSendStatusByActivityKey, useTrackEvent } from '../../hooks'; import usePrevious from '../../hooks/internal/usePrevious'; -const ActivitySendStatusTelemetryComposer: React.FC<{ children?: any }> = ({ children }) => { +const ActivitySendStatusTelemetryComposer = () => { const [activityToSendStatusMap] = useSendStatusByActivityKey(); const previousActivityToSendStatusMap = usePrevious(activityToSendStatusMap); const getActivityByKey = useGetActivityByKey(); @@ -23,13 +21,14 @@ const ActivitySendStatusTelemetryComposer: React.FC<{ children?: any }> = ({ chi currentStatus: currentStatus.toString(), previousStatus: previousStatus.toString(), clientActivityID: activity?.channelData?.clientActivityID, - type: activity?.type?.toString() + type: activity?.type?.toString(), + key }; trackEvent && trackEvent('send-status:change', telemetryPayload); } } } - return children; + return null; }; export default ActivitySendStatusTelemetryComposer; From fa22eb79cb73d16d3ab03fe4613352ebcdcce225 Mon Sep 17 00:00:00 2001 From: Erli Suo Date: Tue, 31 Jan 2023 16:11:52 -0800 Subject: [PATCH 03/13] adding more tests --- ...y.ignoreStatusChange.incomingActivity.html | 51 ++++++++++++ ...try.ignoreStatusChange.incomingActivity.js | 6 ++ ...sendFiles.attachmentUrl.sendingToSent.html | 82 +++++++++++++++++++ ...y.sendFiles.attachmentUrl.sendingToSent.js | 4 + ...tyStatusTelemetry.sendingToSendFailed.html | 22 +++-- ...usTelemetry.sendingToSendFailedToSent.html | 18 ++-- ...activityStatusTelemetry.sendingToSent.html | 8 +- .../ActivitySendStatusTelemetryComposer.tsx | 3 +- 8 files changed, 173 insertions(+), 21 deletions(-) create mode 100644 __tests__/html/activityStatusTelemetry.ignoreStatusChange.incomingActivity.html create mode 100644 __tests__/html/activityStatusTelemetry.ignoreStatusChange.incomingActivity.js create mode 100644 __tests__/html/activityStatusTelemetry.sendFiles.attachmentUrl.sendingToSent.html create mode 100644 __tests__/html/activityStatusTelemetry.sendFiles.attachmentUrl.sendingToSent.js diff --git a/__tests__/html/activityStatusTelemetry.ignoreStatusChange.incomingActivity.html b/__tests__/html/activityStatusTelemetry.ignoreStatusChange.incomingActivity.html new file mode 100644 index 0000000000..3d104e11a7 --- /dev/null +++ b/__tests__/html/activityStatusTelemetry.ignoreStatusChange.incomingActivity.html @@ -0,0 +1,51 @@ + + + + + + + + + +
+ + + diff --git a/__tests__/html/activityStatusTelemetry.ignoreStatusChange.incomingActivity.js b/__tests__/html/activityStatusTelemetry.ignoreStatusChange.incomingActivity.js new file mode 100644 index 0000000000..45852d9685 --- /dev/null +++ b/__tests__/html/activityStatusTelemetry.ignoreStatusChange.incomingActivity.js @@ -0,0 +1,6 @@ +/** @jest-environment ./packages/test/harness/src/host/jest/WebDriverEnvironment.js */ + +describe('ActivityStatusTelemetry', () => { + test('activity status telemetry should not be logged when incoming activity received', () => + runHTML('activityStatusTelemetry.ignoreStatusChange.incomingActivity.html')); +}); diff --git a/__tests__/html/activityStatusTelemetry.sendFiles.attachmentUrl.sendingToSent.html b/__tests__/html/activityStatusTelemetry.sendFiles.attachmentUrl.sendingToSent.html new file mode 100644 index 0000000000..185960d668 --- /dev/null +++ b/__tests__/html/activityStatusTelemetry.sendFiles.attachmentUrl.sendingToSent.html @@ -0,0 +1,82 @@ + + + + + + + + + +
+ + + diff --git a/__tests__/html/activityStatusTelemetry.sendFiles.attachmentUrl.sendingToSent.js b/__tests__/html/activityStatusTelemetry.sendFiles.attachmentUrl.sendingToSent.js new file mode 100644 index 0000000000..d99b4c3760 --- /dev/null +++ b/__tests__/html/activityStatusTelemetry.sendFiles.attachmentUrl.sendingToSent.js @@ -0,0 +1,4 @@ +/** @jest-environment ./packages/test/harness/src/host/jest/WebDriverEnvironment.js */ + +test('text url attachment sent from bot should trigger status change telemetry event', () => runHTML('activityStatusTelemetry.sendFiles.attachmentUrl.sendingToSent.html') +); diff --git a/__tests__/html/activityStatusTelemetry.sendingToSendFailed.html b/__tests__/html/activityStatusTelemetry.sendingToSendFailed.html index 157780592a..5c57052591 100644 --- a/__tests__/html/activityStatusTelemetry.sendingToSendFailed.html +++ b/__tests__/html/activityStatusTelemetry.sendingToSendFailed.html @@ -31,10 +31,10 @@ await pageConditions.uiConnected(); - const sendMessage1stAttempt = await directLine.emulateOutgoingActivity('Egress message 1'); + const sendMessage1stAttempt = await directLine.emulateOutgoingActivity('Egress message 1, sending to send failed'); sendMessage1stAttempt.rejectPostActivity(new Error('artificial error for egress message 1')); - const sendMessage2ndAttempt = await directLine.emulateOutgoingActivity('Egress message 2'); + const sendMessage2ndAttempt = await directLine.emulateOutgoingActivity('Egress message 2, sending to send failed'); sendMessage2ndAttempt.rejectPostActivity(new Error('artificial error for egress message 2')); await pageConditions.became( @@ -46,22 +46,26 @@ 1000 ); - const expectedEventName = "send-status:change"; + const expectedEventName = 'send-status:change'; let lastRecord = telemetryEventTracking.pop(); expect(lastRecord.data).toEqual({ - currentStatus: "send failed", - previousStatus: "sending", + currentStatus: 'send failed', + previousStatus: 'sending', clientActivityID: expect.any(String), - type: "message" + type: 'message', + key: expect.any(String), + hasAttachment: 'false' }); expect(lastRecord.name).toEqual(expectedEventName); lastRecord = telemetryEventTracking.pop(); expect(lastRecord.data).toEqual({ - currentStatus: "send failed", - previousStatus: "sending", + currentStatus: 'send failed', + previousStatus: 'sending', clientActivityID: expect.any(String), - type: "message" + type: 'message', + key: expect.any(String), + hasAttachment: 'false' }); expect(lastRecord.name).toEqual(expectedEventName); }, diff --git a/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.html b/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.html index c916ab444f..1a88275adb 100644 --- a/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.html +++ b/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.html @@ -74,23 +74,27 @@ // // THEN: `channelData.state` should be "sent". await activityChannelDataStateBecame('sent'); - const expectedEventName = "send-status:change"; + const expectedEventName = 'send-status:change'; let lastRecord = telemetryEventTracking.pop(); expect(lastRecord.data).toEqual({ - previousStatus: "send failed", - currentStatus: "sent", + previousStatus: 'send failed', + currentStatus: 'sent', clientActivityID: expect.any(String), - type: "message" + type: 'message', + key: expect.any(String), + hasAttachment: 'false' }); expect(lastRecord.name).toEqual(expectedEventName); lastRecord = telemetryEventTracking.pop(); expect(lastRecord.data).toEqual({ - currentStatus: "send failed", - previousStatus: "sending", + currentStatus: 'send failed', + previousStatus: 'sending', clientActivityID: expect.any(String), - type: "message" + type: 'message', + key: expect.any(String), + hasAttachment: 'false' }); expect(lastRecord.name).toEqual(expectedEventName); }); diff --git a/__tests__/html/activityStatusTelemetry.sendingToSent.html b/__tests__/html/activityStatusTelemetry.sendingToSent.html index 2217906894..1f732c3094 100644 --- a/__tests__/html/activityStatusTelemetry.sendingToSent.html +++ b/__tests__/html/activityStatusTelemetry.sendingToSent.html @@ -36,22 +36,22 @@ sendMessage1stAttempt.resolvePostActivity(); await pageConditions.became( - 'failed to send message', + 'successfully sent message', () => { return pageElements.activityStatuses()[0]?.innerText.indexOf('Just now') !== -1 }, 1000 ); - console.log("debugging activity status: ", pageElements.activityStatuses()); - const expectedEventName = "send-status:change"; let lastRecord = telemetryEventTracking.pop(); expect(lastRecord.data).toEqual({ currentStatus: "sent", previousStatus: "sending", clientActivityID: expect.any(String), - type: "message" + type: "message", + key: expect.any(String), + hasAttachment: "false" }); expect(lastRecord.name).toEqual(expectedEventName); }, diff --git a/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx b/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx index 78e2bca98b..5545f5c01c 100644 --- a/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx +++ b/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx @@ -22,7 +22,8 @@ const ActivitySendStatusTelemetryComposer = () => { previousStatus: previousStatus.toString(), clientActivityID: activity?.channelData?.clientActivityID, type: activity?.type?.toString(), - key + key, + hasAttachment: activity.type === 'message' && activity.attachments?.length > 0 ? 'true' : 'false' }; trackEvent && trackEvent('send-status:change', telemetryPayload); } From ebd51afe9d16d9f32c4c4b1676b0c29e2df485e2 Mon Sep 17 00:00:00 2001 From: Erli Suo Date: Tue, 31 Jan 2023 16:22:33 -0800 Subject: [PATCH 04/13] add documentation for telemetry event --- docs/TELEMETRY.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/TELEMETRY.md b/docs/TELEMETRY.md index 864bcd2d8f..84ea5f8edf 100644 --- a/docs/TELEMETRY.md +++ b/docs/TELEMETRY.md @@ -53,9 +53,10 @@ When the following hooks are called, one or more event measurements will be emit ### Other events -| Name | Description | -| ------ | ------------------------------------------ | -| `init` | Emit when telemetry system has initialized | +| Name | Description | +| -------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `init` | Emit when telemetry system has initialized | +| `send-status:change` | Emit when activity status changes from `sending` to `sent`, `sending` to `send failed` and `send failed` to `sent`. Including `currentStatus`, `previousStatus`, `clientActivityID`, `key`, `hasAttachment` and `type` | ### Exceptions From 86d1328f43e6a683ccb982c573158ccbf947d842 Mon Sep 17 00:00:00 2001 From: Erli Suo Date: Tue, 31 Jan 2023 16:33:25 -0800 Subject: [PATCH 05/13] adding null check --- .../ActivitySendStatusTelemetryComposer.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx b/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx index 5545f5c01c..e7445e31d7 100644 --- a/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx +++ b/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx @@ -23,7 +23,7 @@ const ActivitySendStatusTelemetryComposer = () => { clientActivityID: activity?.channelData?.clientActivityID, type: activity?.type?.toString(), key, - hasAttachment: activity.type === 'message' && activity.attachments?.length > 0 ? 'true' : 'false' + hasAttachment: activity?.type === 'message' && activity?.attachments?.length > 0 ? 'true' : 'false' }; trackEvent && trackEvent('send-status:change', telemetryPayload); } From c36eb51bcba338383206ed25d2fa283f58c74905 Mon Sep 17 00:00:00 2001 From: Erli Suo Date: Wed, 1 Feb 2023 09:28:05 -0800 Subject: [PATCH 06/13] edit change log --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf6caad2f9..e2b8d4ee41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added -- Added function to emit status change telemetry event for activities. PR [#4627](https://github.com/microsoft/BotFramework-WebChat/pull/4627) [@Erli-ms](https://github.com/Erli-ms) +- Added function to emit status change telemetry event for activities. PR [#4631](https://github.com/microsoft/BotFramework-WebChat/pull/4631) [@Erli-ms](https://github.com/Erli-ms) ### Added From 8689dd15eb4c084716d04e89b894968b6c726d5d Mon Sep 17 00:00:00 2001 From: Erli Suo Date: Sat, 11 Feb 2023 13:45:13 -0800 Subject: [PATCH 07/13] address comments --- ...y.ignoreStatusChange.incomingActivity.html | 16 +- ...sendFiles.attachmentUrl.sendingToSent.html | 59 ++++---- ...tyStatusTelemetry.sendingToSendFailed.html | 64 ++++---- ...usTelemetry.sendingToSendFailedToSent.html | 137 ++++++++---------- ...activityStatusTelemetry.sendingToSent.html | 59 ++++---- docs/TELEMETRY.md | 21 ++- .../ActivitySendStatusTelemetryComposer.tsx | 43 ++++-- 7 files changed, 213 insertions(+), 186 deletions(-) diff --git a/__tests__/html/activityStatusTelemetry.ignoreStatusChange.incomingActivity.html b/__tests__/html/activityStatusTelemetry.ignoreStatusChange.incomingActivity.html index 3d104e11a7..acbc1c7ee4 100644 --- a/__tests__/html/activityStatusTelemetry.ignoreStatusChange.incomingActivity.html +++ b/__tests__/html/activityStatusTelemetry.ignoreStatusChange.incomingActivity.html @@ -19,10 +19,6 @@ { directLine, store, - styleOptions: { - groupTimestamp: 60000, - sendTimeout: 5000 - }, onTelemetry: (event) => { if (event?.name === 'send-status:change') { telemetryEventTracking.push(event); @@ -34,17 +30,15 @@ await pageConditions.uiConnected(); + //GIVEN: Simulate an ingress activity await directLine.emulateIncomingActivity('Aloha!'); - await pageConditions.became( - 'send message successfully', - () => pageElements.activityStatuses()[0]?.innerText.indexOf('Just now') !== -1, - 1000 - ); + //WHEN: The activity rendered + await pageConditions.numActivitiesShown(1); + //THEN: No status change event should be logged expect(telemetryEventTracking.length).toEqual(0); - }, - { ignoreErrors: true } + } ); diff --git a/__tests__/html/activityStatusTelemetry.sendFiles.attachmentUrl.sendingToSent.html b/__tests__/html/activityStatusTelemetry.sendFiles.attachmentUrl.sendingToSent.html index 185960d668..9685e4e82a 100644 --- a/__tests__/html/activityStatusTelemetry.sendFiles.attachmentUrl.sendingToSent.html +++ b/__tests__/html/activityStatusTelemetry.sendFiles.attachmentUrl.sendingToSent.html @@ -34,49 +34,50 @@ const store = testHelpers.createStore(); const directLine = testHelpers.createDirectLineEmulator(store); - const telemetryEventTracking = []; + WebChat.renderWebChat( { directLine, - store, onTelemetry: (event) => { - telemetryEventTracking.push(event); + if (event?.name === 'send-status:change') { + telemetryEventTracking.push(event.data); + } }, - styleOptions: { - sendTimeout: 5000 - } + store }, document.getElementById('webchat') ); await pageConditions.uiConnected(); - const sendMessage1stAttempt = await directLine.emulateOutgoingActivity(testAttachmentActivity); - await sendMessage1stAttempt.echoBack(); - sendMessage1stAttempt.resolvePostActivity(); + //GIVEN: send an attachment outgoing activity and resolve it afterwards + await( + await directLine.emulateOutgoingActivity(testAttachmentActivity) + ).resolveAll(); - await pageConditions.became( - 'successfully sent message', - () => { - return pageElements.activityStatuses()[0]?.innerText.indexOf('Just now') !== -1 - }, - 1000 - ); + //WHEN: Activity displayed + await pageConditions.numActivitiesShown(1); - const expectedEventName = 'send-status:change'; - let lastRecord = telemetryEventTracking.pop(); - expect(lastRecord.data).toEqual({ - currentStatus: "sent", - previousStatus: "sending", - clientActivityID: expect.any(String), - type: "message", - key: expect.any(String), - hasAttachment: "true" - }); - expect(lastRecord.name).toEqual(expectedEventName); - }, - { ignoreErrors: true }); + //THEN: The status change event emitted: 'undefined -> sending' and 'sending -> sent' + expect(telemetryEventTracking).toEqual(expect.arrayContaining([ + { + clientActivityID: expect.any(String), + hasAttachment: 'true', + key: expect.any(String), + status: 'sending', + type: 'message' + }, + { + clientActivityID: expect.any(String), + hasAttachment: 'true', + key: expect.any(String), + prevStatus: 'sending', + status: 'sent', + type: 'message' + } + ])); + }); diff --git a/__tests__/html/activityStatusTelemetry.sendingToSendFailed.html b/__tests__/html/activityStatusTelemetry.sendingToSendFailed.html index 5c57052591..66da6cc93c 100644 --- a/__tests__/html/activityStatusTelemetry.sendingToSendFailed.html +++ b/__tests__/html/activityStatusTelemetry.sendingToSendFailed.html @@ -20,10 +20,9 @@ directLine, store, onTelemetry: (event) => { - telemetryEventTracking.push(event); - }, - styleOptions: { - sendTimeout: 5000 + if (event.name == 'send-status:change') { + telemetryEventTracking.push(event.data); + } } }, document.getElementById('webchat') @@ -31,12 +30,14 @@ await pageConditions.uiConnected(); + //GIVEN: Send out 2 outgoing messages and reject afterwards const sendMessage1stAttempt = await directLine.emulateOutgoingActivity('Egress message 1, sending to send failed'); sendMessage1stAttempt.rejectPostActivity(new Error('artificial error for egress message 1')); const sendMessage2ndAttempt = await directLine.emulateOutgoingActivity('Egress message 2, sending to send failed'); sendMessage2ndAttempt.rejectPostActivity(new Error('artificial error for egress message 2')); + //WHEN: The activities become 'Send failed' await pageConditions.became( 'failed to send message', () => { @@ -46,28 +47,39 @@ 1000 ); - const expectedEventName = 'send-status:change'; - let lastRecord = telemetryEventTracking.pop(); - expect(lastRecord.data).toEqual({ - currentStatus: 'send failed', - previousStatus: 'sending', - clientActivityID: expect.any(String), - type: 'message', - key: expect.any(String), - hasAttachment: 'false' - }); - expect(lastRecord.name).toEqual(expectedEventName); - - lastRecord = telemetryEventTracking.pop(); - expect(lastRecord.data).toEqual({ - currentStatus: 'send failed', - previousStatus: 'sending', - clientActivityID: expect.any(String), - type: 'message', - key: expect.any(String), - hasAttachment: 'false' - }); - expect(lastRecord.name).toEqual(expectedEventName); + //THEN: 2 'undefined to sending' events and 2 'sending to send failed' events must be emitted + expect(telemetryEventTracking).toEqual(expect.arrayContaining([ + { + clientActivityID: expect.any(String), + hasAttachment: 'false', + key: expect.any(String), + status: 'sending', + type: 'message' + }, + { + clientActivityID: expect.any(String), + hasAttachment: 'false', + key: expect.any(String), + status: 'sending', + type: 'message' + }, + { + clientActivityID: expect.any(String), + hasAttachment: 'false', + key: expect.any(String), + prevStatus: 'sending', + status: 'send failed', + type: 'message' + }, + { + clientActivityID: expect.any(String), + hasAttachment: 'false', + key: expect.any(String), + prevStatus: 'sending', + status: 'send failed', + type: 'message' + }, + ])); }, { ignoreErrors: true }); diff --git a/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.html b/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.html index 1a88275adb..71314321c1 100644 --- a/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.html +++ b/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.html @@ -9,94 +9,85 @@
diff --git a/__tests__/html/activityStatusTelemetry.sendingToSent.html b/__tests__/html/activityStatusTelemetry.sendingToSent.html index 1f732c3094..63ab78bf43 100644 --- a/__tests__/html/activityStatusTelemetry.sendingToSent.html +++ b/__tests__/html/activityStatusTelemetry.sendingToSent.html @@ -13,49 +13,50 @@ const store = testHelpers.createStore(); const directLine = testHelpers.createDirectLineEmulator(store); - const telemetryEventTracking = []; + WebChat.renderWebChat( { directLine, - store, onTelemetry: (event) => { - telemetryEventTracking.push(event); + if (event?.name === 'send-status:change') { + telemetryEventTracking.push(event.data); + } }, - styleOptions: { - sendTimeout: 5000 - } + store }, document.getElementById('webchat') ); await pageConditions.uiConnected(); - const sendMessage1stAttempt = await directLine.emulateOutgoingActivity('sending to sent'); - await sendMessage1stAttempt.echoBack(); - sendMessage1stAttempt.resolvePostActivity(); + //GIVEN: send an attachment outgoing activity and resolve it afterwards + await( + await directLine.emulateOutgoingActivity('sending to sent') + ).resolveAll(); - await pageConditions.became( - 'successfully sent message', - () => { - return pageElements.activityStatuses()[0]?.innerText.indexOf('Just now') !== -1 - }, - 1000 - ); + //WHEN: Activity displayed + await pageConditions.numActivitiesShown(1); - const expectedEventName = "send-status:change"; - let lastRecord = telemetryEventTracking.pop(); - expect(lastRecord.data).toEqual({ - currentStatus: "sent", - previousStatus: "sending", - clientActivityID: expect.any(String), - type: "message", - key: expect.any(String), - hasAttachment: "false" - }); - expect(lastRecord.name).toEqual(expectedEventName); - }, - { ignoreErrors: true }); + //THEN: The status change event emitted: 'undefined -> sending' and 'sending -> sent' + expect(telemetryEventTracking).toEqual(expect.arrayContaining([ + { + clientActivityID: expect.any(String), + hasAttachment: 'false', + key: expect.any(String), + status: 'sending', + type: 'message' + }, + { + clientActivityID: expect.any(String), + hasAttachment: 'false', + key: expect.any(String), + prevStatus: 'sending', + status: 'sent', + type: 'message' + } + ])); + }); diff --git a/docs/TELEMETRY.md b/docs/TELEMETRY.md index 84ea5f8edf..ca17b62907 100644 --- a/docs/TELEMETRY.md +++ b/docs/TELEMETRY.md @@ -53,10 +53,10 @@ When the following hooks are called, one or more event measurements will be emit ### Other events -| Name | Description | -| -------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `init` | Emit when telemetry system has initialized | -| `send-status:change` | Emit when activity status changes from `sending` to `sent`, `sending` to `send failed` and `send failed` to `sent`. Including `currentStatus`, `previousStatus`, `clientActivityID`, `key`, `hasAttachment` and `type` | +| Name | Description | +| -------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `init` | Emit when telemetry system has initialized | +| `send-status:change` | Emit when activity status changes from `undefined` to `sending`, `sending` to `sent`, `sending` to `send failed` and `send failed` to `sent`. Including `status`, `prevStatus`, `clientActivityID`, `key`, `hasAttachment` and `type` | ### Exceptions @@ -120,6 +120,19 @@ interface TelemetryTimingEndMeasurementEvent extends TelemetryMeasurementEvent { } ``` +To collect `send-status:change` events, the data emitted will be in the type below: + +```ts +type TelemetrySendStatusChangePayload = { + clientActivityID?: string; + hasAttachment?: 'true' | 'false'; + key: string; + prevStatus?: 'sending' | 'send failed' | 'sent'; + status: 'sending' | 'send failed' | 'sent'; + type?: string; +}; +``` + Web Chat may emit a large number of dimensions and measurements to your `onTelemetry` handler. As your telemetry service provider may limit number of dimensions and measurements for a single session or property, you are advised to pick and choose the data you needed before transmitting them to your provider. ## Hooks diff --git a/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx b/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx index e7445e31d7..6e099f1c62 100644 --- a/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx +++ b/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx @@ -2,33 +2,48 @@ import { useGetActivityByKey, useSendStatusByActivityKey, useTrackEvent } from ' import usePrevious from '../../hooks/internal/usePrevious'; +type TelemetrySendStatusChangePayload = { + clientActivityID?: string; + hasAttachment?: 'true' | 'false'; + key: string; + prevStatus?: 'sending' | 'send failed' | 'sent'; + status: 'sending' | 'send failed' | 'sent'; + type?: string; +}; + const ActivitySendStatusTelemetryComposer = () => { const [activityToSendStatusMap] = useSendStatusByActivityKey(); - const previousActivityToSendStatusMap = usePrevious(activityToSendStatusMap); + const prevActivityToSendStatusMap = usePrevious(activityToSendStatusMap); const getActivityByKey = useGetActivityByKey(); const trackEvent = useTrackEvent(); - if (activityToSendStatusMap && previousActivityToSendStatusMap) { - const allActivityKeys = activityToSendStatusMap.keys(); - - for (const key of allActivityKeys) { - const currentStatus = activityToSendStatusMap.get(key); - const previousStatus = previousActivityToSendStatusMap.get(key); + if (prevActivityToSendStatusMap) { + for (const key of activityToSendStatusMap.keys()) { + const status = activityToSendStatusMap.get(key); + const prevStatus = prevActivityToSendStatusMap.get(key); - if (!!currentStatus && !!previousStatus && currentStatus !== previousStatus) { + // `status` is falsy if it is not an outgoing activity. + // `prevStatus` is undefined or a valid status, if it is undefined, it is newly added + // This telemetry data point only emit changes in outgoing activities. + if (status && status !== prevStatus) { const activity = getActivityByKey(key); - const telemetryPayload = { - currentStatus: currentStatus.toString(), - previousStatus: previousStatus.toString(), + const telemetryPayload: TelemetrySendStatusChangePayload = { clientActivityID: activity?.channelData?.clientActivityID, - type: activity?.type?.toString(), + hasAttachment: activity?.type === 'message' && activity?.attachments?.length > 0 ? 'true' : 'false', key, - hasAttachment: activity?.type === 'message' && activity?.attachments?.length > 0 ? 'true' : 'false' + status, + type: activity?.type?.toString() }; - trackEvent && trackEvent('send-status:change', telemetryPayload); + + //only add prevStatus if it is NOT null/undefined + if (prevStatus) { + telemetryPayload.prevStatus = prevStatus; + } + trackEvent('send-status:change', telemetryPayload); } } } + return null; }; From 6f02712b562f04abdf9184d33411c09f0dd8429d Mon Sep 17 00:00:00 2001 From: Erli-ms <55606459+Erli-ms@users.noreply.github.com> Date: Sat, 11 Feb 2023 13:51:52 -0800 Subject: [PATCH 08/13] Update CHANGELOG.md Co-authored-by: William Wong --- CHANGELOG.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e2b8d4ee41..716a76209b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,9 +24,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added -- Added function to emit status change telemetry event for activities. PR [#4631](https://github.com/microsoft/BotFramework-WebChat/pull/4631) [@Erli-ms](https://github.com/Erli-ms) - -### Added +- Added function to emit status change telemetry event for activities, by [@Erli-ms](https://github.com/Erli-ms), in PR [#4631](https://github.com/microsoft/BotFramework-WebChat/pull/4631) - Added ability for developers to customize Web Chat by extending the default UI without having to re-implement existing components. [@dawolff-ms](https://github.com/dawolff-ms) in PR [#4539](https://github.com/microsoft/BotFramework-WebChat/pull/4539) From 2f1a775cae8289490cd8f4813fc622a477f9d404 Mon Sep 17 00:00:00 2001 From: William Wong Date: Mon, 13 Feb 2023 18:49:43 +0000 Subject: [PATCH 09/13] Fix tests --- __tests__/hooks/useTrackDimension.js | 18 ++++++++++++---- __tests__/hooks/useTrackEvent.js | 32 +++++++++++++++++++++------- __tests__/hooks/useTrackException.js | 8 ++++--- __tests__/telemetry.js | 23 ++++++++++++-------- 4 files changed, 57 insertions(+), 24 deletions(-) diff --git a/__tests__/hooks/useTrackDimension.js b/__tests__/hooks/useTrackDimension.js index 85e946a3d6..9a8e078192 100644 --- a/__tests__/hooks/useTrackDimension.js +++ b/__tests__/hooks/useTrackDimension.js @@ -37,12 +37,18 @@ describe('useTrackDimension', () => { await pageObjects.runHook('useTrackDimension', [], trackDimension => trackDimension('hello', 'aloha')); await pageObjects.runHook('useTrackEvent', [], trackEvent => trackEvent('ping')); - await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements.length)).resolves.toBe(1); + await expect( + driver.executeScript(() => window.WebChatTest.telemetryMeasurements.filter(({ name }) => name === 'ping').length) + ).resolves.toBe(1); await pageObjects.runHook('useTrackDimension', [], trackDimension => trackDimension('hello')); await pageObjects.runHook('useTrackEvent', [], trackEvent => trackEvent('ping2')); - await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements)).resolves.toMatchInlineSnapshot(` + await expect( + driver.executeScript(() => + window.WebChatTest.telemetryMeasurements.filter(({ name }) => name === 'ping' || name === 'ping2') + ) + ).resolves.toMatchInlineSnapshot(` Array [ Object { "data": null, @@ -81,7 +87,9 @@ describe('useTrackDimension', () => { await pageObjects.runHook('useTrackDimension', [], trackDimension => trackDimension(123, 'hello')); await pageObjects.runHook('useTrackEvent', [], trackEvent => trackEvent('ping')); - await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements)).resolves.toMatchInlineSnapshot(` + await expect( + driver.executeScript(() => window.WebChatTest.telemetryMeasurements.filter(({ name }) => name === 'ping')) + ).resolves.toMatchInlineSnapshot(` Array [ Object { "data": null, @@ -105,7 +113,9 @@ describe('useTrackDimension', () => { await pageObjects.runHook('useTrackDimension', [], trackDimension => trackDimension('hello', 123)); await pageObjects.runHook('useTrackEvent', [], trackEvent => trackEvent('ping')); - await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements)).resolves.toMatchInlineSnapshot(` + await expect( + driver.executeScript(() => window.WebChatTest.telemetryMeasurements.filter(({ name }) => name === 'ping')) + ).resolves.toMatchInlineSnapshot(` Array [ Object { "data": null, diff --git a/__tests__/hooks/useTrackEvent.js b/__tests__/hooks/useTrackEvent.js index 004b63e9a7..f81f549ec5 100644 --- a/__tests__/hooks/useTrackEvent.js +++ b/__tests__/hooks/useTrackEvent.js @@ -37,7 +37,9 @@ describe('useTrackEvent', () => { test('should track simple event', async () => { await pageObjects.runHook('useTrackEvent', [], trackEvent => trackEvent('hello')); - await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements)).resolves.toMatchInlineSnapshot(` + await expect( + driver.executeScript(() => window.WebChatTest.telemetryMeasurements.filter(({ name }) => name === 'hello')) + ).resolves.toMatchInlineSnapshot(` Array [ Object { "data": null, @@ -61,7 +63,9 @@ describe('useTrackEvent', () => { test('should track simple event using info explicitly', async () => { await pageObjects.runHook('useTrackEvent', [], trackEvent => trackEvent.info('hello')); - await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements)).resolves.toMatchInlineSnapshot(` + await expect( + driver.executeScript(() => window.WebChatTest.telemetryMeasurements.filter(({ name }) => name === 'hello')) + ).resolves.toMatchInlineSnapshot(` Array [ Object { "data": null, @@ -85,7 +89,9 @@ describe('useTrackEvent', () => { test('should track numeric event', async () => { await pageObjects.runHook('useTrackEvent', [], trackEvent => trackEvent.warn('hello', 123)); - await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements)).resolves.toMatchInlineSnapshot(` + await expect( + driver.executeScript(() => window.WebChatTest.telemetryMeasurements.filter(({ name }) => name === 'hello')) + ).resolves.toMatchInlineSnapshot(` Array [ Object { "data": 123, @@ -109,7 +115,9 @@ describe('useTrackEvent', () => { test('should track numeric event', async () => { await pageObjects.runHook('useTrackEvent', [], trackEvent => trackEvent.debug('hello', 'aloha')); - await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements)).resolves.toMatchInlineSnapshot(` + await expect( + driver.executeScript(() => window.WebChatTest.telemetryMeasurements.filter(({ name }) => name === 'hello')) + ).resolves.toMatchInlineSnapshot(` Array [ Object { "data": "aloha", @@ -133,7 +141,9 @@ describe('useTrackEvent', () => { test('should track complex event', async () => { await pageObjects.runHook('useTrackEvent', [], trackEvent => trackEvent.error('hello', { one: 1, hello: 'aloha' })); - await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements)).resolves.toMatchInlineSnapshot(` + await expect( + driver.executeScript(() => window.WebChatTest.telemetryMeasurements.filter(({ name }) => name === 'hello')) + ).resolves.toMatchInlineSnapshot(` Array [ Object { "data": Object { @@ -160,18 +170,24 @@ describe('useTrackEvent', () => { test('should not track event with boolean data', async () => { await pageObjects.runHook('useTrackEvent', [], trackEvent => trackEvent('hello', true)); - await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements)).resolves.toBeFalsy(); + await expect( + driver.executeScript(() => window.WebChatTest.telemetryMeasurements.filter(({ name }) => name === 'hello')) + ).resolves.toHaveLength(0); }); test('should not track event with incompatible complex data', async () => { await pageObjects.runHook('useTrackEvent', [], trackEvent => trackEvent('hello', { truthy: true })); - await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements)).resolves.toBeFalsy(); + await expect( + driver.executeScript(() => window.WebChatTest.telemetryMeasurements.filter(({ name }) => name === 'hello')) + ).resolves.toHaveLength(0); }); test('should not track event with invalid name', async () => { await pageObjects.runHook('useTrackEvent', [], trackEvent => trackEvent(123)); - await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements)).resolves.toBeFalsy(); + await expect( + driver.executeScript(() => window.WebChatTest.telemetryMeasurements.filter(({ name }) => name === 'hello')) + ).resolves.toHaveLength(0); }); }); diff --git a/__tests__/hooks/useTrackException.js b/__tests__/hooks/useTrackException.js index ba57c55a81..aa75d00405 100644 --- a/__tests__/hooks/useTrackException.js +++ b/__tests__/hooks/useTrackException.js @@ -19,7 +19,7 @@ describe('useTrackException', () => { data, dimensions, duration, - error: error.message, + error: error?.message, fatal, name, type @@ -37,7 +37,8 @@ describe('useTrackException', () => { test('should track exception', async () => { await pageObjects.runHook('useTrackException', [], trackException => trackException(new Error('artificial error'))); - await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements)).resolves.toMatchInlineSnapshot(` + await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements.filter(({ error }) => error))) + .resolves.toMatchInlineSnapshot(` Array [ Object { "data": null, @@ -63,7 +64,8 @@ describe('useTrackException', () => { trackException(new Error('non-fatal error'), false) ); - await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements)).resolves.toMatchInlineSnapshot(` + await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements.filter(({ error }) => error))) + .resolves.toMatchInlineSnapshot(` Array [ Object { "data": null, diff --git a/__tests__/telemetry.js b/__tests__/telemetry.js index 547ea89415..2a3c8efb1e 100644 --- a/__tests__/telemetry.js +++ b/__tests__/telemetry.js @@ -31,7 +31,9 @@ describe('telemetry', () => { await driver.wait(uiConnected(), timeouts.directLine); - await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements)).resolves.toMatchInlineSnapshot(` + await expect( + driver.executeScript(() => window.WebChatTest.telemetryMeasurements.filter(({ name }) => name === 'init')) + ).resolves.toMatchInlineSnapshot(` Array [ Object { "data": null, @@ -56,15 +58,18 @@ describe('telemetry', () => { test('should collect fatal error', async () => { const { driver, pageObjects } = await setupWebDriver({ props: { - activityMiddleware: () => () => ({ activity: { text = '' } }) => { - return () => { - if (~text.indexOf('error')) { - throw new Error('artificial error'); - } + activityMiddleware: + () => + () => + ({ activity: { text = '' } }) => { + return () => { + if (~text.indexOf('error')) { + throw new Error('artificial error'); + } - return false; - }; - }, + return false; + }; + }, onTelemetry: event => { const { data, dimensions, duration, error, fatal, name, type, value } = event; From de318d40b1a1a9f2dbcf15d2567a71f4586545af Mon Sep 17 00:00:00 2001 From: William Wong Date: Mon, 13 Feb 2023 19:09:38 +0000 Subject: [PATCH 10/13] Fix test --- __tests__/upload.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/__tests__/upload.js b/__tests__/upload.js index 279e97cbd5..b16e76ee16 100644 --- a/__tests__/upload.js +++ b/__tests__/upload.js @@ -41,7 +41,11 @@ describe('upload a picture', () => { await driver.wait(minNumActivitiesShown(2), timeouts.directLine); await driver.wait(allImagesLoaded(), timeouts.fetchImage); - const telemetryMeasurements = await driver.executeScript(() => window.WebChatTest.telemetryMeasurements); + const telemetryMeasurements = await driver.executeScript(() => + window.WebChatTest.telemetryMeasurements.filter(({ name }) => + ['init', 'sendFiles', 'sendFiles:makeThumbnail'].includes(name) + ) + ); expect(telemetryMeasurements).toHaveProperty('length', 4); expect(telemetryMeasurements[2]).toHaveProperty('name', 'sendFiles:makeThumbnail'); From 5c5d2516a56039c077c79ed7123cdbbcf759e666 Mon Sep 17 00:00:00 2001 From: William Wong Date: Mon, 13 Feb 2023 19:48:47 +0000 Subject: [PATCH 11/13] Fix tests --- __tests__/hooks/useTrackDimension.js | 5 ++++- __tests__/hooks/useTrackEvent.js | 5 ++++- __tests__/hooks/useTrackException.js | 5 ++++- __tests__/hooks/useTrackTiming.js | 17 ++++++++++++----- __tests__/telemetry.js | 6 +++++- __tests__/upload.js | 5 ++++- 6 files changed, 33 insertions(+), 10 deletions(-) diff --git a/__tests__/hooks/useTrackDimension.js b/__tests__/hooks/useTrackDimension.js index 9a8e078192..3913d43cc9 100644 --- a/__tests__/hooks/useTrackDimension.js +++ b/__tests__/hooks/useTrackDimension.js @@ -15,7 +15,7 @@ describe('useTrackDimension', () => { const { data, dimensions, duration, error, name, type } = event; name !== 'init' && - (window.WebChatTest.telemetryMeasurements || (window.WebChatTest.telemetryMeasurements = [])).push({ + window.WebChatTest.telemetryMeasurements.push({ data, dimensions, duration, @@ -24,6 +24,9 @@ describe('useTrackDimension', () => { type }); } + }, + setup: () => { + window.WebChatTest.telemetryMeasurements = []; } }); diff --git a/__tests__/hooks/useTrackEvent.js b/__tests__/hooks/useTrackEvent.js index f81f549ec5..26610d56a5 100644 --- a/__tests__/hooks/useTrackEvent.js +++ b/__tests__/hooks/useTrackEvent.js @@ -15,7 +15,7 @@ describe('useTrackEvent', () => { const { data, dimensions, duration, error, level, name, type } = event; name !== 'init' && - (window.WebChatTest.telemetryMeasurements || (window.WebChatTest.telemetryMeasurements = [])).push({ + window.WebChatTest.telemetryMeasurements.push({ data, dimensions, duration, @@ -25,6 +25,9 @@ describe('useTrackEvent', () => { type }); } + }, + setup: () => { + window.WebChatTest.telemetryMeasurements = []; } }); diff --git a/__tests__/hooks/useTrackException.js b/__tests__/hooks/useTrackException.js index aa75d00405..92d14de09c 100644 --- a/__tests__/hooks/useTrackException.js +++ b/__tests__/hooks/useTrackException.js @@ -15,7 +15,7 @@ describe('useTrackException', () => { const { data, dimensions, duration, error, fatal, name, type } = event; name !== 'init' && - (window.WebChatTest.telemetryMeasurements || (window.WebChatTest.telemetryMeasurements = [])).push({ + window.WebChatTest.telemetryMeasurements.push({ data, dimensions, duration, @@ -25,6 +25,9 @@ describe('useTrackException', () => { type }); } + }, + setup: () => { + window.WebChatTest.telemetryMeasurements = []; } }); diff --git a/__tests__/hooks/useTrackTiming.js b/__tests__/hooks/useTrackTiming.js index 452fa66984..cedf6fc136 100644 --- a/__tests__/hooks/useTrackTiming.js +++ b/__tests__/hooks/useTrackTiming.js @@ -15,7 +15,7 @@ describe('useTrackTiming', () => { const { data, dimensions, duration, error, name, type } = event; name !== 'init' && - (window.WebChatTest.telemetryMeasurements || (window.WebChatTest.telemetryMeasurements = [])).push({ + window.WebChatTest.telemetryMeasurements.push({ data, dimensions, duration, @@ -25,10 +25,13 @@ describe('useTrackTiming', () => { }); } }, - setup: () => + setup: () => { window.WebChatTest.loadScript('https://unpkg.com/lolex@4.0.1/lolex.js').then(() => { window.WebChatTest.clock = lolex.install(); - }) + }); + + window.WebChatTest.telemetryMeasurements = []; + } }); driver = setup.driver; @@ -43,7 +46,9 @@ describe('useTrackTiming', () => { pageObjects.runHook('useTrackTiming', [], trackTiming => trackTiming('ping', () => 123)) ).resolves.toBe(123); - await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements)).resolves.toMatchInlineSnapshot(` + await expect( + driver.executeScript(() => window.WebChatTest.telemetryMeasurements.filter(({ name }) => name === 'ping')) + ).resolves.toMatchInlineSnapshot(` Array [ Object { "data": null, @@ -88,7 +93,9 @@ describe('useTrackTiming', () => { await expect(driver.executeScript(() => window.WebChatTest.result)).resolves.toBe(123); - await expect(driver.executeScript(() => window.WebChatTest.telemetryMeasurements)).resolves.toMatchInlineSnapshot(` + await expect( + driver.executeScript(() => window.WebChatTest.telemetryMeasurements.filter(({ name }) => name === 'ping')) + ).resolves.toMatchInlineSnapshot(` Array [ Object { "data": null, diff --git a/__tests__/telemetry.js b/__tests__/telemetry.js index 2a3c8efb1e..eb70bcf1a2 100644 --- a/__tests__/telemetry.js +++ b/__tests__/telemetry.js @@ -10,12 +10,13 @@ jest.setTimeout(20000); describe('telemetry', () => { test('should collect "init" event', async () => { + const { driver } = await setupWebDriver({ props: { onTelemetry: event => { const { data, dimensions, duration, error, fatal, name, type, value } = event; - (window.WebChatTest.telemetryMeasurements || (window.WebChatTest.telemetryMeasurements = [])).push({ + window.WebChatTest.telemetryMeasurements.push({ data, dimensions, duration, @@ -26,6 +27,9 @@ describe('telemetry', () => { value }); } + }, + setup: () => { + window.WebChatTest.telemetryMeasurements = []; } }); diff --git a/__tests__/upload.js b/__tests__/upload.js index b16e76ee16..85ccc40864 100644 --- a/__tests__/upload.js +++ b/__tests__/upload.js @@ -19,7 +19,7 @@ describe('upload a picture', () => { onTelemetry: event => { const { data, dimensions, duration, error, fatal, name, type, value } = event; - (window.WebChatTest.telemetryMeasurements || (window.WebChatTest.telemetryMeasurements = [])).push({ + window.WebChatTest.telemetryMeasurements.push({ data, dimensions, duration, @@ -31,6 +31,9 @@ describe('upload a picture', () => { }); } }, + setup: () => { + window.WebChatTest.telemetryMeasurements = []; + }, // TODO: [P3] Offline bot did not reply with a downloadable attachment, so we need to use production bot useProductionBot: true }); From 1dbbd8f62b2b07e94bab406677f8ceaa1c46a93d Mon Sep 17 00:00:00 2001 From: William Wong Date: Mon, 13 Feb 2023 20:28:16 +0000 Subject: [PATCH 12/13] Clean up --- ...y.ignoreStatusChange.incomingActivity.html | 6 +- ...sendFiles.attachmentUrl.sendingToSent.html | 53 +++---- ...tyStatusTelemetry.sendingToSendFailed.html | 141 ++++++++++-------- ...usTelemetry.sendingToSendFailedToSent.html | 102 +++++++------ ...activityStatusTelemetry.sendingToSent.html | 48 +++--- .../ActivitySendStatusTelemetryComposer.tsx | 10 +- 6 files changed, 192 insertions(+), 168 deletions(-) diff --git a/__tests__/html/activityStatusTelemetry.ignoreStatusChange.incomingActivity.html b/__tests__/html/activityStatusTelemetry.ignoreStatusChange.incomingActivity.html index acbc1c7ee4..c607f3211a 100644 --- a/__tests__/html/activityStatusTelemetry.ignoreStatusChange.incomingActivity.html +++ b/__tests__/html/activityStatusTelemetry.ignoreStatusChange.incomingActivity.html @@ -30,13 +30,13 @@ await pageConditions.uiConnected(); - //GIVEN: Simulate an ingress activity + // GIVEN: Simulate an ingress activity of "Aloha!". await directLine.emulateIncomingActivity('Aloha!'); - //WHEN: The activity rendered + // WHEN: The activity is rendered. await pageConditions.numActivitiesShown(1); - //THEN: No status change event should be logged + // THEN: No status change event should be logged for incoming activities. expect(telemetryEventTracking.length).toEqual(0); } ); diff --git a/__tests__/html/activityStatusTelemetry.sendFiles.attachmentUrl.sendingToSent.html b/__tests__/html/activityStatusTelemetry.sendFiles.attachmentUrl.sendingToSent.html index 9685e4e82a..8c29f8b88b 100644 --- a/__tests__/html/activityStatusTelemetry.sendFiles.attachmentUrl.sendingToSent.html +++ b/__tests__/html/activityStatusTelemetry.sendFiles.attachmentUrl.sendingToSent.html @@ -30,7 +30,7 @@ name: 'test.txt' } ] - } + }; const store = testHelpers.createStore(); const directLine = testHelpers.createDirectLineEmulator(store); @@ -39,7 +39,7 @@ WebChat.renderWebChat( { directLine, - onTelemetry: (event) => { + onTelemetry: event => { if (event?.name === 'send-status:change') { telemetryEventTracking.push(event.data); } @@ -49,34 +49,35 @@ document.getElementById('webchat') ); + // GIVEN: Web Chat is connected. await pageConditions.uiConnected(); - //GIVEN: send an attachment outgoing activity and resolve it afterwards - await( - await directLine.emulateOutgoingActivity(testAttachmentActivity) - ).resolveAll(); - - //WHEN: Activity displayed + // WHEN: An outgoing activity with attachments is sent. + await (await directLine.emulateOutgoingActivity(testAttachmentActivity)).resolveAll(); await pageConditions.numActivitiesShown(1); - //THEN: The status change event emitted: 'undefined -> sending' and 'sending -> sent' - expect(telemetryEventTracking).toEqual(expect.arrayContaining([ - { - clientActivityID: expect.any(String), - hasAttachment: 'true', - key: expect.any(String), - status: 'sending', - type: 'message' - }, - { - clientActivityID: expect.any(String), - hasAttachment: 'true', - key: expect.any(String), - prevStatus: 'sending', - status: 'sent', - type: 'message' - } - ])); + // THEN: It should emit 2 events in order: + // - undefined -> sending + // - sending -> sent + expect(telemetryEventTracking).toEqual( + expect.arrayContaining([ + { + clientActivityID: expect.any(String), + hasAttachment: 'true', + key: expect.any(String), + status: 'sending', + type: 'message' + }, + { + clientActivityID: expect.any(String), + hasAttachment: 'true', + key: expect.any(String), + prevStatus: 'sending', + status: 'sent', + type: 'message' + } + ]) + ); }); diff --git a/__tests__/html/activityStatusTelemetry.sendingToSendFailed.html b/__tests__/html/activityStatusTelemetry.sendingToSendFailed.html index 66da6cc93c..a65ca84da1 100644 --- a/__tests__/html/activityStatusTelemetry.sendingToSendFailed.html +++ b/__tests__/html/activityStatusTelemetry.sendingToSendFailed.html @@ -9,79 +9,90 @@
diff --git a/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.html b/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.html index 71314321c1..84271b83c0 100644 --- a/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.html +++ b/__tests__/html/activityStatusTelemetry.sendingToSendFailedToSent.html @@ -20,74 +20,86 @@ { directLine, store, - onTelemetry: (event) => { + onTelemetry: event => { if (event.name == 'send-status:change') { telemetryEventTracking.push(event.data); } - }, + } }, document.getElementById('webchat') ); await pageConditions.webChatRendered(); - //GIVEN: User send out a message - const sendingUserEgressMessage = directLine.emulateOutgoingActivity({ + // WHEN: User send out a message + const sendingUserEgressMessage = await directLine.emulateOutgoingActivity({ channelData: { 'webchat:sequence-id': 1 }, from: { role: 'user' }, text: 'msg from user.', type: 'message' - }) + }); - //WHEN: Activity displayed in sending status await pageConditions.numActivitiesShown(1); - //THEN: The status change event emitted: undefined -> sending - expect(telemetryEventTracking).toEqual(expect.arrayContaining([ - { - clientActivityID: expect.any(String), - hasAttachment: 'false', - key: expect.any(String), - status: 'sending', - type: 'message' - }, - ])); + // THEN: A telemetry event should be emitted: undefined -> sending + expect(telemetryEventTracking).toEqual( + expect.arrayContaining([ + { + clientActivityID: expect.any(String), + hasAttachment: 'false', + key: expect.any(String), + status: 'sending', + type: 'message' + } + ]) + ); - //GIVEN: Wait for 20 seconds to let the activity times out + // GIVEN: Wait for 20 seconds to let the activity times out. clock.tick(20000); - //WHEN: Sending message timeout and message status turns into 'Send failed. Retry' + // THEN: The message status should turns into "Send failed. Retry." await pageConditions.became( - 'failed to send message', - () => pageElements.activityStatuses()[0]?.innerText === 'Send failed. Retry.', - 1000 + 'failed to send message', + () => pageElements.activityStatuses()[0]?.innerText === 'Send failed. Retry.', + 1000 ); - //THEN: Telemetry event should be emitted: sending -> send failed - expect(telemetryEventTracking).toEqual(expect.arrayContaining([ - { - clientActivityID: expect.any(String), - hasAttachment: 'false', - key: expect.any(String), - prevStatus: 'sending', - status: 'send failed', - type: 'message' - } - ])); + // THEN: A telemetry event should be emitted: sending -> send failed + expect(telemetryEventTracking).toEqual( + expect.arrayContaining([ + { + clientActivityID: expect.any(String), + hasAttachment: 'false', + key: expect.any(String), + prevStatus: 'sending', + status: 'send failed', + type: 'message' + } + ]) + ); - //GIVEN: Resolved the egressing message - await (await (await sendingUserEgressMessage).resolveAll()); + // GIVEN: The outgoing message successfully sent. + await sendingUserEgressMessage.resolveAll(); - //THEN: Telemetry event should be emitted: send failed -> sent - expect(telemetryEventTracking).toEqual(expect.arrayContaining([ - { - clientActivityID: expect.any(String), - hasAttachment: 'false', - key: expect.any(String), - prevStatus: 'send failed', - status: 'sent', - type: 'message' - } - ])); + // THEN: The message status should turns into "Just now" + await pageConditions.became( + 'message successfully sent', + () => pageElements.activityStatuses()[0]?.innerText.includes('Just now'), + 1000 + ); + + // THEN: A telemetry event should be emitted: send failed -> sent + expect(telemetryEventTracking).toEqual( + expect.arrayContaining([ + { + clientActivityID: expect.any(String), + hasAttachment: 'false', + key: expect.any(String), + prevStatus: 'send failed', + status: 'sent', + type: 'message' + } + ]) + ); }); diff --git a/__tests__/html/activityStatusTelemetry.sendingToSent.html b/__tests__/html/activityStatusTelemetry.sendingToSent.html index 63ab78bf43..3af7da4904 100644 --- a/__tests__/html/activityStatusTelemetry.sendingToSent.html +++ b/__tests__/html/activityStatusTelemetry.sendingToSent.html @@ -18,7 +18,7 @@ WebChat.renderWebChat( { directLine, - onTelemetry: (event) => { + onTelemetry: event => { if (event?.name === 'send-status:change') { telemetryEventTracking.push(event.data); } @@ -30,32 +30,30 @@ await pageConditions.uiConnected(); - //GIVEN: send an attachment outgoing activity and resolve it afterwards - await( - await directLine.emulateOutgoingActivity('sending to sent') - ).resolveAll(); - - //WHEN: Activity displayed + // WHEN: A message activity "sending to sent" is sent. + await (await directLine.emulateOutgoingActivity('sending to sent')).resolveAll(); await pageConditions.numActivitiesShown(1); - //THEN: The status change event emitted: 'undefined -> sending' and 'sending -> sent' - expect(telemetryEventTracking).toEqual(expect.arrayContaining([ - { - clientActivityID: expect.any(String), - hasAttachment: 'false', - key: expect.any(String), - status: 'sending', - type: 'message' - }, - { - clientActivityID: expect.any(String), - hasAttachment: 'false', - key: expect.any(String), - prevStatus: 'sending', - status: 'sent', - type: 'message' - } - ])); + // THEN: Two telemetry events should be emitted: "undefined -> sending" and "sending -> sent" + expect(telemetryEventTracking).toEqual( + expect.arrayContaining([ + { + clientActivityID: expect.any(String), + hasAttachment: 'false', + key: expect.any(String), + status: 'sending', + type: 'message' + }, + { + clientActivityID: expect.any(String), + hasAttachment: 'false', + key: expect.any(String), + prevStatus: 'sending', + status: 'sent', + type: 'message' + } + ]) + ); }); diff --git a/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx b/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx index 6e099f1c62..221a9bc0dc 100644 --- a/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx +++ b/packages/api/src/providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer.tsx @@ -27,18 +27,20 @@ const ActivitySendStatusTelemetryComposer = () => { // This telemetry data point only emit changes in outgoing activities. if (status && status !== prevStatus) { const activity = getActivityByKey(key); + const telemetryPayload: TelemetrySendStatusChangePayload = { - clientActivityID: activity?.channelData?.clientActivityID, - hasAttachment: activity?.type === 'message' && activity?.attachments?.length > 0 ? 'true' : 'false', + clientActivityID: activity?.channelData.clientActivityID, + hasAttachment: activity?.type === 'message' && activity.attachments?.length > 0 ? 'true' : 'false', key, status, - type: activity?.type?.toString() + type: activity?.type }; - //only add prevStatus if it is NOT null/undefined + // Only add prevStatus if it is NOT null/undefined if (prevStatus) { telemetryPayload.prevStatus = prevStatus; } + trackEvent('send-status:change', telemetryPayload); } } From 07dbf8d2601401fdf81a03cbc7effac7c3f360f9 Mon Sep 17 00:00:00 2001 From: William Wong Date: Mon, 13 Feb 2023 20:48:21 +0000 Subject: [PATCH 13/13] Update entry format --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 716a76209b..feca5173af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Added function to emit status change telemetry event for activities, by [@Erli-ms](https://github.com/Erli-ms), in PR [#4631](https://github.com/microsoft/BotFramework-WebChat/pull/4631) -- Added ability for developers to customize Web Chat by extending the default UI without having to re-implement existing components. [@dawolff-ms](https://github.com/dawolff-ms) in PR [#4539](https://github.com/microsoft/BotFramework-WebChat/pull/4539) +- Added ability for developers to customize Web Chat by extending the default UI without having to re-implement existing components, by [@dawolff-ms](https://github.com/dawolff-ms), in PR [#4539](https://github.com/microsoft/BotFramework-WebChat/pull/4539) ### Fixed