[AI-383] Add replay-safe OpenTelemetry meter and logger providers - #1710
[AI-383] Add replay-safe OpenTelemetry meter and logger providers#1710DABH wants to merge 26 commits into
Conversation
Google ADK and similar libraries record OpenTelemetry metrics through the process-global meter provider from code that runs workflow-side, so every workflow replay re-records them. ReplaySafeMeterProvider wraps a user-supplied MeterProvider and drops synchronous instrument recordings made from workflow code during replay, matching the first-execution-only semantics of workflow.metric_meter(). Observable instruments and non-workflow recordings pass through untouched.
At worker configuration, warn when the global OpenTelemetry meter or tracer provider is not replay-safe, pointing users at ReplaySafeMeterProvider and create_tracer_provider. Add a regression test proving 1 real execution + 3 replays leaves ADK metric instruments at their nonzero baseline with ReplaySafeMeterProvider installed, plus a control asserting the 4x inflation without it.
ReplaySafeMeterProvider unconditionally forwarded get_meter attributes (added in opentelemetry 1.26) and create_histogram explicit_bucket_boundaries_advisory (added in 1.30), raising TypeError into caller code on older APIs within the supported range. Forward them only when non-None, matching unwrapped-caller behavior.
There was a problem hiding this comment.
Pull request overview
This PR adds replay-safe OpenTelemetry metrics support for workflow-side instrumentation by introducing a ReplaySafeMeterProvider wrapper and integrating replay-safety warnings into the Google ADK plugin, to prevent metric inflation during Temporal workflow replays.
Changes:
- Added
temporalio.contrib.opentelemetry.ReplaySafeMeterProviderto drop synchronous metric recordings from workflow code during replay. - Updated
GoogleAdkPluginto warn at worker configuration time when global OTel meter/tracer providers are not replay-safe. - Added regression + unit tests, and updated READMEs and CHANGELOG to document the behavior and configuration.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/contrib/opentelemetry/test_meter_provider.py | Unit tests for the replay-safe meter provider passthrough and compatibility behavior. |
| tests/contrib/google_adk_agents/test_replay_metrics.py | Regression test demonstrating replay metric inflation without the wrapper and correctness with it. |
| tests/conftest.py | Adds a fixture to reset global OTel meter provider state across tests. |
| temporalio/contrib/opentelemetry/README.md | Documents replay-safe metrics and how to install the global wrapper. |
| temporalio/contrib/opentelemetry/_meter_provider.py | Implements ReplaySafeMeterProvider and replay-gating wrappers for sync instruments. |
| temporalio/contrib/opentelemetry/init.py | Exports ReplaySafeMeterProvider and ReplaySafeTracerProvider. |
| temporalio/contrib/google_adk_agents/README.md | Documents replay behavior for ADK telemetry and recommended replay-safe global providers. |
| temporalio/contrib/google_adk_agents/_plugin.py | Adds worker-config-time warnings for non-replay-safe global OTel providers. |
| CHANGELOG.md | Adds an entry describing the new provider and plugin warning behavior. |
Suppressed comments (1)
temporalio/contrib/google_adk_agents/_plugin.py:69
- Add
stacklevel(and ideallycategory) to this warning so it points at the user’s worker/client setup callsite instead of inside the plugin implementation. This makes the warning much easier to act on when it fires during worker configuration.
warnings.warn(
"The global OpenTelemetry TracerProvider is not replay-safe: Google ADK "
"creates spans from workflow code, so every workflow replay will "
"re-emit them. Install a replay-safe provider: "
"opentelemetry.trace.set_tracer_provider("
"temporalio.contrib.opentelemetry.create_tracer_provider())"
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Guard the private _Gauge import (added in opentelemetry-api 1.23) and import ReplaySafeMeterProvider into the package lazily so temporalio.contrib.opentelemetry stays importable for tracing-only users on opentelemetry-api < 1.12, with an actionable error on access. - Classify the global meter provider without a module-level private import; when the private proxy class cannot be imported the provider is unclassifiable and no warning is issued. - Forward the context argument to sync instrument wrappers only when set; the parameter was added in opentelemetry-api 1.28 and older instruments reject it. - Add explicit UserWarning category and stacklevel so provider warnings point at the user's Worker(...) call.
ADK emits gen_ai.* log events from workflow code through the global logger provider, so replays duplicate them like metrics and spans. ReplaySafeLoggerProvider wraps a LoggerProvider and drops Logger.emit during replay. The logs API only exists in opentelemetry-api >= 1.15, so the import is guarded like the meter provider's; emit forwards arguments verbatim since its signature changed in 1.38.
Validate the global logger provider alongside meter and tracer, and run the validation from configure_replayer too since Replayer replays are exactly where unsafe providers re-record telemetry. Warn only on providers positively identified as replay-unsafe (OTel SDK providers used directly) so custom wrappers around replay-safe providers no longer trigger false positives, and compute the warning stacklevel dynamically so attribution survives wrapping plugins. Add log-event replay regression tests mirroring the metrics ones.
62120b6 to
891b68b
Compare
The de facto floor of temporalio.contrib.opentelemetry is already 1.24: _tracer_provider.py imports opentelemetry.util._decorator's _agnosticcontextmanager, which was added in opentelemetry-api 1.24, so the declared 1.11.1 floor has been uninstallable in practice for this contrib regardless of the new providers. Aligning the declaration with reality lets the replay-safe providers import _Gauge (exported since 1.23) and the logs API unconditionally, removing the guarded-import machinery in the package __init__ and the subprocess-based import-absence tests. The conditional kwarg forwards stay: get_meter/get_logger attributes arrived in 1.26, synchronous-instrument context in 1.28, and create_histogram's explicit_bucket_boundaries_advisory in 1.30, all above the new floor.
Logger is an ABC whose __init__ records the instrumentation scope, so the wrapper now threads name/version/schema_url from get_logger through to the base class instead of relying on __getattr__ delegation for that state. Fixes the basedpyright reportMissingSuperCall error that failed lint. Also documents that opentelemetry._logs is the import path OpenTelemetry itself sanctions for the logs bridge API while it is pre-GA.
The plugin now warns only when the global meter or tracer provider is an instance of the public OpenTelemetry SDK provider classes; everything else (unset proxies, no-ops, unknown wrappers) stays silent, which drops the private _ProxyMeterProvider/ProxyLoggerProvider imports. The SDK logger provider is not checked because its class is only importable from the underscore namespace opentelemetry.sdk._logs while OTel logs are pre-GA. The opentelemetry.sdk import is guarded for the SDK-less install case, the warning stacklevel walk uses inspect.currentframe, and the conftest OTel global resets are documented as the isolation pattern OpenTelemetry's own test suite uses.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
temporalio/contrib/opentelemetry/_tracer_provider.py:183
- Now that this class is public, the constructor does not enforce the documented relationship between these two arguments. A provider using
TemporalIdGeneratorA plus a separately supplied generator B passes the current check; spans use A, whileid_generator()reports B. Reject a non-identical generator (or derive it from the provider) so the public wrapper cannot expose inconsistent state.
id_generator: The ``TemporalIdGenerator`` used by ``tracer_provider``.
The tracer provider has always forwarded the get_tracer attributes parameter (added in opentelemetry 1.26) unconditionally, so no release of this contrib ever worked for tracing below 1.26. Declaring 1.26 keeps the floor honest at the call level and removes the three conditional forwards for the attributes parameter; the version-gated forwards for parameters newer than the floor (context 1.28, histogram advisory 1.30, emit fields 1.38) remain.
| from typing import TYPE_CHECKING, Any | ||
|
|
||
| # opentelemetry._logs is the import path OpenTelemetry itself documents for | ||
| # the logs bridge API while it is pre-GA (there is no non-underscore |
There was a problem hiding this comment.
This claims it is stable:
https://opentelemetry.io/docs/specs/otel/logs/api/
| def _skip_emitting() -> bool: | ||
| # in_workflow() must be evaluated first: is_replaying() requires an active | ||
| # workflow context. | ||
| return workflow.in_workflow() and workflow.unsafe.is_replaying() |
There was a problem hiding this comment.
Did you consider replaying versus replaying history events?
Queries and update validators are live, at-most-once-per-request operations even when they execute while the workflow is replaying, so ReplaySafeMeterProvider and ReplaySafeLoggerProvider must not drop their recordings. Gate on is_replaying_history_events() instead of is_replaying(), matching the replay-safe tracer.
The Logs API specification is stable; what keeps the import private is that opentelemetry-python has not promoted opentelemetry._logs to a public namespace.
The Span ABC ships add_link as a non-abstract warn-and-no-op default that __getattr__ cannot intercept, so links added through the wrapper after span creation were silently dropped.
OTel adds API surface as non-abstract no-op defaults, so a wrapper method left un-overridden silently swallows telemetry instead of delegating. Enumerate each wrapped ABC's public and abstract surface by reflection so new opentelemetry-api surface fails loudly.
The OTel globals are set-once per process and xdist worksteal ignores xdist_group pinning, so tests share workers with arbitrary siblings. Assert that global provider installs take effect, park proxy meters on a no-op provider around the meter-reset fixture, and uninstrument GoogleADKInstrumentor after test_single_agent_telemetry.
What was changed
temporalio.contrib.opentelemetrygains a replay-safe provider trio, gated onworkflow.in_workflow() and workflow.unsafe.is_replaying():ReplaySafeMeterProvider(new): drops synchronous instrument recordings (add()/record()/set()) made from replaying workflow code; observable instruments and all non-workflow recordings pass through.ReplaySafeLoggerProvider(new): the same gate for the OTel logs bridge, so ADK'sgen_ai.*log events don't re-emit on replay. (opentelemetry._logsis the import path OTel itself documents while the logs API is pre-GA.)ReplaySafeTracerProvider: now exported publicly alongside the other two (was only importable from the private module).Also:
GoogleAdkPluginwarning at worker/replayer configuration — fires only when a global provider is positively replay-unsafe (a raw OTel SDKMeterProvider/TracerProviderinstalled unwrapped); proxies, no-ops, and unknown custom wrappers stay silent. Messages include install snippets and attribute (viastacklevel) to the user'sWorker(...)/Replayer(...)call.>=1.11.1→>=1.26(opentelemetryandlambda-worker-otelextras) — aligns the declaration with reality at the call level: importing the contrib has required otel ≥ 1.24 on main for some time, andReplaySafeTracerProvider.get_tracerhas always forwarded theattributesparameter added in 1.26, so no earlier version ever worked for tracing. With the honest floor there are no guarded imports and noattributesconditionals; the remaining accommodations forward kwargs newer than the floor (context 1.28, histogram advisory 1.30) only when set, and handleLogger.emit's signature change (1.38).Replayerreplays → everygcp.vertex.agentinstrument andgen_ai.*log event stays at its nonzero baseline with the wrappers; controls document the 4× inflation without them. Unit tests cover passthrough, OTel version compatibility, and the warning's fire/stay-silent cases.Why
google-adk records its metrics (
gen_ai.client.token.usage, operation durations, call counts) and emitsgen_ai.*log events through the process-global OTel providers, from flow code this plugin runs workflow-side. Every replay (cache eviction, worker restart, redeploy,max_cached_workflows=0) re-records all of it even though the model/tool activities resolve from history. Measured: 1 real run + 3 replays → exactly 4× on every instrument and log event, while real activity executions stayed at 1 (replays also record bogus near-zero latency samples).workflow.metric_meter()is already replay-safe, but ADK bypasses it via the OTel global API — this puts the equivalent gate at the provider boundary, fixing the whole supported google-adk range without upstream changes. Recordings become first-execution-only, matchingworkflow.metric_meter()semantics.Testing
pytest tests/contrib/google_adk_agents tests/contrib/opentelemetry→ 82 passed, 5 skipped (pre-existing env-gated skips).poe lint(ruff, pyright, mypy, basedpyright, pydocstyle) andpoe gen-docsclean.