[LFXV2-977] Add OpenTelemetry tracing support #41
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: - internal/log/log.go: Wrap JSON handler with slogotel.OtelHandler - pkg/utils/otel_test.go: Fix TestNewPropagator to pass OTelConfig - charts/: Bump version to 0.5.5 - go.mod: Add github.com/remychantenay/slog-otel v1.3.4 Issue: LFXV2-977 Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: Trevor Bramwell <[email protected]>
WalkthroughThe changes introduce OpenTelemetry instrumentation to the application, adding SDK setup utilities, Helm chart configuration for OTEL variables, HTTP handler tracing, and distributed trace context integration with structured logging. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HTTPHandler as HTTP Handler<br/>(otelhttp wrapped)
participant OTEL as OTEL SDK
participant Exporter as OTLP Exporter
participant Logger as Structured Logger<br/>(trace-aware)
Client->>HTTPHandler: HTTP Request
activate HTTPHandler
HTTPHandler->>OTEL: Create Span & Track
activate OTEL
OTEL->>Exporter: Queue Telemetry Data
HTTPHandler->>Logger: Log with Context
activate Logger
Logger->>OTEL: Extract trace_id, span_id
Logger->>Logger: Inject into log attributes
Logger-->>HTTPHandler: Enhanced Log Record
deactivate Logger
OTEL-->>HTTPHandler: Span Complete
deactivate OTEL
HTTPHandler-->>Client: HTTP Response
deactivate HTTPHandler
Exporter->>Exporter: Batch & Send to OTLP Endpoint
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Comment |
There was a problem hiding this comment.
Pull request overview
This pull request adds comprehensive OpenTelemetry (OTel) tracing, metrics, and logging support to the lfx-v2-project-service, enabling distributed tracing and improved observability. The changes include OTel SDK integration, trace-log correlation, and enhanced Helm chart configuration.
Changes:
- Added OTel SDK initialization and graceful shutdown in the main application with configurable exporters for traces, metrics, and logs
- Integrated automatic HTTP request tracing via
otelhttpmiddleware and enhanced structured logging with trace context injection usingslog-otel - Extended Helm chart with comprehensive OTel configuration options and support for arbitrary environment variables
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/utils/otel.go | New utility providing OTel SDK setup with configurable OTLP exporters for traces, metrics, and logs supporting both gRPC and HTTP protocols |
| pkg/utils/otel_test.go | Comprehensive test coverage for OTel configuration parsing, SDK initialization, and utility functions |
| pkg/logging/context.go | New utility functions to extract and inject trace/span IDs into log messages for trace-log correlation |
| internal/log/log.go | Updated structured logging to wrap handler with slog-otel for automatic trace context injection |
| cmd/project-api/main.go | Integrated OTel SDK initialization with graceful shutdown and added otelhttp middleware for automatic request tracing |
| charts/lfx-v2-project-service/values.yaml | Added OTel configuration values and extraEnv support for flexible deployment configuration |
| charts/lfx-v2-project-service/templates/deployment.yaml | Added conditional environment variable injection for OTel configuration |
| charts/lfx-v2-project-service/Chart.yaml | Bumped chart version from 0.5.4 to 0.5.5 |
| go.mod | Added required OTel dependencies and related packages |
| go.sum | Updated checksums for new dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cfg := OTelConfig{ | ||
| ServiceName: "test-service", | ||
| ServiceVersion: "1.0.0", | ||
| Protocol: OTelProtocolGRPC, | ||
| TracesExporter: OTelExporterNone, | ||
| TracesSampleRatio: 1.0, | ||
| MetricsExporter: OTelExporterNone, | ||
| LogsExporter: OTelExporterNone, | ||
| } |
There was a problem hiding this comment.
The test configuration is missing the Propagators field. While the SDK will use defaults when this field is empty, the test should explicitly set it to match the actual usage pattern and ensure all configuration fields are properly tested.
| // Copyright The Linux Foundation and each contributor to LFX. | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| package logging | ||
|
|
||
| import ( | ||
| "context" | ||
| "log/slog" | ||
|
|
||
| "go.opentelemetry.io/otel/trace" | ||
| ) | ||
|
|
||
| // LogAttrsFromContext extracts trace_id and span_id from the context and returns them as slog attributes. | ||
| // Use this to add tracing context to log messages for correlation. | ||
| 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()), | ||
| } | ||
| } | ||
|
|
||
| // LogWithContext returns a logger with trace context attributes added. | ||
| // Usage: logging.LogWithContext(ctx, slog.Default()).Info("message", "key", "value") | ||
| 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 logging context utilities lack test coverage. Tests should verify that LogAttrsFromContext and LogWithContext correctly extract trace and span IDs from valid contexts and return nil/original logger when the context has no valid span.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@internal/log/log.go`:
- Around line 102-110: The function sets the default to the fully wrapped
handler (otelHandler wrapped by contextHandler stored in variable logger) but
currently returns the raw handler h; change the return to return the wrapped
handler (return logger) so the function's slog.Handler return value matches what
was set via slog.SetDefault, ensuring consistency between the configured default
and the function's return.
In `@pkg/utils/otel_test.go`:
- Around line 13-43: The test TestOTelConfigFromEnv_Defaults omits an assertion
for the Propagators default; update the test to assert that
OTelConfigFromEnv().Propagators equals "tracecontext,baggage" (use the existing
cfg variable) so the default Propagators value is verified alongside the other
fields.
- Around line 47-87: TestOTelConfigFromEnv_CustomValues is missing coverage for
OTEL_PROPAGATORS; set t.Setenv("OTEL_PROPAGATORS", "<expected-value>") (e.g.
"tracecontext,baggage") before calling OTelConfigFromEnv() and add an assertion
that cfg.Propagators equals that expected string (compare as a string against
cfg.Propagators) so the test verifies propagators parsing in OTelConfigFromEnv.
🧹 Nitpick comments (3)
pkg/logging/context.go (1)
15-24: Consider reducing duplication between these two functions.Both
LogAttrsFromContextandLogWithContextduplicate the span context extraction and attribute creation logic. You could haveLogWithContextcallLogAttrsFromContextinternally to keep attribute names/logic in sync:♻️ Suggested refactor
func LogWithContext(ctx context.Context, logger *slog.Logger) *slog.Logger { - spanCtx := trace.SpanContextFromContext(ctx) - if !spanCtx.IsValid() { + attrs := LogAttrsFromContext(ctx) + if attrs == nil { return logger } - return logger.With( - slog.String("trace_id", spanCtx.TraceID().String()), - slog.String("span_id", spanCtx.SpanID().String()), - ) + args := make([]any, len(attrs)) + for i, attr := range attrs { + args[i] = attr + } + return logger.With(args...) }Also applies to: 28-37
pkg/utils/otel.go (2)
316-351: Metrics provider with periodic reader.The 30-second interval for the periodic reader is reasonable for metrics collection. Consider making this configurable via
OTelConfigif different environments need different flush intervals.
281-299: Consider extracting common exporter option logic.The HTTP/gRPC branching with endpoint and insecure options is duplicated across all three provider functions. If this pattern grows, consider a helper to reduce duplication. Current implementation is acceptable given the type differences between exporters.
Also applies to: 320-338, 358-376
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
charts/lfx-v2-project-service/Chart.yamlcharts/lfx-v2-project-service/templates/deployment.yamlcharts/lfx-v2-project-service/values.yamlcmd/project-api/main.gogo.modinternal/log/log.gopkg/logging/context.gopkg/utils/otel.gopkg/utils/otel_test.go
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure code formatting and linting pass (make fmt, make lint, make check)
Files:
internal/log/log.gocmd/project-api/main.gopkg/utils/otel_test.gopkg/logging/context.gopkg/utils/otel.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: Place unit tests alongside implementation with the same filename and *_test.go suffix
Each production function must have exactly one corresponding TestFunction test (table with multiple cases allowed)
Use table-driven tests for coverage (subtests over a single TestFunction)
Mock all external dependencies (e.g., repositories, message builders) in unit tests
Focus unit tests on exported package functions
Files:
pkg/utils/otel_test.go
go.mod
📄 CodeRabbit inference engine (CLAUDE.md)
Set module Go version to 1.23+ in go.mod
Files:
go.mod
🧬 Code graph analysis (2)
cmd/project-api/main.go (1)
pkg/utils/otel.go (2)
OTelConfigFromEnv(84-161)SetupOTelSDKWithConfig(171-239)
pkg/utils/otel_test.go (2)
pkg/utils/otel.go (7)
OTelConfigFromEnv(84-161)OTelProtocolGRPC(34-34)OTelExporterNone(41-41)OTelProtocolHTTP(36-36)OTelExporterOTLP(39-39)OTelConfig(45-80)SetupOTelSDKWithConfig(171-239)api/project/v1/gen/project_service/service.go (1)
ServiceName(55-55)
⏰ 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). (4)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (go)
- GitHub Check: Build and Test
- GitHub Check: MegaLinter
🔇 Additional comments (19)
charts/lfx-v2-project-service/Chart.yaml (1)
8-8: LGTM!The chart version bump to 0.5.5 is appropriate for the OpenTelemetry configuration additions.
internal/log/log.go (1)
12-13: LGTM!The slog-otel import is correctly added for trace context integration.
charts/lfx-v2-project-service/values.yaml (2)
52-54: LGTM!The
extraEnvconfiguration provides flexibility for injecting additional environment variables without modifying the chart.
55-88: Well-documented OTel configuration with safe defaults.The configuration properly defaults exporters to "none", ensuring existing deployments aren't affected. The inline documentation is comprehensive and helpful for operators configuring the service.
cmd/project-api/main.go (2)
51-63: LGTM! Proper OTel SDK lifecycle management.The setup correctly:
- Initializes OTel after logging is configured
- Exits on initialization failure
- Uses a fresh
context.Background()in the deferred shutdown to ensure cleanup even if the main context is cancelled
219-220: Correct placement of OTel HTTP instrumentation.Wrapping the handler last makes
otelhttp.NewHandlerthe outermost middleware, ensuring traces capture the full request lifecycle including all middleware processing. The operation name "project-service" provides clear identification in distributed traces.charts/lfx-v2-project-service/templates/deployment.yaml (2)
40-42: LGTM!The
extraEnvrendering correctly useswithto avoid rendering when empty and properly indents the YAML.
43-82: Consider the behavior of conditionals with non-empty default values.Several OTel values have non-empty defaults in
values.yaml(e.g.,protocol: "grpc",insecure: "false",tracesExporter: "none"). The{{- if ... }}conditions will evaluate to true for these, meaning these env vars will always be rendered.This is likely intentional, but if you want the ability to completely omit certain env vars (to rely on SDK defaults), you may need to add a separate enable flag or use empty strings as defaults.
As currently designed, this works correctly for most use cases.
pkg/utils/otel_test.go (3)
104-131: Good test coverage for disabled exporters scenario.This test validates the SDK can initialize and shutdown cleanly when all exporters are disabled. Consider adding a test for enabled exporters (with mock/no-op endpoint) to verify the full initialization path.
135-176: Well-structured table-driven test for resource creation.Good use of table-driven tests per coding guidelines. The test cases cover basic, empty version, and special character scenarios.
180-210: Propagator test validates expected W3C fields.The test correctly verifies the composite propagator includes
traceparent,tracestate, andbaggagefields.go.mod (2)
15-31: OpenTelemetry dependencies added correctly.The OTel dependency versions are internally consistent (v1.39.0 for stable APIs, v0.15.0 for experimental log APIs). The instrumentation and propagator versions align with the core SDK.
21-22: OpenTelemetry logs SDK is experimental (v0.x).The
otlplogexporters andsdk/logpackages are at v0.15.0, indicating experimental/beta APIs. Be aware that breaking changes may occur in minor version updates until v1.0.0 is released.Also applies to: 27-27, 29-29
pkg/utils/otel.go (6)
32-80: Well-documented configuration constants and struct.The constants and
OTelConfigstruct are clearly documented with environment variable mappings and defaults. This makes the API self-documenting.
136-147: Debug log statement is a no-op.
slog.With(...).Debug("OTelConfig")correctly creates an attributed logger and logs a message. However, consider usingslog.Debug("OTelConfig", ...)directly for clarity, or verify this is the intended pattern for structured logging in this codebase.
171-239: Solid SDK setup with proper cleanup orchestration.The shutdown aggregation pattern and
handleErrhelper ensure proper cleanup on partial failures. The conditional provider initialization based on exporter configuration is correct.Note: Calling
SetupOTelSDKWithConfigmultiple times will overwrite global providers without cleaning up previous ones. If idempotency is needed, consider tracking initialization state or documenting single-call usage.
255-274: Good defensive fallback for propagator configuration.The function handles unknown propagators gracefully by logging a warning and falling back to defaults if no valid propagators are configured.
277-313: Trace provider setup with configurable sampling.The trace provider correctly applies the sampling ratio and uses batching with a 1-second timeout. The HTTP/gRPC protocol branching is clean.
354-387: Logger provider uses batch processing.The logger provider follows the same pattern as other providers. Batch processing is appropriate for log export efficiency.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
|
||
| // Wrap with slog-otel handler to add trace_id and span_id from context | ||
| otelHandler := slogotel.OtelHandler{Next: h} | ||
|
|
||
| // Wrap with contextHandler to support context-based attributes | ||
| logger := contextHandler{otelHandler} | ||
| slog.SetDefault(slog.New(logger)) | ||
|
|
||
| return h |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the full function context
cat -n internal/log/log.go | head -120 | tail -40Repository: linuxfoundation/lfx-v2-project-service
Length of output: 1307
🏁 Script executed:
# Check all usages of InitStructureLogConfig
rg -n "InitStructureLogConfig\(\)" --type=go -A2 -B2Repository: linuxfoundation/lfx-v2-project-service
Length of output: 412
🏁 Script executed:
# Also check if the function is exported (capitalized) and its signature
rg -n "func.*InitStructureLogConfig" --type=go -A5Repository: linuxfoundation/lfx-v2-project-service
Length of output: 64
🏁 Script executed:
# Find the function signature
rg -n "func.*InitStructureLogConfig" --type=goRepository: linuxfoundation/lfx-v2-project-service
Length of output: 64
🏁 Script executed:
# Search the entire log.go file to find the function
cat -n internal/log/log.go | grep -E "(^[[:space:]]*[0-9]+-func|return h|InitStructureLogConfig)"Repository: linuxfoundation/lfx-v2-project-service
Length of output: 256
🏁 Script executed:
# Check all usages of InitStructureLogConfig across the codebase
rg "InitStructureLogConfig" --type=go -B1 -A3Repository: linuxfoundation/lfx-v2-project-service
Length of output: 396
Return value does not match the wrapped handler that was set as default.
The function declares a return type of slog.Handler and sets slog.SetDefault with the fully wrapped handler (including OtelHandler and contextHandler), but returns h—the raw JSON handler without any wrapping. This creates an API inconsistency: the function configures a wrapped handler as the default but returns an unwrapped handler.
Although the current sole usage in scripts/root-project-setup/main.go doesn't capture the return value, the inconsistency is problematic for future use. Either return the wrapped logger to match what was set as default, or remove the return value entirely.
Suggested fix
logger := contextHandler{otelHandler}
slog.SetDefault(slog.New(logger))
- return h
+ return logger📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Wrap with slog-otel handler to add trace_id and span_id from context | |
| otelHandler := slogotel.OtelHandler{Next: h} | |
| // Wrap with contextHandler to support context-based attributes | |
| logger := contextHandler{otelHandler} | |
| slog.SetDefault(slog.New(logger)) | |
| return h | |
| // Wrap with slog-otel handler to add trace_id and span_id from context | |
| otelHandler := slogotel.OtelHandler{Next: h} | |
| // Wrap with contextHandler to support context-based attributes | |
| logger := contextHandler{otelHandler} | |
| slog.SetDefault(slog.New(logger)) | |
| return logger |
🤖 Prompt for AI Agents
In `@internal/log/log.go` around lines 102 - 110, The function sets the default to
the fully wrapped handler (otelHandler wrapped by contextHandler stored in
variable logger) but currently returns the raw handler h; change the return to
return the wrapped handler (return logger) so the function's slog.Handler return
value matches what was set via slog.SetDefault, ensuring consistency between the
configured default and the function's return.
| 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()), | ||
| } | ||
| } | ||
|
|
||
| // LogWithContext returns a logger with trace context attributes added. | ||
| // Usage: logging.LogWithContext(ctx, slog.Default()).Info("message", "key", "value") | ||
| 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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if test file exists
fd -t f 'context_test.go' pkg/logging/
echo "---"
# List all files in pkg/logging
fd -t f . pkg/logging/Repository: linuxfoundation/lfx-v2-project-service
Length of output: 108
🏁 Script executed:
#!/bin/bash
# Check if Makefile exists and what targets are available
if [ -f Makefile ]; then
echo "=== Makefile targets ==="
grep -E "^[a-z]+:" Makefile | head -20
fi
echo "---"
# Check for go.mod to understand the project structure
[ -f go.mod ] && echo "go.mod exists"Repository: linuxfoundation/lfx-v2-project-service
Length of output: 277
🏁 Script executed:
#!/bin/bash
# Check Go formatting issues using gofmt
cd pkg/logging && gofmt -l context.go 2>/dev/null
cd - > /dev/nullRepository: linuxfoundation/lfx-v2-project-service
Length of output: 64
🏁 Script executed:
#!/bin/bash
# Run the coding guideline checks specified for Go files
echo "=== Running make fmt check ==="
make fmt 2>&1 | head -30
echo "---"
echo "=== Running make lint check ==="
make lint 2>&1 | grep -A5 "context.go" || echo "No issues found in context.go"
echo "---"
echo "=== Running make check ==="
make check 2>&1 | grep -A5 "context.go" || echo "No issues found in context.go"Repository: linuxfoundation/lfx-v2-project-service
Length of output: 282
🏁 Script executed:
#!/bin/bash
# Check if there are test files for other packages to understand testing patterns
echo "=== Test files in pkg/logging and related packages ==="
fd -t f '_test.go' pkg/
echo "---"
echo "=== Check if there are any tests at all ==="
fd -t f '_test.go' . --max-depth 3 | head -20Repository: linuxfoundation/lfx-v2-project-service
Length of output: 620
Missing unit tests for logging context utilities.
The package is missing test coverage. Add pkg/logging/context_test.go with tests for:
LogAttrsFromContextwith valid/invalid span contextsLogWithContextwith valid/invalid span contexts
Other utility packages in this codebase follow the pattern of including corresponding _test.go files.
| func TestOTelConfigFromEnv_Defaults(t *testing.T) { | ||
| cfg := OTelConfigFromEnv() | ||
|
|
||
| if cfg.ServiceName != "lfx-v2-project-service" { | ||
| t.Errorf("expected default ServiceName 'lfx-v2-project-service', got %q", cfg.ServiceName) | ||
| } | ||
| if cfg.ServiceVersion != "" { | ||
| t.Errorf("expected empty ServiceVersion, got %q", cfg.ServiceVersion) | ||
| } | ||
| if cfg.Protocol != OTelProtocolGRPC { | ||
| t.Errorf("expected default Protocol %q, got %q", OTelProtocolGRPC, cfg.Protocol) | ||
| } | ||
| if cfg.Endpoint != "" { | ||
| t.Errorf("expected empty Endpoint, got %q", cfg.Endpoint) | ||
| } | ||
| if cfg.Insecure != false { | ||
| t.Errorf("expected Insecure false, got %t", cfg.Insecure) | ||
| } | ||
| if cfg.TracesExporter != OTelExporterNone { | ||
| t.Errorf("expected default TracesExporter %q, got %q", OTelExporterNone, cfg.TracesExporter) | ||
| } | ||
| if cfg.TracesSampleRatio != 1.0 { | ||
| t.Errorf("expected default TracesSampleRatio 1.0, got %f", cfg.TracesSampleRatio) | ||
| } | ||
| if cfg.MetricsExporter != OTelExporterNone { | ||
| t.Errorf("expected default MetricsExporter %q, got %q", OTelExporterNone, cfg.MetricsExporter) | ||
| } | ||
| if cfg.LogsExporter != OTelExporterNone { | ||
| t.Errorf("expected default LogsExporter %q, got %q", OTelExporterNone, cfg.LogsExporter) | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing assertion for default Propagators value.
The test verifies defaults for most config fields but omits the Propagators field, which should default to "tracecontext,baggage".
🔧 Add missing assertion
if cfg.LogsExporter != OTelExporterNone {
t.Errorf("expected default LogsExporter %q, got %q", OTelExporterNone, cfg.LogsExporter)
}
+ if cfg.Propagators != "tracecontext,baggage" {
+ t.Errorf("expected default Propagators 'tracecontext,baggage', got %q", cfg.Propagators)
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func TestOTelConfigFromEnv_Defaults(t *testing.T) { | |
| cfg := OTelConfigFromEnv() | |
| if cfg.ServiceName != "lfx-v2-project-service" { | |
| t.Errorf("expected default ServiceName 'lfx-v2-project-service', got %q", cfg.ServiceName) | |
| } | |
| if cfg.ServiceVersion != "" { | |
| t.Errorf("expected empty ServiceVersion, got %q", cfg.ServiceVersion) | |
| } | |
| if cfg.Protocol != OTelProtocolGRPC { | |
| t.Errorf("expected default Protocol %q, got %q", OTelProtocolGRPC, cfg.Protocol) | |
| } | |
| if cfg.Endpoint != "" { | |
| t.Errorf("expected empty Endpoint, got %q", cfg.Endpoint) | |
| } | |
| if cfg.Insecure != false { | |
| t.Errorf("expected Insecure false, got %t", cfg.Insecure) | |
| } | |
| if cfg.TracesExporter != OTelExporterNone { | |
| t.Errorf("expected default TracesExporter %q, got %q", OTelExporterNone, cfg.TracesExporter) | |
| } | |
| if cfg.TracesSampleRatio != 1.0 { | |
| t.Errorf("expected default TracesSampleRatio 1.0, got %f", cfg.TracesSampleRatio) | |
| } | |
| if cfg.MetricsExporter != OTelExporterNone { | |
| t.Errorf("expected default MetricsExporter %q, got %q", OTelExporterNone, cfg.MetricsExporter) | |
| } | |
| if cfg.LogsExporter != OTelExporterNone { | |
| t.Errorf("expected default LogsExporter %q, got %q", OTelExporterNone, cfg.LogsExporter) | |
| } | |
| } | |
| func TestOTelConfigFromEnv_Defaults(t *testing.T) { | |
| cfg := OTelConfigFromEnv() | |
| if cfg.ServiceName != "lfx-v2-project-service" { | |
| t.Errorf("expected default ServiceName 'lfx-v2-project-service', got %q", cfg.ServiceName) | |
| } | |
| if cfg.ServiceVersion != "" { | |
| t.Errorf("expected empty ServiceVersion, got %q", cfg.ServiceVersion) | |
| } | |
| if cfg.Protocol != OTelProtocolGRPC { | |
| t.Errorf("expected default Protocol %q, got %q", OTelProtocolGRPC, cfg.Protocol) | |
| } | |
| if cfg.Endpoint != "" { | |
| t.Errorf("expected empty Endpoint, got %q", cfg.Endpoint) | |
| } | |
| if cfg.Insecure != false { | |
| t.Errorf("expected Insecure false, got %t", cfg.Insecure) | |
| } | |
| if cfg.TracesExporter != OTelExporterNone { | |
| t.Errorf("expected default TracesExporter %q, got %q", OTelExporterNone, cfg.TracesExporter) | |
| } | |
| if cfg.TracesSampleRatio != 1.0 { | |
| t.Errorf("expected default TracesSampleRatio 1.0, got %f", cfg.TracesSampleRatio) | |
| } | |
| if cfg.MetricsExporter != OTelExporterNone { | |
| t.Errorf("expected default MetricsExporter %q, got %q", OTelExporterNone, cfg.MetricsExporter) | |
| } | |
| if cfg.LogsExporter != OTelExporterNone { | |
| t.Errorf("expected default LogsExporter %q, got %q", OTelExporterNone, cfg.LogsExporter) | |
| } | |
| if cfg.Propagators != "tracecontext,baggage" { | |
| t.Errorf("expected default Propagators 'tracecontext,baggage', got %q", cfg.Propagators) | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@pkg/utils/otel_test.go` around lines 13 - 43, The test
TestOTelConfigFromEnv_Defaults omits an assertion for the Propagators default;
update the test to assert that OTelConfigFromEnv().Propagators equals
"tracecontext,baggage" (use the existing cfg variable) so the default
Propagators value is verified alongside the other fields.
| func TestOTelConfigFromEnv_CustomValues(t *testing.T) { | ||
| t.Setenv("OTEL_SERVICE_NAME", "test-service") | ||
| t.Setenv("OTEL_SERVICE_VERSION", "1.2.3") | ||
| t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http") | ||
| t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "localhost:4318") | ||
| t.Setenv("OTEL_EXPORTER_OTLP_INSECURE", "true") | ||
| t.Setenv("OTEL_TRACES_EXPORTER", "otlp") | ||
| t.Setenv("OTEL_TRACES_SAMPLE_RATIO", "0.5") | ||
| t.Setenv("OTEL_METRICS_EXPORTER", "otlp") | ||
| t.Setenv("OTEL_LOGS_EXPORTER", "otlp") | ||
|
|
||
| cfg := OTelConfigFromEnv() | ||
|
|
||
| if cfg.ServiceName != "test-service" { | ||
| t.Errorf("expected ServiceName 'test-service', got %q", cfg.ServiceName) | ||
| } | ||
| if cfg.ServiceVersion != "1.2.3" { | ||
| t.Errorf("expected ServiceVersion '1.2.3', got %q", cfg.ServiceVersion) | ||
| } | ||
| if cfg.Protocol != OTelProtocolHTTP { | ||
| t.Errorf("expected Protocol %q, got %q", OTelProtocolHTTP, cfg.Protocol) | ||
| } | ||
| if cfg.Endpoint != "localhost:4318" { | ||
| t.Errorf("expected Endpoint 'localhost:4318', got %q", cfg.Endpoint) | ||
| } | ||
| if cfg.Insecure != true { | ||
| t.Errorf("expected Insecure true, got %t", cfg.Insecure) | ||
| } | ||
| if cfg.TracesExporter != OTelExporterOTLP { | ||
| t.Errorf("expected TracesExporter %q, got %q", OTelExporterOTLP, cfg.TracesExporter) | ||
| } | ||
| if cfg.TracesSampleRatio != 0.5 { | ||
| t.Errorf("expected TracesSampleRatio 0.5, got %f", cfg.TracesSampleRatio) | ||
| } | ||
| if cfg.MetricsExporter != OTelExporterOTLP { | ||
| t.Errorf("expected MetricsExporter %q, got %q", OTelExporterOTLP, cfg.MetricsExporter) | ||
| } | ||
| if cfg.LogsExporter != OTelExporterOTLP { | ||
| t.Errorf("expected LogsExporter %q, got %q", OTelExporterOTLP, cfg.LogsExporter) | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing test for custom Propagators value.
The test sets and verifies custom values for most fields but omits OTEL_PROPAGATORS. Add coverage for this:
🔧 Add missing assertion
t.Setenv("OTEL_LOGS_EXPORTER", "otlp")
+ t.Setenv("OTEL_PROPAGATORS", "tracecontext,jaeger")
cfg := OTelConfigFromEnv()
// ... existing assertions ...
if cfg.LogsExporter != OTelExporterOTLP {
t.Errorf("expected LogsExporter %q, got %q", OTelExporterOTLP, cfg.LogsExporter)
}
+ if cfg.Propagators != "tracecontext,jaeger" {
+ t.Errorf("expected Propagators 'tracecontext,jaeger', got %q", cfg.Propagators)
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func TestOTelConfigFromEnv_CustomValues(t *testing.T) { | |
| t.Setenv("OTEL_SERVICE_NAME", "test-service") | |
| t.Setenv("OTEL_SERVICE_VERSION", "1.2.3") | |
| t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http") | |
| t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "localhost:4318") | |
| t.Setenv("OTEL_EXPORTER_OTLP_INSECURE", "true") | |
| t.Setenv("OTEL_TRACES_EXPORTER", "otlp") | |
| t.Setenv("OTEL_TRACES_SAMPLE_RATIO", "0.5") | |
| t.Setenv("OTEL_METRICS_EXPORTER", "otlp") | |
| t.Setenv("OTEL_LOGS_EXPORTER", "otlp") | |
| cfg := OTelConfigFromEnv() | |
| if cfg.ServiceName != "test-service" { | |
| t.Errorf("expected ServiceName 'test-service', got %q", cfg.ServiceName) | |
| } | |
| if cfg.ServiceVersion != "1.2.3" { | |
| t.Errorf("expected ServiceVersion '1.2.3', got %q", cfg.ServiceVersion) | |
| } | |
| if cfg.Protocol != OTelProtocolHTTP { | |
| t.Errorf("expected Protocol %q, got %q", OTelProtocolHTTP, cfg.Protocol) | |
| } | |
| if cfg.Endpoint != "localhost:4318" { | |
| t.Errorf("expected Endpoint 'localhost:4318', got %q", cfg.Endpoint) | |
| } | |
| if cfg.Insecure != true { | |
| t.Errorf("expected Insecure true, got %t", cfg.Insecure) | |
| } | |
| if cfg.TracesExporter != OTelExporterOTLP { | |
| t.Errorf("expected TracesExporter %q, got %q", OTelExporterOTLP, cfg.TracesExporter) | |
| } | |
| if cfg.TracesSampleRatio != 0.5 { | |
| t.Errorf("expected TracesSampleRatio 0.5, got %f", cfg.TracesSampleRatio) | |
| } | |
| if cfg.MetricsExporter != OTelExporterOTLP { | |
| t.Errorf("expected MetricsExporter %q, got %q", OTelExporterOTLP, cfg.MetricsExporter) | |
| } | |
| if cfg.LogsExporter != OTelExporterOTLP { | |
| t.Errorf("expected LogsExporter %q, got %q", OTelExporterOTLP, cfg.LogsExporter) | |
| } | |
| } | |
| func TestOTelConfigFromEnv_CustomValues(t *testing.T) { | |
| t.Setenv("OTEL_SERVICE_NAME", "test-service") | |
| t.Setenv("OTEL_SERVICE_VERSION", "1.2.3") | |
| t.Setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http") | |
| t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "localhost:4318") | |
| t.Setenv("OTEL_EXPORTER_OTLP_INSECURE", "true") | |
| t.Setenv("OTEL_TRACES_EXPORTER", "otlp") | |
| t.Setenv("OTEL_TRACES_SAMPLE_RATIO", "0.5") | |
| t.Setenv("OTEL_METRICS_EXPORTER", "otlp") | |
| t.Setenv("OTEL_LOGS_EXPORTER", "otlp") | |
| t.Setenv("OTEL_PROPAGATORS", "tracecontext,jaeger") | |
| cfg := OTelConfigFromEnv() | |
| if cfg.ServiceName != "test-service" { | |
| t.Errorf("expected ServiceName 'test-service', got %q", cfg.ServiceName) | |
| } | |
| if cfg.ServiceVersion != "1.2.3" { | |
| t.Errorf("expected ServiceVersion '1.2.3', got %q", cfg.ServiceVersion) | |
| } | |
| if cfg.Protocol != OTelProtocolHTTP { | |
| t.Errorf("expected Protocol %q, got %q", OTelProtocolHTTP, cfg.Protocol) | |
| } | |
| if cfg.Endpoint != "localhost:4318" { | |
| t.Errorf("expected Endpoint 'localhost:4318', got %q", cfg.Endpoint) | |
| } | |
| if cfg.Insecure != true { | |
| t.Errorf("expected Insecure true, got %t", cfg.Insecure) | |
| } | |
| if cfg.TracesExporter != OTelExporterOTLP { | |
| t.Errorf("expected TracesExporter %q, got %q", OTelExporterOTLP, cfg.TracesExporter) | |
| } | |
| if cfg.TracesSampleRatio != 0.5 { | |
| t.Errorf("expected TracesSampleRatio 0.5, got %f", cfg.TracesSampleRatio) | |
| } | |
| if cfg.MetricsExporter != OTelExporterOTLP { | |
| t.Errorf("expected MetricsExporter %q, got %q", OTelExporterOTLP, cfg.MetricsExporter) | |
| } | |
| if cfg.LogsExporter != OTelExporterOTLP { | |
| t.Errorf("expected LogsExporter %q, got %q", OTelExporterOTLP, cfg.LogsExporter) | |
| } | |
| if cfg.Propagators != "tracecontext,jaeger" { | |
| t.Errorf("expected Propagators 'tracecontext,jaeger', got %q", cfg.Propagators) | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@pkg/utils/otel_test.go` around lines 47 - 87,
TestOTelConfigFromEnv_CustomValues is missing coverage for OTEL_PROPAGATORS; set
t.Setenv("OTEL_PROPAGATORS", "<expected-value>") (e.g. "tracecontext,baggage")
before calling OTelConfigFromEnv() and add an assertion that cfg.Propagators
equals that expected string (compare as a string against cfg.Propagators) so the
test verifies propagators parsing in OTelConfigFromEnv.
This pull request introduces OpenTelemetry (OTel) tracing and logging enhancements to the
lfx-v2-project-service, along with improvements to the Helm chart for easier configuration. The most significant changes include adding OTel instrumentation to the Go service, updating logging to include trace context, and expanding Helm chart values for OTel and environment variable management.OpenTelemetry Instrumentation and Logging Enhancements:
main.go, initializing tracing, metrics, and logs based on environment variables.otelhttp.NewHandlerto automatically generate traces for incoming requests.pkg/logging/context.goutility to extract and inject trace and span IDs into logs for better trace-log correlation.slog-otel, ensuring log entries include trace and span context when available. [1] [2]Helm Chart and Configuration Improvements:
values.yamlanddeployment.yaml) to support OTel configuration (service name, version, endpoint, protocol, exporters, propagators, etc.) and arbitrary extra environment variables for the container. [1] [2]0.5.4to0.5.5to reflect these new features.Dependency Updates:
go.modfor tracing, metrics, logs, exporters, and propagators. [1] [2] [3]Codebase Structure:
main.goto support the new tracing and logging features. [1] [2]These changes collectively enable distributed tracing, improved observability, and more flexible deployment configuration for the service.