Skip to content

[LFXv2-610] Add OpenTelemetry tracing support #40

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.

Sign up for GitHub

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

Open
bramwelt wants to merge 1 commit into
base: main
Choose a base branch
from
+886 −27
Open

[LFXv2-610] Add OpenTelemetry tracing support #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/lfx-v2-indexer-service/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ apiVersion: v2
name: lfx-v2-indexer-service
description: LFX Platform V2 Indexer Service chart
type: application
version: 0.4.13
version: 0.4.14
appVersion: "latest"
43 changes: 43 additions & 0 deletions charts/lfx-v2-indexer-service/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,49 @@ spec:
value: {{ .Values.health.cacheDuration }}
- name: HEALTH_DETAILED_RESPONSE
value: {{ .Values.health.enableDetailedResponse | quote }}
{{- with .Values.app.extraEnv }}
{{- toYaml . | nindent 12 }}
{{- end }}
{{- if .Values.app.otel.serviceName }}
- name: OTEL_SERVICE_NAME
value: {{ .Values.app.otel.serviceName | quote }}
{{- end }}
{{- if .Values.app.otel.serviceVersion }}
- name: OTEL_SERVICE_VERSION
value: {{ .Values.app.otel.serviceVersion | quote }}
{{- end }}
{{- if .Values.app.otel.endpoint }}
- name: OTEL_EXPORTER_OTLP_ENDPOINT
value: {{ .Values.app.otel.endpoint | quote }}
{{- end }}
{{- if .Values.app.otel.protocol }}
- name: OTEL_EXPORTER_OTLP_PROTOCOL
value: {{ .Values.app.otel.protocol | quote }}
{{- end }}
{{- if .Values.app.otel.insecure }}
- name: OTEL_EXPORTER_OTLP_INSECURE
value: {{ .Values.app.otel.insecure | quote }}
{{- end }}
{{- if .Values.app.otel.tracesExporter }}
- name: OTEL_TRACES_EXPORTER
value: {{ .Values.app.otel.tracesExporter | quote }}
{{- end }}
{{- if .Values.app.otel.tracesSampleRatio }}
- name: OTEL_TRACES_SAMPLE_RATIO
value: {{ .Values.app.otel.tracesSampleRatio | quote }}
{{- end }}
Comment on lines +103 to +106
Copy link

@coderabbitai coderabbitai bot Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Potential issue: tracesSampleRatio of "0" won't be rendered.

The {{- if .Values.app.otel.tracesSampleRatio }} condition evaluates to false when the value is "0" or 0, which is a valid sample ratio meaning "sample no traces". If disabling sampling via 0 is a valid use case, consider using a different conditional approach.

🔧 Suggested fix using `ne` comparison 
-            {{- if .Values.app.otel.tracesSampleRatio }}
+            {{- if ne .Values.app.otel.tracesSampleRatio "" }}
             - name: OTEL_TRACES_SAMPLE_RATIO
               value: {{ .Values.app.otel.tracesSampleRatio | quote }}
             {{- end }}

Alternatively, always set the value since there's a sensible default:

-            {{- if .Values.app.otel.tracesSampleRatio }}
             - name: OTEL_TRACES_SAMPLE_RATIO
               value: {{ .Values.app.otel.tracesSampleRatio | default "1.0" | quote }}
-            {{- end }}
📝 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.

Suggested change
{{- if .Values.app.otel.tracesSampleRatio }}
- name: OTEL_TRACES_SAMPLE_RATIO
value: {{ .Values.app.otel.tracesSampleRatio | quote }}
{{- end }}
{{- if ne .Values.app.otel.tracesSampleRatio "" }}
- name: OTEL_TRACES_SAMPLE_RATIO
value: {{ .Values.app.otel.tracesSampleRatio | quote }}
{{- end }}
🤖 Prompt for AI Agents 
In `@charts/lfx-v2-indexer-service/templates/deployment.yaml` around lines 103 -
106, The template condition skips a valid value of 0 because `{{- if
.Values.app.otel.tracesSampleRatio }}` treats 0/ "0" as false; change the
conditional to detect presence instead (e.g., `{{- if ne
.Values.app.otel.tracesSampleRatio nil }}` or `{{- if hasKey .Values.app.otel
"tracesSampleRatio" }}`) or unconditionally emit the OTEL_TRACES_SAMPLE_RATIO
with a sensible default using `default` so that a configured 0 is rendered;
update the block that creates the OTEL_TRACES_SAMPLE_RATIO environment variable
accordingly.

