Skip to content

Commit 62120b6

Browse files
DABHclaude
andcommitted
Harden replay-safe metrics against old opentelemetry-api
- 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. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 8cff628 commit 62120b6

5 files changed

Lines changed: 298 additions & 18 deletions

File tree

‎temporalio/contrib/google_adk_agents/_plugin.py‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@
1010

1111
import opentelemetry.metrics
1212
import opentelemetry.trace
13-
from opentelemetry.metrics import NoOpMeterProvider
14-
from opentelemetry.metrics._internal import _ProxyMeterProvider
13+
from opentelemetry.metrics import MeterProvider, NoOpMeterProvider
1514
from opentelemetry.trace import NoOpTracerProvider, ProxyTracerProvider
1615

1716
from temporalio import workflow
@@ -37,23 +36,47 @@
3736
from temporalio.worker.workflow_sandbox import SandboxedWorkflowRunner
3837

3938

39+
def _meter_provider_replay_safe(provider: MeterProvider) -> bool | None:
40+
"""Classify the global meter provider for replay safety.
41+
42+
Returns True when replay-safe (including providers that drop all
43+
recordings), False when positively identified as replay-unsafe, and None
44+
when it cannot be classified, in which case no warning should be issued
45+
since a false positive is worse than a missed warning.
46+
"""
47+
if isinstance(provider, (ReplaySafeMeterProvider, NoOpMeterProvider)):
48+
return True
49+
try:
50+
# Unlike tracing's public ProxyTracerProvider, the proxy (unset) meter
51+
# provider has no public counterpart. Import the private class lazily
52+
# so a moved or removed symbol cannot break module import; it is
53+
# present in opentelemetry-api 1.12 through at least 1.42.
54+
from opentelemetry.metrics._internal import _ProxyMeterProvider
55+
except ImportError:
56+
return None
57+
return isinstance(provider, _ProxyMeterProvider)
58+
59+
4060
def _warn_if_global_otel_providers_not_replay_safe() -> None:
4161
# ADK records metrics and spans through the process-global OpenTelemetry
4262
# providers from code that runs workflow-side, so a non-replay-safe global
4363
# provider re-emits that telemetry on every workflow replay. Unset (proxy)
4464
# and no-op providers drop recordings and are fine.
65+
#
66+
# stacklevel=4 attributes the warnings to the user's Worker(...) call:
67+
# warn -> this helper -> GoogleAdkPlugin.configure_worker ->
68+
# Worker.__init__ -> user code.
4569
meter_provider = opentelemetry.metrics.get_meter_provider()
46-
if not isinstance(
47-
meter_provider,
48-
(ReplaySafeMeterProvider, NoOpMeterProvider, _ProxyMeterProvider),
49-
):
70+
if _meter_provider_replay_safe(meter_provider) is False:
5071
warnings.warn(
5172
"The global OpenTelemetry MeterProvider is not replay-safe: Google ADK "
5273
"records metrics from workflow code, so every workflow replay will "
5374
"re-record them. Wrap your provider in "
5475
"temporalio.contrib.opentelemetry.ReplaySafeMeterProvider and make it "
5576
"the first and only global provider set: "
56-
"opentelemetry.metrics.set_meter_provider(ReplaySafeMeterProvider(provider))"
77+
"opentelemetry.metrics.set_meter_provider(ReplaySafeMeterProvider(provider))",
78+
UserWarning,
79+
stacklevel=4,
5780
)
5881
tracer_provider = opentelemetry.trace.get_tracer_provider()
5982
if not isinstance(
@@ -65,7 +88,9 @@ def _warn_if_global_otel_providers_not_replay_safe() -> None:
6588
"creates spans from workflow code, so every workflow replay will "
6689
"re-emit them. Install a replay-safe provider: "
6790
"opentelemetry.trace.set_tracer_provider("
68-
"temporalio.contrib.opentelemetry.create_tracer_provider())"
91+
"temporalio.contrib.opentelemetry.create_tracer_provider())",
92+
UserWarning,
93+
stacklevel=4,
6994
)
7095

7196

