-
Notifications
You must be signed in to change notification settings - Fork 2
[FEATURE] Add support for OpenTelemetry #19
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
base: master
Are you sure you want to change the base?
Conversation
|
@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") |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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) | ||
| } | ||
|
|
There was a problem hiding this comment.
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) | ||
| } | ||
|
|
There was a problem hiding this comment.
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) | ||
| } | ||
|
|
There was a problem hiding this comment.
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 | ||
| } | ||
|
|
There was a problem hiding this comment.
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"), |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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
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:
Before creating pull request, look that all points under checklist are done!