From 893c0d49cadfd324ab2c7b7ddd4d52862a7df472 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Fri, 31 Jul 2026 10:49:32 +0800 Subject: [PATCH 01/10] feat(observability): establish request correlation boundary Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- server/skillhub-app/pom.xml | 5 + .../compat/ClawHubCompatAppService.java | 9 +- .../controller/DeviceAuthWebController.java | 9 +- .../controller/UserProfileController.java | 8 +- .../admin/AdminProfileReviewController.java | 10 +- .../admin/AdminSearchController.java | 9 +- .../skillhub/dto/ApiResponseFactory.java | 14 ++- .../exception/GlobalExceptionHandler.java | 15 ++- .../filter/IdempotencyInterceptor.java | 24 +++- .../skillhub/filter/RequestIdFilter.java | 19 +-- .../observability/RequestIdAccessor.java | 78 ++++++++++++ .../logging/CorrelationJsonProvider.java | 50 ++++++++ .../logging/SkillHubEcsEncoder.java | 108 ++++++++++++++++ .../observability/logging/package-info.java | 4 + .../skillhub/observability/package-info.java | 4 + .../security/ApiAccessDeniedHandler.java | 9 +- .../security/ApiAuthenticationEntryPoint.java | 9 +- .../service/LabelAdminAppService.java | 9 +- .../service/PromotionPortalAppService.java | 9 +- .../service/ReviewPortalAppService.java | 9 +- .../service/SkillLabelAppService.java | 9 +- .../src/main/resources/application.yml | 5 + .../src/main/resources/logback-spring.xml | 47 +++++++ .../compat/ClawHubCompatAppServiceTest.java | 4 +- .../UserProfileControllerUnitTest.java | 8 +- .../portal/NotificationControllerTest.java | 4 +- .../NotificationPreferenceControllerTest.java | 4 +- .../exception/GlobalExceptionHandlerTest.java | 12 +- .../filter/AuthContextFilterTest.java | 4 +- .../filter/IdempotencyInterceptorTest.java | 31 ++++- .../skillhub/filter/RequestIdFilterTest.java | 54 +++++++- .../observability/RequestIdAccessorTest.java | 61 +++++++++ .../logging/SkillHubEcsEncoderTest.java | 116 ++++++++++++++++++ .../security/ApiAccessDeniedHandlerTest.java | 14 ++- .../service/LabelAdminAppServiceTest.java | 4 +- .../PromotionPortalAppServiceTest.java | 4 +- .../service/SkillLabelAppServiceTest.java | 4 +- 37 files changed, 721 insertions(+), 75 deletions(-) create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/RequestIdAccessor.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/CorrelationJsonProvider.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/SkillHubEcsEncoder.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/package-info.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/package-info.java create mode 100644 server/skillhub-app/src/main/resources/logback-spring.xml create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/RequestIdAccessorTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/logging/SkillHubEcsEncoderTest.java diff --git a/server/skillhub-app/pom.xml b/server/skillhub-app/pom.xml index aeeee7a33..8914e00e7 100644 --- a/server/skillhub-app/pom.xml +++ b/server/skillhub-app/pom.xml @@ -26,6 +26,11 @@ io.micrometer micrometer-registry-prometheus + + net.logstash.logback + logstash-logback-encoder + 7.4 + org.springdoc springdoc-openapi-starter-webmvc-ui diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatAppService.java index b6fc5b1f6..c2b583f3c 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatAppService.java @@ -21,12 +21,12 @@ import com.iflytek.skillhub.domain.skill.service.SkillQueryService; import com.iflytek.skillhub.domain.social.SkillStarService; import com.iflytek.skillhub.dto.SkillSummaryResponse; +import com.iflytek.skillhub.observability.RequestIdAccessor; import com.iflytek.skillhub.service.SkillSearchAppService; import java.io.IOException; import java.util.HashMap; import java.util.List; import java.util.Map; -import org.slf4j.MDC; import org.springframework.stereotype.Service; import org.springframework.util.StringUtils; import org.springframework.web.multipart.MultipartFile; @@ -49,6 +49,7 @@ public class ClawHubCompatAppService { private final AuditLogService auditLogService; private final CompatSkillLookupService compatSkillLookupService; private final SkillStarService skillStarService; + private final RequestIdAccessor requestIdAccessor; public ClawHubCompatAppService(CanonicalSlugMapper mapper, SkillSearchAppService skillSearchAppService, @@ -58,7 +59,8 @@ public ClawHubCompatAppService(CanonicalSlugMapper mapper, MultipartPackageExtractor multipartPackageExtractor, AuditLogService auditLogService, CompatSkillLookupService compatSkillLookupService, - SkillStarService skillStarService) { + SkillStarService skillStarService, + RequestIdAccessor requestIdAccessor) { this.mapper = mapper; this.skillSearchAppService = skillSearchAppService; this.skillQueryService = skillQueryService; @@ -68,6 +70,7 @@ public ClawHubCompatAppService(CanonicalSlugMapper mapper, this.auditLogService = auditLogService; this.compatSkillLookupService = compatSkillLookupService; this.skillStarService = skillStarService; + this.requestIdAccessor = requestIdAccessor; } public ClawHubSearchResponse search(String q, @@ -430,7 +433,7 @@ private void recordCompatPublishAudit(String userId, "COMPAT_PUBLISH", "SKILL_VERSION", versionId, - MDC.get("requestId"), + requestIdAccessor.current(), clientIp, userAgent, detailJson diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/DeviceAuthWebController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/DeviceAuthWebController.java index c2f47d4ce..71f27a195 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/DeviceAuthWebController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/DeviceAuthWebController.java @@ -6,8 +6,8 @@ import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; import com.iflytek.skillhub.dto.MessageResponse; +import com.iflytek.skillhub.observability.RequestIdAccessor; import jakarta.servlet.http.HttpServletRequest; -import org.slf4j.MDC; import org.springframework.security.core.annotation.AuthenticationPrincipal; import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestBody; @@ -24,13 +24,16 @@ public class DeviceAuthWebController extends BaseApiController { private final DeviceAuthService deviceAuthService; private final AuditLogService auditLogService; + private final RequestIdAccessor requestIdAccessor; public DeviceAuthWebController(ApiResponseFactory responseFactory, DeviceAuthService deviceAuthService, - AuditLogService auditLogService) { + AuditLogService auditLogService, + RequestIdAccessor requestIdAccessor) { super(responseFactory); this.deviceAuthService = deviceAuthService; this.auditLogService = auditLogService; + this.requestIdAccessor = requestIdAccessor; } @PostMapping("/authorize") @@ -45,7 +48,7 @@ public ApiResponse authorizeDevice( "DEVICE_AUTHORIZE", "DEVICE_CODE", null, - MDC.get("requestId"), + requestIdAccessor.current(), httpRequest.getRemoteAddr(), httpRequest.getHeader("User-Agent"), "{\"userCode\":\"" + request.userCode() + "\"}" diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/UserProfileController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/UserProfileController.java index 3265c4b7e..2849455ed 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/UserProfileController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/UserProfileController.java @@ -21,6 +21,7 @@ import com.iflytek.skillhub.dto.UpdateProfileResponse; import com.iflytek.skillhub.dto.UserProfileResponse; import com.iflytek.skillhub.exception.UnauthorizedException; +import com.iflytek.skillhub.observability.RequestIdAccessor; import jakarta.servlet.http.HttpServletRequest; import jakarta.validation.Valid; import org.springframework.security.core.Authentication; @@ -53,19 +54,22 @@ public class UserProfileController extends BaseApiController { private final ProfileChangeRequestRepository changeRequestRepository; private final PlatformSessionService platformSessionService; private final ProfileFieldPolicyConfig fieldPolicyConfig; + private final RequestIdAccessor requestIdAccessor; public UserProfileController(ApiResponseFactory responseFactory, UserProfileService userProfileService, UserAccountRepository userAccountRepository, ProfileChangeRequestRepository changeRequestRepository, PlatformSessionService platformSessionService, - ProfileFieldPolicyConfig fieldPolicyConfig) { + ProfileFieldPolicyConfig fieldPolicyConfig, + RequestIdAccessor requestIdAccessor) { super(responseFactory); this.userProfileService = userProfileService; this.userAccountRepository = userAccountRepository; this.changeRequestRepository = changeRequestRepository; this.platformSessionService = platformSessionService; this.fieldPolicyConfig = fieldPolicyConfig; + this.requestIdAccessor = requestIdAccessor; } /** @@ -143,7 +147,7 @@ public ApiResponse updateProfile( UpdateProfileResult result = userProfileService.updateProfile( principal.userId(), changes, - httpRequest.getHeader("X-Request-Id"), + requestIdAccessor.current(), resolveClientIp(httpRequest), httpRequest.getHeader("User-Agent") ); diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminProfileReviewController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminProfileReviewController.java index 9e0a87f49..cc2edcc7a 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminProfileReviewController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminProfileReviewController.java @@ -9,6 +9,7 @@ import com.iflytek.skillhub.dto.ProfileReviewMutationResponse; import com.iflytek.skillhub.dto.ProfileReviewRejectRequest; import com.iflytek.skillhub.dto.ProfileReviewSummaryResponse; +import com.iflytek.skillhub.observability.RequestIdAccessor; import com.iflytek.skillhub.service.AdminProfileReviewAppService; import jakarta.servlet.http.HttpServletRequest; import jakarta.validation.Valid; @@ -28,13 +29,16 @@ public class AdminProfileReviewController extends BaseApiController { private final AdminProfileReviewAppService appService; private final ProfileReviewService reviewService; + private final RequestIdAccessor requestIdAccessor; public AdminProfileReviewController(ApiResponseFactory responseFactory, AdminProfileReviewAppService appService, - ProfileReviewService reviewService) { + ProfileReviewService reviewService, + RequestIdAccessor requestIdAccessor) { super(responseFactory); this.appService = appService; this.reviewService = reviewService; + this.requestIdAccessor = requestIdAccessor; } /** List profile change requests filtered by status (default: PENDING). */ @@ -58,7 +62,7 @@ public ApiResponse approve( var result = reviewService.approve( id, principal.userId(), - httpRequest.getHeader("X-Request-Id"), + requestIdAccessor.current(), resolveClientIp(httpRequest), httpRequest.getHeader("User-Agent")); return ok("response.success.updated", @@ -77,7 +81,7 @@ public ApiResponse reject( id, principal.userId(), request.comment(), - httpRequest.getHeader("X-Request-Id"), + requestIdAccessor.current(), resolveClientIp(httpRequest), httpRequest.getHeader("User-Agent")); return ok("response.success.updated", diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSearchController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSearchController.java index b6dc264a7..87770644d 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSearchController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSearchController.java @@ -5,8 +5,8 @@ import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; import com.iflytek.skillhub.domain.audit.AuditLogService; +import com.iflytek.skillhub.observability.RequestIdAccessor; import jakarta.servlet.http.HttpServletRequest; -import org.slf4j.MDC; import com.iflytek.skillhub.search.SearchRebuildService; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.core.annotation.AuthenticationPrincipal; @@ -23,13 +23,16 @@ public class AdminSearchController extends BaseApiController { private final SearchRebuildService searchRebuildService; private final AuditLogService auditLogService; + private final RequestIdAccessor requestIdAccessor; public AdminSearchController(ApiResponseFactory responseFactory, SearchRebuildService searchRebuildService, - AuditLogService auditLogService) { + AuditLogService auditLogService, + RequestIdAccessor requestIdAccessor) { super(responseFactory); this.searchRebuildService = searchRebuildService; this.auditLogService = auditLogService; + this.requestIdAccessor = requestIdAccessor; } @PostMapping("/rebuild") @@ -42,7 +45,7 @@ public ApiResponse rebuildAll(@AuthenticationPrincipal PlatformPrincipal p "REBUILD_SEARCH_INDEX", "SEARCH_INDEX", null, - MDC.get("requestId"), + requestIdAccessor.current(), httpRequest.getRemoteAddr(), httpRequest.getHeader("User-Agent"), "{\"scope\":\"ALL\"}" diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/ApiResponseFactory.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/ApiResponseFactory.java index 720958f42..13e7f701b 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/ApiResponseFactory.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/ApiResponseFactory.java @@ -1,8 +1,8 @@ package com.iflytek.skillhub.dto; +import com.iflytek.skillhub.observability.RequestIdAccessor; import org.springframework.context.MessageSource; import org.springframework.stereotype.Component; -import org.slf4j.MDC; import org.springframework.context.i18n.LocaleContextHolder; import java.time.Clock; @@ -13,23 +13,27 @@ public class ApiResponseFactory { private final MessageSource messageSource; private final Clock clock; + private final RequestIdAccessor requestIdAccessor; - public ApiResponseFactory(MessageSource messageSource, Clock clock) { + public ApiResponseFactory(MessageSource messageSource, + Clock clock, + RequestIdAccessor requestIdAccessor) { this.messageSource = messageSource; this.clock = clock; + this.requestIdAccessor = requestIdAccessor; } public ApiResponse ok(String messageCode, T data, Object... args) { String msg = messageSource.getMessage(messageCode, args, messageCode, LocaleContextHolder.getLocale()); - return new ApiResponse<>(0, msg, data, Instant.now(clock), MDC.get("requestId")); + return new ApiResponse<>(0, msg, data, Instant.now(clock), requestIdAccessor.current()); } public ApiResponse error(int code, String messageCode, Object... args) { String msg = messageSource.getMessage(messageCode, args, messageCode, LocaleContextHolder.getLocale()); - return new ApiResponse<>(code, msg, null, Instant.now(clock), MDC.get("requestId")); + return new ApiResponse<>(code, msg, null, Instant.now(clock), requestIdAccessor.current()); } public ApiResponse errorMessage(int code, String msg) { - return new ApiResponse<>(code, msg, null, Instant.now(clock), MDC.get("requestId")); + return new ApiResponse<>(code, msg, null, Instant.now(clock), requestIdAccessor.current()); } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java index 2d30bd54b..712411862 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java @@ -7,12 +7,12 @@ import com.iflytek.skillhub.domain.shared.exception.LocalizedDomainException; import com.iflytek.skillhub.domain.shared.exception.LocalizedMessage; import com.iflytek.skillhub.metrics.SkillHubMetrics; +import com.iflytek.skillhub.observability.RequestIdAccessor; import com.iflytek.skillhub.security.SensitiveLogSanitizer; import com.iflytek.skillhub.storage.StorageAccessException; import jakarta.servlet.http.HttpServletRequest; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.slf4j.MDC; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.security.access.AccessDeniedException; @@ -34,13 +34,16 @@ public class GlobalExceptionHandler { private final ApiResponseFactory apiResponseFactory; private final SensitiveLogSanitizer sensitiveLogSanitizer; private final SkillHubMetrics metrics; + private final RequestIdAccessor requestIdAccessor; public GlobalExceptionHandler(ApiResponseFactory apiResponseFactory, SensitiveLogSanitizer sensitiveLogSanitizer, - SkillHubMetrics metrics) { + SkillHubMetrics metrics, + RequestIdAccessor requestIdAccessor) { this.apiResponseFactory = apiResponseFactory; this.sensitiveLogSanitizer = sensitiveLogSanitizer; this.metrics = metrics; + this.requestIdAccessor = requestIdAccessor; } @ExceptionHandler(LocalizedException.class) @@ -111,7 +114,7 @@ public ResponseEntity> handleStorageAccess(StorageAccessExcept metrics.incrementStorageAccessFailure(ex.getOperation()); logger.warn( "Object storage unavailable [requestId={}, method={}, path={}, userId={}, operation={}, key={}]", - MDC.get("requestId"), + requestIdAccessor.current(), request.getMethod(), sensitiveLogSanitizer.sanitizeRequestTarget(request), resolveUserId(request), @@ -127,7 +130,7 @@ public ResponseEntity> handleStorageAccess(StorageAccessExcept public ResponseEntity handleAsyncRequestTimeout(AsyncRequestTimeoutException ex, HttpServletRequest request) { String path = request.getRequestURI(); if (path != null && path.endsWith("/sse")) { - logger.debug("SSE timeout [requestId={}, path={}]", MDC.get("requestId"), path); + logger.debug("SSE timeout [requestId={}, path={}]", requestIdAccessor.current(), path); return ResponseEntity.noContent().build(); } @@ -140,7 +143,7 @@ public ResponseEntity handleAsyncRequestTimeout(AsyncRequestTimeoutException public ResponseEntity> handleGlobalException(Exception ex, HttpServletRequest request) { logger.error( "Unhandled API exception [requestId={}, method={}, path={}, userId={}]", - MDC.get("requestId"), + requestIdAccessor.current(), request.getMethod(), sensitiveLogSanitizer.sanitizeRequestTarget(request), resolveUserId(request), @@ -153,7 +156,7 @@ public ResponseEntity> handleGlobalException(Exception ex, Htt private void logHandledException(HttpStatus status, String messageCode, HttpServletRequest request) { logger.info( "API request failed [requestId={}, status={}, method={}, path={}, userId={}, code={}]", - MDC.get("requestId"), + requestIdAccessor.current(), status.value(), request.getMethod(), sensitiveLogSanitizer.sanitizeRequestTarget(request), diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/IdempotencyInterceptor.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/IdempotencyInterceptor.java index 9a950d60a..e517f9a8a 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/IdempotencyInterceptor.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/IdempotencyInterceptor.java @@ -5,6 +5,7 @@ import com.iflytek.skillhub.domain.idempotency.IdempotencyRecordRepository; import com.iflytek.skillhub.domain.idempotency.IdempotencyStatus; import com.iflytek.skillhub.dto.ApiResponse; +import com.iflytek.skillhub.observability.RequestIdAccessor; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import org.springframework.data.redis.core.StringRedisTemplate; @@ -34,15 +35,18 @@ public class IdempotencyInterceptor implements HandlerInterceptor { private final IdempotencyRecordRepository idempotencyRecordRepository; private final ObjectMapper objectMapper; private final Clock clock; + private final RequestIdAccessor requestIdAccessor; public IdempotencyInterceptor(StringRedisTemplate redisTemplate, IdempotencyRecordRepository idempotencyRecordRepository, ObjectMapper objectMapper, - Clock clock) { + Clock clock, + RequestIdAccessor requestIdAccessor) { this.redisTemplate = redisTemplate; this.idempotencyRecordRepository = idempotencyRecordRepository; this.objectMapper = objectMapper; this.clock = clock; + this.requestIdAccessor = requestIdAccessor; } /** @@ -56,8 +60,8 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons return true; } - String requestId = request.getHeader(REQUEST_ID_HEADER); - if (requestId == null || requestId.isEmpty()) { + String requestId = resolveRequestId(request); + if (requestId == null) { return true; } @@ -116,8 +120,8 @@ public void afterCompletion(HttpServletRequest request, HttpServletResponse resp return; } - String requestId = request.getHeader(REQUEST_ID_HEADER); - if (requestId == null || requestId.isEmpty()) { + String requestId = resolveRequestId(request); + if (requestId == null) { return; } @@ -139,8 +143,16 @@ public void afterCompletion(HttpServletRequest request, HttpServletResponse resp private void writeDuplicateResponse(HttpServletResponse response) throws Exception { ApiResponse body = new ApiResponse<>(409, "error.request.duplicate", null, - Instant.now(clock), null); + Instant.now(clock), requestIdAccessor.current()); response.setContentType("application/json;charset=UTF-8"); response.getWriter().write(objectMapper.writeValueAsString(body)); } + + private String resolveRequestId(HttpServletRequest request) { + String suppliedRequestId = request.getHeader(REQUEST_ID_HEADER); + if (suppliedRequestId == null || suppliedRequestId.isEmpty()) { + return null; + } + return requestIdAccessor.current(); + } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/RequestIdFilter.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/RequestIdFilter.java index cc5c09323..f5b88bc31 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/RequestIdFilter.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/RequestIdFilter.java @@ -1,10 +1,10 @@ package com.iflytek.skillhub.filter; +import com.iflytek.skillhub.observability.RequestIdAccessor; import jakarta.servlet.FilterChain; import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import org.slf4j.MDC; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; import org.springframework.stereotype.Component; @@ -12,6 +12,7 @@ import java.io.IOException; import java.util.UUID; +import java.util.regex.Pattern; /** * Ensures every request has a request identifier for logs, responses, and downstream audit @@ -22,23 +23,27 @@ public class RequestIdFilter extends OncePerRequestFilter { private static final String REQUEST_ID_HEADER = "X-Request-Id"; - private static final String REQUEST_ID_MDC_KEY = "requestId"; + private static final Pattern VALID_REQUEST_ID = + Pattern.compile("^[A-Za-z0-9][A-Za-z0-9._:-]{0,63}$"); + + private final RequestIdAccessor requestIdAccessor; + + public RequestIdFilter(RequestIdAccessor requestIdAccessor) { + this.requestIdAccessor = requestIdAccessor; + } @Override protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { String requestId = request.getHeader(REQUEST_ID_HEADER); - if (requestId == null || requestId.isBlank()) { + if (requestId == null || !VALID_REQUEST_ID.matcher(requestId).matches()) { requestId = UUID.randomUUID().toString(); } - MDC.put(REQUEST_ID_MDC_KEY, requestId); response.setHeader(REQUEST_ID_HEADER, requestId); - try { + try (RequestIdAccessor.Scope ignored = requestIdAccessor.open(requestId)) { filterChain.doFilter(request, response); - } finally { - MDC.remove(REQUEST_ID_MDC_KEY); } } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/RequestIdAccessor.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/RequestIdAccessor.java new file mode 100644 index 000000000..ecdfb8038 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/RequestIdAccessor.java @@ -0,0 +1,78 @@ +package com.iflytek.skillhub.observability; + +import org.slf4j.MDC; +import org.springframework.stereotype.Component; + +import java.util.Objects; + +/** + * Holds the current SkillHub request identifier independently from the logging implementation. + * + *

The thread-local value is authoritative. MDC is maintained only as a mirror for log + * correlation.

+ */ +@Component +public class RequestIdAccessor { + + public static final String MDC_KEY = "requestId"; + + private final ThreadLocal currentRequestId = new ThreadLocal<>(); + + /** + * Returns the current request identifier, or {@code null} outside a request/task scope. + */ + public String current() { + return currentRequestId.get(); + } + + /** + * Opens a nested request identifier scope on the current thread. + */ + public Scope open(String requestId) { + Objects.requireNonNull(requestId, "requestId must not be null"); + if (requestId.isBlank()) { + throw new IllegalArgumentException("requestId must not be blank"); + } + + String previousRequestId = currentRequestId.get(); + replace(requestId); + return new Scope(previousRequestId, requestId); + } + + void replace(String requestId) { + if (requestId == null) { + currentRequestId.remove(); + MDC.remove(MDC_KEY); + return; + } + currentRequestId.set(requestId); + MDC.put(MDC_KEY, requestId); + } + + /** + * A same-thread, LIFO scope for the request identifier. + */ + public final class Scope implements AutoCloseable { + + private final String previousRequestId; + private final String installedRequestId; + private boolean closed; + + private Scope(String previousRequestId, String installedRequestId) { + this.previousRequestId = previousRequestId; + this.installedRequestId = installedRequestId; + } + + @Override + public void close() { + if (closed) { + return; + } + if (!Objects.equals(currentRequestId.get(), installedRequestId)) { + throw new IllegalStateException("Request ID scopes must close on the owning thread in LIFO order"); + } + replace(previousRequestId); + closed = true; + } + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/CorrelationJsonProvider.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/CorrelationJsonProvider.java new file mode 100644 index 000000000..814f33cee --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/CorrelationJsonProvider.java @@ -0,0 +1,50 @@ +package com.iflytek.skillhub.observability.logging; + +import ch.qos.logback.classic.spi.ILoggingEvent; +import com.fasterxml.jackson.core.JsonGenerator; +import net.logstash.logback.composite.AbstractJsonProvider; + +import java.io.IOException; +import java.util.Map; + +/** + * Writes only the approved correlation fields from MDC. + */ +final class CorrelationJsonProvider extends AbstractJsonProvider { + + private static final String REQUEST_ID_KEY = "requestId"; + private static final String TRACE_ID_KEY = "traceId"; + private static final String SPAN_ID_KEY = "spanId"; + private static final String EXTERNAL_TRACE_ID_KEY = "tid"; + + @Override + public void writeTo(JsonGenerator generator, ILoggingEvent event) throws IOException { + Map mdc = event.getMDCPropertyMap(); + if (mdc == null || mdc.isEmpty()) { + return; + } + + writeIfPresent(generator, "request.id", mdc.get(REQUEST_ID_KEY)); + writeIfPresent( + generator, + "trace.id", + firstPresent(mdc.get(TRACE_ID_KEY), mdc.get(EXTERNAL_TRACE_ID_KEY)) + ); + writeIfPresent(generator, "span.id", mdc.get(SPAN_ID_KEY)); + } + + private String firstPresent(String preferred, String fallback) { + return isPresent(preferred) ? preferred : fallback; + } + + private void writeIfPresent(JsonGenerator generator, String fieldName, String value) + throws IOException { + if (isPresent(value)) { + generator.writeStringField(fieldName, value); + } + } + + private boolean isPresent(String value) { + return value != null && !value.isBlank(); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/SkillHubEcsEncoder.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/SkillHubEcsEncoder.java new file mode 100644 index 000000000..7761b49dd --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/SkillHubEcsEncoder.java @@ -0,0 +1,108 @@ +package com.iflytek.skillhub.observability.logging; + +import ch.qos.logback.classic.spi.ILoggingEvent; +import com.fasterxml.jackson.databind.node.JsonNodeFactory; +import com.fasterxml.jackson.databind.node.ObjectNode; +import net.logstash.logback.composite.GlobalCustomFieldsJsonProvider; +import net.logstash.logback.composite.loggingevent.LogLevelJsonProvider; +import net.logstash.logback.composite.loggingevent.LoggerNameJsonProvider; +import net.logstash.logback.composite.loggingevent.LoggingEventFormattedTimestampJsonProvider; +import net.logstash.logback.composite.loggingevent.LoggingEventJsonProviders; +import net.logstash.logback.composite.loggingevent.LoggingEventThreadNameJsonProvider; +import net.logstash.logback.composite.loggingevent.MessageJsonProvider; +import net.logstash.logback.composite.loggingevent.StackTraceJsonProvider; +import net.logstash.logback.composite.loggingevent.ThrowableClassNameJsonProvider; +import net.logstash.logback.composite.loggingevent.ThrowableMessageJsonProvider; +import net.logstash.logback.encoder.LoggingEventCompositeJsonEncoder; + +/** + * ECS-style JSON encoder with an explicit field allowlist. + */ +public class SkillHubEcsEncoder extends LoggingEventCompositeJsonEncoder { + + private static final String ECS_VERSION = "1.2.0"; + + private String serviceName = "skillhub"; + private String serviceVersion = "unknown"; + private String serviceEnvironment = "local"; + + @Override + public void start() { + if (isStarted()) { + return; + } + setLineSeparator("UNIX"); + setProviders(createProviders()); + super.start(); + } + + public void setServiceName(String serviceName) { + this.serviceName = serviceName; + } + + public void setServiceVersion(String serviceVersion) { + this.serviceVersion = serviceVersion; + } + + public void setServiceEnvironment(String serviceEnvironment) { + this.serviceEnvironment = serviceEnvironment; + } + + private LoggingEventJsonProviders createProviders() { + LoggingEventJsonProviders providers = new LoggingEventJsonProviders(); + + LoggingEventFormattedTimestampJsonProvider timestamp = + new LoggingEventFormattedTimestampJsonProvider(); + timestamp.setFieldName("@timestamp"); + timestamp.setTimeZone("UTC"); + providers.addTimestamp(timestamp); + + LogLevelJsonProvider level = new LogLevelJsonProvider(); + level.setFieldName("log.level"); + providers.addLogLevel(level); + + MessageJsonProvider message = new MessageJsonProvider(); + message.setFieldName("message"); + providers.addMessage(message); + + LoggerNameJsonProvider logger = new LoggerNameJsonProvider(); + logger.setFieldName("log.logger"); + providers.addLoggerName(logger); + + LoggingEventThreadNameJsonProvider thread = new LoggingEventThreadNameJsonProvider(); + thread.setFieldName("process.thread.name"); + providers.addThreadName(thread); + + providers.addGlobalCustomFields(serviceFields()); + providers.addProvider(new CorrelationJsonProvider()); + + ThrowableClassNameJsonProvider errorType = new ThrowableClassNameJsonProvider(); + errorType.setFieldName("error.type"); + errorType.setUseSimpleClassName(false); + providers.addThrowableClassName(errorType); + + ThrowableMessageJsonProvider errorMessage = new ThrowableMessageJsonProvider(); + errorMessage.setFieldName("error.message"); + providers.addThrowableMessage(errorMessage); + + StackTraceJsonProvider stackTrace = new StackTraceJsonProvider(); + stackTrace.setFieldName("error.stack_trace"); + providers.addStackTrace(stackTrace); + + return providers; + } + + private GlobalCustomFieldsJsonProvider serviceFields() { + ObjectNode fields = JsonNodeFactory.instance.objectNode(); + fields.put("ecs.version", ECS_VERSION); + fields.put("service.name", serviceName); + fields.put("service.version", serviceVersion); + fields.put("service.environment", serviceEnvironment); + fields.put("event.dataset", serviceName); + + GlobalCustomFieldsJsonProvider provider = + new GlobalCustomFieldsJsonProvider<>(); + provider.setCustomFieldsNode(fields); + return provider; + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/package-info.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/package-info.java new file mode 100644 index 000000000..6877f0ca4 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/package-info.java @@ -0,0 +1,4 @@ +/** + * Structured logging adapters for SkillHub correlation fields. + */ +package com.iflytek.skillhub.observability.logging; diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/package-info.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/package-info.java new file mode 100644 index 000000000..2c10c3550 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/package-info.java @@ -0,0 +1,4 @@ +/** + * Application-level observability context and integration boundaries. + */ +package com.iflytek.skillhub.observability; diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/security/ApiAccessDeniedHandler.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/security/ApiAccessDeniedHandler.java index 2c930aa64..36ea746eb 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/security/ApiAccessDeniedHandler.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/security/ApiAccessDeniedHandler.java @@ -4,11 +4,11 @@ import com.iflytek.skillhub.auth.token.ApiTokenAccessDeniedException; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; +import com.iflytek.skillhub.observability.RequestIdAccessor; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.slf4j.MDC; import org.springframework.http.MediaType; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.web.access.AccessDeniedHandler; @@ -26,13 +26,16 @@ public class ApiAccessDeniedHandler implements AccessDeniedHandler { private final ObjectMapper objectMapper; private final ApiResponseFactory apiResponseFactory; private final SensitiveLogSanitizer sensitiveLogSanitizer; + private final RequestIdAccessor requestIdAccessor; public ApiAccessDeniedHandler(ObjectMapper objectMapper, ApiResponseFactory apiResponseFactory, - SensitiveLogSanitizer sensitiveLogSanitizer) { + SensitiveLogSanitizer sensitiveLogSanitizer, + RequestIdAccessor requestIdAccessor) { this.objectMapper = objectMapper; this.apiResponseFactory = apiResponseFactory; this.sensitiveLogSanitizer = sensitiveLogSanitizer; + this.requestIdAccessor = requestIdAccessor; } @Override @@ -45,7 +48,7 @@ public void handle(HttpServletRequest request, : null; logger.info( "Forbidden API request [requestId={}, method={}, path={}, reason={}, detail={}]", - MDC.get("requestId"), + requestIdAccessor.current(), request.getMethod(), sensitiveLogSanitizer.sanitizeRequestTarget(request), accessDeniedException.getClass().getSimpleName(), diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/security/ApiAuthenticationEntryPoint.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/security/ApiAuthenticationEntryPoint.java index 8f5de8d25..50c827685 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/security/ApiAuthenticationEntryPoint.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/security/ApiAuthenticationEntryPoint.java @@ -3,11 +3,11 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; +import com.iflytek.skillhub.observability.RequestIdAccessor; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.slf4j.MDC; import org.springframework.http.MediaType; import org.springframework.security.core.AuthenticationException; import org.springframework.security.web.AuthenticationEntryPoint; @@ -25,13 +25,16 @@ public class ApiAuthenticationEntryPoint implements AuthenticationEntryPoint { private final ObjectMapper objectMapper; private final ApiResponseFactory apiResponseFactory; private final SensitiveLogSanitizer sensitiveLogSanitizer; + private final RequestIdAccessor requestIdAccessor; public ApiAuthenticationEntryPoint(ObjectMapper objectMapper, ApiResponseFactory apiResponseFactory, - SensitiveLogSanitizer sensitiveLogSanitizer) { + SensitiveLogSanitizer sensitiveLogSanitizer, + RequestIdAccessor requestIdAccessor) { this.objectMapper = objectMapper; this.apiResponseFactory = apiResponseFactory; this.sensitiveLogSanitizer = sensitiveLogSanitizer; + this.requestIdAccessor = requestIdAccessor; } @Override @@ -40,7 +43,7 @@ public void commence(HttpServletRequest request, AuthenticationException authException) throws IOException { logger.info( "Unauthorized API request [requestId={}, method={}, path={}, reason={}]", - MDC.get("requestId"), + requestIdAccessor.current(), request.getMethod(), sensitiveLogSanitizer.sanitizeRequestTarget(request), authException.getClass().getSimpleName() diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/LabelAdminAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/LabelAdminAppService.java index 23bace70c..88316c508 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/LabelAdminAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/LabelAdminAppService.java @@ -11,9 +11,9 @@ import com.iflytek.skillhub.dto.LabelDefinitionResponse; import com.iflytek.skillhub.dto.LabelSortOrderUpdateRequest; import com.iflytek.skillhub.dto.LabelTranslationResponse; +import com.iflytek.skillhub.observability.RequestIdAccessor; import java.util.List; import java.util.Set; -import org.slf4j.MDC; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.support.TransactionSynchronization; @@ -27,17 +27,20 @@ public class LabelAdminAppService { private final AuditLogService auditLogService; private final RbacService rbacService; private final LabelSearchSyncService labelSearchSyncService; + private final RequestIdAccessor requestIdAccessor; public LabelAdminAppService(LabelDefinitionService labelDefinitionService, SkillLabelService skillLabelService, AuditLogService auditLogService, RbacService rbacService, - LabelSearchSyncService labelSearchSyncService) { + LabelSearchSyncService labelSearchSyncService, + RequestIdAccessor requestIdAccessor) { this.labelDefinitionService = labelDefinitionService; this.skillLabelService = skillLabelService; this.auditLogService = auditLogService; this.rbacService = rbacService; this.labelSearchSyncService = labelSearchSyncService; + this.requestIdAccessor = requestIdAccessor; } public List listAll() { @@ -153,7 +156,7 @@ private void recordAudit(String action, action, "LABEL", targetId, - MDC.get("requestId"), + requestIdAccessor.current(), auditContext != null ? auditContext.clientIp() : null, auditContext != null ? auditContext.userAgent() : null, detailJson diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/PromotionPortalAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/PromotionPortalAppService.java index aab28382c..1c7f6b44c 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/PromotionPortalAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/PromotionPortalAppService.java @@ -12,11 +12,11 @@ import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; import com.iflytek.skillhub.dto.PageResponse; import com.iflytek.skillhub.dto.PromotionResponseDto; +import com.iflytek.skillhub.observability.RequestIdAccessor; import com.iflytek.skillhub.repository.GovernanceQueryRepository; import java.util.Locale; import java.util.Map; import java.util.Set; -import org.slf4j.MDC; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.PageRequest; @@ -32,17 +32,20 @@ public class PromotionPortalAppService { private final GovernanceQueryRepository governanceQueryRepository; private final RbacService rbacService; private final AuditLogService auditLogService; + private final RequestIdAccessor requestIdAccessor; public PromotionPortalAppService(PromotionService promotionService, PromotionRequestRepository promotionRequestRepository, GovernanceQueryRepository governanceQueryRepository, RbacService rbacService, - AuditLogService auditLogService) { + AuditLogService auditLogService, + RequestIdAccessor requestIdAccessor) { this.promotionService = promotionService; this.promotionRequestRepository = promotionRequestRepository; this.governanceQueryRepository = governanceQueryRepository; this.rbacService = rbacService; this.auditLogService = auditLogService; + this.requestIdAccessor = requestIdAccessor; } public PromotionResponseDto submitPromotion(Long sourceSkillId, @@ -235,7 +238,7 @@ private void recordAudit(String action, action, "PROMOTION_REQUEST", targetId, - MDC.get("requestId"), + requestIdAccessor.current(), auditContext != null ? auditContext.clientIp() : null, auditContext != null ? auditContext.userAgent() : null, detailJson diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewPortalAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewPortalAppService.java index 47d8eed5b..96663c9e2 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewPortalAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewPortalAppService.java @@ -13,11 +13,11 @@ import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; import com.iflytek.skillhub.dto.PageResponse; import com.iflytek.skillhub.dto.ReviewTaskResponse; +import com.iflytek.skillhub.observability.RequestIdAccessor; import com.iflytek.skillhub.repository.GovernanceQueryRepository; import java.util.List; import java.util.Map; import java.util.Set; -import org.slf4j.MDC; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; @@ -34,19 +34,22 @@ public class ReviewPortalAppService { private final GovernanceQueryRepository governanceQueryRepository; private final RbacService rbacService; private final AuditLogService auditLogService; + private final RequestIdAccessor requestIdAccessor; public ReviewPortalAppService(ReviewService reviewService, ReviewTaskRepository reviewTaskRepository, NamespaceRepository namespaceRepository, GovernanceQueryRepository governanceQueryRepository, RbacService rbacService, - AuditLogService auditLogService) { + AuditLogService auditLogService, + RequestIdAccessor requestIdAccessor) { this.reviewService = reviewService; this.reviewTaskRepository = reviewTaskRepository; this.namespaceRepository = namespaceRepository; this.governanceQueryRepository = governanceQueryRepository; this.rbacService = rbacService; this.auditLogService = auditLogService; + this.requestIdAccessor = requestIdAccessor; } public ReviewTaskResponse submitReview(Long skillVersionId, @@ -256,7 +259,7 @@ private void recordAudit(String action, action, "REVIEW_TASK", targetId, - MDC.get("requestId"), + requestIdAccessor.current(), auditContext != null ? auditContext.clientIp() : null, auditContext != null ? auditContext.userAgent() : null, detailJson diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLabelAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLabelAppService.java index 25b92081f..5947934e7 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLabelAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLabelAppService.java @@ -18,12 +18,12 @@ import com.iflytek.skillhub.domain.skill.service.SkillSlugResolutionService; import com.iflytek.skillhub.dto.MessageResponse; import com.iflytek.skillhub.dto.SkillLabelDto; +import com.iflytek.skillhub.observability.RequestIdAccessor; import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; -import org.slf4j.MDC; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.support.TransactionSynchronization; @@ -42,6 +42,7 @@ public class SkillLabelAppService { private final AuditLogService auditLogService; private final LabelSearchSyncService labelSearchSyncService; private final SkillSlugResolutionService skillSlugResolutionService; + private final RequestIdAccessor requestIdAccessor; public SkillLabelAppService(NamespaceRepository namespaceRepository, SkillRepository skillRepository, @@ -52,7 +53,8 @@ public SkillLabelAppService(NamespaceRepository namespaceRepository, RbacService rbacService, AuditLogService auditLogService, LabelSearchSyncService labelSearchSyncService, - SkillSlugResolutionService skillSlugResolutionService) { + SkillSlugResolutionService skillSlugResolutionService, + RequestIdAccessor requestIdAccessor) { this.namespaceRepository = namespaceRepository; this.skillRepository = skillRepository; this.visibilityChecker = visibilityChecker; @@ -63,6 +65,7 @@ public SkillLabelAppService(NamespaceRepository namespaceRepository, this.auditLogService = auditLogService; this.labelSearchSyncService = labelSearchSyncService; this.skillSlugResolutionService = skillSlugResolutionService; + this.requestIdAccessor = requestIdAccessor; } public List listSkillLabels(String namespaceSlug, @@ -188,7 +191,7 @@ private void recordAudit(String action, action, "SKILL", targetId, - MDC.get("requestId"), + requestIdAccessor.current(), auditContext != null ? auditContext.clientIp() : null, auditContext != null ? auditContext.userAgent() : null, detailJson diff --git a/server/skillhub-app/src/main/resources/application.yml b/server/skillhub-app/src/main/resources/application.yml index 421e27f68..9983d5e8e 100644 --- a/server/skillhub-app/src/main/resources/application.yml +++ b/server/skillhub-app/src/main/resources/application.yml @@ -94,6 +94,11 @@ spring: enable: ${SPRING_MAIL_SMTP_STARTTLS_ENABLE:false} skillhub: + observability: + log-format: ${SKILLHUB_LOG_FORMAT:text} + log-async-queue-size: ${SKILLHUB_LOG_ASYNC_QUEUE_SIZE:1024} + service-version: ${SKILLHUB_SERVICE_VERSION:unknown} + service-environment: ${SKILLHUB_SERVICE_ENVIRONMENT:local} builtin-skills: enabled: ${SKILLHUB_BUILTIN_SKILLS_ENABLED:true} redis: diff --git a/server/skillhub-app/src/main/resources/logback-spring.xml b/server/skillhub-app/src/main/resources/logback-spring.xml new file mode 100644 index 000000000..30152e8ef --- /dev/null +++ b/server/skillhub-app/src/main/resources/logback-spring.xml @@ -0,0 +1,47 @@ + + + + + + + + + + + + + ${CONSOLE_LOG_CHARSET} + ${CONSOLE_LOG_PATTERN} + + + + + + ${serviceName} + ${serviceVersion} + ${serviceEnvironment} + + + + + ${asyncQueueSize} + 0 + true + false + + + + + + + diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatAppServiceTest.java index c5921f59b..f5eac2d11 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatAppServiceTest.java @@ -15,6 +15,7 @@ import com.iflytek.skillhub.domain.skill.service.SkillPublishService; import com.iflytek.skillhub.domain.skill.service.SkillQueryService; import com.iflytek.skillhub.domain.social.SkillStarService; +import com.iflytek.skillhub.observability.RequestIdAccessor; import com.iflytek.skillhub.service.SkillSearchAppService; import java.util.Map; import java.util.Optional; @@ -40,7 +41,8 @@ class ClawHubCompatAppServiceTest { multipartPackageExtractor, auditLogService, compatSkillLookupService, - skillStarService + skillStarService, + new RequestIdAccessor() ); @Test diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/UserProfileControllerUnitTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/UserProfileControllerUnitTest.java index 43b35ff18..d3e117952 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/UserProfileControllerUnitTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/UserProfileControllerUnitTest.java @@ -10,6 +10,7 @@ import com.iflytek.skillhub.domain.user.UserAccountRepository; import com.iflytek.skillhub.domain.user.UserProfileService; import com.iflytek.skillhub.dto.ApiResponseFactory; +import com.iflytek.skillhub.observability.RequestIdAccessor; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -54,9 +55,11 @@ class UserProfileControllerUnitTest { void setUp() { StaticMessageSource messageSource = new StaticMessageSource(); messageSource.addMessage("response.success.read", Locale.getDefault(), "response.success.read"); + RequestIdAccessor requestIdAccessor = new RequestIdAccessor(); ApiResponseFactory responseFactory = new ApiResponseFactory( messageSource, - Clock.fixed(Instant.parse("2026-03-19T08:00:00Z"), ZoneOffset.UTC) + Clock.fixed(Instant.parse("2026-03-19T08:00:00Z"), ZoneOffset.UTC), + requestIdAccessor ); controller = new UserProfileController( responseFactory, @@ -64,7 +67,8 @@ void setUp() { userAccountRepository, changeRequestRepository, platformSessionService, - fieldPolicyConfig + fieldPolicyConfig, + requestIdAccessor ); given(fieldPolicyConfig.fieldPolicies()).willReturn(Map.of( "displayName", new ProfileFieldPolicyConfig.FieldPolicy(true, false), diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/NotificationControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/NotificationControllerTest.java index 2f344f2cd..f44d29414 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/NotificationControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/NotificationControllerTest.java @@ -12,6 +12,7 @@ import com.iflytek.skillhub.notification.domain.NotificationCategory; import com.iflytek.skillhub.notification.service.NotificationService; import com.iflytek.skillhub.notification.sse.SseEmitterManager; +import com.iflytek.skillhub.observability.RequestIdAccessor; import java.time.Clock; import java.time.Instant; import java.time.ZoneOffset; @@ -42,7 +43,8 @@ void setUp() { messageSource.addMessage("response.success.read", java.util.Locale.getDefault(), "ok"); ApiResponseFactory responseFactory = new ApiResponseFactory( messageSource, - Clock.fixed(Instant.parse("2026-03-20T00:00:00Z"), ZoneOffset.UTC) + Clock.fixed(Instant.parse("2026-03-20T00:00:00Z"), ZoneOffset.UTC), + new RequestIdAccessor() ); controller = new NotificationController(notificationService, sseEmitterManager, new ObjectMapper(), responseFactory); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/NotificationPreferenceControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/NotificationPreferenceControllerTest.java index c89520c30..a160323f7 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/NotificationPreferenceControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/NotificationPreferenceControllerTest.java @@ -15,6 +15,7 @@ import com.iflytek.skillhub.notification.domain.NotificationChannel; import com.iflytek.skillhub.notification.service.NotificationPreferenceService; import com.iflytek.skillhub.notification.service.NotificationPreferenceService.PreferenceView; +import com.iflytek.skillhub.observability.RequestIdAccessor; import java.time.Clock; import java.time.Instant; import java.time.ZoneOffset; @@ -41,7 +42,8 @@ void setUp() { messageSource.addMessage("response.success.updated", java.util.Locale.getDefault(), "ok"); ApiResponseFactory responseFactory = new ApiResponseFactory( messageSource, - Clock.fixed(Instant.parse("2026-03-23T00:00:00Z"), ZoneOffset.UTC) + Clock.fixed(Instant.parse("2026-03-23T00:00:00Z"), ZoneOffset.UTC), + new RequestIdAccessor() ); controller = new NotificationPreferenceController(preferenceService, responseFactory); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java index d8c4ac64e..6aae4cb05 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java @@ -7,6 +7,7 @@ import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; import com.iflytek.skillhub.metrics.SkillHubMetrics; +import com.iflytek.skillhub.observability.RequestIdAccessor; import com.iflytek.skillhub.security.SensitiveLogSanitizer; import jakarta.servlet.http.HttpServletRequest; import java.time.Clock; @@ -40,11 +41,18 @@ class GlobalExceptionHandlerTest { void setUp() { StaticMessageSource messageSource = new StaticMessageSource(); messageSource.addMessage("error.request.timeout", java.util.Locale.getDefault(), "Request timed out"); + RequestIdAccessor requestIdAccessor = new RequestIdAccessor(); ApiResponseFactory responseFactory = new ApiResponseFactory( messageSource, - Clock.fixed(Instant.parse("2026-03-20T00:00:00Z"), ZoneOffset.UTC) + Clock.fixed(Instant.parse("2026-03-20T00:00:00Z"), ZoneOffset.UTC), + requestIdAccessor + ); + handler = new GlobalExceptionHandler( + responseFactory, + sensitiveLogSanitizer, + metrics, + requestIdAccessor ); - handler = new GlobalExceptionHandler(responseFactory, sensitiveLogSanitizer, metrics); } @Test diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/AuthContextFilterTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/AuthContextFilterTest.java index 6bf563374..f3d2d117e 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/AuthContextFilterTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/AuthContextFilterTest.java @@ -11,6 +11,7 @@ import com.iflytek.skillhub.domain.user.UserAccountRepository; import com.iflytek.skillhub.domain.user.UserStatus; import com.iflytek.skillhub.dto.ApiResponseFactory; +import com.iflytek.skillhub.observability.RequestIdAccessor; import jakarta.servlet.FilterChain; import jakarta.servlet.http.HttpSession; import java.time.Clock; @@ -47,7 +48,8 @@ class AuthContextFilterTest { StaticMessageSource messageSource = new StaticMessageSource(); messageSource.addMessage("error.auth.local.accountDisabled", Locale.ENGLISH, "This account has been disabled"); Clock clock = Clock.fixed(Instant.parse("2026-03-18T00:00:00Z"), ZoneOffset.UTC); - ApiResponseFactory apiResponseFactory = new ApiResponseFactory(messageSource, clock); + ApiResponseFactory apiResponseFactory = + new ApiResponseFactory(messageSource, clock, new RequestIdAccessor()); filter = new AuthContextFilter( namespaceMemberRepository, userAccountRepository, diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/IdempotencyInterceptorTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/IdempotencyInterceptorTest.java index 485534fb6..c7e8be1f9 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/IdempotencyInterceptorTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/IdempotencyInterceptorTest.java @@ -5,6 +5,7 @@ import com.iflytek.skillhub.domain.idempotency.IdempotencyRecord; import com.iflytek.skillhub.domain.idempotency.IdempotencyRecordRepository; import com.iflytek.skillhub.domain.idempotency.IdempotencyStatus; +import com.iflytek.skillhub.observability.RequestIdAccessor; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import org.junit.jupiter.api.BeforeEach; @@ -38,6 +39,8 @@ class IdempotencyInterceptorTest { @Mock private ValueOperations valueOperations; + @Mock + private RequestIdAccessor requestIdAccessor; @Mock private HttpServletRequest request; @@ -53,13 +56,20 @@ void setUp() { ObjectMapper objectMapper = new ObjectMapper(); objectMapper.registerModule(new JavaTimeModule()); clock = Clock.fixed(Instant.parse("2026-03-18T00:00:00Z"), ZoneOffset.UTC); - interceptor = new IdempotencyInterceptor(redisTemplate, idempotencyRecordRepository, objectMapper, clock); + interceptor = new IdempotencyInterceptor( + redisTemplate, + idempotencyRecordRepository, + objectMapper, + clock, + requestIdAccessor + ); } @Test void testNewRequestPassesThrough() throws Exception { when(request.getMethod()).thenReturn("POST"); when(request.getHeader("X-Request-Id")).thenReturn("req-123"); + when(requestIdAccessor.current()).thenReturn("req-123"); when(redisTemplate.opsForValue()).thenReturn(valueOperations); when(valueOperations.get("idempotency:req-123")).thenReturn(null); when(idempotencyRecordRepository.findByRequestId("req-123")).thenReturn(Optional.empty()); @@ -70,10 +80,28 @@ void testNewRequestPassesThrough() throws Exception { verify(idempotencyRecordRepository).save(any(IdempotencyRecord.class)); } + @Test + void testProvidedInvalidHeaderUsesEffectiveRequestContext() throws Exception { + when(request.getMethod()).thenReturn("POST"); + when(request.getHeader("X-Request-Id")).thenReturn("invalid request id"); + when(requestIdAccessor.current()).thenReturn("generated-valid-id"); + when(redisTemplate.opsForValue()).thenReturn(valueOperations); + when(valueOperations.get("idempotency:generated-valid-id")).thenReturn(null); + when(idempotencyRecordRepository.findByRequestId("generated-valid-id")) + .thenReturn(Optional.empty()); + + boolean result = interceptor.preHandle(request, response, new Object()); + + assertTrue(result); + verify(idempotencyRecordRepository).findByRequestId("generated-valid-id"); + verify(idempotencyRecordRepository, never()).findByRequestId("invalid request id"); + } + @Test void testDuplicateRequestReturnsCachedResponse() throws Exception { when(request.getMethod()).thenReturn("POST"); when(request.getHeader("X-Request-Id")).thenReturn("req-456"); + when(requestIdAccessor.current()).thenReturn("req-456"); when(redisTemplate.opsForValue()).thenReturn(valueOperations); when(valueOperations.get("idempotency:req-456")).thenReturn("COMPLETED"); @@ -114,6 +142,7 @@ void testGetRequestPassesThrough() throws Exception { void testAfterCompletionUpdatesRecord() throws Exception { when(request.getMethod()).thenReturn("POST"); when(request.getHeader("X-Request-Id")).thenReturn("req-789"); + when(requestIdAccessor.current()).thenReturn("req-789"); when(response.getStatus()).thenReturn(200); when(redisTemplate.opsForValue()).thenReturn(valueOperations); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/RequestIdFilterTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/RequestIdFilterTest.java index 74ac9647b..9386e39ca 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/RequestIdFilterTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/RequestIdFilterTest.java @@ -1,14 +1,19 @@ package com.iflytek.skillhub.filter; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.MvcResult; +import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @SpringBootTest @@ -23,7 +28,8 @@ class RequestIdFilterTest { void shouldGenerateRequestIdWhenNotProvided() throws Exception { mockMvc.perform(get("/api/v1/health")) .andExpect(status().isOk()) - .andExpect(header().exists("X-Request-Id")); + .andExpect(header().exists("X-Request-Id")) + .andExpect(jsonPath("$.requestId").isNotEmpty()); } @Test @@ -32,6 +38,50 @@ void shouldPreserveProvidedRequestId() throws Exception { mockMvc.perform(get("/api/v1/health") .header("X-Request-Id", requestId)) .andExpect(status().isOk()) - .andExpect(header().string("X-Request-Id", requestId)); + .andExpect(header().string("X-Request-Id", requestId)) + .andExpect(jsonPath("$.requestId").value(requestId)); + } + + @Test + void shouldPreserveRequestIdAtMaximumLength() throws Exception { + String requestId = "a".repeat(64); + + mockMvc.perform(get("/api/v1/health") + .header("X-Request-Id", requestId)) + .andExpect(status().isOk()) + .andExpect(header().string("X-Request-Id", requestId)) + .andExpect(jsonPath("$.requestId").value(requestId)); + } + + @ParameterizedTest + @ValueSource(strings = { + "", + "-starts-with-symbol", + "contains space", + "contains/slash", + "包含中文" + }) + void shouldReplaceInvalidRequestId(String requestId) throws Exception { + assertInvalidRequestIdIsReplaced(requestId); + } + + @Test + void shouldReplaceRequestIdLongerThanMaximumLength() throws Exception { + assertInvalidRequestIdIsReplaced("a".repeat(65)); + } + + private void assertInvalidRequestIdIsReplaced(String invalidRequestId) throws Exception { + MvcResult result = mockMvc.perform(get("/api/v1/health") + .header("X-Request-Id", invalidRequestId)) + .andExpect(status().isOk()) + .andExpect(header().exists("X-Request-Id")) + .andExpect(jsonPath("$.requestId").isNotEmpty()) + .andReturn(); + + String effectiveRequestId = result.getResponse().getHeader("X-Request-Id"); + assertThat(effectiveRequestId) + .isNotEqualTo(invalidRequestId) + .matches("^[A-Za-z0-9][A-Za-z0-9._:-]{0,63}$"); + assertThat(result.getResponse().getContentAsString()).contains(effectiveRequestId); } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/RequestIdAccessorTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/RequestIdAccessorTest.java new file mode 100644 index 000000000..e2c49d7fe --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/RequestIdAccessorTest.java @@ -0,0 +1,61 @@ +package com.iflytek.skillhub.observability; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.slf4j.MDC; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; + +class RequestIdAccessorTest { + + private final RequestIdAccessor accessor = new RequestIdAccessor(); + + @AfterEach + void tearDown() { + MDC.clear(); + } + + @Test + void shouldMirrorRequestIdToMdcAndClearItWhenScopeCloses() { + assertThat(accessor.current()).isNull(); + assertThat(MDC.get(RequestIdAccessor.MDC_KEY)).isNull(); + + try (RequestIdAccessor.Scope ignored = accessor.open("req-123")) { + assertThat(accessor.current()).isEqualTo("req-123"); + assertThat(MDC.get(RequestIdAccessor.MDC_KEY)).isEqualTo("req-123"); + } + + assertThat(accessor.current()).isNull(); + assertThat(MDC.get(RequestIdAccessor.MDC_KEY)).isNull(); + } + + @Test + void shouldRestoreOuterScope() { + try (RequestIdAccessor.Scope ignored = accessor.open("outer")) { + try (RequestIdAccessor.Scope nested = accessor.open("inner")) { + assertThat(accessor.current()).isEqualTo("inner"); + } + assertThat(accessor.current()).isEqualTo("outer"); + assertThat(MDC.get(RequestIdAccessor.MDC_KEY)).isEqualTo("outer"); + } + } + + @Test + void shouldUseThreadLocalAsAuthorityWhenMdcIsChangedExternally() { + try (RequestIdAccessor.Scope ignored = accessor.open("authoritative")) { + MDC.put(RequestIdAccessor.MDC_KEY, "logging-only"); + + assertThat(accessor.current()).isEqualTo("authoritative"); + } + + assertThat(accessor.current()).isNull(); + assertThat(MDC.get(RequestIdAccessor.MDC_KEY)).isNull(); + } + + @Test + void shouldRejectBlankRequestId() { + assertThatIllegalArgumentException() + .isThrownBy(() -> accessor.open(" ")); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/logging/SkillHubEcsEncoderTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/logging/SkillHubEcsEncoderTest.java new file mode 100644 index 000000000..9d0df479a --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/logging/SkillHubEcsEncoderTest.java @@ -0,0 +1,116 @@ +package com.iflytek.skillhub.observability.logging; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.spi.LoggingEvent; +import ch.qos.logback.classic.spi.ThrowableProxy; +import ch.qos.logback.classic.util.LogbackMDCAdapter; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.nio.charset.StandardCharsets; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; + +class SkillHubEcsEncoderTest { + + private final ObjectMapper objectMapper = new ObjectMapper(); + private final LoggerContext loggerContext = new LoggerContext(); + private final SkillHubEcsEncoder encoder = new SkillHubEcsEncoder(); + + @BeforeEach + void setUp() { + loggerContext.setMDCAdapter(new LogbackMDCAdapter()); + encoder.setContext(loggerContext); + encoder.setServiceName("skillhub"); + encoder.setServiceVersion("test-sha"); + encoder.setServiceEnvironment("test"); + encoder.start(); + } + + @AfterEach + void tearDown() { + encoder.stop(); + loggerContext.stop(); + } + + @Test + void shouldWriteEcsFieldsAndOnlyApprovedMdcValues() throws Exception { + LoggingEvent event = event("hello"); + event.setMDCPropertyMap(Map.of( + "requestId", "req-123", + "traceId", "trace-123", + "spanId", "span-123", + "authorization", "must-not-leak", + "userEmail", "must-not-leak" + )); + + JsonNode json = encode(event); + + assertThat(json.path("log.level").asText()).isEqualTo("INFO"); + assertThat(json.path("log.logger").asText()).isEqualTo("test.logger"); + assertThat(json.path("message").asText()).isEqualTo("hello"); + assertThat(json.path("service.name").asText()).isEqualTo("skillhub"); + assertThat(json.path("service.version").asText()).isEqualTo("test-sha"); + assertThat(json.path("service.environment").asText()).isEqualTo("test"); + assertThat(json.path("request.id").asText()).isEqualTo("req-123"); + assertThat(json.path("trace.id").asText()).isEqualTo("trace-123"); + assertThat(json.path("span.id").asText()).isEqualTo("span-123"); + assertThat(json.has("authorization")).isFalse(); + assertThat(json.has("userEmail")).isFalse(); + } + + @Test + void shouldPreferMicrometerTraceIdOverExternalAgentFallback() throws Exception { + LoggingEvent event = event("trace precedence"); + event.setMDCPropertyMap(Map.of( + "traceId", "micrometer-trace", + "tid", "external-agent-trace" + )); + + JsonNode json = encode(event); + + assertThat(json.path("trace.id").asText()).isEqualTo("micrometer-trace"); + assertThat(json.fieldNames()).toIterable() + .filteredOn("trace.id"::equals) + .hasSize(1); + } + + @Test + void shouldWriteStructuredExceptionFields() throws Exception { + LoggingEvent event = event("failed"); + event.setThrowableProxy(new ThrowableProxy(new IllegalStateException("boom"))); + + JsonNode json = encode(event); + + assertThat(json.path("error.type").asText()) + .isEqualTo(IllegalStateException.class.getName()); + assertThat(json.path("error.message").asText()).isEqualTo("boom"); + assertThat(json.path("error.stack_trace").asText()) + .contains("IllegalStateException: boom"); + } + + private LoggingEvent event(String message) { + Logger logger = loggerContext.getLogger("test.logger"); + LoggingEvent event = new LoggingEvent( + getClass().getName(), + logger, + Level.INFO, + message, + null, + null + ); + event.setThreadName("test-thread"); + event.setTimeStamp(1_785_465_600_000L); + return event; + } + + private JsonNode encode(LoggingEvent event) throws Exception { + return objectMapper.readTree(new String(encoder.encode(event), StandardCharsets.UTF_8)); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/security/ApiAccessDeniedHandlerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/security/ApiAccessDeniedHandlerTest.java index db8982b75..00ca5e8a8 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/security/ApiAccessDeniedHandlerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/security/ApiAccessDeniedHandlerTest.java @@ -9,6 +9,7 @@ import com.iflytek.skillhub.auth.policy.RouteSecurityPolicyRegistry; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.dto.ApiResponseFactory; +import com.iflytek.skillhub.observability.RequestIdAccessor; import jakarta.servlet.FilterChain; import java.time.Clock; import java.time.Instant; @@ -19,7 +20,6 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.slf4j.MDC; import org.springframework.context.i18n.LocaleContextHolder; import org.springframework.context.support.ResourceBundleMessageSource; import org.springframework.mock.web.MockHttpServletRequest; @@ -33,28 +33,32 @@ class ApiAccessDeniedHandlerTest { private final ObjectMapper objectMapper = new ObjectMapper().findAndRegisterModules(); private ApiAccessDeniedHandler handler; + private RequestIdAccessor.Scope requestIdScope; @BeforeEach void setUp() { ResourceBundleMessageSource messageSource = new ResourceBundleMessageSource(); messageSource.setBasename("messages"); messageSource.setDefaultEncoding("UTF-8"); + RequestIdAccessor requestIdAccessor = new RequestIdAccessor(); ApiResponseFactory responseFactory = new ApiResponseFactory( messageSource, - Clock.fixed(Instant.parse("2026-07-28T00:00:00Z"), ZoneOffset.UTC) + Clock.fixed(Instant.parse("2026-07-28T00:00:00Z"), ZoneOffset.UTC), + requestIdAccessor ); handler = new ApiAccessDeniedHandler( objectMapper, responseFactory, - new SensitiveLogSanitizer() + new SensitiveLogSanitizer(), + requestIdAccessor ); - MDC.put("requestId", "req-610"); + requestIdScope = requestIdAccessor.open("req-610"); LocaleContextHolder.setLocale(Locale.ENGLISH); } @AfterEach void tearDown() { - MDC.clear(); + requestIdScope.close(); LocaleContextHolder.resetLocaleContext(); SecurityContextHolder.clearContext(); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/LabelAdminAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/LabelAdminAppServiceTest.java index d6092f652..60a527d34 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/LabelAdminAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/LabelAdminAppServiceTest.java @@ -14,6 +14,7 @@ import com.iflytek.skillhub.dto.LabelSortOrderItemRequest; import com.iflytek.skillhub.dto.LabelSortOrderUpdateRequest; import com.iflytek.skillhub.dto.LabelTranslationItemRequest; +import com.iflytek.skillhub.observability.RequestIdAccessor; import java.time.Instant; import java.util.List; import java.util.Set; @@ -40,7 +41,8 @@ class LabelAdminAppServiceTest { skillLabelService, auditLogService, rbacService, - labelSearchSyncService + labelSearchSyncService, + new RequestIdAccessor() ); @Test diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/PromotionPortalAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/PromotionPortalAppServiceTest.java index bc824b89e..9d8a5accb 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/PromotionPortalAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/PromotionPortalAppServiceTest.java @@ -6,6 +6,7 @@ import com.iflytek.skillhub.domain.review.PromotionRequestRepository; import com.iflytek.skillhub.domain.review.PromotionService; import com.iflytek.skillhub.dto.PromotionResponseDto; +import com.iflytek.skillhub.observability.RequestIdAccessor; import com.iflytek.skillhub.repository.GovernanceQueryRepository; import java.lang.reflect.Field; import java.util.Set; @@ -47,7 +48,8 @@ void setUp() { promotionRequestRepository, governanceQueryRepository, rbacService, - auditLogService + auditLogService, + new RequestIdAccessor() ); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLabelAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLabelAppServiceTest.java index e51b4e9a6..4d5521966 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLabelAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLabelAppServiceTest.java @@ -15,6 +15,7 @@ import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.domain.skill.VisibilityChecker; import com.iflytek.skillhub.domain.skill.service.SkillSlugResolutionService; +import com.iflytek.skillhub.observability.RequestIdAccessor; import java.lang.reflect.Field; import java.util.List; import java.util.Map; @@ -70,7 +71,8 @@ void setUp() { rbacService, auditLogService, labelSearchSyncService, - skillSlugResolutionService + skillSlugResolutionService, + new RequestIdAccessor() ); } From f805076c5ccafdcd379e34e676f221c17f44704a Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Fri, 31 Jul 2026 11:06:13 +0800 Subject: [PATCH 02/10] feat(observability): add selectable tracing modes Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- server/skillhub-app/pom.xml | 13 ++ .../logging/CorrelationJsonProvider.java | 39 +++++- .../logging/SkillHubEcsEncoder.java | 7 +- .../SkillHubObservabilityProperties.java | 20 +++ .../tracing/SkillHubTracingConfiguration.java | 54 ++++++++ .../observability/tracing/TracingMode.java | 10 ++ ...cingModeAutoConfigurationImportFilter.java | 51 +++++++ .../observability/tracing/package-info.java | 4 + .../main/resources/META-INF/spring.factories | 2 + .../src/main/resources/application.yml | 12 ++ .../src/main/resources/logback-spring.xml | 4 + .../logging/SkillHubEcsEncoderTest.java | 26 ++++ .../SkillHubTracingConfigurationTest.java | 124 ++++++++++++++++++ ...ModeAutoConfigurationImportFilterTest.java | 65 +++++++++ 14 files changed, 426 insertions(+), 5 deletions(-) create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/SkillHubObservabilityProperties.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/SkillHubTracingConfiguration.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/TracingMode.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/TracingModeAutoConfigurationImportFilter.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/package-info.java create mode 100644 server/skillhub-app/src/main/resources/META-INF/spring.factories create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/SkillHubTracingConfigurationTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/TracingModeAutoConfigurationImportFilterTest.java diff --git a/server/skillhub-app/pom.xml b/server/skillhub-app/pom.xml index 8914e00e7..413e7eafc 100644 --- a/server/skillhub-app/pom.xml +++ b/server/skillhub-app/pom.xml @@ -26,6 +26,19 @@ io.micrometer micrometer-registry-prometheus + + io.micrometer + micrometer-tracing-bridge-otel + + + io.opentelemetry + opentelemetry-exporter-otlp + + + org.apache.skywalking + apm-toolkit-logback-1.x + 9.6.0 + net.logstash.logback logstash-logback-encoder diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/CorrelationJsonProvider.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/CorrelationJsonProvider.java index 814f33cee..82c87fd55 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/CorrelationJsonProvider.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/CorrelationJsonProvider.java @@ -3,8 +3,10 @@ import ch.qos.logback.classic.spi.ILoggingEvent; import com.fasterxml.jackson.core.JsonGenerator; import net.logstash.logback.composite.AbstractJsonProvider; +import org.apache.skywalking.apm.toolkit.log.logback.v1.x.mdc.LogbackMDCPatternConverter; import java.io.IOException; +import java.util.List; import java.util.Map; /** @@ -17,22 +19,51 @@ final class CorrelationJsonProvider extends AbstractJsonProvider private static final String SPAN_ID_KEY = "spanId"; private static final String EXTERNAL_TRACE_ID_KEY = "tid"; + private final LogbackMDCPatternConverter externalTraceIdConverter; + + CorrelationJsonProvider(boolean externalTraceIdEnabled) { + if (externalTraceIdEnabled) { + externalTraceIdConverter = new LogbackMDCPatternConverter(); + externalTraceIdConverter.setOptionList(List.of(EXTERNAL_TRACE_ID_KEY)); + externalTraceIdConverter.start(); + } else { + externalTraceIdConverter = null; + } + } + @Override public void writeTo(JsonGenerator generator, ILoggingEvent event) throws IOException { Map mdc = event.getMDCPropertyMap(); - if (mdc == null || mdc.isEmpty()) { - return; - } + mdc = mdc == null ? Map.of() : mdc; writeIfPresent(generator, "request.id", mdc.get(REQUEST_ID_KEY)); writeIfPresent( generator, "trace.id", - firstPresent(mdc.get(TRACE_ID_KEY), mdc.get(EXTERNAL_TRACE_ID_KEY)) + firstPresent(mdc.get(TRACE_ID_KEY), externalTraceId(event, mdc)) ); writeIfPresent(generator, "span.id", mdc.get(SPAN_ID_KEY)); } + private String externalTraceId(ILoggingEvent event, Map mdc) { + String traceId = mdc.get(EXTERNAL_TRACE_ID_KEY); + if (!isPresent(traceId) && externalTraceIdConverter != null) { + traceId = externalTraceIdConverter.convert(event); + } + return normalizeExternalTraceId(traceId); + } + + private String normalizeExternalTraceId(String traceId) { + if (!isPresent(traceId)) { + return null; + } + String normalized = traceId.trim(); + if (normalized.regionMatches(true, 0, "TID:", 0, 4)) { + normalized = normalized.substring(4).trim(); + } + return "N/A".equalsIgnoreCase(normalized) ? null : normalized; + } + private String firstPresent(String preferred, String fallback) { return isPresent(preferred) ? preferred : fallback; } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/SkillHubEcsEncoder.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/SkillHubEcsEncoder.java index 7761b49dd..8dc728bb2 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/SkillHubEcsEncoder.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/logging/SkillHubEcsEncoder.java @@ -25,6 +25,7 @@ public class SkillHubEcsEncoder extends LoggingEventCompositeJsonEncoder { private String serviceName = "skillhub"; private String serviceVersion = "unknown"; private String serviceEnvironment = "local"; + private boolean externalTraceIdEnabled; @Override public void start() { @@ -48,6 +49,10 @@ public void setServiceEnvironment(String serviceEnvironment) { this.serviceEnvironment = serviceEnvironment; } + public void setTracingMode(String tracingMode) { + this.externalTraceIdEnabled = "external-agent".equalsIgnoreCase(tracingMode); + } + private LoggingEventJsonProviders createProviders() { LoggingEventJsonProviders providers = new LoggingEventJsonProviders(); @@ -74,7 +79,7 @@ private LoggingEventJsonProviders createProviders() { providers.addThreadName(thread); providers.addGlobalCustomFields(serviceFields()); - providers.addProvider(new CorrelationJsonProvider()); + providers.addProvider(new CorrelationJsonProvider(externalTraceIdEnabled)); ThrowableClassNameJsonProvider errorType = new ThrowableClassNameJsonProvider(); errorType.setFieldName("error.type"); diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/SkillHubObservabilityProperties.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/SkillHubObservabilityProperties.java new file mode 100644 index 000000000..c27a3bfc2 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/SkillHubObservabilityProperties.java @@ -0,0 +1,20 @@ +package com.iflytek.skillhub.observability.tracing; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +/** + * Startup-time observability choices owned by SkillHub. + */ +@ConfigurationProperties(prefix = "skillhub.observability") +public class SkillHubObservabilityProperties { + + private TracingMode tracingMode = TracingMode.NONE; + + public TracingMode getTracingMode() { + return tracingMode; + } + + public void setTracingMode(TracingMode tracingMode) { + this.tracingMode = tracingMode; + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/SkillHubTracingConfiguration.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/SkillHubTracingConfiguration.java new file mode 100644 index 000000000..ea3872b25 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/SkillHubTracingConfiguration.java @@ -0,0 +1,54 @@ +package com.iflytek.skillhub.observability.tracing; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.Environment; +import org.springframework.util.StringUtils; + +/** + * Validates tracing mode combinations that SkillHub can determine at startup. + */ +@Configuration(proxyBeanMethods = false) +@EnableConfigurationProperties(SkillHubObservabilityProperties.class) +public class SkillHubTracingConfiguration { + + private static final Logger log = LoggerFactory.getLogger(SkillHubTracingConfiguration.class); + + @Bean + TracingModeGuard tracingModeGuard( + SkillHubObservabilityProperties properties, + Environment environment + ) { + TracingMode mode = properties.getTracingMode(); + String otlpEndpoint = environment.getProperty("management.otlp.tracing.endpoint"); + if (mode != TracingMode.OTEL_SDK && StringUtils.hasText(otlpEndpoint)) { + throw new IllegalStateException( + "management.otlp.tracing.endpoint requires " + + "skillhub.observability.tracing-mode=otel-sdk" + ); + } + if (mode == TracingMode.OTEL_SDK + && Boolean.FALSE.equals(environment.getProperty( + "management.tracing.enabled", + Boolean.class + ))) { + throw new IllegalStateException( + "management.tracing.enabled=false conflicts with " + + "skillhub.observability.tracing-mode=otel-sdk" + ); + } + if (mode == TracingMode.EXTERNAL_AGENT) { + log.warn( + "External tracing agent mode selected. SkillHub cannot verify the agent " + + "identity; deployment must provide exactly one tracing agent" + ); + } + return new TracingModeGuard(mode); + } + + record TracingModeGuard(TracingMode mode) { + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/TracingMode.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/TracingMode.java new file mode 100644 index 000000000..feaf11712 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/TracingMode.java @@ -0,0 +1,10 @@ +package com.iflytek.skillhub.observability.tracing; + +/** + * Selects the single tracing implementation that may be active in the application process. + */ +public enum TracingMode { + NONE, + OTEL_SDK, + EXTERNAL_AGENT +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/TracingModeAutoConfigurationImportFilter.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/TracingModeAutoConfigurationImportFilter.java new file mode 100644 index 000000000..55a33fda6 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/TracingModeAutoConfigurationImportFilter.java @@ -0,0 +1,51 @@ +package com.iflytek.skillhub.observability.tracing; + +import org.springframework.boot.autoconfigure.AutoConfigurationImportFilter; +import org.springframework.boot.autoconfigure.AutoConfigurationMetadata; +import org.springframework.context.EnvironmentAware; +import org.springframework.core.env.Environment; + +import java.util.Set; + +/** + * Keeps the OpenTelemetry SDK outside the application context unless the deployment explicitly + * selects {@code otel-sdk}. The normal Spring Boot NOOP tracer remains available in the other + * modes. + */ +public final class TracingModeAutoConfigurationImportFilter + implements AutoConfigurationImportFilter, EnvironmentAware { + + static final String TRACING_MODE_PROPERTY = "skillhub.observability.tracing-mode"; + + private static final Set OTEL_AUTO_CONFIGURATIONS = Set.of( + "org.springframework.boot.actuate.autoconfigure.opentelemetry.OpenTelemetryAutoConfiguration", + "org.springframework.boot.actuate.autoconfigure.tracing.OpenTelemetryAutoConfiguration", + "org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpAutoConfiguration" + ); + + private Environment environment; + + @Override + public boolean[] match( + String[] autoConfigurationClasses, + AutoConfigurationMetadata autoConfigurationMetadata + ) { + boolean otelSdkEnabled = environment != null + && "otel-sdk".equalsIgnoreCase( + environment.getProperty(TRACING_MODE_PROPERTY, "none") + ); + boolean[] matches = new boolean[autoConfigurationClasses.length]; + for (int index = 0; index < autoConfigurationClasses.length; index++) { + String autoConfigurationClass = autoConfigurationClasses[index]; + matches[index] = autoConfigurationClass != null + && (otelSdkEnabled + || !OTEL_AUTO_CONFIGURATIONS.contains(autoConfigurationClass)); + } + return matches; + } + + @Override + public void setEnvironment(Environment environment) { + this.environment = environment; + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/package-info.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/package-info.java new file mode 100644 index 000000000..a60438958 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/package-info.java @@ -0,0 +1,4 @@ +/** + * Startup tracing mode selection and auto-configuration boundaries. + */ +package com.iflytek.skillhub.observability.tracing; diff --git a/server/skillhub-app/src/main/resources/META-INF/spring.factories b/server/skillhub-app/src/main/resources/META-INF/spring.factories new file mode 100644 index 000000000..e8a70f68e --- /dev/null +++ b/server/skillhub-app/src/main/resources/META-INF/spring.factories @@ -0,0 +1,2 @@ +org.springframework.boot.autoconfigure.AutoConfigurationImportFilter=\ +com.iflytek.skillhub.observability.tracing.TracingModeAutoConfigurationImportFilter diff --git a/server/skillhub-app/src/main/resources/application.yml b/server/skillhub-app/src/main/resources/application.yml index 9983d5e8e..10e9638eb 100644 --- a/server/skillhub-app/src/main/resources/application.yml +++ b/server/skillhub-app/src/main/resources/application.yml @@ -95,6 +95,7 @@ spring: skillhub: observability: + tracing-mode: ${SKILLHUB_TRACING_MODE:none} log-format: ${SKILLHUB_LOG_FORMAT:text} log-async-queue-size: ${SKILLHUB_LOG_ASYNC_QUEUE_SIZE:1024} service-version: ${SKILLHUB_SERVICE_VERSION:unknown} @@ -214,6 +215,17 @@ skillhub: email: ${BOOTSTRAP_ADMIN_EMAIL:admin@skillhub.local} management: + tracing: + sampling: + probability: ${SKILLHUB_TRACING_SAMPLING_PROBABILITY:0.1} + baggage: + enabled: false + propagation: + type: W3C + otlp: + tracing: + timeout: ${SKILLHUB_OTLP_TIMEOUT:5s} + compression: ${SKILLHUB_OTLP_COMPRESSION:gzip} health: mail: enabled: ${MANAGEMENT_HEALTH_MAIL_ENABLED:false} diff --git a/server/skillhub-app/src/main/resources/logback-spring.xml b/server/skillhub-app/src/main/resources/logback-spring.xml index 30152e8ef..c9372168c 100644 --- a/server/skillhub-app/src/main/resources/logback-spring.xml +++ b/server/skillhub-app/src/main/resources/logback-spring.xml @@ -5,6 +5,9 @@ + @@ -30,6 +33,7 @@ ${serviceName} ${serviceVersion} ${serviceEnvironment} + ${tracingMode} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/logging/SkillHubEcsEncoderTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/logging/SkillHubEcsEncoderTest.java index 9d0df479a..e417d97eb 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/logging/SkillHubEcsEncoderTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/logging/SkillHubEcsEncoderTest.java @@ -81,6 +81,32 @@ void shouldPreferMicrometerTraceIdOverExternalAgentFallback() throws Exception { .hasSize(1); } + @Test + void shouldNormalizeExternalAgentTraceId() throws Exception { + encoder.stop(); + encoder.setTracingMode("external-agent"); + encoder.start(); + LoggingEvent event = event("external trace"); + event.setMDCPropertyMap(Map.of("tid", "TID: external-agent-trace")); + + JsonNode json = encode(event); + + assertThat(json.path("trace.id").asText()).isEqualTo("external-agent-trace"); + } + + @Test + void shouldNotWriteToolkitSentinelAsTraceId() throws Exception { + encoder.stop(); + encoder.setTracingMode("external-agent"); + encoder.start(); + LoggingEvent event = event("no external agent"); + event.setMDCPropertyMap(Map.of("tid", "TID: N/A")); + + JsonNode json = encode(event); + + assertThat(json.has("trace.id")).isFalse(); + } + @Test void shouldWriteStructuredExceptionFields() throws Exception { LoggingEvent event = event("failed"); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/SkillHubTracingConfigurationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/SkillHubTracingConfigurationTest.java new file mode 100644 index 000000000..1c5913ce5 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/SkillHubTracingConfigurationTest.java @@ -0,0 +1,124 @@ +package com.iflytek.skillhub.observability.tracing; + +import io.micrometer.tracing.Tracer; +import io.micrometer.tracing.otel.bridge.OtelTracer; +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.exporter.otlp.http.trace.OtlpHttpSpanExporter; +import org.junit.jupiter.api.Test; +import org.slf4j.MDC; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; + +import static org.assertj.core.api.Assertions.assertThat; + +class SkillHubTracingConfigurationTest { + + private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() + .withUserConfiguration(TestApplication.class) + .withPropertyValues( + "spring.flyway.enabled=false", + "spring.jpa.hibernate.ddl-auto=none" + ); + + @Test + void noneModeShouldUseNoopTracerAndNoOtelSdk() { + contextRunner + .withPropertyValues("skillhub.observability.tracing-mode=none") + .run(context -> { + assertThat(context).hasNotFailed(); + assertThat(context.getBean(Tracer.class)).isSameAs(Tracer.NOOP); + assertThat(context).doesNotHaveBean(OpenTelemetry.class); + assertThat(context).doesNotHaveBean(OtlpHttpSpanExporter.class); + }); + } + + @Test + void externalAgentModeShouldUseNoopTracerAndNoOtelSdk() { + contextRunner + .withPropertyValues("skillhub.observability.tracing-mode=external-agent") + .run(context -> { + assertThat(context).hasNotFailed(); + assertThat(context.getBean(Tracer.class)).isSameAs(Tracer.NOOP); + assertThat(context).doesNotHaveBean(OpenTelemetry.class); + assertThat(context).doesNotHaveBean(OtlpHttpSpanExporter.class); + }); + } + + @Test + void otelSdkModeWithoutEndpointShouldCreateInProcessTracerOnly() { + contextRunner + .withPropertyValues( + "skillhub.observability.tracing-mode=otel-sdk", + "management.tracing.sampling.probability=1.0", + "management.tracing.baggage.enabled=false", + "management.tracing.propagation.type=W3C" + ) + .run(context -> { + assertThat(context).hasNotFailed(); + assertThat(context.getBean(Tracer.class)).isInstanceOf(OtelTracer.class); + assertThat(context).hasSingleBean(OpenTelemetry.class); + assertThat(context).doesNotHaveBean(OtlpHttpSpanExporter.class); + }); + } + + @Test + void otelSdkModeShouldCreateExporterOnlyWhenEndpointIsConfigured() { + contextRunner + .withPropertyValues( + "skillhub.observability.tracing-mode=otel-sdk", + "management.otlp.tracing.endpoint=http://127.0.0.1:4318/v1/traces" + ) + .run(context -> { + assertThat(context).hasNotFailed(); + assertThat(context).hasSingleBean(OtlpHttpSpanExporter.class); + }); + } + + @Test + void nonOtelModeShouldRejectConfiguredOtlpEndpoint() { + contextRunner + .withPropertyValues( + "skillhub.observability.tracing-mode=none", + "management.otlp.tracing.endpoint=http://127.0.0.1:4318/v1/traces" + ) + .run(context -> assertThat(context).hasFailed()); + } + + @Test + void otelSdkModeShouldRejectDisabledTracing() { + contextRunner + .withPropertyValues( + "skillhub.observability.tracing-mode=otel-sdk", + "management.tracing.enabled=false" + ) + .run(context -> assertThat(context).hasFailed()); + } + + @Test + void otelSdkScopeShouldPublishTraceCorrelationToMdc() { + contextRunner + .withPropertyValues( + "skillhub.observability.tracing-mode=otel-sdk", + "management.tracing.sampling.probability=1.0" + ) + .run(context -> { + Tracer tracer = context.getBean(Tracer.class); + io.micrometer.tracing.Span span = tracer.nextSpan().name("test-span").start(); + try (Tracer.SpanInScope ignored = tracer.withSpan(span)) { + assertThat(MDC.get("traceId")).hasSize(32); + assertThat(MDC.get("spanId")).hasSize(16); + } finally { + span.end(); + MDC.clear(); + } + }); + } + + @Configuration(proxyBeanMethods = false) + @EnableAutoConfiguration + @Import(SkillHubTracingConfiguration.class) + static class TestApplication { + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/TracingModeAutoConfigurationImportFilterTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/TracingModeAutoConfigurationImportFilterTest.java new file mode 100644 index 000000000..6cfbb7d32 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/TracingModeAutoConfigurationImportFilterTest.java @@ -0,0 +1,65 @@ +package com.iflytek.skillhub.observability.tracing; + +import org.junit.jupiter.api.Test; +import org.springframework.boot.autoconfigure.AutoConfigurationMetadata; +import org.springframework.mock.env.MockEnvironment; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +class TracingModeAutoConfigurationImportFilterTest { + + private static final String CORE_OTEL_AUTO_CONFIGURATION = + "org.springframework.boot.actuate.autoconfigure.opentelemetry.OpenTelemetryAutoConfiguration"; + private static final String TRACING_OTEL_AUTO_CONFIGURATION = + "org.springframework.boot.actuate.autoconfigure.tracing.OpenTelemetryAutoConfiguration"; + private static final String OTLP_AUTO_CONFIGURATION = + "org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpAutoConfiguration"; + private static final String NOOP_AUTO_CONFIGURATION = + "org.springframework.boot.actuate.autoconfigure.tracing.NoopTracerAutoConfiguration"; + + private final TracingModeAutoConfigurationImportFilter filter = + new TracingModeAutoConfigurationImportFilter(); + + @Test + void shouldExcludeApplicationOtelForDefaultNoneMode() { + filter.setEnvironment(new MockEnvironment()); + + assertThat(matches()).containsExactly(false, false, false, true, false); + } + + @Test + void shouldExcludeApplicationOtelForExternalAgentMode() { + filter.setEnvironment(new MockEnvironment() + .withProperty( + TracingModeAutoConfigurationImportFilter.TRACING_MODE_PROPERTY, + "external-agent" + )); + + assertThat(matches()).containsExactly(false, false, false, true, false); + } + + @Test + void shouldEnableApplicationOtelOnlyForOtelSdkMode() { + filter.setEnvironment(new MockEnvironment() + .withProperty( + TracingModeAutoConfigurationImportFilter.TRACING_MODE_PROPERTY, + "otel-sdk" + )); + + assertThat(matches()).containsExactly(true, true, true, true, false); + } + + private boolean[] matches() { + return filter.match( + new String[]{ + CORE_OTEL_AUTO_CONFIGURATION, + TRACING_OTEL_AUTO_CONFIGURATION, + OTLP_AUTO_CONFIGURATION, + NOOP_AUTO_CONFIGURATION, + null + }, + mock(AutoConfigurationMetadata.class) + ); + } +} From 91fb155ef17ec27dd7e9abb7c6e205da8f3c305a Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Fri, 31 Jul 2026 11:22:12 +0800 Subject: [PATCH 03/10] feat(observability): propagate async trace context Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../iflytek/skillhub/config/AsyncConfig.java | 6 +- .../skillhub/config/SkillScannerConfig.java | 7 +- .../RequestIdThreadLocalAccessor.java | 37 ++ ...illHubContextPropagationConfiguration.java | 51 +++ .../ContextPropagationConfigurationTest.java | 320 ++++++++++++++++++ .../tracing/HttpTracePropagationTest.java | 178 ++++++++++ .../auth/oauth/GitLabClaimsExtractor.java | 8 + 7 files changed, 604 insertions(+), 3 deletions(-) create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/RequestIdThreadLocalAccessor.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/SkillHubContextPropagationConfiguration.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/ContextPropagationConfigurationTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/HttpTracePropagationTest.java diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/AsyncConfig.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/AsyncConfig.java index 7feb8955b..10a6f5a5f 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/AsyncConfig.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/AsyncConfig.java @@ -2,6 +2,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.task.support.ContextPropagatingTaskDecorator; import org.springframework.scheduling.annotation.EnableAsync; import org.springframework.scheduling.annotation.EnableScheduling; import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor; @@ -19,12 +20,15 @@ public class AsyncConfig { @Bean(name = "skillhubEventExecutor") - public Executor skillhubEventExecutor() { + public Executor skillhubEventExecutor( + ContextPropagatingTaskDecorator contextPropagatingTaskDecorator + ) { ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); executor.setCorePoolSize(2); executor.setMaxPoolSize(4); executor.setQueueCapacity(100); executor.setThreadNamePrefix("skillhub-event-"); + executor.setTaskDecorator(contextPropagatingTaskDecorator); executor.setRejectedExecutionHandler(new ThreadPoolExecutor.CallerRunsPolicy()); executor.initialize(); return executor; diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/SkillScannerConfig.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/SkillScannerConfig.java index d6b7a5ec7..9753e982f 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/SkillScannerConfig.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/SkillScannerConfig.java @@ -26,7 +26,10 @@ public class SkillScannerConfig { @Bean @ConditionalOnProperty(prefix = "skillhub.security.scanner", name = "enabled", havingValue = "true") - public HttpClient scannerHttpClient(SkillScannerProperties properties) { + public HttpClient scannerHttpClient( + WebClient.Builder webClientBuilder, + SkillScannerProperties properties + ) { int readTimeoutMs = properties.getReadTimeoutMs(); int connectTimeoutMs = properties.getConnectTimeoutMs(); @@ -52,7 +55,7 @@ public HttpClient scannerHttpClient(SkillScannerProperties properties) { .codecs(configurer -> configurer.defaultCodecs().maxInMemorySize(100 * 1024 * 1024)) .build(); - WebClient webClient = WebClient.builder() + WebClient webClient = webClientBuilder.clone() .clientConnector(new ReactorClientHttpConnector(reactorClient)) .exchangeStrategies(strategies) .build(); diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/RequestIdThreadLocalAccessor.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/RequestIdThreadLocalAccessor.java new file mode 100644 index 000000000..9d1deffbe --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/RequestIdThreadLocalAccessor.java @@ -0,0 +1,37 @@ +package com.iflytek.skillhub.observability; + +import io.micrometer.context.ThreadLocalAccessor; + +/** + * Captures and restores the authoritative Request ID scope for asynchronous execution. + */ +public final class RequestIdThreadLocalAccessor implements ThreadLocalAccessor { + + public static final String KEY = "skillhub.request-id"; + + private final RequestIdAccessor requestIdAccessor; + + public RequestIdThreadLocalAccessor(RequestIdAccessor requestIdAccessor) { + this.requestIdAccessor = requestIdAccessor; + } + + @Override + public Object key() { + return KEY; + } + + @Override + public String getValue() { + return requestIdAccessor.current(); + } + + @Override + public void setValue(String value) { + requestIdAccessor.replace(value); + } + + @Override + public void setValue() { + requestIdAccessor.replace(null); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/SkillHubContextPropagationConfiguration.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/SkillHubContextPropagationConfiguration.java new file mode 100644 index 000000000..e0eaa710f --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/SkillHubContextPropagationConfiguration.java @@ -0,0 +1,51 @@ +package com.iflytek.skillhub.observability; + +import com.iflytek.skillhub.observability.tracing.SkillHubObservabilityProperties; +import com.iflytek.skillhub.observability.tracing.TracingMode; +import io.micrometer.context.ContextRegistry; +import io.micrometer.context.ContextSnapshotFactory; +import io.micrometer.observation.ObservationRegistry; +import io.micrometer.tracing.Tracer; +import io.micrometer.tracing.contextpropagation.ObservationAwareSpanThreadLocalAccessor; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.task.support.ContextPropagatingTaskDecorator; + +/** + * Defines the context captured by SkillHub-managed asynchronous executors. + */ +@Configuration(proxyBeanMethods = false) +public class SkillHubContextPropagationConfiguration { + + @Bean + ContextRegistry skillHubContextRegistry( + RequestIdAccessor requestIdAccessor, + SkillHubObservabilityProperties observabilityProperties, + ObservationRegistry observationRegistry, + Tracer tracer + ) { + ContextRegistry registry = new ContextRegistry() + .loadContextAccessors() + .loadThreadLocalAccessors(); + registry.registerThreadLocalAccessor( + new RequestIdThreadLocalAccessor(requestIdAccessor) + ); + if (observabilityProperties.getTracingMode() == TracingMode.OTEL_SDK) { + registry.registerThreadLocalAccessor( + new ObservationAwareSpanThreadLocalAccessor(observationRegistry, tracer) + ); + } + return registry; + } + + @Bean + ContextPropagatingTaskDecorator skillHubContextPropagatingTaskDecorator( + ContextRegistry skillHubContextRegistry + ) { + ContextSnapshotFactory snapshotFactory = ContextSnapshotFactory.builder() + .contextRegistry(skillHubContextRegistry) + .clearMissing(true) + .build(); + return new ContextPropagatingTaskDecorator(snapshotFactory); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/ContextPropagationConfigurationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/ContextPropagationConfigurationTest.java new file mode 100644 index 000000000..ceb1b351e --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/ContextPropagationConfigurationTest.java @@ -0,0 +1,320 @@ +package com.iflytek.skillhub.observability; + +import com.iflytek.skillhub.config.AsyncConfig; +import com.iflytek.skillhub.observability.tracing.SkillHubTracingConfiguration; +import io.micrometer.observation.Observation; +import io.micrometer.observation.ObservationRegistry; +import io.micrometer.tracing.Span; +import io.micrometer.tracing.Tracer; +import org.junit.jupiter.api.Test; +import org.slf4j.MDC; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.core.task.TaskDecorator; +import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor; + +import java.util.concurrent.Callable; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.FutureTask; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class ContextPropagationConfigurationTest { + + private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() + .withUserConfiguration(TestApplication.class) + .withPropertyValues( + "spring.flyway.enabled=false", + "spring.jpa.hibernate.ddl-auto=none" + ); + + @Test + void requestIdShouldPropagateRestoreNestedScopeAndNotLeakOnThreadReuse() { + contextRunner + .withPropertyValues("skillhub.observability.tracing-mode=none") + .run(context -> { + RequestIdAccessor requestIdAccessor = + context.getBean(RequestIdAccessor.class); + TaskDecorator taskDecorator = context.getBean(TaskDecorator.class); + ExecutorService worker = Executors.newSingleThreadExecutor(); + try { + ContextValues propagated; + try (RequestIdAccessor.Scope ignored = + requestIdAccessor.open("request-one")) { + propagated = execute(worker, taskDecorator, () -> { + assertThat(requestIdAccessor.current()) + .isEqualTo("request-one"); + try (RequestIdAccessor.Scope nested = + requestIdAccessor.open("nested")) { + assertThat(requestIdAccessor.current()) + .isEqualTo("nested"); + } + return currentValues(requestIdAccessor, null); + }); + } + + assertThat(propagated.requestId()).isEqualTo("request-one"); + assertThat(propagated.mdcRequestId()).isEqualTo("request-one"); + + try (RequestIdAccessor.Scope ignored = + requestIdAccessor.open("request-failure")) { + assertThatThrownBy(() -> execute( + worker, + taskDecorator, + () -> { + throw new IllegalStateException("expected failure"); + } + )).hasCauseInstanceOf(IllegalStateException.class); + } + + ContextValues clean = execute( + worker, + taskDecorator, + () -> currentValues(requestIdAccessor, null) + ); + assertThat(clean.requestId()).isNull(); + assertThat(clean.mdcRequestId()).isNull(); + } finally { + worker.shutdownNow(); + MDC.clear(); + } + }); + } + + @Test + void configuredEventExecutorShouldPropagateRequestId() { + contextRunner + .withPropertyValues("skillhub.observability.tracing-mode=none") + .run(context -> { + RequestIdAccessor requestIdAccessor = + context.getBean(RequestIdAccessor.class); + Executor executor = context.getBean("skillhubEventExecutor", Executor.class); + try (RequestIdAccessor.Scope ignored = + requestIdAccessor.open("configured-executor")) { + FutureTask task = new FutureTask<>( + () -> currentValues(requestIdAccessor, null) + ); + executor.execute(task); + + assertThat(task.get(5, TimeUnit.SECONDS).requestId()) + .isEqualTo("configured-executor"); + } finally { + MDC.clear(); + } + }); + } + + @Test + void callerRunsPolicyShouldRestoreCallerScopeAndLeaveWorkerClean() { + contextRunner + .withPropertyValues("skillhub.observability.tracing-mode=none") + .run(context -> { + RequestIdAccessor requestIdAccessor = + context.getBean(RequestIdAccessor.class); + TaskDecorator taskDecorator = context.getBean(TaskDecorator.class); + ThreadPoolTaskExecutor executor = callerRunsExecutor(taskDecorator); + CountDownLatch workerStarted = new CountDownLatch(1); + CountDownLatch releaseWorker = new CountDownLatch(1); + FutureTask blockingTask = new FutureTask<>(() -> { + workerStarted.countDown(); + releaseWorker.await(5, TimeUnit.SECONDS); + return null; + }); + try { + executor.execute(blockingTask); + assertThat(workerStarted.await(5, TimeUnit.SECONDS)).isTrue(); + + AtomicReference callerRunValues = + new AtomicReference<>(); + String callerThread = Thread.currentThread().getName(); + try (RequestIdAccessor.Scope ignored = + requestIdAccessor.open("caller-request")) { + executor.execute(() -> { + assertThat(Thread.currentThread().getName()) + .isEqualTo(callerThread); + callerRunValues.set(currentValues( + requestIdAccessor, + null + )); + }); + assertThat(requestIdAccessor.current()) + .isEqualTo("caller-request"); + assertThat(MDC.get(RequestIdAccessor.MDC_KEY)) + .isEqualTo("caller-request"); + } + + assertThat(callerRunValues.get().requestId()) + .isEqualTo("caller-request"); + releaseWorker.countDown(); + blockingTask.get(5, TimeUnit.SECONDS); + + FutureTask cleanTask = new FutureTask<>( + () -> currentValues(requestIdAccessor, null) + ); + executor.execute(cleanTask); + ContextValues clean = cleanTask.get(5, TimeUnit.SECONDS); + assertThat(clean.requestId()).isNull(); + assertThat(clean.mdcRequestId()).isNull(); + } finally { + releaseWorker.countDown(); + executor.shutdown(); + MDC.clear(); + } + }); + } + + @Test + void otelSpanShouldPropagateAndBeClearedAfterTask() { + contextRunner + .withPropertyValues( + "skillhub.observability.tracing-mode=otel-sdk", + "management.tracing.sampling.probability=1.0" + ) + .run(context -> { + RequestIdAccessor requestIdAccessor = + context.getBean(RequestIdAccessor.class); + TaskDecorator taskDecorator = context.getBean(TaskDecorator.class); + Tracer tracer = context.getBean(Tracer.class); + ExecutorService worker = Executors.newSingleThreadExecutor(); + Span span = tracer.nextSpan().name("parent").start(); + try { + ContextValues propagated; + try (Tracer.SpanInScope ignored = tracer.withSpan(span)) { + propagated = execute( + worker, + taskDecorator, + () -> currentValues(requestIdAccessor, tracer) + ); + } + + assertThat(propagated.traceId()) + .isEqualTo(span.context().traceId()); + assertThat(propagated.mdcTraceId()) + .isEqualTo(span.context().traceId()); + + ContextValues clean = execute( + worker, + taskDecorator, + () -> currentValues(requestIdAccessor, tracer) + ); + assertThat(clean.traceId()).isNull(); + assertThat(clean.mdcTraceId()).isNull(); + } finally { + span.end(); + worker.shutdownNow(); + MDC.clear(); + } + }); + } + + @Test + void otelObservationShouldPropagateItsTraceAndRestoreWorker() { + contextRunner + .withPropertyValues( + "skillhub.observability.tracing-mode=otel-sdk", + "management.tracing.sampling.probability=1.0" + ) + .run(context -> { + RequestIdAccessor requestIdAccessor = + context.getBean(RequestIdAccessor.class); + TaskDecorator taskDecorator = context.getBean(TaskDecorator.class); + Tracer tracer = context.getBean(Tracer.class); + ObservationRegistry observationRegistry = + context.getBean(ObservationRegistry.class); + ExecutorService worker = Executors.newSingleThreadExecutor(); + Observation observation = Observation + .createNotStarted("parent-observation", observationRegistry) + .start(); + try { + ContextValues propagated; + String parentTraceId; + try (Observation.Scope ignored = observation.openScope()) { + assertThat(tracer.currentSpan()).isNotNull(); + parentTraceId = tracer.currentSpan().context().traceId(); + propagated = execute( + worker, + taskDecorator, + () -> currentValues(requestIdAccessor, tracer) + ); + } + + assertThat(propagated.traceId()).isEqualTo(parentTraceId); + assertThat(propagated.mdcTraceId()).isEqualTo(parentTraceId); + + ContextValues clean = execute( + worker, + taskDecorator, + () -> currentValues(requestIdAccessor, tracer) + ); + assertThat(clean.traceId()).isNull(); + assertThat(clean.mdcTraceId()).isNull(); + } finally { + observation.stop(); + worker.shutdownNow(); + MDC.clear(); + } + }); + } + + private ContextValues currentValues( + RequestIdAccessor requestIdAccessor, + Tracer tracer + ) { + Span currentSpan = tracer == null ? null : tracer.currentSpan(); + return new ContextValues( + requestIdAccessor.current(), + MDC.get(RequestIdAccessor.MDC_KEY), + currentSpan == null ? null : currentSpan.context().traceId(), + MDC.get("traceId") + ); + } + + private T execute( + Executor executor, + TaskDecorator taskDecorator, + Callable action + ) throws Exception { + FutureTask task = new FutureTask<>(action); + executor.execute(taskDecorator.decorate(task)); + return task.get(5, TimeUnit.SECONDS); + } + + private ThreadPoolTaskExecutor callerRunsExecutor(TaskDecorator taskDecorator) { + ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); + executor.setCorePoolSize(1); + executor.setMaxPoolSize(1); + executor.setQueueCapacity(0); + executor.setTaskDecorator(taskDecorator); + executor.setRejectedExecutionHandler(new ThreadPoolExecutor.CallerRunsPolicy()); + executor.initialize(); + return executor; + } + + private record ContextValues( + String requestId, + String mdcRequestId, + String traceId, + String mdcTraceId + ) { + } + + @Configuration(proxyBeanMethods = false) + @EnableAutoConfiguration + @Import({ + SkillHubTracingConfiguration.class, + SkillHubContextPropagationConfiguration.class, + RequestIdAccessor.class, + AsyncConfig.class + }) + static class TestApplication { + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/HttpTracePropagationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/HttpTracePropagationTest.java new file mode 100644 index 000000000..0e0b4c184 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/HttpTracePropagationTest.java @@ -0,0 +1,178 @@ +package com.iflytek.skillhub.observability.tracing; + +import com.iflytek.skillhub.auth.oauth.GitLabClaimsExtractor; +import com.iflytek.skillhub.auth.oauth.OAuthClaims; +import com.iflytek.skillhub.config.SkillScannerConfig; +import com.iflytek.skillhub.config.SkillScannerProperties; +import com.iflytek.skillhub.infra.http.HttpClient; +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpServer; +import io.micrometer.tracing.Span; +import io.micrometer.tracing.Tracer; +import org.junit.jupiter.api.Test; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.security.oauth2.client.registration.ClientRegistration; +import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest; +import org.springframework.security.oauth2.core.AuthorizationGrantType; +import org.springframework.security.oauth2.core.OAuth2AccessToken; +import org.springframework.security.oauth2.core.user.DefaultOAuth2User; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; +import java.time.Instant; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; + +import static org.assertj.core.api.Assertions.assertThat; + +class HttpTracePropagationTest { + + private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() + .withUserConfiguration(TestApplication.class) + .withPropertyValues( + "spring.flyway.enabled=false", + "spring.jpa.hibernate.ddl-auto=none", + "skillhub.observability.tracing-mode=otel-sdk", + "management.tracing.sampling.probability=1.0", + "skillhub.security.scanner.enabled=true" + ); + + @Test + void shouldPropagateW3cContextToScannerButNotExternalGitLab() throws Exception { + try (HeaderCaptureServer scannerServer = + new HeaderCaptureServer("text/plain", "scanner-ok"); + HeaderCaptureServer gitLabServer = + new HeaderCaptureServer( + "application/json", + """ + [ + { + "email": "alice@gitlab.example", + "confirmed_at": "2026-04-16T08:00:00Z" + } + ] + """ + )) { + contextRunner.run(context -> { + Tracer tracer = context.getBean(Tracer.class); + HttpClient scannerClient = + context.getBean("scannerHttpClient", HttpClient.class); + GitLabClaimsExtractor gitLabClaimsExtractor = + context.getBean(GitLabClaimsExtractor.class); + Span span = tracer.nextSpan().name("outbound-boundary").start(); + try (Tracer.SpanInScope ignored = tracer.withSpan(span)) { + assertThat(scannerClient.get( + scannerServer.url("/health"), + String.class + )).isEqualTo("scanner-ok"); + + OAuthClaims claims = gitLabClaimsExtractor.extract( + gitLabRequest(gitLabServer.url("/api/v4/user")), + new DefaultOAuth2User( + List.of(), + Map.of( + "id", 42, + "username", "alice", + "email", "alice+pending@gitlab.example" + ), + "username" + ) + ); + assertThat(claims.email()) + .isEqualTo("alice@gitlab.example"); + } finally { + span.end(); + } + + String scannerTraceparent = scannerServer.traceparent(); + assertThat(scannerTraceparent) + .matches("^00-[0-9a-f]{32}-[0-9a-f]{16}-0[01]$"); + assertThat(scannerTraceparent.substring(3, 35)) + .isEqualTo(span.context().traceId()); + assertThat(gitLabServer.traceparent()).isNull(); + }); + } + } + + private OAuth2UserRequest gitLabRequest(String userInfoUri) { + ClientRegistration registration = ClientRegistration + .withRegistrationId("gitlab") + .clientId("client-id") + .clientSecret("client-secret") + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .redirectUri("{baseUrl}/login/oauth2/code/{registrationId}") + .scope("read_user", "email") + .authorizationUri("https://gitlab.example/oauth/authorize") + .tokenUri("https://gitlab.example/oauth/token") + .userInfoUri(userInfoUri) + .userNameAttributeName("username") + .clientName("GitLab") + .build(); + OAuth2AccessToken accessToken = new OAuth2AccessToken( + OAuth2AccessToken.TokenType.BEARER, + "test-token", + Instant.now(), + Instant.now().plusSeconds(3600) + ); + return new OAuth2UserRequest(registration, accessToken); + } + + private static final class HeaderCaptureServer implements AutoCloseable { + + private final HttpServer server; + private final AtomicReference traceparent = new AtomicReference<>(); + + private HeaderCaptureServer(String contentType, String body) throws IOException { + byte[] response = body.getBytes(StandardCharsets.UTF_8); + server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + server.createContext("/", exchange -> respond( + exchange, + contentType, + response + )); + server.start(); + } + + private void respond( + HttpExchange exchange, + String contentType, + byte[] response + ) throws IOException { + traceparent.set(exchange.getRequestHeaders().getFirst("traceparent")); + exchange.getResponseHeaders().set("Content-Type", contentType); + exchange.sendResponseHeaders(200, response.length); + try (var responseBody = exchange.getResponseBody()) { + responseBody.write(response); + } + } + + private String url(String path) { + return "http://127.0.0.1:" + server.getAddress().getPort() + path; + } + + private String traceparent() { + return traceparent.get(); + } + + @Override + public void close() { + server.stop(0); + } + } + + @Configuration(proxyBeanMethods = false) + @EnableAutoConfiguration + @Import({ + SkillHubTracingConfiguration.class, + SkillScannerConfig.class, + SkillScannerProperties.class, + GitLabClaimsExtractor.class + }) + static class TestApplication { + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/GitLabClaimsExtractor.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/GitLabClaimsExtractor.java index d6c840a27..7a744a30e 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/GitLabClaimsExtractor.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/GitLabClaimsExtractor.java @@ -27,6 +27,14 @@ public class GitLabClaimsExtractor implements OAuthClaimsExtractor { private final RestClient restClient; + /** + * Uses an external-service client that is intentionally not customized with application + * tracing. Trace context must not be propagated to a user-configured GitLab host. + */ + public GitLabClaimsExtractor() { + this(RestClient.builder()); + } + public GitLabClaimsExtractor(RestClient.Builder restClientBuilder) { this.restClient = restClientBuilder .defaultHeader(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE) From 8b3ad8b14e6397db731a58dda1a673bf72e30a7c Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Fri, 31 Jul 2026 11:30:42 +0800 Subject: [PATCH 04/10] docs(observability): document tracing deployment modes Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .env.release.example | 13 + compose.release.yml | 9 + docs/09-deployment.md | 128 ++++- ...6-07-31-observability-construction-plan.md | 523 ++++++++++++++++++ docs/observability-decision-map.md | 124 +++++ ...26-07-31-observability-common-solutions.md | 120 ++++ 6 files changed, 915 insertions(+), 2 deletions(-) create mode 100644 docs/2026-07-31-observability-construction-plan.md create mode 100644 docs/observability-decision-map.md create mode 100644 docs/research/2026-07-31-observability-common-solutions.md diff --git a/.env.release.example b/.env.release.example index 59387e0f8..1b8e38e69 100644 --- a/.env.release.example +++ b/.env.release.example @@ -53,6 +53,19 @@ API_PORT=8080 WEB_PORT=80 SESSION_COOKIE_SECURE=false +# Observability defaults require no Collector or tracing backend. +# Use json in container deployments when stdout is collected centrally. +SKILLHUB_TRACING_MODE=none +SKILLHUB_LOG_FORMAT=text +SKILLHUB_LOG_ASYNC_QUEUE_SIZE=1024 +SKILLHUB_SERVICE_VERSION=unknown +SKILLHUB_SERVICE_ENVIRONMENT=production +SKILLHUB_TRACING_SAMPLING_PROBABILITY=0.1 +# Set only with SKILLHUB_TRACING_MODE=otel-sdk. +MANAGEMENT_OTLP_TRACING_ENDPOINT= +SKILLHUB_OTLP_TIMEOUT=5s +SKILLHUB_OTLP_COMPRESSION=gzip + # Zero-config runtime validation uses local storage. # Switch to `s3` and fill the fields below before a real production deployment. SKILLHUB_STORAGE_PROVIDER=local diff --git a/compose.release.yml b/compose.release.yml index 733187688..db306bda8 100644 --- a/compose.release.yml +++ b/compose.release.yml @@ -88,6 +88,15 @@ services: SKILLHUB_SECURITY_SCANNER_URL: http://skill-scanner:8000 SKILLHUB_SECURITY_SCANNER_MODE: upload SKILLHUB_AUTH_DIRECT_ENABLED: ${SKILLHUB_AUTH_DIRECT_ENABLED:-false} + SKILLHUB_TRACING_MODE: ${SKILLHUB_TRACING_MODE:-none} + SKILLHUB_LOG_FORMAT: ${SKILLHUB_LOG_FORMAT:-text} + SKILLHUB_LOG_ASYNC_QUEUE_SIZE: ${SKILLHUB_LOG_ASYNC_QUEUE_SIZE:-1024} + SKILLHUB_SERVICE_VERSION: ${SKILLHUB_SERVICE_VERSION:-unknown} + SKILLHUB_SERVICE_ENVIRONMENT: ${SKILLHUB_SERVICE_ENVIRONMENT:-production} + SKILLHUB_TRACING_SAMPLING_PROBABILITY: ${SKILLHUB_TRACING_SAMPLING_PROBABILITY:-0.1} + MANAGEMENT_OTLP_TRACING_ENDPOINT: ${MANAGEMENT_OTLP_TRACING_ENDPOINT:-} + SKILLHUB_OTLP_TIMEOUT: ${SKILLHUB_OTLP_TIMEOUT:-5s} + SKILLHUB_OTLP_COMPRESSION: ${SKILLHUB_OTLP_COMPRESSION:-gzip} BOOTSTRAP_ADMIN_ENABLED: ${BOOTSTRAP_ADMIN_ENABLED:-false} BOOTSTRAP_ADMIN_USER_ID: ${BOOTSTRAP_ADMIN_USER_ID:-docker-admin} BOOTSTRAP_ADMIN_USERNAME: ${BOOTSTRAP_ADMIN_USERNAME:-admin} diff --git a/docs/09-deployment.md b/docs/09-deployment.md index bcf362894..847391c76 100644 --- a/docs/09-deployment.md +++ b/docs/09-deployment.md @@ -320,8 +320,132 @@ override 或部署平台环境变量把上述 `SPRING_SECURITY_*` 变量注入 ` | 维度 | 方案 | |------|------| | 健康检查 | `web/nginx-health`、`server/actuator/health` | -| 日志 | 容器 stdout / stderr | -| 指标 | Spring Boot Actuator，后续可接 Prometheus | +| 请求关联 | 响应头和日志中的 `X-Request-Id` / `request.id` | +| 日志 | 文本或 ECS 风格 JSON，均输出到容器 stdout / stderr | +| Trace | `none`、Micrometer + OTel SDK、或外部 Java Agent 三选一 | +| 指标 | Spring Boot Actuator；Prometheus 是可选后端，不是 Trace 前置条件 | + +### 10.1 通用配置 + +默认配置不要求 Collector、SkyWalking 或 Elasticsearch： + +```dotenv +SKILLHUB_TRACING_MODE=none +SKILLHUB_LOG_FORMAT=text +SKILLHUB_SERVICE_VERSION=v0.2.15 +SKILLHUB_SERVICE_ENVIRONMENT=production +``` + +部署环境建议将 `SKILLHUB_LOG_FORMAT` 设为 `json`，由 Filebeat、Fluent Bit 或容器平台 +采集 stdout。SkillHub 不直接连接 Elasticsearch。JSON 日志使用以下稳定字段： + +- `request.id`：SkillHub 请求、响应和审计关联 ID。 +- `trace.id`、`span.id`：当前存在有效 Trace 时输出。 +- `service.name`、`service.version`、`service.environment`。 + +`SKILLHUB_LOG_ASYNC_QUEUE_SIZE` 默认是 `1024`。JSON 日志队列是有界且非阻塞的；采集端 +阻塞时允许丢弃日志以保护业务线程，数据库中的 `audit_log` 仍是审计事实来源。 + +### 10.2 三种 Tracing 模式 + +三种模式只能选择一种，切换后需要重启： + +| 模式 | 适用场景 | 必需配置 | +|------|----------|----------| +| `none` | 不部署链路追踪 | `SKILLHUB_TRACING_MODE=none` | +| `otel-sdk` | 厂商中立 OTLP/Collector | 模式、采样率；需要导出时再配置 endpoint | +| `external-agent` | 使用 SkyWalking Agent 原生能力 | 模式、唯一的外部 Agent；不得配置 OTLP endpoint | + +OTel SDK 模式的最小配置： + +```dotenv +SKILLHUB_TRACING_MODE=otel-sdk +SKILLHUB_LOG_FORMAT=json +SKILLHUB_TRACING_SAMPLING_PROBABILITY=0.1 +MANAGEMENT_OTLP_TRACING_ENDPOINT=http://otel-collector:4318/v1/traces +SKILLHUB_OTLP_TIMEOUT=5s +SKILLHUB_OTLP_COMPRESSION=gzip +``` + +未设置 `MANAGEMENT_OTLP_TRACING_ENDPOINT` 时，`otel-sdk` 仍可建立进程内 Trace，但不会 +创建 OTLP Exporter，也不会尝试连接默认地址。`none` 或 `external-agent` 模式配置 +endpoint 会启动失败。 + +External Agent 模式的应用侧配置： + +```dotenv +SKILLHUB_TRACING_MODE=external-agent +SKILLHUB_LOG_FORMAT=json +``` + +部署平台还必须通过 JVM 启动参数挂载且只挂载一个 Agent。SkillHub 无法可靠识别任意 +Java Agent，因此上线前应检查实际 `JAVA_TOOL_OPTIONS` 或容器启动命令，确认没有同时启用 +OTel Agent、SkyWalking Agent 和应用内 `otel-sdk`。SkyWalking Agent 模式可以通过官方 +Logback Toolkit 输出 `trace.id`；`span.id` 是否可用取决于 Agent 版本。 + +### 10.3 OTel Collector 接入 SkyWalking + +下面是只转发 Trace 的最小 Collector 配置： + +```yaml +receivers: + otlp: + protocols: + http: + endpoint: 0.0.0.0:4318 + +processors: + batch: {} + +exporters: + otlp/skywalking: + endpoint: skywalking-oap:11800 + tls: + insecure: true + +service: + pipelines: + traces: + receivers: [otlp] + processors: [batch] + exporters: [otlp/skywalking] +``` + +SkyWalking OAP 10.3 还需要启用 OTLP Trace handler、Zipkin receiver 和 Zipkin query： + +```dotenv +SW_OTEL_RECEIVER_ENABLED_HANDLERS=otlp-traces +SW_RECEIVER_ZIPKIN=default +SW_QUERY_ZIPKIN=default +``` + +应用使用 Collector 的 OTLP/HTTP `4318` 端口，Collector 使用 OAP 的 OTLP/gRPC +`11800` 端口。生产环境应按网络边界配置 TLS；上例中的 `insecure: true` 只适用于受控的 +容器内部网络。 + +SkyWalking 10.3 会把 OTLP Trace 转换为 Zipkin Trace，并通过 Zipkin Query/Lens 查询。 +这条路径不提供 SkyWalking Java Agent 的完整原生拓扑、慢 SQL 和 Profiling 能力。需要 +这些能力时使用 `external-agent`，不要同时启用 `otel-sdk`。 + +### 10.4 日志与 Trace 联查 + +JSON 日志由采集器写入 Elasticsearch 后，在 Kibana 通过 `trace.id` 查询；同一个 +`trace.id` 可在 SkyWalking 的 Zipkin Query/Lens 或 Agent 原生查询界面中定位调用链。 +`request.id` 始终可以用于 SkillHub 内部日志和审计关联。 + +当采样率小于 `1.0` 时，日志仍是全量输出，因此部分日志虽有请求关联信息，但在 +SkyWalking 中没有被保留的 Trace。这是头部采样的预期行为。 + +### 10.5 回滚 + +遇到观测后端异常时： + +1. 将 `SKILLHUB_TRACING_MODE` 改为 `none`。 +2. 删除 `MANAGEMENT_OTLP_TRACING_ENDPOINT`。 +3. 需要进一步降低日志开销时，将 `SKILLHUB_LOG_FORMAT` 改为 `text`。 +4. 滚动重启 Server。 + +关闭 Trace 和 JSON 日志不会改变请求、数据库或异步任务的业务语义。 ## 11 安全扫描服务 diff --git a/docs/2026-07-31-observability-construction-plan.md b/docs/2026-07-31-observability-construction-plan.md new file mode 100644 index 000000000..688528352 --- /dev/null +++ b/docs/2026-07-31-observability-construction-plan.md @@ -0,0 +1,523 @@ +# SkillHub 日志关联与链路追踪建设方案 + +> 日期：2026-07-31 +> +> 状态：Accepted（2026-07-31，按本文分阶段实施和验证） +> +> 关联：GitHub Issue #597 +> 适用基线：Spring Boot 3.2.3、Java 21、Logback、Micrometer Actuator + +## 1. 背景 + +SkillHub 已经使用 `X-Request-Id` 关联 API 响应、业务日志和审计记录，但目前仍存在以下问题： + +- 部分应用服务和 DTO 直接读取 SLF4J MDC，可观测性实现泄漏到了业务代码。 +- `X-Request-Id` 接受任意客户端输入，没有统一的长度和字符约束。 +- `@Async` 线程池没有显式传播请求和 Trace 上下文，异步日志可能丢失关联信息。 +- 当前没有标准分布式 Trace，无法通过一个 ID 串联 SkillHub、Scanner 等服务调用。 +- 日志字段尚未形成适合 Elasticsearch/Kibana 查询的稳定结构。 + +本方案用最小建设成本建立通用日志关联与链路追踪基础设施。它不负责建设完整的企业 +可观测性平台，也不把日志、Trace 或 Metrics 逻辑写入业务处理器。 + +Issue #597 中“搜索索引可靠异步交付”应作为独立问题处理，不属于本文范围。 + +## 2. 建设目标 + +一期需要实现： + +1. 每个 HTTP 请求都有合法的 `request.id`。 +2. 启用 Tracing 时，日志包含标准 `trace.id` 和 `span.id`。 +3. `otel-sdk` 模式使用 W3C `traceparent` / `tracestate` 传播 Trace Context。 +4. 业务代码不直接读写 MDC，也不直接依赖 OpenTelemetry 或 SkyWalking API。 +5. 现有 Spring `@Async` 执行器能够正确传播并清理上下文。 +6. 日志以结构化 JSON 输出到 stdout，可由 Filebeat/Fluent Bit 采集到 + Elasticsearch/Kibana。 +7. Trace 可以选择通过 OTLP Collector 接入 SkyWalking。 +8. Collector、SkyWalking、Elasticsearch 或日志采集器不可用时，SkillHub 业务继续运行。 +9. 同一进程只能有一个实际生效的 Tracer。 + +本方案按多个小阶段、小提交实施和验证，全部通过后再统一创建一个替代 PR。 + +## 3. 非目标 + +一期不建设： + +- 搜索索引可靠队列、重试、死信和重放。 +- 多租户差异化采样和运行时动态采样。 +- Spring Cloud Config、Nacos 或可写 Actuator 配置端点。 +- 应用内 OTLP 熔断器或自定义重试框架。 +- 审计日志归档、物理隔离和 WORM 存储。 +- 通用 PII/DLP 检测平台。 +- Prometheus/Grafana/Kibana 告警模板和容量规划平台。 +- Spring Boot 2.x 或 Java 17 兼容。 +- 在业务类上增加 Trace 注解或要求业务开发者操作 Span。 + +## 4. 总体架构 + +```text +HTTP request + │ + ├─ RequestIdFilter + │ └─ request.id + │ + └─ Micrometer Observation / Tracing + ├─ MDC correlation + │ └─ JSON stdout + │ └─ Filebeat / Fluent Bit + │ └─ Elasticsearch / Kibana + │ + └─ OpenTelemetry Bridge + └─ OTLP + └─ OpenTelemetry Collector + └─ SkyWalking OAP +``` + +稳定边界是： + +- 应用内使用 Micrometer Observation/Tracing。 +- `otel-sdk` 模式跨进程使用 W3C Trace Context。 +- Trace 导出使用 OTLP。 +- 日志使用 ECS 风格字段。 +- SkyWalking、Elasticsearch 和 Kibana 都是部署适配器，不进入业务模型。 + +## 5. 运行模式 + +通过一个启动期配置选择运行模式： + +```yaml +skillhub: + observability: + tracing-mode: ${SKILLHUB_TRACING_MODE:none} +``` + +允许值和确定行为： + +| 模式 | Micrometer Tracer | OTLP Exporter | 外部 Agent | 无 Agent/endpoint 时 | +|------|-------------------|---------------|------------|---------------------| +| `none` | NOOP | 无 | 不支持 | 只有 `request.id` | +| `otel-sdk` | OTel Bridge | 配置 endpoint 时创建 | 不支持 | 仍建立进程内 Trace，但不导出 | +| `external-agent` | NOOP | 无 | 可选 | 记录警告并退化为只有 `request.id` | + +运行模式是启动期不变量，不支持热切换。 + +必须保证： + +- `none` 和 `external-agent` 不创建应用内 OTel Span。 +- `otel-sdk` 不支持同时启用 SkyWalking、OTel 或其他外部 Tracing Agent。 +- `external-agent` 不创建 OTLP Exporter。 +- SkillHub 配置能够识别的冲突应在启动时失败；任意 Java Agent 无法被应用可靠识别，因此 + 部署检查和原型测试还必须验证实际 JVM 参数中只有一个 Tracer。 + +一期实现并验证三种模式的互斥边界和日志关联。`external-agent` 只验证 SkyWalking Agent +接管 Trace 后不会与应用内 OTel Tracer 冲突；SkyWalking 特有高级能力不进入 SkillHub +核心代码。 + +## 6. 关联字段契约 + +### 6.1 对外日志字段 + +日志输出统一使用： + +| 字段 | 必需性 | 含义 | +|------|--------|------| +| `request.id` | HTTP 请求或显式任务上下文中存在 | SkillHub API、响应和审计关联 ID | +| `trace.id` | 当前存在有效 Trace 时 | 分布式 Trace ID | +| `span.id` | 当前 Tracer 能提供时 | 当前调用节点 ID | +| `service.name` | 始终存在 | 固定为 `skillhub` | +| `service.version` | 部署时提供 | 发布版本或镜像对应 Commit | +| `service.environment` | 部署时提供 | 当前部署环境 | + +`request.id` 与 `trace.id` 不能合并： + +- `request.id` 属于 SkillHub API 契约，可出现在响应和审计记录中。 +- `trace.id` 属于可选的分布式追踪上下文，可能被采样或关闭。 + +启动日志以及没有显式任务上下文的后台维护日志允许不包含 `request.id`。 + +### 6.2 内部字段映射 + +日志基础设施负责字段映射，业务代码不感知具体 MDC 键： + +| 来源 | 内部字段 | 输出字段 | +|------|----------|----------| +| SkillHub Request Context | `requestId` | `request.id` | +| Micrometer OTel Bridge | `traceId` | `trace.id` | +| Micrometer OTel Bridge | `spanId` | `span.id` | +| SkyWalking Logback Toolkit 事件转换器 | `tid` | `trace.id` | + +SkyWalking Agent 是否能稳定提供独立 `span.id` 以实际原型结果为准。无法稳定提供时允许只 +输出 `trace.id`，不得解析不稳定的内部字符串格式。 + +External Agent 模式通过 SkyWalking 官方 Logback Toolkit 从当前日志事件读取 `tid`； +这不是业务代码读取 MDC，也不能假定 `tid` 一定存在于异步日志线程的 MDC 中。日志编码器 +只读取允许的关联字段，不得把整个 MDC Map 自动写入 JSON。 + +## 7. Request ID + +### 7.1 输入规则 + +客户端可以传入 `X-Request-Id`，但必须同时满足： + +- 长度为 1–64 个字符。 +- 首字符是字母或数字。 +- 其余字符只允许字母、数字、`.`、`_`、`:`、`-`。 + +建议校验表达式： + +```regex +^[A-Za-z0-9][A-Za-z0-9._:-]{0,63}$ +``` + +请求头缺失、为空或不合法时，服务端生成 UUID。响应始终返回最终采用的 +`X-Request-Id`。 + +### 7.2 代码边界 + +新增通用 `RequestIdAccessor` 和对应的 Request ID Scope： + +- Filter 负责解析、校验、建立和清理 Request ID 上下文。 +- 独立 ThreadLocal Scope 是 Request ID 的进程内权威来源。 +- 为该 Scope 注册 Micrometer `ThreadLocalAccessor`，由 + `ContextPropagatingTaskDecorator` 捕获、恢复和清理。 +- Scope 同步维护日志所需的 MDC 镜像，但读取方不能把 MDC 当作权威来源。 +- API 响应工厂通过该抽象读取 Request ID。 +- 审计编排通过该抽象或明确参数读取 Request ID。 +- 应用服务、Controller 和 DTO 不再直接调用 `MDC.get()`。 +- MDC 只作为日志适配器，不再作为业务上下文的权威来源。 + +## 8. Tracing 配置 + +`skillhub-app` 使用 Spring Boot 3.2.3 管理的依赖版本： + +```xml + + io.micrometer + micrometer-tracing-bridge-otel + + + io.opentelemetry + opentelemetry-exporter-otlp + +``` + +基础配置： + +```yaml +management: + tracing: + sampling: + probability: ${SKILLHUB_TRACING_SAMPLING_PROBABILITY:0.1} + baggage: + enabled: false + propagation: + type: W3C + otlp: + tracing: + timeout: ${SKILLHUB_OTLP_TIMEOUT:5s} + compression: ${SKILLHUB_OTLP_COMPRESSION:gzip} +``` + +基础配置不得为 OTLP endpoint 提供默认地址。只有 `otel-sdk` 部署显式设置以下标准 +Spring Boot 配置时才创建 Exporter： + +```bash +MANAGEMENT_OTLP_TRACING_ENDPOINT=http://otel-collector:4318/v1/traces +``` + +一期沿用 OpenTelemetry 1.31 的默认 BatchSpanProcessor 有界队列和丢弃策略，不增加应用内 +重试、熔断或自定义队列实现。 + +## 9. 日志输出 + +### 9.1 输出模式 + +- 本地开发默认使用可读的文本日志。 +- `SKILLHUB_LOG_FORMAT=json` 启用 ECS 风格 JSON stdout。 +- JSON 编码器显式输出标准字段和三个关联字段，不启用“输出全部 MDC”。 +- JSON ConsoleAppender 外包一层 Logback AsyncAppender，初始队列容量为 1024，并允许通过 + `SKILLHUB_LOG_ASYNC_QUEUE_SIZE` 调整。 +- AsyncAppender 使用非阻塞策略；队列耗尽时日志可能丢失，审计事实不依赖该通道。 +- 异常使用 `error.type`、`error.message`、`error.stack_trace`。 +- 队列容量保持可配置，默认值在原型压测后固定，不在设计阶段猜测。 +- 异常和队列丢弃行为必须在测试中验证。 + +示例： + +```json +{ + "@timestamp": "2026-07-31T10:10:10.123Z", + "log.level": "INFO", + "service.name": "skillhub", + "service.version": "0.2.15", + "service.environment": "test", + "request.id": "req-123", + "trace.id": "4bf92f3577b34da6a3ce929d0e0e4736", + "span.id": "00f067aa0ba902b7", + "log.logger": "com.iflytek.skillhub...", + "message": "..." +} +``` + +应用只输出 stdout，不直接依赖 Elasticsearch SDK，也不直接写 Elasticsearch。 + +### 9.2 审计边界 + +`audit_log` 数据库记录仍是审计事实来源。stdout 日志不能代替审计记录，审计留存和归档 +不在本方案中处理。 + +## 10. 上下文传播 + +### 10.1 Spring 异步执行器 + +为现有 `skillhubEventExecutor` 配置 Spring Framework 6.1 的 +`ContextPropagatingTaskDecorator`： + +- 提交任务时捕获 Request ID 和 Trace Context。 +- 执行任务时恢复上下文。 +- 执行完成后在 `finally` 中清理。 +- `CallerRunsPolicy` 触发时也必须保持正确的嵌套作用域。 + +测试必须重复复用同一工作线程，证明不同请求之间不会串号。 + +### 10.2 长生命周期后台线程 + +Redis Stream 消费循环、Reclaimer 和其他长生命周期线程不继承应用启动线程或任意请求的 +MDC。需要追踪具体任务时，由通用任务执行边界建立新的上下文。 + +一期不把 HTTP Trace Context 写入搜索业务 payload，也不改造可靠任务状态机。 + +### 10.3 HTTP 出站 + +一期只管理两类 HTTP Client： + +- 内部 Scanner Client：使用 Spring 管理且带 Observation 的 Builder，传播 W3C Trace + Context。 +- 其他现有 Client：GitHub、GitLab、内置 Skill 公网下载和 S3 Client 均不在一期新增 + Trace Context 传播。 + +后续新增 Client 必须明确选择内部或外部配置，不能依赖全局 Host 正则或在业务代码中手工 +删除 Header。 + +## 11. SkyWalking 与 Elasticsearch 接入 + +### 11.1 OTel SDK 模式 + +推荐链路： + +```text +SkillHub + → OTLP/HTTP + → OpenTelemetry Collector + → OTLP + → SkyWalking OAP +``` + +Collector 用于协议适配和后端路由，不是 SkillHub 的启动依赖。 + +SkyWalking 10.3 的 OTLP Trace 会转换为 Zipkin Trace，并通过 Zipkin Query/Lens UI 查询。 +它不等价于 SkyWalking Java Agent 的原生拓扑、慢 SQL 和 Profiling 能力，部署文档必须 +明确该差异。原型报告必须记录实际使用的 Maven 依赖、Collector、OAP 和 Agent 版本及 +查询结果。 + +### 11.2 External Agent 模式 + +需要 SkyWalking 原生能力时： + +- 使用 `external-agent`。 +- 不配置 SkillHub OTLP endpoint。 +- 由部署环境挂载并启动 SkyWalking Java Agent。 +- 使用 SkyWalking 官方 Logback Toolkit 提供 Trace ID。 +- 日志基础设施将 `tid` 映射为 `trace.id`。 + +### 11.3 日志链路 + +```text +SkillHub JSON stdout + → Filebeat / Fluent Bit + → Elasticsearch + → Kibana +``` + +Kibana 使用 `trace.id` 查询日志，SkyWalking 使用同一个 Trace ID 查询调用链。 + +## 12. 实施步骤 + +### 阶段一：Request ID 与日志边界 + +1. 增加 Request ID 校验。 +2. 建立 `RequestIdAccessor`。 +3. 移除应用服务、Controller、DTO 对 MDC 的直接读取。 +4. 增加允许字段明确的结构化日志配置。 +5. 增加 Request ID 和日志字段测试。 + +可观察结果： + +- 非法 Request ID 被替换。 +- API 响应和审计记录仍使用同一 Request ID。 +- 业务类不再 import `org.slf4j.MDC`。 + +### 阶段二：Micrometer + OTel + +1. 增加 Tracing Bridge 和 OTLP Exporter 依赖。 +2. 增加 `none`、`otel-sdk`、`external-agent` 模式。 +3. 设置 W3C、关闭 baggage、配置采样率。 +4. 保证无 endpoint 时不会产生网络连接。 +5. 保证每个模式只存在一个实际 Tracer。 + +可观察结果： + +- `none` 模式只有 `request.id`。 +- `otel-sdk` 模式日志出现标准 Trace 字段。 +- `external-agent` 模式不会产生应用内 OTel Trace。 + +### 阶段三：传播边界 + +1. 为 `skillhubEventExecutor` 增加上下文传播。 +2. 验证线程复用、嵌套任务和 `CallerRunsPolicy`。 +3. 让内部 Scanner Client 使用 Spring 管理且可观测的 Client Builder。 +4. 验证外部 HTTP Client 不发送 Trace Context。 + +### 阶段四：部署示例与远端验证 + +1. 提供最小 OTel Collector 配置示例。 +2. 补充 SkyWalking OTLP 与 Agent 模式差异。 +3. 将待测分支合入 `big-main`，记录合入后的精确 Commit SHA。 +4. 构建绑定 `big-main` SHA 的测试镜像。 +5. 在共享测试机使用独立容器、网络、数据卷和动态端口运行三个原型。 +6. 生成中文测试报告并保存在本地私有目录，不提交开源仓库。 + +每个阶段使用独立的小提交并保留在同一实现分支；前一阶段的范围测试通过后再进入下一 +阶段。公开 Issue 和 PR 统一在阶段五创建。 + +### 阶段五：社区交付（最后执行） + +该阶段必须在远端验证全部通过后执行： + +1. 创建新的可观测性建设 Issue，说明它承接 #597 中的“通用日志关联与链路追踪”部分。 +2. 搜索索引可靠异步交付继续作为独立问题，不混入新的可观测性 Issue。 +3. 从经过验证的实现分支创建新的 PR，并关联新 Issue。 +4. PR 只包含公开代码、配置、自动化测试和公开部署说明；不得包含测试机地址、凭证、 + 私有端口、原始远端日志或本地中文测试报告。 +5. 在 #597、#644 及其他被替代的关联项中回复： + - 原问题是否真实存在。 + - 为什么不采用原 PR 的实现。 + - 新方案的边界和主要改动。 + - 已完成的自动化及远端验证摘要。 + - 新 Issue 和替代 PR 的链接。 +6. 确认维护者需要的信息完整后，关闭已被替代的 PR；不在验证完成前抢先关闭。 +7. #597 等关联 Issue 只根据剩余问题是否已有明确承接决定关闭、缩小范围或继续保留， + 不因替代 PR 创建而自动关闭。 +8. 新 PR 通过 Review 和 CI 后，确认 PR Head 仍等于已验证的功能 SHA，且该 SHA 可从已 + 测试的 `big-main` SHA 到达；满足后才允许更新 `main`。 +9. 如果 Review 或 CI 修复改变了代码、配置或测试脚本，则原验证证据失效：先将新 SHA + 合入 `big-main`，重新构建镜像并完成受影响的远端验证，再更新 `main`。 + +## 13. 验证方案 + +### 13.1 自动化测试 + +至少覆盖： + +- 未传 Request ID 时自动生成。 +- 合法 Request ID 被保留。 +- 空值、超长值和非法字符被替换。 +- Filter 正常、异常退出后都清理上下文。 +- API 响应、审计和日志中的 Request ID 一致。 +- JSON 只输出允许的关联字段。 +- Trace 采样率在测试中设为 `1.0` 后可稳定断言。 +- `@Async` 线程恢复父上下文。 +- 连续复用同一线程执行不同请求时不串号。 +- `CallerRunsPolicy` 下上下文正确恢复。 +- `none`、`otel-sdk`、`external-agent` 的 Spring Context 互斥。 +- 未配置 OTLP endpoint 时不创建网络导出。 +- 内部 Scanner 请求携带 `traceparent`。 +- 外部 HTTP 请求不携带 `traceparent`。 + +### 13.2 远端原型 + +#### 原型 A：none + +- 不部署 Collector。 +- SkillHub 正常启动并完成核心 Smoke Test。 +- 日志存在 `request.id`，不存在伪造的 Trace 字段。 + +#### 原型 B：otel-sdk + +- SkillHub → Collector → SkyWalking 跑通。 +- JSON 日志进入 Elasticsearch/Kibana。 +- Kibana 与 SkyWalking 能用同一 `trace.id` 查询。 +- Collector 停止后 SkillHub API 和异步任务继续工作。 + +#### 原型 C：external-agent + +- SkyWalking Java Agent 提供原生 Trace。 +- 应用内 OTel Exporter 不工作。 +- 日志能用 SkyWalking Trace ID 关联。 +- 不产生双 Trace、重复 Span 或两个冲突的 Trace ID。 + +### 13.3 远端测试场景 + +- HTTP 成功、4xx、5xx 和未认证请求。 +- Scanner 成功、超时和失败。 +- 异步事件正常执行和抛出异常。 +- 并发请求重复使用线程池。 +- Collector 启动、停止和恢复。 +- 日志采集器停止或消费变慢。 +- 采样率 `0.0`、`0.1` 和 `1.0`。 +- 容器收到 SIGTERM 后日志和 Trace 的关闭行为。 +- 日志中不出现 Authorization、Cookie、Token、密码和完整请求体。 + +## 14. 验收标准 + +以下条件全部满足后，一期才算完成： + +- [ ] 三种模式行为与本文一致。 +- [ ] 业务代码不再直接读取或写入 MDC。 +- [ ] Request ID 校验、响应和审计关联测试通过。 +- [ ] 日志字段符合约定，且不输出完整 MDC。 +- [ ] Spring 异步执行器上下文传播和隔离测试通过。 +- [ ] 内外部 HTTP 传播边界测试通过。 +- [ ] 无 OTLP endpoint 时不存在外部连接尝试。 +- [ ] Collector 中断不影响 SkillHub 业务结果。 +- [ ] OTel SDK 与 SkyWalking Agent 不会同时产生 Trace。 +- [ ] `make test-backend-app` 通过。 +- [ ] `make typecheck-web` 和 `make lint-web` 通过。 +- [ ] 基于 `big-main` 合入后精确 SHA 构建的远端三个原型通过。 +- [ ] 中文测试报告保存在本地私有目录。 +- [ ] 新的可观测性 Issue 和替代 PR 已创建并互相关联。 +- [ ] #597、#644 等关联项已获得清晰回复，被替代的旧 PR 已关闭。 +- [ ] 关联 Issue 已根据剩余范围分别关闭、缩小范围或保留，且状态理由清楚。 +- [ ] 新 PR Head 与已验证功能 SHA 一致，且可从已测试的 `big-main` SHA 到达。 +- [ ] 通过验证后才允许更新 `main`。 + +## 15. 回滚 + +出现问题时： + +1. 将 `SKILLHUB_TRACING_MODE` 改为 `none`。 +2. 删除 `MANAGEMENT_OTLP_TRACING_ENDPOINT`。 +3. 将 `SKILLHUB_LOG_FORMAT` 改为 `text`。 +4. 保留 Request ID 和原有文本日志能力。 +5. 通过滚动重启恢复，不进行运行时模式切换。 + +Tracing 和结构化日志关闭后不得影响 SkillHub 的业务状态、数据库状态或任务执行语义。 + +## 16. 已知限制 + +- 10% Head Sampling 下，全量日志中的部分 `trace.id` 在 SkyWalking 中没有对应 Trace。 +- SkyWalking OTLP 模式的展示能力弱于原生 Java Agent。 +- 日志队列在背压时可能丢弃日志，这是保护业务线程的预期行为。 +- External Agent 提供哪些 MDC 字段取决于具体 Agent 和版本。 +- 一期只处理通用关联和传播，不保证搜索索引异步交付可靠性。 + +## 17. 参考资料 + +- [Spring Boot 3.2.3 Tracing](https://docs.spring.io/spring-boot/docs/3.2.3/reference/html/actuator.html#actuator.micrometer-tracing) +- [Micrometer Tracing](https://docs.micrometer.io/tracing/reference/) +- [OpenTelemetry Java OTLP Exporter](https://opentelemetry.io/docs/languages/java/exporters/) +- [W3C Trace Context](https://www.w3.org/TR/trace-context/) +- [SkyWalking OpenTelemetry Trace](https://skywalking.apache.org/docs/main/v10.3.0/en/setup/backend/otlp-trace/) +- [SkyWalking Logback Toolkit](https://skywalking.apache.org/docs/skywalking-java/next/en/setup/service-agent/java-agent/application-toolkit-logback-1.x/) +- [Elastic ECS Tracing Fields](https://www.elastic.co/docs/reference/ecs/ecs-tracing) +- [方案调研](./research/2026-07-31-observability-common-solutions.md) diff --git a/docs/observability-decision-map.md b/docs/observability-decision-map.md new file mode 100644 index 000000000..7ca4483b8 --- /dev/null +++ b/docs/observability-decision-map.md @@ -0,0 +1,124 @@ +# 通用可观测性决策图 + +目标：为 SkillHub 建立独立、通用、可插拔的日志关联、指标和链路追踪基础设施。 +它观察 HTTP、线程池、定时任务和可靠任务等执行边界，但不进入业务模型和业务载荷。 + +边界： + +- Servlet Filter、执行器装饰器、调度拦截器和任务执行拦截器负责建立/恢复上下文。 +- 业务代码不读写 MDC，不负责创建通用 Span，也不负责统计任务生命周期指标。 +- 使用 W3C Trace Context；日志后端、Metrics 后端和 Trace Exporter 均可替换。 +- 上下文是有长度限制的基础设施元数据，不进入业务 payload。 +- Collector、Exporter 或 Metrics 后端不可用时，主业务和任务内核继续工作。 + +必须满足的不变量： + +- 每个执行边界都正确建立作用域并在 `finally` 清理，线程复用不得串号。 +- 日志稳定输出 `requestId`、`traceId`、`spanId`；任务执行时额外输出执行资源标识。 +- Trace 与 Metrics 可关闭、可替换；关闭后不得改变业务行为。 +- 指标只使用低基数维度，业务 ID 不进入标签。 +- 采集端不可用必须异步、限时、限队列并 fail-open。 + +## #1：可观测性是否与业务和任务状态机彻底分离？ + +Blocked by: 无 +Type: Grilling + +### Question + +可观测性是否只通过通用执行边界和生命周期信号接入，不进入业务处理器？ + +### Answer + +已确认。可靠任务内核只发布通用生命周期信号；可观测性拦截器把执行资源标识加入日志、 +Span 和指标。搜索处理器只处理搜索，不认识 MDC、OpenTelemetry 或 Prometheus。 + +## #2：通用关联身份和传播协议是什么？ + +Blocked by: #1 +Type: Research + +### Question + +如何区分现有 `X-Request-Id`、W3C `traceId/spanId` 和执行资源标识，并跨 HTTP、线程池、 +调度器和持久化任务边界传播？ + +### Answer + +已确定： + +- `requestId` 是 SkillHub 的请求/审计关联标识，不冒充分布式 Trace。 +- `traceId/spanId` 由 Tracer 生成，跨进程只使用 W3C `traceparent/tracestate`。 +- 定时任务或可靠任务的执行资源 ID 只作为当前执行作用域属性，不进入业务 payload。 +- HTTP、线程池、调度器和持久化 carrier 的注入/提取全部位于基础设施拦截器。 +- 不传播任意 MDC Map；baggage 默认关闭，任何允许项都必须低敏、限长、显式配置。 +- 无效或不可信的公网 Trace Context 按 W3C 规则丢弃，服务端控制采样。 + +常见方案和候选组合见 +[Java / Spring 通用日志关联与链路追踪方案调研](./research/2026-07-31-observability-common-solutions.md)。 + +## #3：采用 Micrometer Observation、OpenTelemetry API/SDK 还是 Java Agent？ + +Blocked by: #2 +Type: Research + +### Question + +哪种组合最适配 Spring Boot 3.2.3，并同时支持无 Collector 运行、可选 OTLP 和稳定日志关联？ + +### Answer + +已选择三模式： + +- `none`：不创建应用内 OTel SDK 或 Exporter，只保留 Request ID。 +- `otel-sdk`：使用 Micrometer Tracing + OTel Bridge；配置 OTLP endpoint 时才导出。 +- `external-agent`：应用内使用 NOOP Tracer，由部署环境提供唯一的外部 Agent。 + +应用代码只依赖 Micrometer/Observation 边界，不依赖 OTel SDK 或 SkyWalking API。 +自动配置测试已证明三种模式互斥，错误的 endpoint/mode 组合会在启动时失败。 + +## #4：如何证明上下文传播、日志输出和故障降级正确？ + +Blocked by: #3 +Type: Prototype + +### Question + +验证线程复用隔离、嵌套作用域、异步/调度/持久化任务边界、采样、Exporter 超时、 +Collector 中断、队列打满和关闭观测能力等场景。 + +### Answer + +本地原型已证明： + +- Request ID Scope 在线程复用、嵌套 Scope、异常退出和 `CallerRunsPolicy` 下均能恢复并 + 清理。 +- Micrometer 手工 Span 和 Observation 均能随 `skillhubEventExecutor` 传播。 +- Scanner 使用 Spring 管理的 `WebClient.Builder` 传播 W3C `traceparent`。 +- 面向用户配置的 GitLab 外部 Client 不传播 Trace Context。 +- `none / otel-sdk / external-agent` 的应用上下文和 Exporter 条件符合设计。 + +Collector 中断、日志背压、采样率和关闭行为仍由 `big-main` 精确 SHA 镜像的远端原型验证。 + +## #5：如何形成可部署闭环？ + +Blocked by: #4 +Type: Research + +### Question + +确定 stdout 格式、可选 JSON、Prometheus 或 OTLP Metrics、Trace Exporter、暴露边界、 +低基数告警和运维文档。 + +### Answer + +已确定最小交付： + +- 文本日志用于本地开发，ECS 风格 JSON stdout 用于部署环境。 +- JSON 日志只输出白名单关联字段，通过有界非阻塞 AsyncAppender 保护业务线程。 +- Trace 可经 OTLP Collector 路由到 SkyWalking；需要 SkyWalking 原生能力时改用唯一的 + Java Agent。 +- Prometheus 继续作为可选 Metrics 后端，不是本期链路关联的前置条件。 + +部署配置和三模式操作说明写入 `docs/09-deployment.md`；远端实测结果只保存在本地私有 +中文报告中。 diff --git a/docs/research/2026-07-31-observability-common-solutions.md b/docs/research/2026-07-31-observability-common-solutions.md new file mode 100644 index 000000000..92dbbee6b --- /dev/null +++ b/docs/research/2026-07-31-observability-common-solutions.md @@ -0,0 +1,120 @@ +# Java / Spring 通用日志关联与链路追踪方案调研 + +调研时间：2026-07-31 +适用基线：SkillHub，Spring Boot 3.2.3、Java 21、Logback、Micrometer Actuator + +## 结论 + +当前 Java/Spring 生态已经基本收敛到以下组合： + +1. 使用 W3C `traceparent` / `tracestate` 作为跨进程传播协议。 +2. Spring 应用内使用 Micrometer Observation/Tracing，底层桥接 OpenTelemetry。 +3. 云原生或需要广覆盖自动插桩时使用 OpenTelemetry Java Agent。 +4. 使用 OTLP 把 Trace 发往 Collector，再由 Collector 路由到 Tempo、Jaeger、Zipkin、 + SkyWalking 或商业后端。 +5. 日志只消费当前上下文中的 `traceId` / `spanId`，业务代码不操作 MDC。 + +Spring Cloud Sleuth、手写 MDC/TID、TLog/TTL 和厂商 Agent 仍能见到，但不应作为 +SkillHub 新机制的协议核心。 + +## 常见方案比较 + +| 方案 | 常见使用场景 | 优点 | 主要缺口 | +|---|---|---|---| +| Filter + MDC + TaskDecorator | 单体应用、只要求按 ID 查日志 | 简单、无采集端 | 没有真实 Span；容易漏线程/客户端边界；手写传播易串号 | +| Micrometer Tracing + OTel bridge | Spring Boot 3.x 应用内建观测 | Spring 官方路径；自动日志关联；便于自定义基础设施 Observation | 覆盖依赖 Spring 已观测的组件；线程池仍要正确配置上下文传播 | +| OpenTelemetry Java Agent | Kubernetes、统一运维、需要 JDBC/Redis/HTTP 等广覆盖 | OTel 官方对 Spring Boot 的默认建议；零代码；覆盖面最大 | 需要部署 Agent；必须实测启动/CPU/内存开销；自定义持久化任务边界仍需扩展 | +| OpenTelemetry Spring Boot Starter | Native Image、不能挂 Agent、需要应用 YAML 配置 | OTel SDK 原生集成；适合 Agent 不可用场景 | OTel 官方不把它作为普通 Spring Boot 的默认选择；需要单独管理 OTel BOM | +| SkyWalking/Elastic/Pinpoint 等 Agent | 已统一采购或部署特定 APM 的企业 | 自动插桩成熟、开箱 UI | 协议和后端绑定更强；不适合作为开源产品内部 API | +| Spring Cloud Sleuth | Spring Boot 2.x 历史项目 | 旧生态成熟 | 官方明确不支持 Spring Boot 3.x，核心已迁移到 Micrometer Tracing | + +## 官方事实 + +### Spring Boot + +- Spring Boot 3.2.3 Actuator 为 Micrometer Tracing 提供依赖管理和自动配置。 +- OTel 组合使用 `micrometer-tracing-bridge-otel`；OTLP 使用 + `opentelemetry-exporter-otlp`。 +- 启用 Micrometer Tracing 后，Spring Boot 默认把 `traceId`、`spanId` 放入 MDC，并 + 支持通过 `logging.pattern.correlation` 固定日志格式。 +- Spring Boot 3.2.3 默认产生 W3C 上下文，并可消费 W3C、B3、B3 Multi；新设计应只 + 产生 W3C，兼容消费策略可单独配置。 +- 自动 HTTP 传播依赖 Spring 自动配置的 HTTP Client Builder；自行 `new` 客户端会 + 绕过传播。 +- Spring Framework 6.1 提供 `ContextPropagatingTaskDecorator`，用于恢复日志和 + Observation 上下文；官方同时提醒大量极小任务会有传播开销。 + +### OpenTelemetry + +- OTel 官方把 Java Agent 列为普通 Spring Boot 应用的默认零代码方案，因为它比 + Spring Boot Starter 提供更多开箱插桩。 +- Starter 主要面向 Native Image、Agent 启动开销不满足要求、已有其他 Java Agent， + 或需要通过 Spring 配置文件管理 OTel 的场景。 +- Java Agent 覆盖 Spring Web MVC、JDBC、Lettuce、Java Executors、Logback 等 + SkillHub 关键边界。 +- Agent 的 Logback MDC 默认键为 `trace_id`、`span_id`、`trace_flags`；Micrometer + 默认键为 `traceId`、`spanId`。若支持两种运行模式，必须统一日志字段，不能让查询方 + 感知两套命名。 +- OTel 官方要求在目标部署环境实测 Agent 开销，没有通用的固定开销数字；采样率、 + JDBC/Redis Span 数量和资源限制都会影响结果。 + +### W3C Trace Context + +- `traceparent` / `tracestate` 是厂商中立的传播协议。 +- Header 必须按标准校验；无效上下文应丢弃并创建新 Trace。 +- Trace Context 不得携带用户身份、IP、Token 或其他敏感信息。 +- 公网调用方可伪造 sampled 标志，因此采样和费用控制必须由服务端约束。 + +## 开源项目观察 + +- OpenTelemetry Demo 的 Java 服务直接在镜像中挂载 + `opentelemetry-javaagent.jar`，通过标准 `OTEL_*` 配置连接 Collector，代表 + 云原生 Agent 路径。 +- Spring Petclinic Microservices 使用 Spring Boot tracing starter 和 Zipkin 后端， + 代表 Spring 原生集成路径。后端选择不同，但应用侧仍依赖 Spring 观测抽象。 +- RuoYi-Cloud-Plus 预留 SkyWalking Java Agent 和 OAP/UI，代表厂商 Agent 路径； + 适用于组织已统一使用 SkyWalking 的情况，不适合作为 SkillHub 的内部协议。 + +## 对 SkillHub 的候选结论 + +应用代码的稳定边界应是 Spring 的 Observation/Tracing 抽象与 W3C 协议，而不是某个 +日志或 APM 产品： + +```text +HTTP / Executor / Scheduler / Reliable Task boundary + │ + Observability interceptor + │ + Micrometer Observation / Tracing facade + │ + OpenTelemetry bridge + W3C + │ + optional OTLP exporter / Collector +``` + +候选主运行模式： + +- 应用内使用 Micrometer Tracing + OpenTelemetry bridge，保证 Spring Boot 3.2.3 + 原生整合、统一 MDC 字段和自定义基础设施 Observation。 +- OTLP Exporter 默认关闭；开启后只负责异步导出，不改变请求结果。 +- Java Agent 作为高级部署模式，用于获得 JDBC、Redis、第三方 HTTP Client 等更广 + 自动插桩。Agent 与应用内自动插桩不得同时启用，除非原型证明不会产生重复 Span。 +- 无 Trace SDK/Agent 时仍保留 `requestId` 日志关联；Trace 是增强能力，不是业务前置条件。 + +最终选择仍需原型验证：同一请求的 Span 是否重复、线程池上下文是否串号、Collector +中断是否影响延迟、日志字段是否一致、关闭 tracing 后业务行为是否完全不变。 + +## 参考资料 + +- [Spring Boot 3.2.3 Tracing](https://docs.spring.io/spring-boot/docs/3.2.3/reference/html/actuator.html#actuator.micrometer-tracing) +- [Spring Boot current Tracing](https://docs.spring.io/spring-boot/reference/actuator/tracing.html) +- [Spring Framework 6.1 ContextPropagatingTaskDecorator](https://docs.spring.io/spring-framework/docs/6.1.4/javadoc-api/org/springframework/core/task/support/ContextPropagatingTaskDecorator.html) +- [OpenTelemetry Java Agent](https://opentelemetry.io/docs/zero-code/java/agent/) +- [OpenTelemetry Spring Boot Starter](https://opentelemetry.io/docs/zero-code/java/spring-boot-starter/) +- [OpenTelemetry Java supported libraries](https://opentelemetry.io/docs/zero-code/java/agent/supported-libraries/) +- [OpenTelemetry Java Agent performance](https://opentelemetry.io/docs/zero-code/java/agent/performance/) +- [W3C Trace Context](https://www.w3.org/TR/trace-context/) +- [Spring Cloud Sleuth end-of-line notice](https://docs.spring.io/spring-cloud-sleuth/docs/current/reference/html/) +- [OpenTelemetry Demo](https://github.com/open-telemetry/opentelemetry-demo) +- [Spring Petclinic Microservices](https://github.com/spring-petclinic/spring-petclinic-microservices) +- [RuoYi-Cloud-Plus](https://github.com/dromara/RuoYi-Cloud-Plus) From e9a913e30b81516c1dff67f4e9c249ced38a29d6 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Fri, 31 Jul 2026 15:51:31 +0800 Subject: [PATCH 05/10] fix(observability): tighten tracing integration boundaries Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .env.release.example | 2 +- compose.release.yml | 2 +- docs/09-deployment.md | 10 +- ...6-07-31-observability-construction-plan.md | 12 +- docs/observability-decision-map.md | 24 +-- docs/observability-developer-guide.md | 138 ++++++++++++++++++ .../auth/oauth/GitLabClaimsExtractor.java | 2 + 7 files changed, 171 insertions(+), 19 deletions(-) create mode 100644 docs/observability-developer-guide.md diff --git a/.env.release.example b/.env.release.example index 1b8e38e69..3f8380b2c 100644 --- a/.env.release.example +++ b/.env.release.example @@ -56,7 +56,7 @@ SESSION_COOKIE_SECURE=false # Observability defaults require no Collector or tracing backend. # Use json in container deployments when stdout is collected centrally. SKILLHUB_TRACING_MODE=none -SKILLHUB_LOG_FORMAT=text +SKILLHUB_LOG_FORMAT=json SKILLHUB_LOG_ASYNC_QUEUE_SIZE=1024 SKILLHUB_SERVICE_VERSION=unknown SKILLHUB_SERVICE_ENVIRONMENT=production diff --git a/compose.release.yml b/compose.release.yml index db306bda8..e9cb2a443 100644 --- a/compose.release.yml +++ b/compose.release.yml @@ -89,7 +89,7 @@ services: SKILLHUB_SECURITY_SCANNER_MODE: upload SKILLHUB_AUTH_DIRECT_ENABLED: ${SKILLHUB_AUTH_DIRECT_ENABLED:-false} SKILLHUB_TRACING_MODE: ${SKILLHUB_TRACING_MODE:-none} - SKILLHUB_LOG_FORMAT: ${SKILLHUB_LOG_FORMAT:-text} + SKILLHUB_LOG_FORMAT: ${SKILLHUB_LOG_FORMAT:-json} SKILLHUB_LOG_ASYNC_QUEUE_SIZE: ${SKILLHUB_LOG_ASYNC_QUEUE_SIZE:-1024} SKILLHUB_SERVICE_VERSION: ${SKILLHUB_SERVICE_VERSION:-unknown} SKILLHUB_SERVICE_ENVIRONMENT: ${SKILLHUB_SERVICE_ENVIRONMENT:-production} diff --git a/docs/09-deployment.md b/docs/09-deployment.md index 847391c76..6a6a52762 100644 --- a/docs/09-deployment.md +++ b/docs/09-deployment.md @@ -331,13 +331,14 @@ override 或部署平台环境变量把上述 `SPRING_SECURITY_*` 变量注入 ` ```dotenv SKILLHUB_TRACING_MODE=none -SKILLHUB_LOG_FORMAT=text +SKILLHUB_LOG_FORMAT=json SKILLHUB_SERVICE_VERSION=v0.2.15 SKILLHUB_SERVICE_ENVIRONMENT=production ``` -部署环境建议将 `SKILLHUB_LOG_FORMAT` 设为 `json`，由 Filebeat、Fluent Bit 或容器平台 -采集 stdout。SkillHub 不直接连接 Elasticsearch。JSON 日志使用以下稳定字段： +发布 Compose 默认使用 ECS 风格 JSON，由 Filebeat、Fluent Bit 或容器平台采集 stdout。 +本地源码开发仍可使用 `SKILLHUB_LOG_FORMAT=text`。SkillHub 不直接连接 Elasticsearch。 +JSON 日志使用以下稳定字段： - `request.id`：SkillHub 请求、响应和审计关联 ID。 - `trace.id`、`span.id`：当前存在有效 Trace 时输出。 @@ -447,6 +448,9 @@ SkyWalking 中没有被保留的 Trace。这是头部采样的预期行为。 关闭 Trace 和 JSON 日志不会改变请求、数据库或异步任务的业务语义。 +开发者接入统一标准的最小步骤、内部/外部 HTTP Client 传播边界和扩展点见： +[可观测性开发者接入指南](./observability-developer-guide.md)。 + ## 11 安全扫描服务 如果要启用 `skill-scanner` 后端链路，当前仓库建议按下面的方式部署： diff --git a/docs/2026-07-31-observability-construction-plan.md b/docs/2026-07-31-observability-construction-plan.md index 688528352..cbbe85feb 100644 --- a/docs/2026-07-31-observability-construction-plan.md +++ b/docs/2026-07-31-observability-construction-plan.md @@ -35,7 +35,8 @@ Issue #597 中“搜索索引可靠异步交付”应作为独立问题处理， Elasticsearch/Kibana。 7. Trace 可以选择通过 OTLP Collector 接入 SkyWalking。 8. Collector、SkyWalking、Elasticsearch 或日志采集器不可用时，SkillHub 业务继续运行。 -9. 同一进程只能有一个实际生效的 Tracer。 +9. SkillHub 应用配置只能启用一个应用内 Tracer；`external-agent` 模式下唯一外部 + Agent 由部署参数和发布检查保证。 本方案按多个小阶段、小提交实施和验证，全部通过后再统一创建一个替代 PR。 @@ -104,14 +105,15 @@ skillhub: 必须保证： - `none` 和 `external-agent` 不创建应用内 OTel Span。 -- `otel-sdk` 不支持同时启用 SkyWalking、OTel 或其他外部 Tracing Agent。 +- `otel-sdk` 不支持同时启用 SkyWalking、OTel 或其他外部 Tracing Agent；应用只能校验 + 自身 endpoint/mode 冲突，不能可靠识别任意 JVM Agent。 - `external-agent` 不创建 OTLP Exporter。 - SkillHub 配置能够识别的冲突应在启动时失败；任意 Java Agent 无法被应用可靠识别，因此 部署检查和原型测试还必须验证实际 JVM 参数中只有一个 Tracer。 -一期实现并验证三种模式的互斥边界和日志关联。`external-agent` 只验证 SkyWalking Agent -接管 Trace 后不会与应用内 OTel Tracer 冲突；SkyWalking 特有高级能力不进入 SkillHub -核心代码。 +一期实现并验证三种模式的应用上下文互斥边界和日志关联。`external-agent` 只验证 +SkyWalking Agent 接管 Trace 时应用内 OTel Tracer/Exporter 不工作；“只挂载一个外部 +Agent”属于部署验收项。SkyWalking 特有高级能力不进入 SkillHub 核心代码。 ## 6. 关联字段契约 diff --git a/docs/observability-decision-map.md b/docs/observability-decision-map.md index 7ca4483b8..3f9ad9188 100644 --- a/docs/observability-decision-map.md +++ b/docs/observability-decision-map.md @@ -1,11 +1,14 @@ # 通用可观测性决策图 目标：为 SkillHub 建立独立、通用、可插拔的日志关联、指标和链路追踪基础设施。 -它观察 HTTP、线程池、定时任务和可靠任务等执行边界，但不进入业务模型和业务载荷。 +当前实现覆盖 HTTP、SkillHub 管理的线程池和明确接入的内部 HTTP Client；定时任务、 +Redis Stream 消费循环和 Reclaimer 是独立后台边界，不继承 HTTP 上下文。 +这些边界不进入业务模型和业务载荷。 边界： -- Servlet Filter、执行器装饰器、调度拦截器和任务执行拦截器负责建立/恢复上下文。 +- Servlet Filter、执行器装饰器和明确接入的 Client Builder 负责建立/恢复上下文。 +- 定时任务和 Redis Stream 目前只输出自身执行日志，不自动继承请求或 Trace 上下文。 - 业务代码不读写 MDC，不负责创建通用 Span，也不负责统计任务生命周期指标。 - 使用 W3C Trace Context；日志后端、Metrics 后端和 Trace Exporter 均可替换。 - 上下文是有长度限制的基础设施元数据，不进入业务 payload。 @@ -13,8 +16,9 @@ 必须满足的不变量： -- 每个执行边界都正确建立作用域并在 `finally` 清理，线程复用不得串号。 -- 日志稳定输出 `requestId`、`traceId`、`spanId`；任务执行时额外输出执行资源标识。 +- 每个已纳入本期的执行边界都正确建立作用域并在 `finally` 清理，线程复用不得串号。 +- 日志稳定输出 `requestId`、`traceId`、`spanId`（存在时）；任务执行资源标识不由本期 + 可观测性自动生成。 - Trace 与 Metrics 可关闭、可替换；关闭后不得改变业务行为。 - 指标只使用低基数维度，业务 ID 不进入标签。 - 采集端不可用必须异步、限时、限队列并 fail-open。 @@ -40,8 +44,8 @@ Type: Research ### Question -如何区分现有 `X-Request-Id`、W3C `traceId/spanId` 和执行资源标识，并跨 HTTP、线程池、 -调度器和持久化任务边界传播？ +如何区分现有 `X-Request-Id`、W3C `traceId/spanId` 和执行资源标识，并跨 HTTP、线程池 +边界传播？ ### Answer @@ -50,7 +54,7 @@ Type: Research - `requestId` 是 SkillHub 的请求/审计关联标识，不冒充分布式 Trace。 - `traceId/spanId` 由 Tracer 生成，跨进程只使用 W3C `traceparent/tracestate`。 - 定时任务或可靠任务的执行资源 ID 只作为当前执行作用域属性，不进入业务 payload。 -- HTTP、线程池、调度器和持久化 carrier 的注入/提取全部位于基础设施拦截器。 +- HTTP 和线程池 carrier 的注入/提取位于基础设施拦截器；持久化任务不携带 HTTP Trace。 - 不传播任意 MDC Map；baggage 默认关闭，任何允许项都必须低敏、限长、显式配置。 - 无效或不可信的公网 Trace Context 按 W3C 规则丢弃，服务端控制采样。 @@ -84,8 +88,9 @@ Type: Prototype ### Question -验证线程复用隔离、嵌套作用域、异步/调度/持久化任务边界、采样、Exporter 超时、 -Collector 中断、队列打满和关闭观测能力等场景。 +验证线程复用隔离、嵌套作用域、异步任务边界、采样、Exporter 超时、Collector 中断、 +队列打满和关闭观测能力等场景。调度任务和 Redis Stream 的独立后台边界只验证不继承 +请求上下文。 ### Answer @@ -97,6 +102,7 @@ Collector 中断、队列打满和关闭观测能力等场景。 - Scanner 使用 Spring 管理的 `WebClient.Builder` 传播 W3C `traceparent`。 - 面向用户配置的 GitLab 外部 Client 不传播 Trace Context。 - `none / otel-sdk / external-agent` 的应用上下文和 Exporter 条件符合设计。 +- `@Scheduled` 和 Redis Stream/Reclaimer 不继承请求上下文，保持独立后台执行边界。 Collector 中断、日志背压、采样率和关闭行为仍由 `big-main` 精确 SHA 镜像的远端原型验证。 diff --git a/docs/observability-developer-guide.md b/docs/observability-developer-guide.md new file mode 100644 index 000000000..205a921c7 --- /dev/null +++ b/docs/observability-developer-guide.md @@ -0,0 +1,138 @@ +# 可观测性开发者接入指南 + +本文说明 SkillHub 代码如何接入统一的日志关联和链路追踪标准。 +开发者不需要直接操作 MDC、OpenTelemetry SDK 或 SkyWalking API。 + +## 1. 统一标准 + +| 信息 | 来源 | 日志字段 | 传播方式 | +|---|---|---|---| +| 请求关联 ID | `RequestIdFilter` / `RequestIdAccessor` | `request.id` | `X-Request-Id` | +| 分布式 Trace ID | Micrometer Tracing | `trace.id` | W3C `traceparent` | +| Span ID | Micrometer Tracing | `span.id` | 当前 Trace Scope | + +`request.id` 是 SkillHub 的请求/审计关联标识，不等同于 `trace.id`。 +请求没有链路追踪时仍应保留 `request.id`。 + +## 2. 运行模式 + +通过 `SKILLHUB_TRACING_MODE` 选择一种模式，修改后重启应用： + +- `none`：默认模式。无应用内 OTel SDK 和 OTLP 导出，只保留 `request.id`。 +- `otel-sdk`：使用 Micrometer Tracing + OTel Bridge；配置 + `MANAGEMENT_OTLP_TRACING_ENDPOINT` 后才向 Collector 导出。 +- `external-agent`：应用内 Tracer 为 NOOP，由部署环境提供唯一的外部 Agent。 + SkillHub 只能校验自身配置，不能识别任意 JVM Agent；唯一 Agent 是部署检查项。 + +`none`/`external-agent` 不能配置 OTLP endpoint；`otel-sdk` 与外部 Tracing Agent +不得在同一进程中叠加。 + +## 3. 开发者接入方式 + +### 3.1 普通 HTTP 请求 + +不需要增加代码。`RequestIdFilter` 会生成或校验 `X-Request-Id`，并在请求结束时清理 +线程上下文。Micrometer Tracing 负责在 `otel-sdk` 模式下创建 HTTP Observation 和 Trace。 + +业务代码不要： + +- `MDC.put` / `MDC.remove` 写入请求关联字段； +- 手工解析或拼接 `traceparent`； +- 在日志中输出完整 MDC Map。 + +### 3.2 Spring 异步任务 + +优先使用已有的 `skillhubEventExecutor`： + +```java +@Async("skillhubEventExecutor") +public void handleEvent(SkillPublishedEvent event) { + // 直接记录日志即可，request.id/trace.id/span.id 会按提交时的上下文恢复 +} +``` + +新增 Spring 管理的线程池时，注入统一的 +`ContextPropagatingTaskDecorator`，不要自己复制 MDC： + +```java +@Bean +ThreadPoolTaskExecutor myExecutor( + ContextPropagatingTaskDecorator contextDecorator +) { + ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); + executor.setTaskDecorator(contextDecorator); + executor.initialize(); + return executor; +} +``` + +该装饰器负责捕获、恢复和清理 `RequestIdAccessor` 与 OTel Observation Scope。 + +### 3.3 内部 HTTP 服务 + +内部服务调用必须使用 Spring 管理的 `WebClient.Builder`，这样 `otel-sdk` 模式下会 +自动传播 W3C Trace Context： + +```java +@Bean +HttpClient scannerClient( + WebClient.Builder builder +) { + return new WebClientHttpClient(builder.build()); +} +``` + +Scanner 是当前已接入的内部客户端。新增内部客户端时，应补一个测试，断言请求包含合法 +的 `traceparent`。 + +### 3.4 外部 HTTP 服务 + +面向用户配置的 GitLab、第三方 API 等外部服务不要复用内部观测 Builder，也不要手工 +删除 Header。使用明确不接入 SkillHub Observation 的客户端，并补测试断言请求不包含 +`traceparent`。 + +### 3.5 定时任务和 Redis Stream + +当前实现把定时任务、Redis Stream 消费循环和 Reclaimer 视为独立后台执行边界： + +- 不继承任意 HTTP 请求的 `request.id` 或 `trace.id`； +- 不把 HTTP Trace Context 写入 Redis 业务载荷； +- 日志仍可使用 ECS 格式和固定服务字段； +- 若未来需要任务级关联，应增加独立的任务执行 ID/Observation carrier，并单独设计 + 持久化与重试语义。 + +因此，不要假设在 `@Scheduled` 或 Stream consumer 中能自动查到发起 HTTP 请求的 Trace。 + +## 4. 可扩展点 + +| 扩展需求 | 应扩展的位置 | 不应修改的位置 | +|---|---|---| +| 新增请求关联来源 | `RequestIdFilter` / `RequestIdAccessor` | 业务 Controller、DTO | +| 新增线程上下文 | `RequestIdThreadLocalAccessor` / `ContextRegistry` | 每个任务的 `MDC` 代码 | +| 新增 Tracing 后端 | Micrometer Bridge / Collector 配置 | 业务服务 | +| 新增日志字段 | `SkillHubEcsEncoder` 白名单 | “输出全部 MDC” | +| 新增内部 HTTP 客户端 | Spring `WebClient.Builder` + propagation test | URL 正则删 Header | +| 新增外部 HTTP 客户端 | 独立客户端构建入口 + no-propagation test | 依赖全局默认行为 | + +## 5. 接入验收清单 + +新增一个执行边界或客户端时，至少补充： + +1. `none` 模式下业务结果不变； +2. `otel-sdk` 模式下内部调用的 `traceparent` 合法； +3. 外部调用不携带 `traceparent`； +4. 线程复用后上下文被清理，不发生串号； +5. 日志只出现 `request.id`、`trace.id`、`span.id` 等白名单字段； +6. Collector 不可用时不影响业务结果。 + +运行后端验证使用： + +```bash +make test-backend-app +``` + +部署级变更再运行： + +```bash +make staging +``` diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/GitLabClaimsExtractor.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/GitLabClaimsExtractor.java index 7a744a30e..6f0a87b40 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/GitLabClaimsExtractor.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/GitLabClaimsExtractor.java @@ -3,6 +3,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest; @@ -31,6 +32,7 @@ public class GitLabClaimsExtractor implements OAuthClaimsExtractor { * Uses an external-service client that is intentionally not customized with application * tracing. Trace context must not be propagated to a user-configured GitLab host. */ + @Autowired public GitLabClaimsExtractor() { this(RestClient.builder()); } From 3f14d6e3ead11374075768db0b7210d66f96c60e Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Mon, 3 Aug 2026 11:57:18 +0800 Subject: [PATCH 06/10] fix(observability): harden operational log privacy Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../exception/GlobalExceptionHandler.java | 25 ++--- .../skillhub/filter/RequestLoggingFilter.java | 20 ---- .../exception/GlobalExceptionHandlerTest.java | 102 +++++++++++++++++- .../filter/RequestLoggingFilterTest.java | 17 +-- 4 files changed, 121 insertions(+), 43 deletions(-) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java index 712411862..c2c807b68 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java @@ -1,7 +1,6 @@ package com.iflytek.skillhub.exception; import com.iflytek.skillhub.auth.exception.AuthFlowException; -import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; import com.iflytek.skillhub.domain.shared.exception.LocalizedDomainException; @@ -113,11 +112,11 @@ public ResponseEntity> handleSessionInvalidated( public ResponseEntity> handleStorageAccess(StorageAccessException ex, HttpServletRequest request) { metrics.incrementStorageAccessFailure(ex.getOperation()); logger.warn( - "Object storage unavailable [requestId={}, method={}, path={}, userId={}, operation={}, key={}]", + "Object storage unavailable [requestId={}, method={}, path={}, authentication={}, operation={}, key={}]", requestIdAccessor.current(), request.getMethod(), sensitiveLogSanitizer.sanitizeRequestTarget(request), - resolveUserId(request), + resolveAuthenticationState(request), ex.getOperation(), ex.getKey(), ex @@ -142,11 +141,11 @@ public ResponseEntity handleAsyncRequestTimeout(AsyncRequestTimeoutException @ExceptionHandler(Exception.class) public ResponseEntity> handleGlobalException(Exception ex, HttpServletRequest request) { logger.error( - "Unhandled API exception [requestId={}, method={}, path={}, userId={}]", + "Unhandled API exception [requestId={}, method={}, path={}, authentication={}]", requestIdAccessor.current(), request.getMethod(), sensitiveLogSanitizer.sanitizeRequestTarget(request), - resolveUserId(request), + resolveAuthenticationState(request), ex ); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body( @@ -155,12 +154,12 @@ public ResponseEntity> handleGlobalException(Exception ex, Htt private void logHandledException(HttpStatus status, String messageCode, HttpServletRequest request) { logger.info( - "API request failed [requestId={}, status={}, method={}, path={}, userId={}, code={}]", + "API request failed [requestId={}, status={}, method={}, path={}, authentication={}, code={}]", requestIdAccessor.current(), status.value(), request.getMethod(), sensitiveLogSanitizer.sanitizeRequestTarget(request), - resolveUserId(request), + resolveAuthenticationState(request), messageCode ); } @@ -173,13 +172,11 @@ private ResponseEntity> renderLocalizedError(LocalizedMessage apiResponseFactory.error(status.value(), error.messageCode(), error.messageArgs())); } - private String resolveUserId(HttpServletRequest request) { - if (!(request.getUserPrincipal() instanceof Authentication authentication)) { - return "anonymous"; + private String resolveAuthenticationState(HttpServletRequest request) { + if (request.getUserPrincipal() instanceof Authentication authentication + && authentication.isAuthenticated()) { + return "authenticated"; } - if (authentication.getPrincipal() instanceof PlatformPrincipal principal) { - return principal.userId(); - } - return authentication.getName(); + return "anonymous"; } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/RequestLoggingFilter.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/RequestLoggingFilter.java index d46a1e49d..1de3809b2 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/RequestLoggingFilter.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/RequestLoggingFilter.java @@ -16,7 +16,6 @@ import org.springframework.web.util.ContentCachingResponseWrapper; import java.io.IOException; -import java.io.UnsupportedEncodingException; import java.util.Set; /** @@ -27,8 +26,6 @@ public class RequestLoggingFilter extends OncePerRequestFilter { private static final Logger log = LoggerFactory.getLogger(RequestLoggingFilter.class); - private static final int MAX_LOG_BODY_LENGTH = 200; - private static final Set SKIP_PREFIXES = Set.of( "/actuator", "/favicon.ico", "/assets/" ); @@ -85,11 +82,6 @@ private void logRequest(ContentCachingRequestWrapper request, ContentCachingResp sb.append(" | UA: ").append(truncate(userAgent, 80)); } - String requestBody = getRequestBody(request); - if (requestBody != null && !requestBody.isBlank()) { - sb.append(" | Body: ").append(requestBody); - } - log.info(sb.toString()); } @@ -117,18 +109,6 @@ private void prepareSseResponse(HttpServletResponse response) { response.setHeader("X-Accel-Buffering", "no"); } - private String getRequestBody(ContentCachingRequestWrapper request) { - byte[] buf = request.getContentAsByteArray(); - if (buf.length > 0) { - try { - return truncate(new String(buf, request.getCharacterEncoding()), MAX_LOG_BODY_LENGTH); - } catch (UnsupportedEncodingException e) { - return "[unknown encoding]"; - } - } - return null; - } - private String truncate(String value, int maxLength) { if (value == null || value.length() <= maxLength) { return value; diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java index 6aae4cb05..b29c54289 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java @@ -4,28 +4,44 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.when; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; import com.iflytek.skillhub.metrics.SkillHubMetrics; import com.iflytek.skillhub.observability.RequestIdAccessor; import com.iflytek.skillhub.security.SensitiveLogSanitizer; +import com.iflytek.skillhub.storage.StorageAccessException; import jakarta.servlet.http.HttpServletRequest; import java.time.Clock; import java.time.Instant; import java.time.ZoneOffset; +import java.util.List; +import java.util.Set; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.slf4j.LoggerFactory; import org.springframework.context.support.StaticMessageSource; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.web.context.request.async.AsyncRequestTimeoutException; @ExtendWith(MockitoExtension.class) class GlobalExceptionHandlerTest { + private static final String STABLE_USER_ID = "stable-user-123"; + + private final Logger logger = + (Logger) LoggerFactory.getLogger(GlobalExceptionHandler.class); + @Mock private SensitiveLogSanitizer sensitiveLogSanitizer; @@ -36,12 +52,14 @@ class GlobalExceptionHandlerTest { private HttpServletRequest request; private GlobalExceptionHandler handler; + private ListAppender appender; + private RequestIdAccessor requestIdAccessor; @BeforeEach void setUp() { StaticMessageSource messageSource = new StaticMessageSource(); messageSource.addMessage("error.request.timeout", java.util.Locale.getDefault(), "Request timed out"); - RequestIdAccessor requestIdAccessor = new RequestIdAccessor(); + requestIdAccessor = new RequestIdAccessor(); ApiResponseFactory responseFactory = new ApiResponseFactory( messageSource, Clock.fixed(Instant.parse("2026-03-20T00:00:00Z"), ZoneOffset.UTC), @@ -55,6 +73,14 @@ void setUp() { ); } + @AfterEach + void tearDown() { + if (appender != null) { + logger.detachAppender(appender); + appender.stop(); + } + } + @Test void handleAsyncRequestTimeout_shouldReturnNoContentForSseRequests() { when(request.getRequestURI()).thenReturn("/api/v1/notifications/sse"); @@ -67,6 +93,7 @@ void handleAsyncRequestTimeout_shouldReturnNoContentForSseRequests() { @Test void handleAsyncRequestTimeout_shouldReturnApiEnvelopeForNonSseRequests() { + attachAppender(); when(request.getRequestURI()).thenReturn("/api/v1/publish"); when(request.getMethod()).thenReturn("POST"); when(sensitiveLogSanitizer.sanitizeRequestTarget(request)).thenReturn("/api/v1/publish"); @@ -78,6 +105,53 @@ void handleAsyncRequestTimeout_shouldReturnApiEnvelopeForNonSseRequests() { ApiResponse body = (ApiResponse) response.getBody(); assertThat(body.code()).isEqualTo(408); assertThat(body.msg()).isEqualTo("Request timed out"); + assertThat(loggedMessages()).anySatisfy(message -> assertThat(message) + .contains("authentication=anonymous") + .doesNotContain("userId=")); + } + + @Test + void handleGlobalException_shouldLogAuthenticationWithoutStableUserId() { + authenticateRequest(); + attachAppender(); + when(request.getMethod()).thenReturn("GET"); + when(sensitiveLogSanitizer.sanitizeRequestTarget(request)) + .thenReturn("/api/v1/skills/sensitive"); + + try (RequestIdAccessor.Scope ignored = requestIdAccessor.open("request-123")) { + ResponseEntity> response = handler.handleGlobalException( + new RuntimeException("boom"), request); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); + } + + assertThat(loggedMessages()).anySatisfy(message -> assertThat(message) + .contains("requestId=request-123") + .contains("authentication=authenticated") + .doesNotContain(STABLE_USER_ID) + .doesNotContain("userId=")); + } + + @Test + void handleStorageAccess_shouldLogAuthenticationWithoutStableUserId() { + authenticateRequest(); + attachAppender(); + when(request.getMethod()).thenReturn("GET"); + when(sensitiveLogSanitizer.sanitizeRequestTarget(request)) + .thenReturn("/api/v1/skills/test/download"); + + StorageAccessException exception = new StorageAccessException( + "download", "skills/test.zip", new RuntimeException("unavailable")); + try (RequestIdAccessor.Scope ignored = requestIdAccessor.open("request-123")) { + ResponseEntity> response = + handler.handleStorageAccess(exception, request); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.SERVICE_UNAVAILABLE); + } + + assertThat(loggedMessages()).anySatisfy(message -> assertThat(message) + .contains("requestId=request-123") + .contains("authentication=authenticated") + .doesNotContain(STABLE_USER_ID) + .doesNotContain("userId=")); } @Test @@ -100,4 +174,30 @@ void handleSessionInvalidated_shouldRethrowNonSessionException() { assertThatThrownBy(() -> handler.handleSessionInvalidated(ex, request)) .isSameAs(ex); } + + private void authenticateRequest() { + PlatformPrincipal principal = new PlatformPrincipal( + STABLE_USER_ID, + "User", + "user@example.com", + null, + "local", + Set.of("USER") + ); + when(request.getUserPrincipal()).thenReturn( + new UsernamePasswordAuthenticationToken(principal, null, List.of())); + } + + private void attachAppender() { + logger.setLevel(Level.INFO); + appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + } + + private List loggedMessages() { + return appender.list.stream() + .map(ILoggingEvent::getFormattedMessage) + .toList(); + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/RequestLoggingFilterTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/RequestLoggingFilterTest.java index 11ec0aecb..18e6f88a7 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/RequestLoggingFilterTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/RequestLoggingFilterTest.java @@ -36,16 +36,17 @@ void tearDown() { } @Test - void doFilterInternal_truncatesLongRequestBodyAndOmitsResponseBody() + void doFilterInternal_omitsRequestAndResponseBodies() throws ServletException, IOException { RequestLoggingFilter filter = new RequestLoggingFilter(); - String longBody = "x".repeat(5_000); + String requestBody = "{\"username\":\"alice\",\"password\":\"super-secret\"}"; + String responseBody = "x".repeat(5_000); attachAppender(); MockHttpServletRequest request = new MockHttpServletRequest("POST", "/api/test"); request.setCharacterEncoding(StandardCharsets.UTF_8.name()); request.setContentType("application/json"); - request.setContent(longBody.getBytes(StandardCharsets.UTF_8)); + request.setContent(requestBody.getBytes(StandardCharsets.UTF_8)); MockHttpServletResponse response = new MockHttpServletResponse(); response.setCharacterEncoding(StandardCharsets.UTF_8.name()); @@ -53,17 +54,17 @@ void doFilterInternal_truncatesLongRequestBodyAndOmitsResponseBody() FilterChain filterChain = (req, res) -> { req.getReader().lines().count(); res.setContentType("application/json"); - res.getWriter().write(longBody); + res.getWriter().write(responseBody); }; filter.doFilter(request, response, filterChain); List loggedMessages = loggedMessages(); - assertThat(loggedMessages).anySatisfy(message -> - assertThat(message).contains("Body: " + "x".repeat(200) + "...[truncated]")); - assertThat(loggedMessages).noneMatch(message -> message.contains("Body: " + longBody)); + assertThat(loggedMessages).anyMatch(message -> message.contains("POST /api/test")); + assertThat(loggedMessages).noneMatch(message -> message.contains("Body:")); + assertThat(loggedMessages).noneMatch(message -> message.contains("super-secret")); assertThat(loggedMessages).noneMatch(message -> message.contains("Response Body:")); - assertThat(response.getContentAsString()).isEqualTo(longBody); + assertThat(response.getContentAsString()).isEqualTo(responseBody); } @Test From 5058cc3387554d85e87e54862461ceb73c997c18 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Mon, 3 Aug 2026 14:46:03 +0800 Subject: [PATCH 07/10] feat(observability): propagate message trace context Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- ...6-07-31-observability-construction-plan.md | 16 +- docs/observability-decision-map.md | 25 +- docs/observability-developer-guide.md | 28 ++- .../skillhub/config/RedisStreamConfig.java | 14 +- .../skillhub/filter/RequestIdFilter.java | 5 +- .../observability/MessageCarrierAdapter.java | 15 ++ .../MessageObservationSupport.java | 161 +++++++++++++ .../observability/RequestIdAccessor.java | 15 ++ .../stream/AbstractStreamConsumer.java | 34 ++- .../stream/RedisStreamMessageCarrier.java | 34 +++ .../stream/RedissonScanTaskProducer.java | 17 +- .../skillhub/stream/ScanTaskConsumer.java | 20 +- .../iflytek/skillhub/stream/package-info.java | 4 + .../MessageObservationSupportTest.java | 217 ++++++++++++++++++ .../stream/AbstractStreamConsumerTest.java | 41 +++- .../RedissonScanTaskProducerLoggingTest.java | 9 +- .../stream/RedissonScanTaskProducerTest.java | 37 ++- .../stream/ScanTaskConsumerLoggingTest.java | 6 +- .../ScanTaskConsumerPathSafetyTest.java | 6 +- .../skillhub/stream/ScanTaskConsumerTest.java | 6 +- 20 files changed, 655 insertions(+), 55 deletions(-) create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/MessageCarrierAdapter.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/MessageObservationSupport.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedisStreamMessageCarrier.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/package-info.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/MessageObservationSupportTest.java diff --git a/docs/2026-07-31-observability-construction-plan.md b/docs/2026-07-31-observability-construction-plan.md index cbbe85feb..d03651a07 100644 --- a/docs/2026-07-31-observability-construction-plan.md +++ b/docs/2026-07-31-observability-construction-plan.md @@ -282,12 +282,16 @@ MANAGEMENT_OTLP_TRACING_ENDPOINT=http://otel-collector:4318/v1/traces 测试必须重复复用同一工作线程，证明不同请求之间不会串号。 -### 10.2 长生命周期后台线程 +### 10.2 消息队列与长生命周期后台线程 -Redis Stream 消费循环、Reclaimer 和其他长生命周期线程不继承应用启动线程或任意请求的 -MDC。需要追踪具体任务时，由通用任务执行边界建立新的上下文。 +Redis Stream 消费循环和 Reclaimer 不继承应用启动线程或任意请求的 MDC。Producer 通过 +通用消息 Observation 把 W3C Trace Context 与受控 Request ID 注入 transport metadata； +Consumer/Reclaimer 逐条提取、建立 Scope，并在处理结束后清理。Scanner HTTP 调用自然成为 +Consumer Span 的子调用。 -一期不把 HTTP Trace Context 写入搜索业务 payload，也不改造可靠任务状态机。 +上下文不写入 `ScanTask` 或搜索业务 payload，也不改变可靠任务状态机。普通定时任务没有 +上游 carrier，仍建立独立执行上下文；长期延迟任务使用稳定任务 ID 或 Span Link，不维持 +超长父 Span。 ### 10.3 HTTP 出站 @@ -434,6 +438,8 @@ Kibana 使用 `trace.id` 查询日志，SkyWalking 使用同一个 Trace ID 查 - `none`、`otel-sdk`、`external-agent` 的 Spring Context 互斥。 - 未配置 OTLP endpoint 时不创建网络导出。 - 内部 Scanner 请求携带 `traceparent`。 +- Redis Stream Producer/Consumer 保持同一 Trace 和 Request ID，处理结束后线程不串号。 +- 重试发布和 Reclaimer 重新消费仍能恢复消息关联上下文。 - 外部 HTTP 请求不携带 `traceparent`。 ### 13.2 远端原型 @@ -463,6 +469,7 @@ Kibana 使用 `trace.id` 查询日志，SkyWalking 使用同一个 Trace ID 查 - HTTP 成功、4xx、5xx 和未认证请求。 - Scanner 成功、超时和失败。 - 异步事件正常执行和抛出异常。 +- Redis Stream 正常消费、失败重试、Pending Reclaim 和重复投递。 - 并发请求重复使用线程池。 - Collector 启动、停止和恢复。 - 日志采集器停止或消费变慢。 @@ -479,6 +486,7 @@ Kibana 使用 `trace.id` 查询日志，SkyWalking 使用同一个 Trace ID 查 - [ ] Request ID 校验、响应和审计关联测试通过。 - [ ] 日志字段符合约定，且不输出完整 MDC。 - [ ] Spring 异步执行器上下文传播和隔离测试通过。 +- [ ] Redis Stream 消息上下文传播、重试、Reclaimer 和隔离测试通过。 - [ ] 内外部 HTTP 传播边界测试通过。 - [ ] 无 OTLP endpoint 时不存在外部连接尝试。 - [ ] Collector 中断不影响 SkillHub 业务结果。 diff --git a/docs/observability-decision-map.md b/docs/observability-decision-map.md index 3f9ad9188..fc9c039a1 100644 --- a/docs/observability-decision-map.md +++ b/docs/observability-decision-map.md @@ -1,14 +1,15 @@ # 通用可观测性决策图 目标：为 SkillHub 建立独立、通用、可插拔的日志关联、指标和链路追踪基础设施。 -当前实现覆盖 HTTP、SkillHub 管理的线程池和明确接入的内部 HTTP Client；定时任务、 -Redis Stream 消费循环和 Reclaimer 是独立后台边界，不继承 HTTP 上下文。 -这些边界不进入业务模型和业务载荷。 +当前实现覆盖 HTTP、SkillHub 管理的线程池、Redis Stream 和明确接入的内部 HTTP Client； +普通定时任务没有上游 carrier，仍是独立后台边界。Redis Stream 上下文只进入 transport +metadata，不进入业务模型和业务载荷。 边界： -- Servlet Filter、执行器装饰器和明确接入的 Client Builder 负责建立/恢复上下文。 -- 定时任务和 Redis Stream 目前只输出自身执行日志，不自动继承请求或 Trace 上下文。 +- Servlet Filter、执行器装饰器、消息 Observation 和明确接入的 Client Builder 负责 + 建立/恢复上下文。 +- Redis Stream Producer 注入、Consumer/Reclaimer 逐条提取；定时任务不继承任意请求。 - 业务代码不读写 MDC，不负责创建通用 Span，也不负责统计任务生命周期指标。 - 使用 W3C Trace Context；日志后端、Metrics 后端和 Trace Exporter 均可替换。 - 上下文是有长度限制的基础设施元数据，不进入业务 payload。 @@ -54,7 +55,8 @@ Type: Research - `requestId` 是 SkillHub 的请求/审计关联标识，不冒充分布式 Trace。 - `traceId/spanId` 由 Tracer 生成，跨进程只使用 W3C `traceparent/tracestate`。 - 定时任务或可靠任务的执行资源 ID 只作为当前执行作用域属性，不进入业务 payload。 -- HTTP 和线程池 carrier 的注入/提取位于基础设施拦截器；持久化任务不携带 HTTP Trace。 +- HTTP、线程池和消息 carrier 的注入/提取位于基础设施层；消息上下文是 transport + metadata，不是任务业务字段。 - 不传播任意 MDC Map；baggage 默认关闭，任何允许项都必须低敏、限长、显式配置。 - 无效或不可信的公网 Trace Context 按 W3C 规则丢弃，服务端控制采样。 @@ -88,9 +90,9 @@ Type: Prototype ### Question -验证线程复用隔离、嵌套作用域、异步任务边界、采样、Exporter 超时、Collector 中断、 -队列打满和关闭观测能力等场景。调度任务和 Redis Stream 的独立后台边界只验证不继承 -请求上下文。 +验证线程复用隔离、嵌套作用域、异步任务边界、消息传播、采样、Exporter 超时、 +Collector 中断、队列打满和关闭观测能力等场景。调度任务验证不继承请求上下文； +Redis Stream 验证逐条注入、提取和清理。 ### Answer @@ -99,10 +101,13 @@ Type: Prototype - Request ID Scope 在线程复用、嵌套 Scope、异常退出和 `CallerRunsPolicy` 下均能恢复并 清理。 - Micrometer 手工 Span 和 Observation 均能随 `skillhubEventExecutor` 传播。 +- Redis Stream Producer/Consumer 通过通用消息 Observation 传播 W3C Trace Context 和 + 受控 Request ID，Reclaimer 从原消息重新提取。 - Scanner 使用 Spring 管理的 `WebClient.Builder` 传播 W3C `traceparent`。 - 面向用户配置的 GitLab 外部 Client 不传播 Trace Context。 - `none / otel-sdk / external-agent` 的应用上下文和 Exporter 条件符合设计。 -- `@Scheduled` 和 Redis Stream/Reclaimer 不继承请求上下文，保持独立后台执行边界。 +- `@Scheduled` 保持独立后台执行边界；Redis Stream/Reclaimer 不继承线程上下文，而是 + 从每条消息的 transport metadata 恢复。 Collector 中断、日志背压、采样率和关闭行为仍由 `big-main` 精确 SHA 镜像的远端原型验证。 diff --git a/docs/observability-developer-guide.md b/docs/observability-developer-guide.md index 205a921c7..405105edd 100644 --- a/docs/observability-developer-guide.md +++ b/docs/observability-developer-guide.md @@ -91,17 +91,27 @@ Scanner 是当前已接入的内部客户端。新增内部客户端时，应补 删除 Header。使用明确不接入 SkillHub Observation 的客户端，并补测试断言请求不包含 `traceparent`。 -### 3.5 定时任务和 Redis Stream +### 3.5 Redis Stream 和定时任务 -当前实现把定时任务、Redis Stream 消费循环和 Reclaimer 视为独立后台执行边界： +Redis Stream 已通过 `MessageObservationSupport` 接入通用消息传播： -- 不继承任意 HTTP 请求的 `request.id` 或 `trace.id`； -- 不把 HTTP Trace Context 写入 Redis 业务载荷； -- 日志仍可使用 ECS 格式和固定服务字段； -- 若未来需要任务级关联，应增加独立的任务执行 ID/Observation carrier，并单独设计 - 持久化与重试语义。 +- Producer 把 `traceparent`、`tracestate` 和受控的 `skillhub.request_id` 写入 Stream + transport metadata，不修改 `ScanTask` 等业务对象； +- `AbstractStreamConsumer` 逐条提取上下文并建立 `CONSUMER` Observation，在 `finally` + 中恢复线程原状态； +- Consumer 内部调用 Scanner 时，Spring 管理的 `WebClient` 自动创建同一 Trace 的子 Span； +- 重试发布发生在当前 Consumer Scope 内，新消息继续携带关联上下文；Reclaimer 处理原消息 + 时重新从消息提取，不继承 Reclaimer 线程的上下文； +- `none` 和 `external-agent` 模式仍传播 Request ID；应用保证完整 W3C Trace 的模式是 + `otel-sdk`，外部 Agent 的跨 Stream Trace 能力取决于对应 Agent 插件。 -因此，不要假设在 `@Scheduled` 或 Stream consumer 中能自动查到发起 HTTP 请求的 Trace。 +新增 Redis Stream Consumer 应继承 `AbstractStreamConsumer`，新增 Producer 应调用 +`MessageObservationSupport.observePublish`。其他消息中间件只实现自身 carrier 的 +`MessageCarrierAdapter`；传播核心不依赖 Redis、Redisson 或 `Map`。不要在业务 DTO、MDC +或日志代码中复制上下文。 + +普通 `@Scheduled` 任务没有上游消息 carrier，仍是独立后台边界；需要长期任务关联时应使用 +稳定任务 ID，而不是把任意历史 HTTP Span 保持为超长父 Span。 ## 4. 可扩展点 @@ -113,6 +123,7 @@ Scanner 是当前已接入的内部客户端。新增内部客户端时，应补 | 新增日志字段 | `SkillHubEcsEncoder` 白名单 | “输出全部 MDC” | | 新增内部 HTTP 客户端 | Spring `WebClient.Builder` + propagation test | URL 正则删 Header | | 新增外部 HTTP 客户端 | 独立客户端构建入口 + no-propagation test | 依赖全局默认行为 | +| 新增消息队列边界 | `MessageObservationSupport` + `MessageCarrierAdapter` | 业务 DTO、手工 MDC/OTel API | ## 5. 接入验收清单 @@ -124,6 +135,7 @@ Scanner 是当前已接入的内部客户端。新增内部客户端时，应补 4. 线程复用后上下文被清理，不发生串号； 5. 日志只出现 `request.id`、`trace.id`、`span.id` 等白名单字段； 6. Collector 不可用时不影响业务结果。 +7. 消息 Producer/Consumer 使用同一 Trace，Request ID 不串号，重试和 Reclaimer 不丢关联。 运行后端验证使用： diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/RedisStreamConfig.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/RedisStreamConfig.java index 8c479cfeb..87ca07624 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/RedisStreamConfig.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/config/RedisStreamConfig.java @@ -4,6 +4,7 @@ import com.iflytek.skillhub.domain.security.SecurityScanService; import com.iflytek.skillhub.domain.security.SecurityScanner; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.observability.MessageObservationSupport; import com.iflytek.skillhub.storage.ObjectStorageService; import com.iflytek.skillhub.stream.RedissonScanTaskProducer; import com.iflytek.skillhub.stream.ScanTaskConsumer; @@ -37,8 +38,11 @@ public class RedisStreamConfig { private Duration reclaimInterval; @Bean - public RedissonScanTaskProducer redisScanTaskProducer(RedissonClient redissonClient) { - return new RedissonScanTaskProducer(redissonClient, streamKey); + public RedissonScanTaskProducer redisScanTaskProducer( + RedissonClient redissonClient, + MessageObservationSupport messageObservationSupport + ) { + return new RedissonScanTaskProducer(redissonClient, streamKey, messageObservationSupport); } @Bean @@ -47,7 +51,8 @@ public ScanTaskConsumer scanTaskConsumer(RedissonClient redissonClient, SecurityScanService securityScanService, SkillVersionRepository skillVersionRepository, ScanTaskProducer scanTaskProducer, - ObjectStorageService objectStorageService) { + ObjectStorageService objectStorageService, + MessageObservationSupport messageObservationSupport) { return new ScanTaskConsumer( redissonClient, streamKey, @@ -60,7 +65,8 @@ public ScanTaskConsumer scanTaskConsumer(RedissonClient redissonClient, reclaimEnabled, reclaimMinIdle, reclaimBatchSize, - reclaimInterval + reclaimInterval, + messageObservationSupport ); } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/RequestIdFilter.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/RequestIdFilter.java index f5b88bc31..d3122e44b 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/RequestIdFilter.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/RequestIdFilter.java @@ -12,7 +12,6 @@ import java.io.IOException; import java.util.UUID; -import java.util.regex.Pattern; /** * Ensures every request has a request identifier for logs, responses, and downstream audit @@ -23,8 +22,6 @@ public class RequestIdFilter extends OncePerRequestFilter { private static final String REQUEST_ID_HEADER = "X-Request-Id"; - private static final Pattern VALID_REQUEST_ID = - Pattern.compile("^[A-Za-z0-9][A-Za-z0-9._:-]{0,63}$"); private final RequestIdAccessor requestIdAccessor; @@ -36,7 +33,7 @@ public RequestIdFilter(RequestIdAccessor requestIdAccessor) { protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { String requestId = request.getHeader(REQUEST_ID_HEADER); - if (requestId == null || !VALID_REQUEST_ID.matcher(requestId).matches()) { + if (!RequestIdAccessor.isValid(requestId)) { requestId = UUID.randomUUID().toString(); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/MessageCarrierAdapter.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/MessageCarrierAdapter.java new file mode 100644 index 000000000..2fb263184 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/MessageCarrierAdapter.java @@ -0,0 +1,15 @@ +package com.iflytek.skillhub.observability; + +import io.micrometer.observation.transport.Propagator; + +/** + * Adapts transport-specific message headers to the common observation boundary. + */ +public interface MessageCarrierAdapter + extends Propagator.Getter, Propagator.Setter { + + /** + * Removes every value associated with a transport header. + */ + void remove(C carrier, String key); +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/MessageObservationSupport.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/MessageObservationSupport.java new file mode 100644 index 000000000..4e84e1bd6 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/MessageObservationSupport.java @@ -0,0 +1,161 @@ +package com.iflytek.skillhub.observability; + +import io.micrometer.observation.Observation; +import io.micrometer.observation.ObservationRegistry; +import io.micrometer.observation.transport.Kind; +import io.micrometer.observation.transport.ReceiverContext; +import io.micrometer.observation.transport.SenderContext; +import org.springframework.stereotype.Component; + +import java.util.Objects; +import java.util.Set; +import java.util.function.Supplier; + +/** + * Propagates tracing and request correlation across asynchronous message transports. + * + *

The message carrier owns transport metadata. Business payloads remain independent from + * Micrometer, OpenTelemetry, MDC, and a concrete tracing backend.

+ */ +@Component +public class MessageObservationSupport { + + public static final String REQUEST_ID_FIELD = "skillhub.request_id"; + + private static final Set OWNED_TRANSPORT_FIELDS = Set.of( + REQUEST_ID_FIELD, + "traceparent", + "tracestate", + "baggage" + ); + + private final ObservationRegistry observationRegistry; + private final RequestIdAccessor requestIdAccessor; + + public MessageObservationSupport( + ObservationRegistry observationRegistry, + RequestIdAccessor requestIdAccessor + ) { + this.observationRegistry = observationRegistry; + this.requestIdAccessor = requestIdAccessor; + } + + /** + * Observes a message publish operation and injects the current transport context. + */ + public T observePublish( + String messagingSystem, + String destination, + C carrier, + MessageCarrierAdapter carrierAdapter, + Supplier action + ) { + validateArguments(messagingSystem, destination, carrier, action); + Objects.requireNonNull(carrierAdapter, "carrierAdapter must not be null"); + OWNED_TRANSPORT_FIELDS.forEach(field -> carrierAdapter.remove(carrier, field)); + String requestId = requestIdAccessor.current(); + if (RequestIdAccessor.isValid(requestId)) { + carrierAdapter.set(carrier, REQUEST_ID_FIELD, requestId); + } + + SenderContext senderContext = new SenderContext<>(carrierAdapter, Kind.PRODUCER); + senderContext.setCarrier(carrier); + senderContext.setRemoteServiceName(messagingSystem); + return observe( + "skillhub.message.publish", + destination + " publish", + messagingSystem, + destination, + "publish", + senderContext, + action + ); + } + + /** + * Extracts a message transport context and observes processing inside its scope. + */ + public T observeProcess( + String messagingSystem, + String destination, + C carrier, + MessageCarrierAdapter carrierAdapter, + Supplier action + ) { + validateArguments(messagingSystem, destination, carrier, action); + Objects.requireNonNull(carrierAdapter, "carrierAdapter must not be null"); + ReceiverContext receiverContext = new ReceiverContext<>(carrierAdapter, Kind.CONSUMER); + receiverContext.setCarrier(carrier); + receiverContext.setRemoteServiceName(messagingSystem); + + String propagatedRequestId = carrierAdapter.get(carrier, REQUEST_ID_FIELD); + RequestIdAccessor.Scope requestIdScope = requestIdAccessor.openNullable( + RequestIdAccessor.isValid(propagatedRequestId) ? propagatedRequestId : null + ); + try (requestIdScope) { + return observe( + "skillhub.message.process", + destination + " process", + messagingSystem, + destination, + "process", + receiverContext, + action + ); + } + } + + /** + * Marks the currently active message Observation as failed when processing handles the + * exception without rethrowing it. + */ + public void recordCurrentError(Throwable error) { + Objects.requireNonNull(error, "error must not be null"); + Observation currentObservation = observationRegistry.getCurrentObservation(); + if (currentObservation != null) { + currentObservation.error(error); + } + } + + private T observe( + String observationName, + String contextualName, + String messagingSystem, + String destination, + String operation, + Observation.Context transportContext, + Supplier action + ) { + Observation observation = Observation + .createNotStarted(observationName, () -> transportContext, observationRegistry) + .contextualName(contextualName) + .lowCardinalityKeyValue("messaging.system", messagingSystem) + .lowCardinalityKeyValue("messaging.operation.type", operation) + .highCardinalityKeyValue("messaging.destination.name", destination) + .start(); + try (Observation.Scope ignored = observation.openScope()) { + return action.get(); + } catch (RuntimeException | Error error) { + observation.error(error); + throw error; + } finally { + observation.stop(); + } + } + + private void validateArguments( + String messagingSystem, + String destination, + Object carrier, + Supplier action + ) { + if (messagingSystem == null || messagingSystem.isBlank()) { + throw new IllegalArgumentException("messagingSystem must not be blank"); + } + if (destination == null || destination.isBlank()) { + throw new IllegalArgumentException("destination must not be blank"); + } + Objects.requireNonNull(carrier, "carrier must not be null"); + Objects.requireNonNull(action, "action must not be null"); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/RequestIdAccessor.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/RequestIdAccessor.java index ecdfb8038..d6589b88c 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/RequestIdAccessor.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/RequestIdAccessor.java @@ -4,6 +4,7 @@ import org.springframework.stereotype.Component; import java.util.Objects; +import java.util.regex.Pattern; /** * Holds the current SkillHub request identifier independently from the logging implementation. @@ -16,6 +17,9 @@ public class RequestIdAccessor { public static final String MDC_KEY = "requestId"; + private static final Pattern VALID_REQUEST_ID = + Pattern.compile("^[A-Za-z0-9][A-Za-z0-9._:-]{0,63}$"); + private final ThreadLocal currentRequestId = new ThreadLocal<>(); /** @@ -25,6 +29,13 @@ public String current() { return currentRequestId.get(); } + /** + * Returns whether a value is safe to use as a request identifier across transport boundaries. + */ + public static boolean isValid(String requestId) { + return requestId != null && VALID_REQUEST_ID.matcher(requestId).matches(); + } + /** * Opens a nested request identifier scope on the current thread. */ @@ -34,6 +45,10 @@ public Scope open(String requestId) { throw new IllegalArgumentException("requestId must not be blank"); } + return openNullable(requestId); + } + + Scope openNullable(String requestId) { String previousRequestId = currentRequestId.get(); replace(requestId); return new Scope(previousRequestId, requestId); diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/AbstractStreamConsumer.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/AbstractStreamConsumer.java index 229af8514..01a73e77d 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/AbstractStreamConsumer.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/AbstractStreamConsumer.java @@ -1,5 +1,6 @@ package com.iflytek.skillhub.stream; +import com.iflytek.skillhub.observability.MessageObservationSupport; import jakarta.annotation.PostConstruct; import jakarta.annotation.PreDestroy; import org.redisson.api.AutoClaimResult; @@ -39,6 +40,7 @@ public abstract class AbstractStreamConsumer { private final Duration reclaimMinIdle; private final int reclaimBatchSize; private final Duration reclaimInterval; + private final MessageObservationSupport messageObservationSupport; private final AtomicBoolean running = new AtomicBoolean(false); private RStream stream; @@ -47,8 +49,18 @@ public abstract class AbstractStreamConsumer { protected AbstractStreamConsumer(RedissonClient redissonClient, String streamKey, - String groupName) { - this(redissonClient, streamKey, groupName, true, Duration.ofMinutes(2), 20, Duration.ofSeconds(30)); + String groupName, + MessageObservationSupport messageObservationSupport) { + this( + redissonClient, + streamKey, + groupName, + true, + Duration.ofMinutes(2), + 20, + Duration.ofSeconds(30), + messageObservationSupport + ); } protected AbstractStreamConsumer(RedissonClient redissonClient, @@ -57,7 +69,8 @@ protected AbstractStreamConsumer(RedissonClient redissonClient, boolean reclaimEnabled, Duration reclaimMinIdle, int reclaimBatchSize, - Duration reclaimInterval) { + Duration reclaimInterval, + MessageObservationSupport messageObservationSupport) { this.redissonClient = redissonClient; this.streamKey = streamKey; this.groupName = groupName; @@ -65,6 +78,7 @@ protected AbstractStreamConsumer(RedissonClient redissonClient, this.reclaimMinIdle = reclaimMinIdle; this.reclaimBatchSize = reclaimBatchSize; this.reclaimInterval = reclaimInterval; + this.messageObservationSupport = messageObservationSupport; this.consumerName = consumerPrefix() + "-" + UUID.randomUUID().toString().substring(0, 8); } @@ -195,6 +209,19 @@ private void processMessages(Map> messages) } void handleMessage(StreamMessageId messageId, Map data) { + messageObservationSupport.observeProcess( + RedisStreamMessageCarrier.MESSAGING_SYSTEM, + streamKey, + data, + RedisStreamMessageCarrier.ADAPTER, + () -> { + handleMessageInScope(messageId, data); + return null; + } + ); + } + + private void handleMessageInScope(StreamMessageId messageId, Map data) { T payload = parsePayload(messageId.toString(), data); if (payload == null) { acknowledge(messageId); @@ -208,6 +235,7 @@ void handleMessage(StreamMessageId messageId, Map data) { markCompleted(payload); acknowledge(messageId); } catch (Exception e) { + messageObservationSupport.recordCurrentError(e); handleFailure(payload, retryCount, e); acknowledge(messageId); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedisStreamMessageCarrier.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedisStreamMessageCarrier.java new file mode 100644 index 000000000..acbb349b1 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedisStreamMessageCarrier.java @@ -0,0 +1,34 @@ +package com.iflytek.skillhub.stream; + +import com.iflytek.skillhub.observability.MessageCarrierAdapter; + +import java.util.Map; + +/** + * Adapts Redis Stream field maps to the transport-neutral message observation boundary. + */ +final class RedisStreamMessageCarrier { + + static final String MESSAGING_SYSTEM = "redis"; + + static final MessageCarrierAdapter> ADAPTER = + new MessageCarrierAdapter<>() { + @Override + public String get(Map carrier, String key) { + return carrier.get(key); + } + + @Override + public void set(Map carrier, String key, String value) { + carrier.put(key, value); + } + + @Override + public void remove(Map carrier, String key) { + carrier.remove(key); + } + }; + + private RedisStreamMessageCarrier() { + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedissonScanTaskProducer.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedissonScanTaskProducer.java index 047215d38..144693f1f 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedissonScanTaskProducer.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedissonScanTaskProducer.java @@ -2,6 +2,7 @@ import com.iflytek.skillhub.domain.security.ScanTask; import com.iflytek.skillhub.domain.security.ScanTaskProducer; +import com.iflytek.skillhub.observability.MessageObservationSupport; import org.redisson.api.RStream; import org.redisson.api.RedissonClient; import org.redisson.api.StreamMessageId; @@ -19,10 +20,16 @@ public class RedissonScanTaskProducer implements ScanTaskProducer { private final RedissonClient redissonClient; private final String streamKey; + private final MessageObservationSupport messageObservationSupport; - public RedissonScanTaskProducer(RedissonClient redissonClient, String streamKey) { + public RedissonScanTaskProducer( + RedissonClient redissonClient, + String streamKey, + MessageObservationSupport messageObservationSupport + ) { this.redissonClient = redissonClient; this.streamKey = streamKey; + this.messageObservationSupport = messageObservationSupport; } @Override @@ -43,7 +50,13 @@ public void publishScanTask(ScanTask task) { } RStream stream = redissonClient.getStream(streamKey, StringCodec.INSTANCE); - StreamMessageId messageId = stream.add(StreamAddArgs.entries(fields)); + StreamMessageId messageId = messageObservationSupport.observePublish( + RedisStreamMessageCarrier.MESSAGING_SYSTEM, + streamKey, + fields, + RedisStreamMessageCarrier.ADAPTER, + () -> stream.add(StreamAddArgs.entries(fields)) + ); log.info("Published scan task: taskId={}, versionId={}, bundleKey={}, hasSkillPath={}, recordId={}", task.taskId(), task.versionId(), task.bundleKey(), task.skillPath() != null && !task.skillPath().isBlank(), messageId); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/ScanTaskConsumer.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/ScanTaskConsumer.java index 19722d872..fce9bcc2e 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/ScanTaskConsumer.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/ScanTaskConsumer.java @@ -9,6 +9,7 @@ import com.iflytek.skillhub.domain.security.SecurityScanner; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.skill.SkillVersionStatus; +import com.iflytek.skillhub.observability.MessageObservationSupport; import com.iflytek.skillhub.storage.ObjectStorageService; import org.redisson.api.RedissonClient; @@ -38,8 +39,9 @@ public ScanTaskConsumer(RedissonClient redissonClient, SecurityScanService securityScanService, SkillVersionRepository skillVersionRepository, ScanTaskProducer scanTaskProducer, - ObjectStorageService objectStorageService) { - super(redissonClient, streamKey, groupName); + ObjectStorageService objectStorageService, + MessageObservationSupport messageObservationSupport) { + super(redissonClient, streamKey, groupName, messageObservationSupport); this.securityScanner = securityScanner; this.securityScanService = securityScanService; this.skillVersionRepository = skillVersionRepository; @@ -58,8 +60,18 @@ public ScanTaskConsumer(RedissonClient redissonClient, boolean reclaimEnabled, Duration reclaimMinIdle, int reclaimBatchSize, - Duration reclaimInterval) { - super(redissonClient, streamKey, groupName, reclaimEnabled, reclaimMinIdle, reclaimBatchSize, reclaimInterval); + Duration reclaimInterval, + MessageObservationSupport messageObservationSupport) { + super( + redissonClient, + streamKey, + groupName, + reclaimEnabled, + reclaimMinIdle, + reclaimBatchSize, + reclaimInterval, + messageObservationSupport + ); this.securityScanner = securityScanner; this.securityScanService = securityScanService; this.skillVersionRepository = skillVersionRepository; diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/package-info.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/package-info.java new file mode 100644 index 000000000..30c4e344e --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/package-info.java @@ -0,0 +1,4 @@ +/** + * Redis Stream transport adapters and background consumers. + */ +package com.iflytek.skillhub.stream; diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/MessageObservationSupportTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/MessageObservationSupportTest.java new file mode 100644 index 000000000..02f114df8 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/MessageObservationSupportTest.java @@ -0,0 +1,217 @@ +package com.iflytek.skillhub.observability; + +import com.iflytek.skillhub.observability.tracing.SkillHubTracingConfiguration; +import io.micrometer.observation.Observation; +import io.micrometer.observation.ObservationRegistry; +import io.micrometer.tracing.Span; +import io.micrometer.tracing.Tracer; +import org.junit.jupiter.api.Test; +import org.slf4j.MDC; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; + +class MessageObservationSupportTest { + + private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() + .withUserConfiguration(TestApplication.class) + .withPropertyValues( + "spring.flyway.enabled=false", + "spring.jpa.hibernate.ddl-auto=none" + ); + + @Test + void shouldPropagateTraceAndRequestIdAcrossMessageBoundaryWithoutLeakingWorkerContext() { + contextRunner + .withPropertyValues( + "skillhub.observability.tracing-mode=otel-sdk", + "management.tracing.sampling.probability=1.0" + ) + .run(context -> { + ObservationRegistry observationRegistry = context.getBean(ObservationRegistry.class); + RequestIdAccessor requestIdAccessor = context.getBean(RequestIdAccessor.class); + Tracer tracer = context.getBean(Tracer.class); + MessageObservationSupport support = new MessageObservationSupport( + observationRegistry, + requestIdAccessor + ); + TestCarrier carrier = new TestCarrier(); + Observation parent = Observation.start("publish-request", observationRegistry); + String parentTraceId; + String producerSpanId; + + try (RequestIdAccessor.Scope ignored = requestIdAccessor.open("request-async-1"); + Observation.Scope observationScope = parent.openScope()) { + parentTraceId = tracer.currentSpan().context().traceId(); + producerSpanId = support.observePublish( + "redis", + "skillhub:scan:requests", + carrier, + TEST_CARRIER_ADAPTER, + () -> tracer.currentSpan().context().spanId() + ); + } finally { + parent.stop(); + } + + assertThat(carrier.get(MessageObservationSupport.REQUEST_ID_FIELD)) + .isEqualTo("request-async-1"); + assertThat(carrier.get("traceparent")) + .matches("^00-" + parentTraceId + "-[0-9a-f]{16}-0[01]$"); + + ExecutorService worker = Executors.newSingleThreadExecutor(); + try { + ContextValues consumed = worker.submit(() -> support.observeProcess( + "redis", + "skillhub:scan:requests", + carrier, + TEST_CARRIER_ADAPTER, + () -> currentValues(requestIdAccessor, tracer) + )).get(5, TimeUnit.SECONDS); + + assertThat(consumed.requestId()).isEqualTo("request-async-1"); + assertThat(consumed.mdcRequestId()).isEqualTo("request-async-1"); + assertThat(consumed.traceId()).isEqualTo(parentTraceId); + assertThat(consumed.spanId()).isNotEqualTo(producerSpanId); + + ContextValues clean = worker.submit( + () -> currentValues(requestIdAccessor, tracer) + ).get(5, TimeUnit.SECONDS); + assertThat(clean.requestId()).isNull(); + assertThat(clean.mdcRequestId()).isNull(); + assertThat(clean.traceId()).isNull(); + assertThat(clean.spanId()).isNull(); + } finally { + worker.shutdownNow(); + MDC.clear(); + } + }); + } + + @Test + void shouldClearMissingOrInvalidMessageContextAndRestoreOuterScope() { + RequestIdAccessor requestIdAccessor = new RequestIdAccessor(); + MessageObservationSupport support = new MessageObservationSupport( + ObservationRegistry.NOOP, + requestIdAccessor + ); + TestCarrier carrier = new TestCarrier(); + carrier.set(MessageObservationSupport.REQUEST_ID_FIELD, "invalid request id"); + + try (RequestIdAccessor.Scope ignored = requestIdAccessor.open("outer-request")) { + String valueInsideMessage = support.observeProcess( + "test-broker", + "jobs", + carrier, + TEST_CARRIER_ADAPTER, + requestIdAccessor::current + ); + + assertThat(valueInsideMessage).isNull(); + assertThat(requestIdAccessor.current()).isEqualTo("outer-request"); + } finally { + MDC.clear(); + } + } + + @Test + void shouldRemoveCallerSuppliedTransportContextBeforePublishingWithoutTracer() { + RequestIdAccessor requestIdAccessor = new RequestIdAccessor(); + MessageObservationSupport support = new MessageObservationSupport( + ObservationRegistry.NOOP, + requestIdAccessor + ); + TestCarrier carrier = new TestCarrier(); + carrier.set("traceparent", "caller-controlled"); + carrier.set(MessageObservationSupport.REQUEST_ID_FIELD, "caller-controlled"); + + try (RequestIdAccessor.Scope ignored = requestIdAccessor.open("trusted-request")) { + support.observePublish( + "test-broker", + "jobs", + carrier, + TEST_CARRIER_ADAPTER, + () -> null + ); + } finally { + MDC.clear(); + } + + assertThat(carrier.get("traceparent")).isNull(); + assertThat(carrier.get(MessageObservationSupport.REQUEST_ID_FIELD)) + .isEqualTo("trusted-request"); + } + + private ContextValues currentValues(RequestIdAccessor requestIdAccessor, Tracer tracer) { + Span currentSpan = tracer.currentSpan(); + return new ContextValues( + requestIdAccessor.current(), + MDC.get(RequestIdAccessor.MDC_KEY), + currentSpan == null ? null : currentSpan.context().traceId(), + currentSpan == null ? null : currentSpan.context().spanId() + ); + } + + private record ContextValues( + String requestId, + String mdcRequestId, + String traceId, + String spanId + ) { + } + + private static final MessageCarrierAdapter TEST_CARRIER_ADAPTER = + new MessageCarrierAdapter<>() { + @Override + public String get(TestCarrier carrier, String key) { + return carrier.get(key); + } + + @Override + public void set(TestCarrier carrier, String key, String value) { + carrier.set(key, value); + } + + @Override + public void remove(TestCarrier carrier, String key) { + carrier.set(key, null); + } + }; + + private static final class TestCarrier { + private final List
headers = new ArrayList<>(); + + private void set(String key, String value) { + headers.removeIf(header -> header.key().equals(key)); + if (value != null) { + headers.add(new Header(key, value)); + } + } + + private String get(String key) { + return headers.stream() + .filter(header -> header.key().equals(key)) + .map(Header::value) + .findFirst() + .orElse(null); + } + } + + private record Header(String key, String value) { + } + + @Configuration(proxyBeanMethods = false) + @EnableAutoConfiguration + @Import({SkillHubTracingConfiguration.class, RequestIdAccessor.class}) + static class TestApplication { + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/AbstractStreamConsumerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/AbstractStreamConsumerTest.java index 6a5436f05..bbb58a709 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/AbstractStreamConsumerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/AbstractStreamConsumerTest.java @@ -1,5 +1,8 @@ package com.iflytek.skillhub.stream; +import com.iflytek.skillhub.observability.MessageObservationSupport; +import com.iflytek.skillhub.observability.RequestIdAccessor; +import io.micrometer.observation.ObservationRegistry; import io.lettuce.core.RedisBusyException; import org.junit.jupiter.api.Test; import org.redisson.api.AutoClaimResult; @@ -104,6 +107,25 @@ void handleMessage_reusesStreamInstanceForAcknowledgement() { assertThat(consumer.streamCreationCount.get()).isEqualTo(1); } + @Test + void handleMessage_restoresPropagatedRequestIdOnlyWhileProcessing() { + @SuppressWarnings("unchecked") + RStream stream = mock(RStream.class); + RequestIdAccessor requestIdAccessor = new RequestIdAccessor(); + TestConsumer consumer = new TestConsumer(stream, requestIdAccessor); + + consumer.handleMessage( + new StreamMessageId(8, 0), + Map.of( + "payload", "correlated", + MessageObservationSupport.REQUEST_ID_FIELD, "request-stream-1" + ) + ); + + assertThat(consumer.processedRequestId).isEqualTo("request-stream-1"); + assertThat(requestIdAccessor.current()).isNull(); + } + @Test void detectsBusyGroupWhenWrappedInRedisSystemException() { RedisSystemException wrapped = new RedisSystemException( @@ -116,11 +138,27 @@ void detectsBusyGroupWhenWrappedInRedisSystemException() { private static class TestConsumer extends AbstractStreamConsumer { private final RStream stream; + private final RequestIdAccessor requestIdAccessor; private boolean fail; + private String processedRequestId; private TestConsumer(RStream stream) { - super(mock(RedissonClient.class), "scan-stream", "scan-group", true, Duration.ofMinutes(2), 20, Duration.ofSeconds(30)); + this(stream, new RequestIdAccessor()); + } + + private TestConsumer(RStream stream, RequestIdAccessor requestIdAccessor) { + super( + mock(RedissonClient.class), + "scan-stream", + "scan-group", + true, + Duration.ofMinutes(2), + 20, + Duration.ofSeconds(30), + new MessageObservationSupport(ObservationRegistry.NOOP, requestIdAccessor) + ); this.stream = stream; + this.requestIdAccessor = requestIdAccessor; } @Override @@ -154,6 +192,7 @@ protected void markProcessing(String payload) { @Override protected void processBusiness(String payload) { + processedRequestId = requestIdAccessor.current(); if (fail) { throw new IllegalStateException("boom"); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/RedissonScanTaskProducerLoggingTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/RedissonScanTaskProducerLoggingTest.java index 542908dfb..a184aa30c 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/RedissonScanTaskProducerLoggingTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/RedissonScanTaskProducerLoggingTest.java @@ -5,6 +5,9 @@ import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.read.ListAppender; import com.iflytek.skillhub.domain.security.ScanTask; +import com.iflytek.skillhub.observability.MessageObservationSupport; +import com.iflytek.skillhub.observability.RequestIdAccessor; +import io.micrometer.observation.ObservationRegistry; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.redisson.api.RStream; @@ -43,7 +46,11 @@ void publishScanTask_logsBundleKeyPresence() { RedissonClient redissonClient = mock(RedissonClient.class); doReturn(typedStream).when(redissonClient).getStream("skillhub:scan:requests", StringCodec.INSTANCE); when(stream.add(any())).thenReturn(new StreamMessageId(1, 0)); - RedissonScanTaskProducer producer = new RedissonScanTaskProducer(redissonClient, "skillhub:scan:requests"); + RedissonScanTaskProducer producer = new RedissonScanTaskProducer( + redissonClient, + "skillhub:scan:requests", + new MessageObservationSupport(ObservationRegistry.NOOP, new RequestIdAccessor()) + ); attachAppender(); producer.publishScanTask(new ScanTask( diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/RedissonScanTaskProducerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/RedissonScanTaskProducerTest.java index 003c1158f..ccf22ea3a 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/RedissonScanTaskProducerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/RedissonScanTaskProducerTest.java @@ -1,12 +1,16 @@ package com.iflytek.skillhub.stream; import com.iflytek.skillhub.domain.security.ScanTask; +import com.iflytek.skillhub.observability.MessageObservationSupport; +import com.iflytek.skillhub.observability.RequestIdAccessor; +import io.micrometer.observation.ObservationRegistry; import org.junit.jupiter.api.Test; import org.redisson.api.RStream; import org.redisson.api.RedissonClient; import org.redisson.api.StreamMessageId; import org.redisson.client.codec.StringCodec; import org.redisson.api.stream.StreamAddArgs; +import org.redisson.api.stream.StreamAddParams; import org.mockito.ArgumentCaptor; import java.util.Map; @@ -29,21 +33,32 @@ void publishScanTask_writesExpectedFieldsToConfiguredStream() { RedissonClient redissonClient = mock(RedissonClient.class); doReturn(typedStream).when(redissonClient).getStream("skillhub:scan:requests", StringCodec.INSTANCE); when(stream.add(any())).thenReturn(new StreamMessageId(1, 0)); - RedissonScanTaskProducer producer = new RedissonScanTaskProducer(redissonClient, "skillhub:scan:requests"); + RequestIdAccessor requestIdAccessor = new RequestIdAccessor(); + RedissonScanTaskProducer producer = new RedissonScanTaskProducer( + redissonClient, + "skillhub:scan:requests", + new MessageObservationSupport(ObservationRegistry.NOOP, requestIdAccessor) + ); - producer.publishScanTask(new ScanTask( - "task-1", - 42L, - "/tmp/skill", - null, - "publisher-1", - 1711260000000L, - Map.of("scannerType", "skill-scanner") - )); + try (RequestIdAccessor.Scope ignored = requestIdAccessor.open("request-stream-1")) { + producer.publishScanTask(new ScanTask( + "task-1", + 42L, + "/tmp/skill", + null, + "publisher-1", + 1711260000000L, + Map.of("scannerType", "skill-scanner") + )); + } verify(redissonClient).getStream("skillhub:scan:requests", StringCodec.INSTANCE); ArgumentCaptor> argsCaptor = ArgumentCaptor.forClass(StreamAddArgs.class); verify(stream).add(argsCaptor.capture()); - assertThat(argsCaptor.getValue()).isNotNull(); + assertThat(argsCaptor.getValue()).isInstanceOf(StreamAddParams.class); + @SuppressWarnings("unchecked") + StreamAddParams params = (StreamAddParams) argsCaptor.getValue(); + assertThat(params.getEntries()) + .containsEntry(MessageObservationSupport.REQUEST_ID_FIELD, "request-stream-1"); } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java index 293a3dd7d..8f473b656 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java @@ -13,8 +13,11 @@ import com.iflytek.skillhub.domain.skill.SkillVersion; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.skill.SkillVersionStatus; +import com.iflytek.skillhub.observability.MessageObservationSupport; +import com.iflytek.skillhub.observability.RequestIdAccessor; import com.iflytek.skillhub.storage.ObjectMetadata; import com.iflytek.skillhub.storage.ObjectStorageService; +import io.micrometer.observation.ObservationRegistry; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.redisson.api.RStream; @@ -163,7 +166,8 @@ private TestableLoggingConsumer(SecurityScanner securityScanner, securityScanService, skillVersionRepository, scanTaskProducer, - objectStorageService + objectStorageService, + new MessageObservationSupport(ObservationRegistry.NOOP, new RequestIdAccessor()) ); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerPathSafetyTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerPathSafetyTest.java index 984acbd24..914ffd2fe 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerPathSafetyTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerPathSafetyTest.java @@ -6,7 +6,10 @@ import com.iflytek.skillhub.domain.security.SecurityScanService; import com.iflytek.skillhub.domain.security.SecurityScanner; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.observability.MessageObservationSupport; +import com.iflytek.skillhub.observability.RequestIdAccessor; import com.iflytek.skillhub.storage.ObjectStorageService; +import io.micrometer.observation.ObservationRegistry; import java.lang.reflect.Method; import java.nio.file.Files; import java.nio.file.Path; @@ -25,7 +28,8 @@ void cleanupTempPath_ignoresPathsOutsideScanTempDirectory() throws Exception { org.mockito.Mockito.mock(SecurityScanService.class), org.mockito.Mockito.mock(SkillVersionRepository.class), org.mockito.Mockito.mock(ScanTaskProducer.class), - org.mockito.Mockito.mock(ObjectStorageService.class) + org.mockito.Mockito.mock(ObjectStorageService.class), + new MessageObservationSupport(ObservationRegistry.NOOP, new RequestIdAccessor()) ); Path outsideFile = Files.createTempFile("scan-cleanup-", ".txt"); Files.writeString(outsideFile, "keep"); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerTest.java index c8c55f0f7..7f8cabaaa 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerTest.java @@ -14,8 +14,11 @@ import com.iflytek.skillhub.domain.skill.SkillVersion; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.skill.SkillVersionStatus; +import com.iflytek.skillhub.observability.MessageObservationSupport; +import com.iflytek.skillhub.observability.RequestIdAccessor; import com.iflytek.skillhub.storage.ObjectStorageService; import com.iflytek.skillhub.storage.ObjectMetadata; +import io.micrometer.observation.ObservationRegistry; import org.junit.jupiter.api.Test; import org.redisson.api.RStream; import org.redisson.api.RedissonClient; @@ -303,7 +306,8 @@ private TestableScanTaskConsumer(SecurityScanner securityScanner, securityScanService, skillVersionRepository, scanTaskProducer, - objectStorageService + objectStorageService, + new MessageObservationSupport(ObservationRegistry.NOOP, new RequestIdAccessor()) ); this.stream = mock(RStream.class); } From 801818bba2a7376327bd0c2bc79e1088deed5ee0 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Mon, 3 Aug 2026 15:09:55 +0800 Subject: [PATCH 08/10] fix(observability): document message propagation semantics Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../observability/MessageCarrierAdapter.java | 6 +- .../MessageObservationSupport.java | 27 +++++-- .../stream/AbstractStreamConsumer.java | 3 + .../stream/RedisStreamMessageCarrier.java | 4 ++ .../stream/RedissonScanTaskProducer.java | 1 + .../MessageObservationSupportTest.java | 70 +++++++++++++++++++ 6 files changed, 105 insertions(+), 6 deletions(-) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/MessageCarrierAdapter.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/MessageCarrierAdapter.java index 2fb263184..721d4167a 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/MessageCarrierAdapter.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/MessageCarrierAdapter.java @@ -4,12 +4,16 @@ /** * Adapts transport-specific message headers to the common observation boundary. + * + *

Implementations should operate on a transport envelope or header collection, never on a + * business DTO. Micrometer uses the inherited getter and setter to extract and inject propagation + * fields without exposing a concrete tracing implementation to the transport.

*/ public interface MessageCarrierAdapter extends Propagator.Getter, Propagator.Setter { /** - * Removes every value associated with a transport header. + * Removes every value associated with a transport header before trusted context is injected. */ void remove(C carrier, String key); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/MessageObservationSupport.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/MessageObservationSupport.java index 4e84e1bd6..48f25995e 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/MessageObservationSupport.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/MessageObservationSupport.java @@ -15,13 +15,20 @@ * Propagates tracing and request correlation across asynchronous message transports. * *

The message carrier owns transport metadata. Business payloads remain independent from - * Micrometer, OpenTelemetry, MDC, and a concrete tracing backend.

+ * Micrometer, OpenTelemetry, MDC, and a concrete tracing backend. A transport integrates by + * providing a {@link MessageCarrierAdapter}, then wrapping the actual send and per-message + * processing operations with this component.

*/ @Component public class MessageObservationSupport { + /** + * Reserved transport field for log correlation when distributed tracing is disabled. + */ public static final String REQUEST_ID_FIELD = "skillhub.request_id"; + // The propagation boundary owns these fields. Removing existing values prevents business + // metadata from forging a parent trace or leaking stale context into a newly published message. private static final Set OWNED_TRANSPORT_FIELDS = Set.of( REQUEST_ID_FIELD, "traceparent", @@ -53,6 +60,9 @@ public T observePublish( validateArguments(messagingSystem, destination, carrier, action); Objects.requireNonNull(carrierAdapter, "carrierAdapter must not be null"); OWNED_TRANSPORT_FIELDS.forEach(field -> carrierAdapter.remove(carrier, field)); + + // Request ID is propagated independently because it must remain useful in modes where no + // tracing handler is registered. Micrometer injects W3C trace fields when tracing is active. String requestId = requestIdAccessor.current(); if (RequestIdAccessor.isValid(requestId)) { carrierAdapter.set(carrier, REQUEST_ID_FIELD, requestId); @@ -63,10 +73,11 @@ public T observePublish( senderContext.setRemoteServiceName(messagingSystem); return observe( "skillhub.message.publish", - destination + " publish", + "publish " + destination, messagingSystem, destination, "publish", + "send", senderContext, action ); @@ -88,6 +99,9 @@ public T observeProcess( receiverContext.setCarrier(carrier); receiverContext.setRemoteServiceName(messagingSystem); + // Micrometer scopes the extracted trace context when the Observation starts. Scope the + // independently propagated Request ID over the same processing boundary and restore both + // before the worker thread is reused. String propagatedRequestId = carrierAdapter.get(carrier, REQUEST_ID_FIELD); RequestIdAccessor.Scope requestIdScope = requestIdAccessor.openNullable( RequestIdAccessor.isValid(propagatedRequestId) ? propagatedRequestId : null @@ -95,10 +109,11 @@ public T observeProcess( try (requestIdScope) { return observe( "skillhub.message.process", - destination + " process", + "process " + destination, messagingSystem, destination, "process", + "process", receiverContext, action ); @@ -122,7 +137,8 @@ private T observe( String contextualName, String messagingSystem, String destination, - String operation, + String operationName, + String operationType, Observation.Context transportContext, Supplier action ) { @@ -130,7 +146,8 @@ private T observe( .createNotStarted(observationName, () -> transportContext, observationRegistry) .contextualName(contextualName) .lowCardinalityKeyValue("messaging.system", messagingSystem) - .lowCardinalityKeyValue("messaging.operation.type", operation) + .lowCardinalityKeyValue("messaging.operation.name", operationName) + .lowCardinalityKeyValue("messaging.operation.type", operationType) .highCardinalityKeyValue("messaging.destination.name", destination) .start(); try (Observation.Scope ignored = observation.openScope()) { diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/AbstractStreamConsumer.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/AbstractStreamConsumer.java index 01a73e77d..67ab8fef5 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/AbstractStreamConsumer.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/AbstractStreamConsumer.java @@ -205,6 +205,7 @@ private void processMessages(Map> messages) if (messages == null || messages.isEmpty()) { return; } + // Scope each entry independently because one XREADGROUP batch may contain unrelated traces. messages.forEach(this::handleMessage); } @@ -243,6 +244,8 @@ private void handleMessageInScope(StreamMessageId messageId, Map private void handleFailure(T payload, int retryCount, Exception e) { if (retryCount < MAX_RETRY_COUNT) { + // Retry publication remains inside the current consumer scope, so the new producer + // span and message carrier continue the original trace. retryMessage(payload, retryCount + 1); return; } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedisStreamMessageCarrier.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedisStreamMessageCarrier.java index acbb349b1..0752d1fcb 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedisStreamMessageCarrier.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedisStreamMessageCarrier.java @@ -6,6 +6,10 @@ /** * Adapts Redis Stream field maps to the transport-neutral message observation boundary. + * + *

Redis Stream entries do not have a separate header collection, so reserved propagation fields + * share the entry map with business fields. {@code MessageObservationSupport} owns and sanitizes + * those reserved fields; business payload types remain unaware of them.

*/ final class RedisStreamMessageCarrier { diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedissonScanTaskProducer.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedissonScanTaskProducer.java index 144693f1f..691669c55 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedissonScanTaskProducer.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/stream/RedissonScanTaskProducer.java @@ -50,6 +50,7 @@ public void publishScanTask(ScanTask task) { } RStream stream = redissonClient.getStream(streamKey, StringCodec.INSTANCE); + // Starting the producer Observation injects trusted propagation fields before XADD. StreamMessageId messageId = messageObservationSupport.observePublish( RedisStreamMessageCarrier.MESSAGING_SYSTEM, streamKey, diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/MessageObservationSupportTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/MessageObservationSupportTest.java index 02f114df8..cc1b585d7 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/MessageObservationSupportTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/MessageObservationSupportTest.java @@ -1,7 +1,9 @@ package com.iflytek.skillhub.observability; import com.iflytek.skillhub.observability.tracing.SkillHubTracingConfiguration; +import io.micrometer.common.KeyValue; import io.micrometer.observation.Observation; +import io.micrometer.observation.ObservationHandler; import io.micrometer.observation.ObservationRegistry; import io.micrometer.tracing.Span; import io.micrometer.tracing.Tracer; @@ -151,6 +153,74 @@ void shouldRemoveCallerSuppliedTransportContextBeforePublishingWithoutTracer() { .isEqualTo("trusted-request"); } + @Test + void shouldUseOpenTelemetryMessagingOperationSemantics() { + ObservationRegistry observationRegistry = ObservationRegistry.create(); + List stoppedContexts = new ArrayList<>(); + observationRegistry.observationConfig().observationHandler( + new ObservationHandler() { + @Override + public void onStop(Observation.Context context) { + stoppedContexts.add(context); + } + + @Override + public boolean supportsContext(Observation.Context context) { + return true; + } + } + ); + MessageObservationSupport support = new MessageObservationSupport( + observationRegistry, + new RequestIdAccessor() + ); + TestCarrier carrier = new TestCarrier(); + + support.observePublish( + "redis", + "skillhub:scan:requests", + carrier, + TEST_CARRIER_ADAPTER, + () -> null + ); + support.observeProcess( + "redis", + "skillhub:scan:requests", + carrier, + TEST_CARRIER_ADAPTER, + () -> null + ); + + Observation.Context publish = findContext(stoppedContexts, "skillhub.message.publish"); + assertThat(publish.getContextualName()).isEqualTo("publish skillhub:scan:requests"); + assertThat(lowCardinalityValue(publish, "messaging.operation.name")).isEqualTo("publish"); + assertThat(lowCardinalityValue(publish, "messaging.operation.type")).isEqualTo("send"); + + Observation.Context process = findContext(stoppedContexts, "skillhub.message.process"); + assertThat(process.getContextualName()).isEqualTo("process skillhub:scan:requests"); + assertThat(lowCardinalityValue(process, "messaging.operation.name")).isEqualTo("process"); + assertThat(lowCardinalityValue(process, "messaging.operation.type")).isEqualTo("process"); + } + + private Observation.Context findContext( + List contexts, + String observationName + ) { + return contexts.stream() + .filter(context -> observationName.equals(context.getName())) + .findFirst() + .orElseThrow(); + } + + private String lowCardinalityValue(Observation.Context context, String key) { + for (KeyValue keyValue : context.getLowCardinalityKeyValues()) { + if (key.equals(keyValue.getKey())) { + return keyValue.getValue(); + } + } + return null; + } + private ContextValues currentValues(RequestIdAccessor requestIdAccessor, Tracer tracer) { Span currentSpan = tracer.currentSpan(); return new ContextValues( From 41e0776b4c2815b832d475440d31a9c99c1b6e69 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:36:52 +0800 Subject: [PATCH 09/10] test(auth): isolate security context between tests Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../skillhub/auth/oauth/OAuth2LoginHandlersTest.java | 7 +++++++ .../auth/token/ApiTokenAuthenticationFilterTest.java | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginHandlersTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginHandlersTest.java index 52c0077bd..750d29d33 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginHandlersTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2LoginHandlersTest.java @@ -1,12 +1,14 @@ package com.iflytek.skillhub.auth.oauth; import jakarta.servlet.http.HttpSession; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.user.DefaultOAuth2User; @@ -22,6 +24,11 @@ class OAuth2LoginHandlersTest { + @AfterEach + void clearSecurityContext() { + SecurityContextHolder.clearContext(); + } + @Test void successHandler_redirectsToStoredReturnTo() throws Exception { OAuthLoginFlowService oauthLoginFlowService = mock(OAuthLoginFlowService.class); diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilterTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilterTest.java index d9f030a9d..fa9fb2cf9 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilterTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenAuthenticationFilterTest.java @@ -10,6 +10,7 @@ import com.iflytek.skillhub.domain.user.UserAccountRepository; import com.iflytek.skillhub.domain.user.UserStatus; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.mock.web.MockFilterChain; import org.springframework.mock.web.MockHttpServletRequest; @@ -43,6 +44,11 @@ class ApiTokenAuthenticationFilterTest { scopeService ); + @BeforeEach + void initializeSecurityContext() { + SecurityContextHolder.clearContext(); + } + @AfterEach void clearSecurityContext() { SecurityContextHolder.clearContext(); From 9290fe3ca222964335f94949506a40849fffc5aa Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Mon, 3 Aug 2026 19:25:19 +0800 Subject: [PATCH 10/10] fix(observability): skip otlp exporter without endpoint Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- ...cingModeAutoConfigurationImportFilter.java | 17 ++++++++++--- .../SkillHubTracingConfigurationTest.java | 18 ++++++++++++++ ...ModeAutoConfigurationImportFilterTest.java | 24 +++++++++++++++++++ 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/TracingModeAutoConfigurationImportFilter.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/TracingModeAutoConfigurationImportFilter.java index 55a33fda6..d2aa538a8 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/TracingModeAutoConfigurationImportFilter.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/observability/tracing/TracingModeAutoConfigurationImportFilter.java @@ -19,7 +19,10 @@ public final class TracingModeAutoConfigurationImportFilter private static final Set OTEL_AUTO_CONFIGURATIONS = Set.of( "org.springframework.boot.actuate.autoconfigure.opentelemetry.OpenTelemetryAutoConfiguration", - "org.springframework.boot.actuate.autoconfigure.tracing.OpenTelemetryAutoConfiguration", + "org.springframework.boot.actuate.autoconfigure.tracing.OpenTelemetryAutoConfiguration" + ); + + private static final Set OTLP_EXPORT_AUTO_CONFIGURATIONS = Set.of( "org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpAutoConfiguration" ); @@ -34,12 +37,16 @@ public boolean[] match( && "otel-sdk".equalsIgnoreCase( environment.getProperty(TRACING_MODE_PROPERTY, "none") ); + boolean otlpExportEnabled = otelSdkEnabled + && hasText(environment.getProperty("management.otlp.tracing.endpoint")); boolean[] matches = new boolean[autoConfigurationClasses.length]; for (int index = 0; index < autoConfigurationClasses.length; index++) { String autoConfigurationClass = autoConfigurationClasses[index]; matches[index] = autoConfigurationClass != null - && (otelSdkEnabled - || !OTEL_AUTO_CONFIGURATIONS.contains(autoConfigurationClass)); + && ((otelSdkEnabled + || !OTEL_AUTO_CONFIGURATIONS.contains(autoConfigurationClass)) + && (otlpExportEnabled + || !OTLP_EXPORT_AUTO_CONFIGURATIONS.contains(autoConfigurationClass))); } return matches; } @@ -48,4 +55,8 @@ public boolean[] match( public void setEnvironment(Environment environment) { this.environment = environment; } + + private static boolean hasText(String value) { + return value != null && !value.isBlank(); + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/SkillHubTracingConfigurationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/SkillHubTracingConfigurationTest.java index 1c5913ce5..99e72a0e8 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/SkillHubTracingConfigurationTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/SkillHubTracingConfigurationTest.java @@ -63,6 +63,24 @@ void otelSdkModeWithoutEndpointShouldCreateInProcessTracerOnly() { }); } + @Test + void otelSdkModeWithEmptyEndpointShouldCreateInProcessTracerOnly() { + contextRunner + .withPropertyValues( + "skillhub.observability.tracing-mode=otel-sdk", + "management.otlp.tracing.endpoint=", + "management.tracing.sampling.probability=1.0", + "management.tracing.baggage.enabled=false", + "management.tracing.propagation.type=W3C" + ) + .run(context -> { + assertThat(context).hasNotFailed(); + assertThat(context.getBean(Tracer.class)).isInstanceOf(OtelTracer.class); + assertThat(context).hasSingleBean(OpenTelemetry.class); + assertThat(context).doesNotHaveBean(OtlpHttpSpanExporter.class); + }); + } + @Test void otelSdkModeShouldCreateExporterOnlyWhenEndpointIsConfigured() { contextRunner diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/TracingModeAutoConfigurationImportFilterTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/TracingModeAutoConfigurationImportFilterTest.java index 6cfbb7d32..e465010ae 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/TracingModeAutoConfigurationImportFilterTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/observability/tracing/TracingModeAutoConfigurationImportFilterTest.java @@ -47,9 +47,33 @@ void shouldEnableApplicationOtelOnlyForOtelSdkMode() { "otel-sdk" )); + assertThat(matches()).containsExactly(true, true, false, true, false); + } + + @Test + void shouldEnableOtlpExporterOnlyWhenEndpointHasText() { + filter.setEnvironment(new MockEnvironment() + .withProperty( + TracingModeAutoConfigurationImportFilter.TRACING_MODE_PROPERTY, + "otel-sdk" + ) + .withProperty("management.otlp.tracing.endpoint", "http://127.0.0.1:4318/v1/traces")); + assertThat(matches()).containsExactly(true, true, true, true, false); } + @Test + void shouldExcludeOtlpExporterWhenEndpointIsEmpty() { + filter.setEnvironment(new MockEnvironment() + .withProperty( + TracingModeAutoConfigurationImportFilter.TRACING_MODE_PROPERTY, + "otel-sdk" + ) + .withProperty("management.otlp.tracing.endpoint", "")); + + assertThat(matches()).containsExactly(true, true, false, true, false); + } + private boolean[] matches() { return filter.match( new String[]{