From 34c8546995dc1b011419c1d13baf04c01a476b7a Mon Sep 17 00:00:00 2001 From: Boris Serdiuk Date: Mon, 29 Jan 2024 18:08:09 +0100 Subject: [PATCH] chore: Update metric length limits --- .../base-component/__tests__/metrics-init.ts | 36 ++ .../base-component/__tests__/metrics.test.ts | 389 ++++++++++++++++++ .../__tests__/panorama-metrics.test.ts | 74 ++++ .../__tests__/use-component-metrics.test.tsx | 42 -- .../base-component/metrics/log-clients.ts | 32 +- 5 files changed, 525 insertions(+), 48 deletions(-) create mode 100644 src/internal/base-component/__tests__/metrics-init.ts create mode 100644 src/internal/base-component/__tests__/metrics.test.ts create mode 100644 src/internal/base-component/__tests__/panorama-metrics.test.ts diff --git a/src/internal/base-component/__tests__/metrics-init.ts b/src/internal/base-component/__tests__/metrics-init.ts new file mode 100644 index 0000000..1b689cd --- /dev/null +++ b/src/internal/base-component/__tests__/metrics-init.ts @@ -0,0 +1,36 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { Metrics } from '../metrics/metrics'; + +declare global { + interface Window { + AWSC?: any; + } +} + +const metrics = new Metrics('components', '3.0 (HEAD)'); + +// This test file is separate from metrics.test.ts because we want to test an uninitialized Metrics module +describe('Metrics.initMetrics', () => { + beforeEach(() => { + window.AWSC = { + Clog: { + log: () => {}, + }, + }; + jest.spyOn(window.AWSC.Clog, 'log'); + }); + + test('is required before sending metrics', () => { + const consoleSpy = jest.spyOn(console, 'error'); + + metrics.sendMetric('name', 0); + expect(window.AWSC.Clog.log).toHaveBeenCalledTimes(0); + expect(consoleSpy).toHaveBeenCalled(); + + metrics.initMetrics('default'); + metrics.sendMetric('name', 0); + expect(window.AWSC.Clog.log).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/internal/base-component/__tests__/metrics.test.ts b/src/internal/base-component/__tests__/metrics.test.ts new file mode 100644 index 0000000..9d20daf --- /dev/null +++ b/src/internal/base-component/__tests__/metrics.test.ts @@ -0,0 +1,389 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { MetricsTestHelper, Metrics } from '../metrics/metrics'; + +declare global { + interface Window { + AWSC?: any; + panorama?: any; + } +} + +function mockConsoleError() { + beforeEach(() => { + jest.spyOn(console, 'error').mockImplementation(() => { + // setting to noop to prevent printing expected errors into console + }); + }); + + afterEach(() => { + expect(console.error).not.toHaveBeenCalled(); + }); +} + +describe('Client Metrics support', () => { + const metrics = new Metrics('dummy-package', '1.0'); + + const checkMetric = (metricName: string, detail: string[]) => { + const detailObject = { + o: detail[0], + s: detail[1], + t: detail[2], + a: detail[3], + f: detail[4], + v: detail[5], + }; + expect(window.AWSC.Clog.log).toHaveBeenCalledWith(metricName, 1, JSON.stringify(detailObject)); + expect(window.AWSC.Clog.log).toHaveBeenCalledTimes(1); + }; + + beforeEach(() => { + metrics.initMetrics('default'); + window.AWSC = { + Clog: { + log: () => {}, + }, + }; + window.panorama = () => {}; + jest.spyOn(window, 'panorama'); + jest.spyOn(window.AWSC.Clog, 'log'); + }); + + afterEach(() => { + jest.clearAllMocks(); + new MetricsTestHelper().resetOneTimeMetricsCache(); + }); + + describe('sendMetric', () => { + test('does nothing when window.AWSC is undefined', () => { + delete window.AWSC; + metrics.sendMetric('name', 0); // only proves no exception thrown + }); + + test('does nothing when window.AWSC.Clog is undefined', () => { + window.AWSC = undefined; + metrics.sendMetric('name', 0); // only proves no exception thrown + }); + + test('does nothing when window.AWSC.Clog.log is undefined', () => { + window.AWSC = { + Clog: undefined, + }; + metrics.sendMetric('name', 0); // only proves no exception thrown + }); + + describe('within an iframe', () => { + mockConsoleError(); + const setupIframe = () => { + const parentWindow = { ...window }; + Object.defineProperty(window, 'parent', { value: parentWindow }); + Object.defineProperty(parentWindow, 'parent', { value: parentWindow }); + }; + + test('does nothing when AWSC is not defined in the parent iframe', () => { + delete window.AWSC; + setupIframe(); + expect(window.parent.AWSC).toBeUndefined(); + + metrics.sendMetric('name', 0); // only proves no exception thrown + }); + + test('works if parent has AWSC', () => { + setupIframe(); + delete window.AWSC; + window.parent.AWSC = { + Clog: { + log: () => {}, + }, + }; + jest.spyOn(window.parent.AWSC.Clog, 'log'); + + metrics.sendMetric('name', 0, undefined); + expect(window.parent.AWSC.Clog.log).toHaveBeenCalledWith('name', 0, undefined); + }); + }); + + describe('when window.AWSC.Clog.log is defined', () => { + mockConsoleError(); + + test('delegates to window.AWSC.Clog.log when defined', () => { + metrics.sendMetric('name', 0, undefined); + expect(window.AWSC.Clog.log).toHaveBeenCalledWith('name', 0, undefined); + }); + + describe('Metric name validation', () => { + const tryValidMetric = (metricName: string) => { + test(`calls AWSC.Clog.log when valid metric name used (${metricName})`, () => { + metrics.sendMetric(metricName, 1, 'detail'); + expect(window.AWSC.Clog.log).toHaveBeenCalledWith(metricName, 1, 'detail'); + }); + }; + + const tryInvalidMetric = (metricName: string) => { + test(`logs an error when invalid metric name used (${metricName})`, () => { + metrics.sendMetric(metricName, 0, 'detail'); + expect(console.error).toHaveBeenCalledWith(`Invalid metric name: ${metricName}`); + jest.mocked(console.error).mockReset(); + }); + }; + + test('logs and error when metric name is too long', () => { + // 1001 char: too long + const longName = '1234567890'.repeat(100) + 'x'; + metrics.sendMetric(longName, 0, 'detail'); + expect(console.error).toHaveBeenCalledWith(`Metric name ${longName} is too long`); + jest.mocked(console.error).mockReset(); + }); + + tryValidMetric('1'); // min length 1 char + tryValidMetric('123456789'); // digits are ok + tryValidMetric('lowerUPPER'); // lower and uppercase chars ok + tryValidMetric('dash-dash-dash'); // dashes ok + tryValidMetric('1234567890'.repeat(100)); // 1000 chars: max length + + tryInvalidMetric(''); // too short, empty string not allowed + tryInvalidMetric('colons:not:allowed'); // invalid characters + tryInvalidMetric('spaces not allowed'); // invalid characters + }); + + describe('Metric detail validation', () => { + test('accepts details below the character limit', () => { + const validDetail = 'a'.repeat(4000); + metrics.sendMetric('metricName', 1, validDetail); + expect(window.AWSC.Clog.log).toHaveBeenCalledWith('metricName', 1, validDetail); + }); + + test('throws an error when detail is too long', () => { + const invalidDetail = 'a'.repeat(4001); + metrics.sendMetric('metricName', 0, invalidDetail); + expect(console.error).toHaveBeenCalledWith(`Detail for metric metricName is too long: ${invalidDetail}`); + jest.mocked(console.error).mockReset(); + }); + }); + }); + }); + + describe('sendMetricOnce', () => { + test('logs a metric name only once', () => { + metrics.sendMetricOnce('my-event', 1); + expect(window.AWSC.Clog.log).toHaveBeenCalledWith('my-event', 1, undefined); + expect(window.AWSC.Clog.log).toHaveBeenCalledTimes(1); + + metrics.sendMetricOnce('my-event', 2); + expect(window.AWSC.Clog.log).toHaveBeenCalledTimes(1); + + metrics.sendMetricOnce('My-Event', 3); + expect(window.AWSC.Clog.log).toHaveBeenCalledWith('My-Event', 3, undefined); + expect(window.AWSC.Clog.log).toHaveBeenCalledTimes(2); + }); + }); + + describe('sendMetricObject', () => { + describe('correctly maps input object to metric name', () => { + test('applies default values for theme (default) and framework (react)', () => { + metrics.sendMetricObject( + { + source: 'pkg', + action: 'used', + version: '5.0', + }, + 1 + ); + checkMetric('awsui_pkg_d50', ['main', 'pkg', 'default', 'used', 'react', '5.0']); + }); + + const versionTestCases = [ + ['', ['', '']], + ['5', ['', '5']], + ['5.100000000', ['5100000000', '5.100000000']], + ['5.7.0', ['57', '5.7.0']], + ['5.7 dkjhkhsgdjh', ['57', '5.7dkjhkhsgdjh']], + ['5.7.0 kjfhgjhdshjsjd', ['57', '5.7.0kjfhgjhdshjsjd']], + ]; + + versionTestCases.forEach(testCase => { + test(`correctly interprets version ${testCase[0]}`, () => { + metrics.sendMetricObject( + { + source: 'pkg', + action: 'used', + version: testCase[0] as string, + }, + 1 + ); + checkMetric(`awsui_pkg_d${testCase[1][0]}`, ['main', 'pkg', 'default', 'used', 'react', testCase[1][1]]); + }); + }); + }); + }); + + describe('sendMetricObjectOnce', () => { + test('logs a metric only once if same source and action', () => { + const metricObj = { + source: 'pkg', + action: 'used', + version: '5.0', + }; + + metrics.sendMetricObjectOnce(metricObj, 1); + metrics.sendMetricObjectOnce(metricObj, 1); + expect(window.AWSC.Clog.log).toHaveBeenCalledTimes(1); + }); + test('logs a metric only once if same source and action but different versions', () => { + metrics.sendMetricObjectOnce( + { + source: 'pkg1', + action: 'used', + version: '5.0', + }, + 1 + ); + metrics.sendMetricObjectOnce( + { + source: 'pkg1', + action: 'used', + version: '6.0', + }, + 1 + ); + expect(window.AWSC.Clog.log).toHaveBeenCalledTimes(1); + }); + test('logs a metric multiple times if same source but different actions', () => { + metrics.sendMetricObjectOnce( + { + source: 'pkg2', + action: 'used', + version: '5.0', + }, + 1 + ); + metrics.sendMetricObjectOnce( + { + source: 'pkg2', + action: 'loaded', + version: '5.0', + }, + 1 + ); + expect(window.AWSC.Clog.log).toHaveBeenCalledTimes(2); + }); + }); + + describe('initMetrics', () => { + test('sets theme', () => { + const metrics = new Metrics('dummy-package', 'dummy-version'); + metrics.initMetrics('dummy-theme'); + + // check that the theme is correctly set + metrics.sendMetricObject( + { + source: 'pkg', + action: 'used', + version: '5.0', + }, + 1 + ); + checkMetric(`awsui_pkg_d50`, ['main', 'pkg', 'dummy-theme', 'used', 'react', '5.0']); + }); + }); + + describe('logComponentUsed', () => { + test('logs the usage of the given component name', () => { + metrics.logComponentUsed('DummyComponentName'); + checkMetric(`awsui_DummyComponentName_d10`, ['main', 'DummyComponentName', 'default', 'used', 'react', '1.0']); + }); + }); + + describe('logComponentLoaded', () => { + test('logs the component loaded metric', () => { + metrics.logComponentLoaded(); + checkMetric(`awsui_dummy-package_d10`, ['main', 'dummy-package', 'default', 'loaded', 'react', '1.0']); + }); + }); + + describe('sendPanoramaMetric', () => { + test('does nothing when panorama is undefined', () => { + delete window.panorama; + metrics.sendPanoramaMetric({}); // only proves no exception thrown + }); + + describe('when panorama is defined', () => { + const metric = { + eventContext: 'context', + eventDetail: 'detail', + eventType: 'type', + eventValue: 'value', + }; + + mockConsoleError(); + + test('delegates to window.panorama when defined', () => { + const mockDateNow = new Date('2022-12-16T00:00:00.00Z').valueOf(); + jest.spyOn(global.Date, 'now').mockImplementationOnce(() => mockDateNow); + + metrics.sendPanoramaMetric(metric); + expect(window.panorama).toHaveBeenCalledWith('trackCustomEvent', { ...metric, timestamp: mockDateNow }); + }); + + describe('Metric detail validation', () => { + test('accepts event detail up to 200 characters', () => { + const inputMetric = { + ...metric, + eventDetail: new Array(201).join('a'), + }; + + metrics.sendPanoramaMetric(inputMetric); + expect(window.panorama).toHaveBeenCalledWith('trackCustomEvent', expect.objectContaining(inputMetric)); + }); + + test('throws an error when detail is too long', () => { + const invalidMetric = { + ...metric, + eventDetail: 'a'.repeat(4001), + }; + + metrics.sendPanoramaMetric(invalidMetric); + expect(console.error).toHaveBeenCalledWith( + `Event detail for metric is too long: ${invalidMetric.eventDetail}` + ); + jest.mocked(console.error).mockReset(); + }); + + test('accepts event detail as an object', () => { + const inputMetric = { + ...metric, + eventDetail: { + name: 'Hello', + }, + }; + + const expectedMetric = { + ...metric, + eventDetail: JSON.stringify(inputMetric.eventDetail), + }; + + metrics.sendPanoramaMetric(inputMetric); + expect(window.panorama).toHaveBeenCalledWith('trackCustomEvent', expect.objectContaining(expectedMetric)); + }); + + test('accepts event value as an object', () => { + const inputMetric = { + ...metric, + eventValue: { + name: 'Hello', + }, + }; + + const expectedMetric = { + ...metric, + eventValue: JSON.stringify(inputMetric.eventValue), + }; + + metrics.sendPanoramaMetric(inputMetric); + expect(window.panorama).toHaveBeenCalledWith('trackCustomEvent', expect.objectContaining(expectedMetric)); + }); + }); + }); + }); +}); diff --git a/src/internal/base-component/__tests__/panorama-metrics.test.ts b/src/internal/base-component/__tests__/panorama-metrics.test.ts new file mode 100644 index 0000000..8d0542b --- /dev/null +++ b/src/internal/base-component/__tests__/panorama-metrics.test.ts @@ -0,0 +1,74 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { PanoramaClient } from '../metrics/log-clients'; + +declare global { + interface Window { + panorama?: any; + } +} + +describe('PanoramaClient', () => { + const panorama = new PanoramaClient(); + let consoleSpy: jest.SpyInstance; + + beforeEach(() => { + window.panorama = jest.fn(); + consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + test('sends simple metrics', () => { + panorama.sendMetric({ eventType: 'custom', eventValue: 'value' }); + + expect(window.panorama).toHaveBeenCalledWith( + 'trackCustomEvent', + expect.objectContaining({ eventType: 'custom', eventValue: 'value' }) + ); + }); + + test('converts objects to strings', () => { + panorama.sendMetric({ eventType: 'custom', eventValue: { test: 'value' }, eventDetail: { test: 'detail' } }); + + expect(window.panorama).toHaveBeenCalledWith( + 'trackCustomEvent', + expect.objectContaining({ eventType: 'custom', eventValue: '{"test":"value"}', eventDetail: '{"test":"detail"}' }) + ); + }); + + test('prints an error when event details are too long', () => { + const eventDetail = 'a'.repeat(4001); + panorama.sendMetric({ eventType: 'custom', eventDetail }); + + expect(window.panorama).not.toHaveBeenCalled(); + expect(consoleSpy).toHaveBeenCalledWith(`Event detail for metric is too long: ${eventDetail}`); + }); + + test('prints an error when event type is too long', () => { + const eventType = 'a'.repeat(51); + panorama.sendMetric({ eventType }); + + expect(window.panorama).not.toHaveBeenCalled(); + expect(consoleSpy).toHaveBeenCalledWith(`Event type for metric is too long: ${eventType}`); + }); + + test('prints an error when event value is too long', () => { + const eventValue = 'a'.repeat(4001); + panorama.sendMetric({ eventValue }); + + expect(window.panorama).not.toHaveBeenCalled(); + expect(consoleSpy).toHaveBeenCalledWith(`Event value for metric is too long: ${eventValue}`); + }); + + test('prints an error when event context is too long', () => { + const eventContext = 'a'.repeat(4001); + panorama.sendMetric({ eventContext }); + + expect(window.panorama).not.toHaveBeenCalled(); + expect(consoleSpy).toHaveBeenCalledWith(`Event context for metric is too long: ${eventContext}`); + }); +}); diff --git a/src/internal/base-component/__tests__/use-component-metrics.test.tsx b/src/internal/base-component/__tests__/use-component-metrics.test.tsx index 45877f9..ef7fc4c 100644 --- a/src/internal/base-component/__tests__/use-component-metrics.test.tsx +++ b/src/internal/base-component/__tests__/use-component-metrics.test.tsx @@ -6,13 +6,11 @@ import { render } from '@testing-library/react'; import { MetricsTestHelper } from '../metrics/metrics'; import { formatVersionForMetricName, formatMajorVersionForMetricDetail } from '../metrics/formatters'; import { useComponentMetrics } from '../component-metrics'; -import { PanoramaClient } from '../metrics/log-clients'; declare global { interface Window { AWSC?: any; AWSUI_METRIC_ORIGIN?: string; - panorama?: any; } } @@ -122,43 +120,3 @@ describe('useComponentMetrics', () => { ); }); }); - -describe('PanoramaClient', () => { - const panorama = new PanoramaClient(); - let consoleSpy: jest.SpyInstance; - - beforeEach(() => { - window.panorama = jest.fn(); - consoleSpy = jest.spyOn(console, 'error'); - }); - - afterEach(() => { - jest.clearAllMocks(); - }); - - it('sends simple metrics', () => { - panorama.sendMetric({ eventType: 'custom', eventValue: 'value' }); - - expect(window.panorama).toHaveBeenCalledWith( - 'trackCustomEvent', - expect.objectContaining({ eventType: 'custom', eventValue: 'value' }) - ); - }); - - it('converts objects to strings', () => { - panorama.sendMetric({ eventType: 'custom', eventValue: { test: 'value' }, eventDetail: { test: 'detail' } }); - - expect(window.panorama).toHaveBeenCalledWith( - 'trackCustomEvent', - expect.objectContaining({ eventType: 'custom', eventValue: '{"test":"value"}', eventDetail: '{"test":"detail"}' }) - ); - }); - - it('prints error when the details are too long', () => { - const eventDetail = new Array(202).join('a'); - panorama.sendMetric({ eventType: 'custom', eventDetail }); - - expect(window.panorama).not.toHaveBeenCalled(); - expect(consoleSpy).toHaveBeenCalledWith(`Detail for metric is too long: ${eventDetail}`); - }); -}); diff --git a/src/internal/base-component/metrics/log-clients.ts b/src/internal/base-component/metrics/log-clients.ts index 3437275..651b846 100644 --- a/src/internal/base-component/metrics/log-clients.ts +++ b/src/internal/base-component/metrics/log-clients.ts @@ -23,6 +23,10 @@ export interface MetricsLogItem { version: string; } +function validateLength(value: string | undefined, maxLength: number): boolean { + return !value || value.length <= maxLength; +} + /** * Console Platform's client logging JS API client. */ @@ -31,11 +35,15 @@ export class CLogClient { * Sends metric but only if Console Platform client logging JS API is present in the page. */ sendMetric(metricName: string, value: number, detail?: string): void { - if (!metricName || !/^[a-zA-Z0-9_-]{1,32}$/.test(metricName)) { + if (!metricName || !/^[a-zA-Z0-9_-]+$/.test(metricName)) { console.error(`Invalid metric name: ${metricName}`); return; } - if (detail && detail.length > 200) { + if (!validateLength(metricName, 1000)) { + console.error(`Metric name ${metricName} is too long`); + return; + } + if (!validateLength(detail, 4000)) { console.error(`Detail for metric ${metricName} is too long: ${detail}`); return; } @@ -75,13 +83,25 @@ export class PanoramaClient { if (typeof metric.eventDetail === 'object') { metric.eventDetail = JSON.stringify(metric.eventDetail); } - if (metric.eventDetail && metric.eventDetail.length > 200) { - console.error(`Detail for metric is too long: ${metric.eventDetail}`); - return; - } if (typeof metric.eventValue === 'object') { metric.eventValue = JSON.stringify(metric.eventValue); } + if (!validateLength(metric.eventDetail, 4000)) { + console.error(`Event detail for metric is too long: ${metric.eventDetail}`); + return; + } + if (!validateLength(metric.eventValue, 4000)) { + console.error(`Event value for metric is too long: ${metric.eventValue}`); + return; + } + if (!validateLength(metric.eventContext, 4000)) { + console.error(`Event context for metric is too long: ${metric.eventContext}`); + return; + } + if (!validateLength(metric.eventType, 50)) { + console.error(`Event type for metric is too long: ${metric.eventType}`); + return; + } const panorama = this.findPanorama(window); if (typeof panorama === 'function') { panorama('trackCustomEvent', { ...metric, timestamp: Date.now() });