From bbcc07802ad4994843db18a01cb2b9cff6a7069e Mon Sep 17 00:00:00 2001 From: Reino Muhl Date: Thu, 8 Feb 2024 13:53:27 -0500 Subject: [PATCH] feat(contentful): Add statsd metrics to contentful Because: * Want to track number of requests and timings of those requests made to contentful. This commit: * Adds event emitter to Contentful Client, similar to PayPal Client. * Add statsd to Contentful Manager to to track requests timings on contentful client event. Closes #FXA-9031 --- .../src/lib/stripe-mapper.service.spec.ts | 5 +- .../src/lib/contentful.client.spec.ts | 50 +++++++++++++ .../contentful/src/lib/contentful.client.ts | 75 +++++++++++++++++++ .../src/lib/contentful.manager.spec.ts | 27 ++++++- .../contentful/src/lib/contentful.manager.ts | 22 +++++- packages/fxa-auth-server/bin/key_server.js | 2 +- 6 files changed, 176 insertions(+), 5 deletions(-) diff --git a/libs/payments/legacy/src/lib/stripe-mapper.service.spec.ts b/libs/payments/legacy/src/lib/stripe-mapper.service.spec.ts index 8bf3312034..b8bfd11bee 100644 --- a/libs/payments/legacy/src/lib/stripe-mapper.service.spec.ts +++ b/libs/payments/legacy/src/lib/stripe-mapper.service.spec.ts @@ -26,9 +26,10 @@ describe('StripeMapperService', () => { .mockResolvedValue( mockContentfulConfigUtil as unknown as PurchaseWithDetailsOfferingContentUtil ); - const contentfulClient = {} as ContentfulClient; + const contentfulClient = new ContentfulClient({} as any); + const mockStatsd = {} as any; stripeMapper = new StripeMapperService( - new ContentfulManager(contentfulClient) + new ContentfulManager(contentfulClient, mockStatsd) ); }); diff --git a/libs/shared/contentful/src/lib/contentful.client.spec.ts b/libs/shared/contentful/src/lib/contentful.client.spec.ts index 6f11655cdd..bf5e81b098 100644 --- a/libs/shared/contentful/src/lib/contentful.client.spec.ts +++ b/libs/shared/contentful/src/lib/contentful.client.spec.ts @@ -24,6 +24,7 @@ jest.mock('graphql-request', () => ({ describe('ContentfulClient', () => { let contentfulClient: ContentfulClient; + const onCallback = jest.fn(); beforeEach(() => { contentfulClient = new ContentfulClient({ @@ -33,6 +34,11 @@ describe('ContentfulClient', () => { graphqlSpaceId: faker.string.uuid(), graphqlEnvironment: faker.string.uuid(), }); + contentfulClient.on('response', onCallback); + }); + + afterEach(() => { + onCallback.mockClear(); }); describe('query', () => { @@ -67,6 +73,23 @@ describe('ContentfulClient', () => { }, }); }); + + it('emits event and on second request emits event with cache true', async () => { + expect(onCallback).toHaveBeenCalledWith( + expect.objectContaining({ method: 'query', cache: false }) + ); + + (contentfulClient.client.request as jest.Mock).mockResolvedValueOnce( + mockResponse + ); + result = await contentfulClient.query(offeringQuery, { + id, + locale, + }); + expect(onCallback).toHaveBeenCalledWith( + expect.objectContaining({ method: 'query', cache: true }) + ); + }); }); it('throws an error when the graphql request fails', async () => { @@ -81,6 +104,14 @@ describe('ContentfulClient', () => { locale, }) ).rejects.toThrow(new ContentfulError([error])); + + expect(onCallback).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'query', + cache: false, + error: expect.anything(), + }) + ); }); }); @@ -125,6 +156,18 @@ describe('ContentfulClient', () => { await contentfulClient.getLocale(ACCEPT_LANGUAGE); expect(global.fetch).toBeCalledTimes(1); }); + + it('emits event and on second request emits event with cache true', async () => { + await contentfulClient.getLocale(ACCEPT_LANGUAGE); + expect(onCallback).toHaveBeenCalledWith( + expect.objectContaining({ method: 'getLocales', cache: false }) + ); + + await contentfulClient.getLocale(ACCEPT_LANGUAGE); + expect(onCallback).toHaveBeenCalledWith( + expect.objectContaining({ method: 'getLocales', cache: true }) + ); + }); }); describe('errors', () => { @@ -145,6 +188,13 @@ describe('ContentfulClient', () => { cdnErrorResult.message ) ); + expect(onCallback).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'getLocales', + cache: false, + error: expect.anything(), + }) + ); }); it('throws a cdn execution error when contentful cant be reached', async () => { diff --git a/libs/shared/contentful/src/lib/contentful.client.ts b/libs/shared/contentful/src/lib/contentful.client.ts index 736bf0ddc8..5bfcbd6139 100644 --- a/libs/shared/contentful/src/lib/contentful.client.ts +++ b/libs/shared/contentful/src/lib/contentful.client.ts @@ -15,9 +15,21 @@ import { ContentfulError, } from './contentful.error'; import { ContentfulErrorResponse } from './types'; +import EventEmitter from 'events'; const DEFAULT_CACHE_TTL = 300000; // Milliseconds +interface EventResponse { + method: string; + requestStartTime: number; + requestEndTime: number; + elapsed: number; + cache: boolean; + query?: TypedDocumentNode; + variables?: string; + error?: Error; +} + @Injectable() export class ContentfulClient { client = new GraphQLClient( @@ -25,9 +37,16 @@ export class ContentfulClient { ); private locales: string[] = []; private graphqlResultCache: Record = {}; + private emitter: EventEmitter; + public on: ( + event: 'response', + listener: (response: EventResponse) => void + ) => EventEmitter; constructor(private contentfulClientConfig: ContentfulClientConfig) { this.setupCacheBust(); + this.emitter = new EventEmitter(); + this.on = this.emitter.on.bind(this.emitter); } async getLocale(acceptLanguage: string): Promise { @@ -50,7 +69,21 @@ export class ContentfulClient { ); const cacheKey = variablesString + query; + const emitterResponse = { + method: 'query', + query, + variables: variablesString, + requestStartTime: Date.now(), + cache: false, + }; + if (this.graphqlResultCache[cacheKey]) { + this.emitter.emit('response', { + ...emitterResponse, + requestEndTime: emitterResponse.requestStartTime, + elapsed: 0, + cache: true, + }); return this.graphqlResultCache[cacheKey] as Result; } @@ -60,22 +93,57 @@ export class ContentfulClient { variables, }); + const requestEndTime = Date.now(); + this.emitter.emit('response', { + ...emitterResponse, + elapsed: requestEndTime - emitterResponse.requestStartTime, + requestEndTime, + }); + this.graphqlResultCache[cacheKey] = response; return response; } catch (e) { + const requestEndTime = Date.now(); + this.emitter.emit('response', { + ...emitterResponse, + elapsed: requestEndTime - emitterResponse.requestStartTime, + requestEndTime, + error: e, + }); + throw new ContentfulError([e]); } } private async getLocales(): Promise { + const emitterResponse = { + method: 'getLocales', + requestStartTime: Date.now(), + cache: false, + }; + if (!!this.locales?.length) { + this.emitter.emit('response', { + ...emitterResponse, + cache: true, + elapsed: 0, + requestEndTime: emitterResponse.requestStartTime, + }); return this.locales; } try { const localesUrl = `${this.contentfulClientConfig.cdnApiUri}/spaces/${this.contentfulClientConfig.graphqlSpaceId}/environments/${this.contentfulClientConfig.graphqlEnvironment}/locales?access_token=${this.contentfulClientConfig.graphqlApiKey}`; const response = await fetch(localesUrl); + + const requestEndTime = Date.now(); + this.emitter.emit('response', { + ...emitterResponse, + elapsed: requestEndTime - emitterResponse.requestStartTime, + requestEndTime, + }); + const results = await response.json(); if (!response.ok) { @@ -91,6 +159,13 @@ export class ContentfulClient { return this.locales; } catch (error) { + const requestEndTime = Date.now(); + this.emitter.emit('response', { + ...emitterResponse, + elapsed: requestEndTime - emitterResponse.requestStartTime, + requestEndTime, + error, + }); if (error instanceof ContentfulCDNError) { throw error; } else { diff --git a/libs/shared/contentful/src/lib/contentful.manager.spec.ts b/libs/shared/contentful/src/lib/contentful.manager.spec.ts index 1b661283cb..4b529cdd63 100644 --- a/libs/shared/contentful/src/lib/contentful.manager.spec.ts +++ b/libs/shared/contentful/src/lib/contentful.manager.spec.ts @@ -19,15 +19,21 @@ import { } from '../../src'; import { PurchaseWithDetailsOfferingContentUtil } from './queries/purchase-with-details-offering-content'; import { PurchaseWithDetailsOfferingContentByPlanIdsResultFactory } from './queries/purchase-with-details-offering-content/factories'; +import { StatsD } from 'hot-shots'; describe('ContentfulManager', () => { let manager: ContentfulManager; let mockClient: ContentfulClient; + let mockStatsd: StatsD; beforeEach(async () => { - mockClient = {} as any; + mockClient = new ContentfulClient({} as any); + mockStatsd = { + timing: jest.fn().mockReturnValue({}), + } as unknown as StatsD; const module: TestingModule = await Test.createTestingModule({ providers: [ + { provide: StatsD, useValue: mockStatsd }, { provide: ContentfulClient, useValue: mockClient }, ContentfulManager, ], @@ -40,6 +46,25 @@ describe('ContentfulManager', () => { expect(manager).toBeDefined(); }); + it('should call statsd for incoming events', async () => { + const queryData = EligibilityContentByPlanIdsQueryFactory({ + purchaseCollection: { items: [], total: 0 }, + }); + mockClient.client.request = jest.fn().mockResolvedValue(queryData); + await manager.getPurchaseDetailsForEligibility(['test']); + expect(mockStatsd.timing).toHaveBeenCalledWith( + 'contentful_request', + expect.any(Number), + undefined, + { + method: 'query', + error: 'false', + cache: 'false', + operationName: 'EligibilityContentByPlanIds', + } + ); + }); + describe('getPurchaseDetailsForEligibility', () => { it('should return empty result', async () => { const queryData = EligibilityContentByPlanIdsQueryFactory({ diff --git a/libs/shared/contentful/src/lib/contentful.manager.ts b/libs/shared/contentful/src/lib/contentful.manager.ts index 858862a38a..39b4a268dd 100644 --- a/libs/shared/contentful/src/lib/contentful.manager.ts +++ b/libs/shared/contentful/src/lib/contentful.manager.ts @@ -30,10 +30,30 @@ import { servicesWithCapabilitiesQuery, } from './queries/services-with-capabilities'; import { DeepNonNullable } from './types'; +import { StatsD } from 'hot-shots'; +import { getOperationName } from '@apollo/client/utilities'; @Injectable() export class ContentfulManager { - constructor(private client: ContentfulClient) {} + constructor(private client: ContentfulClient, private statsd: StatsD) { + this.client.on('response', (response) => { + const defaultTags = { + method: response.method, + error: response.error ? 'true' : 'false', + cache: `${response.cache}`, + }; + const operationName = response.query && getOperationName(response.query); + const tags = operationName + ? { ...defaultTags, operationName } + : defaultTags; + this.statsd.timing( + 'contentful_request', + response.elapsed, + undefined, + tags + ); + }); + } async getPurchaseDetailsForCapabilityServiceByPlanIds( stripePlanIds: string[] diff --git a/packages/fxa-auth-server/bin/key_server.js b/packages/fxa-auth-server/bin/key_server.js index 33cb74271e..9c9acd2fa7 100755 --- a/packages/fxa-auth-server/bin/key_server.js +++ b/packages/fxa-auth-server/bin/key_server.js @@ -129,7 +129,7 @@ async function run(config) { graphqlSpaceId: config.contentful.spaceId, graphqlEnvironment: config.contentful.environment, }); - const contentfulManager = new ContentfulManager(contentfulClient); + const contentfulManager = new ContentfulManager(contentfulClient, statsd); Container.set(ContentfulManager, contentfulManager); const capabilityManager = new CapabilityManager(contentfulManager); const eligibilityManager = new EligibilityManager(contentfulManager);