Skip to content

Add environment-driven OpenTelemetry initialization#32

Merged
mariusvniekerk merged 4 commits into
mainfrom
feat/opentelemetry-init
Jul 10, 2026
Merged

Add environment-driven OpenTelemetry initialization#32
mariusvniekerk merged 4 commits into
mainfrom
feat/opentelemetry-init

Conversation

@mariusvniekerk

Copy link
Copy Markdown
Contributor

Why

Kenn tools currently have to recreate OpenTelemetry provider, exporter, propagation, and shutdown wiring in each application. That makes standard environment-driven configuration harder to adopt consistently and leaves lifecycle/error handling up to every caller.

The standard autoexport fallback would also attempt OTLP export to localhost when no exporter selector is configured. That is a poor default for reusable CLI and developer-tool infrastructure, where telemetry should remain inert until the operator explicitly enables a signal.

Approach

Provide one app-neutral initializer for traces and metrics that preserves standard OpenTelemetry environment variables and native SDK option types. The only deliberate policy difference is that unset trace and metric exporter selectors mean off; explicit otlp, console, prometheus, and none values retain contrib behavior. Logs remain on the existing slog path.

Initialization registers globals only after all fallible setup succeeds, cleans up partial setup, and returns one shutdown function that attempts both providers without hiding either error.

Validation

  • go build ./...
  • go tool gotestsum --format pkgname-and-test-fails -- ./... — 583 passed, 2 expected skips
  • go vet ./...
  • golangci-lint run ./telemetry/... — 0 issues
  • git diff --check

Repo-wide golangci-lint run ./... still reports three pre-existing findings in git/cmd and vector/sqlitevec; this change introduces no telemetry lint findings.

generated by a clanker

Callers currently have to rebuild the same provider, exporter, propagation, and shutdown wiring, which makes OpenTelemetry setup inconsistent across Kenn tools.

Centralize that bootstrap around the standard contrib mechanisms while keeping export opt-in. This avoids surprising attempts to connect to a localhost OTLP endpoint when a CLI or development tool has not explicitly configured a signal exporter, without replacing OpenTelemetry's native resource and provider extension points.

Generated with Codex
@roborev-ci

roborev-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (060e2f8)

The change is generally sound, but it misses standard OpenTelemetry environment resource configuration.

Medium

  • telemetry/init.go:101Init combines caller-supplied resource options with resource.Default() but does not apply the SDK environment resource detector. As a result, OTEL_SERVICE_NAME and OTEL_RESOURCE_ATTRIBUTES are ignored, potentially exporting telemetry with an unknown/default service identity even when exporters are fully configured through environment variables. Add resource.WithFromEnv() before caller options so explicit WithResourceOptions values can override environment values, and add coverage for OTEL_SERVICE_NAME or OTEL_RESOURCE_ATTRIBUTES.

Reviewers: 2 done | Synthesis: codex, 5s | Total: 1m34s

Resource defaults and provider construction both consult the OpenTelemetry environment, but the default resource is process-cached and its merge precedence can obscure a later environment value.

Read environment attributes during each telemetry initialization before applying caller resource options. This makes OTEL resource configuration explicit and ensures deliberate caller attributes retain final precedence.

Generated with Codex
@roborev-ci

roborev-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (0eaa7da)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 1m51s

Merging fresh environment attributes over resource.Default still retained labels that the process-cached default had read earlier and that operators later removed.

Construct the standard service, environment, and SDK resource afresh for each initialization, then apply caller options. Treat partial environment parsing as a warning so valid labels remain usable without turning malformed optional metadata into an application startup failure.

Generated with Codex
@roborev-ci

roborev-ci Bot commented Jul 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (afc5e1a)

Code changes look sound overall, but one medium-severity dependency vulnerability should be resolved before merging.

Medium

  • Default baggage propagation exposes services to CVE-2026-41178telemetry/init.go:158, go.mod:17

    Init globally registers autoprop.NewTextMapPropagator(config.propagators...). When no propagators are configured and OTEL_PROPAGATORS is unset, autoprop defaults to TraceContext plus Baggage. The pinned OpenTelemetry version (v1.43.0) is affected by CVE-2026-41178 / GHSA-5wrp-cwcj-q835, allowing oversized or malformed attacker-controlled baggage headers to amplify CPU, memory, and log usage in instrumented HTTP/gRPC services.

    Upgrade all go.opentelemetry.io/otel modules to v1.44.0 or later and use a compatible autoprop release. Until then, use an explicit safe default such as propagation.TraceContext{} and prevent exposed services from re-enabling baggage through OTEL_PROPAGATORS.


Reviewers: 2 done | Synthesis: codex, 9s | Total: 3m30s

Default baggage propagation on OpenTelemetry 1.43 is vulnerable to oversized-header CPU, memory, and log amplification, so the helper must not expose callers to the affected parser.

Upgrade the aligned OpenTelemetry and contrib modules to their patched releases. Keep recoverable environment parsing isolated from caller detectors, and preserve the SDK service-instance feature gate so resource hardening does not change identity cardinality.

Generated with Codex
@roborev-ci

roborev-ci Bot commented Jul 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (51bb1b2)

No Medium, High, or Critical findings were identified.


Reviewers: 2 done | Synthesis: codex, 6s | Total: 4m55s

@mariusvniekerk mariusvniekerk merged commit fc05aad into main Jul 10, 2026
8 checks passed
@mariusvniekerk mariusvniekerk deleted the feat/opentelemetry-init branch July 10, 2026 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant