feat(2737): re-order test cases. Move fee collector.

interledger · Aug 5, 2024 · a8b7932 · a8b7932
1 parent 4093058
commit a8b7932
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 110 deletions.
diff --git a/packages/backend/src/open_payments/payment/outgoing/lifecycle.ts b/packages/backend/src/open_payments/payment/outgoing/lifecycle.ts
@@ -94,6 +94,17 @@ export async function handleSending(
     deps.telemetry.recordHistogram('ilp_pay_time_ms', payDuration, {
       description: 'Time to complete an ILP payment'
     })
+
+    await deps.telemetry.incrementCounterWithTransactionFeeAmount(
+      'payment_fees',
+      payment.sentAmount,
+      payment.receiveAmount,
+      {
+        description: 'Amount sent through the network as fees',
+        valueType: ValueType.DOUBLE
+      },
+      false // do not preserve privacy
+    )
   }
 
   await handleCompleted(deps, payment)
@@ -149,19 +160,6 @@ async function handleCompleted(
     state: OutgoingPaymentState.Completed
   })
 
-  if (deps.telemetry) {
-    await deps.telemetry.incrementCounterWithTransactionFeeAmount(
-      'payment_fees',
-      payment.sentAmount,
-      payment.receiveAmount,
-      {
-        description: 'Amount sent through the network as fees',
-        valueType: ValueType.DOUBLE
-      },
-      false // do not preserve privacy
-    )
-  }
-
   await sendWebhookEvent(
     deps,
     payment,

diff --git a/packages/backend/src/telemetry/service.test.ts b/packages/backend/src/telemetry/service.test.ts
@@ -122,6 +122,96 @@ describe('TelemetryServiceImpl', () => {
     expect(histogram?.record).toHaveBeenCalledTimes(2)
   })
 
+  describe('incrementCounterWithTransactionFeeAmount', () => {
+    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.incrementCounterWithTransactionFeeAmount(
+        'test_fee_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.incrementCounterWithTransactionFeeAmount(
+        'test_fee_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.incrementCounterWithTransactionFeeAmount(
+        'test_fee_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')
+
+      await telemetryService.incrementCounterWithTransactionFeeAmount(
+        'test_fee_counter',
+        {
+          value: 100n,
+          assetCode: 'USD',
+          assetScale: 2
+        },
+        {
+          value: 50n,
+          assetCode: 'USD',
+          assetScale: 2
+        }
+      )
+
+      expect(spyConvert).toHaveBeenCalled()
+      expect(spyIncCounter).toHaveBeenCalled()
+    })
+  })
+
   describe('incrementCounterWithTransactionAmount', () => {
     it('should try to convert using aseRatesService and fallback to internalRatesService', async () => {
       const aseConvertSpy = jest
@@ -258,101 +348,4 @@ describe('TelemetryServiceImpl', () => {
       )
     })
   })
-
-  describe('incrementCounterWithTransactionFeeAmount', () => {
-    it('should not record fee when there is no fee value', async () => {
-      const aseConvertSpy = jest
-        .spyOn(aseRatesService, 'convert')
-        .mockImplementation(() =>
-          Promise.resolve(ConvertError.InvalidDestinationPrice)
-        )
-      const internalConvertSpy = jest.spyOn(internalRatesService, 'convert')
-
-      const spyIncCounter = jest.spyOn(telemetryService, 'incrementCounter')
-
-      await telemetryService.incrementCounterWithTransactionFeeAmount(
-        'test_fee_counter',
-        {
-          value: 100n,
-          assetCode: 'USD',
-          assetScale: 2
-        },
-        {
-          value: 100n,
-          assetCode: 'USD',
-          assetScale: 2
-        }
-      )
-
-      expect(spyIncCounter).not.toHaveBeenCalled()
-      expect(aseConvertSpy).toHaveBeenCalled()
-      expect(internalConvertSpy).toHaveBeenCalled()
-    })
-
-    it('should not record fee negative fee value', async () => {
-      const spyConvert = jest.spyOn(aseRatesService, 'convert')
-      const spyIncCounter = jest.spyOn(telemetryService, 'incrementCounter')
-
-      await telemetryService.incrementCounterWithTransactionFeeAmount(
-        'test_fee_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.incrementCounterWithTransactionFeeAmount(
-        'test_fee_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')
-
-      await telemetryService.incrementCounterWithTransactionFeeAmount(
-        'test_fee_counter',
-        {
-          value: 100n,
-          assetCode: 'USD',
-          assetScale: 2
-        },
-        {
-          value: 50n,
-          assetCode: 'USD',
-          assetScale: 2
-        }
-      )
-
-      expect(spyConvert).toHaveBeenCalled()
-      expect(spyIncCounter).toHaveBeenCalled()
-    })
-  })
 })