diff --git a/localenv/telemetry/otel-collector-config.yaml b/localenv/telemetry/otel-collector-config.yaml index 32e08f8902..d577a0ec78 100644 --- a/localenv/telemetry/otel-collector-config.yaml +++ b/localenv/telemetry/otel-collector-config.yaml @@ -8,6 +8,8 @@ processors: batch: exporters: + logging: + loglevel: info debug: verbosity: detailed prometheus: @@ -18,6 +20,9 @@ exporters: insecure: true service: + telemetry: + logs: + level: warn pipelines: metrics: receivers: [otlp] diff --git a/packages/backend/src/app.ts b/packages/backend/src/app.ts index c56544f38f..d0cdd19f35 100644 --- a/packages/backend/src/app.ts +++ b/packages/backend/src/app.ts @@ -215,8 +215,8 @@ const WALLET_ADDRESS_PATH = '/:walletAddressPath+' export interface AppServices { logger: Promise - telemetry?: Promise - internalRatesService?: Promise + telemetry: Promise + internalRatesService: Promise knex: Promise axios: Promise config: Promise diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index a34d784ef7..dfa64c0ef4 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -47,14 +47,14 @@ import { } from './payment-method/ilp/ilp_plugin' import { createHttpTokenService } from './payment-method/ilp/peer-http-token/service' import { createPeerService } from './payment-method/ilp/peer/service' -import { - createIlpPaymentService, - ServiceDependencies as IlpPaymentServiceDependencies -} from './payment-method/ilp/service' +import { createIlpPaymentService } from './payment-method/ilp/service' import { createSPSPRoutes } from './payment-method/ilp/spsp/routes' import { createStreamCredentialsService } from './payment-method/ilp/stream-credentials/service' import { createRatesService } from './rates/service' -import { TelemetryService, createTelemetryService } from './telemetry/service' +import { + createTelemetryService, + createNoopTelemetryService +} from './telemetry/service' import { createWebhookService } from './webhook/service' BigInt.prototype.toJSON = function () { @@ -135,29 +135,32 @@ export function initIocContainer( }) }) - if (config.enableTelemetry) { - container.singleton('internalRatesService', async (deps) => { - return createRatesService({ - logger: await deps.use('logger'), - exchangeRatesUrl: config.telemetryExchangeRatesUrl, - exchangeRatesLifetime: config.telemetryExchangeRatesLifetime - }) + container.singleton('internalRatesService', async (deps) => { + return createRatesService({ + logger: await deps.use('logger'), + exchangeRatesUrl: config.telemetryExchangeRatesUrl, + exchangeRatesLifetime: config.telemetryExchangeRatesLifetime }) + }) - container.singleton('telemetry', async (deps) => { - const config = await deps.use('config') - return createTelemetryService({ - logger: await deps.use('logger'), - aseRatesService: await deps.use('ratesService'), - internalRatesService: await deps.use('internalRatesService')!, - instanceName: config.instanceName, - collectorUrls: config.openTelemetryCollectors, - exportIntervalMillis: config.openTelemetryExportInterval, - baseAssetCode: 'USD', - baseScale: 4 - }) + container.singleton('telemetry', async (deps) => { + const config = await deps.use('config') + + if (!config.enableTelemetry) { + return createNoopTelemetryService() + } + + return createTelemetryService({ + logger: await deps.use('logger'), + aseRatesService: await deps.use('ratesService'), + internalRatesService: await deps.use('internalRatesService')!, + instanceName: config.instanceName, + collectorUrls: config.openTelemetryCollectors, + exportIntervalMillis: config.openTelemetryExportInterval, + baseAssetCode: 'USD', + baseScale: 4 }) - } + }) container.singleton('openApi', async () => { const resourceServerSpec = await getResourceServerOpenAPI() @@ -356,10 +359,6 @@ export function initIocContainer( container.singleton('connectorApp', async (deps) => { const config = await deps.use('config') - let telemetry: TelemetryService | undefined - if (config.enableTelemetry) { - telemetry = await deps.use('telemetry') - } return await createConnectorService({ logger: await deps.use('logger'), redis: await deps.use('redis'), @@ -370,7 +369,7 @@ export function initIocContainer( ratesService: await deps.use('ratesService'), streamServer: await deps.use('streamServer'), ilpAddress: config.ilpAddress, - telemetry + telemetry: await deps.use('telemetry') }) }) @@ -425,19 +424,14 @@ export function initIocContainer( }) container.singleton('ilpPaymentService', async (deps) => { - const serviceDependencies: IlpPaymentServiceDependencies = { + return await createIlpPaymentService({ logger: await deps.use('logger'), knex: await deps.use('knex'), config: await deps.use('config'), makeIlpPlugin: await deps.use('makeIlpPlugin'), - ratesService: await deps.use('ratesService') - } - - if (config.enableTelemetry) { - serviceDependencies.telemetry = await deps.use('telemetry') - } - - return createIlpPaymentService(serviceDependencies) + ratesService: await deps.use('ratesService'), + telemetry: await deps.use('telemetry') + }) }) container.singleton('paymentMethodHandlerService', async (deps) => { @@ -469,7 +463,6 @@ export function initIocContainer( }) container.singleton('outgoingPaymentService', async (deps) => { - const config = await deps.use('config') return await createOutgoingPaymentService({ logger: await deps.use('logger'), knex: await deps.use('knex'), @@ -481,9 +474,7 @@ export function initIocContainer( peerService: await deps.use('peerService'), walletAddressService: await deps.use('walletAddressService'), quoteService: await deps.use('quoteService'), - telemetry: config.enableTelemetry - ? await deps.use('telemetry') - : undefined + telemetry: await deps.use('telemetry') }) }) @@ -548,10 +539,8 @@ export const gracefulShutdown = async ( await redis.quit() redis.disconnect() - if (config.enableTelemetry) { - const telemetry = await container.use('telemetry') - telemetry?.shutdown() - } + const telemetry = await container.use('telemetry') + telemetry.shutdown() } export const start = async ( diff --git a/packages/backend/src/open_payments/payment/outgoing/lifecycle.ts b/packages/backend/src/open_payments/payment/outgoing/lifecycle.ts index f273e008a1..1b402db2d3 100644 --- a/packages/backend/src/open_payments/payment/outgoing/lifecycle.ts +++ b/packages/backend/src/open_payments/payment/outgoing/lifecycle.ts @@ -85,26 +85,24 @@ export async function handleSending( }) const payEndTime = Date.now() - if (deps.telemetry) { - const payDuration = payEndTime - payStartTime - 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 - } - ) - ]) - } + const payDuration = payEndTime - payStartTime + 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) } diff --git a/packages/backend/src/open_payments/payment/outgoing/service.ts b/packages/backend/src/open_payments/payment/outgoing/service.ts index 050700aca1..336444e518 100644 --- a/packages/backend/src/open_payments/payment/outgoing/service.ts +++ b/packages/backend/src/open_payments/payment/outgoing/service.ts @@ -66,7 +66,7 @@ export interface ServiceDependencies extends BaseService { paymentMethodHandlerService: PaymentMethodHandlerService walletAddressService: WalletAddressService quoteService: QuoteService - telemetry?: TelemetryService + telemetry: TelemetryService } export async function createOutgoingPaymentService( diff --git a/packages/backend/src/payment-method/ilp/connector/core/rafiki.ts b/packages/backend/src/payment-method/ilp/connector/core/rafiki.ts index a6095fb064..6740d21886 100644 --- a/packages/backend/src/payment-method/ilp/connector/core/rafiki.ts +++ b/packages/backend/src/payment-method/ilp/connector/core/rafiki.ts @@ -63,7 +63,7 @@ export interface TransferOptions { export interface RafikiServices { //router: Router accounting: AccountingService - telemetry?: TelemetryService + telemetry: TelemetryService walletAddresses: WalletAddressService logger: Logger incomingPayments: IncomingPaymentService @@ -143,7 +143,7 @@ export class Rafiki { get walletAddresses(): WalletAddressService { return config.walletAddresses }, - get telemetry(): TelemetryService | undefined { + get telemetry(): TelemetryService { return config.telemetry }, logger @@ -162,9 +162,7 @@ export class Rafiki { const response = new IlpResponse() const telemetry = this.publicServer.context.services.telemetry - if (telemetry) { - incrementPreparePacketCount(unfulfillable, prepare.amount, telemetry) - } + incrementPreparePacketCount(unfulfillable, prepare.amount, telemetry) await this.routes( { @@ -194,23 +192,21 @@ export class Rafiki { ) if (!response.rawReply) throw new Error('error generating reply') - if (telemetry) { - const { code, scale } = sourceAccount.asset - incrementFulfillOrRejectPacketCount( - unfulfillable, - prepare.amount, - response, - telemetry - ) - await incrementAmount( - unfulfillable, - prepare.amount, - response, - code, - scale, - telemetry - ) - } + const { code, scale } = sourceAccount.asset + incrementFulfillOrRejectPacketCount( + unfulfillable, + prepare.amount, + response, + telemetry + ) + await incrementAmount( + unfulfillable, + prepare.amount, + response, + code, + scale, + telemetry + ) return response.rawReply } diff --git a/packages/backend/src/payment-method/ilp/connector/index.ts b/packages/backend/src/payment-method/ilp/connector/index.ts index 9f3ecc3f95..8dd2f27f0d 100644 --- a/packages/backend/src/payment-method/ilp/connector/index.ts +++ b/packages/backend/src/payment-method/ilp/connector/index.ts @@ -38,7 +38,7 @@ interface ServiceDependencies extends BaseService { peerService: PeerService streamServer: StreamServer ilpAddress: string - telemetry?: TelemetryService + telemetry: TelemetryService } export async function createConnectorService({ diff --git a/packages/backend/src/payment-method/ilp/service.ts b/packages/backend/src/payment-method/ilp/service.ts index 4974ffab24..b21c80db32 100644 --- a/packages/backend/src/payment-method/ilp/service.ts +++ b/packages/backend/src/payment-method/ilp/service.ts @@ -22,7 +22,7 @@ export interface ServiceDependencies extends BaseService { config: IAppConfig ratesService: RatesService makeIlpPlugin: (options: IlpPluginOptions) => IlpPlugin - telemetry?: TelemetryService + telemetry: TelemetryService } export async function createIlpPaymentService( @@ -94,16 +94,14 @@ async function getQuote( } const payEndTime = Date.now() - if (deps.telemetry) { - const rateProbeDuraiton = payEndTime - rateProbeStartTime - deps.telemetry.recordHistogram( - 'ilp_rate_probe_time_ms', - rateProbeDuraiton, - { - description: 'Time to get an ILP quote' - } - ) - } + const rateProbeDuraiton = payEndTime - rateProbeStartTime + deps.telemetry.recordHistogram( + 'ilp_rate_probe_time_ms', + rateProbeDuraiton, + { + description: 'Time to get an ILP quote' + } + ) // Pay.startQuote should return PaymentError.InvalidSourceAmount or // PaymentError.InvalidDestinationAmount for non-positive amounts. // Outgoing payments' sendAmount or receiveAmount should never be diff --git a/packages/backend/src/telemetry/service.test.ts b/packages/backend/src/telemetry/service.test.ts index 3834212f01..da0ae289cb 100644 --- a/packages/backend/src/telemetry/service.test.ts +++ b/packages/backend/src/telemetry/service.test.ts @@ -5,7 +5,11 @@ import { Config } from '../config/app' import { ConvertError, RatesService } from '../rates/service' import { TestContainer, createTestApp } from '../tests/app' import { mockCounter, mockHistogram } from '../tests/telemetry' -import { TelemetryService } from './service' +import { + NoopTelemetryServiceImpl, + TelemetryService, + TelemetryServiceImpl +} from './service' import { Counter, Histogram } from '@opentelemetry/api' import { privacy } from './privacy' import { mockRatesApi } from '../tests/rates' @@ -30,406 +34,442 @@ jest.mock('@opentelemetry/sdk-metrics', () => ({ })) })) -describe('TelemetryServiceImpl', () => { - let deps: IocContract - let appContainer: TestContainer - let telemetryService: TelemetryService - let aseRatesService: RatesService - let internalRatesService: RatesService - - let apiRequestCount = 0 - const exchangeRatesUrl = 'http://example-rates.com' - - const exampleRates = { - USD: { - EUR: 2 - }, - EUR: { - USD: 1.12 +describe('Telemetry Service', () => { + describe('Telemtry Enabled', () => { + let deps: IocContract + let appContainer: TestContainer + let telemetryService: TelemetryService + 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 => { - deps = initIocContainer({ - ...Config, - enableTelemetry: true, - telemetryExchangeRatesUrl: 'http://example-rates.com', - telemetryExchangeRatesLifetime: 100, - openTelemetryCollectors: [] + beforeAll(async (): Promise => { + deps = initIocContainer({ + ...Config, + enableTelemetry: true, + telemetryExchangeRatesUrl: 'http://example-rates.com', + telemetryExchangeRatesLifetime: 100, + openTelemetryCollectors: [] + }) + + appContainer = await createTestApp(deps) + 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] + }) }) - appContainer = await createTestApp(deps) - telemetryService = await deps.use('telemetry')! - aseRatesService = await deps.use('ratesService')! - internalRatesService = await deps.use('internalRatesService')! + afterAll(async (): Promise => { + await appContainer.shutdown() + }) - mockRatesApi(exchangeRatesUrl, (base) => { - apiRequestCount++ - return exampleRates[base as keyof typeof exampleRates] + test('telemetryService instance should be real implementation', () => { + expect(telemetryService instanceof TelemetryServiceImpl).toBe(true) }) - }) - afterAll(async (): Promise => { - await appContainer.shutdown() - }) + it('should create a counter with source attribute for a new metric', () => { + const name = 'test_counter' + const amount = 1 + const attributes = { test: 'attribute' } - it('should create a counter with source attribute for a new metric', () => { - const name = 'test_counter' - const amount = 1 - const attributes = { test: 'attribute' } + telemetryService.incrementCounter(name, amount, attributes) - telemetryService.incrementCounter(name, amount, attributes) + expect(mockCounter.add).toHaveBeenCalledWith( + amount, + expect.objectContaining({ + ...attributes, + source: expect.any(String) + }) + ) + }) - expect(mockCounter.add).toHaveBeenCalledWith( - amount, - expect.objectContaining({ - ...attributes, - source: expect.any(String) - }) - ) - }) + it('should create a histogram with source attribute for a new metric', () => { + const name = 'test_histogram' + const amount = 1 + const attributes = { test: 'attribute' } - it('should create a histogram with source attribute for a new metric', () => { - const name = 'test_histogram' - const amount = 1 - const attributes = { test: 'attribute' } + telemetryService.recordHistogram(name, amount, attributes) - telemetryService.recordHistogram(name, amount, attributes) + expect(mockHistogram.record).toHaveBeenCalledWith( + amount, + expect.objectContaining({ + ...attributes, + source: expect.any(String) + }) + ) + }) - expect(mockHistogram.record).toHaveBeenCalledWith( - amount, - expect.objectContaining({ - ...attributes, - source: expect.any(String) - }) - ) - }) + it('should use existing counter when incrementCounter is called for an existing metric', () => { + const name = 'test_counter' - it('should use existing counter when incrementCounter is called for an existing metric', () => { - const name = 'test_counter' + telemetryService.incrementCounter(name, 1) + telemetryService.incrementCounter(name, 1) - telemetryService.incrementCounter(name, 1) - telemetryService.incrementCounter(name, 1) + //"any" to access private ts class member variable + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const counters: Map = (telemetryService as any).counters - //"any" to access private ts class member variable - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const counters: Map = (telemetryService as any).counters + expect(counters.size).toBe(1) + expect(counters.has(name)).toBe(true) - expect(counters.size).toBe(1) - expect(counters.has(name)).toBe(true) + const counter = counters.get(name) + expect(counter?.add).toHaveBeenCalledTimes(2) + }) - const counter = counters.get(name) - expect(counter?.add).toHaveBeenCalledTimes(2) - }) + it('should use existing histogram when recordHistogram is called for an existing metric', () => { + const name = 'test_histogram' - it('should use existing histogram when recordHistogram is called for an existing metric', () => { - const name = 'test_histogram' + telemetryService.recordHistogram(name, 1) + telemetryService.recordHistogram(name, 1) - telemetryService.recordHistogram(name, 1) - telemetryService.recordHistogram(name, 1) + //"any" to access private ts class member variable + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const histograms: Map = (telemetryService as any) + .histograms - //"any" to access private ts class member variable - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const histograms: Map = (telemetryService as any) - .histograms + expect(histograms.size).toBe(1) + expect(histograms.has(name)).toBe(true) - expect(histograms.size).toBe(1) - expect(histograms.has(name)).toBe(true) + const histogram = histograms.get(name) + expect(histogram?.record).toHaveBeenCalledTimes(2) + }) - const histogram = histograms.get(name) - 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 + } + ) - 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') + expect(spyAseConvert).toHaveBeenCalled() + expect(spyIncCounter).not.toHaveBeenCalled() + }) - await telemetryService.incrementCounterWithTransactionAmountDifference( - 'test_amount_diff_counter', - { - value: 100n, - assetCode: 'USD', - assetScale: 2 - }, - { - value: 100n, - assetCode: 'USD', - assetScale: 2 - } - ) + 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(spyAseConvert).toHaveBeenCalled() - expect(spyIncCounter).not.toHaveBeenCalled() - }) + 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 not record fee negative fee value', async () => { - const spyConvert = jest.spyOn(aseRatesService, 'convert') - const spyIncCounter = jest.spyOn(telemetryService, 'incrementCounter') + it('should record since it is a valid fee', async () => { + const spyConvert = jest.spyOn(aseRatesService, 'convert') + const spyIncCounter = jest.spyOn(telemetryService, 'incrementCounter') - await telemetryService.incrementCounterWithTransactionAmountDifference( - 'test_amount_diff_counter', - { + const source = { value: 100n, assetCode: 'USD', assetScale: 2 - }, - { - value: 101n, + } + const destination = { + value: 50n, assetCode: 'USD', assetScale: 2 } - ) - expect(spyConvert).toHaveBeenCalled() - expect(spyIncCounter).not.toHaveBeenCalled() - }) + const name = 'test_amount_diff_counter' + await telemetryService.incrementCounterWithTransactionAmountDifference( + name, + source, + destination + ) - it('should not record zero amounts', async () => { - const spyConvert = jest.spyOn(aseRatesService, 'convert') - const spyIncCounter = jest.spyOn(telemetryService, 'incrementCounter') + 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, {}) + }) - await telemetryService.incrementCounterWithTransactionAmountDifference( - 'test_amount_diff_counter', - { - value: 0n, + 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 - }, - { - value: 0n, - assetCode: 'USD', + } + const destination = { + value: 50n, + assetCode: 'EUR', 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 - ) + 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, {}) + 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) + }) }) - 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 - } + describe('incrementCounterWithTransactionAmount', () => { + it('should try to convert using aseRatesService and fallback to internalRatesService', async () => { + const aseConvertSpy = jest + .spyOn(aseRatesService, 'convert') + .mockImplementation(() => + Promise.resolve(ConvertError.InvalidDestinationPrice) + ) + const internalConvertSpy = jest + .spyOn(internalRatesService, 'convert') + .mockImplementation(() => Promise.resolve(10000n)) + + await telemetryService.incrementCounterWithTransactionAmount( + 'test_counter', + { + value: 100n, + assetCode: 'USD', + assetScale: 2 + } + ) - const name = 'test_amount_diff_counter' - await telemetryService.incrementCounterWithTransactionAmountDifference( - name, - source, - destination - ) + expect(aseConvertSpy).toHaveBeenCalled() + expect(internalConvertSpy).toHaveBeenCalled() + }) - 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 + it('should not call the fallback internalRatesService if aseRatesService call is successful', async () => { + const aseConvertSpy = jest + .spyOn(aseRatesService, 'convert') + .mockImplementation(() => Promise.resolve(500n)) + const internalConvertSpy = jest.spyOn(internalRatesService, 'convert') + + await telemetryService.incrementCounterWithTransactionAmount( + 'test_counter', + { + value: 100n, + assetCode: 'USD', + assetScale: 2 } - }) - ) - 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 - .spyOn(aseRatesService, 'convert') - .mockImplementation(() => - Promise.resolve(ConvertError.InvalidDestinationPrice) ) - const internalConvertSpy = jest - .spyOn(internalRatesService, 'convert') - .mockImplementation(() => Promise.resolve(10000n)) - await telemetryService.incrementCounterWithTransactionAmount( - 'test_counter', - { - value: 100n, - assetCode: 'USD', - assetScale: 2 - } - ) - - expect(aseConvertSpy).toHaveBeenCalled() - expect(internalConvertSpy).toHaveBeenCalled() - }) + expect(aseConvertSpy).toHaveBeenCalled() + expect(internalConvertSpy).not.toHaveBeenCalled() + }) - it('should not call the fallback internalRatesService if aseRatesService call is successful', async () => { - const aseConvertSpy = jest - .spyOn(aseRatesService, 'convert') - .mockImplementation(() => Promise.resolve(500n)) - const internalConvertSpy = jest.spyOn(internalRatesService, 'convert') + it('should apply privacy', 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)) + const applyPrivacySpy = jest + .spyOn(privacy, 'applyPrivacy') + .mockReturnValue(123) + const incrementCounterSpy = jest.spyOn( + telemetryService, + 'incrementCounter' + ) - await telemetryService.incrementCounterWithTransactionAmount( - 'test_counter', - { - value: 100n, - assetCode: 'USD', - assetScale: 2 - } - ) + const counterName = 'test_counter' + await telemetryService.incrementCounterWithTransactionAmount( + counterName, + { + value: 100n, + assetCode: 'USD', + assetScale: 2 + } + ) - expect(aseConvertSpy).toHaveBeenCalled() - expect(internalConvertSpy).not.toHaveBeenCalled() - }) + expect(applyPrivacySpy).toHaveBeenCalledWith(Number(convertedAmount)) + expect(incrementCounterSpy).toHaveBeenCalledWith( + counterName, + 123, + expect.any(Object) + ) + }) - it('should apply privacy', 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)) - const applyPrivacySpy = jest - .spyOn(privacy, 'applyPrivacy') - .mockReturnValue(123) - const incrementCounterSpy = jest.spyOn( - telemetryService, - 'incrementCounter' - ) + it('should not collect telemetry when conversion returns InvalidDestinationPrice', async () => { + const convertSpy = 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(ConvertError.InvalidDestinationPrice) + ) + + const incrementCounterSpy = jest.spyOn( + telemetryService, + 'incrementCounter' + ) - const counterName = 'test_counter' - await telemetryService.incrementCounterWithTransactionAmount( - counterName, - { - value: 100n, - assetCode: 'USD', - assetScale: 2 - } - ) + await telemetryService.incrementCounterWithTransactionAmount( + 'test_counter', + { + value: 100n, + assetCode: 'USD', + assetScale: 2 + } + ) - expect(applyPrivacySpy).toHaveBeenCalledWith(Number(convertedAmount)) - expect(incrementCounterSpy).toHaveBeenCalledWith( - counterName, - 123, - expect.any(Object) - ) - }) + expect(convertSpy).toHaveBeenCalled() + expect(incrementCounterSpy).not.toHaveBeenCalled() + }) - it('should not collect telemetry when conversion returns InvalidDestinationPrice', async () => { - const convertSpy = 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(ConvertError.InvalidDestinationPrice) + it('should collect telemetry when conversion is successful', async () => { + const convertSpy = 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(10000n)) + const incrementCounterSpy = jest.spyOn( + telemetryService, + 'incrementCounter' ) + const obfuscatedAmount = 12000 + jest.spyOn(privacy, 'applyPrivacy').mockReturnValue(obfuscatedAmount) - const incrementCounterSpy = jest.spyOn( - telemetryService, - 'incrementCounter' - ) + const counterName = 'test_counter' - await telemetryService.incrementCounterWithTransactionAmount( - 'test_counter', - { - value: 100n, - assetCode: 'USD', - assetScale: 2 - } - ) + await telemetryService.incrementCounterWithTransactionAmount( + counterName, + { + value: 100n, + assetCode: 'USD', + assetScale: 2 + } + ) - expect(convertSpy).toHaveBeenCalled() - expect(incrementCounterSpy).not.toHaveBeenCalled() + expect(convertSpy).toHaveBeenCalled() + expect(incrementCounterSpy).toHaveBeenCalledWith( + counterName, + obfuscatedAmount, + expect.any(Object) + ) + }) + }) + }) + describe('Telemetry Disabled', () => { + let deps: IocContract + let appContainer: TestContainer + let telemetryService: TelemetryService + + beforeAll(async (): Promise => { + deps = initIocContainer({ + ...Config, + enableTelemetry: false + }) + appContainer = await createTestApp(deps) + telemetryService = await deps.use('telemetry')! }) - it('should collect telemetry when conversion is successful', async () => { - const convertSpy = 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(10000n)) - const incrementCounterSpy = jest.spyOn( - telemetryService, - 'incrementCounter' - ) - const obfuscatedAmount = 12000 - jest.spyOn(privacy, 'applyPrivacy').mockReturnValue(obfuscatedAmount) + afterAll(async (): Promise => { + await appContainer.shutdown() + }) - const counterName = 'test_counter' + test('telemetryService instance should be no-op implementation', () => { + expect(telemetryService instanceof NoopTelemetryServiceImpl).toBe(true) + }) - await telemetryService.incrementCounterWithTransactionAmount( - counterName, - { - value: 100n, - assetCode: 'USD', - assetScale: 2 - } - ) + test('NoopTelemetryServiceImpl should not get meter ', () => { + telemetryService.recordHistogram('testhistogram', 1) + telemetryService.incrementCounter('testcounter', 1) - expect(convertSpy).toHaveBeenCalled() - expect(incrementCounterSpy).toHaveBeenCalledWith( - counterName, - obfuscatedAmount, - expect.any(Object) - ) + expect(mockCounter.add).toHaveBeenCalledTimes(0) + expect(mockHistogram.record).toHaveBeenCalledTimes(0) }) }) }) diff --git a/packages/backend/src/telemetry/service.ts b/packages/backend/src/telemetry/service.ts index d0c03435a9..520e241964 100644 --- a/packages/backend/src/telemetry/service.ts +++ b/packages/backend/src/telemetry/service.ts @@ -32,7 +32,7 @@ export interface TelemetryService { ): void } -interface TelemetryServiceDependencies extends BaseService { +export interface TelemetryServiceDependencies extends BaseService { instanceName: string collectorUrls: string[] exportIntervalMillis?: number @@ -50,7 +50,11 @@ export function createTelemetryService( return new TelemetryServiceImpl(deps) } -class TelemetryServiceImpl implements TelemetryService { +export function createNoopTelemetryService(): TelemetryService { + return new NoopTelemetryServiceImpl() +} + +export class TelemetryServiceImpl implements TelemetryService { private instanceName: string private meterProvider?: MeterProvider private internalRatesService: RatesService @@ -178,7 +182,7 @@ class TelemetryServiceImpl implements TelemetryService { public recordHistogram( name: string, value: number, - attributes: Record = {} + attributes?: Record ): void { const histogram = this.getOrCreateHistogram(name) histogram.record(value, { @@ -216,3 +220,46 @@ class TelemetryServiceImpl implements TelemetryService { return converted } } + +/* eslint-disable @typescript-eslint/no-unused-vars */ +export class NoopTelemetryServiceImpl implements TelemetryService { + constructor() {} + + public async shutdown(): Promise { + // do nothing + } + + public incrementCounter( + name: string, + value: number, + attributes?: Record + ): void { + // do nothing + } + + public recordHistogram( + name: string, + value: number, + attributes?: Record + ): void { + // do nothing + } + + public async incrementCounterWithTransactionAmountDifference( + name: string, + amountSource: { value: bigint; assetCode: string; assetScale: number }, + amountDestination: { value: bigint; assetCode: string; assetScale: number }, + attributes?: Record + ): Promise { + // do nothing + } + + public async incrementCounterWithTransactionAmount( + name: string, + amount: { value: bigint; assetCode: string; assetScale: number }, + attributes: Record = {}, + preservePrivacy = true + ): Promise { + // do nothing + } +}