Skip to content

Commit

Permalink
Merge branch 'main' into 2808/mk/tracing
Browse files Browse the repository at this point in the history
# Conflicts:
#	localenv/telemetry/README.md
#	localenv/telemetry/docker-compose.yml
#	localenv/telemetry/grafana/provisioning/dashboards/default.yaml
#	localenv/telemetry/grafana/provisioning/dashboards/example.json
#	localenv/telemetry/grafana/provisioning/datasources/datasources.yaml
#	localenv/telemetry/otel-collector-config.yaml
#	packages/backend/src/telemetry/service.ts
  • Loading branch information
mkurapov committed Jul 18, 2024
2 parents 886535c + 771d189 commit e70be7a
Show file tree
Hide file tree
Showing 11 changed files with 103 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ providers:
- name: Example
type: file
options:
path: /etc/grafana/provisioning/dashboards
path: /etc/grafana/provisioning/dashboards
27 changes: 20 additions & 7 deletions packages/backend/src/open_payments/payment/outgoing/lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
18 changes: 16 additions & 2 deletions packages/backend/src/telemetry/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@ 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', () => ({
...jest.requireActual('@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)
})
}
}))
Expand Down Expand Up @@ -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()

Expand Down
31 changes: 22 additions & 9 deletions packages/backend/src/telemetry/service.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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
Expand Down Expand Up @@ -47,7 +41,8 @@ class TelemetryServiceImpl implements TelemetryService {
private internalRatesService: RatesService
private aseRatesService: RatesService

private counters = new Map()
private counters: Map<string, Counter> = new Map()
private histograms: Map<string, Histogram> = new Map()
constructor(private deps: TelemetryServiceDependencies) {
this.instanceName = deps.instanceName
this.internalRatesService = deps.internalRatesService
Expand All @@ -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
Expand Down
14 changes: 13 additions & 1 deletion packages/backend/src/tests/telemetry.ts
Original file line number Diff line number Diff line change
@@ -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<bigint | ConvertError> {
Expand Down Expand Up @@ -36,6 +42,12 @@ export class MockTelemetryService implements TelemetryService {
): Counter<Attributes> {
return mockCounter
}
public getOrCreateHistogramMetric(
_name: string,
_options?: MetricOptions | undefined
): Histogram<Attributes> {
return mockHistogram
}
public getInstanceName(): string | undefined {
return 'serviceName'
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand All @@ -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.
13 changes: 8 additions & 5 deletions packages/frontend/app/routes/auth.login.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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`
)
}
}

Expand Down
13 changes: 8 additions & 5 deletions packages/frontend/app/routes/auth.recovery.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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`
)
}
}

Expand Down
8 changes: 6 additions & 2 deletions packages/frontend/app/routes/settings.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -39,7 +43,7 @@ export const loader = async ({ request }: LoaderFunctionArgs) => {

return { responseData }
} else {
return redirect(
return redirectDocument(
`${variables.kratosBrowserPublicUrl}/self-service/settings/browser`
)
}
Expand Down
8 changes: 4 additions & 4 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion test/integration/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit e70be7a

Please sign in to comment.