fix(google-adk-agents): Node workflow loading, OTel composition, and telemetry replay-safety tests - #2276
Open
DABH wants to merge 14 commits into
Open
fix(google-adk-agents): Node workflow loading, OTel composition, and telemetry replay-safety tests#2276DABH wants to merge 14 commits into
DABH wants to merge 14 commits into
Conversation
The workflow bundle crashed at load on Node (Bun masked both crashes by exposing web globals): - @google/genai's web build dereferences ReadableStream at module load, before user imports could evaluate the polyfill barrel. Prepend the load-polyfills module to workflowInterceptorModules so it evaluates per workflow, before the user's workflow module, regardless of import order. - @google/adk's utils/client_labels.js runs new AsyncLocalStorage() from node:async_hooks at module load. Redirect async_hooks to a shim re-exporting the sandbox-injected AsyncLocalStorage global.
@opentelemetry/sdk-trace-base and @opentelemetry/resources were aliased to empty modules for the whole workflow bundle, so composing this plugin with @temporalio/interceptors-opentelemetry failed every workflow task with 'tracing.BasicTracerProvider is not a constructor' — disabling the SDK's replay-gated workflow span path (the only egress for ADK's gcp.vertex.agent spans) for all workflows on the worker. Both packages are pure JS and load-safe in the sandbox; keep them real.
Run a two-turn agent workflow composed with OpenTelemetryPlugin and the workflow cache disabled (every workflow task replays the whole history): the workflow must complete and export exactly one gcp.vertex.agent span per real operation — replays add none. Also pin that without the OpenTelemetry plugin, sandbox spans never reach a process-global tracer provider.
- Fix README default-case wording: it's the absent tracer provider, not an impossibility of running an OTel SDK in the sandbox, that drops ADK spans. - Add README caution that workflow task retries (unlike replays) re-emit spans, so export is at-least-once. - Pin in plugin unit tests that the pure-JS OTel packages stay unstubbed and that load-polyfills is prepended to workflowInterceptorModules. - Document the converter-modules-evaluate-first gap in the bundler recipe.
Un-stubbing @opentelemetry/sdk-trace-base exposed @opentelemetry/core's browser build, which dereferences the performance global at module load. ESM workflow files dodge that chain through harmony-import pruning, but tsc-compiled CommonJS workflow files — including the published lib/ artifacts and converter modules importing @google/adk — evaluate it eagerly, failing every workflow task with 'ReferenceError: performance is not defined'. Install a deterministic performance shim (mapped onto the sandbox-patched Date.now, mirroring interceptors-opentelemetry's workflow runtime shim) in load-polyfills, and add E2E regression tests for the compiled-CJS workflow layout and the documented converter-module workaround.
@google/adk pins an exact @opentelemetry/api version, so the Workflow bundle can contain two api copies. ADK's telemetry/tracing.js caches trace.getTracer() at module load; when a user interceptor or converter module evaluates @google/adk before the OTel interceptor factories register the sandbox tracer provider, that tracer binds the other copy's never-delegated provider and every ADK span is silently dropped while the interceptor's own spans keep exporting. Rewrite all bundle requests for @opentelemetry/api to ADK's own resolution in the sandbox-compat webpack plugin, pin the rewrite with a unit contract test, and add an E2E test that evaluates ADK from a user workflowModules entry and asserts exact span counts.
Move the converter-module workaround into the README's telemetry cautions (the PR description already pointed there), note the polyfill loader now includes the performance shim, document the single @opentelemetry/api copy pinning, and extend the google-adk-agents changelog entry with the replay-safe span-export composition.
A workflow task retry re-executes its segment and legitimately re-emits spans through the replay-gated sink, so exact-count assertions could flake on slow runners; degrade to a lower bound when the history shows WorkflowTaskTimedOut/Failed events. Also make the performance-shim comment precise about the interceptor package's unconditional shim.
DABH
force-pushed
the
adk-telemetry-replay-safety
branch
from
August 1, 2026 09:02
becf512 to
c38dfd4
Compare
Replace the sandbox-compat plugin's beforeResolve rewrite of @opentelemetry/api with an exact-match resolve.alias entry in the webpackConfigHook — the same declarative surface the Worker bundler itself uses — and assert the config shape in the unit test instead of driving a fake compiler through the plugin's tap. Also export an empty interceptors factory from the polyfill loader so its workflowInterceptorModules entry satisfies the documented interceptor-module contract rather than relying on the runtime skipping modules without one, and point the recipe docs at the initRuntime evaluation-order contract.
Array-form resolve.alias resolves first-match-first, so prepend the @opentelemetry/api pin instead of appending it, matching the object branch where the pin is spread last. Cover the array branch in the config-shape test and note the ADK BaseTool contract at the external _getDeclaration call site.
Object-form resolve.alias entries also match in key insertion order, so a user prefix-form '@opentelemetry/api' key spread before the pin used to capture the bare specifier in object form while losing to the unshifted pin in array form. Place the pin key first in the object branch so both forms agree: the pin wins the bare specifier over any user entry, and a user prefix-form entry still applies to subpath imports.
Mirror packages/test so REUSE_V8_CONTEXT=false runs every worker (and the replayer) in per-workflow-VM mode, giving the polyfill loader and telemetry gating coverage in both sandbox modes.
ADK 1.5.0 added tools/load_web_page.js on the barrel path, which parses its blocked-CIDR tables at module load, calling net.isIP in the process. With net aliased to an empty module (like the other disallowed builtins), isIP is undefined and every workflow task fails at bundle load: on a fresh install resolving the ^1.4.0 peer range, the documented quick-start hangs in workflow-task retry with no usable diagnostic. Redirect net/node:net to a deterministic shim implementing isIP/isIPv4/isIPv6 with Node's own address grammar (the lib/internal/net.js regexes): pure string parsing, frozen in the bundle so classification cannot drift across replay, and no socket surface. A parity test pins the classifiers to node:net's across the addresses ADK parses at load plus classification corners, and the test workflows module mirrors ADK's top-level pattern so every E2E bundle in the suite fails loudly if the shim regresses. The devDependency stays at ^1.4.0: the workspace's minimumReleaseAge (two weeks) blocks resolving 1.5.0 until 2026-08-13. The full suite was additionally verified against a local @google/adk@1.5.0 install.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #2120 (targets
maplexu/google-adk-agents-contrib) — fixes issues found during a telemetry replay-safety assessment of the ADK integration.What was changed
Fix Node workflow-load crashes (failing all 8 Node CI jobs on feat(google-adk-agents): add @temporalio/google-adk-agents package #2120; Bun passes only because it exposes web streams natively).
BundleOptions.workflowInterceptorModulesinconfigureBundler, so it evaluates per-workflow after the activator is set and before user workflow code — regardless of user import order. It exports an emptyinterceptorsfactory to satisfy the documented interceptor-module contract. An entry-preload approach was deliberately avoided: entry code evaluates before any activator exists, and in reusable-V8-context mode (the default) that no-op evaluation would be cached permanently, so polyfills would never install.node:async_hooksis shimmed to the sandbox's own workflow-scopedAsyncLocalStorage(same approach ascontrib/langsmith), fixing@google/adk's top-levelnew AsyncLocalStorage(). This is strictly stronger than ADK's own browser fallback shim (context survives awaits) and is deterministic with sandbox-managed lifetime.performanceshim (timeOrigin/now()mapped onto the sandbox-patchedDate.now(), mirroring the shim@temporalio/interceptors-opentelemetryinstalls from its own workflow runtime module). Un-stubbing@opentelemetry/sdk-trace-base(item 2) exposes@opentelemetry/core's browser build, which dereferences theperformanceglobal at module load. ESM workflow files dodge that chain through harmony-import pruning, but tsc-compiled CommonJS workflow files — including the publishedlib/artifacts and converter modules that import@google/adk— evaluate it eagerly and failed every workflow task withReferenceError: performance is not defined.Stop stubbing
@opentelemetry/sdk-trace-baseand@opentelemetry/resourcesbundle-wide. Both are pure-JS and sandbox-safe; stubbing them broke composition with@temporalio/interceptors-opentelemetry/OpenTelemetryPlugin— the SDK's only replay-safe workflow span path — failing every workflow task withtracing.BasicTracerProvider is not a constructor, despite the README advising users to compose observability plugins with this one. The node-only OTel packages (exporters,sdk-trace-node,sdk-metrics,sdk-logs, resource detectors) remain stubbed, so ADK'stelemetry/setup.jsstill cannot create an un-gated egress inside the sandbox.Pin
@opentelemetry/apito a single copy in the Workflow bundle. ADK pins an exact@opentelemetry/apiversion while other packages in the bundle (notably@temporalio/interceptors-opentelemetry) may resolve a different one, so the bundle could contain two api copies. ADK'stelemetry/tracing.jscachestrace.getTracer(...)at module load; if that binds a different copy than the one the OTel interceptor registers its tracer provider on (e.g. a user interceptor or converter module evaluates@google/adkbefore the interceptor factories run), every ADK span is silently dropped while the interceptor's own spans keep exporting.configureBundlernow adds an exact-match ($)resolve.aliasentry — the same declarative surface the Worker bundler itself uses — mapping the bare@opentelemetry/apispecifier to the copy@google/adkitself resolves (obtained through ordinary Node resolution from ADK's entry point, not a hardcoded path).@opentelemetry/api-logsand subpath imports resolve normally. Precedence is identical in bothresolve.aliasforms: alias entries resolve first-match-first and the pin is placed first, so it wins the bare specifier over any user alias entry (exact- or prefix-form), while a user prefix-form@opentelemetry/apientry still applies to subpath imports.Telemetry regression tests (
telemetry.test.ts,compiled-cjs.test.ts, plusplugin.test.tscontract pins):adkSpansExportOncePerOperationUnderReplay: two-turn agent workflow with[OpenTelemetryPlugin, GoogleAdkPlugin]andmaxCachedWorkflows: 0; asserts the history contains ≥3 workflow tasks (replays actually happened) and the in-memory exporter holds exactly 2×call_llm, 2×invocation, 2×invoke_agent assistantfor scopegcp.vertex.agent— one span per real operation, none added by replays. The exact-count assertion is gated on a retry-free history: a workflow task retry (timeout/failure) is not a replay and legitimately re-emits its segment's spans, so when a slow runner caused retries the test degrades to a lower bound, which still fails on silently dropped spans. This pins the property that ADK's workflow-side spans export replay-safely through the sink gate, and fails if anything regresses toward the N+1 over-counting we found (and fixed) in the Go and Python ADK adapters (contrib/googleadk: add replay-safe OpenTelemetry provider wrappers sdk-go#2514, Add replay-safe OpenTelemetry meter and logger providers; warn in Google ADK plugin sdk-python#1710).adkSpansExportWhenAdkEvaluatesBeforeInterceptorFactories: a userinterceptors.workflowModulesentry imports@google/adk, so ADK's module-loadtrace.getTracer(...)runs before any interceptor factory registers the sandbox tracer provider; exact span counts must still hold — this pins the single-api-copy property of item 3.adkSpansDoNotLeakToProcessGlobalProvider: a worker-process global tracer provider observes zerogcp.vertex.agentspans — pins the isolate boundary the replay-safety rests on.compiled-cjs.test.ts: the tsc-compiled CommonJS workflow layout (what a compiled-TS production worker and the publishedlib/artifacts actually run) loads and runs withGoogleAdkPluginalone, and a custom payload-converter module importing@google/adkworks via the documented workaround — the module shapes that crashed on theperformanceglobal before item 1's shim.REUSE_V8_CONTEXTlikepackages/test, so the polyfill loader and telemetry gating can be exercised in per-workflow-VM mode as well as the default reusable-V8-context mode.README: "Telemetry and observability" section. ADK's
gen_aispans (includinggen_ai.usage.*token attributes) are created inside the workflow sandbox and are dropped by default; installing the OpenTelemetry interceptors exports them replay-safely (first-execution-only), with the single-api-copy pin documented alongside. Explicit cautions: don't register custom telemetry sinks withcallDuringReplay: true(reintroduces over-counting); span export is at-least-once (workflow task retries — unlike replays — can re-emit);call_llmspans carry full request/response payloads as attributes, so point the processor somewhere approved for prompt content. Documents the known gap that custom payload/failure converter modules evaluate before interceptor modules, with the workaround.Why
ADK records telemetry via the global OpenTelemetry API from code that runs workflow-side. In the Go and Python SDK adapters this caused verified replay over-counting (1 real execution + N replays → N+1 observations of token usage/latency). The TypeScript architecture is immune to that by construction — the isolate-local OTel API plus replay-gated sinks yield exactly one export per real operation — but as written the integration instead lost all agent-loop telemetry silently, and hard-broke the one supported way to get it out. This PR fixes the composition, fixes the Node workflow-load crashes that failed all 8 Node CI jobs on #2120, and pins the replay-safety property with tests so it stays true.
Testing
Full contrib suite on Node v26.5.0 against a real dev server: 57 tests passed, in both the default reusable-V8-context mode and with
REUSE_V8_CONTEXT=false(previously the E2E files crashed at workflow load withReadableStream is not defined). Causality verified by stashing the plugin fixes and reproducing the exact CI failure. ESLint and Prettier clean.CI status: on the last CI run the
google-adk-agentssuite passed on every integration job (56/56 at that head), including the five jobs shown as failed — those were cancelled by the job-leveltimeout-minutes, not by test failures. Three of the five hung incontrib/langsmith(signal-childtest timeout; a package this PR does not touch, and the same flake appears on recentmainruns —mainhas since landed #2274 to speed that suite up); the other two had zero test failures and simply exceeded the time budget late in the run. Rebasing the #2120 stack onto currentmain(picking up #2274) and re-running should get Node CI green.