Skip to content

fix(gapic-generator): [Approach 1] thread default_host to OTel helpers instead of mutating client_options #18933

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
chalmerlowe wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

fix(gapic-generator): [Approach 1] thread default_host to OTel helpers instead of mutating client_options #18933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ad65d4c
fix(gapic-generator): default client_options.api_endpoint to transpor…
chalmerlowe Oct 27, 2026
81e79e2
fix(gapic-generator): shallow-copy client_options and handle None whe…
chalmerlowe Oct 30, 2026
8ac1e81
test(gapic-generator): assert transport._client_options in otel chann…
chalmerlowe Oct 30, 2026
b4d0ba1
refactor(observability): thread default_host explicitly instead of mu…
chalmerlowe Oct 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,15 @@ ASYNC_WRAP_METHOD_SUPPORTS_KIND = WRAP_METHOD_SUPPORTS_TRACING or hasattr(
gapic_v1.method_async, "_DEFAULT_ASYNC_TRANSPORT_KIND"
)

# The `default_host` parameter in gapic_v1.method.wrap_method,
# gapic_v1.method_async.wrap_method, _observability.get_otel_interceptor, and
# _observability.apply_otel_async_channel_interceptors allows transports to pass
# their resolved default host without mutating caller-owned client_options.
SUPPORTS_DEFAULT_HOST = bool(
getattr(gapic_v1.method.wrap_method, "__kwdefaults__", None)
and "default_host" in gapic_v1.method.wrap_method.__kwdefaults__
)
Comment on lines +106 to +109

@gemini-code-assist gemini-code-assist Bot Oct 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current check for default_host support relies on __kwdefaults__. However, default_host is defined as a standard positional-or-keyword argument (not a keyword-only argument) in gapic_v1.method.wrap_method. Therefore, __kwdefaults__ will be None, and SUPPORTS_DEFAULT_HOST will always evaluate to False, silently disabling the feature in generated clients.\n\nUsing inspect.signature is a robust and standard way to check if the parameter is supported.

import inspect\n\nSUPPORTS_DEFAULT_HOST = \"default_host\" in inspect.signature(gapic_v1.method.wrap_method).parameters


try:
from google.api_core import grpc_helpers
except ImportError: # pragma: NO COVER
Expand Down
15 changes: 13 additions & 2 deletions ...or/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/base.py.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ from typing import {% if service.any_extended_operations_methods %}Any, {% endif
from {{package_path}} import gapic_version as package_version
from {{package_path}}._compat import (
ASYNC_WRAP_METHOD_SUPPORTS_KIND as _ASYNC_WRAP_METHOD_SUPPORTS_KIND,
SUPPORTS_DEFAULT_HOST as _SUPPORTS_DEFAULT_HOST,
WRAP_METHOD_SUPPORTS_TRACING as _WRAP_METHOD_SUPPORTS_TRACING,
)
_ASYNC_WRAP_METHOD_SUPPORTS_TRACING = _WRAP_METHOD_SUPPORTS_TRACING
Expand Down Expand Up @@ -162,7 +163,13 @@ class {{ service.name }}Transport(abc.ABC):
host += ':443'
self._host = host

if isinstance(client_options, dict):
client_options = client_options_lib.from_dict(client_options)

self._client_options = client_options
self._default_host: Optional[str] = (
None if self._ignore_credentials else self._host
)
self._wrapped_methods: Dict[Callable, Callable] = {}

@property
Expand All @@ -181,6 +188,8 @@ class {{ service.name }}Transport(abc.ABC):
"""
if _WRAP_METHOD_SUPPORTS_TRACING:
kwargs["client_options"] = self._client_options
if _SUPPORTS_DEFAULT_HOST:
kwargs["default_host"] = self._default_host
try:
kind = self.kind
except NotImplementedError:
Expand All @@ -193,7 +202,7 @@ class {{ service.name }}Transport(abc.ABC):
# - Era 2 (>= 2.29.0, < 2.36.0): `kind` is supported (prevents REST from
# falling back to gRPC wrapping), but tracing args are not.
# (These fallbacks can be removed once google-api-core < 2.36.0 is no longer supported.)
for k in ["client_options", "method_name", "is_streaming"]:
for k in ["client_options", "method_name", "is_streaming", "default_host"]:
kwargs.pop(k, None)

try:
Expand All @@ -220,6 +229,8 @@ class {{ service.name }}Transport(abc.ABC):
"""
if _ASYNC_WRAP_METHOD_SUPPORTS_TRACING:
kwargs["client_options"] = self._client_options
if _SUPPORTS_DEFAULT_HOST:
kwargs["default_host"] = self._default_host
try:
kind = self.kind
except NotImplementedError:
Expand All @@ -232,7 +243,7 @@ class {{ service.name }}Transport(abc.ABC):
# - Era 2 (>= 2.29.0, < 2.36.0): `kind` is supported (prevents REST from
# falling back to gRPC wrapping), but tracing args are not.
# (These fallbacks can be removed once google-api-core < 2.36.0 is no longer supported.)
for k in ["client_options", "method_name", "is_streaming"]:
for k in ["client_options", "method_name", "is_streaming", "default_host"]:
kwargs.pop(k, None)

try:
Expand Down
11 changes: 9 additions & 2 deletions ...or/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ from typing import Callable, Dict, Optional, Sequence, Tuple, Union
from typing import TYPE_CHECKING

{% set package_path = api.naming.module_namespace|join('.') + "." + api.naming.versioned_module_name %}
from {{package_path}}._compat import _observability, apply_channel_interceptors
from {{package_path}}._compat import (
_observability,
SUPPORTS_DEFAULT_HOST as _SUPPORTS_DEFAULT_HOST,
apply_channel_interceptors,
)

from google.api_core import client_options as client_options_lib
{% if service.has_resumable_upload_methods %}
Expand Down Expand Up @@ -287,9 +291,12 @@ class {{ service.name }}GrpcTransport({{ service.name }}Transport):
)

