Improve telemetry instrumentation on core flows (#581)

Makefile

@@ -120,8 +120,21 @@ docker/mpc/stop:
 	docker-compose -f docker-compose.mpc.yml stop
 
 # === Git ===
+
 git/tag/push:
 	git tag armory-$(TAG)
 	git tag policy-engine-$(TAG)
 	git tag vault-$(TAG)
 	git push origin armory-$(TAG) policy-engine-$(TAG) vault-$(TAG)
+
+# === Database ===
+
+db/setup:
+	make armory/db/setup
+	make policy-engine/db/setup
+	make vault/db/setup
+
+db/generate-types:
+	make armory/db/generate-types
+	make policy-engine/db/generate-types
+	make vault/db/generate-types

README.md

@@ -182,6 +182,15 @@ and [Trace/Span
 Name](./packages/nestjs-shared/src/lib/module/open-telemetry/service/trace.service.ts)
 adopted conventions.
 
+### Debugging
+
+If you want to debug traces, the best way is to run `make docker/otel/up` and
+check the Jaeger UI at http://localhost:16686.
+
+In contrast, if you want to debug application instrumentation, open the `.env`
+file and set `OTEL_LOG_LEVEL=debug` to have a better view of what the SDK is
+doing.
+
 ## Troubleshooting
 
 ### DB URL in env variable fails when using `docker run`, but works when running outside docker

apps/armory/.env.default

@@ -43,6 +43,7 @@ MANAGED_DATASTORE_BASE_URL=http://localhost:3005/v1/data
 
 # === OpenTelemetry configuration ===
 
+# See https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/
 OTEL_SDK_DISABLED=true
 # OTEL Collector container HTTP port.
 OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318

apps/armory/src/armory.constant.ts

@@ -1,4 +1,10 @@
-import { adminApiKeySecurity, clientIdSecurity, clientSecretSecurity } from '@narval/nestjs-shared'
+import {
+  REQUEST_HEADER_CLIENT_ID,
+  REQUEST_HEADER_CLIENT_SECRET,
+  adminApiKeySecurity,
+  clientIdSecurity,
+  clientSecretSecurity
+} from '@narval/nestjs-shared'
 import { AssetId } from '@narval/policy-engine-shared'
 import { ClassSerializerInterceptor, ValidationPipe } from '@nestjs/common'
 import { APP_FILTER, APP_INTERCEPTOR, APP_PIPE } from '@nestjs/core'
@@ -49,8 +55,6 @@ export const DEFAULT_HTTP_MODULE_PROVIDERS = [
 // Headers
 //
 
-export const REQUEST_HEADER_CLIENT_ID = 'x-client-id'
-export const REQUEST_HEADER_CLIENT_SECRET = 'x-client-secret'
 export const REQUEST_HEADER_API_KEY = 'x-api-key'
 
 //

apps/armory/src/armory.module.ts

@@ -1,5 +1,5 @@
 import { ConfigModule } from '@narval/config-module'
-import { HttpLoggerMiddleware, LoggerModule, OpenTelemetryModule } from '@narval/nestjs-shared'
+import { HttpLoggerMiddleware, LoggerModule, OpenTelemetryModule, TrackClientIdMiddleware } from '@narval/nestjs-shared'
 import { ClassSerializerInterceptor, MiddlewareConsumer, Module, NestModule } from '@nestjs/common'
 import { APP_INTERCEPTOR } from '@nestjs/core'
 import { AppModule } from './app/app.module'
@@ -42,6 +42,6 @@ export class ArmoryModule implements NestModule {
   }
 
   configure(consumer: MiddlewareConsumer): void {
-    consumer.apply(HttpLoggerMiddleware).forRoutes('*')
+    consumer.apply(HttpLoggerMiddleware, TrackClientIdMiddleware).forRoutes('*')
   }
 }

apps/armory/src/main.ts

@@ -4,7 +4,7 @@ import { instrumentTelemetry } from '@narval/open-telemetry'
 // ensure proper instrumentation. The instrumentation packages patches Node.js
 // runtime - if NestFactory or other dependencies load first, they'll use the
 // unpatched runtime and won't be instrumented correctly.
-instrumentTelemetry({ serviceName: 'auth' })
+instrumentTelemetry({ serviceName: 'armory' })
 
 import { ConfigService } from '@narval/config-module'
 import { LoggerService, withApiVersion, withCors, withLogger, withSwagger } from '@narval/nestjs-shared'

apps/armory/src/managed-data-store/__test__/e2e/data-store.spec.ts

@@ -1,5 +1,11 @@
 import { ConfigModule } from '@narval/config-module'
-import { LoggerModule, OpenTelemetryModule, secret } from '@narval/nestjs-shared'
+import {
+  LoggerModule,
+  OpenTelemetryModule,
+  REQUEST_HEADER_CLIENT_ID,
+  REQUEST_HEADER_CLIENT_SECRET,
+  secret
+} from '@narval/nestjs-shared'
 import {
   Criterion,
   Entities,
@@ -17,7 +23,6 @@ import { MockProxy, mock } from 'jest-mock-extended'
 import request from 'supertest'
 import { generatePrivateKey } from 'viem/accounts'
 import { load } from '../../../armory.config'
-import { REQUEST_HEADER_CLIENT_ID, REQUEST_HEADER_CLIENT_SECRET } from '../../../armory.constant'
 import { ClientService } from '../../../client/core/service/client.service'
 import { ClusterService } from '../../../policy-engine/core/service/cluster.service'
 import { TestPrismaService } from '../../../shared/module/persistence/service/test-prisma.service'

apps/armory/src/managed-data-store/core/service/entity-data-store.service.ts

@@ -1,4 +1,4 @@
-import { MetricService, TraceService } from '@narval/nestjs-shared'
+import { MetricService, OTEL_ATTR_CLIENT_ID, TraceService } from '@narval/nestjs-shared'
 import { EntityStore } from '@narval/policy-engine-shared'
 import { HttpStatus, Inject, Injectable, NotFoundException } from '@nestjs/common'
 import { Counter } from '@opentelemetry/api'
@@ -27,10 +27,10 @@ export class EntityDataStoreService extends SignatureService {
   }
 
   async getEntities(clientId: string): Promise<EntityStore | null> {
-    this.getCounter.add(1, { clientId })
+    this.getCounter.add(1, { [OTEL_ATTR_CLIENT_ID]: clientId })
 
     const span = this.traceService.startSpan(`${EntityDataStoreService.name}.getEntities`, {
-      attributes: { clientId }
+      attributes: { [OTEL_ATTR_CLIENT_ID]: clientId }
     })
 
     const entityStore = await this.entityDataStoreRepository.getLatestDataStore(clientId)
@@ -43,10 +43,10 @@ export class EntityDataStoreService extends SignatureService {
   }
 
   async setEntities(clientId: string, payload: EntityStore) {
-    this.setCounter.add(1, { clientId })
+    this.setCounter.add(1, { [OTEL_ATTR_CLIENT_ID]: clientId })
 
     const span = this.traceService.startSpan(`${EntityDataStoreService.name}.setEntities`, {
-      attributes: { clientId }
+      attributes: { [OTEL_ATTR_CLIENT_ID]: clientId }
     })
 
     const client = await this.clientService.findById(clientId)

apps/armory/src/managed-data-store/core/service/policy-data-store.service.ts

@@ -1,4 +1,4 @@
-import { MetricService, TraceService } from '@narval/nestjs-shared'
+import { MetricService, OTEL_ATTR_CLIENT_ID, TraceService } from '@narval/nestjs-shared'
 import { PolicyStore } from '@narval/policy-engine-shared'
 import { HttpStatus, Inject, Injectable, NotFoundException } from '@nestjs/common'
 import { Counter } from '@opentelemetry/api'
@@ -26,10 +26,10 @@ export class PolicyDataStoreService extends SignatureService {
   }
 
   async getPolicies(clientId: string): Promise<PolicyStore | null> {
-    this.getCounter.add(1, { clientId })
+    this.getCounter.add(1, { [OTEL_ATTR_CLIENT_ID]: clientId })
 
     const span = this.traceService.startSpan(`${PolicyDataStoreService.name}.getPolicies`, {
-      attributes: { clientId }
+      attributes: { [OTEL_ATTR_CLIENT_ID]: clientId }
     })
 
     const policyStore = await this.policyDataStoreRepository.getLatestDataStore(clientId)
@@ -42,10 +42,10 @@ export class PolicyDataStoreService extends SignatureService {
   }
 
   async setPolicies(clientId: string, payload: PolicyStore) {
-    this.setCounter.add(1, { clientId })
+    this.setCounter.add(1, { [OTEL_ATTR_CLIENT_ID]: clientId })
 
     const span = this.traceService.startSpan(`${PolicyDataStoreService.name}.setPolicies`, {
-      attributes: { clientId }
+      attributes: { [OTEL_ATTR_CLIENT_ID]: clientId }
     })
 
     const client = await this.clientService.findById(clientId)

apps/armory/src/managed-data-store/shared/guard/__test__/unit/data-store.guard.spec.ts

@@ -1,9 +1,8 @@
-import { secret } from '@narval/nestjs-shared'
+import { REQUEST_HEADER_CLIENT_ID, REQUEST_HEADER_CLIENT_SECRET, secret } from '@narval/nestjs-shared'
 import { getPublicKey, privateKeyToJwk } from '@narval/signature'
 import { ExecutionContext } from '@nestjs/common'
 import { mock } from 'jest-mock-extended'
 import { generatePrivateKey } from 'viem/accounts'
-import { REQUEST_HEADER_CLIENT_ID, REQUEST_HEADER_CLIENT_SECRET } from '../../../../../armory.constant'
 import { ClientService } from '../../../../../client/core/service/client.service'
 import { Client } from '../../../../../client/core/type/client.type'
 import { ApplicationException } from '../../../../../shared/exception/application.exception'

apps/armory/src/managed-data-store/shared/guard/data-store.guard.ts

@@ -1,6 +1,5 @@
-import { secret } from '@narval/nestjs-shared'
+import { REQUEST_HEADER_CLIENT_ID, REQUEST_HEADER_CLIENT_SECRET, secret } from '@narval/nestjs-shared'
 import { CanActivate, ExecutionContext, HttpStatus, Injectable } from '@nestjs/common'
-import { REQUEST_HEADER_CLIENT_ID, REQUEST_HEADER_CLIENT_SECRET } from '../../../armory.constant'
 import { ClientService } from '../../../client/core/service/client.service'
 import { ApplicationException } from '../../../shared/exception/application.exception'
 

apps/armory/src/orchestration/__test__/e2e/authorization-request.spec.ts

@@ -1,5 +1,5 @@
 import { ConfigModule } from '@narval/config-module'
-import { LoggerModule, OpenTelemetryModule, secret } from '@narval/nestjs-shared'
+import { LoggerModule, OpenTelemetryModule, REQUEST_HEADER_CLIENT_ID, secret } from '@narval/nestjs-shared'
 import { Action, AuthorizationRequest, Eip712TypedData, FIXTURE, Request } from '@narval/policy-engine-shared'
 import { getQueueToken } from '@nestjs/bull'
 import { HttpStatus, INestApplication } from '@nestjs/common'
@@ -10,7 +10,7 @@ import { mock } from 'jest-mock-extended'
 import request from 'supertest'
 import { stringToHex } from 'viem'
 import { load } from '../../../armory.config'
-import { AUTHORIZATION_REQUEST_PROCESSING_QUEUE, REQUEST_HEADER_CLIENT_ID } from '../../../armory.constant'
+import { AUTHORIZATION_REQUEST_PROCESSING_QUEUE } from '../../../armory.constant'
 import { PersistenceModule } from '../../../shared/module/persistence/persistence.module'
 import { TestPrismaService } from '../../../shared/module/persistence/service/test-prisma.service'
 import { QueueModule } from '../../../shared/module/queue/queue.module'

apps/armory/src/orchestration/core/service/__test__/unit/authorization-request.service.spec.ts

@@ -1,4 +1,12 @@
-import { LoggerModule, LoggerService, NullLoggerService } from '@narval/nestjs-shared'
+import {
+  LoggerModule,
+  LoggerService,
+  MetricService,
+  NullLoggerService,
+  OTEL_ATTR_CLIENT_ID,
+  OpenTelemetryModule,
+  StatefulMetricService
+} from '@narval/nestjs-shared'
 import {
   Action,
   AuthorizationRequest,
@@ -44,6 +52,7 @@ describe(AuthorizationRequestService.name, () => {
   let clusterServiceMock: MockProxy<ClusterService>
   let priceServiceMock: MockProxy<PriceService>
   let feedServiceMock: MockProxy<FeedService>
+  let statefulMetricService: StatefulMetricService
   let service: AuthorizationRequestService
 
   const authzRequest: AuthorizationRequest = generateAuthorizationRequest({
@@ -65,7 +74,7 @@ describe(AuthorizationRequestService.name, () => {
     feedServiceMock = mock<FeedService>()
 
     module = await Test.createTestingModule({
-      imports: [LoggerModule],
+      imports: [LoggerModule, OpenTelemetryModule.forTest()],
       providers: [
         AuthorizationRequestService,
         {
@@ -104,6 +113,23 @@ describe(AuthorizationRequestService.name, () => {
     }).compile()
 
     service = module.get<AuthorizationRequestService>(AuthorizationRequestService)
+    statefulMetricService = module.get(MetricService)
+  })
+
+  describe('create', () => {
+    it('increments create counter metric', async () => {
+      await service.create(authzRequest)
+
+      expect(statefulMetricService.counters).toEqual([
+        {
+          name: 'authorization_request_create_count',
+          value: 1,
+          attributes: {
+            [OTEL_ATTR_CLIENT_ID]: authzRequest.clientId
+          }
+        }
+      ])
+    })
   })
 
   describe('approve', () => {
@@ -234,6 +260,21 @@ describe(AuthorizationRequestService.name, () => {
         createdAt: expect.any(Date)
       })
     })
+
+    it('increments evaluation counter metric', async () => {
+      await service.evaluate(authzRequest)
+
+      expect(statefulMetricService.counters).toEqual([
+        {
+          name: 'authorization_request_evaluation_count',
+          value: 1,
+          attributes: {
+            [OTEL_ATTR_CLIENT_ID]: authzRequest.clientId,
+            'domain.authorization_request.status': AuthorizationRequestStatus.PERMITTED
+          }
+        }
+      ])
+    })
   })
 
   describe('process', () => {

apps/armory/src/orchestration/core/service/authorization-request.service.ts

@@ -1,4 +1,4 @@
-import { LoggerService } from '@narval/nestjs-shared'
+import { LoggerService, MetricService, OTEL_ATTR_CLIENT_ID } from '@narval/nestjs-shared'
 import {
   Action,
   AuthorizationRequest,
@@ -9,7 +9,8 @@ import {
   JwtString
 } from '@narval/policy-engine-shared'
 import { Intent, Intents } from '@narval/transaction-request-intent'
-import { HttpStatus, Injectable } from '@nestjs/common'
+import { HttpStatus, Inject, Injectable } from '@nestjs/common'
+import { Counter } from '@opentelemetry/api'
 import { v4 as uuid } from 'uuid'
 import { AUTHORIZATION_REQUEST_PROCESSING_QUEUE_ATTEMPTS, FIAT_ID_USD } from '../../../armory.constant'
 import { FeedService } from '../../../data-feed/core/service/feed.service'
@@ -40,6 +41,9 @@ const getStatus = (decision: string): AuthorizationRequestStatus => {
 
 @Injectable()
 export class AuthorizationRequestService {
+  private createCounter: Counter
+  private evaluationCounter: Counter
+
   constructor(
     private authzRequestRepository: AuthorizationRequestRepository,
     private authzRequestApprovalRepository: AuthorizationRequestApprovalRepository,
@@ -48,10 +52,16 @@ export class AuthorizationRequestService {
     private priceService: PriceService,
     private clusterService: ClusterService,
     private feedService: FeedService,
-    private logger: LoggerService
-  ) {}
+    private logger: LoggerService,
+    @Inject(MetricService) private metricService: MetricService
+  ) {
+    this.createCounter = this.metricService.createCounter('authorization_request_create_count')
+    this.evaluationCounter = this.metricService.createCounter('authorization_request_evaluation_count')
+  }
 
   async create(input: CreateAuthorizationRequest): Promise<AuthorizationRequest> {
+    this.createCounter.add(1, { [OTEL_ATTR_CLIENT_ID]: input.clientId })
+
     const now = new Date()
 
     const authzRequest = await this.authzRequestRepository.create({
@@ -156,6 +166,12 @@ export class AuthorizationRequestService {
     })
 
     const status = getStatus(evaluation.decision)
+
+    this.evaluationCounter.add(1, {
+      [OTEL_ATTR_CLIENT_ID]: input.clientId,
+      'domain.authorization_request.status': status
+    })
+
     // NOTE: we will track the transfer before we update the status to PERMITTED so that we don't have a brief window where a second transfer can come in before the history is tracked.
     // TODO: (@wcalderipe, 01/02/24) Move to the TransferTrackingService.
     if (input.request.action === Action.SIGN_TRANSACTION && status === AuthorizationRequestStatus.PERMITTED) {

apps/armory/src/policy-engine/core/service/__test__/integration/cluster.service.spec.ts

@@ -1,5 +1,12 @@
 import { ConfigModule, ConfigService } from '@narval/config-module'
-import { LoggerModule, MetricService, OpenTelemetryModule, StatefulMetricService, secret } from '@narval/nestjs-shared'
+import {
+  LoggerModule,
+  MetricService,
+  OTEL_ATTR_CLIENT_ID,
+  OpenTelemetryModule,
+  StatefulMetricService,
+  secret
+} from '@narval/nestjs-shared'
 import {
   DataStoreConfiguration,
   Decision,
@@ -357,7 +364,9 @@ describe(ClusterService.name, () => {
         {
           name: 'cluster_sync_count',
           value: 1,
-          attributes: { clientId: 'test-client-id' }
+          attributes: {
+            [OTEL_ATTR_CLIENT_ID]: 'test-client-id'
+          }
         }
       ])
     })

apps/armory/src/policy-engine/core/service/cluster.service.ts

@@ -1,5 +1,5 @@
 import { ConfigService } from '@narval/config-module'
-import { LoggerService, MetricService, TraceService } from '@narval/nestjs-shared'
+import { LoggerService, MetricService, OTEL_ATTR_CLIENT_ID, TraceService } from '@narval/nestjs-shared'
 import { Decision, EvaluationRequest, EvaluationResponse } from '@narval/policy-engine-shared'
 import { PublicKey, verifyJwt } from '@narval/signature'
 import { HttpStatus, Inject, Injectable } from '@nestjs/common'
@@ -223,10 +223,10 @@ export class ClusterService {
   }
 
   async sync(clientId: string) {
-    this.syncCounter.add(1, { clientId })
+    this.syncCounter.add(1, { [OTEL_ATTR_CLIENT_ID]: clientId })
 
     const span = this.traceService.startSpan(`${ClusterService.name}.sync`, {
-      attributes: { clientId }
+      attributes: { [OTEL_ATTR_CLIENT_ID]: clientId }
     })
 
     const nodes = await this.findNodesByClientId(clientId)

apps/armory/src/shared/decorator/__test__/unit/client-id.decorator.spec.ts

@@ -1,5 +1,5 @@
+import { REQUEST_HEADER_CLIENT_ID } from '@narval/nestjs-shared'
 import { ExecutionContext } from '@nestjs/common'
-import { REQUEST_HEADER_CLIENT_ID } from '../../../../armory.constant'
 import { factory } from '../../../decorator/client-id.decorator'
 
 describe('clientId Decorator', () => {