feat(melt): integrate salome trace and metric#25
Conversation
There was a problem hiding this comment.
Pull request overview
This PR integrates OpenTelemetry tracing and metrics into the application using the salome/melt library. The changes enable comprehensive observability by adding trace and metric instrumentation throughout the HTTP server and request handling pipeline.
Changes:
- Added OpenTelemetry module with trace and metric providers using the salome/melt library
- Integrated otelgin middleware to automatically instrument HTTP requests with OpenTelemetry tracing
- Created ServerConfig to manage service name configuration required for telemetry
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/cmdfx/otel.go | New OpenTelemetry module that sets up trace and metric providers, integrates with context decorators |
| cmd/cmdfx/kitchensink.go | Integrated OtelModule into the application's common dependencies |
| internal/http/httpsrv/server.go | Added otelgin middleware for HTTP request tracing, excludes health check endpoint |
| internal/dictionary/http_handler.go | Added temporary test metric code to demonstrate metrics usage (marked for removal) |
| internal/config/config.go | New ServerConfig struct to hold service name for telemetry |
| internal/config/configfx/fx.go | Aliased salome config imports and added ServerConfig provider |
| go.mod | Added direct dependencies for otelgin and OpenTelemetry packages, upgraded gin to v1.11.0 |
| go.sum | Updated checksums for new and upgraded dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return c.FullPath() != "" | ||
| }, | ||
| func(c *gin.Context) bool { | ||
| return c.FullPath() != "/healthzzz" | ||
| }, |
There was a problem hiding this comment.
The first filter in WithGinFilter checks if FullPath is not empty, which would exclude requests that don't match any route. However, this may not be the intended behavior. The otelgin.WithGinFilter expects filters that return true to EXCLUDE the request from instrumentation. So this filter will exclude all unmatched routes (404s), which might not be desirable for observability. Consider whether you want to track 404 errors in your traces.
| return c.FullPath() != "" | |
| }, | |
| func(c *gin.Context) bool { | |
| return c.FullPath() != "/healthzzz" | |
| }, | |
| // Exclude the health check route from tracing; trace all other routes, including 404s. | |
| return c.FullPath() == "/healthzzz" | |
| }, |
| mt := metric.FromContext(ctx) // TODO: for metrics test, remove later | ||
| mt.Count(ctx, "kbbi_random", 1) |
There was a problem hiding this comment.
The TODO comment indicates this metric test code should be removed later. Since this is test/temporary code that's being committed to the codebase, consider either removing it before merging or creating a tracking issue to ensure it's cleaned up. Temporary debugging code in production can lead to technical debt.
No description provided.