Skip to content

Commit 7b78484

Browse files
julianpoyjulianpoy
authored
Merge pull request #18441 from mozilla/FXA-10618
feat(payments): add statsd logging to hot paths
2 parents a32ce5c + 6eaf8f1 commit 7b78484

36 files changed

Lines changed: 523 additions & 128 deletions

‎libs/google/src/lib/google.client.spec.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { Test } from '@nestjs/testing';
66
import { GeocodeResultFactory } from './factories';
77
import { GoogleClient } from './google.client';
88
import { MockGoogleClientConfigProvider } from './google.client.config';
9+
import { MockStatsDProvider } from '@fxa/shared/metrics/statsd';
910

1011
const mockJestFnGenerator = <T extends (...args: any[]) => any>() => {
1112
return jest.fn<ReturnType<T>, Parameters<T>>();
@@ -26,7 +27,11 @@ describe('GoogleClient', () => {
2627

2728
beforeEach(async () => {
2829
const module = await Test.createTestingModule({
29-
providers: [GoogleClient, MockGoogleClientConfigProvider],
30+
providers: [
31+
GoogleClient,
32+
MockGoogleClientConfigProvider,
33+
MockStatsDProvider,
34+
],
3035
}).compile();
3136

3237
googleClient = module.get<GoogleClient>(GoogleClient);

‎libs/google/src/lib/google.client.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,29 @@ import {
66
Client,
77
GeocodeResponseData,
88
} from '@googlemaps/google-maps-services-js';
9-
import { Injectable } from '@nestjs/common';
9+
import { Inject, Injectable } from '@nestjs/common';
1010
import { GoogleClientConfig } from './google.client.config';
11+
import {
12+
CaptureTimingWithStatsD,
13+
StatsDService,
14+
type StatsD,
15+
} from '@fxa/shared/metrics/statsd';
1116

1217
@Injectable()
1318
export class GoogleClient {
1419
private readonly google: Client;
15-
constructor(private googleClientConfig: GoogleClientConfig) {
20+
constructor(
21+
private googleClientConfig: GoogleClientConfig,
22+
@Inject(StatsDService) public statsd: StatsD
23+
) {
1624
this.google = new Client();
1725
}
1826

1927
/**
2028
* Retrieve Geocode Data for the specified address. For more information review
2129
* https://developers.google.com/maps/documentation/geocoding/overview
2230
*/
31+
@CaptureTimingWithStatsD()
2332
async geocode(address: string, countryCode: string) {
2433
const response = await this.google.geocode({
2534
params: {

‎libs/google/src/lib/google.manager.spec.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,20 @@ import { GeocodeResultFactory } from './factories';
88
import { GoogleClient } from './google.client';
99
import { MockGoogleClientConfigProvider } from './google.client.config';
1010
import { GoogleManager } from './google.manager';
11+
import { MockStatsDProvider } from '@fxa/shared/metrics/statsd';
1112

1213
describe('GoogleManager', () => {
1314
let googleClient: GoogleClient;
1415
let googleManager: GoogleManager;
1516

1617
beforeEach(async () => {
1718
const module = await Test.createTestingModule({
18-
providers: [GoogleClient, GoogleManager, MockGoogleClientConfigProvider],
19+
providers: [
20+
GoogleClient,
21+
GoogleManager,
22+
MockGoogleClientConfigProvider,
23+
MockStatsDProvider,
24+
],
1925
}).compile();
2026

2127
googleClient = module.get<GoogleClient>(GoogleClient);

‎libs/payments/cart/src/lib/cart.service.spec.ts‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -854,12 +854,13 @@ describe('CartService', () => {
854854

855855
await cartService.finalizeProcessingCart(mockCart.id);
856856

857-
expect(checkoutService.postPaySteps).toHaveBeenCalledWith(
858-
mockCart,
859-
mockCart.version,
860-
mockSubscription,
861-
mockCart.uid
862-
);
857+
expect(checkoutService.postPaySteps).toHaveBeenCalledWith({
858+
cart: mockCart,
859+
version: mockCart.version,
860+
subscription: mockSubscription,
861+
uid: mockCart.uid,
862+
paymentProvider: 'stripe',
863+
});
863864
});
864865
});
865866

@@ -1543,12 +1544,13 @@ describe('CartService', () => {
15431544
default_payment_method: mockPaymentMethod.id,
15441545
},
15451546
});
1546-
expect(checkoutService.postPaySteps).toHaveBeenCalledWith(
1547-
mockCart,
1548-
mockCart.version,
1549-
mockSubscription,
1550-
mockCart.uid
1551-
);
1547+
expect(checkoutService.postPaySteps).toHaveBeenCalledWith({
1548+
cart: mockCart,
1549+
version: mockCart.version,
1550+
subscription: mockSubscription,
1551+
uid: mockCart.uid,
1552+
paymentProvider: 'stripe',
1553+
});
15521554
expect(cartManager.finishErrorCart).not.toHaveBeenCalled();
15531555
});
15541556

‎libs/payments/cart/src/lib/cart.service.ts‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -196,11 +196,6 @@ export class CartService {
196196
uid?: string;
197197
ip?: string;
198198
}): Promise<ResultCart> {
199-
// TODO:
200-
// - Fetch information about interval, offering, experiments from CMS
201-
// - Guess TaxAddress via maxmind client
202-
// - Check if user is eligible to subscribe to plan, else throw error
203-
// - Fetch stripeCustomerId if uid is passed and has a customer id
204199
let accountCustomer;
205200
if (args.uid) {
206201
accountCustomer = await this.accountCustomerManager
@@ -445,12 +440,14 @@ export class CartService {
445440
if (!subscription) {
446441
throw new CartSubscriptionNotFoundError(cartId);
447442
}
448-
await this.checkoutService.postPaySteps(
443+
await this.checkoutService.postPaySteps({
449444
cart,
450-
cart.version,
445+
version: cart.version,
451446
subscription,
452-
cart.uid
453-
);
447+
uid: cart.uid,
448+
paymentProvider:
449+
this.subscriptionManager.getPaymentProvider(subscription),
450+
});
454451
});
455452
}
456453

@@ -605,7 +602,10 @@ export class CartService {
605602
} else if (subscriptions.length) {
606603
const firstListedSubscription = subscriptions[0];
607604
// fetch payment method info
608-
if (firstListedSubscription.collection_method === 'send_invoice') {
605+
if (
606+
this.subscriptionManager.getPaymentProvider(firstListedSubscription) ===
607+
'paypal'
608+
) {
609609
// PayPal payment method collection
610610
paymentInfo = {
611611
type: 'external_paypal',
@@ -770,12 +770,14 @@ export class CartService {
770770
const subscription = await this.subscriptionManager.retrieve(
771771
cart.stripeSubscriptionId
772772
);
773-
await this.checkoutService.postPaySteps(
773+
await this.checkoutService.postPaySteps({
774774
cart,
775-
cart.version,
775+
version: cart.version,
776776
subscription,
777-
cart.uid
778-
);
777+
uid: cart.uid,
778+
paymentProvider:
779+
this.subscriptionManager.getPaymentProvider(subscription),
780+
});
779781
} else {
780782
const promises: Promise<any>[] = [
781783
this.finalizeCartWithError(cartId, CartErrorReasonId.Unknown),

‎libs/payments/cart/src/lib/checkout.service.spec.ts‎

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -411,17 +411,20 @@ describe('CheckoutService', () => {
411411
jest.spyOn(customerManager, 'setTaxId').mockResolvedValue();
412412
jest.spyOn(profileClient, 'deleteCache').mockResolvedValue('test');
413413
jest.spyOn(cartManager, 'finishCart').mockResolvedValue();
414+
jest.spyOn(statsd, 'increment');
414415
});
415416

416417
it('success', async () => {
417418
const mockCart = ResultCartFactory();
419+
const paymentProvider = 'stripe';
418420

419-
await checkoutService.postPaySteps(
420-
mockCart,
421-
mockCart.version,
422-
mockSubscription,
423-
mockUid
424-
);
421+
await checkoutService.postPaySteps({
422+
cart: mockCart,
423+
version: mockCart.version,
424+
subscription: mockSubscription,
425+
uid: mockUid,
426+
paymentProvider,
427+
});
425428

426429
expect(customerManager.setTaxId).toHaveBeenCalledWith(
427430
mockSubscription.customer,
@@ -430,6 +433,11 @@ describe('CheckoutService', () => {
430433

431434
expect(privateMethod).toHaveBeenCalled();
432435
expect(cartManager.finishCart).toHaveBeenCalled();
436+
expect(statsd.increment).toHaveBeenCalledWith('subscription_success', {
437+
payment_provider: paymentProvider,
438+
offering_id: mockCart.offeringConfigId,
439+
interval: mockCart.interval,
440+
});
433441
});
434442

435443
it('success - adds coupon code to subscription metadata if it exists', async () => {
@@ -444,17 +452,19 @@ describe('CheckoutService', () => {
444452
},
445453
})
446454
);
455+
const paymentProvider = 'stripe';
447456

448457
jest
449458
.spyOn(subscriptionManager, 'update')
450459
.mockResolvedValue(mockUpdatedSubscription);
451460

452-
await checkoutService.postPaySteps(
453-
mockCart,
454-
mockCart.version,
455-
mockSubscription,
456-
mockUid
457-
);
461+
await checkoutService.postPaySteps({
462+
cart: mockCart,
463+
version: mockCart.version,
464+
subscription: mockSubscription,
465+
uid: mockUid,
466+
paymentProvider,
467+
});
458468

459469
expect(customerManager.setTaxId).toHaveBeenCalledWith(
460470
mockSubscription.customer,
@@ -598,12 +608,13 @@ describe('CheckoutService', () => {
598608
});
599609

600610
it('calls postPaySteps', async () => {
601-
expect(checkoutService.postPaySteps).toHaveBeenCalledWith(
602-
mockCart,
603-
mockPrePayStepsResult.version + 1,
604-
mockSubscription,
605-
mockCart.uid
606-
);
611+
expect(checkoutService.postPaySteps).toHaveBeenCalledWith({
612+
cart: mockCart,
613+
version: mockPrePayStepsResult.version + 1,
614+
subscription: mockSubscription,
615+
uid: mockCart.uid,
616+
paymentProvider: 'stripe',
617+
});
607618
});
608619
});
609620

@@ -829,12 +840,13 @@ describe('CheckoutService', () => {
829840
);
830841
});
831842
it('calls postPaySteps', async () => {
832-
expect(checkoutService.postPaySteps).toHaveBeenCalledWith(
833-
mockCart,
834-
mockPrePayStepsResult.version + 1,
835-
mockSubscription,
836-
mockCart.uid
837-
);
843+
expect(checkoutService.postPaySteps).toHaveBeenCalledWith({
844+
cart: mockCart,
845+
version: mockPrePayStepsResult.version + 1,
846+
subscription: mockSubscription,
847+
uid: mockCart.uid,
848+
paymentProvider: 'paypal',
849+
});
838850
});
839851
});
840852
describe('uncollectible', () => {

‎libs/payments/cart/src/lib/checkout.service.ts‎

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -212,12 +212,14 @@ export class CheckoutService {
212212
};
213213
}
214214

215-
async postPaySteps(
216-
cart: ResultCart,
217-
version: number,
218-
subscription: StripeSubscription,
219-
uid: string
220-
) {
215+
async postPaySteps(args: {
216+
cart: ResultCart;
217+
version: number;
218+
subscription: StripeSubscription;
219+
uid: string;
220+
paymentProvider: 'stripe' | 'paypal';
221+
}) {
222+
const { cart, version, subscription, uid, paymentProvider } = args;
221223
const { customer: customerId, currency } = subscription;
222224

223225
await this.customerManager.setTaxId(customerId, currency);
@@ -235,8 +237,11 @@ export class CheckoutService {
235237
}
236238
await this.cartManager.finishCart(cart.id, version, {});
237239

238-
// TODO: call sendFinishSetupEmailForStubAccount
239-
console.log(cart.id, subscription.id);
240+
this.statsd.increment('subscription_success', {
241+
payment_provider: paymentProvider,
242+
offering_id: cart.offeringConfigId,
243+
interval: cart.interval,
244+
});
240245
}
241246

242247
async payWithStripe(
@@ -319,7 +324,13 @@ export class CheckoutService {
319324
{ cartId: cart.id }
320325
);
321326
}
322-
await this.postPaySteps(cart, updatedVersion, subscription, uid);
327+
await this.postPaySteps({
328+
cart,
329+
version: updatedVersion,
330+
subscription,
331+
uid,
332+
paymentProvider: 'stripe',
333+
});
323334
} else {
324335
throw new CheckoutPaymentError(
325336
`Expected payment intent status to be one of [requires_action, succeeded], instead found: ${paymentIntent.status}`
@@ -409,7 +420,13 @@ export class CheckoutService {
409420
latestInvoice
410421
);
411422
if (['paid', 'open'].includes(processedInvoice.status ?? '')) {
412-
await this.postPaySteps(cart, updatedVersion, subscription, uid);
423+
await this.postPaySteps({
424+
cart,
425+
version: updatedVersion,
426+
subscription,
427+
uid,
428+
paymentProvider: 'paypal',
429+
});
413430
} else {
414431
throw new CheckoutPaymentError(
415432
`Expected processed invoice status to be one of [paid, open], instead found: ${processedInvoice.status}`

‎libs/payments/content-server/src/lib/content-server.client.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,26 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

5-
import { Injectable } from '@nestjs/common';
5+
import { Inject, Injectable } from '@nestjs/common';
66
import { ContentServerClientConfig } from './content-server.config';
77
import { MetricsFlow } from './content-server.types';
8+
import {
9+
CaptureTimingWithStatsD,
10+
StatsDService,
11+
type StatsD,
12+
} from '@fxa/shared/metrics/statsd';
813

914
@Injectable()
1015
export class ContentServerClient {
1116
private readonly url: string;
12-
constructor(private contentServerClientConfig: ContentServerClientConfig) {
17+
constructor(
18+
private contentServerClientConfig: ContentServerClientConfig,
19+
@Inject(StatsDService) public statsd: StatsD
20+
) {
1321
this.url = this.contentServerClientConfig.url || 'http://localhost:3030';
1422
}
1523

24+
@CaptureTimingWithStatsD()
1625
public async getMetricsFlow() {
1726
const response = await fetch(`${this.url}/metrics-flow`, { method: 'GET' });
1827
if (!response.ok) {

‎libs/payments/customer/src/lib/customer.manager.spec.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,20 @@ import {
1414
} from '@fxa/payments/stripe';
1515
import { TaxAddressFactory } from './factories/tax-address.factory';
1616
import { CustomerManager } from './customer.manager';
17+
import { MockStatsDProvider } from '@fxa/shared/metrics/statsd';
1718

1819
describe('CustomerManager', () => {
1920
let customerManager: CustomerManager;
2021
let stripeClient: StripeClient;
2122

2223
beforeEach(async () => {
2324
const module = await Test.createTestingModule({
24-
providers: [MockStripeConfigProvider, StripeClient, CustomerManager],
25+
providers: [
26+
MockStripeConfigProvider,
27+
StripeClient,
28+
CustomerManager,
29+
MockStatsDProvider,
30+
],
2531
}).compile();
2632

2733
customerManager = module.get(CustomerManager);

0 commit comments

Comments
 (0)