From eccf2dd6ac380b4711871c71d4e5b1b7050bab20 Mon Sep 17 00:00:00 2001
From: Max Kurapov <max@interledger.org>
Date: Mon, 2 Dec 2024 19:26:05 +0100
Subject: [PATCH] chore(backend): fix incrementCounterWithTransactionAmount
 function

---
 .../backend/src/telemetry/service.test.ts     | 59 ++++++++++++++++---
 packages/backend/src/telemetry/service.ts     | 11 ++--
 2 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/packages/backend/src/telemetry/service.test.ts b/packages/backend/src/telemetry/service.test.ts
index c8e59f025f..c599b2d335 100644
--- a/packages/backend/src/telemetry/service.test.ts
+++ b/packages/backend/src/telemetry/service.test.ts
@@ -13,6 +13,7 @@ import {
 import { Counter, Histogram } from '@opentelemetry/api'
 import { privacy } from './privacy'
 import { mockRatesApi } from '../tests/rates'
+import { ConvertResults } from '../rates/util'
 
 jest.mock('@opentelemetry/api', () => ({
   ...jest.requireActual('@opentelemetry/api'),
@@ -355,14 +356,17 @@ describe('Telemetry Service', () => {
         expect(internalConvertSpy).not.toHaveBeenCalled()
       })
 
-      it('should apply privacy', async () => {
+      it('should apply privacy by default', async () => {
         const convertedAmount = 500n
 
         jest
           //"any" to access private ts class member variable
           // eslint-disable-next-line @typescript-eslint/no-explicit-any
           .spyOn(telemetryService as any, 'convertAmount')
-          .mockImplementation(() => Promise.resolve(convertedAmount))
+          .mockResolvedValueOnce({
+            scaledExchangeRate: 1,
+            amount: convertedAmount
+          } as ConvertResults)
         const applyPrivacySpy = jest
           .spyOn(privacy, 'applyPrivacy')
           .mockReturnValue(123)
@@ -378,14 +382,49 @@ describe('Telemetry Service', () => {
             value: 100n,
             assetCode: 'USD',
             assetScale: 2
-          }
+          },
+          undefined
         )
 
         expect(applyPrivacySpy).toHaveBeenCalledWith(Number(convertedAmount))
+        expect(incrementCounterSpy).toHaveBeenCalledWith(counterName, 123, {})
+      })
+
+      it('should not apply privacy if preservePrivacy is false', async () => {
+        const convertedAmount = 500n
+
+        jest
+          //"any" to access private ts class member variable
+          // eslint-disable-next-line @typescript-eslint/no-explicit-any
+          .spyOn(telemetryService as any, 'convertAmount')
+          .mockResolvedValueOnce({
+            scaledExchangeRate: 1,
+            amount: convertedAmount
+          } as ConvertResults)
+
+        const applyPrivacySpy = jest.spyOn(privacy, 'applyPrivacy')
+        const incrementCounterSpy = jest.spyOn(
+          telemetryService,
+          'incrementCounter'
+        )
+
+        const counterName = 'test_counter'
+        await telemetryService.incrementCounterWithTransactionAmount(
+          counterName,
+          {
+            value: 100n,
+            assetCode: 'USD',
+            assetScale: 2
+          },
+          undefined,
+          false
+        )
+
+        expect(applyPrivacySpy).not.toHaveBeenCalled()
         expect(incrementCounterSpy).toHaveBeenCalledWith(
           counterName,
-          123,
-          expect.any(Object)
+          Number(convertedAmount),
+          {}
         )
       })
 
@@ -421,7 +460,10 @@ describe('Telemetry Service', () => {
           //"any" to access private ts class member variable
           // eslint-disable-next-line @typescript-eslint/no-explicit-any
           .spyOn(telemetryService as any, 'convertAmount')
-          .mockImplementation(() => Promise.resolve(10000n))
+          .mockResolvedValueOnce({
+            scaledExchangeRate: 1,
+            amount: 100n
+          } as ConvertResults)
         const incrementCounterSpy = jest.spyOn(
           telemetryService,
           'incrementCounter'
@@ -437,14 +479,15 @@ describe('Telemetry Service', () => {
             value: 100n,
             assetCode: 'USD',
             assetScale: 2
-          }
+          },
+          { attribute: 'metric attribute' }
         )
 
         expect(convertSpy).toHaveBeenCalled()
         expect(incrementCounterSpy).toHaveBeenCalledWith(
           counterName,
           obfuscatedAmount,
-          expect.any(Object)
+          { attribute: 'metric attribute' }
         )
       })
     })
diff --git a/packages/backend/src/telemetry/service.ts b/packages/backend/src/telemetry/service.ts
index 30426df973..0075845ec4 100644
--- a/packages/backend/src/telemetry/service.ts
+++ b/packages/backend/src/telemetry/service.ts
@@ -173,14 +173,13 @@ export class TelemetryServiceImpl implements TelemetryService {
         return
       }
 
-      const obfuscatedAmount = preservePrivacy
-        ? privacy.applyPrivacy(Number(converted))
-        : Number(converted)
-      this.incrementCounter(name, obfuscatedAmount, attributes)
+      const finalAmount = preservePrivacy
+        ? privacy.applyPrivacy(Number(converted.amount))
+        : Number(converted.amount)
+      this.incrementCounter(name, finalAmount, attributes)
     } catch (e) {
-      this.deps.logger.error(e, `Unable to collect telemetry`)
+      this.deps.logger.error(e, 'Unable to collect telemetry')
     }
-    return Promise.resolve()
   }
 
   public recordHistogram(