feat: add OpenTelemetry tracing and Pyroscope profiling to the router - #417
Open
anantadwi13 wants to merge 8 commits into
Open
feat: add OpenTelemetry tracing and Pyroscope profiling to the router#417anantadwi13 wants to merge 8 commits into
anantadwi13 wants to merge 8 commits into
Conversation
anantadwi13
force-pushed
the
improve-profiling-tracing-implementation
branch
from
August 18, 2026 16:23
bddd64c to
8e79a92
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #417 +/- ##
=======================================
Coverage 96.38% 96.38%
=======================================
Files 26 26
Lines 2101 2101
=======================================
Hits 2025 2025
Misses 76 76
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
anantadwi13
force-pushed
the
improve-profiling-tracing-implementation
branch
2 times, most recently
from
August 19, 2026 04:35
8e79a92 to
0baf466
Compare
Migrate engines/router's tracer from OpenTracing/Jaeger-client to OpenTelemetry (OTLP HTTP exporter, W3C + B3 trace-context propagation), and add pyroscope-go continuous profiling to both engines/router and api. api also gains its own OTel HTTP-layer tracing alongside New Relic, and templates Pyroscope config (including generic HTTP auth headers) into deployed router pods via RouterDefaults. - engines/router: new OTel-based tracing.Tracer interface, B3 propagation, pyroscope-go profiling package, dev-compose wiring. - api: OTel tracing + pyroscope-go profiler in server.Run(), Otel/ Pyroscope config sections, PyroscopeEnabled persisted on RouterVersion.LogConfig, env vars injected into router pods. - Config validation: required_if=Enabled checks and empty-address guards for OTel/Pyroscope endpoints. - OpenAPI spec, example.yaml, and generated SDK client updated to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… config to api Restores the classic Jaeger/Thrift tracer in engines/router as a deprecated, independently-toggleable backend that runs simultaneously alongside the existing OTel/OTLP backend, since a downstream system still requires Jaeger's native Thrift ingestion. A MultiTracer fans out span creation to both backends when both are enabled, tracking each backend's own parent span chain independently so nested spans stay correctly parented per backend. engines/router: - Restore JaegerConfig (Enabled/CollectorEndpoint/ReporterAgentHost/ ReporterAgentPort, Thrift transport), add a separate OtelConfig (Enabled/CollectorEndpoint/SamplingRatio, OTLP HTTP). - New JaegerTracer adapts the classic jaeger-client-go (OpenTracing API) onto the trace.Tracer/trace.Span API directly via a hand-rolled facade (go.opentelemetry.io/otel/bridge/opentracing bridges the opposite direction and can't be used for this). - New MultiTracer fans out to every enabled backend via a dedicated context key (multiParentsKey) so each backend's nested spans parent from its own prior span, not a shared identity. - Tracer interface drops InitGlobalTracer in favour of typed newOtelTracer/newJaegerTracer constructors, since the two backends take different config types. - Local dev compose/env wired for both backends. api (purely additive — nothing persisted, public, or SDK-facing renamed): - RouterDefaults, models.LogConfig, and the OpenAPI contract gain OtelEnabled/OtelCollectorEndpoint next to the existing (now Deprecated:-commented) Jaeger fields. - servicebuilder injects APP_OTEL_ENABLED/APP_OTEL_COLLECTOR_ENDPOINT alongside the existing Jaeger env vars on every deployed router. - Go and Python generated clients regenerated from the updated spec. Every Jaeger* identifier, config field, env var, and the OpenAPI jaeger_enabled property is marked deprecated in favour of the Otel equivalent, with no removal date yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Injects POD_NAME/POD_NAMESPACE into the router container via the Kubernetes downward API and reports them as pod_name/pod_namespace Pyroscope tags (alongside the existing router_name tag), so profiles from individual replicas of a router deployment can be told apart. A new RouterDefaults.PyroscopeIncludePodTags flag (default true) lets an operator opt out of the pod tags deployment-wide, e.g. if per-pod label cardinality is undesirable in Pyroscope. It's templated into routers as APP_PYROSCOPE_INCLUDE_POD_TAGS, mirroring how PyroscopeServerAddress/PyroscopeHTTPHeaders are already passed through. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Bump Go 1.22 -> 1.25 across api/router/experiment/hardcoded-plugin go.mod files (CI's GO_VERSION, and the golang:1.22-alpine base image in each component's own build Dockerfile): the new OTel OTLP HTTP exporter pulls grpc-gateway v2.19.0 -> kr/pretty v0.3.1, whose fmtsort import only exists in go-internal v1.16.0+, which requires go>=1.25. This only surfaced in engines/experiment (and its example plugin) because both locally `replace` engines/router, so `go mod tidy`/`vendor` must resolve the combined dependency graph. - Bump golangci-lint 1.56.2 -> 2.12.2 (and golangci-lint-action v2/v3 -> v7, migrating the shared .golangci.yml to the v2 config schema): no v1.x golangci-lint release supports Go 1.25's export data format. v2 also merges the `gosimple` linter into `staticcheck`, which by default pulls in the `stylecheck`/quickfix check families this repo never enabled -- restricted staticcheck.checks to keep the same check scope as before. - Add //nolint:staticcheck at every intentional usage of the newly-deprecated Jaeger config fields/types, since the PR's own backward-compat code paths trigger SA1019 on their own deprecated symbols. - Fix vet/lint findings that only surfaced once Go 1.25's govet and golangci-lint v2.12.2 ran over the affected packages: non-constant format strings passed to fmt.Errorf, reflect.Ptr -> reflect.Pointer, a named return shadowing the builtin `error` identifier, and two redundant nil-checks before len() on slices. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
anantadwi13
force-pushed
the
improve-profiling-tracing-implementation
branch
from
August 19, 2026 04:36
0baf466 to
a56ee84
Compare
…C needed Knative's Revision admission webhook rejects env[].valueFrom.fieldRef unless the alpha kubernetes.podspec-fieldref feature gate is set to Enabled in the target cluster (disabled by default), so router deploys were failing outright with: admission webhook "validation.webhook.serving.knative.dev" denied the request: validation failed: must not set the field(s): ...valueFrom.fieldRef Rather than reading Knative's own config-features ConfigMap to detect this (which needs a new RBAC grant per target cluster, separate from whatever Turing already needs there), probe for the capability with a dry-run of the exact deploy request itself, using only the create/update permission on Knative Services that Turing's deploy credential already requires for every deploy. DeployKnativeService now: if the built service has any env[].valueFrom.fieldRef (the router's POD_NAME/POD_NAMESPACE), it dry-runs the Create/Update first. If the cluster rejects it with that specific admission error, the fields are stripped in place and a warning is logged; otherwise the error propagates as a normal deploy failure. The real Create/Update then proceeds as before, with or without those fields depending on the probe. buildRouterEnvs/NewRouterService go back to unconditionally including the fieldRef env vars -- all the capability handling lives in the cluster/controller layer instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
anantadwi13
force-pushed
the
improve-profiling-tracing-implementation
branch
from
August 19, 2026 10:23
0ee4fa7 to
f37ef03
Compare
…o 0.01 The router engine already supported configuring its OTel trace sampling ratio via APP_OTEL_SAMPLING_RATIO (engines/router/missionctl/config, default 1 -- sample everything), but Turing's API had no way to set it: RouterDefaults had no corresponding field, and buildRouterEnvs never set the env var, so every deployed router silently sampled 100% of traces regardless of the API's own Otel.SamplingRatio (which only affects the API server's own self-tracing, not routers it deploys). Add RouterDefaults.OtelSamplingRatio, threaded into buildRouterEnvs as APP_OTEL_SAMPLING_RATIO, so operators can configure this per Turing deployment like the other RouterDefaults fields. Also lower the default sampling ratio from 1 to 0.01 in three places: the API's own Otel.SamplingRatio, the new RouterDefaults.OtelSamplingRatio default, and the router engine's own default when the env var is unset -- sampling everything is rarely necessary and gets expensive at scale. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Trailing newline only, no content changes. Touches sdk/** so this PR's sdk workflow computes a fresh dev version and re-runs the publish job, after the previous run collided with an already-published PyPI version (0.16.2.post10.dev0 -- the dev version scheme derives its "postN" purely from commit count since the last tag, which is shared across branches, so an unrelated commit elsewhere had already claimed that exact version). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds RouterDefaults.PyroscopeCustomTags (map[string]string), serialized into a single APP_PYROSCOPE_CUSTOM_TAGS env var the same way PyroscopeHTTPHeaders already is. The router merges these into its Pyroscope tags alongside the built-in router_name/pod_name/ pod_namespace tags, which always win on key collision. Also adds the same CustomTags support to the Turing API's own Pyroscope config (config.PyroscopeConfig, api/turing/server/ instrumentation.go), which profiles the API server itself and previously had no way to attach custom tags. It further adds Pyroscope.IncludePodTags (default true), tagging API profiles with pod_name/pod_namespace so individual replicas can be told apart -- the POD_NAME/POD_NAMESPACE env vars are left for the deployer to populate via the Kubernetes downward API (e.g. turing.extraEnvs in the Helm chart), since the API's own deployment isn't templated by this codebase the way routers are. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
engines/router's tracer from OpenTracing/Jaeger-client to OpenTelemetry (OTLP HTTP exporter, W3C + B3 propagation), and addspyroscope-gocontinuous profiling to bothengines/routerandapi.apialso gains its own OTel HTTP-layer tracing alongside New Relic, with OTel/Pyroscope config templated into deployed router pods viaRouterDefaults.engines/routeras a deprecated, independently-toggleable backend that can run simultaneously alongside OTel (aMultiTracerfans out span creation to both,keeping each backend's parent-span chain independent), since a downstream system still requires Jaeger's native Thrift ingestion. Every
Jaeger*identifier/env var/OpenAPI field is marked deprecated in favour ofthe
Otelequivalent, with no removal date yet.pod_name/pod_namespace(via the Kubernetes downward API), so individual replicas of a multi-replica router deployment can be told apart in Pyroscope, alongside theexisting
router_nametag. Includes aRouterDefaults.PyroscopeIncludePodTagsflag (defaulttrue) to opt out deployment-wide.api/engines/router/engines/experiment(the new OTel exporter's dependency graph requires it), migrates.golangci.ymlto the v2 config schema, and fixes the vet/lint findings that only surfaced under the new toolchain.kubernetes.podspec-fieldreffeature gate is disabled (the default) — Knative's admission webhook rejects thePOD_NAME/POD_NAMESPACEdownward-API env vars used for pod-identity tagging above.
DeployKnativeServicenow dry-runs the deploy first and silently drops those env vars if the cluster rejects them, using only the create/updatepermission Turing's deploy credential already needs — no new RBAC required on any target cluster.
RouterDefaults.OtelSamplingRatio(env varAPP_OTEL_SAMPLING_RATIO), which previously had no way to be configured from the API even though the router enginealready supported it. Also lowers the default sampling ratio from
1(sample everything) to0.01for the API's own tracing, the new router default, and the router engine's own fallback.Testing
api/engines/routerunit tests pass (servicebuilder,config,instrumentation/profiling,instrumentation/tracing).clusterpackage coverage for the dry-run fallback (TestDeployKnativeServiceFieldRefRejectedFallsBackWithoutIt) verifies the fieldRef env vars are stripped and the deploy retried when the target clusterrejects them.
test-e2e) exercises the fallback path implicitly, since its Knative Serving version predates thekubernetes.podspec-fieldreffeature flag entirely — there's currently no e2e coverage of a clusterwhere the flag is enabled.