{{- if .Values.app.otel.metricsExporter }}
- name: OTEL_METRICS_EXPORTER
value: {{ .Values.app.otel.metricsExporter | quote }}
{{- end }}
{{- if .Values.app.otel.logsExporter }}
- name: OTEL_LOGS_EXPORTER
value: {{ .Values.app.otel.logsExporter | quote }}
{{- end }}
{{- if .Values.app.otel.propagators }}
- name: OTEL_PROPAGATORS
value: {{ .Values.app.otel.propagators | quote }}
{{- end }}
Comment on lines +79 to +118
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deployment template unconditionally sets OTEL environment variables when corresponding values are defined in values.yaml, even if they are empty strings. The if conditionals check for the presence of the field but don't verify it's not an empty string. For example, if serviceName: "" is set, the template will still inject OTEL_SERVICE_NAME="" as an environment variable, which may not be the intended behavior. Consider using if and .Values.app.otel.serviceName (ne .Values.app.otel.serviceName "") or similar to ensure only non-empty values are injected.

Copilot uses AI. Check for mistakes.
ports:
- containerPort: {{ .Values.app.port }}
name: web
Expand Down
37 changes: 37 additions & 0 deletions charts/lfx-v2-indexer-service/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,43 @@ app:
readTimeout: "5s"
writeTimeout: "5s"
shutdownTimeout: "10s"
# extraEnv allows adding additional environment variables to the container.
# Supports both simple values and Kubernetes field references.
extraEnv: []
# otel is the configuration for OpenTelemetry tracing
otel:
# serviceName is the service name for OpenTelemetry resource identification
# (default: "lfx-v2-indexer-service")
serviceName: ""
# serviceVersion is the service version for OpenTelemetry resource identification
# (default: "1.0.0")
serviceVersion: ""
Comment on lines +77 to +79
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The values.yaml documentation claims the default for serviceVersion is "1.0.0", but the actual code in otel.go (line 90) doesn't provide any default value for serviceVersion - it uses whatever is in the environment variable or an empty string. This creates a discrepancy between the documented behavior and actual behavior. Either update the code to provide "1.0.0" as the default, or update the documentation to clarify that the default is an empty string.

Copilot uses AI. Check for mistakes.
# protocol specifies the OTLP protocol: "grpc" or "http"
# (default: "grpc")
protocol: "grpc"
# endpoint is the OTLP collector endpoint
# For gRPC: typically "host:4317", for HTTP: typically "host:4318"
endpoint: ""
# insecure disables TLS for the OTLP connection
# Set to "true" for in-cluster communication without TLS
insecure: "false"
# tracesExporter specifies the traces exporter: "otlp" or "none"
# (default: "none")
tracesExporter: "none"
# tracesSampleRatio specifies the sampling ratio for traces (0.0 to 1.0)
# A value of 1.0 means all traces are sampled, 0.5 means 50% are sampled
# (default: "1.0")
tracesSampleRatio: "1.0"
# metricsExporter specifies the metrics exporter: "otlp" or "none"
# (default: "none")
metricsExporter: "none"
# logsExporter specifies the logs exporter: "otlp" or "none"
# (default: "none")
logsExporter: "none"
# propagators specifies the propagators to use, comma-separated
# Supported values: "tracecontext", "baggage", "jaeger"
# (default: "tracecontext,baggage")
propagators: "tracecontext,baggage,jaeger"
Comment on lines +82 to +105
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default values in values.yaml use explicit values for fields that have documented defaults in the code. For example, protocol: "grpc", insecure: "false", tracesExporter: "none", etc. are set to their default values. This creates redundancy and potential for drift between the documented defaults in otel.go and the Helm chart defaults. Consider setting these to empty strings (like serviceName and endpoint) to allow the Go code defaults to take effect, or add clear comments indicating these values override code defaults.

Copilot uses AI. Check for mistakes.
Comment on lines +102 to +105
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default propagators value in values.yaml is "tracecontext,baggage,jaeger" but the code default in otel.go line 118 is "tracecontext,baggage" (without jaeger). This inconsistency means that deployments using default values.yaml will have different behavior than running the application with default environment variables. Consider aligning these defaults or documenting why they differ.

Copilot uses AI. Check for mistakes.

# image is the configuration for the container image
image:
Expand Down
15 changes: 15 additions & 0 deletions cmd/lfx-indexer/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/linuxfoundation/lfx-v2-indexer-service/internal/container"
"github.com/linuxfoundation/lfx-v2-indexer-service/pkg/logging"
"github.com/linuxfoundation/lfx-v2-indexer-service/pkg/utils"
)

