From 3d7e232e8c97050246c4e1b8f9a0d241b4f36f67 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Mon, 2 May 2022 21:29:29 -0600 Subject: [PATCH 1/5] Add a telemetry abstraction for MR Widget Extensions --- .../components/extensions/telemetry.js | 167 ++++++++++++++++++ .../vue_merge_request_widget/constants.js | 4 + 2 files changed, 171 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..6e13cac76d5ad9 --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/telemetry.js @@ -0,0 +1,167 @@ +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 || {}; + + if (toHll.view) { + isViewed({ execute: api.trackRedisHllUserEvent, track: toHll.view }); + } + if (toCounts.view) { + isViewed({ execute: api.trackRedisCounterEvent, track: toCounts.view }); + } + + if (toHll.expand) { + isExpanded({ + execute: api.trackRedisHllUserEvent, + track: toHll.expand, + special: ({ execute, track, busEvent }) => { + if (busEvent.type) { + track.forEach((event) => { + execute(`${event}_${busEvent.type}`); + }); + } + }, + }); + } + if (toCounts.expand) { + isExpanded({ + execute: api.trackRedisCounterEvent, + track: toCounts.expand, + special: ({ execute, track, busEvent }) => { + if (busEvent.type) { + track.forEach((event) => { + execute(`${event}_${busEvent.type}`); + }); + } + }, + }); + } + + if (toHll.clickFullReport) { + fullReportIsClicked({ execute: api.trackRedisHllUserEvent, track: toHll.clickFullReport }); + } + if (toCounts.clickFullReport) { + fullReportIsClicked({ execute: api.trackRedisCounterEvent, track: toHll.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 533bb38a88c0ee..4340ab9d075e23 100644 --- a/app/assets/javascripts/vue_merge_request_widget/constants.js +++ b/app/assets/javascripts/vue_merge_request_widget/constants.js @@ -165,4 +165,8 @@ 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 }; -- GitLab From 562f3bd0c28171e8d1c6cf843a2883d4dd82f293 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Mon, 2 May 2022 21:30:20 -0600 Subject: [PATCH 2/5] 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 a6b37d93979a5c..2f1f7bff0bada6 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() { @@ -265,6 +269,9 @@ export default { this.toggleCollapsed(e); } }, + onClickedFullReport() { + this.telemetry.fullReportClicked(); + }, generateText, }, EXTENSION_ICON_CLASS, @@ -302,6 +309,7 @@ export default {
Date: Mon, 2 May 2022 21:32:10 -0600 Subject: [PATCH 3/5] 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 e9267fab7b31ccc01bf3d469a98f843dc9332eda Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Mon, 2 May 2022 21:32:37 -0600 Subject: [PATCH 4/5] 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 2103a9b15b679e..c640b3b6ced7da 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 @@ -14,7 +14,6 @@ export default { name: 'WidgetTestSummary', enablePolling: true, i18n, - expandEvent: 'i_testing_summary_widget_total', props: ['testResultsPath', 'headBlobPath', 'pipeline'], computed: { summary(data) { -- GitLab From 2d9e8a5f3c1ebdffe3f5a0ca7cb4dd1e3b733a0d Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 10 May 2022 02:08:45 -0600 Subject: [PATCH 5/5] 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 | 41 ++++++++++++++++++- 1 file changed, 39 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 6e13cac76d5ad9..09bdffe467f8cc 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,25 @@ const nonStandardEvents = { }, }; +function combineDeepArray(path, ...objects) { + const parts = path.split('.'); + const allEntries = objects.reduce((entries, currentObject) => { + let traversed = currentObject; + + parts.forEach((part) => { + traversed = traversed?.[part]; + }); + + if (traversed) { + entries = [...entries, ...traversed]; + } + + return entries; + }, []); + + return Array.from(new Set(allEntries)); +} + function simplifyWidgetName(componentName) { const noWidget = componentName.replace(/^Widget/, ''); @@ -141,7 +159,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