Skip to content

Add coverage for Agent Gateway tracing SDK lifecycle across reloads - #7864

Merged
Aias00 merged 22 commits into
apache:masterfrom
BobSong-dev:test/agent-gateway-p004
Oct 30, 2026
Merged

Aias00 merged 22 commits into
apache:masterfrom
BobSong-dev:test/agent-gateway-p004

Conversation

@BobSong-dev

@BobSong-dev BobSong-dev commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

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

  • Add AgentMcpTracingLifecycleTest to exercise enabled, disabled, and re-enabled plugin contexts under one application-owned OpenTelemetrySdk.
  • Assert plugin reload does not close the parent SDK, does not register another SdkTracerProvider, and that one terminal finish exports exactly one logical MCP span.
  • Assert the application can continue exporting spans after plugin context shutdown.

Verification

  • Windows/JDK 17: Maven Wrapper clean verify for 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.
  • RISC-V/QEMU/JDK 17: equivalent 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.
  • Cross-platform audit matched 4,666 source files and individual test identities.
  • git diff --check: passed.

@Aias00 Aias00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

…p004

# Conflicts:
#	shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-proxy/src/test/java/org/apache/shenyu/plugin/ai/proxy/enhanced/AiProxyPluginTest.java
@BobSong-dev

Copy link
Copy Markdown
Contributor Author

Thank you for the detailed review. I have rebuilt the branch with normal merges and pushed 68b770b0b.

Local verification:

  • python .github/scripts/check-agent-gateway-migrations.py: 24 matching Agent Gateway seed rows for each of mysql/ob/og/oracle/pg.
  • python .github/scripts/test-agent-gateway-migrations.py: 11 tests passed.
  • AgentMcpTracingLifecycleTest: 1 test passed (0 failures/errors/skips), with the related 28-module Maven reactor and Checkstyle passing.
  • git diff --check: passed.

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.

@BobSong-dev
BobSong-dev marked this pull request as ready for review October 28, 2026 13:48

@Aias00 Aias00 left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Aias00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Aias00
Aias00 merged commit 8b03f7c into apache:master Oct 30, 2026
29 checks passed
@Aias00 Aias00 added this to the 2.7.2 milestone Nov 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants