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

Feature/improve instrumentation #179

Merged
merged 6 commits into from
Mar 5, 2025

Conversation

augusto-draxx
Copy link
Contributor

Midaz Console Pull Request Checklist

Pull Request Type

  • UI
  • Core (Business Logic)
  • Security
  • internationalization
  • Pipeline
  • Documentation

Checklist

Please check each item after it's completed.

  • I have tested these changes locally.
  • I have updated the documentation accordingly.
  • I have added necessary comments to the code, especially in complex areas.
  • I have ensured that my changes adhere to the project's coding standards.
  • I have checked for any potential security issues.
  • I have ensured that all tests pass.
  • I have updated the version appropriately (if applicable).
  • I have confirmed this code is ready for review.

Additional Notes

Obs: Please, always remember to target your PR to develop branch instead of main.

@Copilot Copilot bot review requested due to automatic review settings March 5, 2025 15:06
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR enhances instrumentation and observability across the core modules by integrating OpenTelemetry for tracing, metrics, and logs, while also updating tests and logger implementations. Key changes include:

  • Adding and testing a new OtelTracerProvider for custom span handling.
  • Updating the logger repository to output JSON stringified log entries.
  • Expanding container and Next.js configuration to support additional OpenTelemetry instrumentation.

Reviewed Changes

File Description
src/core/infrastructure/observability/otel-tracer-provider.{ts,test.ts} Introduces a new tracer provider and its tests for span creation and termination.
src/core/infrastructure/utils/http-fetch-utils.{ts,test.ts} Injects the tracer provider and uses it to manage spans during HTTP fetch calls.
src/core/infrastructure/observability/instrumentation-config.ts Updates instrumentation configuration to include OpenTelemetry exporters/ instrumentations.
src/core/application/logger/logger-aggregator.ts Augments log events with additional context details in log messages.
src/core/infrastructure/logger/pino-logger-repository.ts Adjusts logger repository to log stringified JSON instead of raw objects.
src/core/infrastructure/container-registry/{observability/otel-module.ts, container-registry.ts} Registers the OtelTracerProvider in dependency injection containers.
next.config.mjs Expands Next.js configuration to support a broader set of instrumentation packages.

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/core/application/logger/logger-aggregator.ts:118

  • [nitpick] Consider renaming 'LogEventDetails' to 'logEventDetails' to follow camelCase naming conventions consistently.
const LogEventDetails = {

@@ -60,26 +60,26 @@ export class PinoLoggerRepository implements LoggerRepository {

info(message: string, context: LogContext, metadata?: LogMetadata): void {
const logEntry = this.createLogEntry('INFO', message, metadata, context)
this.logger.info(logEntry)
this.logger.info(JSON.stringify(logEntry))
Copy link
Preview

Copilot AI Mar 5, 2025

Choose a reason for hiding this comment

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

Using JSON.stringify for logging structured objects may hinder downstream processing that expects structured data. It is recommended to pass the log object directly.

Suggested change
this.logger.info(JSON.stringify(logEntry))
this.logger.info(logEntry)

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@augusto-draxx augusto-draxx merged commit 877ecea into develop Mar 5, 2025
6 of 7 checks passed
@augusto-draxx augusto-draxx deleted the feature/improve-instrumentation branch March 5, 2025 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants