Skip to content

feat: add lifecycle tracing for controller and manager#658

Open
Liquorice-Ma wants to merge 2 commits into
openkruise:masterfrom
Liquorice-Ma:tracing
Open

feat: add lifecycle tracing for controller and manager#658
Liquorice-Ma wants to merge 2 commits into
openkruise:masterfrom
Liquorice-Ma:tracing

Conversation

@Liquorice-Ma

Copy link
Copy Markdown
Contributor

Ⅰ. Describe what this PR does

This PR adds OpenTelemetry distributed tracing to sandbox-controller and sandbox-manager, providing end-to-end observability of sandbox lifecycle operations — from API request through controller reconcile.New pkg/tracing/ package — reusable tracing infrastructure:
provider.go — TracerProvider init with OTLP gRPC exporter; --tracing-mode flag (otel/none)
propagator.go — W3C Trace Context propagation via CRD annotations (manager→controller)
middleware.go — HTTP middleware; normalizes X-Request-ID into TraceID
reconcile.go — Reconcile span helpers for controller (StartReconcileSpan, StartChildSpan)
processor.go — FilteringSpanProcessor drops no-op Reconcile spans (iterations with no write operation)
idgenerator.go — RequestIDGenerator makes TraceID = request ID for unified trace-log correlation
Controller instrumentation — Reconcile span with trace context from CR annotations; child spans for each lifecycle phase (EnsureSandboxRunning/Paused/Resumed/Upgraded/Terminated) and write operations (CreatePod, DeletePod, UpdateStatus).Manager instrumentation — HTTP middleware root span; API/infra-layer spans (ClaimSandbox, CloneSandbox, DeleteSandbox); InjectTraceContext writes trace context into CR annotations before creating CRs.Observability manifests — OTel Collector, Jaeger, and Grafana deployment configs under config/.New CLI flags — --tracing-mode (controller default otel, manager default none), --tracing-endpoint, --tracing-sampling-ratio, --tracing-insecure.

Ⅱ. Does this pull request fix one issue?

NONE

Ⅲ. Describe how to verify it

Deploy observability stack: kubectl apply -k config/otel-collector && kubectl apply -k config/jaeger && kubectl apply -k config/grafana
Enable tracing on manager (--tracing-mode=otel); controller is enabled by default.
Trigger sandbox operations and verify traces in Jaeger (kubectl port-forward svc/jaeger-query -n sandbox-system 16686:16686).
Verify traceID field in controller/manager logs matches Jaeger traces.
Run unit tests: go test ./pkg/tracing/... ./pkg/controller/sandbox/... ./pkg/sandbox-manager/...

Ⅳ. Special notes for reviews

No-op Reconcile filtering: Controller always creates a Reconcile span (so child write spans have a valid parent), but FilteringSpanProcessor drops it if no write operation occurred. Only Reconciles with CreatePod/DeletePod/PatchPod/Checkpoint/UpdateStatus are retained.
TraceID = RequestID: Custom RequestIDGenerator makes TraceID equal to X-Request-ID (normalized to 32-char hex), enabling direct trace-log correlation.
Trace context propagation: Manager injects W3C trace context into CR annotations before creating CRs; controller extracts it in StartReconcileSpan. CRs without the annotation (e.g., kubectl-created) start a new root trace.
Default values: Controller defaults to otel (assumes OTel Collector in cluster); manager defaults to none (safer for standalone deployments).
Design docs: See docs/specs/2026-07-02-sandbox-otel-tracing-design.md and docs/proposals/20260709-request-id-as-traceid.md.

@kruise-bot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign zmberg for approval by writing /assign @zmberg in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.18009% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.33%. Comparing base (cca9221) to head (23c1c73).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
pkg/controller/sandbox/core/common_control.go 79.06% 9 Missing ⚠️
pkg/sandbox-manager/infra/sandboxcr/sandbox.go 63.63% 7 Missing and 1 partial ⚠️
pkg/controller/sandbox/sandbox_controller.go 93.54% 2 Missing and 2 partials ⚠️
pkg/tracing/provider.go 90.69% 2 Missing and 2 partials ⚠️
pkg/servers/e2b/core.go 25.00% 3 Missing ⚠️
...ller/sandbox/core/common_inplace_update_handler.go 80.00% 1 Missing and 1 partial ⚠️
pkg/controller/sandbox/core/pod_control.go 83.33% 2 Missing ⚠️
pkg/sandbox-manager/api.go 95.83% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #658      +/-   ##
==========================================
+ Coverage   80.10%   80.33%   +0.22%     
==========================================
  Files         230      237       +7     
  Lines       17931    18236     +305     
==========================================
+ Hits        14364    14649     +285     
- Misses       2980     2994      +14     
- Partials      587      593       +6     
Flag Coverage Δ
unittests 80.33% <92.18%> (+0.22%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Liquorice-Ma
Liquorice-Ma force-pushed the tracing branch 9 times, most recently from 420b6ae to 31a25a1 Compare July 17, 2026 09:25
Liquorice-Ma and others added 2 commits July 17, 2026 17:46
- Add pkg/tracing package with TracerProvider, span management, W3C trace context propagation
- Migrate all controllers to klog.FromContext(ctx) for traceID injection
- Replace --tracing-enabled with --tracing-mode flag (otel/none)
- Add DefaultEndpoint variable for configurable tracing endpoint
- Add FilteringSpanProcessor to drop no-op Reconcile spans
- Use custom IDGenerator to set TraceID = request ID
- Inject traceID into controller logs for unified trace-log correlation
- Add Grafana dashboard and OTel Collector deployment configs
- Align Reconcile execution order with upstream/master

Co-authored-by: WWKKAA <1938897817@qq.com>
Signed-off-by: 马赫 <mahe@bupt.edu.cn>
- Replace 'skip-pkg-cache' with 'skip-cache' (renamed in v9.x)
- Move 'mod: readonly' into 'args' (no longer a direct input)
- Add idgenerator_test.go: cover WithRequestID, NewIDs (valid/invalid
  request IDs, random fallback), NewSpanID uniqueness
- Add propagator tests: annotationCarrier Get/Set/Keys methods
- Add provider test: TLS (non-insecure) credentials path
- Add reconcile tests: TraceIDFromContext, StartReconcileSpan stores
  trace ID in context
- Add processor test: Shutdown and ForceFlush direct coverage

Package coverage: 98.2%
- idgenerator.go: 13.33% -> 100%
- propagator.go: 81.25% -> 100%
- processor.go: 84.61% -> 100%
- provider.go: 86.04% -> 91.3%
- reconcile.go: 88.57% -> 100%

Co-authored-by: WWKKAA <1938897817@qq.com>
Signed-off-by: 马赫 <mahe@bupt.edu.cn>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants