From 696200e17ec2a610ea8f8dac8d43a06a507345a3 Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Thu, 27 Aug 2026 13:17:02 -0400 Subject: [PATCH 1/3] feat(secretmanager): integrate eager channel creation and interceptor application - Use _observability.create_channel_with_otel in SecretManagerServiceClient - Use grpc_helpers.apply_interceptors in SecretManagerServiceGrpcTransport - Add unit tests for channel injection and interceptor wiring --- .../services/secret_manager_service/client.py | 44 ++++++---- .../secret_manager_service/transports/grpc.py | 12 ++- .../google-cloud-secret-manager/noxfile.py | 4 +- packages/google-cloud-secret-manager/setup.py | 2 +- .../testing/constraints-3.10.txt | 2 +- .../test_secret_manager_service.py | 80 ++++++++++++++++++- 6 files changed, 122 insertions(+), 22 deletions(-) diff --git a/packages/google-cloud-secret-manager/google/cloud/secretmanager_v1/services/secret_manager_service/client.py b/packages/google-cloud-secret-manager/google/cloud/secretmanager_v1/services/secret_manager_service/client.py index ff26bddcc57d..d909680cafc5 100644 --- a/packages/google-cloud-secret-manager/google/cloud/secretmanager_v1/services/secret_manager_service/client.py +++ b/packages/google-cloud-secret-manager/google/cloud/secretmanager_v1/services/secret_manager_service/client.py @@ -35,17 +35,16 @@ ) import google.protobuf +from google.api_core import _observability, gapic_v1 from google.api_core import client_options as client_options_lib from google.api_core import exceptions as core_exceptions -from google.api_core import gapic_v1 from google.api_core import retry as retries from google.auth import credentials as ga_credentials # type: ignore from google.auth.exceptions import MutualTLSChannelError # type: ignore from google.auth.transport import mtls # type: ignore from google.auth.transport.grpc import SslCredentials # type: ignore -from google.oauth2 import service_account # type: ignore - from google.cloud.secretmanager_v1 import gapic_version as package_version +from google.oauth2 import service_account # type: ignore try: OptionalRetry = Union[retries.Retry, gapic_v1.method._MethodDefault, None] @@ -68,7 +67,6 @@ import google.protobuf.field_mask_pb2 as field_mask_pb2 # type: ignore import google.protobuf.timestamp_pb2 as timestamp_pb2 # type: ignore from google.cloud.location import locations_pb2 # type: ignore - from google.cloud.secretmanager_v1.services.secret_manager_service import pagers from google.cloud.secretmanager_v1.types import resources, service @@ -746,17 +744,33 @@ def __init__( else cast(Callable[..., SecretManagerServiceTransport], transport) ) # initialize with the provided callable or the passed in class - self._transport = transport_init( - credentials=credentials, - credentials_file=self._client_options.credentials_file, - host=self._api_endpoint, - scopes=self._client_options.scopes, - client_cert_source_for_mtls=self._client_cert_source, - quota_project_id=self._client_options.quota_project_id, - client_info=client_info, - always_use_jwt_access=True, - api_audience=self._client_options.api_audience, - ) + transport_kwargs = { + "credentials": credentials, + "credentials_file": self._client_options.credentials_file, + "host": self._api_endpoint, + "scopes": self._client_options.scopes, + "client_cert_source_for_mtls": self._client_cert_source, + "quota_project_id": self._client_options.quota_project_id, + "client_info": client_info, + "always_use_jwt_access": True, + "api_audience": self._client_options.api_audience, + } + + if transport_init is SecretManagerServiceGrpcTransport: + if _observability.is_otel_capabilities_enabled(self._client_options): + transport_kwargs["channel"] = ( + _observability.create_channel_with_otel( + SecretManagerServiceGrpcTransport.create_channel, + client_options=self._client_options, + host=self._api_endpoint, + credentials=credentials, + credentials_file=self._client_options.credentials_file, + scopes=self._client_options.scopes, + quota_project_id=self._client_options.quota_project_id, + ) + ) + + self._transport = transport_init(**transport_kwargs) if "async" not in str(self._transport): if CLIENT_LOGGING_SUPPORTED and _LOGGER.isEnabledFor( diff --git a/packages/google-cloud-secret-manager/google/cloud/secretmanager_v1/services/secret_manager_service/transports/grpc.py b/packages/google-cloud-secret-manager/google/cloud/secretmanager_v1/services/secret_manager_service/transports/grpc.py index 51530553e705..6dcebc3da558 100644 --- a/packages/google-cloud-secret-manager/google/cloud/secretmanager_v1/services/secret_manager_service/transports/grpc.py +++ b/packages/google-cloud-secret-manager/google/cloud/secretmanager_v1/services/secret_manager_service/transports/grpc.py @@ -27,12 +27,12 @@ import grpc # type: ignore import proto # type: ignore from google.api_core import gapic_v1, grpc_helpers +from google.api_core.grpc_helpers import ClientInterceptor from google.auth import credentials as ga_credentials # type: ignore from google.auth.transport.grpc import SslCredentials # type: ignore from google.cloud.location import locations_pb2 # type: ignore -from google.protobuf.json_format import MessageToJson - from google.cloud.secretmanager_v1.types import resources, service +from google.protobuf.json_format import MessageToJson from .base import DEFAULT_CLIENT_INFO, SecretManagerServiceTransport @@ -148,6 +148,7 @@ def __init__( client_info: gapic_v1.client_info.ClientInfo = DEFAULT_CLIENT_INFO, always_use_jwt_access: Optional[bool] = False, api_audience: Optional[str] = None, + interceptors: Optional[Sequence[ClientInterceptor]] = None, ) -> None: """Instantiate the transport. @@ -198,6 +199,9 @@ def __init__( to the service that will be set when using certain 3rd party authentication flows. Audience is typically a resource identifier. If not set, the host value will be used as a default. + interceptors (Optional[Sequence[ClientInterceptor]]): + Additional interceptors to be injected into the gRPC channel pipeline. + These are executed in order. Raises: google.auth.exceptions.MutualTLSChannelError: If mutual TLS transport @@ -274,6 +278,10 @@ def __init__( ], ) + self._grpc_channel = grpc_helpers.apply_interceptors( + self._grpc_channel, interceptors + ) + self._interceptor = _LoggingClientInterceptor() self._logged_channel = grpc.intercept_channel( self._grpc_channel, self._interceptor diff --git a/packages/google-cloud-secret-manager/noxfile.py b/packages/google-cloud-secret-manager/noxfile.py index 3943f9aea974..edc95c3289fb 100644 --- a/packages/google-cloud-secret-manager/noxfile.py +++ b/packages/google-cloud-secret-manager/noxfile.py @@ -71,7 +71,9 @@ "pytest-asyncio", ] UNIT_TEST_EXTERNAL_DEPENDENCIES: List[str] = [] -UNIT_TEST_LOCAL_DEPENDENCIES: List[str] = [] +UNIT_TEST_LOCAL_DEPENDENCIES: List[str] = [ + "../google-api-core[tracing,testing]", +] UNIT_TEST_DEPENDENCIES: List[str] = [] UNIT_TEST_EXTRAS: List[str] = [] UNIT_TEST_EXTRAS_BY_PYTHON: Dict[str, List[str]] = {} diff --git a/packages/google-cloud-secret-manager/setup.py b/packages/google-cloud-secret-manager/setup.py index 69abc90c64cb..551996a45c94 100644 --- a/packages/google-cloud-secret-manager/setup.py +++ b/packages/google-cloud-secret-manager/setup.py @@ -44,7 +44,7 @@ release_status = "Development Status :: 5 - Production/Stable" dependencies = [ - "google-api-core[grpc] >= 2.25.0, <3.0.0", + "google-api-core[grpc] >= 2.35.0, <3.0.0", # Exclude incompatible versions of `google-auth` # See https://github.com/googleapis/google-cloud-python/issues/12364 "google-auth >= 2.14.1, <3.0.0,!=2.24.0,!=2.25.0", diff --git a/packages/google-cloud-secret-manager/testing/constraints-3.10.txt b/packages/google-cloud-secret-manager/testing/constraints-3.10.txt index 0ce4b3d6e6f5..9e261ce48fe5 100644 --- a/packages/google-cloud-secret-manager/testing/constraints-3.10.txt +++ b/packages/google-cloud-secret-manager/testing/constraints-3.10.txt @@ -4,7 +4,7 @@ # pinning their versions to their lower bounds. # For example, if setup.py has "google-cloud-foo >= 1.14.0, < 2.0.0", # then this file should have google-cloud-foo==1.14.0 -google-api-core==2.25.0 +google-api-core==2.35.0 google-auth==2.14.1 grpcio==1.59.0 proto-plus==1.26.1 diff --git a/packages/google-cloud-secret-manager/tests/unit/gapic/secretmanager_v1/test_secret_manager_service.py b/packages/google-cloud-secret-manager/tests/unit/gapic/secretmanager_v1/test_secret_manager_service.py index 722ac1109a06..62b578d879e0 100644 --- a/packages/google-cloud-secret-manager/tests/unit/gapic/secretmanager_v1/test_secret_manager_service.py +++ b/packages/google-cloud-secret-manager/tests/unit/gapic/secretmanager_v1/test_secret_manager_service.py @@ -61,8 +61,6 @@ from google.auth import credentials as ga_credentials from google.auth.exceptions import MutualTLSChannelError from google.cloud.location import locations_pb2 -from google.oauth2 import service_account - from google.cloud.secretmanager_v1.services.secret_manager_service import ( SecretManagerServiceAsyncClient, SecretManagerServiceClient, @@ -70,6 +68,7 @@ transports, ) from google.cloud.secretmanager_v1.types import resources, service +from google.oauth2 import service_account CRED_INFO_JSON = { "credential_source": "/path/to/file", @@ -770,6 +769,83 @@ def test_secret_manager_service_client_client_options( ) +def test_secret_manager_service_client_otel_channel_injection_enabled(): + mock_wrapped_channel = mock.Mock() + + with ( + mock.patch( + "google.cloud.secretmanager_v1.services.secret_manager_service.client._observability.is_otel_capabilities_enabled", + return_value=True, + ) as mock_is_enabled, + mock.patch( + "google.cloud.secretmanager_v1.services.secret_manager_service.client._observability.create_channel_with_otel", + return_value=mock_wrapped_channel, + ) as mock_create_channel_with_otel, + mock.patch.object( + transports.SecretManagerServiceGrpcTransport, "__init__", return_value=None + ) as patched_transport_init, + ): + client = SecretManagerServiceClient(transport="grpc") + + mock_is_enabled.assert_called_once() + mock_create_channel_with_otel.assert_called_once_with( + transports.SecretManagerServiceGrpcTransport.create_channel, + client_options=client._client_options, + host=client._api_endpoint, + credentials=None, + credentials_file=None, + scopes=None, + quota_project_id=None, + ) + called_kwargs = patched_transport_init.call_args.kwargs + assert called_kwargs.get("channel") is mock_wrapped_channel + + +def test_secret_manager_service_client_otel_channel_injection_disabled(): + with ( + mock.patch( + "google.cloud.secretmanager_v1.services.secret_manager_service.client._observability.is_otel_capabilities_enabled", + return_value=False, + ) as mock_is_enabled, + mock.patch( + "google.cloud.secretmanager_v1.services.secret_manager_service.client._observability.create_channel_with_otel", + ) as mock_create_channel_with_otel, + mock.patch.object( + transports.SecretManagerServiceGrpcTransport, "__init__", return_value=None + ) as patched_transport_init, + ): + SecretManagerServiceClient(transport="grpc") + + mock_is_enabled.assert_called_once() + mock_create_channel_with_otel.assert_not_called() + called_kwargs = patched_transport_init.call_args.kwargs + assert "channel" not in called_kwargs + + +def test_secret_manager_service_grpc_transport_interceptors(): + mock_interceptor = mock.Mock() + mock_channel = mock.Mock() + + with ( + mock.patch.object( + transports.SecretManagerServiceGrpcTransport, + "create_channel", + return_value=mock_channel, + ), + mock.patch( + "google.api_core.grpc_helpers.apply_interceptors", + return_value=mock_channel, + ) as mock_apply_interceptors, + ): + transport = transports.SecretManagerServiceGrpcTransport( + interceptors=[mock_interceptor], + ) + + mock_apply_interceptors.assert_called_once_with( + mock_channel, [mock_interceptor] + ) + + @pytest.mark.parametrize( "client_class,transport_class,transport_name,use_client_cert_env", [ From f2b6829e1ece71740bec74f5c4a5b14d78e88d24 Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Thu, 27 Aug 2026 13:56:53 -0400 Subject: [PATCH 2/3] docs(secretmanager): add descriptive docstrings to OTel and transport unit tests --- .../test_secret_manager_service.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/google-cloud-secret-manager/tests/unit/gapic/secretmanager_v1/test_secret_manager_service.py b/packages/google-cloud-secret-manager/tests/unit/gapic/secretmanager_v1/test_secret_manager_service.py index 62b578d879e0..3e3e57851a96 100644 --- a/packages/google-cloud-secret-manager/tests/unit/gapic/secretmanager_v1/test_secret_manager_service.py +++ b/packages/google-cloud-secret-manager/tests/unit/gapic/secretmanager_v1/test_secret_manager_service.py @@ -770,6 +770,15 @@ def test_secret_manager_service_client_client_options( def test_secret_manager_service_client_otel_channel_injection_enabled(): + """Proves that when OpenTelemetry tracing is enabled: + + 1. SecretManagerServiceClient detects the feature flag via + _observability.is_otel_capabilities_enabled. + 2. The client eagerly invokes _observability.create_channel_with_otel with + SecretManagerServiceGrpcTransport.create_channel and client configuration. + 3. The eagerly created and wrapped OTel channel is injected into the transport's + constructor kwargs under the 'channel' key. + """ mock_wrapped_channel = mock.Mock() with ( @@ -802,6 +811,13 @@ def test_secret_manager_service_client_otel_channel_injection_enabled(): def test_secret_manager_service_client_otel_channel_injection_disabled(): + """Proves that when OpenTelemetry tracing is disabled: + + 1. SecretManagerServiceClient checks the feature flag and finds it disabled. + 2. Eager channel creation via _observability.create_channel_with_otel is skipped. + 3. No 'channel' argument is passed to the transport constructor, preserving lazy + channel initialization in the transport. + """ with ( mock.patch( "google.cloud.secretmanager_v1.services.secret_manager_service.client._observability.is_otel_capabilities_enabled", @@ -823,6 +839,10 @@ def test_secret_manager_service_client_otel_channel_injection_disabled(): def test_secret_manager_service_grpc_transport_interceptors(): + """Proves that SecretManagerServiceGrpcTransport accepts custom client interceptors + and invokes grpc_helpers.apply_interceptors to inject them into the underlying + gRPC channel pipeline. + """ mock_interceptor = mock.Mock() mock_channel = mock.Mock() From 2669ca4f5a9c700be144e76944da7d8c35ddf02f Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Fri, 28 Aug 2026 09:19:06 -0400 Subject: [PATCH 3/3] feat(secretmanager): pass lazy partial channel factory when OTel tracing enabled - Use functools.partial to bind create_channel_with_otel with Transport.create_channel and client_options - Eliminate manual extraction of host, credentials, scopes, and quota_project_id in client - Update unit tests to verify functools.partial factory binding and lazy transport kwargs --- .../services/secret_manager_service/client.py | 19 +++++------ .../test_secret_manager_service.py | 34 ++++++++----------- 2 files changed, 24 insertions(+), 29 deletions(-) diff --git a/packages/google-cloud-secret-manager/google/cloud/secretmanager_v1/services/secret_manager_service/client.py b/packages/google-cloud-secret-manager/google/cloud/secretmanager_v1/services/secret_manager_service/client.py index d909680cafc5..4a6f1775f772 100644 --- a/packages/google-cloud-secret-manager/google/cloud/secretmanager_v1/services/secret_manager_service/client.py +++ b/packages/google-cloud-secret-manager/google/cloud/secretmanager_v1/services/secret_manager_service/client.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. # +import functools import json import logging as std_logging import os @@ -756,18 +757,16 @@ def __init__( "api_audience": self._client_options.api_audience, } + # When OpenTelemetry tracing is enabled, bind create_channel_with_otel + # using functools.partial and pass it as the channel factory. + # This preserves lazy channel instantiation inside the Transport and avoids + # duplicating channel initialization arguments here in the client. if transport_init is SecretManagerServiceGrpcTransport: if _observability.is_otel_capabilities_enabled(self._client_options): - transport_kwargs["channel"] = ( - _observability.create_channel_with_otel( - SecretManagerServiceGrpcTransport.create_channel, - client_options=self._client_options, - host=self._api_endpoint, - credentials=credentials, - credentials_file=self._client_options.credentials_file, - scopes=self._client_options.scopes, - quota_project_id=self._client_options.quota_project_id, - ) + transport_kwargs["channel"] = functools.partial( + _observability.create_channel_with_otel, + SecretManagerServiceGrpcTransport.create_channel, + client_options=self._client_options, ) self._transport = transport_init(**transport_kwargs) diff --git a/packages/google-cloud-secret-manager/tests/unit/gapic/secretmanager_v1/test_secret_manager_service.py b/packages/google-cloud-secret-manager/tests/unit/gapic/secretmanager_v1/test_secret_manager_service.py index 3e3e57851a96..37d10d0abc38 100644 --- a/packages/google-cloud-secret-manager/tests/unit/gapic/secretmanager_v1/test_secret_manager_service.py +++ b/packages/google-cloud-secret-manager/tests/unit/gapic/secretmanager_v1/test_secret_manager_service.py @@ -14,6 +14,7 @@ # limitations under the License. # import asyncio +import functools import json import math import os @@ -774,22 +775,16 @@ def test_secret_manager_service_client_otel_channel_injection_enabled(): 1. SecretManagerServiceClient detects the feature flag via _observability.is_otel_capabilities_enabled. - 2. The client eagerly invokes _observability.create_channel_with_otel with - SecretManagerServiceGrpcTransport.create_channel and client configuration. - 3. The eagerly created and wrapped OTel channel is injected into the transport's - constructor kwargs under the 'channel' key. + 2. The client binds _observability.create_channel_with_otel using + functools.partial with SecretManagerServiceGrpcTransport.create_channel and client_options. + 3. The bound channel factory callable is passed into transport kwargs under 'channel', + allowing the Transport to initialize the channel lazily with its own parameters. """ - mock_wrapped_channel = mock.Mock() - with ( mock.patch( "google.cloud.secretmanager_v1.services.secret_manager_service.client._observability.is_otel_capabilities_enabled", return_value=True, ) as mock_is_enabled, - mock.patch( - "google.cloud.secretmanager_v1.services.secret_manager_service.client._observability.create_channel_with_otel", - return_value=mock_wrapped_channel, - ) as mock_create_channel_with_otel, mock.patch.object( transports.SecretManagerServiceGrpcTransport, "__init__", return_value=None ) as patched_transport_init, @@ -797,17 +792,18 @@ def test_secret_manager_service_client_otel_channel_injection_enabled(): client = SecretManagerServiceClient(transport="grpc") mock_is_enabled.assert_called_once() - mock_create_channel_with_otel.assert_called_once_with( + called_kwargs = patched_transport_init.call_args.kwargs + assert "channel" in called_kwargs + channel_factory = called_kwargs["channel"] + assert isinstance(channel_factory, functools.partial) + assert ( + channel_factory.func + is google.cloud.secretmanager_v1.services.secret_manager_service.client._observability.create_channel_with_otel + ) + assert channel_factory.args == ( transports.SecretManagerServiceGrpcTransport.create_channel, - client_options=client._client_options, - host=client._api_endpoint, - credentials=None, - credentials_file=None, - scopes=None, - quota_project_id=None, ) - called_kwargs = patched_transport_init.call_args.kwargs - assert called_kwargs.get("channel") is mock_wrapped_channel + assert channel_factory.keywords == {"client_options": client._client_options} def test_secret_manager_service_client_otel_channel_injection_disabled():