Repository navigation
Add coverage for Agent Gateway tracing SDK lifecycle across reloads - #7864
Conversation
Aias00
left a comment
There was a problem hiding this comment.
Incremental review and validation of the tracing-lifecycle test: AgentMcpTracingLifecycleTest passed its enabled -> disabled -> enabled context sequence. It checks that the application-owned SDK survives plugin shutdown and exports one logical MCP span despite duplicate finish callbacks. No new blocking issue found in that incremental test.
Validation used head b3f34c8 plus a clean, uncommitted merge of master aab453f to resolve the inherited AI PII parent-version mismatch; no PR source was manually patched.
This remains a stacked Draft depending on #7860. Its current diff still includes edits to released 2.7.1 -> 2.7.2 migrations and the old standalone order script. Please incorporate #7860's newer f9eb7ba/a34127bc8 migration fix and rebuild the branch so this PR contains only its tracing-lifecycle delta. This comment is validation of that delta, not approval of the inherited migration changes.
Validation used isolated worktrees, the Maven wrapper, JDK 21 with the project compiler settings, and targeted tests. Checkstyle passed; RAT and JaCoCo were skipped. This does not claim a full reactor, e2e, or live deployment test run.
…to test/agent-gateway-p004
…p004 # Conflicts: # shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-proxy/src/test/java/org/apache/shenyu/plugin/ai/proxy/enhanced/AiProxyPluginTest.java
|
Thank you for the detailed review. I have rebuilt the branch with normal merges and pushed
Local verification:
I have not rerun the full reactor, e2e, RISC-V, RAT, JaCoCo, or remote CI locally. The branch remains a stacked draft depending on #7860. |
…to test/agent-gateway-p004
There was a problem hiding this comment.
Reviewed affc669.
The branch now incorporates the corrected migrations and current-master compatibility baseline. The tracing-lifecycle regression verifies that the application-owned SDK survives plugin shutdown/re-enable and duplicate terminal callbacks do not export duplicate logical spans. The related context, observation, migration and starter regressions pass; no blocking issue found in the reviewed incremental test change.
Coordinate the merge order with the request-context prerequisite.
Targeted test report: 368 cases, 0 failures, 0 errors, 0 skipped.
Validation used ./mvnw, Amazon Corretto 21 and project target Java 17. Targeted Checkstyle, separately executed Apache RAT and git diff --check pass. No full-repository or Docker E2E pass is claimed; required CI must succeed before merge.
CI follow-up: current-head pr_build failed RemoteResourceNetworkTest.cancellingAnActualHttpReadDoesNotCancelItsPeer (pending expected 0, observed 1): https://github.com/apache/shenyu/actions/runs/40711198680/job/122921196355 . An isolated local rerun of all four RemoteResourceNetworkTest cases passes. The resource-read cancellation cleanup is unchanged by this PR; the client delta concerns trace headers and shared-handshake context. This local result does not clear the CI failure: rerun or investigate the required job before merging.
Aias00
left a comment
There was a problem hiding this comment.
Re-reviewed after integration with master. The PR diff is now exactly the additional 80-line tracing lifecycle test; its production changes are already present through #7860. AgentMcpTracingLifecycleTest passes on the updated starter reactor. Checkstyle and git diff --check pass. Required GitHub checks must pass on this new head before merge.
Depends on #7860
Background
#7860 introduces gateway-owned MCP tracing with an application-provided OpenTelemetry SDK. Existing tests cover tracing behavior, but not the lifecycle when the plugin context is closed and recreated while the application SDK remains active. A regression here could shut down the application-owned SDK or duplicate span exports across reloads.
This draft is stacked on #7860 and therefore transitively includes the prerequisite #7827. It should not be merged independently before #7860.
Changes
AgentMcpTracingLifecycleTestto exercise enabled, disabled, and re-enabled plugin contexts under one application-ownedOpenTelemetrySdk.SdkTracerProvider, and that one terminal finish exports exactly one logical MCP span.Verification
clean verifyfor the Agent Gateway starter and upstream modules: 27 modules, 312 suites, 2,161 tests, 0 failures, 0 errors, 1 existing skipped test; Checkstyle and RAT passed.sh ./mvnw clean verify: 27 modules, 312 suites, 2,161 tests, 0 failures, 0 errors, 1 existing skipped test; container exit 0 and not OOM-killed.git diff --check: passed.