‎temporalio/contrib/opentelemetry/__init__.py‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,24 @@
55
propagation for distributed tracing.
66
"""
77

8+
from typing import Any
9+
810
from temporalio.contrib.opentelemetry._interceptor import (
911
TracingInterceptor,
1012
TracingWorkflowInboundInterceptor,
1113
)
12-
from temporalio.contrib.opentelemetry._meter_provider import ReplaySafeMeterProvider
14+
15+
_meter_provider_import_error: ImportError | None = None
16+
try:
17+
from temporalio.contrib.opentelemetry._meter_provider import (
18+
ReplaySafeMeterProvider,
19+
)
20+
except ImportError as err:
21+
# opentelemetry-api < 1.12 has no opentelemetry.metrics module. Keep the
22+
# tracing integration importable and raise a clear error only when
23+
# ReplaySafeMeterProvider is actually accessed (see __getattr__ below).
24+
_meter_provider_import_error = err
25+
1326
from temporalio.contrib.opentelemetry._otel_interceptor import OpenTelemetryInterceptor
1427
from temporalio.contrib.opentelemetry._plugin import OpenTelemetryPlugin
1528
from temporalio.contrib.opentelemetry._tracer_provider import (
@@ -26,3 +39,16 @@
2639
"ReplaySafeTracerProvider",
2740
"create_tracer_provider",
2841
]
42+
43+
44+
def __getattr__(name: str) -> Any:
45+
# Only reachable for ReplaySafeMeterProvider when the guarded import above
46+
# failed; otherwise the module attribute exists and this is never called.
47+
if name == "ReplaySafeMeterProvider":
48+
raise ImportError(
49+
"ReplaySafeMeterProvider requires the OpenTelemetry metrics API "
50+
"(opentelemetry.metrics), which the installed opentelemetry-api "
51+
"version does not provide. Install opentelemetry-api >= 1.12 "
52+
"(>= 1.23 for synchronous gauge support)."
53+
) from _meter_provider_import_error
54+
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")

‎temporalio/contrib/opentelemetry/_meter_provider.py‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
from collections.abc import Sequence
2-
from typing import Any
2+
from typing import TYPE_CHECKING, Any
33

44
from opentelemetry.context import Context
55
from opentelemetry.metrics import (
@@ -12,13 +12,32 @@
1212
ObservableGauge,
1313
ObservableUpDownCounter,
1414
UpDownCounter,
15-
_Gauge,
1615
)
1716
from opentelemetry.util.types import Attributes
1817

1918
from temporalio import workflow
2019

2120

21+
def _forward_context_kwarg(context: Context | None) -> dict[str, Any]:
22+
# Forward the context only when set: the parameter was added to the
23+
# synchronous instrument methods in opentelemetry 1.28 and older
24+
# instruments raise TypeError when it is passed.
25+
return {} if context is None else {"context": context}
26+
27+
28+
if TYPE_CHECKING:
29+
from opentelemetry.metrics import _Gauge
30+
else:
31+
try:
32+
from opentelemetry.metrics import _Gauge
33+
except ImportError:
34+
# Synchronous gauges (and Meter.create_gauge) were added in
35+
# opentelemetry-api 1.23. Fall back to a plain base class so this
36+
# module stays importable on older versions in the supported range;
37+
# meters there have no create_gauge to wrap anyway.
38+
_Gauge = object
39+
40+
2241
def _skip_recording() -> bool:
2342
# in_workflow() must be evaluated first: is_replaying() requires an active
2443
# workflow context.
@@ -41,7 +60,7 @@ def add(
4160
if _skip_recording():
4261
# Skip recording metrics during workflow replay to avoid duplicate telemetry
4362
return
44-
self._counter.add(amount, attributes, context)
63+
self._counter.add(amount, attributes, **_forward_context_kwarg(context))
4564

4665

4766
class _ReplaySafeUpDownCounter(UpDownCounter):
@@ -60,7 +79,7 @@ def add(
6079
if _skip_recording():
6180
# Skip recording metrics during workflow replay to avoid duplicate telemetry
6281
return
63-
self._counter.add(amount, attributes, context)
82+
self._counter.add(amount, attributes, **_forward_context_kwarg(context))
6483

6584

6685
class _ReplaySafeHistogram(Histogram):
@@ -79,7 +98,7 @@ def record(
7998
if _skip_recording():
8099
# Skip recording metrics during workflow replay to avoid duplicate telemetry
81100
return
82-
self._histogram.record(amount, attributes, context)
101+
self._histogram.record(amount, attributes, **_forward_context_kwarg(context))
83102

84103

85104
class _ReplaySafeGauge(_Gauge):
@@ -98,7 +117,7 @@ def set(
98117
if _skip_recording():
99118
# Skip recording metrics during workflow replay to avoid duplicate telemetry
100119
return
101-
self._gauge.set(amount, attributes, context)
120+
self._gauge.set(amount, attributes, **_forward_context_kwarg(context))
102121

103122

104123
class _ReplaySafeMeter(Meter):
@@ -154,6 +173,8 @@ def create_gauge(
154173
unit: str = "",
155174
description: str = "",
156175
) -> _Gauge:
176+
# On opentelemetry-api < 1.23 the underlying meter has no create_gauge
177+
# and this raises AttributeError, same as calling it on the raw meter.
157178
return _ReplaySafeGauge(self._meter.create_gauge(name, unit, description))
158179

159180
# Observable instruments pass through unwrapped: their callbacks run on the

‎tests/contrib/google_adk_agents/test_replay_metrics.py‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
recordings while leaving first-execution recordings intact.
1010
"""
1111

12+
import sys
1213
import uuid
1314
import warnings
1415
from collections.abc import AsyncGenerator
@@ -23,6 +24,8 @@
2324
from google.adk.runners import InMemoryRunner
2425
from google.adk.utils.context_utils import Aclosing
2526
from google.genai import types
27+
from opentelemetry.metrics import NoOpMeterProvider
28+
from opentelemetry.metrics._internal import _ProxyMeterProvider
2629
from opentelemetry.sdk.metrics import MeterProvider
2730
from opentelemetry.sdk.metrics.export import InMemoryMetricReader
2831
from opentelemetry.sdk.trace import TracerProvider
@@ -287,3 +290,63 @@ def test_plugin_does_not_warn_with_unset_providers(
287290
warnings.simplefilter("always")
288291
GoogleAdkPlugin().configure_worker(_worker_config())
289292
assert not [w for w in recorded if "replay-safe" in str(w.message)]
293+
294+
295+
def test_meter_provider_replay_safety_classification(monkeypatch: pytest.MonkeyPatch):
296+
from temporalio.contrib.google_adk_agents._plugin import (
297+
_meter_provider_replay_safe,
298+
)
299+
300+
assert _meter_provider_replay_safe(ReplaySafeMeterProvider(MeterProvider())) is True
301+
assert _meter_provider_replay_safe(NoOpMeterProvider()) is True
302+
assert _meter_provider_replay_safe(_ProxyMeterProvider()) is True
303+
assert _meter_provider_replay_safe(MeterProvider()) is False
304+
305+
# When the private proxy class cannot be imported, providers other than
306+
# the replay-safe/no-op ones are unclassifiable and must not warn.
307+
monkeypatch.setitem(sys.modules, "opentelemetry.metrics._internal", None)
308+
assert _meter_provider_replay_safe(MeterProvider()) is None
309+
assert _meter_provider_replay_safe(NoOpMeterProvider()) is True
310+
311+
312+
def test_plugin_does_not_warn_when_proxy_detection_unavailable(
313+
monkeypatch: pytest.MonkeyPatch,
314+
reset_otel_meter_provider, # type: ignore[reportUnusedParameter]
315+
reset_otel_tracer_provider, # type: ignore[reportUnusedParameter]
316+
):
317+
# A replay-unsafe provider is set, but with the private proxy class
318+
# unimportable it cannot be classified, so the plugin stays silent rather
319+
# than risking a false positive.
320+
opentelemetry.metrics.set_meter_provider(MeterProvider())
321+
monkeypatch.setitem(sys.modules, "opentelemetry.metrics._internal", None)
322+
with warnings.catch_warnings(record=True) as recorded:
323+
warnings.simplefilter("always")
324+
GoogleAdkPlugin().configure_worker(_worker_config())
325+
assert not [
326+
w for w in recorded if "MeterProvider is not replay-safe" in str(w.message)
327+
]
328+
329+
330+
async def test_plugin_warning_points_at_worker_construction(
331+
client: Client,
332+
reset_otel_meter_provider, # type: ignore[reportUnusedParameter]
333+
reset_otel_tracer_provider, # type: ignore[reportUnusedParameter]
334+
):
335+
# stacklevel on the warning must attribute it to the user's Worker(...)
336+
# call, i.e. this file, not SDK internals.
337+
opentelemetry.metrics.set_meter_provider(MeterProvider())
338+
with warnings.catch_warnings(record=True) as recorded:
339+
warnings.simplefilter("always")
340+
Worker(
341+
client,
342+
task_queue=f"replay-metrics-{uuid.uuid4()}",
343+
activities=[replay_metrics_get_weather],
344+
workflows=[ReplayMetricsAgent],
345+
plugins=[GoogleAdkPlugin()],
346+
)
347+
warned = [
348+
w for w in recorded if "MeterProvider is not replay-safe" in str(w.message)
349+
]
350+
assert len(warned) == 1
351+
assert warned[0].category is UserWarning
352+
assert warned[0].filename == __file__

0 commit comments

Comments
 (0)