feat(2737): add fees as metric for outgoing payment.

packages/backend/src/open_payments/payment/outgoing/lifecycle.ts

@@ -8,6 +8,7 @@ import {
 import { ServiceDependencies } from './service'
 import { Receiver } from '../../receiver/model'
 import { TransactionOrKnex } from 'objection'
+import { ValueType } from '@opentelemetry/api'
 
 // "payment" is locked by the "deps.knex" transaction.
 export async function handleSending(
@@ -85,14 +86,24 @@ export async function handleSending(
   const payEndTime = Date.now()
 
   if (deps.telemetry) {
-    deps.telemetry.incrementCounter('transactions_total', 1, {
-      description: 'Count of funded transactions'
-    })
-
     const payDuration = payEndTime - payStartTime
-    deps.telemetry.recordHistogram('ilp_pay_time_ms', payDuration, {
-      description: 'Time to complete an ILP payment'
-    })
+    await Promise.all([
+      deps.telemetry.incrementCounter('transactions_total', 1, {
+        description: 'Count of funded transactions'
+      }),
+      deps.telemetry.recordHistogram('ilp_pay_time_ms', payDuration, {
+        description: 'Time to complete an ILP payment'
+      }),
+      deps.telemetry.incrementCounterWithTransactionAmountDifference(
+        'transaction_fee_amounts',
+        payment.sentAmount,
+        payment.receiveAmount,
+        {
+          description: 'Amount sent through the network as fees',
+          valueType: ValueType.DOUBLE
+        }
+      )
+    ])
   }
 
   await handleCompleted(deps, payment)
@@ -147,6 +158,7 @@ async function handleCompleted(
   await payment.$query(deps.knex).patch({
     state: OutgoingPaymentState.Completed
   })
+
   await sendWebhookEvent(
     deps,
     payment,

packages/backend/src/open_payments/payment/outgoing/service.test.ts

@@ -50,6 +50,7 @@ import { mockRatesApi } from '../../../tests/rates'
 import { UnionOmit } from '../../../shared/utils'
 import { QuoteError } from '../../quote/errors'
 import { withConfigOverride } from '../../../tests/helpers'
+import { TelemetryService } from '../../../telemetry/service'
 
 describe('OutgoingPaymentService', (): void => {
   let deps: IocContract<AppServices>
@@ -58,6 +59,7 @@ describe('OutgoingPaymentService', (): void => {
   let accountingService: AccountingService
   let paymentMethodHandlerService: PaymentMethodHandlerService
   let quoteService: QuoteService
+  let telemetryService: TelemetryService
   let knex: Knex
   let walletAddressId: string
   let incomingPayment: IncomingPayment
@@ -243,12 +245,17 @@ describe('OutgoingPaymentService', (): void => {
       XRP: exchangeRate
     }))
 
-    deps = await initIocContainer({ ...Config, exchangeRatesUrl })
+    deps = await initIocContainer({
+      ...Config,
+      exchangeRatesUrl,
+      enableTelemetry: true
+    })
     appContainer = await createTestApp(deps)
     outgoingPaymentService = await deps.use('outgoingPaymentService')
     accountingService = await deps.use('accountingService')
     paymentMethodHandlerService = await deps.use('paymentMethodHandlerService')
     quoteService = await deps.use('quoteService')
+    telemetryService = (await deps.use('telemetry'))!
     config = await deps.use('config')
     knex = appContainer.knex
   })
@@ -1154,11 +1161,58 @@ describe('OutgoingPaymentService', (): void => {
       return payment
     }
 
+    test('COMPLETED with Telemetry Fee Counter (receiveAmount < incomingPayment.incomingAmount)', async (): Promise<void> => {
+      const spyTelFeeAmount = jest.spyOn(
+        telemetryService,
+        'incrementCounterWithTransactionAmountDifference'
+      )
+      const spyCounter = jest.spyOn(telemetryService, 'incrementCounter')
+
+      incomingPayment = await createIncomingPayment(deps, {
+        walletAddressId: receiverWalletAddress.id,
+        incomingAmount: {
+          value: receiveAmount.value * 2n,
+          assetCode: receiverWalletAddress.asset.code,
+          assetScale: receiverWalletAddress.asset.scale
+        }
+      })
+      assert.ok(incomingPayment.walletAddress)
+
+      const createdPayment = await setup({
+        receiver: incomingPayment.getUrl(incomingPayment.walletAddress),
+        receiveAmount,
+        method: 'ilp'
+      })
+
+      mockPaymentHandlerPay(createdPayment, incomingPayment)
+      const payment = await processNext(
+        createdPayment.id,
+        OutgoingPaymentState.Completed
+      )
+      await expectOutcome(payment, {
+        accountBalance: 0n,
+        amountSent: payment.debitAmount.value,
+        amountDelivered: payment.receiveAmount.value,
+        incomingPaymentReceived: payment.receiveAmount.value,
+        withdrawAmount: 0n
+      })
+      // [incrementCounterWithTransactionAmountDifference] called and [incrementCounter] only once due to [Count of funded transactions]
+      expect(spyTelFeeAmount).toHaveBeenCalledTimes(1)
+      expect(spyCounter).toHaveBeenCalledTimes(1)
+      expect(spyCounter).toHaveBeenCalledTimes(1)
+    })
+
     test.each`
       debitAmount    | receiveAmount
       ${debitAmount} | ${undefined}
       ${undefined}   | ${receiveAmount}
     `('COMPLETED', async ({ debitAmount, receiveAmount }): Promise<void> => {
+      const spyTelFeeAmount = jest.spyOn(
+        telemetryService,
+        'incrementCounterWithTransactionAmountDifference'
+      )
+      const spyCounter = jest.spyOn(telemetryService, 'incrementCounter')
+
       const createdPayment = await setup({
         receiver,
         debitAmount,
@@ -1179,6 +1233,8 @@ describe('OutgoingPaymentService', (): void => {
         incomingPaymentReceived: payment.receiveAmount.value,
         withdrawAmount: 0n
       })
+      expect(spyTelFeeAmount).toHaveBeenCalledTimes(1)
+      expect(spyCounter).toHaveBeenCalledTimes(1)
     })
 
     test('COMPLETED (receiveAmount < incomingPayment.incomingAmount)', async (): Promise<void> => {

packages/backend/src/open_payments/payment/outgoing/service.ts

@@ -237,9 +237,7 @@ async function createOutgoingPayment(
       const peer = await deps.peerService.getByDestinationAddress(
         receiver.ilpAddress
       )
-      if (peer) {
-        await payment.$query(trx).patch({ peerId: peer.id })
-      }
+      if (peer) await payment.$query(trx).patch({ peerId: peer.id })
 
       await sendWebhookEvent(
         deps,

packages/backend/src/telemetry/service.test.ts

@@ -8,6 +8,7 @@ import { mockCounter, mockHistogram } from '../tests/telemetry'
 import { TelemetryService } from './service'
 import { Counter, Histogram } from '@opentelemetry/api'
 import { privacy } from './privacy'
+import { mockRatesApi } from '../tests/rates'
 
 jest.mock('@opentelemetry/api', () => ({
   ...jest.requireActual('@opentelemetry/api'),
@@ -36,6 +37,18 @@ describe('TelemetryServiceImpl', () => {
   let aseRatesService: RatesService
   let internalRatesService: RatesService
 
+  let apiRequestCount = 0
+  const exchangeRatesUrl = 'http://example-rates.com'
+
+  const exampleRates = {
+    USD: {
+      EUR: 2
+    },
+    EUR: {
+      USD: 1.12
+    }
+  }
+
   beforeAll(async (): Promise<void> => {
     deps = initIocContainer({
       ...Config,
@@ -49,6 +62,11 @@ describe('TelemetryServiceImpl', () => {
     telemetryService = await deps.use('telemetry')!
     aseRatesService = await deps.use('ratesService')!
     internalRatesService = await deps.use('internalRatesService')!
+
+    mockRatesApi(exchangeRatesUrl, (base) => {
+      apiRequestCount++
+      return exampleRates[base as keyof typeof exampleRates]
+    })
   })
 
   afterAll(async (): Promise<void> => {
@@ -122,6 +140,162 @@ describe('TelemetryServiceImpl', () => {
     expect(histogram?.record).toHaveBeenCalledTimes(2)
   })
 
+  describe('incrementCounterWithTransactionAmountDifference', () => {
+    it('should not record fee when there is no fee value', async () => {
+      const spyAseConvert = jest.spyOn(aseRatesService, 'convert')
+      const spyIncCounter = jest.spyOn(telemetryService, 'incrementCounter')
+
+      await telemetryService.incrementCounterWithTransactionAmountDifference(
+        'test_amount_diff_counter',
+        {
+          value: 100n,
+          assetCode: 'USD',
+          assetScale: 2
+        },
+        {
+          value: 100n,
+          assetCode: 'USD',
+          assetScale: 2
+        }
+      )
+
+      expect(spyAseConvert).toHaveBeenCalled()
+      expect(spyIncCounter).not.toHaveBeenCalled()
+    })
+
+    it('should not record fee negative fee value', async () => {
+      const spyConvert = jest.spyOn(aseRatesService, 'convert')
+      const spyIncCounter = jest.spyOn(telemetryService, 'incrementCounter')
+
+      await telemetryService.incrementCounterWithTransactionAmountDifference(
+        'test_amount_diff_counter',
+        {
+          value: 100n,
+          assetCode: 'USD',
+          assetScale: 2
+        },
+        {
+          value: 101n,
+          assetCode: 'USD',
+          assetScale: 2
+        }
+      )
+
+      expect(spyConvert).toHaveBeenCalled()
+      expect(spyIncCounter).not.toHaveBeenCalled()
+    })
+
+    it('should not record zero amounts', async () => {
+      const spyConvert = jest.spyOn(aseRatesService, 'convert')
+      const spyIncCounter = jest.spyOn(telemetryService, 'incrementCounter')
+
+      await telemetryService.incrementCounterWithTransactionAmountDifference(
+        'test_amount_diff_counter',
+        {
+          value: 0n,
+          assetCode: 'USD',
+          assetScale: 2
+        },
+        {
+          value: 0n,
+          assetCode: 'USD',
+          assetScale: 2
+        }
+      )
+
+      expect(spyConvert).not.toHaveBeenCalled()
+      expect(spyIncCounter).not.toHaveBeenCalled()
+    })
+
+    it('should record since it is a valid fee', async () => {
+      const spyConvert = jest.spyOn(aseRatesService, 'convert')
+      const spyIncCounter = jest.spyOn(telemetryService, 'incrementCounter')
+
+      const source = {
+        value: 100n,
+        assetCode: 'USD',
+        assetScale: 2
+      }
+      const destination = {
+        value: 50n,
+        assetCode: 'USD',
+        assetScale: 2
+      }
+
+      const name = 'test_amount_diff_counter'
+      await telemetryService.incrementCounterWithTransactionAmountDifference(
+        name,
+        source,
+        destination
+      )
+
+      expect(spyConvert).toHaveBeenCalledTimes(2)
+      expect(spyConvert).toHaveBeenNthCalledWith(
+        1,
+        expect.objectContaining({
+          sourceAmount: source.value,
+          sourceAsset: { code: source.assetCode, scale: source.assetScale }
+        })
+      )
+      expect(spyConvert).toHaveBeenNthCalledWith(
+        2,
+        expect.objectContaining({
+          sourceAmount: destination.value,
+          sourceAsset: {
+            code: destination.assetCode,
+            scale: destination.assetScale
+          }
+        })
+      )
+      // Ensure the [incrementCounter] was called with the correct calculated value. Expected 5000 due to scale = 4.
+      expect(spyIncCounter).toHaveBeenCalledWith(name, 5000, {})
+    })
+
+    it('should record since it is a valid fee for different assets', async () => {
+      const spyConvert = jest.spyOn(aseRatesService, 'convert')
+      const spyIncCounter = jest.spyOn(telemetryService, 'incrementCounter')
+
+      const source = {
+        value: 100n,
+        assetCode: 'USD',
+        assetScale: 2
+      }
+      const destination = {
+        value: 50n,
+        assetCode: 'EUR',
+        assetScale: 2
+      }
+
+      const name = 'test_amount_diff_counter'
+      await telemetryService.incrementCounterWithTransactionAmountDifference(
+        name,
+        source,
+        destination
+      )
+
+      expect(spyConvert).toHaveBeenCalledTimes(2)
+      expect(spyConvert).toHaveBeenNthCalledWith(
+        1,
+        expect.objectContaining({
+          sourceAmount: source.value,
+          sourceAsset: { code: source.assetCode, scale: source.assetScale }
+        })
+      )
+      expect(spyConvert).toHaveBeenNthCalledWith(
+        2,
+        expect.objectContaining({
+          sourceAmount: destination.value,
+          sourceAsset: {
+            code: destination.assetCode,
+            scale: destination.assetScale
+          }
+        })
+      )
+      expect(spyIncCounter).toHaveBeenCalledWith(name, 4400, {})
+      expect(apiRequestCount).toBe(1)
+    })
+  })
+
   describe('incrementCounterWithTransactionAmount', () => {
     it('should try to convert using aseRatesService and fallback to internalRatesService', async () => {
       const aseConvertSpy = jest