Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Report metric errors #110

Merged
merged 1 commit into from
Jan 13, 2025
Merged

feat: Report metric errors #110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 29 additions & 7 deletions src/internal/base-component/__tests__/panorama-metrics.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,39 +44,61 @@ describe('PanoramaClient', () => {
const eventDetail = 'a'.repeat(4001);
panorama.sendMetric({ eventType: 'custom', eventDetail });

expect(window.panorama).not.toHaveBeenCalled();
expect(window.panorama).toHaveBeenCalledTimes(1);
expect(window.panorama).toHaveBeenCalledWith('trackCustomEvent', {
eventName: 'awsui-metric-error',
eventDetail: expect.stringMatching(/Event detail for metric is too long:.*/),
});
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);
const errorMessage = `Event type for metric is too long: ${eventType}`;
panorama.sendMetric({ eventType });

expect(window.panorama).not.toHaveBeenCalled();
expect(consoleSpy).toHaveBeenCalledWith(`Event type for metric is too long: ${eventType}`);
expect(window.panorama).toHaveBeenCalledTimes(1);
expect(window.panorama).toHaveBeenCalledWith('trackCustomEvent', {
eventName: 'awsui-metric-error',
eventDetail: errorMessage,
});
expect(consoleSpy).toHaveBeenCalledWith(errorMessage);
});

test('prints an error when event name is too long', () => {
const eventName = 'a'.repeat(1001);
const errorMessage = `Event name for metric is too long: ${eventName}`;
panorama.sendMetric({ eventName });

expect(window.panorama).not.toHaveBeenCalled();
expect(consoleSpy).toHaveBeenCalledWith(`Event name for metric is too long: ${eventName}`);
expect(window.panorama).toHaveBeenCalledTimes(1);
expect(window.panorama).toHaveBeenCalledWith('trackCustomEvent', {
eventName: 'awsui-metric-error',
eventDetail: errorMessage,
});
expect(consoleSpy).toHaveBeenCalledWith(errorMessage);
});

test('prints an error when event value is too long', () => {
const eventValue = 'a'.repeat(4001);
panorama.sendMetric({ eventValue });

expect(window.panorama).not.toHaveBeenCalled();
expect(window.panorama).toHaveBeenCalledTimes(1);
expect(window.panorama).toHaveBeenCalledWith('trackCustomEvent', {
eventName: 'awsui-metric-error',
eventDetail: expect.stringMatching(/Event value for metric is too long:.*/),
});
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(window.panorama).toHaveBeenCalledTimes(1);
expect(window.panorama).toHaveBeenCalledWith('trackCustomEvent', {
eventName: 'awsui-metric-error',
eventDetail: expect.stringMatching(/Event context for metric is too long:.*/),
});
expect(consoleSpy).toHaveBeenCalledWith(`Event context for metric is too long: ${eventContext}`);
});

Expand Down
27 changes: 20 additions & 7 deletions src/internal/base-component/metrics/log-clients.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ export interface MetricsV2EventItem {
timestamp?: number;
}

type PanoramaFunction = (event: 'trackCustomEvent', data: MetricsV2EventItem) => void;

function validateLength(value: string | undefined, maxLength: number): boolean {
return !value || value.length <= maxLength;
}
Expand Down Expand Up @@ -86,7 +88,7 @@ export class PanoramaClient {
*/
sendMetric(metric: MetricsV2EventItem): boolean {
const panorama = this.findPanorama(window);
if (typeof panorama !== 'function') {
if (!panorama) {
return false;
}
if (typeof metric.eventDetail === 'object') {
Expand All @@ -96,30 +98,41 @@ export class PanoramaClient {
metric.eventValue = JSON.stringify(metric.eventValue);
}
if (!validateLength(metric.eventName, 1000)) {
console.error(`Event name for metric is too long: ${metric.eventName}`);
this.onMetricError(`Event name for metric is too long: ${metric.eventName}`);
return true;
}
if (!validateLength(metric.eventDetail, 4000)) {
console.error(`Event detail for metric is too long: ${metric.eventDetail}`);
this.onMetricError(`Event detail for metric is too long: ${metric.eventDetail}`);
return true;
}
if (!validateLength(metric.eventValue, 4000)) {
console.error(`Event value for metric is too long: ${metric.eventValue}`);
this.onMetricError(`Event value for metric is too long: ${metric.eventValue}`);
return true;
}
if (!validateLength(metric.eventContext, 4000)) {
console.error(`Event context for metric is too long: ${metric.eventContext}`);
this.onMetricError(`Event context for metric is too long: ${metric.eventContext}`);
return true;
}
if (!validateLength(metric.eventType, 50)) {
console.error(`Event type for metric is too long: ${metric.eventType}`);
this.onMetricError(`Event type for metric is too long: ${metric.eventType}`);
return true;
}
panorama('trackCustomEvent', { timestamp: Date.now(), ...metric });
return true;
}

private findPanorama(currentWindow?: MetricsWindow): any | undefined {
private onMetricError(message: string) {
console.error(message);
const panorama = this.findPanorama(window);
if (panorama) {
panorama('trackCustomEvent', {
Copy link
Member Author

Choose a reason for hiding this comment

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

Theoretically, this could be a this.sendMetric() call. But I decided to avoid recursion for safety and call the panorama function directly.

Safety > code reuse

eventName: 'awsui-metric-error',
eventDetail: message.slice(0, 4000),
});
}
}

private findPanorama(currentWindow?: MetricsWindow): PanoramaFunction | undefined {
try {
if (typeof currentWindow?.panorama === 'function') {
return currentWindow?.panorama;
Expand Down
Loading