HYPERFLEET-762: Add OpenTelemetry configuration to standard environme… #89
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import ( | |
| "log/slog" | ||
| "os" | ||
| "os/signal" | ||
| "strconv" | ||
| "syscall" | ||
|
|
||
| "github.com/spf13/cobra" | ||
|
|
@@ -69,14 +70,45 @@ func runServe(cmd *cobra.Command, args []string) { | |
| logger.Info(ctx, config.DumpConfig(environments.Environment().Config)) | ||
|
|
||
| var tp *trace.TracerProvider | ||
|
|
||
| // Check for deprecated HYPERFLEET_LOGGING_OTEL_ENABLED variable | ||
| if deprecatedEnv := os.Getenv("HYPERFLEET_LOGGING_OTEL_ENABLED"); deprecatedEnv != "" { | ||
| logger.With(ctx, | ||
| "deprecated_variable", "HYPERFLEET_LOGGING_OTEL_ENABLED", | ||
| "replacement", "TRACING_ENABLED", | ||
| ).Warn("HYPERFLEET_LOGGING_OTEL_ENABLED is deprecated and ignored. Please use TRACING_ENABLED instead.") | ||
| } | ||
|
|
||
| // Determine if tracing is enabled using TRACING_ENABLED (tracing standard) | ||
| var tracingEnabled bool | ||
| if tracingEnv := os.Getenv("TRACING_ENABLED"); tracingEnv != "" { | ||
|
Contributor
There was a problem hiding this comment. Can we align on using a single variable for enabling tracing? I see the standard says Then, the docs mention I think we should consolidate in a single one and keep it consistent across repositories. AFAIK
Contributor
Author
There was a problem hiding this comment. Got it! I’ll remove HYPERFLEET_LOGGING_OTEL_ENABLED then. I’ll keep the OTEL_ variables* for consistency with hyperfleet-sentinel. Does that make sense?
Contributor
There was a problem hiding this comment. Also to clarify, I meant to use
Contributor
Author
There was a problem hiding this comment.
Collaborator
There was a problem hiding this comment. I am afraid the standards need to be updated to env with
Contributor
Author
There was a problem hiding this comment. I got your point. In that case, should we update the tracing standard first so it aligns with the configuration standard as well?
Contributor
There was a problem hiding this comment. I opened a follow-up ticket for this: https://redhat.atlassian.net/browse/HYPERFLEET-816 |
||
| if enabled, err := strconv.ParseBool(tracingEnv); err == nil { | ||
| tracingEnabled = enabled | ||
| } else { | ||
| logger.With(ctx, | ||
| logger.FieldTracingEnabled, tracingEnv, | ||
| "falling_back_to", environments.Environment().Config.Logging.OTel.Enabled). | ||
| WithError(err).Warn("Invalid TRACING_ENABLED value, falling back to config") | ||
| tracingEnabled = environments.Environment().Config.Logging.OTel.Enabled | ||
| } | ||
| } else { | ||
| // Use config default if TRACING_ENABLED not set | ||
| tracingEnabled = environments.Environment().Config.Logging.OTel.Enabled | ||
| } | ||
|
|
||
| if tracingEnabled { | ||
| // OpenTelemetry configuration is driven entirely by standard environment variables: | ||
| serviceName := "hyperfleet-api" | ||
| if svcName := os.Getenv("OTEL_SERVICE_NAME"); svcName != "" { | ||
| serviceName = svcName | ||
| } | ||
|
|
||
| traceProvider, err := telemetry.InitTraceProvider(ctx, serviceName, api.Version) | ||
|
Comment on lines
+101
to
+106
Contributor
There was a problem hiding this comment. Both sentinel and adapter have a Then, here we are reading a variable that is otel specific.... I wonder if that should be done inside the |
||
| if err != nil { | ||
| logger.WithError(ctx, err).Warn("Failed to initialize OpenTelemetry") | ||
| } else { | ||
| tp = traceProvider | ||
| logger.With(ctx, logger.FieldServiceName, serviceName).Info("OpenTelemetry initialized") | ||
| } | ||
| } else { | ||
| logger.With(ctx, logger.FieldOTelEnabled, false).Info("OpenTelemetry disabled") | ||
|
|
@@ -89,7 +121,7 @@ func runServe(cmd *cobra.Command, args []string) { | |
| "masking_enabled", environments.Environment().Config.Logging.Masking.Enabled, | ||
| ).Info("Logger initialized") | ||
|
|
||
| apiServer := server.NewAPIServer(tracingEnabled) | ||
| go apiServer.Start() | ||
|
|
||
| metricsServer := server.NewMetricsServer() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,10 +32,12 @@ require ( | |
| github.com/testcontainers/testcontainers-go/modules/postgres v0.33.0 | ||
| github.com/yaacov/tree-search-language v0.0.0-20190923184055-1c2dad2e354b | ||
| go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.62.0 | ||
| go.opentelemetry.io/otel v1.40.0 | ||
| go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.40.0 | ||
| go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.40.0 | ||
| go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.40.0 | ||
| go.opentelemetry.io/otel/sdk v1.40.0 | ||
| go.opentelemetry.io/otel/trace v1.40.0 | ||
| go.uber.org/mock v0.6.0 | ||
| gopkg.in/resty.v1 v1.12.0 | ||
| gorm.io/datatypes v1.2.7 | ||
|
|
@@ -53,6 +55,7 @@ require ( | |
| github.com/aymerick/douceur v0.2.0 // indirect | ||
| github.com/beorn7/perks v1.0.1 // indirect | ||
| github.com/cenkalti/backoff/v4 v4.2.1 // indirect | ||
| github.com/cenkalti/backoff/v5 v5.0.3 // indirect | ||
| github.com/cespare/xxhash/v2 v2.3.0 // indirect | ||
| github.com/containerd/errdefs v1.0.0 // indirect | ||
| github.com/containerd/errdefs/pkg v0.3.0 // indirect | ||
|
|
@@ -80,6 +83,7 @@ require ( | |
| github.com/golang/protobuf v1.5.4 // indirect | ||
| github.com/google/go-cmp v0.7.0 // indirect | ||
| github.com/gorilla/css v1.0.0 // indirect | ||
| github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.7 // indirect | ||
| github.com/inconshreveable/mousetrap v1.1.0 // indirect | ||
| github.com/jackc/pgpassfile v1.0.0 // indirect | ||
| github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect | ||
|
|
@@ -133,17 +137,19 @@ require ( | |
| github.com/woodsbury/decimal128 v1.3.0 // indirect | ||
| github.com/yusufpapurcu/wmi v1.2.4 // indirect | ||
| go.opentelemetry.io/auto/sdk v1.2.1 // indirect | ||
| go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.40.0 // indirect | ||
| go.opentelemetry.io/otel/metric v1.40.0 // indirect | ||
| go.opentelemetry.io/proto/otlp v1.9.0 // indirect | ||
| go.yaml.in/yaml/v3 v3.0.4 // indirect | ||
| golang.org/x/crypto v0.47.0 // indirect | ||
| golang.org/x/net v0.49.0 // indirect | ||
| golang.org/x/sync v0.19.0 // indirect | ||
| golang.org/x/sys v0.40.0 // indirect | ||
| golang.org/x/text v0.33.0 // indirect | ||
| google.golang.org/genproto/googleapis/api v0.0.0-20260128011058-8636f8732409 // indirect | ||
| google.golang.org/genproto/googleapis/rpc v0.0.0-20260128011058-8636f8732409 // indirect | ||
| google.golang.org/grpc v1.78.0 // indirect | ||
|
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Verify grpc vulnerability and check latest available version
echo "=== Current grpc version in go.mod ==="
rg -n 'google\.golang\.org/grpc' go.mod
echo ""
echo "=== Check for grpc usage in codebase ==="
rg -n 'google\.golang\.org/grpc' --type go -g '!go.mod' -g '!go.sum' | head -20
echo ""
echo "=== Check otlptracegrpc usage (creates gRPC connections) ==="
rg -n 'otlptracegrpc' --type go
echo ""
echo "=== Query OSV for grpc v1.78.0 advisories ==="
curl -s -X POST https://api.osv.dev/v1/query \
-H "Content-Type: application/json" \
-d '{"package": {"ecosystem": "Go", "name": "google.golang.org/grpc"}, "version": "1.78.0"}' | \
python3 -c "import sys,json; d=json.load(sys.stdin); vulns=d.get('vulns',[]); print(f'Found {len(vulns)} vulnerability(ies)'); [print(f\" - {v.get('id')}: {v.get('summary','N/A')}\") for v in vulns]"Repository: openshift-hyperfleet/hyperfleet-api Length of output: 776 Upgrade The current version v1.78.0 has a critical authorization bypass (CVE identifiers: GO-2026-4762, GHSA-p77j-4mvh-x3m3) involving missing leading slash validation in The codebase uses gRPC through 🧰 Tools🪛 OSV Scanner (2.3.5)[CRITICAL] 151-151: google.golang.org/grpc 1.78.0: Authorization bypass in gRPC-Go via missing leading slash in :path in google.golang.org/grpc (GO-2026-4762) [CRITICAL] 151-151: google.golang.org/grpc 1.78.0: gRPC-Go has an authorization bypass via missing leading slash in :path 🤖 Prompt for AI Agents |
||
| google.golang.org/protobuf v1.36.11 // indirect | ||
| gopkg.in/yaml.v3 v3.0.1 // indirect | ||
| gorm.io/driver/mysql v1.5.6 // indirect | ||
| ) | ||
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.
The PR description says
HYPERFLEET_LOGGING_OTEL_ENABLEDstill works via Viper, but the Viperbindings for
logging.otel.enabledwere removed fromloader.go(bothbindAllEnvVarsandbindFlags). This means the env var is truly dead — it won't flow into the config struct.The deprecation warning correctly says "ignored", but existing deployments that used
HYPERFLEET_LOGGING_OTEL_ENABLED=falseto disable tracing will break silently, since thedefault flipped to
true.Please, Update the PR description to clearly state
HYPERFLEET_LOGGING_OTEL_ENABLEDis nolonger supported.