Skip to content

feat(observability): add generic request correlation and tracing foundation - #664

Merged
XiaoSeS merged 11 commits into
mainfrom
feature/observability-foundation
Aug 4, 2026
Merged

feat(observability): add generic request correlation and tracing foundation#664
XiaoSeS merged 11 commits into
mainfrom
feature/observability-foundation

Conversation

@XiaoSeS

@XiaoSeS XiaoSeS commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

What

  • establish a validated request-correlation boundary with safe X-Request-Id handling
  • emit ECS-style JSON logs with explicit request.id, trace.id, and span.id correlation fields
  • add selectable none, otel-sdk, and external-agent tracing modes
  • propagate request/tracing context across executor tasks, Redis Stream scanner messages, and supported HTTP boundaries
  • keep OTLP export conditional and fail-open when no collector endpoint is configured

Why

The follow-up from #592 mixed two separate concerns: generic observability infrastructure and reliable search-index delivery. This PR implements only the reusable observability foundation. Search-index retry, compensation, dead-letter, and replay semantics remain independent work tracked by #597.

Closes #663

How

  • use Micrometer Tracing with the OpenTelemetry bridge so application code is not coupled to one tracing backend
  • keep none, otel-sdk, and external-agent mutually exclusive and configuration-driven
  • skip OTLP exporter auto-configuration unless an endpoint is explicitly configured
  • copy and restore context around executor tasks to prevent pooled-thread leakage
  • inject request/tracing context into Redis Stream scanner messages and restore it in consumers
  • preserve sensitive-header redaction and Request ID validation at the logging boundary

Final maintainer validation

Exact PR head:

  • 9290fe3ca222964335f94949506a40849fffc5aa

Exact integrated big-main validation commit:

  • b459b1a82c686603ec0929c68ef69a067943c898

Local validation:

  • targeted tracing regression: 13 tests, 0 failures, 0 errors
  • skillhub-app -am Java 21 backend regression: passed
  • structured test matrix: 12/12 cases passed
  • code review gate: no unresolved blocker/high findings

Runtime and remote validation:

  • current PR head exact-SHA local release preview passed smoke and runtime auth-config checks
  • integrated big-main three-component release image set built from production Dockerfiles
  • server, web, and scanner image OCI revisions all match b459b1a82c686603ec0929c68ef69a067943c898
  • integrated big-main local release Compose preview passed smoke and runtime auth-config checks
  • shared remote test machine isolated validation passed: 22/22 assertions, 0 failures, 0 warnings
  • remote coverage included image identity, dynamic loopback ports, release smoke, Web runtime config, Request ID valid/missing/invalid paths, ECS JSON logs, exception-log privacy, bootstrap admin login, publishing a test Skill, Redis Stream scanner async request.id + traceparent propagation, exception scan, cleanup, and pre-existing runtime preservation

CI and merge readiness:

  • Server Unit Tests, Web Build And Test, Docs Build, E2E (Real Services), Script Regression Tests, Dependency Review, DCO, and CLA passed
  • CodeQL matrix job was skipped by workflow configuration
  • PR state is MERGEABLE/CLEAN
  • public-content privacy scan passed for this PR and validation metadata

Impact

  • no API contract or database migration change
  • tracing is optional; none remains the zero-backend mode
  • otel-sdk can run without an OTLP endpoint and will not fail startup
  • external-agent mode requires the deployment to provide exactly one compatible tracing agent
  • search-index reliable delivery is intentionally not included

Related

XiaoSeS added 4 commits July 31, 2026 10:49
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
@XiaoSeS

XiaoSeS commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

补充维护者门禁结果:

维护者侧的完整就绪扫描还发现,当前公开 big-main 可达历史中存在与本 PR 无关的既有敏感环境值。为避免掩盖或改写他人历史,本 PR 暂不宣称 merge-ready;仓库历史清理完成后可复用本次已绑定的功能和远端验证证据,并重新执行最终门禁。

Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
@FenjuFu

FenjuFu commented Aug 1, 2026

Copy link
Copy Markdown
Member

This is solid observability groundwork, and I'm treating it as part of the same architecture track as #630 — so, per my note there, I'm not single-approving a 2.8k-line foundational change on the auth/observability boundary; that's a maintainer call on direction. But I do want to flag a three-way overlap on the observability line that needs sequencing before any of these merge, because all three touch the same seams:

  1. fix(search): improve async rebuild observability #644 (already approved, closes Label search rebuild: async path loses MDC and has no retry/observability #597) — adds MDC context propagation across skillhubEventExecutor plus a rebuild-failure metric.
  2. fix(observability): avoid raw user IDs in generic exception logs #673 (just approved) — redacts userIdauthentication in GlobalExceptionHandler's log lines.
  3. This PR feat(observability): add generic request correlation and tracing foundation #664 — also edits GlobalExceptionHandler, adds ECS JSON logging with correlation fields, and propagates context across async tasks.

Two concrete collisions:

Suggested order: land #673 (smallest, pure redaction) → reconcile #644's MDC decorator with this PR's tracing propagation → then this. @dongjiang1989 flagging for the sequencing/direction call. Happy to re-review the correlation-boundary code specifically once the overlap with #644/#673 is settled.

XiaoSeS added 6 commits August 3, 2026 11:57
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
@XiaoSeS

XiaoSeS commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Final maintainer validation update:

  • PR head verified: 9290fe3ca222964335f94949506a40849fffc5aa
  • Integrated big-main validation commit: b459b1a82c686603ec0929c68ef69a067943c898
  • Local Java 21 validation passed, including the OTLP-empty-endpoint regression.
  • Current PR head exact-SHA local release preview passed smoke and runtime auth-config checks.
  • Integrated big-main server/web/scanner release images all carry the expected OCI revision.
  • Shared remote isolated validation passed: 22/22 assertions, 0 failures, 0 warnings.
  • Remote coverage included Request ID boundaries, ECS JSON logs, exception-log privacy, publishing a test Skill, Redis Stream scanner async request.id + traceparent propagation, exception scan, cleanup, and preservation of the pre-existing runtime.
  • GitHub checks required for this PR passed; CodeQL matrix was skipped by workflow configuration.
  • Public-content privacy scan passed for this PR metadata and validation notes.
  • PR is currently MERGEABLE/CLEAN.

The earlier maintainer note about not declaring merge readiness is superseded by this final gate result.

@XiaoSeS
XiaoSeS merged commit 55e5c1e into main Aug 4, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(observability): add generic request correlation and tracing foundation

2 participants