diff --git a/localenv/telemetry/grafana/provisioning/dashboards/default.yaml b/localenv/telemetry/grafana/provisioning/dashboards/default.yaml index 0e1e0e33ba..190f281c6c 100644 --- a/localenv/telemetry/grafana/provisioning/dashboards/default.yaml +++ b/localenv/telemetry/grafana/provisioning/dashboards/default.yaml @@ -4,4 +4,4 @@ providers: - name: Example type: file options: - path: /etc/grafana/provisioning/dashboards \ No newline at end of file + path: /etc/grafana/provisioning/dashboards diff --git a/packages/backend/src/open_payments/payment/outgoing/lifecycle.ts b/packages/backend/src/open_payments/payment/outgoing/lifecycle.ts index e482311f44..150aec5409 100644 --- a/packages/backend/src/open_payments/payment/outgoing/lifecycle.ts +++ b/packages/backend/src/open_payments/payment/outgoing/lifecycle.ts @@ -75,19 +75,32 @@ export async function handleSending( throw LifecycleError.BadState } + const payStartTime = Date.now() await deps.paymentMethodHandlerService.pay('ILP', { receiver, outgoingPayment: payment, finalDebitAmount: maxDebitAmount, finalReceiveAmount: maxReceiveAmount }) - deps.telemetry - ?.getOrCreateMetric('transactions_total', { - description: 'Count of funded transactions' - }) - .add(1, { - source: deps.telemetry.getInstanceName() - }) + const payEndTime = Date.now() + + if (deps.telemetry) { + deps.telemetry + .getOrCreateMetric('transactions_total', { + description: 'Count of funded transactions' + }) + .add(1, { + source: deps.telemetry.getInstanceName() + }) + const payDuration = payEndTime - payStartTime + deps.telemetry + .getOrCreateHistogramMetric('ilp_pay_time_ms', { + description: 'Time to complete an ILP payment' + }) + .record(payDuration, { + source: deps.telemetry.getInstanceName() + }) + } await handleCompleted(deps, payment) } diff --git a/packages/backend/src/telemetry/service.test.ts b/packages/backend/src/telemetry/service.test.ts index 08616ee218..52ef956f67 100644 --- a/packages/backend/src/telemetry/service.test.ts +++ b/packages/backend/src/telemetry/service.test.ts @@ -4,7 +4,7 @@ import { AppServices } from '../app' import { Config } from '../config/app' import { ConvertError, RatesService } from '../rates/service' import { TestContainer, createTestApp } from '../tests/app' -import { mockCounter } from '../tests/telemetry' +import { mockCounter, mockHistogram } from '../tests/telemetry' import { TelemetryService } from './service' jest.mock('@opentelemetry/api', () => ({ @@ -12,7 +12,8 @@ jest.mock('@opentelemetry/api', () => ({ metrics: { setGlobalMeterProvider: jest.fn(), getMeter: jest.fn().mockReturnValue({ - createCounter: jest.fn().mockImplementation(() => mockCounter) + createCounter: jest.fn().mockImplementation(() => mockCounter), + createHistogram: jest.fn().mockImplementation(() => mockHistogram) }) } })) @@ -64,6 +65,19 @@ describe('TelemetryServiceImpl', () => { expect(retrievedCounter).toBe(existingCounter) }) + it('should create a histogram when getOrCreateHistogramMetric is called for a new metric', () => { + const histogram = telemetryService.getOrCreateHistogramMetric('testMetric') + expect(histogram).toBe(mockHistogram) + }) + + it('should return an existing histogram when getOrCreateHistogramMetric is called for an existing metric', () => { + const existingHistogram = + telemetryService.getOrCreateHistogramMetric('existingMetric') + const retrievedHistogram = + telemetryService.getOrCreateHistogramMetric('existingMetric') + expect(retrievedHistogram).toBe(existingHistogram) + }) + it('should return the instance name when calling getInstanceName', () => { const serviceName = telemetryService.getInstanceName() diff --git a/packages/backend/src/telemetry/service.ts b/packages/backend/src/telemetry/service.ts index 592c18fccc..48c94dee6c 100644 --- a/packages/backend/src/telemetry/service.ts +++ b/packages/backend/src/telemetry/service.ts @@ -1,11 +1,4 @@ -import { - Counter, - Histogram, - MetricOptions, - Tracer, - metrics, - trace -} from '@opentelemetry/api' +import { Counter, Histogram, MetricOptions, metrics } from '@opentelemetry/api' import { MeterProvider } from '@opentelemetry/sdk-metrics' import { ConvertError, RatesService, isConvertError } from '../rates/service' @@ -15,6 +8,7 @@ import { BaseService } from '../shared/baseService' export interface TelemetryService { shutdown(): void getOrCreateMetric(name: string, options?: MetricOptions): Counter + getOrCreateHistogramMetric(name: string, options?: MetricOptions): Histogram getInstanceName(): string | undefined getBaseAssetCode(): string getBaseScale(): number @@ -47,7 +41,8 @@ class TelemetryServiceImpl implements TelemetryService { private internalRatesService: RatesService private aseRatesService: RatesService - private counters = new Map() + private counters: Map = new Map() + private histograms: Map = new Map() constructor(private deps: TelemetryServiceDependencies) { this.instanceName = deps.instanceName this.internalRatesService = deps.internalRatesService @@ -58,6 +53,24 @@ class TelemetryServiceImpl implements TelemetryService { await this.meterProvider?.shutdown() } + private createHistogram(name: string, options: MetricOptions | undefined) { + const histogram = metrics + .getMeter(METER_NAME) + .createHistogram(name, options) + this.histograms.set(name, histogram) + return histogram + } + public getOrCreateHistogramMetric( + name: string, + options?: MetricOptions + ): Histogram { + const existing = this.histograms.get(name) + if (existing) { + return existing + } + return this.createHistogram(name, options) + } + private createCounter( name: string, options: MetricOptions | undefined diff --git a/packages/backend/src/tests/telemetry.ts b/packages/backend/src/tests/telemetry.ts index de7b204290..d9678bb7bb 100644 --- a/packages/backend/src/tests/telemetry.ts +++ b/packages/backend/src/tests/telemetry.ts @@ -1,9 +1,15 @@ -import { Attributes, Counter, MetricOptions } from '@opentelemetry/api' +import { + Attributes, + Counter, + Histogram, + MetricOptions +} from '@opentelemetry/api' import { TelemetryService } from '../telemetry/service' import { ConvertError, Rates, RatesService } from '../rates/service' import { ConvertOptions } from '../rates/util' export const mockCounter = { add: jest.fn() } as Counter +export const mockHistogram = { record: jest.fn() } as Histogram export class MockRatesService implements RatesService { async convert(): Promise { @@ -36,6 +42,12 @@ export class MockTelemetryService implements TelemetryService { ): Counter { return mockCounter } + public getOrCreateHistogramMetric( + _name: string, + _options?: MetricOptions | undefined + ): Histogram { + return mockHistogram + } public getInstanceName(): string | undefined { return 'serviceName' } diff --git a/packages/documentation/src/content/docs/telemetry/overview.md b/packages/documentation/src/content/docs/telemetry/overview.md index b9c7df9f6d..8527f9c499 100644 --- a/packages/documentation/src/content/docs/telemetry/overview.md +++ b/packages/documentation/src/content/docs/telemetry/overview.md @@ -60,7 +60,7 @@ If an ASE does not provide the necessary exchange rate for a transaction, the te ## Instrumentation -Rafiki currently has two counter metrics. All data points (counter increases) are exported to collection endpoints at a configurable interval (default recommended to 15s). +Rafiki currently has three counter metrics. All data points (counter increases and histogram records) are exported to collection endpoints at a configurable interval (default recommended to 15s). Currently collected metrics: @@ -70,5 +70,8 @@ Currently collected metrics: - `transactions_amount` - Counter metric - Description: “Amount sent through the network”. - This amount metric increases by the amount sent in each ILP packet. +- `ilp_pay_time_ms` - Histogram metric + - Description: “Time to complete an ILP payment” + - This histogram metric records the time taken to make an ILP payment. **Note**: The current implementation only collects metrics on the SENDING side of a transaction. Metrics for external open-payments transactions RECEIVED by a Rafiki instance in the network are not collected. diff --git a/packages/frontend/app/routes/auth.login.tsx b/packages/frontend/app/routes/auth.login.tsx index cc01536a81..d4a54a18ff 100644 --- a/packages/frontend/app/routes/auth.login.tsx +++ b/packages/frontend/app/routes/auth.login.tsx @@ -1,4 +1,8 @@ -import { json, type LoaderFunctionArgs } from '@remix-run/node' +import { + json, + type LoaderFunctionArgs, + redirectDocument +} from '@remix-run/node' import { uuidSchema } from '~/lib/validate.server' import { isUiNodeInputAttributes } from '@ory/integrations/ui' import type { UiContainer } from '@ory/client' @@ -34,10 +38,9 @@ export const loader = async ({ request }: LoaderFunctionArgs) => { const recoveryUrl = `${variables.kratosBrowserPublicUrl}/self-service/recovery/browser` return { responseData, recoveryUrl } } else { - throw json(null, { - status: 400, - statusText: 'No Kratos login flow ID found.' - }) + return redirectDocument( + `${variables.kratosBrowserPublicUrl}/self-service/login/browser` + ) } } diff --git a/packages/frontend/app/routes/auth.recovery.tsx b/packages/frontend/app/routes/auth.recovery.tsx index d6b165abc8..37dd1ace7e 100644 --- a/packages/frontend/app/routes/auth.recovery.tsx +++ b/packages/frontend/app/routes/auth.recovery.tsx @@ -1,4 +1,8 @@ -import { json, type LoaderFunctionArgs } from '@remix-run/node' +import { + json, + type LoaderFunctionArgs, + redirectDocument +} from '@remix-run/node' import { uuidSchema } from '~/lib/validate.server' import { isUiNodeInputAttributes } from '@ory/integrations/ui' import type { UiContainer } from '@ory/client' @@ -35,10 +39,9 @@ export const loader = async ({ request }: LoaderFunctionArgs) => { return { responseData } } else { - throw json(null, { - status: 400, - statusText: 'No Kratos account recovery flow ID found.' - }) + return redirectDocument( + `${variables.kratosBrowserPublicUrl}/self-service/recovery/browser` + ) } } diff --git a/packages/frontend/app/routes/settings.tsx b/packages/frontend/app/routes/settings.tsx index b97fdc8378..7edd807bfd 100644 --- a/packages/frontend/app/routes/settings.tsx +++ b/packages/frontend/app/routes/settings.tsx @@ -1,4 +1,8 @@ -import { json, redirect, type LoaderFunctionArgs } from '@remix-run/node' +import { + json, + redirectDocument, + type LoaderFunctionArgs +} from '@remix-run/node' import { uuidSchema } from '~/lib/validate.server' import { isUiNodeInputAttributes, @@ -39,7 +43,7 @@ export const loader = async ({ request }: LoaderFunctionArgs) => { return { responseData } } else { - return redirect( + return redirectDocument( `${variables.kratosBrowserPublicUrl}/self-service/settings/browser` ) } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 40a5546b15..e7ad6277a5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -748,8 +748,8 @@ importers: test/integration: devDependencies: '@apollo/client': - specifier: ^3.10.6 - version: 3.10.6(graphql@16.8.1) + specifier: ^3.10.8 + version: 3.10.8(graphql@16.8.1) '@interledger/http-signature-utils': specifier: 2.0.2 version: 2.0.2 @@ -835,8 +835,8 @@ packages: graphql: 16.8.1 dev: false - /@apollo/client@3.10.6(graphql@16.8.1): - resolution: {integrity: sha512-3lLFGJtzC1/mEnK11BRf+Bf8536kBQUSB1G9yMtcRsxmY+tCKdTPzsP3fMUKy10BPIE0sDUY1pux3iMPIn2vow==} + /@apollo/client@3.10.8(graphql@16.8.1): + resolution: {integrity: sha512-UaaFEitRrPRWV836wY2L7bd3HRCfbMie1jlYMcmazFAK23MVhz/Uq7VG1nwbotPb5xzFsw5RF4Wnp2G3dWPM3g==} peerDependencies: graphql: ^15.0.0 || ^16.0.0 graphql-ws: ^5.5.5 diff --git a/test/integration/package.json b/test/integration/package.json index ec2fcb24c3..548b0ca0a7 100644 --- a/test/integration/package.json +++ b/test/integration/package.json @@ -14,7 +14,7 @@ "author": "", "license": "ISC", "devDependencies": { - "@apollo/client": "^3.10.6", + "@apollo/client": "^3.10.8", "@interledger/http-signature-utils": "2.0.2", "@interledger/open-payments": "6.11.1", "@koa/bodyparser": "^5.1.1",