Repository navigation
Conversation
Base the factory migration on the separately releasable compatibility fix, retain its regression coverage under the 3.x lifecycle, and remove duplicate hook-linkage tests.
zhongkechen
changed the title
GitHub Actions
Failure
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. ### Description Fixes #763. An older visible OpenTelemetry API can lack `GlobalOpenTelemetry.isSet()` or `getOrNoop()`. The resulting `NoSuchMethodError` previously escaped plugin dispatch and failed the customer invocation. The plugin now checks those APIs before global-provider binding and reports incompatible dependencies without installing a no-op global. Runtime hook dispatch also isolates `LinkageError`, so healthy plugins and the handler continue. This is an independently mergeable 2.x fix for work item 7. Public APIs, constructors, instance lifetimes, dependency versions, and provider configuration validation are unchanged. Fatal JVM errors and `ThreadDeath` retain their existing propagation behavior. Align incompatible dependencies to restore telemetry; containment does not make an unsupported API export spans. Merge this PR into **main first**. The separate major factory migration in #782 is stacked on this branch; after this PR merges, update #782 from main and retarget it to main. Do not merge #782 into this 2.x branch. The CI callers preserve their existing serial shared-resource groups and retain pending runs. Java 17 Build now runs a required installed-artifact compatibility matrix; dependency-resolution and probe failures fail CI rather than skipping coverage. ### Validation - Final integrated Java 17 `mvn -B clean verify`: **2,171 tests, 31 existing skips, zero failures/errors**, including main's CodeBuild and test-environment changes. - Regression controls on the unfixed baseline reproduced 28 hook-containment failures and two global-provider linkage errors. Focused coverage checks all seven hooks, original fatal identity, healthy-plugin continuation, and suspension/resume without repeating a completed step. - Installed-artifact matrix: **16/16 passed**. Uses actual released core/plugin/testing SDK 2.2.1, candidate core/plugin jars, API/context 1.49.0 and 1.66.0, and both views. The old/old + 1.49 cases reproduce the exact customer failure; the fixed combinations preserve handler execution. It checks real ServiceLoader discovery, unchanged instance lifetime, late global registration, compatible Workflow export, and selected jar provenance. - Actual released OpenTelemetry Java agent **2.32.0**, Corretto 17, plugin jar loaded as the agent extension: **16/16 local cases passed** across the same core/plugin/API/view combinations. The agent exported a control span; compatible API 1.66 exported Workflow spans. Older API cases preserved the handler or reproduced the expected old/old failure. This separate probe neither resets the global provider nor injects the installed-extension marker. It validates this exact agent setup, not every ADOT/Lambda layer version. - `PluginRunner` public signatures match the 2.x main baseline. No production POM or dependency-floor changes. ### Checklist - [x] Final integrated local build and regression controls passed - [x] Required released/current artifact matrix added to CI - [x] Actual released Java-agent verification completed - [ ] Current-head CI and actual AI review completed; valid findings addressed
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
GitHub Actions
Failure
|
|
||
| /** Gives cooperative operation owners their existing fatal cleanup allowance before plugin finalization. */ | ||
| void awaitFatalOperationCleanup() { | ||
| if (pluginFatal.get() == null) return; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Validated and fixed in fc75e95. An already-selected VirtualMachineError or ThreadDeath from the root handler or output serialization now enters the existing fatal shutdown path before finalization, including when no plugins are configured. This reuses the shared cleanup deadline, cooperative interruption, and checkpoint abort; it does not introduce a new timeout or fatal classifier.
Public execution controls cover both failure stages, exact fatal identity, plugins/no plugins, and cooperative/interrupt-ignoring peers. All 16 fatal cases timed out before the fix. They now return the original fatal within the existing cleanup budget; cooperative peer cleanup precedes the single RETRYING end hook, and an uncooperative peer cannot hold the caller indefinitely. Two ordinary-error controls retain the existing wait and no-interruption behavior.
All 84 focused tests and full Java 17 validation pass (2,758 tests, zero failures/errors, 33 skips), as do formatting and the four-case 3.x artifact matrix. The selected-error and frozen end-snapshot policies are unchanged.
| if (preservePluginMdc && !settled && !result.isCompletedExceptionally()) { | ||
| // Worker restoration follows completion callbacks. Report a fatal that follows a successful | ||
| // result through the existing owner-cleanup boundary: before end dispatch it changes the | ||
| // invocation outcome; after dispatch it only escapes the owner. An already-failed task retains | ||
| // its primary failure and normal try-with-resources suppression. | ||
| normalizedFailure = ExceptionHelper.unwrapAsyncFailure(failure); | ||
| if (isFatal(normalizedFailure)) onPostCompletionFatal.accept((Error) normalizedFailure); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
The failed-task case deliberately retains the primary failure and standard try-with-resources suppression, as covered by failedBodyRetainsPrimaryFailureAndSuppressedRestoration and explained in #782 (comment). Moving future publication would not by itself change Java's primary-failure precedence; replacing it with a suppressed cleanup fatal would be a separate error-policy change. The successful-task fatal before end dispatch has its own direct/asynchronous regression coverage and reporting path.
Completion callbacks retain task MDC for end hooks. Ordinary restoration remains best effort when the adapter rejects clear/set, and an arbitrary user-provided executor does not provide a guarantee that an escaped task failure retires its worker. The existing primary-failure and late-finalization contracts are retained here rather than introducing that additional policy change.
This comment has been minimized.
This comment has been minimized.
GitHub Actions
Error
Codex AI reviewOne high-severity lifecycle issue remains in handler-worker MDC restoration. Reviewed commit |
This branch was successfully deployed
2 participants
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Description
Isolate mutable plugin state between concurrent Lambda Managed Instances executions by creating and releasing plugin instances per invocation. The 3.x factory API receives the same
InvocationInfosnapshot as the invocation-start hook, including the runtime-captured X-Ray header; customextract(info)implementations retain their dispatch path.Handler scopes close on their owner thread. Reported fatal errors wake the caller, preserve the bounded cleanup allowance, and settle queued and active checkpoint futures without waiting for a blocked backend call. Running peer operations receive cooperative interruption with one bounded cleanup allowance; their real completion remains owned by their worker. End-hook dispatch preserves completion ownership and MDC. The selected invocation outcome remains frozen once end dispatch begins. Agent-backed spans also avoid carrying an application-loader sampling decision into unrelated spans. Cyclic or unreadable exception cause chains and diagnostic failures cannot disrupt nonfatal plugin containment.
Dependency and scope
#780 has merged into
main, and this PR now targetsmain. The base-history synchronization preserves the previously validated file tree: the squashed main tree is identical to the #780 head already in this branch's ancestry.The backward-compatible OpenTelemetry API/version-linkage fix for #763 belongs to #780. Its global-provider guards and shared linkage tests are inherited through the base branch. This PR contains the separate 3.x factory contract, per-invocation lifetime, handler-scope cleanup, and fatal-error propagation/finalization behavior. Tests use factory registration and fresh plugin instances on resume. This is a major-version migration.
Validation
mvn -o -B clean verify: 2,702 tests, 33 existing skips, zero failures/errors.InvocationInfocompatibility cases and the accepted record-pattern source boundary.89d96c322c82fdf4aa73291e4c98b6a9d0fdadaa; its tracked file tree is identical to the fully green5cf04d1877553652d21d0f688cb0ddbb97359578. The preceding head78373abd264dea4c4c5ee5b2cc7c8982639c71afpassed all six OTel suites through the pull-request workflow on this stacked base; the earlier manual-dispatch OIDC failure does not block that PR workflow path. Local validation does not count as a cloud-conformance pass for the new head.