[LFXV2-609] Add OpenTelemetry tracing support #30
Conversation
Integrate slog-otel to automatically include trace_id and span_id from OpenTelemetry context in log output, enabling correlation between logs and distributed traces. Changes: - pkg/log/log.go: Wrap JSON handler with slogotel.OtelHandler - pkg/utils/otel_test.go: Fix TestNewPropagator to pass OTelConfig - charts/: Bump version to 0.4.10 - go.mod: Add github.com/remychantenay/slog-otel v1.3.4 Issue: LFXV2-609 Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: Trevor Bramwell <[email protected]>
WalkthroughThis PR introduces OpenTelemetry SDK integration to the query service. Changes include a Helm chart version bump, new OTEL environment variable configuration in the deployment template and values, Go dependency additions for OpenTelemetry packages, core OTEL setup utilities with support for traces, metrics, and logs, HTTP request instrumentation, and logging enhancements to inject trace context. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant OTel as OTEL SDK
participant Providers as Providers<br/>(Trace/Metric/Log)
participant Exporter as OTLP Exporter
participant Handler as HTTP Handler
App->>OTel: SetupOTelSDKWithConfig(ctx, config)
OTel->>OTel: Create Resource<br/>(service name/version)
OTel->>OTel: Create Propagator<br/>(tracecontext, baggage, jaeger)
OTel->>Providers: Initialize TracerProvider<br/>(with OTLP exporter)
OTel->>Providers: Initialize MeterProvider<br/>(with OTLP exporter)
OTel->>Providers: Initialize LoggerProvider<br/>(with OTLP exporter)
Providers->>Exporter: Configure OTLP transport<br/>(gRPC/HTTP endpoint)
OTel-->>App: Return shutdown function
App->>App: Start HTTP server
App->>Handler: Wrap with otelhttp.NewHandler()
App->>Handler: Incoming HTTP request
Handler->>Providers: Extract/inject span context
Handler->>Providers: Record trace data
Handler-->>App: HTTP response
Providers->>Exporter: Batch send traces/metrics/logs
App->>OTel: Call shutdown function
Providers->>Exporter: Flush pending data
OTel-->>App: Shutdown complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
🧰 Additional context used📓 Path-based instructions (2)**/*_test.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
cmd/main.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧬 Code graph analysis (2)pkg/utils/otel_test.go (1)
cmd/main.go (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (21)
✏️ Tip: You can disable this entire section by setting Comment |
There was a problem hiding this comment.
Pull request overview
This pull request introduces comprehensive OpenTelemetry (OTel) support to the lfx-v2-query-service, enabling distributed tracing, metrics, and log correlation. The implementation includes a new OTel utility package, HTTP instrumentation, enhanced logging with trace context, and declarative Helm chart configuration.
Changes:
- Added OTel SDK setup with configurable exporters for traces, metrics, and logs
- Instrumented HTTP server with automatic span generation for incoming requests
- Enhanced logging to inject trace and span IDs for correlation with distributed traces
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/utils/otel.go |
New utility package providing OTel configuration and SDK initialization with support for OTLP exporters |
pkg/utils/otel_test.go |
Comprehensive test coverage for OTel configuration parsing, SDK setup, and provider functions |
pkg/logging/context.go |
New helpers to extract trace context from context and add to log attributes |
pkg/log/log.go |
Wrapped logging handler with slog-otel to automatically inject trace context into logs |
cmd/main.go |
Initialize OTel SDK on startup with proper shutdown handling |
cmd/http.go |
Wrap HTTP handler with OTel instrumentation for automatic trace span generation |
charts/lfx-v2-query-service/values.yaml |
Added OTel configuration options with sensible defaults |
charts/lfx-v2-query-service/templates/deployment.yaml |
Inject OTel environment variables into deployment based on Helm values |
charts/lfx-v2-query-service/Chart.yaml |
Bump chart version to 0.4.10 |
go.mod, go.sum |
Added OTel SDK dependencies and updated related packages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func LogAttrsFromContext(ctx context.Context) []slog.Attr { | ||
| spanCtx := trace.SpanContextFromContext(ctx) | ||
| if !spanCtx.IsValid() { | ||
| return nil | ||
| } | ||
| return []slog.Attr{ | ||
| slog.String("trace_id", spanCtx.TraceID().String()), | ||
| slog.String("span_id", spanCtx.SpanID().String()), | ||
| } | ||
| } |
There was a problem hiding this comment.
The LogAttrsFromContext function lacks test coverage. Given that other packages in this repository have comprehensive test coverage (e.g., pkg/utils/otel_test.go, pkg/paging/codec_test.go), this function should also have tests covering scenarios like valid span context, invalid span context, and nil context.
| func LogWithContext(ctx context.Context, logger *slog.Logger) *slog.Logger { | ||
| spanCtx := trace.SpanContextFromContext(ctx) | ||
| if !spanCtx.IsValid() { | ||
| return logger | ||
| } | ||
| return logger.With( | ||
| slog.String("trace_id", spanCtx.TraceID().String()), | ||
| slog.String("span_id", spanCtx.SpanID().String()), | ||
| ) | ||
| } |
There was a problem hiding this comment.
The LogWithContext function lacks test coverage. Tests should verify that trace context is correctly added to the logger when a valid span context exists, and that the original logger is returned unchanged when no valid span context is present.
This pull request introduces comprehensive OpenTelemetry (OTel) support to the
lfx-v2-query-service, enabling distributed tracing, metrics, and log correlation for improved observability. It adds OTel configuration options to the Helm chart and application, instruments the HTTP server, and enhances logging with trace context. The most significant changes are grouped below.OpenTelemetry integration and configuration:
pkg/utils/otel.go) that provides OTel configuration via environment variables, sets up OTel SDK (tracing, metrics, logs), and configures exporters and propagators. This enables flexible OTel setup based on deployment needs.values.yamlanddeployment.yaml) to allow configuring OTel environment variables such as service name, protocol, endpoint, exporters, and propagators, making OTel setup declarative and deployment-friendly. [1] [2]0.4.10to reflect these configuration changes.Application instrumentation and startup:
cmd/main.goto initialize the OTel SDK on startup using the new utility, ensuring that all telemetry is exported as configured and that shutdown is handled gracefully. [1] [2]otelhttp.NewHandler) to automatically generate trace spans for incoming HTTP requests. [1] [2]Logging improvements for trace correlation:
slog-otelhandler and a new helper inpkg/logging/context.go, allowing logs to be correlated with distributed traces. [1] [2] [3]Dependency updates:
go.modto include OTel SDKs, exporters, and theslog-otelhandler, ensuring all new functionality is supported.These changes collectively provide robust observability for the service, making it easier to monitor, debug, and analyze system behavior in production environments.