channel_interceptors = list(interceptors) if interceptors else []
_otel_interceptor_kwargs = (
{"default_host": self._default_host} if _SUPPORTS_DEFAULT_HOST else {}
)
if (
_observability is not None
and (otel_interceptor := _observability.get_otel_interceptor(self._client_options)) is not None
and (otel_interceptor := _observability.get_otel_interceptor(self._client_options, **_otel_interceptor_kwargs)) is not None
and otel_interceptor not in channel_interceptors
and not any(getattr(i, "_is_otel_interceptor", None) is True for i in channel_interceptors)
):
Expand Down
11 changes: 9 additions & 2 deletions .../templates/%namespace/%name_%version/%sub/services/%service/transports/grpc_asyncio.py.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ from typing import {% if service.has_resumable_upload_methods %}Any, {% endif %}
from typing import TYPE_CHECKING

{% set package_path = api.naming.module_namespace|join('.') + "." + api.naming.versioned_module_name %}
from {{package_path}}._compat import _observability, apply_async_channel_interceptors
from {{package_path}}._compat import (
_observability,
SUPPORTS_DEFAULT_HOST as _SUPPORTS_DEFAULT_HOST,
apply_async_channel_interceptors,
)

from google.api_core import client_options as client_options_lib
from google.api_core import gapic_v1
Expand Down Expand Up @@ -349,8 +353,11 @@ class {{ service.grpc_asyncio_transport_name }}({{ service.name }}Transport):
# NOTE: Coverage tool ignores async interceptors in environments running
# legacy google-api-core (< 2.36.0) where OpenTelemetry is unavailable.
# TODO (Issue #18657): Remove once lowest constraints require google-api-core >= v2.43.0.
_otel_interceptor_kwargs = (
{"default_host": self._default_host} if _SUPPORTS_DEFAULT_HOST else {}
) # pragma: NO COVER
self._grpc_channel = _observability.apply_otel_async_channel_interceptors(
self._grpc_channel, self._client_options
self._grpc_channel, self._client_options, **_otel_interceptor_kwargs
) # pragma: NO COVER

self._logged_channel = self._grpc_channel
Expand Down
111 changes: 105 additions & 6 deletions .../gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -996,7 +996,14 @@ def test_{{ service.name|snake_case }}_grpc_transport_otel_channel_interceptor()
client_options=options,
)

