-
Notifications
You must be signed in to change notification settings - Fork 58
chore(server): improve the code related to otel #1938
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
Conversation
31e23e5 to
0a78f1e
Compare
0a78f1e to
03f54bf
Compare
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.
Pull request overview
This PR refactors the OpenTelemetry tracing implementation by replacing the Google Cloud Platform-specific trace exporter with a more flexible OTLP (OpenTelemetry Protocol) exporter. The changes introduce support for multiple exporter types (GCP, Jaeger, and OTLP) configurable via environment variables, providing better flexibility for different deployment environments.
Key changes:
- Replaced GCP-specific trace exporter with OTLP exporter that supports multiple backends
- Added comprehensive environment variable configuration for OpenTelemetry settings
- Restructured tracing code into a dedicated
otelpackage with improved separation of concerns
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| server/internal/app/otel/otel.go | New package implementing OTLP-based tracer initialization with support for GCP, Jaeger, and generic OTLP exporters |
| server/internal/app/otel/middleware.go | New middleware wrapper for Echo framework with path-based trace skipping |
| server/internal/app/config/config.go | Added OpenTelemetry configuration fields for endpoint, exporter type, batching, and sampling |
| server/internal/app/main.go | Updated tracer initialization to use new otel package with configuration from environment variables |
| server/internal/app/app.go | Modified to conditionally apply OpenTelemetry middleware based on configuration |
| server/internal/app/server.go | Added service name differentiation for main API vs internal API traces |
| server/internal/app/tracer.go | Removed legacy GCP and Jaeger tracer initialization code |
| server/go.mod, server/go.sum | Updated OpenTelemetry dependencies to v1.38.0 and added OTLP exporter packages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wilfredmulenga
left a comment
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.
Looks good
Overview
Google exportertoOTLP exporterWhat I've done
What I haven't done
How I tested
Which point I want you to review particularly
Memo
Checklist