From e8b312b7198d71286246a0c47f2167c29b369ca0 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Mon, 2 May 2022 21:29:29 -0600 Subject: [PATCH 01/17] Add a telemetry abstraction for MR Widget Extensions --- .../components/extensions/telemetry.js | 169 ++++++++++++++++++ .../vue_merge_request_widget/constants.js | 4 + 2 files changed, 173 insertions(+) create mode 100644 app/assets/javascripts/vue_merge_request_widget/components/extensions/telemetry.js diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/telemetry.js b/app/assets/javascripts/vue_merge_request_widget/components/extensions/telemetry.js new file mode 100644 index 00000000000000..6711a837f7616a --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/telemetry.js @@ -0,0 +1,169 @@ +import { merge } from 'lodash'; +import api from '~/api'; +import createEventHub from '~/helpers/event_hub_factory'; +import { + TELEMETRY_WIDGET_VIEWED, + TELEMETRY_WIDGET_EXPANDED, + TELEMETRY_WIDGET_FULL_REPORT_CLICKED, +} from '../../constants'; + +/* + * Additional events to send beyond the defaults for certain widget extensions + */ +const nonStandardEvents = { + codeQuality: { + uniqueUser: { + expand: ['i_testing_code_quality_widget_total'], + }, + counter: {}, + }, + terraform: { + uniqueUser: { + expand: ['i_testing_terraform_widget_total'], + }, + counter: {}, + }, + issues: { + uniqueUser: { + expand: ['i_testing_load_performance_widget_total'], + }, + counter: {}, + }, + testReport: { + uniqueUser: { + expand: ['i_testing_summary_widget_total'], + }, + counter: {}, + }, +}; + +function simplifyWidgetName(componentName) { + const noWidget = componentName.replace(/^Widget/, ''); + + return noWidget.charAt(0).toLowerCase() + noWidget.slice(1); +} + +function baseRedisEventName(extensionName) { + const redisEventName = extensionName.replace(/([A-Z])/g, '_$1').toLowerCase(); + + return `i_merge_request_widget_${redisEventName}`; +} + +function whenable(bus) { + return function when(event) { + return ({ execute, track, special }) => { + bus.$on(event, (busEvent) => { + track.forEach((redisEvent) => { + execute(redisEvent); + }); + + special?.({ event, execute, track, bus, busEvent }); + }); + }; + }; +} + +function defaultBehaviorEvents({ bus, config }) { + const when = whenable(bus); + const isViewed = when(TELEMETRY_WIDGET_VIEWED); + const isExpanded = when(TELEMETRY_WIDGET_EXPANDED); + const fullReportIsClicked = when(TELEMETRY_WIDGET_FULL_REPORT_CLICKED); + const toHll = config?.uniqueUser || {}; + const toCounts = config?.counter || {}; + const user = api.trackRedisHllUserEvent.bind(api); + const count = api.trackRedisCounterEvent.bind(api); + + if (toHll.view) { + isViewed({ execute: user, track: toHll.view }); + } + if (toCounts.view) { + isViewed({ execute: count, track: toCounts.view }); + } + + if (toHll.expand) { + isExpanded({ + execute: user, + track: toHll.expand, + special: ({ execute, track, busEvent }) => { + if (busEvent.type) { + track.forEach((event) => { + execute(`${event}_${busEvent.type}`); + }); + } + }, + }); + } + if (toCounts.expand) { + isExpanded({ + execute: count, + track: toCounts.expand, + special: ({ execute, track, busEvent }) => { + if (busEvent.type) { + track.forEach((event) => { + execute(`${event}_${busEvent.type}`); + }); + } + }, + }); + } + + if (toHll.clickFullReport) { + fullReportIsClicked({ execute: user, track: toHll.clickFullReport }); + } + if (toCounts.clickFullReport) { + fullReportIsClicked({ execute: count, track: toCounts.clickFullReport }); + } +} + +function baseTelemetry(componentName) { + const simpleExtensionName = simplifyWidgetName(componentName); + const additionalNonStandard = nonStandardEvents[simpleExtensionName] || {}; + /* + * Telemetry config format is: + * { + * TELEMETRYTYPE: { + * BEHAVIOR: [ EVENTNAME, ... ] + * } + * } + * + * Right now, there are currently configurations for these telemetry types: + * - uniqueUser is sent to RedisHLL + * - counter is sent to a regular Redis counter + */ + const defaultTelemetry = { + uniqueUser: { + view: [`${baseRedisEventName(simpleExtensionName)}_view`], + expand: [`${baseRedisEventName(simpleExtensionName)}_expand`], + clickFullReport: [`${baseRedisEventName(simpleExtensionName)}_click_full_report`], + }, + counter: { + view: [`${baseRedisEventName(simpleExtensionName)}_count_view`], + expand: [`${baseRedisEventName(simpleExtensionName)}_count_expand`], + clickFullReport: [`${baseRedisEventName(simpleExtensionName)}_count_click_full_report`], + }, + }; + + return merge({}, defaultTelemetry, nonStandardEvents[simpleExtensionName] || {}); +} + +export function createTelemetryHub(componentName) { + const bus = createEventHub(); + const config = baseTelemetry(componentName); + + defaultBehaviorEvents({ bus, config }); + + return { + viewed() { + bus.$emit(TELEMETRY_WIDGET_VIEWED); + }, + expanded({ type }) { + bus.$emit(TELEMETRY_WIDGET_EXPANDED, { type }); + }, + fullReportClicked() { + bus.$emit(TELEMETRY_WIDGET_FULL_REPORT_CLICKED); + }, + /* I want a Record here: #{ ...config } // and then the comment would be: This is for debugging only, changing your reference to it does nothing. 😘 */ + config: Object.freeze({ ...config }), // This is *intended* to be for debugging only, but it's pretty mutable, and it has references to live data in child props + bus, + }; +} diff --git a/app/assets/javascripts/vue_merge_request_widget/constants.js b/app/assets/javascripts/vue_merge_request_widget/constants.js index 2c770392acf3a5..c148a35209f3e6 100644 --- a/app/assets/javascripts/vue_merge_request_widget/constants.js +++ b/app/assets/javascripts/vue_merge_request_widget/constants.js @@ -166,6 +166,10 @@ export const EXTENSION_ICON_CLASS = { export const EXTENSION_SUMMARY_FAILED_CLASS = 'gl-text-red-500'; export const EXTENSION_SUMMARY_NEUTRAL_CLASS = 'gl-text-gray-700'; +export const TELEMETRY_WIDGET_VIEWED = 'WIDGET_VIEWED'; +export const TELEMETRY_WIDGET_EXPANDED = 'WIDGET_EXPANDED'; +export const TELEMETRY_WIDGET_FULL_REPORT_CLICKED = 'WIDGET_FULL_REPORT_CLICKED'; + export { STATE_MACHINE }; export const INVALID_RULES_DOCS_PATH = helpPagePath( -- GitLab From ec3ba94a71d9ec4ace5d07b2ac2b379f0396627b Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Mon, 2 May 2022 21:30:20 -0600 Subject: [PATCH 02/17] Implement telemetry using the new abstraction at the base component --- .../components/extensions/actions.vue | 4 +++ .../components/extensions/base.vue | 26 ++++++++++++------- .../extensions/issues.js | 2 +- .../extensions/test_report/index.js | 1 + 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/actions.vue b/app/assets/javascripts/vue_merge_request_widget/components/extensions/actions.vue index d878a1fa2e09a1..d9f68942829416 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/actions.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/actions.vue @@ -26,6 +26,10 @@ export default { }, methods: { onClickAction(action) { + if (action.fullReport) { + this.$emit('clickedFullReport'); + } + if (action.onClick) { action.onClick(); } diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue index 0e927d22a5a2d1..643c15aa548595 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue @@ -6,10 +6,8 @@ import { GlTooltipDirective, GlIntersectionObserver, } from '@gitlab/ui'; -import { once } from 'lodash'; import * as Sentry from '@sentry/browser'; import { DynamicScroller, DynamicScrollerItem } from 'vendor/vue-virtual-scroller'; -import api from '~/api'; import { sprintf, s__, __ } from '~/locale'; import Poll from '~/lib/utils/poll'; import { normalizeHeaders } from '~/lib/utils/common_utils'; @@ -17,6 +15,7 @@ import { EXTENSION_ICON_CLASS, EXTENSION_ICONS } from '../../constants'; import StatusIcon from './status_icon.vue'; import Actions from './actions.vue'; import ChildContent from './child_content.vue'; +import { createTelemetryHub } from './telemetry'; import { generateText } from './utils'; export const LOADING_STATES = { @@ -50,6 +49,7 @@ export default { showFade: false, modalData: undefined, modalName: undefined, + telemetry: true, }; }, computed: { @@ -132,20 +132,24 @@ export default { } }, }, + created() { + if (this.telemetry) { + this.telemetry = createTelemetryHub(this.$options.name); + } + }, mounted() { this.loadCollapsedData(); + + this.telemetry.viewed(); }, methods: { - triggerRedisTracking: once(function triggerRedisTracking() { - if (this.$options.expandEvent) { - api.trackRedisHllUserEvent(this.$options.expandEvent); - } - }), toggleCollapsed(e) { if (this.isCollapsible && !e?.target?.closest('.btn:not(.btn-icon),a')) { - this.isCollapsed = !this.isCollapsed; + if (this.isCollapsed) { + this.telemetry.expanded({ type: this.statusIconName || 'loading' }); + } - this.triggerRedisTracking(); + this.isCollapsed = !this.isCollapsed; } }, initExtensionMultiPolling() { @@ -263,6 +267,9 @@ export default { this.toggleCollapsed(e); } }, + onClickedFullReport() { + this.telemetry.fullReportClicked(); + }, generateText, }, EXTENSION_ICON_CLASS, @@ -300,6 +307,7 @@ export default {
Date: Mon, 2 May 2022 21:32:10 -0600 Subject: [PATCH 03/17] Do not perform telemetry on the accessibility widget It would be nice to expand the abstraction to allow for more fine-grained control, like turning off individual default telemetry events, or adding custom ones. Being able to do this may allow telemetry for widgets like the accessibility widget. --- .../extensions/accessibility/index.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/assets/javascripts/vue_merge_request_widget/extensions/accessibility/index.js b/app/assets/javascripts/vue_merge_request_widget/extensions/accessibility/index.js index 168f10bd1487df..65c7e35f571dad 100644 --- a/app/assets/javascripts/vue_merge_request_widget/extensions/accessibility/index.js +++ b/app/assets/javascripts/vue_merge_request_widget/extensions/accessibility/index.js @@ -11,6 +11,11 @@ export default { error: s__('Reports|Accessibility scanning failed loading results'), }, props: ['accessibilityReportPath'], + data() { + return { + telemetry: false, + }; + }, computed: { statusIcon() { return this.collapsedData.status === 'failed' -- GitLab From 4458c39276abcf3348d94fb581d8892113707a6a Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Mon, 2 May 2022 21:32:37 -0600 Subject: [PATCH 04/17] Remove per-widget expand event implementations --- .../vue_merge_request_widget/extensions/code_quality/index.js | 1 - .../javascripts/vue_merge_request_widget/extensions/issues.js | 1 - .../vue_merge_request_widget/extensions/terraform/index.js | 1 - .../vue_merge_request_widget/extensions/test_report/index.js | 1 - 4 files changed, 4 deletions(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/extensions/code_quality/index.js b/app/assets/javascripts/vue_merge_request_widget/extensions/code_quality/index.js index cea8df2484bfad..2477429af5bef6 100644 --- a/app/assets/javascripts/vue_merge_request_widget/extensions/code_quality/index.js +++ b/app/assets/javascripts/vue_merge_request_widget/extensions/code_quality/index.js @@ -13,7 +13,6 @@ export default { loading: s__('ciReport|Code Quality test metrics results are being parsed'), error: s__('ciReport|Code Quality failed loading results'), }, - expandEvent: 'i_testing_code_quality_widget_total', computed: { summary() { const { newErrors, resolvedErrors, errorSummary } = this.collapsedData; diff --git a/app/assets/javascripts/vue_merge_request_widget/extensions/issues.js b/app/assets/javascripts/vue_merge_request_widget/extensions/issues.js index c847622b88b2b8..a7aaa2f4476abc 100644 --- a/app/assets/javascripts/vue_merge_request_widget/extensions/issues.js +++ b/app/assets/javascripts/vue_merge_request_widget/extensions/issues.js @@ -12,7 +12,6 @@ export default { label: 'Issues', loading: 'Loading issues...', }, - expandEvent: 'i_testing_load_performance_widget_total', // Add an array of props // These then get mapped to values stored in the MR Widget store props: ['targetProjectFullPath', 'conflictsDocsPath'], diff --git a/app/assets/javascripts/vue_merge_request_widget/extensions/terraform/index.js b/app/assets/javascripts/vue_merge_request_widget/extensions/terraform/index.js index 8fcc4f818ec1d8..349014875aa75c 100644 --- a/app/assets/javascripts/vue_merge_request_widget/extensions/terraform/index.js +++ b/app/assets/javascripts/vue_merge_request_widget/extensions/terraform/index.js @@ -23,7 +23,6 @@ export default { reportErrored: s__('Terraform|Generating the report caused an error.'), fullLog: __('Full log'), }, - expandEvent: 'i_testing_terraform_widget_total', props: ['terraformReportsPath'], computed: { // Extension computed props diff --git a/app/assets/javascripts/vue_merge_request_widget/extensions/test_report/index.js b/app/assets/javascripts/vue_merge_request_widget/extensions/test_report/index.js index 9e59265ac1b9f1..164bda33b957dd 100644 --- a/app/assets/javascripts/vue_merge_request_widget/extensions/test_report/index.js +++ b/app/assets/javascripts/vue_merge_request_widget/extensions/test_report/index.js @@ -16,7 +16,6 @@ export default { name: 'WidgetTestSummary', enablePolling: true, i18n, - expandEvent: 'i_testing_summary_widget_total', props: ['testResultsPath', 'headBlobPath', 'pipeline'], modalComponent: TestCaseDetails, computed: { -- GitLab From cb3b0fd63237a1d1119b772572f8460ad41c58a7 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 10 May 2022 02:08:45 -0600 Subject: [PATCH 05/17] Switch to a custom deep merge Lodash's merge overwrites array entries. This is probably preferable in some cases, but in our case, we truly want a _merge_: where two array items are both retained but then deduplicated. --- .../components/extensions/telemetry.js | 42 ++++++++++++++++++- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/telemetry.js b/app/assets/javascripts/vue_merge_request_widget/components/extensions/telemetry.js index 6711a837f7616a..fc98d78727443d 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/telemetry.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/telemetry.js @@ -1,4 +1,3 @@ -import { merge } from 'lodash'; import api from '~/api'; import createEventHub from '~/helpers/event_hub_factory'; import { @@ -37,6 +36,26 @@ const nonStandardEvents = { }, }; +function combineDeepArray(path, ...objects) { + const parts = path.split('.'); + const allEntries = objects.reduce((entries, currentObject) => { + let expandedEntries = entries; + let traversed = currentObject; + + parts.forEach((part) => { + traversed = traversed?.[part]; + }); + + if (traversed) { + expandedEntries = [...entries, ...traversed]; + } + + return expandedEntries; + }, []); + + return Array.from(new Set(allEntries)); +} + function simplifyWidgetName(componentName) { const noWidget = componentName.replace(/^Widget/, ''); @@ -143,7 +162,26 @@ function baseTelemetry(componentName) { }, }; - return merge({}, defaultTelemetry, nonStandardEvents[simpleExtensionName] || {}); + return { + uniqueUser: { + view: combineDeepArray('uniqueUser.view', defaultTelemetry, additionalNonStandard), + expand: combineDeepArray('uniqueUser.expand', defaultTelemetry, additionalNonStandard), + clickFullReport: combineDeepArray( + 'uniqueUser.clickFullReport', + defaultTelemetry, + additionalNonStandard, + ), + }, + counter: { + view: combineDeepArray('counter.view', defaultTelemetry, additionalNonStandard), + expand: combineDeepArray('counter.expand', defaultTelemetry, additionalNonStandard), + clickFullReport: combineDeepArray( + 'counter.clickFullReport', + defaultTelemetry, + additionalNonStandard, + ), + }, + }; } export function createTelemetryHub(componentName) { -- GitLab From 2099b89b3580f06952a3d04e10d9fb6122d3786c Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 8 Jun 2022 16:21:42 -0600 Subject: [PATCH 06/17] Test new telemetry framework --- .../vue_mr_widget/mr_widget_options_spec.js | 90 ++++++++++++++++--- .../frontend/vue_mr_widget/test_extensions.js | 17 ++++ 2 files changed, 95 insertions(+), 12 deletions(-) diff --git a/spec/frontend/vue_mr_widget/mr_widget_options_spec.js b/spec/frontend/vue_mr_widget/mr_widget_options_spec.js index bfd921d5e4e236..2ebd63ecd3824c 100644 --- a/spec/frontend/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/frontend/vue_mr_widget/mr_widget_options_spec.js @@ -29,6 +29,7 @@ import { workingExtension, collapsedDataErrorExtension, fullDataErrorExtension, + fullReportExtension, pollingExtension, pollingErrorExtension, multiPollingExtension, @@ -924,18 +925,6 @@ describe('MrWidgetOptions', () => { expect(wrapper.text()).toContain('Test extension summary count: 1'); }); - it('triggers trackRedisHllUserEvent API call', async () => { - await waitForPromises(); - - wrapper - .find('[data-testid="widget-extension"] [data-testid="toggle-button"]') - .trigger('click'); - - await nextTick(); - - expect(api.trackRedisHllUserEvent).toHaveBeenCalledWith('test_expand_event'); - }); - it('renders full data', async () => { await waitForPromises(); @@ -1159,4 +1148,81 @@ describe('MrWidgetOptions', () => { itHandlesTheException(); }); }); + + describe('telemetry', () => { + afterEach(() => { + registeredExtensions.extensions = []; + }); + + it('triggers view events when mounted', async () => { + registerExtension(workingExtension()); + createComponent(); + + await waitForPromises(); + + expect(api.trackRedisHllUserEvent).toHaveBeenCalledTimes(1); + expect(api.trackRedisHllUserEvent).toHaveBeenCalledWith( + 'i_merge_request_widget_test_extension_view', + ); + expect(api.trackRedisCounterEvent).toHaveBeenCalledTimes(1); + expect(api.trackRedisCounterEvent).toHaveBeenCalledWith( + 'i_merge_request_widget_test_extension_count_view', + ); + }); + + it('triggers expand events when the expand button is clicked', async () => { + registerExtension(workingExtension()); + createComponent(); + + await waitForPromises(); + + api.trackRedisHllUserEvent.mockClear(); + api.trackRedisCounterEvent.mockClear(); + + wrapper + .find('[data-testid="widget-extension"] [data-testid="toggle-button"]') + .trigger('click'); + + await nextTick(); + + // The default working extension is a "warning" type, which generates a second - more specific - telemetry event for expansions + expect(api.trackRedisHllUserEvent).toHaveBeenCalledTimes(2); + expect(api.trackRedisHllUserEvent).toHaveBeenCalledWith( + 'i_merge_request_widget_test_extension_expand', + ); + expect(api.trackRedisHllUserEvent).toHaveBeenCalledWith( + 'i_merge_request_widget_test_extension_expand_warning', + ); + expect(api.trackRedisCounterEvent).toHaveBeenCalledTimes(2); + expect(api.trackRedisCounterEvent).toHaveBeenCalledWith( + 'i_merge_request_widget_test_extension_count_expand', + ); + expect(api.trackRedisCounterEvent).toHaveBeenCalledWith( + 'i_merge_request_widget_test_extension_count_expand_warning', + ); + }); + + it('triggers the "full report clicked" events when the appropriate button is clicked', async () => { + registerExtension(fullReportExtension); + createComponent(); + + await waitForPromises(); + + api.trackRedisHllUserEvent.mockClear(); + api.trackRedisCounterEvent.mockClear(); + + wrapper.find('[data-testid="widget-extension"] [href="testref"]').trigger('click'); + + await nextTick(); + + expect(api.trackRedisHllUserEvent).toHaveBeenCalledTimes(1); + expect(api.trackRedisHllUserEvent).toHaveBeenCalledWith( + 'i_merge_request_widget_test_extension_click_full_report', + ); + expect(api.trackRedisCounterEvent).toHaveBeenCalledTimes(1); + expect(api.trackRedisCounterEvent).toHaveBeenCalledWith( + 'i_merge_request_widget_test_extension_count_click_full_report', + ); + }); + }); }); diff --git a/spec/frontend/vue_mr_widget/test_extensions.js b/spec/frontend/vue_mr_widget/test_extensions.js index d4a96703e038e1..cbaaeff44d973c 100644 --- a/spec/frontend/vue_mr_widget/test_extensions.js +++ b/spec/frontend/vue_mr_widget/test_extensions.js @@ -109,6 +109,23 @@ export const pollingExtension = { enablePolling: true, }; +export const fullReportExtension = { + ...workingExtension(), + computed: { + ...workingExtension().computed, + tertiaryButtons() { + return [ + { + text: 'test', + href: `testref`, + target: '_blank', + fullReport: true, + }, + ]; + }, + }, +}; + export const multiPollingExtension = (endpointsToBePolled) => ({ name: 'WidgetTestMultiPollingExtension', props: [], -- GitLab From 5dbff9c3a2177513c7b942b64435421f3ac3631e Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 15 Jun 2022 22:34:39 -0600 Subject: [PATCH 07/17] Move the telemetry toggle to $options This has two good effects: 1. Separates the boolean from the holding data when the telemetry is created (if it is) 2. It's much easier to pass through the extension framework than a data property. --- .../vue_merge_request_widget/components/extensions/base.vue | 5 +++-- .../vue_merge_request_widget/components/extensions/index.js | 1 + .../extensions/accessibility/index.js | 6 +----- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue index 643c15aa548595..21643b7671e32e 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue @@ -26,6 +26,7 @@ export const LOADING_STATES = { }; export default { + telemetry: true, components: { GlButton, GlLoadingIcon, @@ -49,7 +50,7 @@ export default { showFade: false, modalData: undefined, modalName: undefined, - telemetry: true, + telemetry: null, }; }, computed: { @@ -133,7 +134,7 @@ export default { }, }, created() { - if (this.telemetry) { + if (this.$options.telemetry) { this.telemetry = createTelemetryHub(this.$options.name); } }, diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/index.js b/app/assets/javascripts/vue_merge_request_widget/components/extensions/index.js index f22c6c756cd98e..f4fcf4c95710e0 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/index.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/index.js @@ -16,6 +16,7 @@ export const registerExtension = (extension) => { required: true, }, }, + telemetry: extension.telemetry, i18n: extension.i18n, expandEvent: extension.expandEvent, enablePolling: extension.enablePolling, diff --git a/app/assets/javascripts/vue_merge_request_widget/extensions/accessibility/index.js b/app/assets/javascripts/vue_merge_request_widget/extensions/accessibility/index.js index 65c7e35f571dad..f14e80d0be6c39 100644 --- a/app/assets/javascripts/vue_merge_request_widget/extensions/accessibility/index.js +++ b/app/assets/javascripts/vue_merge_request_widget/extensions/accessibility/index.js @@ -6,16 +6,12 @@ import { EXTENSION_ICONS } from '../../constants'; export default { name: 'WidgetAccessibility', enablePolling: true, + telemetry: false, i18n: { loading: s__('Reports|Accessibility scanning results are being parsed'), error: s__('Reports|Accessibility scanning failed loading results'), }, props: ['accessibilityReportPath'], - data() { - return { - telemetry: false, - }; - }, computed: { statusIcon() { return this.collapsedData.status === 'failed' -- GitLab From cc13bbbb260d8386576867d0a6faf0730455ba5e Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 15 Jun 2022 22:35:21 -0600 Subject: [PATCH 08/17] Short-circuit reporting telemetry if it is turned off --- .../vue_merge_request_widget/components/extensions/base.vue | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue index 21643b7671e32e..6aa3289e0a2235 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue @@ -141,13 +141,13 @@ export default { mounted() { this.loadCollapsedData(); - this.telemetry.viewed(); + this.telemetry?.viewed(); }, methods: { toggleCollapsed(e) { if (this.isCollapsible && !e?.target?.closest('.btn:not(.btn-icon),a')) { if (this.isCollapsed) { - this.telemetry.expanded({ type: this.statusIconName || 'loading' }); + this.telemetry?.expanded({ type: this.statusIconName || 'loading' }); } this.isCollapsed = !this.isCollapsed; @@ -269,7 +269,7 @@ export default { } }, onClickedFullReport() { - this.telemetry.fullReportClicked(); + this.telemetry?.fullReportClicked(); }, generateText, }, -- GitLab From e0ec12c7e3bbc1e1e22c8cc4290b760c7be13aac Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 15 Jun 2022 22:35:56 -0600 Subject: [PATCH 09/17] Remove unnecessary async/await --- .../frontend/vue_mr_widget/mr_widget_options_spec.js | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/spec/frontend/vue_mr_widget/mr_widget_options_spec.js b/spec/frontend/vue_mr_widget/mr_widget_options_spec.js index 2ebd63ecd3824c..feaa1321744df4 100644 --- a/spec/frontend/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/frontend/vue_mr_widget/mr_widget_options_spec.js @@ -1154,12 +1154,10 @@ describe('MrWidgetOptions', () => { registeredExtensions.extensions = []; }); - it('triggers view events when mounted', async () => { + it('triggers view events when mounted', () => { registerExtension(workingExtension()); createComponent(); - await waitForPromises(); - expect(api.trackRedisHllUserEvent).toHaveBeenCalledTimes(1); expect(api.trackRedisHllUserEvent).toHaveBeenCalledWith( 'i_merge_request_widget_test_extension_view', @@ -1183,8 +1181,6 @@ describe('MrWidgetOptions', () => { .find('[data-testid="widget-extension"] [data-testid="toggle-button"]') .trigger('click'); - await nextTick(); - // The default working extension is a "warning" type, which generates a second - more specific - telemetry event for expansions expect(api.trackRedisHllUserEvent).toHaveBeenCalledTimes(2); expect(api.trackRedisHllUserEvent).toHaveBeenCalledWith( @@ -1202,19 +1198,15 @@ describe('MrWidgetOptions', () => { ); }); - it('triggers the "full report clicked" events when the appropriate button is clicked', async () => { + it('triggers the "full report clicked" events when the appropriate button is clicked', () => { registerExtension(fullReportExtension); createComponent(); - await waitForPromises(); - api.trackRedisHllUserEvent.mockClear(); api.trackRedisCounterEvent.mockClear(); wrapper.find('[data-testid="widget-extension"] [href="testref"]').trigger('click'); - await nextTick(); - expect(api.trackRedisHllUserEvent).toHaveBeenCalledTimes(1); expect(api.trackRedisHllUserEvent).toHaveBeenCalledWith( 'i_merge_request_widget_test_extension_click_full_report', -- GitLab From 1fc6a725e820ec10aa5be4eeaaa22c03bfeb79b3 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 15 Jun 2022 22:36:19 -0600 Subject: [PATCH 10/17] Test the telemetry when it's disabled --- .../vue_mr_widget/mr_widget_options_spec.js | 28 +++++++++++++++++++ .../frontend/vue_mr_widget/test_extensions.js | 5 ++++ 2 files changed, 33 insertions(+) diff --git a/spec/frontend/vue_mr_widget/mr_widget_options_spec.js b/spec/frontend/vue_mr_widget/mr_widget_options_spec.js index feaa1321744df4..cc5a701a1f774d 100644 --- a/spec/frontend/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/frontend/vue_mr_widget/mr_widget_options_spec.js @@ -30,6 +30,7 @@ import { collapsedDataErrorExtension, fullDataErrorExtension, fullReportExtension, + noTelemetryExtension, pollingExtension, pollingErrorExtension, multiPollingExtension, @@ -1216,5 +1217,32 @@ describe('MrWidgetOptions', () => { 'i_merge_request_widget_test_extension_count_click_full_report', ); }); + + describe('when disabled', () => { + afterEach(() => { + registeredExtensions.extensions = []; + }); + + it("doesn't emit any telemetry events", async () => { + registerExtension(noTelemetryExtension); + createComponent(); + + await waitForPromises(); + + api.trackRedisHllUserEvent.mockClear(); + api.trackRedisCounterEvent.mockClear(); + + // click expand button + wrapper + .find('[data-testid="widget-extension"] [data-testid="toggle-button"]') + .trigger('click'); + + // click full report link + wrapper.find('[data-testid="widget-extension"] [href="testref"]').trigger('click'); + + expect(api.trackRedisHllUserEvent).not.toHaveBeenCalled(); + expect(api.trackRedisCounterEvent).not.toHaveBeenCalled(); + }); + }); }); }); diff --git a/spec/frontend/vue_mr_widget/test_extensions.js b/spec/frontend/vue_mr_widget/test_extensions.js index cbaaeff44d973c..76644e0be77f51 100644 --- a/spec/frontend/vue_mr_widget/test_extensions.js +++ b/spec/frontend/vue_mr_widget/test_extensions.js @@ -126,6 +126,11 @@ export const fullReportExtension = { }, }; +export const noTelemetryExtension = { + ...fullReportExtension, + telemetry: false, +}; + export const multiPollingExtension = (endpointsToBePolled) => ({ name: 'WidgetTestMultiPollingExtension', props: [], -- GitLab From e043ef58d74f69247ed77843720c14f79a6a543c Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 15 Jun 2022 23:08:14 -0600 Subject: [PATCH 11/17] Don't handle the `null` result from the icon name It shouldn't be possible based on the logic. --- .../vue_merge_request_widget/components/extensions/base.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue index 6aa3289e0a2235..b4e5d5f39e7cbf 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue @@ -147,7 +147,7 @@ export default { toggleCollapsed(e) { if (this.isCollapsible && !e?.target?.closest('.btn:not(.btn-icon),a')) { if (this.isCollapsed) { - this.telemetry?.expanded({ type: this.statusIconName || 'loading' }); + this.telemetry?.expanded({ type: this.statusIconName }); } this.isCollapsed = !this.isCollapsed; -- GitLab From 155dde1c55d806a36ca87359ccbc48562e3dd466 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 15 Jun 2022 23:43:54 -0600 Subject: [PATCH 12/17] Use a generic action event to handle any action clicks This also handles any action clicks on any child_content, or any child_content of that child_content, or... ...well it's infinite, so it handles that. --- .../components/extensions/actions.vue | 5 +---- .../components/extensions/base.vue | 9 ++++++--- .../components/extensions/child_content.vue | 5 +++++ .../extensions/terraform/index.js | 1 + .../extentions/terraform/index_spec.js | 19 +++++++++++++++++++ 5 files changed, 32 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/actions.vue b/app/assets/javascripts/vue_merge_request_widget/components/extensions/actions.vue index d9f68942829416..655ceb5f7001e0 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/actions.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/actions.vue @@ -26,10 +26,7 @@ export default { }, methods: { onClickAction(action) { - if (action.fullReport) { - this.$emit('clickedFullReport'); - } - + this.$emit('clickedAction', action); if (action.onClick) { action.onClick(); } diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue index b4e5d5f39e7cbf..4ba620da00abfd 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue @@ -268,8 +268,10 @@ export default { this.toggleCollapsed(e); } }, - onClickedFullReport() { - this.telemetry?.fullReportClicked(); + onClickedAction(action) { + if (action.fullReport) { + this.telemetry?.fullReportClicked(); + } }, generateText, }, @@ -308,7 +310,7 @@ export default {
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/child_content.vue b/app/assets/javascripts/vue_merge_request_widget/components/extensions/child_content.vue index 39a76e3b4c4a42..c4f10301adafe7 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/child_content.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/child_content.vue @@ -39,6 +39,9 @@ export default { isArray(arr) { return Array.isArray(arr); }, + onClickedAction(...args) { + this.$emit('clickedAction', ...args); + }, generateText, }, }; @@ -85,6 +88,7 @@ export default { :widget="widgetLabel" :tertiary-buttons="data.actions" class="gl-ml-auto gl-pl-3" + @clickedAction="onClickedAction" />

diff --git a/app/assets/javascripts/vue_merge_request_widget/extensions/terraform/index.js b/app/assets/javascripts/vue_merge_request_widget/extensions/terraform/index.js index 349014875aa75c..6611aedcb07df3 100644 --- a/app/assets/javascripts/vue_merge_request_widget/extensions/terraform/index.js +++ b/app/assets/javascripts/vue_merge_request_widget/extensions/terraform/index.js @@ -112,6 +112,7 @@ export default { href: report.job_path, text: this.$options.i18n.fullLog, target: '_blank', + fullReport: true, }; actions.push(action); } diff --git a/spec/frontend/vue_mr_widget/extentions/terraform/index_spec.js b/spec/frontend/vue_mr_widget/extentions/terraform/index_spec.js index 70e1d7b6267236..77b3576a3d3ef9 100644 --- a/spec/frontend/vue_mr_widget/extentions/terraform/index_spec.js +++ b/spec/frontend/vue_mr_widget/extentions/terraform/index_spec.js @@ -1,6 +1,7 @@ import MockAdapter from 'axios-mock-adapter'; import { mountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; +import api from '~/api'; import axios from '~/lib/utils/axios_utils'; import Poll from '~/lib/utils/poll'; import extensionsContainer from '~/vue_merge_request_widget/components/extensions/container'; @@ -14,6 +15,8 @@ import { invalidPlanWithoutName, } from '../../components/terraform/mock_data'; +jest.mock('~/api.js'); + describe('Terraform extension', () => { let wrapper; let mock; @@ -130,6 +133,22 @@ describe('Terraform extension', () => { } }); }); + + it('responds with the correct telemetry when the deeply nested "Full log" link is clicked', () => { + api.trackRedisHllUserEvent.mockClear(); + api.trackRedisCounterEvent.mockClear(); + + findListItem(0).find('[data-testid="extension-actions-button"]').trigger('click'); + + expect(api.trackRedisHllUserEvent).toHaveBeenCalledTimes(1); + expect(api.trackRedisHllUserEvent).toHaveBeenCalledWith( + 'i_merge_request_widget_terraform_click_full_report', + ); + expect(api.trackRedisCounterEvent).toHaveBeenCalledTimes(1); + expect(api.trackRedisCounterEvent).toHaveBeenCalledWith( + 'i_merge_request_widget_terraform_count_click_full_report', + ); + }); }); describe('polling', () => { -- GitLab From 278e327e9f1ae8d1ff7579d762a70ca8484e9c44 Mon Sep 17 00:00:00 2001 From: Anna Vovchenko Date: Thu, 16 Jun 2022 06:05:56 +0000 Subject: [PATCH 13/17] Add underscore to make comment more readable --- .../components/extensions/telemetry.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/telemetry.js b/app/assets/javascripts/vue_merge_request_widget/components/extensions/telemetry.js index fc98d78727443d..aec3a35f37c4cd 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/telemetry.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/telemetry.js @@ -140,8 +140,8 @@ function baseTelemetry(componentName) { /* * Telemetry config format is: * { - * TELEMETRYTYPE: { - * BEHAVIOR: [ EVENTNAME, ... ] + * TELEMETRY_TYPE: { + * BEHAVIOR: [ EVENT_NAME, ... ] * } * } * -- GitLab From b558181d776f15d81ac7bc29eb5d7d19465042be Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Thu, 16 Jun 2022 00:13:48 -0600 Subject: [PATCH 14/17] Switch to find helper functions --- .../vue_mr_widget/mr_widget_options_spec.js | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/spec/frontend/vue_mr_widget/mr_widget_options_spec.js b/spec/frontend/vue_mr_widget/mr_widget_options_spec.js index cc5a701a1f774d..a20ee13d6fd690 100644 --- a/spec/frontend/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/frontend/vue_mr_widget/mr_widget_options_spec.js @@ -51,6 +51,8 @@ describe('MrWidgetOptions', () => { const COLLABORATION_MESSAGE = 'Members who can merge are allowed to add commits'; const findExtensionToggleButton = () => wrapper.find('[data-testid="widget-extension"] [data-testid="toggle-button"]'); + const findExtensionLink = (linkHref) => + wrapper.find(`[data-testid="widget-extension"] [href="${linkHref}"]`); beforeEach(() => { gl.mrWidgetData = { ...mockData }; @@ -1178,9 +1180,7 @@ describe('MrWidgetOptions', () => { api.trackRedisHllUserEvent.mockClear(); api.trackRedisCounterEvent.mockClear(); - wrapper - .find('[data-testid="widget-extension"] [data-testid="toggle-button"]') - .trigger('click'); + findExtensionToggleButton().trigger('click'); // The default working extension is a "warning" type, which generates a second - more specific - telemetry event for expansions expect(api.trackRedisHllUserEvent).toHaveBeenCalledTimes(2); @@ -1206,7 +1206,7 @@ describe('MrWidgetOptions', () => { api.trackRedisHllUserEvent.mockClear(); api.trackRedisCounterEvent.mockClear(); - wrapper.find('[data-testid="widget-extension"] [href="testref"]').trigger('click'); + findExtensionLink('testref').trigger('click'); expect(api.trackRedisHllUserEvent).toHaveBeenCalledTimes(1); expect(api.trackRedisHllUserEvent).toHaveBeenCalledWith( @@ -1232,13 +1232,8 @@ describe('MrWidgetOptions', () => { api.trackRedisHllUserEvent.mockClear(); api.trackRedisCounterEvent.mockClear(); - // click expand button - wrapper - .find('[data-testid="widget-extension"] [data-testid="toggle-button"]') - .trigger('click'); - - // click full report link - wrapper.find('[data-testid="widget-extension"] [href="testref"]').trigger('click'); + findExtensionToggleButton().trigger('click'); + findExtensionLink('testref').trigger('click'); // The "full report" link expect(api.trackRedisHllUserEvent).not.toHaveBeenCalled(); expect(api.trackRedisCounterEvent).not.toHaveBeenCalled(); -- GitLab From 040853ef3c60fed58c43d2a619af38a89b370159 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Thu, 16 Jun 2022 00:34:26 -0600 Subject: [PATCH 15/17] Test non-standard extension tracking events --- .../vue_mr_widget/mr_widget_options_spec.js | 71 +++++++++++++------ 1 file changed, 50 insertions(+), 21 deletions(-) diff --git a/spec/frontend/vue_mr_widget/mr_widget_options_spec.js b/spec/frontend/vue_mr_widget/mr_widget_options_spec.js index a20ee13d6fd690..3d668b2cd45850 100644 --- a/spec/frontend/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/frontend/vue_mr_widget/mr_widget_options_spec.js @@ -1171,31 +1171,60 @@ describe('MrWidgetOptions', () => { ); }); - it('triggers expand events when the expand button is clicked', async () => { - registerExtension(workingExtension()); - createComponent(); + describe('expand button', () => { + it('triggers expand events when clicked', async () => { + registerExtension(workingExtension()); + createComponent(); - await waitForPromises(); + await waitForPromises(); - api.trackRedisHllUserEvent.mockClear(); - api.trackRedisCounterEvent.mockClear(); + api.trackRedisHllUserEvent.mockClear(); + api.trackRedisCounterEvent.mockClear(); - findExtensionToggleButton().trigger('click'); + findExtensionToggleButton().trigger('click'); - // The default working extension is a "warning" type, which generates a second - more specific - telemetry event for expansions - expect(api.trackRedisHllUserEvent).toHaveBeenCalledTimes(2); - expect(api.trackRedisHllUserEvent).toHaveBeenCalledWith( - 'i_merge_request_widget_test_extension_expand', - ); - expect(api.trackRedisHllUserEvent).toHaveBeenCalledWith( - 'i_merge_request_widget_test_extension_expand_warning', - ); - expect(api.trackRedisCounterEvent).toHaveBeenCalledTimes(2); - expect(api.trackRedisCounterEvent).toHaveBeenCalledWith( - 'i_merge_request_widget_test_extension_count_expand', - ); - expect(api.trackRedisCounterEvent).toHaveBeenCalledWith( - 'i_merge_request_widget_test_extension_count_expand_warning', + // The default working extension is a "warning" type, which generates a second - more specific - telemetry event for expansions + expect(api.trackRedisHllUserEvent).toHaveBeenCalledTimes(2); + expect(api.trackRedisHllUserEvent).toHaveBeenCalledWith( + 'i_merge_request_widget_test_extension_expand', + ); + expect(api.trackRedisHllUserEvent).toHaveBeenCalledWith( + 'i_merge_request_widget_test_extension_expand_warning', + ); + expect(api.trackRedisCounterEvent).toHaveBeenCalledTimes(2); + expect(api.trackRedisCounterEvent).toHaveBeenCalledWith( + 'i_merge_request_widget_test_extension_count_expand', + ); + expect(api.trackRedisCounterEvent).toHaveBeenCalledWith( + 'i_merge_request_widget_test_extension_count_expand_warning', + ); + }); + + it.each` + widgetName | nonStandardEvent + ${'WidgetCodeQuality'} | ${'i_testing_code_quality_widget_total'} + ${'WidgetTerraform'} | ${'i_testing_terraform_widget_total'} + ${'WidgetIssues'} | ${'i_testing_load_performance_widget_total'} + ${'WidgetTestReport'} | ${'i_testing_summary_widget_total'} + `( + "sends non-standard events for the '$widgetName' widget", + async ({ widgetName, nonStandardEvent }) => { + const definition = { + ...workingExtension(), + name: widgetName, + }; + + registerExtension(definition); + createComponent(); + + await waitForPromises(); + + api.trackRedisHllUserEvent.mockClear(); + + findExtensionToggleButton().trigger('click'); + + expect(api.trackRedisHllUserEvent).toHaveBeenCalledWith(nonStandardEvent); + }, ); }); -- GitLab From c3da4d1a12a0984a87c4bd407cc30b14ccebea25 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Thu, 16 Jun 2022 00:41:43 -0600 Subject: [PATCH 16/17] Don't reset the mocked API - it should always be zero --- spec/frontend/vue_mr_widget/mr_widget_options_spec.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/spec/frontend/vue_mr_widget/mr_widget_options_spec.js b/spec/frontend/vue_mr_widget/mr_widget_options_spec.js index 3d668b2cd45850..6abbb052aefe06 100644 --- a/spec/frontend/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/frontend/vue_mr_widget/mr_widget_options_spec.js @@ -1258,9 +1258,6 @@ describe('MrWidgetOptions', () => { await waitForPromises(); - api.trackRedisHllUserEvent.mockClear(); - api.trackRedisCounterEvent.mockClear(); - findExtensionToggleButton().trigger('click'); findExtensionLink('testref').trigger('click'); // The "full report" link -- GitLab From 3935bd4ea5830b701edb509240a0e4b89bd79470 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Thu, 16 Jun 2022 09:57:12 -0600 Subject: [PATCH 17/17] Only collect the action button to re-emit the click event --- .../components/extensions/child_content.vue | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/child_content.vue b/app/assets/javascripts/vue_merge_request_widget/components/extensions/child_content.vue index c4f10301adafe7..38f83a61b30bf2 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/child_content.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/child_content.vue @@ -39,8 +39,8 @@ export default { isArray(arr) { return Array.isArray(arr); }, - onClickedAction(...args) { - this.$emit('clickedAction', ...args); + onClickedAction(action) { + this.$emit('clickedAction', action); }, generateText, }, -- GitLab