From 636707a1844c80857e2408a9e0e8c17a3c229a87 Mon Sep 17 00:00:00 2001 From: William Calderipe Date: Tue, 19 Nov 2024 14:06:42 +0100 Subject: [PATCH 1/3] Add job ID and metadata in the span attributes --- .../orchestration/orchestration.constant.ts | 2 + ...thorization-request-processing.consumer.ts | 47 ++++++++++++++++++- ...thorization-request-processing.producer.ts | 30 ++++++++++-- .../open-telemetry/service/trace.service.ts | 2 +- 4 files changed, 73 insertions(+), 8 deletions(-) diff --git a/apps/armory/src/orchestration/orchestration.constant.ts b/apps/armory/src/orchestration/orchestration.constant.ts index 1d2f19201..c5d3ede2d 100644 --- a/apps/armory/src/orchestration/orchestration.constant.ts +++ b/apps/armory/src/orchestration/orchestration.constant.ts @@ -81,3 +81,5 @@ export const ACTION_REQUEST = new Map([ } ] ]) + +export const OTEL_ATTR_JOB_ID = 'job.id' diff --git a/apps/armory/src/orchestration/queue/consumer/authorization-request-processing.consumer.ts b/apps/armory/src/orchestration/queue/consumer/authorization-request-processing.consumer.ts index 46ac39c5f..1a27b705e 100644 --- a/apps/armory/src/orchestration/queue/consumer/authorization-request-processing.consumer.ts +++ b/apps/armory/src/orchestration/queue/consumer/authorization-request-processing.consumer.ts @@ -1,6 +1,8 @@ -import { LoggerService } from '@narval/nestjs-shared' +import { LoggerService, TraceService } from '@narval/nestjs-shared' import { AuthorizationRequestProcessingJob } from '@narval/policy-engine-shared' import { OnQueueActive, OnQueueCompleted, OnQueueFailed, Process, Processor } from '@nestjs/bull' +import { Inject } from '@nestjs/common' +import { SpanStatusCode } from '@opentelemetry/api' import { Job } from 'bull' import { v4 as uuid } from 'uuid' import { @@ -13,12 +15,19 @@ import { InvalidAttestationSignatureException } from '../../../policy-engine/cor import { UnreachableClusterException } from '../../../policy-engine/core/exception/unreachable-cluster.exception' import { AuthorizationRequestAlreadyProcessingException } from '../../core/exception/authorization-request-already-processing.exception' import { AuthorizationRequestService } from '../../core/service/authorization-request.service' +import { OTEL_ATTR_JOB_ID } from '../../orchestration.constant' + +const OTEL_ATTR_JOB_ATTEMPS_MADE = 'job.attempts_made' +const OTEL_ATTR_JOB_FAILED = 'job.failed' +const OTEL_ATTR_JOB_MAX_RETRIES = 'job.max_retries' +const OTEL_ATTR_JOB_RETRIED = 'job.retried' @Processor(AUTHORIZATION_REQUEST_PROCESSING_QUEUE) export class AuthorizationRequestProcessingConsumer { constructor( private authzService: AuthorizationRequestService, - private logger: LoggerService + private logger: LoggerService, + @Inject(TraceService) private traceService: TraceService ) {} @Process() @@ -28,9 +37,15 @@ export class AuthorizationRequestProcessingConsumer { data: job.data }) + const span = this.traceService.startSpan(`${AuthorizationRequestProcessingConsumer.name}.process`, { + attributes: { [OTEL_ATTR_JOB_ID]: job.id } + }) + try { await this.authzService.process(job.id.toString(), job.attemptsMade) } catch (error) { + span.recordException(error) + // Short-circuits the retry mechanism on unrecoverable domain errors. // // IMPORTANT: To stop retrying a job in Bull, the process must return an @@ -39,7 +54,15 @@ export class AuthorizationRequestProcessingConsumer { return error } + // If the error ins't recoverable, set the span status to error. + span.setStatus({ + code: SpanStatusCode.ERROR, + message: error.message + }) + throw error + } finally { + span.end() } } @@ -66,9 +89,15 @@ export class AuthorizationRequestProcessingConsumer { @OnQueueCompleted() async onCompleted(job: Job, result: unknown) { + const span = this.traceService.startSpan(`${AuthorizationRequestProcessingConsumer.name}.onCompleted`, { + attributes: { [OTEL_ATTR_JOB_ID]: job.id } + }) + if (result instanceof Error) { this.logger.error('Stop processing authorization request due to unrecoverable error', result) + span.setAttribute(OTEL_ATTR_JOB_FAILED, true).end() + await this.authzService.fail(job.id.toString(), { ...result, id: uuid() @@ -82,6 +111,8 @@ export class AuthorizationRequestProcessingConsumer { data: job.data, result }) + + span.setAttribute(OTEL_ATTR_JOB_FAILED, false).end() } @OnQueueFailed() @@ -93,8 +124,17 @@ export class AuthorizationRequestProcessingConsumer { error } + const span = this.traceService.startSpan(`${AuthorizationRequestProcessingConsumer.name}.onCompleted`, { + attributes: { + [OTEL_ATTR_JOB_ID]: log.id, + [OTEL_ATTR_JOB_MAX_RETRIES]: log.maxAttempts, + [OTEL_ATTR_JOB_ATTEMPS_MADE]: log.attemptsMade + } + }) + if (job.attemptsMade >= AUTHORIZATION_REQUEST_PROCESSING_QUEUE_ATTEMPTS) { this.logger.error('Process authorization request failed', log) + span.setAttribute(OTEL_ATTR_JOB_RETRIED, false) await this.authzService.fail(job.id.toString(), { id: uuid(), @@ -102,6 +142,9 @@ export class AuthorizationRequestProcessingConsumer { }) } else { this.logger.log('Retrying to process authorization request', log) + span.setAttribute(OTEL_ATTR_JOB_RETRIED, true) } + + span.end() } } diff --git a/apps/armory/src/orchestration/queue/producer/authorization-request-processing.producer.ts b/apps/armory/src/orchestration/queue/producer/authorization-request-processing.producer.ts index 786265c3c..d052d39ab 100644 --- a/apps/armory/src/orchestration/queue/producer/authorization-request-processing.producer.ts +++ b/apps/armory/src/orchestration/queue/producer/authorization-request-processing.producer.ts @@ -1,17 +1,18 @@ -import { LoggerService } from '@narval/nestjs-shared' +import { LoggerService, OTEL_ATTR_CLIENT_ID, TraceService } from '@narval/nestjs-shared' import { AuthorizationRequest, AuthorizationRequestProcessingJob, AuthorizationRequestStatus } from '@narval/policy-engine-shared' import { InjectQueue } from '@nestjs/bull' -import { Injectable, OnApplicationBootstrap } from '@nestjs/common' +import { Inject, Injectable, OnApplicationBootstrap } from '@nestjs/common' import { BackoffOptions, Job, Queue } from 'bull' import { AUTHORIZATION_REQUEST_PROCESSING_QUEUE, AUTHORIZATION_REQUEST_PROCESSING_QUEUE_ATTEMPTS, AUTHORIZATION_REQUEST_PROCESSING_QUEUE_BACKOFF } from '../../../armory.constant' +import { OTEL_ATTR_JOB_ID } from '../../orchestration.constant' import { AuthorizationRequestRepository } from '../../persistence/repository/authorization-request.repository' type JobOption = { @@ -30,11 +31,16 @@ export class AuthorizationRequestProcessingProducer implements OnApplicationBoot @InjectQueue(AUTHORIZATION_REQUEST_PROCESSING_QUEUE) private processingQueue: Queue, private authzRequestRepository: AuthorizationRequestRepository, - private logger: LoggerService + private logger: LoggerService, + @Inject(TraceService) private traceService: TraceService ) {} async add(authzRequest: AuthorizationRequest): Promise> { - return this.processingQueue.add( + const span = this.traceService.startSpan(`${AuthorizationRequestProcessingProducer.name}.add`, { + attributes: { [OTEL_ATTR_CLIENT_ID]: authzRequest.id } + }) + + const job = await this.processingQueue.add( { id: authzRequest.id }, @@ -43,9 +49,15 @@ export class AuthorizationRequestProcessingProducer implements OnApplicationBoot ...DEFAULT_JOB_OPTIONS } ) + + span.setAttribute(OTEL_ATTR_JOB_ID, job.id).end() + + return job } async bulkAdd(requests: AuthorizationRequest[]): Promise[]> { + const span = this.traceService.startSpan(`${AuthorizationRequestProcessingProducer.name}.bulkAdd`) + const jobs = requests.map(({ id }) => ({ data: { id }, opts: { @@ -54,10 +66,16 @@ export class AuthorizationRequestProcessingProducer implements OnApplicationBoot } })) - return this.processingQueue.addBulk(jobs) + const addedJobs = await this.processingQueue.addBulk(jobs) + + span.end() + + return addedJobs } async onApplicationBootstrap() { + const span = this.traceService.startSpan(`${AuthorizationRequestProcessingProducer.name}.onApplicationBootstrap`) + const requests = await this.authzRequestRepository.findByStatus(AuthorizationRequestStatus.CREATED) if (requests.length) { @@ -67,5 +85,7 @@ export class AuthorizationRequestProcessingProducer implements OnApplicationBoot await this.bulkAdd(requests) } + + span.end() } } diff --git a/packages/nestjs-shared/src/lib/module/open-telemetry/service/trace.service.ts b/packages/nestjs-shared/src/lib/module/open-telemetry/service/trace.service.ts index 2b7284858..73ba2e682 100644 --- a/packages/nestjs-shared/src/lib/module/open-telemetry/service/trace.service.ts +++ b/packages/nestjs-shared/src/lib/module/open-telemetry/service/trace.service.ts @@ -5,7 +5,7 @@ export const TraceService = Symbol('TraceService') /** * OpenTelemetry Trace/Span Name Conventions * - * Format: .. + * Format: `..` * * Common Operation Types: * - http: Web operations From cbd5b7d1e603220aa616d2b4af9c1eb0251bc95d Mon Sep 17 00:00:00 2001 From: William Calderipe Date: Tue, 19 Nov 2024 14:07:18 +0100 Subject: [PATCH 2/3] Add node engine >=21 --- Makefile | 4 ++-- README.md | 4 ++-- package-lock.json | 3 +++ package.json | 3 +++ 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index b8e891f7a..abe06b7f8 100644 --- a/Makefile +++ b/Makefile @@ -19,10 +19,10 @@ TERM_GREEN := \033[0;32m # === Install === install: - npm install + npm install --engine-strict install/ci: - npm ci + npm ci --engine-strict # === Setup === diff --git a/README.md b/README.md index 1ded0ec56..e084627a2 100644 --- a/README.md +++ b/README.md @@ -107,7 +107,7 @@ dependency `@narval-xyz/armory-mpc-module`. 1. Create a `.npmrc` file in the root of this project. 1. Get the values from someone who has them. -1. Now `npm install` should work. +1. Now `make install` should work. ## Generating a new project @@ -133,7 +133,7 @@ packages to NPM. 1. Run `make packages/release` and follow the prompts to bump the projects' versions. -1. Run `npm install` to update `package-lock.json`. +1. Run `make install` to update `package-lock.json`. 1. Commit and push the changes to your branch. 1. After your branch is merged, manually trigger the [packages pipeline to publish](https://github.com/narval-xyz/armory/actions/workflows/packages-publish.yml) diff --git a/package-lock.json b/package-lock.json index 7e64feef5..303a05c2c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -151,6 +151,9 @@ "typescript": "5.4.5", "zod-fixture": "2.5.1" }, + "engines": { + "node": ">=21.0.0" + }, "optionalDependencies": { "@narval-xyz/armory-mpc-module": "0.0.1" } diff --git a/package.json b/package.json index 616288ac7..dfd7a9251 100644 --- a/package.json +++ b/package.json @@ -2,6 +2,9 @@ "name": "@narval/source", "version": "v0.0.13", "license": "MPL-2.0", + "engines": { + "node": ">=21.0.0" + }, "workspaces": [ "packages/*" ], From eb78a4154b4c8eccc23dce63a3fb5ccadcfefadd Mon Sep 17 00:00:00 2001 From: William Calderipe Date: Tue, 19 Nov 2024 14:24:34 +0100 Subject: [PATCH 3/3] Enforce node version 20 due to Vercel limitation --- .github/workflows/armory.yml | 2 +- .github/workflows/packages-publish.yml | 2 +- .github/workflows/packages.yml | 2 +- .github/workflows/policy-engine.yml | 2 +- .github/workflows/vault.yml | 2 +- deploy/armory.dockerfile | 4 ++-- package-lock.json | 2 +- package.json | 2 +- 8 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/armory.yml b/.github/workflows/armory.yml index b0ace9211..5e251f4e0 100644 --- a/.github/workflows/armory.yml +++ b/.github/workflows/armory.yml @@ -65,7 +65,7 @@ jobs: - name: Install node.js uses: actions/setup-node@v4 with: - node-version: '21' + node-version: '20' cache: 'npm' - name: Install dependencies diff --git a/.github/workflows/packages-publish.yml b/.github/workflows/packages-publish.yml index a6ddfeeb7..b2370c698 100644 --- a/.github/workflows/packages-publish.yml +++ b/.github/workflows/packages-publish.yml @@ -20,7 +20,7 @@ jobs: - name: Install node.js uses: actions/setup-node@v4 with: - node-version: '21' + node-version: '20' registry-url: https://registry.npmjs.org/ cache: 'npm' diff --git a/.github/workflows/packages.yml b/.github/workflows/packages.yml index c676cbc78..aeb2683e8 100644 --- a/.github/workflows/packages.yml +++ b/.github/workflows/packages.yml @@ -37,7 +37,7 @@ jobs: - name: Install node.js uses: actions/setup-node@v4 with: - node-version: '21' + node-version: '20' cache: 'npm' - name: Install dependencies diff --git a/.github/workflows/policy-engine.yml b/.github/workflows/policy-engine.yml index 00de11968..f78a5a2c7 100644 --- a/.github/workflows/policy-engine.yml +++ b/.github/workflows/policy-engine.yml @@ -54,7 +54,7 @@ jobs: - name: Install node.js uses: actions/setup-node@v4 with: - node-version: '21' + node-version: '20' cache: 'npm' - name: Install dependencies diff --git a/.github/workflows/vault.yml b/.github/workflows/vault.yml index 05493b8ab..ce8b38c25 100644 --- a/.github/workflows/vault.yml +++ b/.github/workflows/vault.yml @@ -52,7 +52,7 @@ jobs: - name: Install node.js uses: actions/setup-node@v4 with: - node-version: '21' + node-version: '20' cache: 'npm' - name: Install dependencies diff --git a/deploy/armory.dockerfile b/deploy/armory.dockerfile index 13f7ea537..7c5614893 100644 --- a/deploy/armory.dockerfile +++ b/deploy/armory.dockerfile @@ -1,4 +1,4 @@ -FROM node:21 as build +FROM node:20 as build # Set the working directory WORKDIR /usr/src/app @@ -19,7 +19,7 @@ RUN make armory/db/generate-types && \ make armory/build && \ rm -rf apps/ && rm -rf packages/ -FROM node:21 as final +FROM node:20 as final WORKDIR /usr/src/app diff --git a/package-lock.json b/package-lock.json index 303a05c2c..5a67bd2c0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -152,7 +152,7 @@ "zod-fixture": "2.5.1" }, "engines": { - "node": ">=21.0.0" + "node": "^20.0.0" }, "optionalDependencies": { "@narval-xyz/armory-mpc-module": "0.0.1" diff --git a/package.json b/package.json index dfd7a9251..0cdacf040 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "version": "v0.0.13", "license": "MPL-2.0", "engines": { - "node": ">=21.0.0" + "node": "^20.0.0" }, "workspaces": [ "packages/*"