Skip to content

Commit 43731f3

Browse files
authored
refactor(api-core): simplify OTel interceptor helpers and extract tracer provider (#18263)
## Summary Refactors `google.api_core._observability` to simplify OpenTelemetry interceptor instantiation and use concrete return types: - Extracts `_get_tracer_provider(client_options)` to centralize tracer provider lookup across `ClientOptions` objects and dictionaries. - Removes the private `_get_otel_interceptor(is_async: bool)` helper, allowing `get_otel_interceptor` and `get_otel_async_interceptor` to directly invoke synchronous and asynchronous OpenTelemetry APIs with concrete return types. - Adds dedicated unit tests for `_get_tracer_provider` and maintains symmetrical tests for sync and async interceptor getters. Addresses review feedback on #18236.
1 parent 024899e commit 43731f3

2 files changed

Lines changed: 65 additions & 109 deletions

File tree

packages/google-api-core/google/api_core/_observability.py

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,12 @@
2424
from google.api_core.client_options import ClientOptions
2525

2626
if TYPE_CHECKING:
27-
# flake8: grpc is imported only for static analysis and type annotations
27+
# flake8: grpc, trace, and ClientInterceptor are imported only for static analysis and type annotations
28+
# The `# noqa: F401` comment avoids flake8 "imported but not used" errors.
2829
import grpc # noqa: F401
30+
import opentelemetry.trace # noqa: F401
31+
32+
from google.api_core.grpc_helpers import ClientInterceptor # noqa: F401
2933

3034
_TRACER_PROVIDER = "tracer_provider"
3135

@@ -60,31 +64,23 @@ def is_otel_capabilities_enabled(
6064
return False
6165

6266

63-
def _get_otel_interceptor(
67+
def _get_tracer_provider(
6468
client_options: ClientOptions | dict[str, Any] | None = None,
65-
is_async: bool = False,
66-
) -> Any:
67-
"""Instantiates a sync or async OpenTelemetry gRPC client interceptor.
69+
) -> opentelemetry.trace.TracerProvider | None:
70+
"""Extracts the OpenTelemetry tracer provider from client options if present.
6871
6972
Args:
7073
client_options: The client options object or dictionary.
71-
is_async: If True, returns an async interceptor (`aio_client_interceptors`),
72-
otherwise returns a sync interceptor (`client_interceptor`).
7374
7475
Returns:
75-
Any: The instantiated OpenTelemetry client interceptor.
76+
opentelemetry.trace.TracerProvider | None: The tracer provider if present,
77+
None otherwise.
7678
"""
77-
import opentelemetry.instrumentation.grpc as otel_grpc # type: ignore[import-not-found]
78-
79-
tracer_provider = None
8079
if isinstance(client_options, dict):
81-
tracer_provider = client_options.get(_TRACER_PROVIDER)
80+
return client_options.get(_TRACER_PROVIDER)
8281
elif client_options is not None:
83-
tracer_provider = getattr(client_options, _TRACER_PROVIDER, None)
84-
85-
if is_async:
86-
return otel_grpc.aio_client_interceptors(tracer_provider=tracer_provider)
87-
return otel_grpc.client_interceptor(tracer_provider=tracer_provider)
82+
return getattr(client_options, _TRACER_PROVIDER, None)
83+
return None
8884

8985

9086
def get_otel_interceptor(
@@ -97,15 +93,17 @@ def get_otel_interceptor(
9793
and extracting the tracer provider.
9894
9995
Returns:
100-
Optional[Callable[[grpc.Channel], grpc.Channel]]: An interceptor callable if OpenTelemetry
96+
Callable[[grpc.Channel], grpc.Channel] | None: An interceptor callable if OpenTelemetry
10197
tracing is enabled and installed, None otherwise.
10298
"""
10399
if not is_otel_capabilities_enabled(client_options):
104100
return None
105101

106102
import opentelemetry.instrumentation.grpc as otel_grpc # type: ignore[import-not-found]
107103

108-
interceptor = _get_otel_interceptor(client_options, is_async=False)
104+
interceptor: ClientInterceptor = otel_grpc.client_interceptor(
105+
tracer_provider=_get_tracer_provider(client_options)
106+
)
109107

110108
def otel_interceptor(channel: grpc.Channel) -> grpc.Channel:
111109
return otel_grpc.intercept_channel(channel, interceptor)
@@ -123,10 +121,15 @@ def get_otel_async_interceptor(
123121
and extracting the tracer provider.
124122
125123
Returns:
126-
Optional[Sequence[grpc.aio.ClientInterceptor]]: Instantiated OpenTelemetry async
124+
Sequence[grpc.aio.ClientInterceptor] | None: Instantiated OpenTelemetry async
127125
client interceptors if tracing is enabled and installed, None otherwise.
128126
"""
129127
if not is_otel_capabilities_enabled(client_options):
130128
return None
131129

132-
return _get_otel_interceptor(client_options, is_async=True)
130+
# Ignored by mypy: Optional dependency only loaded if early-return is skipped
131+
import opentelemetry.instrumentation.grpc as otel_grpc # type: ignore[import-not-found]
132+
133+
return otel_grpc.aio_client_interceptors(
134+
tracer_provider=_get_tracer_provider(client_options)
135+
)

packages/google-api-core/tests/unit/test_observability.py

Lines changed: 41 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,15 @@
2323

2424

2525
def test_is_otel_capabilities_enabled_disabled(monkeypatch):
26+
"""Proves that is_otel_capabilities_enabled returns False when the tracing environment variable is disabled."""
2627
monkeypatch.setenv("GOOGLE_SDK_EXPERIMENTAL_PYTHON_TRACING_ENABLED", "false")
2728
assert not _observability.is_otel_capabilities_enabled()
2829

2930

3031
def test_is_otel_capabilities_enabled_otel_missing(monkeypatch):
32+
"""Proves that is_otel_capabilities_enabled returns False when tracing is enabled
33+
but OpenTelemetry gRPC instrumentation is not installed.
34+
"""
3135
monkeypatch.setenv("GOOGLE_SDK_EXPERIMENTAL_PYTHON_TRACING_ENABLED", "true")
3236
# Simulate OTel not being installed by blocking imports
3337
monkeypatch.setitem(sys.modules, "opentelemetry.instrumentation.grpc", None)
@@ -36,6 +40,9 @@ def test_is_otel_capabilities_enabled_otel_missing(monkeypatch):
3640

3741

3842
def test_is_otel_capabilities_enabled_otel_installed(monkeypatch):
43+
"""Proves that is_otel_capabilities_enabled returns True when tracing is enabled
44+
and OpenTelemetry gRPC instrumentation is installed.
45+
"""
3946
monkeypatch.setenv("GOOGLE_SDK_EXPERIMENTAL_PYTHON_TRACING_ENABLED", "true")
4047

4148
mock_otel = mock.Mock()
@@ -57,7 +64,7 @@ def test_is_otel_capabilities_enabled_experimental_requires_env_var(monkeypatch)
5764
env var set to 'true' raises FeatureGatingError (Fail Fast).
5865
"""
5966
monkeypatch.delenv("GOOGLE_SDK_EXPERIMENTAL_PYTHON_TRACING_ENABLED", raising=False)
60-
options = ClientOptions(tracer_provider=object())
67+
options = ClientOptions(tracer_provider=mock.Mock())
6168

6269
with pytest.raises(
6370
FeatureGatingError,
@@ -83,115 +90,54 @@ def test_is_otel_capabilities_enabled_experimental_enabled_with_config(monkeypat
8390
sys.modules, "opentelemetry.instrumentation.grpc", mock_otel_grpc
8491
)
8592

86-
options = ClientOptions(tracer_provider=object())
93+
options = ClientOptions(tracer_provider=mock.Mock())
8794
assert _observability.is_otel_capabilities_enabled(options)
8895

8996

90-
def test_get_otel_interceptor_sync_default(monkeypatch):
91-
mock_otel = mock.Mock()
92-
mock_otel_grpc = mock_otel.instrumentation.grpc
93-
mock_interceptor = mock.Mock()
94-
mock_otel_grpc.client_interceptor.return_value = mock_interceptor
95-
96-
monkeypatch.setitem(sys.modules, "opentelemetry", mock_otel)
97-
monkeypatch.setitem(
98-
sys.modules, "opentelemetry.instrumentation", mock_otel.instrumentation
99-
)
100-
monkeypatch.setitem(
101-
sys.modules, "opentelemetry.instrumentation.grpc", mock_otel_grpc
102-
)
103-
104-
result = _observability._get_otel_interceptor()
105-
assert result is mock_interceptor
106-
mock_otel_grpc.client_interceptor.assert_called_once_with(tracer_provider=None)
97+
def test_get_tracer_provider_default():
98+
"""Proves that _get_tracer_provider returns None when no client_options are supplied."""
99+
assert _observability._get_tracer_provider() is None
107100

108101

109-
def test_get_otel_interceptor_sync_config(monkeypatch):
110-
mock_tracer_provider = object()
102+
def test_get_tracer_provider_config():
103+
"""Proves that _get_tracer_provider extracts the tracer provider when supplied
104+
via a ClientOptions instance.
105+
"""
106+
mock_tracer_provider = mock.Mock()
111107
options = ClientOptions(tracer_provider=mock_tracer_provider)
108+
assert _observability._get_tracer_provider(options) is mock_tracer_provider
112109

113-
mock_otel = mock.Mock()
114-
mock_otel_grpc = mock_otel.instrumentation.grpc
115-
mock_interceptor = mock.Mock()
116-
mock_otel_grpc.client_interceptor.return_value = mock_interceptor
117-
118-
monkeypatch.setitem(sys.modules, "opentelemetry", mock_otel)
119-
monkeypatch.setitem(
120-
sys.modules, "opentelemetry.instrumentation", mock_otel.instrumentation
121-
)
122-
monkeypatch.setitem(
123-
sys.modules, "opentelemetry.instrumentation.grpc", mock_otel_grpc
124-
)
125-
126-
result = _observability._get_otel_interceptor(client_options=options)
127-
assert result is mock_interceptor
128-
mock_otel_grpc.client_interceptor.assert_called_once_with(
129-
tracer_provider=mock_tracer_provider
130-
)
131110

132-
133-
def test_get_otel_interceptor_sync_dict_config(monkeypatch):
134-
mock_tracer_provider = object()
111+
def test_get_tracer_provider_dict_config():
112+
"""Proves that _get_tracer_provider extracts the tracer provider when supplied
113+
via a dictionary configuration.
114+
"""
115+
mock_tracer_provider = mock.Mock()
135116
options = {"tracer_provider": mock_tracer_provider}
136-
137-
mock_otel = mock.Mock()
138-
mock_otel_grpc = mock_otel.instrumentation.grpc
139-
mock_interceptor = mock.Mock()
140-
mock_otel_grpc.client_interceptor.return_value = mock_interceptor
141-
142-
monkeypatch.setitem(sys.modules, "opentelemetry", mock_otel)
143-
monkeypatch.setitem(
144-
sys.modules, "opentelemetry.instrumentation", mock_otel.instrumentation
145-
)
146-
monkeypatch.setitem(
147-
sys.modules, "opentelemetry.instrumentation.grpc", mock_otel_grpc
148-
)
149-
150-
result = _observability._get_otel_interceptor(client_options=options)
151-
assert result is mock_interceptor
152-
mock_otel_grpc.client_interceptor.assert_called_once_with(
153-
tracer_provider=mock_tracer_provider
154-
)
155-
156-
157-
def test_get_otel_interceptor_async(monkeypatch):
158-
mock_tracer_provider = object()
159-
options = ClientOptions(tracer_provider=mock_tracer_provider)
160-
161-
mock_otel = mock.Mock()
162-
mock_otel_grpc = mock_otel.instrumentation.grpc
163-
mock_async_interceptors = [mock.Mock()]
164-
mock_otel_grpc.aio_client_interceptors.return_value = mock_async_interceptors
165-
166-
monkeypatch.setitem(sys.modules, "opentelemetry", mock_otel)
167-
monkeypatch.setitem(
168-
sys.modules, "opentelemetry.instrumentation", mock_otel.instrumentation
169-
)
170-
monkeypatch.setitem(
171-
sys.modules, "opentelemetry.instrumentation.grpc", mock_otel_grpc
172-
)
173-
174-
result = _observability._get_otel_interceptor(client_options=options, is_async=True)
175-
assert result is mock_async_interceptors
176-
mock_otel_grpc.aio_client_interceptors.assert_called_once_with(
177-
tracer_provider=mock_tracer_provider
178-
)
117+
assert _observability._get_tracer_provider(options) is mock_tracer_provider
179118

180119

181120
def test_get_otel_interceptor_disabled(monkeypatch):
121+
"""Proves that get_otel_interceptor returns None when tracing is disabled."""
182122
monkeypatch.setenv("GOOGLE_SDK_EXPERIMENTAL_PYTHON_TRACING_ENABLED", "false")
183123
assert _observability.get_otel_interceptor() is None
184124

185125

186126
def test_get_otel_interceptor_otel_missing(monkeypatch):
127+
"""Proves that get_otel_interceptor returns None when OpenTelemetry gRPC
128+
instrumentation is not installed.
129+
"""
187130
monkeypatch.setenv("GOOGLE_SDK_EXPERIMENTAL_PYTHON_TRACING_ENABLED", "true")
188131
monkeypatch.setitem(sys.modules, "opentelemetry.instrumentation.grpc", None)
189132
assert _observability.get_otel_interceptor() is None
190133

191134

192135
def test_get_otel_interceptor_enabled(monkeypatch):
136+
"""Proves that get_otel_interceptor creates a synchronous OpenTelemetry client
137+
interceptor with the resolved tracer provider and returns a channel-intercepting callable.
138+
"""
193139
monkeypatch.setenv("GOOGLE_SDK_EXPERIMENTAL_PYTHON_TRACING_ENABLED", "true")
194-
mock_tracer_provider = object()
140+
mock_tracer_provider = mock.Mock()
195141
options = ClientOptions(tracer_provider=mock_tracer_provider)
196142

197143
mock_raw_channel = mock.Mock(name="raw_channel")
@@ -232,7 +178,7 @@ def test_get_otel_interceptor_with_apply_channel_interceptors(monkeypatch):
232178
from google.api_core import grpc_helpers
233179

234180
monkeypatch.setenv("GOOGLE_SDK_EXPERIMENTAL_PYTHON_TRACING_ENABLED", "true")
235-
mock_tracer_provider = object()
181+
mock_tracer_provider = mock.Mock()
236182
options = ClientOptions(tracer_provider=mock_tracer_provider)
237183

238184
mock_raw_channel = mock.Mock(name="raw_channel")
@@ -266,19 +212,26 @@ def test_get_otel_interceptor_with_apply_channel_interceptors(monkeypatch):
266212

267213

268214
def test_get_otel_async_interceptor_disabled(monkeypatch):
215+
"""Proves that get_otel_async_interceptor returns None when tracing is disabled."""
269216
monkeypatch.setenv("GOOGLE_SDK_EXPERIMENTAL_PYTHON_TRACING_ENABLED", "false")
270217
assert _observability.get_otel_async_interceptor() is None
271218

272219

273220
def test_get_otel_async_interceptor_otel_missing(monkeypatch):
221+
"""Proves that get_otel_async_interceptor returns None when OpenTelemetry gRPC
222+
instrumentation is not installed.
223+
"""
274224
monkeypatch.setenv("GOOGLE_SDK_EXPERIMENTAL_PYTHON_TRACING_ENABLED", "true")
275225
monkeypatch.setitem(sys.modules, "opentelemetry.instrumentation.grpc", None)
276226
assert _observability.get_otel_async_interceptor() is None
277227

278228

279229
def test_get_otel_async_interceptor_enabled(monkeypatch):
230+
"""Proves that get_otel_async_interceptor instantiates and returns asynchronous
231+
OpenTelemetry client interceptors with the resolved tracer provider.
232+
"""
280233
monkeypatch.setenv("GOOGLE_SDK_EXPERIMENTAL_PYTHON_TRACING_ENABLED", "true")
281-
mock_tracer_provider = object()
234+
mock_tracer_provider = mock.Mock()
282235
options = ClientOptions(tracer_provider=mock_tracer_provider)
283236

284237
mock_async_interceptors = [mock.Mock(name="otel_async_interceptor")]

0 commit comments

Comments
 (0)