Skip to content

Conversation

@KostLinux
Copy link
Contributor

Description

Add support for OpenTelemetry which is one of the main tracing tools in enterprise applications. The configuration is platform agnostic, which means that any observability platform is supported.

Tested on NewRelic.

Fixes # (issue)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas (optional)
  • I have made corresponding changes to the documentation (optional)
  • My changes generate no new warnings
  • I've performed/added tests/lints that prove my fix is effective or that my feature works (optional)

Before creating pull request, look that all points under checklist are done!

@Matrix278 Matrix278 self-requested a review March 24, 2025 09:06
@Matrix278 Matrix278 self-assigned this Mar 24, 2025
@Matrix278 Matrix278 added the enhancement New feature or request label Mar 24, 2025
@Matrix278
Copy link
Owner

@KostLinux Please make fixes for conflicts. I have updated action workflows and go, linter versions + fixes from them.

// @Router /users/{user_id} [get]
func (controller *user) UserByID(ctx *gin.Context) {
// Validate path params
spanCtx, span := controller.tracer.Start(ctx.Request.Context(), "UserByID")
Copy link
Owner

@Matrix278 Matrix278 Apr 6, 2025

Choose a reason for hiding this comment

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

Lets just call the spanCtx as normal ctx, the way you did within other folders

)
if err != nil {
logger.Fatalf("connecting to database failed. %v", err)
logger.Fatalf("failed to create database connection: %v", err)
Copy link
Owner

Choose a reason for hiding this comment

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

Lets make the same style as before, failed at the end.

creating database connection failed

if args.Get(0) == nil {
return nil, args.Error(1)
}

Copy link
Owner

Choose a reason for hiding this comment

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

Lets leave the newline here after closing }

if args.Get(0) == nil {
return nil, args.Error(1)
}

Copy link
Owner

Choose a reason for hiding this comment

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

Lets leave the newline here after closing }

if args.Get(0) == nil {
return nil, args.Error(1)
}

Copy link
Owner

Choose a reason for hiding this comment

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

Lets leave the newline here after closing }

if errors.Is(err, sql.ErrNoRows) {
return nil, commonerrors.ErrUserNotFound
}

Copy link
Owner

Choose a reason for hiding this comment

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

Lets leave the newline here after closing }

return &user{
db: db,
db: db,
tracer: otel.Tracer("repository/user"),
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, so we always should pass it like that where we are tracing?


// Enable runtime metrics
if err := runtime.Start(runtime.WithMinimumReadMemStatsInterval(time.Second)); err != nil {
return nil, fmt.Errorf("failed to start runtime metrics: %w", err)
Copy link
Owner

@Matrix278 Matrix278 Apr 6, 2025

Choose a reason for hiding this comment

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

Lets use the logger here logger.Errorf and place failed at the end.

starting runtime metrics failed

client := otlptracegrpc.NewClient(opts...)
traceExporter, err := otlptrace.New(ctx, client)
if err != nil {
return nil, fmt.Errorf("failed to create trace exporter: %w", err)
Copy link
Owner

Choose a reason for hiding this comment

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

Lets use the logger here logger.Errorf and place failed at the end.

creating trace exporter failed

),
)
if err != nil {
return nil, fmt.Errorf("failed to create Otel SDK resource: %w", err)
Copy link
Owner

Choose a reason for hiding this comment

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

Lets use the logger here logger.Errorf and place failed at the end.

creating Otel SDK resource failed

ctx := context.Background()
shutdown, err := newOtelCollector(ctx, cfg)
if err != nil {
logger.Fatalf("failed to setup OpenTelemetry: %v", err)
Copy link
Owner

Choose a reason for hiding this comment

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

Lets place failed at the end.

setuping OpenTelemetry failed

return func() {
logger.Infof("Shutting down OpenTelemetry")
if err := shutdown(context.Background()); err != nil {
logger.Errorf("failed to shutdown OpenTelemetry: %v", err)
Copy link
Owner

Choose a reason for hiding this comment

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

Lets place failed at the end.

shutdowning OpenTelemetry failed

otel.Endpoint = cfg.Telemetry.Endpoint
otel.Headers = cfg.Telemetry.Headers

cleanup := telemetry.InitTracer(otel)
Copy link
Owner

Choose a reason for hiding this comment

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

Lets call it tracerCleanup

}

// TelemetryNew creates a new Telemetry configuration from environment
func TelemetryNew() *Telemetry {
Copy link
Owner

Choose a reason for hiding this comment

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

Lets call it NewTelemetry() or NewOpenTelemetry() even

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants