feat: otel support #2236
feat: otel support #2236
Conversation
|
WalkthroughThe changes introduce OpenTelemetry-based tracing to the workflows service, enhancing observability by integrating Jaeger for distributed tracing. Key updates include setting up tracing in the Prisma schema, initializing tracing in the main application, and configuring Docker Compose for Jaeger. Additionally, the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant Service
participant TracingSDK
participant Jaeger
participant Sentry
User->>Service: Send Request
activate Service
Service->>TracingSDK: Start Trace
activate TracingSDK
TracingSDK->>Jaeger: Export Trace Data
deactivate TracingSDK
Service->>Sentry: Log with TraceID
deactivate Service
Sentry->>Jaeger: Correlate TraceID
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
PR Description updated to latest commit (bd87c13) |
PR Review
Code feedback:
✨ Review tool usage guide:Overview: With a configuration file, use the following template:
See the review usage page for a comprehensive guide on using this tool. |
PR Code Suggestions
✨ Improve tool usage guide:Overview: With a configuration file, use the following template:
See the improve usage page for a more comprehensive guide on using this tool. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
pnpm-lock.yamlis excluded by:!**/*.yamlservices/workflows-service/docker-compose.otel.ymlis excluded by:!**/*.ymlservices/workflows-service/package.jsonis excluded by:!**/*.json
Files selected for processing (7)
- services/workflows-service/prisma/data-migrations (1 hunks)
- services/workflows-service/prisma/schema.prisma (1 hunks)
- services/workflows-service/src/common/app-logger/app-logger.service.ts (1 hunks)
- services/workflows-service/src/common/middlewares/request-id.middleware.ts (3 hunks)
- services/workflows-service/src/main.ts (1 hunks)
- services/workflows-service/src/sentry/sentry.service.ts (2 hunks)
- services/workflows-service/src/traces/tracer.ts (1 hunks)
Additional comments: 7
services/workflows-service/prisma/data-migrations (1)
- 1-1: The update to the subproject commit reference is noted. Ensure that the new commit (
b0c26b898feb0705286b4d7f0266bcd740062ee6) has been thoroughly tested for stability and compatibility with the main project.services/workflows-service/src/common/app-logger/app-logger.service.ts (1)
- 41-41: The simplification of the
getLogMetadatamethod in theAppLoggerServiceclass is noted. Ensure that the simplification has not removed any necessary functionality or error handling that might be required for correct operation.services/workflows-service/src/sentry/sentry.service.ts (1)
- 43-49: The addition of logic to retrieve and set
traceIdin theSentryServiceclass using OpenTelemetry API is a good enhancement for observability. Ensure that the security implications of exposing trace IDs have been considered and mitigated.services/workflows-service/src/traces/tracer.ts (1)
- 20-66: The setup of OpenTelemetry tracing in the
tracer.tsfile is comprehensive and well-structured. Ensure that the performance implications of the tracing configuration, especially the batch span processor settings, have been reviewed and optimized for the application's needs.services/workflows-service/src/common/middlewares/request-id.middleware.ts (1)
- 69-81: The updates to use OpenTelemetry trace IDs in the
RequestIdMiddlewareclass, including setting theX-Request-IDheader and adding error logging for undefined trace IDs, align well with the goal of enhancing observability. Ensure that the error handling for undefined trace IDs is robust and that any potential security implications of exposing trace IDs are considered.services/workflows-service/src/main.ts (1)
- 1-4: Starting the OpenTelemetry tracing SDK before other imports in
main.tsis a good practice for ensuring comprehensive tracing coverage. Ensure that this change does not introduce any unintended side effects, especially in terms of the order of imports and initialization.services/workflows-service/prisma/schema.prisma (1)
- 8-8: Enabling the
tracingpreview feature in the Prisma client generator configuration is a positive step towards enhancing observability. Ensure that the stability and compatibility of this feature have been thoroughly tested with the project's requirements.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- services/workflows-service/src/sentry/sentry.service.ts (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- services/workflows-service/src/sentry/sentry.service.ts
…-otel-traces-support
There was a problem hiding this comment.
Actionable comments posted: 1
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!pnpm-lock.yaml
Files selected for processing (2)
- services/workflows-service/prisma/schema.prisma (1 hunks)
- services/workflows-service/src/main.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- services/workflows-service/prisma/schema.prisma
Additional Context Used
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- services/workflows-service/docker-compose.otel.yml (1 hunks)
Additional Context Used
Additional comments not posted (1)
services/workflows-service/docker-compose.otel.yml (1)
1-15: The Docker Compose configuration for the Jaeger tracing service is well-defined, with a specific version of the Jaeger image being used (jaegertracing/all-in-one:1.55) and a comprehensive set of ports exposed for its operation. This setup aligns with the objective of enhancing observability through OpenTelemetry integration for development purposes. The use of a specific version addresses previous concerns about stability and consistency with the production environment.
There was a problem hiding this comment.
Actionable comments posted: 2
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- services/workflows-service/src/traces/tracer.ts (1 hunks)
Additional Context Used
Additional comments not posted (2)
services/workflows-service/src/traces/tracer.ts (2)
22-24: ThegetTracerfunction is correctly implemented to retrieve the default tracer. This is a straightforward and standard approach in OpenTelemetry SDK usage.
26-55: The tracing SDK setup includes a comprehensive configuration that covers exporter, propagator, resource detectors, resources, span processors, and instrumentations. A few points to consider:
- Ensure that the
B3Propagatoraligns with the tracing system and infrastructure used in the rest of your environment.- The use of
dockerCGroupV1Detectoras a resource detector is a good practice for containerized environments. Verify that it aligns with your deployment strategy.- The
SimpleSpanProcessorwith aConsoleSpanExportermight be useful for debugging but consider removing or disabling it in production environments to avoid performance impacts.- The
requestHookinExpressInstrumentationfor capturing request headers can potentially capture sensitive information. Ensure that this does not violate privacy policies or security guidelines.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!pnpm-lock.yaml
Files selected for processing (4)
- services/workflows-service/package.json (1 hunks)
- services/workflows-service/prisma/data-migrations (1 hunks)
- services/workflows-service/prisma/schema.prisma (1 hunks)
- services/workflows-service/src/common/app-logger/app-logger.service.ts (1 hunks)
Files skipped from review due to trivial changes (3)
- services/workflows-service/prisma/data-migrations
- services/workflows-service/prisma/schema.prisma
- services/workflows-service/src/common/app-logger/app-logger.service.ts
Additional comments not posted (16)
services/workflows-service/package.json (16)
63-63: Ensure compatibility of@opentelemetry/apiwith other OpenTelemetry packages.
64-64: Check if@opentelemetry/auto-instrumentations-nodeis the latest stable version and compatible with the Node.js version used in the project.
65-65: Confirm that@opentelemetry/exporter-jaegerintegrates smoothly with the Jaeger setup specified in the Docker Compose file.
66-66: Validate the version and configuration of@opentelemetry/exporter-trace-otlp-httpto ensure it meets the project's requirements.
67-67: Ensure that@opentelemetry/exporter-trace-otlp-protois configured correctly and compatible with the project's tracing infrastructure.Verification successful
The verification process has successfully located the tracing configuration file and confirmed the use of the OTLP exporter within the project's tracing infrastructure. Here are the details:
- The tracing configuration is managed in the file
services/workflows-service/src/traces/tracer.ts.- This file includes the import and instantiation of
OTLPTraceExporterfrom the@opentelemetry/exporter-trace-otlp-httppackage.Given this information, it appears that the project is indeed configured to use an OTLP exporter, specifically the HTTP variant, which is compatible with the
@opentelemetry/exporter-trace-otlp-protopackage mentioned in the review comment.
- File:
services/workflows-service/src/traces/tracer.ts
- Import statement:
import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-http';- Usage:
const traceExporter = new OTLPTraceExporter();Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the configuration and compatibility of the OTLP Proto exporter. # Test: Check the configuration in the tracing setup. grep 'OTLPProtoExporter' src/traces/tracer.tsLength of output: 99
Script:
#!/bin/bash # Description: Locate the correct tracing configuration file and verify OTLP exporter configuration. # Find files related to tracing configuration echo "Searching for tracing configuration files:" fd --type f "tracer" src # Check for mentions of OTLP in the codebase echo "Searching for mentions of OTLP in the codebase:" rg "OTLP" srcLength of output: 373
Script:
#!/bin/bash # Description: Broad search for tracing configuration files and mentions of OTLP in the entire repository. # Find files related to tracing configuration in the entire repository echo "Searching for tracing configuration files in the entire repository:" fd --type f "tracer" # Check for mentions of OTLP in the entire repository echo "Searching for mentions of OTLP in the entire repository:" rg "OTLP"Length of output: 566
68-68: Check for any known issues with@opentelemetry/instrumentation-aws-sdkversion0.39.1regarding AWS SDK calls.
69-69: Verify that@opentelemetry/instrumentation-expressis the appropriate version for the project's Express framework version.
70-70: Ensure@opentelemetry/instrumentation-httpis up-to-date and compatible with the project's HTTP handling.
71-71: Confirm that@opentelemetry/instrumentation-nestjs-coreintegrates well with the project's NestJS version.
72-72: Check if@opentelemetry/instrumentation-winstonis compatible with the project's Winston logging setup.
73-73: Ensure@opentelemetry/propagator-b3is correctly configured for use in the project.Verification successful
The verification process has confirmed that the
@opentelemetry/propagator-b3dependency is indeed configured for use in the project. The script found an import statement and a configuration line usingB3Propagatorin a file related to tracing, which matches the dependency mentioned in thepackage.json.Configuration of
@opentelemetry/propagator-b3verified successfully.
- The dependency is imported and configured correctly in the project's tracing setup.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the configuration of the B3 propagator in the tracing setup. # Test: Search for the B3 propagator configuration in the codebase. grep 'B3Propagator' src/traces/tracer.tsLength of output: 94
Script:
#!/bin/bash # Description: Locate and verify the configuration of the B3 propagator in the tracing setup. # Step 1: Find files related to tracing configuration fd 'tracer\.ts' --exec grep 'B3Propagator' {}Length of output: 149
74-74: Verify the integration of@opentelemetry/resource-detector-dockerwith the Docker environment specified in the project.
75-75: Confirm that@opentelemetry/resourcesis utilized effectively in the project's resource management.Verification successful
The verification process has confirmed that the
@opentelemetry/resourcespackage is indeed utilized within the project. The filetracer.ts, which was initially not found due to an incorrect path, is located atservices/workflows-service/src/traces/tracer.tsand it includes references to@opentelemetry/resources. This confirms the effective use of the package in the project's resource management.
- The file
tracer.tsis correctly located and uses@opentelemetry/resources.- The package is also referenced in
services/workflows-service/package.jsonandpnpm-lock.yaml, indicating it is properly integrated and managed within the project's dependencies.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the use of OpenTelemetry resources in the project. # Test: Search for the usage of OpenTelemetry resources in the codebase. grep 'Resources' src/traces/tracer.tsLength of output: 91
Script:
#!/bin/bash # Description: Search for the file `tracer.ts` and usage of `@opentelemetry/resources` across the repository. # Find the file `tracer.ts` in the repository echo "Searching for tracer.ts in the repository:" fd tracer.ts # Search for the usage of `@opentelemetry/resources` in the codebase echo "Searching for usage of @opentelemetry/resources:" rg '@opentelemetry/resources' --files-with-matchesLength of output: 418
76-76: Check that@opentelemetry/sdk-nodeis set up correctly and functioning as expected in the project.
77-77: Ensure that@opentelemetry/semantic-conventionsare adhered to in the project's tracing and logging.
79-79: Verify that@prisma/instrumentationis correctly integrated and functioning for database query tracing.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (3)
services/workflows-service/src/common/app-logger/app-logger.service.ts (2)
Line range hint
9-9: Decorators are not valid here. Please check the context or remove if misplaced.
Line range hint
29-29: Avoid usinganytype for better type safety.- const payload: any = { ...this.getLogMetadata(), logData }; + const payload: LogPayload = { ...this.getLogMetadata(), logData };services/workflows-service/src/sentry/sentry.service.ts (1)
Line range hint
73-75: Avoid usinganytype to ensure type safety.- let user = this.cls.get('entity'); - if (!user && request) { - if (request.user) { - user = request.user; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } else if ((request as any).tokenScope) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - user = (request as any).tokenScope; - } - } + let user = this.cls.get('entity') || request?.user || request?.tokenScope;Tools
Biome
[error] 1-2: All these imports are only used as types.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!pnpm-lock.yaml
Files selected for processing (6)
- services/workflows-service/package.json (1 hunks)
- services/workflows-service/prisma/data-migrations (1 hunks)
- services/workflows-service/prisma/schema.prisma (1 hunks)
- services/workflows-service/src/common/app-logger/app-logger.service.ts (1 hunks)
- services/workflows-service/src/sentry/sentry.service.ts (2 hunks)
- services/workflows-service/src/traces/tracer.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- services/workflows-service/prisma/data-migrations
- services/workflows-service/prisma/schema.prisma
Additional context used
Biome
services/workflows-service/src/common/app-logger/app-logger.service.ts
[error] 9-9: Decorators are not valid here.
[error] 29-29: Unexpected any. Specify a different type.
[error] 1-1: All these imports are only used as types.
[error] 1-2: Some named imports are only used as types.
[error] 2-3: All these imports are only used as types.
services/workflows-service/src/sentry/sentry.service.ts
[error] 73-73: Unexpected any. Specify a different type.
[error] 75-75: Unexpected any. Specify a different type.
[error] 1-2: All these imports are only used as types.
services/workflows-service/src/traces/tracer.ts
[error] 50-50: Unexpected any. Specify a different type.
[error] 50-50: Unexpected any. Specify a different type.
Additional comments not posted (3)
services/workflows-service/src/common/app-logger/app-logger.service.ts (1)
47-47: Ensure the newentitylogic aligns with intended use cases.services/workflows-service/src/sentry/sentry.service.ts (1)
47-53: Ensure trace ID retrieval and setting are robust and handle possible null values effectively.services/workflows-service/package.json (1)
63-77: The new dependencies are appropriate for the OpenTelemetry integration.
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!pnpm-lock.yaml
Files selected for processing (3)
- services/workflows-service/package.json (1 hunks)
- services/workflows-service/src/traces/http-intrumentation.ts (1 hunks)
- services/workflows-service/src/traces/tracer.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- services/workflows-service/package.json
Additional context used
Biome
services/workflows-service/src/traces/http-intrumentation.ts
[error] 2-2: A Node.js builtin module should be imported with the node: protocol.
[error] 1-2: All these imports are only used as types.
services/workflows-service/src/traces/tracer.ts
[error] 49-49: Unexpected any. Specify a different type.
[error] 49-49: Unexpected any. Specify a different type.
Additional comments not posted (3)
services/workflows-service/src/traces/tracer.ts (3)
19-19: Ensure theOTEL_EXPORTER_OTLP_TRACES_ENDPOINTenvironment variable is utilized for the endpoint configuration.
49-52: Avoid usinganytype inrequestHookto ensure type safety.Tools
Biome
[error] 49-49: Unexpected any. Specify a different type.
[error] 49-49: Unexpected any. Specify a different type.
57-62: Consider using a more robust logging framework instead ofconsole.errorfor better error tracking and management in production environments.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…o/ballerine into bal-1657-otel-traces-support
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
services/workflows-service/src/traces/http-intrumentation.ts (1)
4-18: Consider adding type annotations to improve code readability and maintainability.const createFilter = (filters: RegExp[] | string[]): (url: string) => boolean => { // Function body remains the same };
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!pnpm-lock.yaml
Files selected for processing (2)
- services/workflows-service/src/traces/http-intrumentation.ts (1 hunks)
- services/workflows-service/src/traces/tracer.ts (1 hunks)
Additional context used
Biome
services/workflows-service/src/traces/http-intrumentation.ts
[error] 1-2: All these imports are only used as types.
services/workflows-service/src/traces/tracer.ts
[error] 52-52: Unexpected any. Specify a different type.
[error] 52-52: Unexpected any. Specify a different type.
Additional comments not posted (2)
services/workflows-service/src/traces/tracer.ts (2)
22-24: The implementation ofgetTracerfunction is correct and concise.
26-57: The configuration of the OpenTelemetry SDK intracingSdkis comprehensive and well-implemented.Tools
Biome
[error] 52-52: Unexpected any. Specify a different type.
[error] 52-52: Unexpected any. Specify a different type.
| @@ -0,0 +1,33 @@ | |||
| import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; | |||
| import { IncomingMessage } from 'node:http'; | |||
There was a problem hiding this comment.
Use import type for imports that are only used for type information to optimize TypeScript compilation.
- import { IncomingMessage } from 'node:http';
+ import type { IncomingMessage } from 'node:http';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.
| import { IncomingMessage } from 'node:http'; | |
| import type { IncomingMessage } from 'node:http'; |
Tools
Biome
[error] 1-2: All these imports are only used as types.
| process.on('SIGTERM', () => { | ||
| tracingSdk | ||
| .shutdown() | ||
| .then(() => console.log('Tracing terminated')) | ||
| .catch((error: unknown) => console.error('Error terminating tracing', error)); | ||
| }); |
There was a problem hiding this comment.
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.
| 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)); | |
| }); |
…o/ballerine into bal-1657-otel-traces-support
User description
Description
Elaborate on the subject, motivation, and context.
Related issues
Breaking changes
How these changes were tested
Examples and references
Checklist
Type
enhancement
Description
Changes walkthrough
tracer.ts
Integrate OpenTelemetry Tracing with OTLP Exporterservices/workflows-service/src/traces/tracer.ts
for various services including HTTP, Winston, NestJS, AWS SDK, Prisma,
and Express.
request-id.middleware.ts
Use OpenTelemetry Trace ID in Request ID Middlewareservices/workflows-service/src/common/middlewares/request-id.middleware.ts
setReqIdwithsetTraceIdto use trace ID from OpenTelemetryspans.
sentry.service.ts
Enhance Sentry Integration with OpenTelemetry Trace IDservices/workflows-service/src/sentry/sentry.service.ts
tracking.
app-logger.service.ts
Simplify Log Metadata Retrievalservices/workflows-service/src/common/app-logger/app-logger.service.ts
main.ts
Initialize OpenTelemetry Tracing on Startupservices/workflows-service/src/main.ts
schema.prisma
Enable Tracing Feature in Prisma Schemaservices/workflows-service/prisma/schema.prisma
package.json
Add OpenTelemetry Dependencies for Tracingservices/workflows-service/package.json
instrumentation.
docker-compose.otel.yml
Setup Docker Compose for Jaeger Tracing Environmentservices/workflows-service/docker-compose.otel.yml
with Jaeger.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
AppLoggerServicefor more efficient log handling.Enhancements