mock_get_interceptor.assert_called_once_with(options)
expected_kwargs = (
{"default_host": transport._default_host}
if transports.grpc._SUPPORTS_DEFAULT_HOST
else {}
)
mock_get_interceptor.assert_called_once_with(
transport._client_options, **expected_kwargs
)
mock_apply_interceptors.assert_called_once_with(
mock_channel, [mock_otel_interceptor]
)
Expand Down Expand Up @@ -1070,7 +1077,14 @@ def test_{{ service.name|snake_case }}_grpc_asyncio_transport_otel_channel_inter
client_options=options,
)

mock_apply_async_interceptors.assert_called_once_with(mock_channel, options)
expected_kwargs = (
{"default_host": transport._default_host}
if transports.grpc_asyncio._SUPPORTS_DEFAULT_HOST
else {}
)
mock_apply_async_interceptors.assert_called_once_with(
mock_channel, transport._client_options, **expected_kwargs
)
assert mock_create_channel.call_count == 1
assert transport.grpc_channel == mock_channel

Expand Down Expand Up @@ -2477,7 +2491,8 @@ def test_api_key_credentials(client_class, transport_class):

def test_{{ service.name|snake_case }}_base_transport_wrap_method_modern_api_core(monkeypatch):
monkeypatch.setattr(transports.base, "_WRAP_METHOD_SUPPORTS_TRACING", True)
# 1. Concrete transport: passes client_options=None and kind="{{ opts.transport[0] }}"
monkeypatch.setattr(transports.base, "_SUPPORTS_DEFAULT_HOST", True)
# 1. Concrete transport: passes client_options=None, default_host, and kind="{{ opts.transport[0] }}"
with mock.patch.object(transports.base.gapic_v1.method, "wrap_method") as mock_wrap:
transport = transports.{{ service.name }}{{ opts.transport[0].capitalize() }}Transport(
credentials=ga_credentials.AnonymousCredentials()
Expand All @@ -2486,8 +2501,10 @@ def test_{{ service.name|snake_case }}_base_transport_wrap_method_modern_api_cor
transport._wrap_method(func)
assert mock_wrap.call_args.kwargs["kind"] == "{{ opts.transport[0] }}"
assert "client_options" in mock_wrap.call_args.kwargs
assert mock_wrap.call_args.kwargs["default_host"] == transport._default_host

# 2. Base transport: kind raises NotImplementedError and is cleanly omitted
# 2. Base transport with _SUPPORTS_DEFAULT_HOST=False: kind raises NotImplementedError and default_host is omitted
monkeypatch.setattr(transports.base, "_SUPPORTS_DEFAULT_HOST", False)
with mock.patch.object(transports.base.gapic_v1.method, "wrap_method") as mock_wrap:
base_transport = transports.{{ service.name }}Transport(
credentials=ga_credentials.AnonymousCredentials()
Expand All @@ -2496,6 +2513,84 @@ def test_{{ service.name|snake_case }}_base_transport_wrap_method_modern_api_cor
base_transport._wrap_method(func)
assert "kind" not in mock_wrap.call_args.kwargs
assert "client_options" in mock_wrap.call_args.kwargs
assert "default_host" not in mock_wrap.call_args.kwargs


def test_{{ service.name|snake_case }}_base_transport_client_options_none():
transport = transports.{{ service.name }}Transport(
host="default.example.com:443",
credentials=ga_credentials.AnonymousCredentials(),
client_options=None,
)
assert transport._client_options is None
assert transport._default_host == "default.example.com:443"


@pytest.mark.parametrize(
"initial_endpoint",
[
pytest.param(None, id="unset_api_endpoint"),
pytest.param("custom.example.com:443", id="explicit_api_endpoint"),
],
)
def test_{{ service.name|snake_case }}_base_transport_client_options_object(
initial_endpoint,
):
options = client_options.ClientOptions(api_endpoint=initial_endpoint)
transport = transports.{{ service.name }}Transport(
host="default.example.com:443",
credentials=ga_credentials.AnonymousCredentials(),
client_options=options,
)
assert transport._client_options is options
assert transport._client_options.api_endpoint == initial_endpoint
assert transport._default_host == "default.example.com:443"


@pytest.mark.parametrize(
"initial_options,expected_endpoint",
[
pytest.param(
{},
None,
id="unset_api_endpoint",
),
pytest.param(
{"api_endpoint": "custom.example.com:443"},
"custom.example.com:443",
id="explicit_api_endpoint",
),
],
)
def test_{{ service.name|snake_case }}_base_transport_client_options_dict(
initial_options, expected_endpoint
):
options = dict(initial_options)
transport = transports.{{ service.name }}Transport(
host="default.example.com:443",
credentials=ga_credentials.AnonymousCredentials(),
client_options=options,
)
assert isinstance(transport._client_options, client_options.ClientOptions)
assert transport._client_options.api_endpoint == expected_endpoint
assert transport._default_host == "default.example.com:443"
assert options == initial_options


def test_{{ service.name|snake_case }}_base_transport_default_host_ignored_when_ignore_credentials():
class _CustomChannelTransport(transports.{{ service.name }}Transport):
def __init__(self, **kwargs):
self._ignore_credentials = True
super().__init__(**kwargs)

options = client_options.ClientOptions()
transport = _CustomChannelTransport(
host="default.example.com:443",
client_options=options,
)
assert transport._client_options is options
assert transport._client_options.api_endpoint is None
assert transport._default_host is None


def test_{{ service.name|snake_case }}_base_transport_wrap_method_older_api_core_fallbacks(monkeypatch):
Expand Down Expand Up @@ -2575,7 +2670,8 @@ def test_{{ service.name|snake_case }}_base_transport_wrap_method_older_api_core
{% if 'grpc' in opts.transport %}
def test_{{ service.name|snake_case }}_base_transport_wrap_async_method_modern_api_core(monkeypatch):
monkeypatch.setattr(transports.base, "_ASYNC_WRAP_METHOD_SUPPORTS_TRACING", True)
# 1. Concrete transport: passes client_options=None and kind="grpc_asyncio"
monkeypatch.setattr(transports.base, "_SUPPORTS_DEFAULT_HOST", True)
# 1. Concrete transport: passes client_options=None, default_host, and kind="grpc_asyncio"
with mock.patch.object(transports.base.gapic_v1.method_async, "wrap_method") as mock_wrap:
transport = transports.{{ service.grpc_asyncio_transport_name }}(
credentials=ga_credentials.AnonymousCredentials()
Expand All @@ -2584,8 +2680,10 @@ def test_{{ service.name|snake_case }}_base_transport_wrap_async_method_modern_a
transport._wrap_async_method(func)
assert mock_wrap.call_args.kwargs["kind"] == "grpc_asyncio"
assert "client_options" in mock_wrap.call_args.kwargs
assert mock_wrap.call_args.kwargs["default_host"] == transport._default_host

# 2. Base transport: kind raises NotImplementedError and is cleanly omitted
# 2. Base transport with _SUPPORTS_DEFAULT_HOST=False: kind raises NotImplementedError and default_host is omitted
monkeypatch.setattr(transports.base, "_SUPPORTS_DEFAULT_HOST", False)
with mock.patch.object(transports.base.gapic_v1.method_async, "wrap_method") as mock_wrap:
base_transport = transports.{{ service.name }}Transport(
credentials=ga_credentials.AnonymousCredentials()
Expand All @@ -2594,6 +2692,7 @@ def test_{{ service.name|snake_case }}_base_transport_wrap_async_method_modern_a
base_transport._wrap_async_method(func)
assert "kind" not in mock_wrap.call_args.kwargs
assert "client_options" in mock_wrap.call_args.kwargs
assert "default_host" not in mock_wrap.call_args.kwargs


def test_{{ service.name|snake_case }}_base_transport_wrap_async_method_older_api_core_fallbacks(monkeypatch):
Expand Down
9 changes: 9 additions & 0 deletions packages/gapic-generator/tests/integration/goldens/asset/google/cloud/asset_v1/_compat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,15 @@ def trace_http_request(*args: Any, **kwargs: Any) -> _FallbackTraceContext: # t
gapic_v1.method_async, "_DEFAULT_ASYNC_TRANSPORT_KIND"
)

# The `default_host` parameter in gapic_v1.method.wrap_method,
# gapic_v1.method_async.wrap_method, _observability.get_otel_interceptor, and
# _observability.apply_otel_async_channel_interceptors allows transports to pass
# their resolved default host without mutating caller-owned client_options.
SUPPORTS_DEFAULT_HOST = bool(
getattr(gapic_v1.method.wrap_method, "__kwdefaults__", None)
and "default_host" in gapic_v1.method.wrap_method.__kwdefaults__
)

try:
from google.api_core import grpc_helpers
except ImportError: # pragma: NO COVER
Expand Down
15 changes: 13 additions & 2 deletions ...integration/goldens/asset/google/cloud/asset_v1/services/asset_service/transports/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from google.cloud.asset_v1 import gapic_version as package_version
from google.cloud.asset_v1._compat import (
ASYNC_WRAP_METHOD_SUPPORTS_KIND as _ASYNC_WRAP_METHOD_SUPPORTS_KIND,
SUPPORTS_DEFAULT_HOST as _SUPPORTS_DEFAULT_HOST,
WRAP_METHOD_SUPPORTS_TRACING as _WRAP_METHOD_SUPPORTS_TRACING,
)
_ASYNC_WRAP_METHOD_SUPPORTS_TRACING = _WRAP_METHOD_SUPPORTS_TRACING
Expand Down Expand Up @@ -133,7 +134,13 @@ def __init__(
host += ':443'
self._host = host

if isinstance(client_options, dict):
client_options = client_options_lib.from_dict(client_options)

self._client_options = client_options
self._default_host: Optional[str] = (
None if self._ignore_credentials else self._host
)
self._wrapped_methods: Dict[Callable, Callable] = {}

@property
Expand All @@ -152,6 +159,8 @@ def _wrap_method(self, func, *args, **kwargs):
"""
if _WRAP_METHOD_SUPPORTS_TRACING:
kwargs["client_options"] = self._client_options
if _SUPPORTS_DEFAULT_HOST:
kwargs["default_host"] = self._default_host
try:
kind = self.kind
except NotImplementedError:
Expand All @@ -164,7 +173,7 @@ def _wrap_method(self, func, *args, **kwargs):
# - Era 2 (>= 2.29.0, < 2.36.0): `kind` is supported (prevents REST from
# falling back to gRPC wrapping), but tracing args are not.
# (These fallbacks can be removed once google-api-core < 2.36.0 is no longer supported.)
for k in ["client_options", "method_name", "is_streaming"]:
for k in ["client_options", "method_name", "is_streaming", "default_host"]:
kwargs.pop(k, None)

try:
Expand All @@ -191,6 +200,8 @@ def _wrap_async_method(self, func, *args, **kwargs):
"""
if _ASYNC_WRAP_METHOD_SUPPORTS_TRACING:
kwargs["client_options"] = self._client_options
if _SUPPORTS_DEFAULT_HOST:
kwargs["default_host"] = self._default_host
try:
kind = self.kind
except NotImplementedError:
Expand All @@ -203,7 +214,7 @@ def _wrap_async_method(self, func, *args, **kwargs):
# - Era 2 (>= 2.29.0, < 2.36.0): `kind` is supported (prevents REST from
# falling back to gRPC wrapping), but tracing args are not.
# (These fallbacks can be removed once google-api-core < 2.36.0 is no longer supported.)
for k in ["client_options", "method_name", "is_streaming"]:
for k in ["client_options", "method_name", "is_streaming", "default_host"]:
kwargs.pop(k, None)

try:
Expand Down
11 changes: 9 additions & 2 deletions ...integration/goldens/asset/google/cloud/asset_v1/services/asset_service/transports/grpc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@
from typing import Callable, Dict, Optional, Sequence, Tuple, Union
from typing import TYPE_CHECKING

from google.cloud.asset_v1._compat import _observability, apply_channel_interceptors
from google.cloud.asset_v1._compat import (
_observability,
SUPPORTS_DEFAULT_HOST as _SUPPORTS_DEFAULT_HOST,
apply_channel_interceptors,
)

from google.api_core import client_options as client_options_lib
from google.api_core import grpc_helpers
Expand Down Expand Up @@ -277,9 +281,12 @@ def __init__(self, *,
)

channel_interceptors = list(interceptors) if interceptors else []
_otel_interceptor_kwargs = (
{"default_host": self._default_host} if _SUPPORTS_DEFAULT_HOST else {}
)
if (
_observability is not None
and (otel_interceptor := _observability.get_otel_interceptor(self._client_options)) is not None
and (otel_interceptor := _observability.get_otel_interceptor(self._client_options, **_otel_interceptor_kwargs)) is not None
and otel_interceptor not in channel_interceptors
and not any(getattr(i, "_is_otel_interceptor", None) is True for i in channel_interceptors)
):
Expand Down
Loading
Loading