Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: otel support #2236

Open
wants to merge 20 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,502 changes: 1,472 additions & 30 deletions pnpm-lock.yaml

Large diffs are not rendered by default.

15 changes: 15 additions & 0 deletions services/workflows-service/docker-compose.otel.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
version: '3'
services:
tracing:
image: jaegertracing/all-in-one:latest
liorzam marked this conversation as resolved.
Show resolved Hide resolved
ports:
- 6831:6831/udp
- 6832:6832/udp
- 5778:5778
- 16686:16686
- 4317:4317
- 4318:4318
- 14250:14250
- 14268:14268
- 14269:14269
- 9411:9411
26 changes: 21 additions & 5 deletions services/workflows-service/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,23 @@
"@nestjs/platform-express": "^9.3.12",
"@nestjs/serve-static": "3.0.1",
"@nestjs/testing": "^9.3.12",
"@opentelemetry/api": "^1.8.0",
"@opentelemetry/auto-instrumentations-node": "^0.43.0",
"@opentelemetry/exporter-jaeger": "^1.22.0",
"@opentelemetry/exporter-trace-otlp-http": "^0.49.1",
"@opentelemetry/exporter-trace-otlp-proto": "^0.49.1",
"@opentelemetry/instrumentation-aws-sdk": "^0.39.1",
"@opentelemetry/instrumentation-express": "^0.36.1",
"@opentelemetry/instrumentation-http": "^0.49.1",
"@opentelemetry/instrumentation-nestjs-core": "^0.35.0",
"@opentelemetry/instrumentation-winston": "^0.35.0",
"@opentelemetry/propagator-b3": "^1.22.0",
"@opentelemetry/resource-detector-docker": "^0.1.2",
"@opentelemetry/resources": "^1.22.0",
"@opentelemetry/sdk-node": "^0.49.1",
"@opentelemetry/semantic-conventions": "^1.22.0",
"@prisma/client": "4.16.2",
"@prisma/instrumentation": "^5.11.0",
"@sentry/cli": "^2.17.5",
"@sentry/integrations": "^7.52.1",
"@sentry/node": "^7.52.1",
Expand All @@ -70,8 +86,8 @@
"ajv-keywords": "^5.1.0",
"aws-cloudfront-sign": "3.0.2",
"axios": "^1.6.2",
"base64-stream": "^1.0.0",
"axios-retry": "^4.0.0",
"base64-stream": "^1.0.0",
"bcrypt": "5.1.0",
"class-transformer": "0.5.1",
"class-validator": "0.14.0",
Expand All @@ -84,8 +100,8 @@
"i18n-iso-countries": "^7.6.0",
"i18next": "^22.4.9",
"is-what": "^4.1.15",
"json-stable-stringify": "^1.1.1",
"js-base64": "^3.7.6",
"json-stable-stringify": "^1.1.1",
"lodash": "^4.17.21",
"mime": "^3.0.0",
"multer-s3": "3.0.1",
Expand All @@ -107,21 +123,21 @@
"@cspell/cspell-types": "^6.31.1",
"@nestjs/cli": "9.3.0",
"@nestjs/swagger": "6.2.1",
"@types/base64-stream": "^1.0.5",
"@total-typescript/ts-reset": "^0.5.1",
"@types/base64-stream": "^1.0.5",
"@types/bcrypt": "5.0.0",
"@types/concat-stream": "^2.0.3",
"@types/cookie-session": "^2.0.44",
"@types/deep-diff": "^1.0.5",
"@types/express": "4.17.9",
"@types/jest": "^26.0.19",
"@types/js-base64": "^3.3.1",
"@types/json-stable-stringify": "^1.0.36",
"@types/lodash": "^4.14.191",
"@types/mime": "^3.0.4",
"@types/multer": "^1.4.7",
"@types/multer-s3": "^3.0.0",
"@types/node": "^18.14.6",
"@types/deep-diff": "^1.0.5",
"@types/json-stable-stringify": "^1.0.36",
"@types/object-hash": "^3.0.6",
"@types/passport": "^1.0.12",
"@types/passport-http": "0.3.9",
Expand Down
2 changes: 1 addition & 1 deletion services/workflows-service/prisma/data-migrations
Submodule data-migrations updated from caac99 to b0c26b
1 change: 1 addition & 0 deletions services/workflows-service/prisma/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ datasource db {

generator client {
provider = "prisma-client-js"
previewFeatures = ["tracing"]
}

enum UserStatus {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,7 @@ export class AppLoggerService implements LoggerService, OnModuleDestroy {
}

private getLogMetadata() {
const metadata = {
reqId: undefined,
};

const reqId = this.cls.get('requestId');

if (reqId) {
metadata.reqId = reqId;
}
const metadata = {};

const entity = this.cls.get('entity');

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { trace, context } from '@opentelemetry/api';
import { Injectable, NestMiddleware } from '@nestjs/common';
import { AppLoggerService } from '@/common/app-logger/app-logger.service';
import { NextFunction, Request, Response } from 'express';
Expand All @@ -21,7 +22,7 @@ export class RequestIdMiddleware implements NestMiddleware {

if (isRelevantReq) {
reqMetadata = getReqMetadataObj(req);
this.setReqId(req, res);
this.setTraceId(req, res);
this.logger.debug(`Incoming request`, reqMetadata);
}

Expand Down Expand Up @@ -65,11 +66,19 @@ export class RequestIdMiddleware implements NestMiddleware {
next();
}

private setReqId(req: Request<unknown>, res: Response<unknown>) {
private setTraceId(req: Request<unknown>, res: Response<unknown>) {
const activeSpan = trace.getSpan(context.active());

try {
req.id = randomUUID();
this.cls.set('requestId', req.id);
res.setHeader('X-Request-ID', req.id);
const traceId = activeSpan?.spanContext().traceId;

this.cls.set('traceId', traceId);

if (traceId) {
res.setHeader('X-Request-ID', traceId);
} else {
this.logger.error('traceId is undefined');
}
} catch (e) {
// Mainly for debugging purposes. See https://github.com/Papooch/nestjs-cls/issues/67
this.logger.error('Could not set requestId');
Expand Down
4 changes: 4 additions & 0 deletions services/workflows-service/src/main.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import { tracingSdk } from './traces/tracer';

tracingSdk.start();
liorzam marked this conversation as resolved.
Show resolved Hide resolved

import '@total-typescript/ts-reset';

import passport from 'passport';
Expand Down
9 changes: 8 additions & 1 deletion services/workflows-service/src/sentry/sentry.service.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import api from '@opentelemetry/api';
import { ClsService } from 'nestjs-cls';
import { HttpException, Injectable } from '@nestjs/common';
import * as Sentry from '@sentry/node';
Expand Down Expand Up @@ -39,7 +40,13 @@ export class SentryService {
}

private _setExtraData(scope: Sentry.Scope, request?: Request) {
scope.setExtra('reqId', this.cls.get('requestId'));
MatanYadaev marked this conversation as resolved.
Show resolved Hide resolved
let traceId = this.cls.get('traceId');
if (!traceId) {
const activeSpan = api.trace.getSpan(api.context.active());
traceId = activeSpan?.spanContext().traceId;
}

scope.setExtra('traceId', traceId);
scope.setExtra('assigneeId', this.cls.get('assigneeId'));
scope.setExtra('workflowId', this.cls.get('workflowId'));

Expand Down
66 changes: 66 additions & 0 deletions services/workflows-service/src/traces/tracer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import {
SEMRESATTRS_SERVICE_VERSION,
SEMRESATTRS_SERVICE_NAME,
} from '@opentelemetry/semantic-conventions';
import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-http';
import { Resource } from '@opentelemetry/resources';
import { trace } from '@opentelemetry/api';
import * as opentelemetry from '@opentelemetry/sdk-node';
import { getNodeAutoInstrumentations } from '@opentelemetry/auto-instrumentations-node';
import { dockerCGroupV1Detector } from '@opentelemetry/resource-detector-docker';
import { B3Propagator } from '@opentelemetry/propagator-b3';
import { version } from '../../package.json';
import { WinstonInstrumentation } from '@opentelemetry/instrumentation-winston';
import { NestInstrumentation } from '@opentelemetry/instrumentation-nestjs-core';
import { AwsInstrumentation } from '@opentelemetry/instrumentation-aws-sdk';
import { ExpressInstrumentation } from '@opentelemetry/instrumentation-express';
import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
import { PrismaInstrumentation } from '@prisma/instrumentation';

const traceExporter = new OTLPTraceExporter({
// port configured in the Collector config, defaults to 4317
url: 'http://localhost:4318/v1/traces',
MatanYadaev marked this conversation as resolved.
Show resolved Hide resolved
});

export const getTracer = () => {
return trace.getTracer('default');
};

export const tracingSdk = new opentelemetry.NodeSDK({
traceExporter,
textMapPropagator: new B3Propagator(),
resourceDetectors: [dockerCGroupV1Detector],
resource: Resource.default().merge(
new Resource({
[SEMRESATTRS_SERVICE_NAME]: 'wf-service',
[SEMRESATTRS_SERVICE_VERSION]: version,
}),
),
spanProcessors: [
new opentelemetry.tracing.BatchSpanProcessor(traceExporter, {
maxExportBatchSize: 1000,
maxQueueSize: 1000,
}),
new opentelemetry.tracing.SimpleSpanProcessor(new opentelemetry.tracing.ConsoleSpanExporter()),
],
instrumentations: [
new HttpInstrumentation(),
new WinstonInstrumentation(),
new NestInstrumentation({ enabled: false }),
new AwsInstrumentation(),
new PrismaInstrumentation({ middleware: true }),
new ExpressInstrumentation({
requestHook: (span: any, reqInfo: any) => {
span.setAttribute('request-headers', reqInfo.request.headers);
},
}),
liorzam marked this conversation as resolved.
Show resolved Hide resolved
],
});

// gracefully shut down the SDK on process exit
process.on('SIGTERM', () => {
tracingSdk
.shutdown()
.then(() => console.log('Tracing terminated'))
.catch((error: unknown) => console.error('Error terminating tracing', error));
});
Comment on lines +66 to +71
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using a more robust logging framework instead of console.error for better error tracking and management in production environments.

- .catch((error: unknown) => console.error('Error terminating tracing', error));
+ .catch((error: unknown) => logger.error('Error terminating tracing', error));

Note: This suggestion assumes the existence of a logger object configured elsewhere in your application.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
process.on('SIGTERM', () => {
tracingSdk
.shutdown()
.then(() => console.log('Tracing terminated'))
.catch((error: unknown) => console.error('Error terminating tracing', error));
});
process.on('SIGTERM', () => {
tracingSdk
.shutdown()
.then(() => console.log('Tracing terminated'))
.catch((error: unknown) => logger.error('Error terminating tracing', error));
});

Loading