func main() {
Expand All @@ -26,6 +27,20 @@ func main() {
// Handle early exits (help, config-check)
handleEarlyExits(flags, logger)

// Set up OpenTelemetry SDK.
ctx := context.Background()
otelConfig := utils.OTelConfigFromEnv()
otelShutdown, err := utils.SetupOTelSDKWithConfig(ctx, otelConfig)
if err != nil {
logger.Error("error setting up OpenTelemetry SDK", "error", err)
os.Exit(1)
}
defer func() {
if shutdownErr := otelShutdown(context.Background()); shutdownErr != nil {
logger.Error("error shutting down OpenTelemetry SDK", "error", shutdownErr)
}
}()
Comment on lines +38 to +42
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The OpenTelemetry SDK is set up and shutdown using separate contexts. During setup in main.go, ctx := context.Background() is used (line 31), but the shutdown is called with a fresh context.Background() (line 39). While this may work, it's more conventional to use a timeout context for shutdown operations to ensure they complete within a reasonable time frame. Consider using context.WithTimeout(context.Background(), 5*time.Second) or similar for the shutdown context to prevent hanging during shutdown.

Copilot uses AI. Check for mistakes.

// Log configuration with sources for transparency
logger.Info("Configuration loaded",
"port", flags.Port,
Expand Down
7 changes: 6 additions & 1 deletion cmd/lfx-indexer/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"time"

"github.com/linuxfoundation/lfx-v2-indexer-service/internal/container"
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
)

// createHTTPServer creates and configures the HTTP server with health check routes
Expand All @@ -20,6 +21,10 @@ func createHTTPServer(container *container.Container, bind string) *http.Server
mux := http.NewServeMux()
container.HealthHandler.RegisterRoutes(mux)

// Wrap the handler with OpenTelemetry instrumentation
var handler http.Handler = mux
handler = otelhttp.NewHandler(handler, "indexer-service")

// Create HTTP server with CLI overrides
var addr string
if bind == "*" {
Expand All @@ -30,7 +35,7 @@ func createHTTPServer(container *container.Container, bind string) *http.Server

return &http.Server{
Addr: addr,
Handler: mux,
Handler: handler,
ReadTimeout: container.Config.Server.ReadTimeout,
WriteTimeout: container.Config.Server.WriteTimeout,
ReadHeaderTimeout: 3 * time.Second, // Security: prevent slowloris attacks
Expand Down
45 changes: 37 additions & 8 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,57 @@

module github.com/linuxfoundation/lfx-v2-indexer-service

go 1.24
go 1.24.0

require (
github.com/auth0/go-jwt-middleware/v2 v2.3.0
github.com/nats-io/nats.go v1.43.0
github.com/opensearch-project/opensearch-go/v2 v2.3.0
github.com/stretchr/testify v1.10.0
github.com/remychantenay/slog-otel v1.3.4
github.com/stretchr/testify v1.11.1
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.64.0
go.opentelemetry.io/contrib/propagators/jaeger v1.39.0
go.opentelemetry.io/otel v1.39.0
go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.15.0
go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp v0.15.0
go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.39.0
go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v1.39.0
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.39.0
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.39.0
go.opentelemetry.io/otel/log v0.15.0
go.opentelemetry.io/otel/sdk v1.39.0
go.opentelemetry.io/otel/sdk/log v0.15.0
go.opentelemetry.io/otel/sdk/metric v1.39.0
go.opentelemetry.io/otel/trace v1.39.0
)

require (
github.com/cenkalti/backoff/v5 v5.0.3 // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/felixge/httpsnoop v1.0.4 // indirect
github.com/go-logr/logr v1.4.3 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/google/uuid v1.6.0 // indirect
github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.3 // indirect
github.com/klauspost/compress v1.18.0 // indirect
github.com/kr/pretty v0.3.1 // indirect
github.com/nats-io/nkeys v0.4.11 // indirect
github.com/nats-io/nuid v1.0.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/rogpeppe/go-internal v1.13.1 // indirect
github.com/stretchr/objx v0.5.2 // indirect
golang.org/x/crypto v0.40.0 // indirect
golang.org/x/sync v0.11.0 // indirect
golang.org/x/sys v0.34.0 // indirect
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
go.opentelemetry.io/auto/sdk v1.2.1 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.39.0 // indirect
go.opentelemetry.io/otel/metric v1.39.0 // indirect
go.opentelemetry.io/proto/otlp v1.9.0 // indirect
golang.org/x/crypto v0.44.0 // indirect
golang.org/x/net v0.47.0 // indirect
golang.org/x/sync v0.18.0 // indirect
golang.org/x/sys v0.39.0 // indirect
golang.org/x/text v0.31.0 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20251202230838-ff82c1b0f217 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20251202230838-ff82c1b0f217 // indirect
google.golang.org/grpc v1.77.0 // indirect
google.golang.org/protobuf v1.36.10 // indirect
gopkg.in/go-jose/go-jose.v2 v2.6.3 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
Loading
Loading