Skip to content

Commit 70753b4

Browse files
ohmayrvchudnov-g
ohmayr
and
vchudnov-g
authored
chore: add test coverage for debug logging (#2281)
Co-authored-by: Victor Chudnovsky <vchudnov@google.com>
1 parent 1044768 commit 70753b4

32 files changed

Lines changed: 1425 additions & 282 deletions

File tree

‎packages/gapic-generator/.github/workflows/tests.yaml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ jobs:
5858
# TODO(https://github.com/googleapis/gapic-generator-python/issues/2121) Remove `showcase_w_rest_async` target when async rest is GA.
5959
python: ["3.7", "3.13"]
6060
target: [showcase, showcase_alternative_templates, showcase_w_rest_async]
61+
logging_scope: ["", "google"]
62+
6163
runs-on: ubuntu-latest
6264
steps:
6365
- uses: actions/checkout@v4
@@ -90,6 +92,9 @@ jobs:
9092
unzip protoc-${PROTOC_VERSION}.zip
9193
sudo ln -s /usr/src/protoc/bin/protoc /usr/local/bin/protoc
9294
- name: Run showcase tests.
95+
env:
96+
# TODO(https://github.com/googleapis/gapic-generator-python/issues/2286): Construct nox sessions with logging enabled.
97+
GOOGLE_SDK_PYTHON_LOGGING_SCOPE: ${{ matrix.logging_scope }}
9398
run: nox -s ${{ matrix.target }}-${{ matrix.python }}
9499
showcase-mtls:
95100
if: ${{ false }} # TODO(dovs): reenable when #1218 is fixed
@@ -143,6 +148,7 @@ jobs:
143148
python: ["3.7", "3.8", "3.9", "3.10", "3.11", "3.12", "3.13"]
144149
# TODO(https://github.com/googleapis/gapic-generator-python/issues/2121) Remove `_w_rest_async` variant when async rest is GA.
145150
variant: ['', _alternative_templates, _mixins, _alternative_templates_mixins, _w_rest_async]
151+
logging_scope: ["", "google"]
146152
runs-on: ubuntu-latest
147153
steps:
148154
- uses: actions/checkout@v4
@@ -166,6 +172,9 @@ jobs:
166172
- name: Install nox.
167173
run: python -m pip install nox
168174
- name: Run unit tests.
175+
env:
176+
# TODO(https://github.com/googleapis/gapic-generator-python/issues/2286): Construct nox sessions with logging enabled.
177+
GOOGLE_SDK_PYTHON_LOGGING_SCOPE: ${{ matrix.logging_scope }}
169178
run: nox -s showcase_unit${{ matrix.variant }}-${{ matrix.python }}
170179
showcase-unit-add-iam-methods:
171180
runs-on: ubuntu-latest

‎packages/gapic-generator/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/_test_mixins.py.j2‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ def test_{{ name|snake_case }}_rest_bad_request(transport: str = 'rest', request
1717
response_value.status_code = 400
1818
response_value.request = Request()
1919
req.return_value = response_value
20+
req.return_value.headers = {"header-1": "value-1", "header-2": "value-2"}
2021
client.{{ name|snake_case }}(request)
2122

2223
@pytest.mark.parametrize("request_type", [
@@ -50,6 +51,7 @@ def test_{{ name|snake_case }}_rest(request_type):
5051

5152
response_value._content = json_return_value.encode('UTF-8')
5253
req.return_value = response_value
54+
req.return_value.headers = {"header-1": "value-1", "header-2": "value-2"}
5355

5456
response = client.{{ name|snake_case }}(request)
5557

‎packages/gapic-generator/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1096,6 +1096,7 @@ def test_{{ method_name }}_rest(request_type, transport: str = 'rest'):
10961096
req.return_value = Response()
10971097
req.return_value.status_code = 500
10981098
req.return_value.request = PreparedRequest()
1099+
req.return_value.headers = {"header-1": "value-1", "header-2": "value-2"}
10991100
{% if method.void %}
11001101
json_return_value = ''
11011102
{% elif method.server_streaming %}
@@ -1249,6 +1250,7 @@ def test_{{ method.name|snake_case }}_rest(request_type):
12491250

12501251
response_value._content = json_return_value.encode('UTF-8')
12511252
req.return_value = response_value
1253+
req.return_value.headers = {"header-1": "value-1", "header-2": "value-2"}
12521254
{% if method.client_streaming %}
12531255
response = client.{{ method.safe_name|snake_case }}(iter(requests))
12541256
{% elif method.server_streaming %}
@@ -1495,6 +1497,7 @@ def test_{{ method_name }}_rest_interceptors(null_interceptor):
14951497
}
14961498

14971499
req.return_value = Response()
1500+
req.return_value.headers = {"header-1": "value-1", "header-2": "value-2"}
14981501
req.return_value.status_code = 200
14991502
req.return_value.request = PreparedRequest()
15001503
{% if not method.void %}
@@ -1545,6 +1548,7 @@ def test_{{ method_name }}_rest_bad_request(transport: str = 'rest', request_typ
15451548
response_value.status_code = 400
15461549
response_value.request = Request()
15471550
req.return_value = response_value
1551+
req.return_value.headers = {"header-1": "value-1", "header-2": "value-2"}
15481552
{% if method.client_streaming %}
15491553
client.{{ method.safe_name|snake_case }}(iter(requests))
15501554
{% else %}

‎packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/_shared_macros.j2‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ def _get_http_options():
197197
method_name (str): The method name.
198198
service: The service.
199199
is_async (bool): Used to determine the code path i.e. whether for sync or async call. #}
200-
{% macro rest_call_method_common(body_spec, method_name, service, is_async=False) %}
200+
{% macro rest_call_method_common(body_spec, method_name, service, is_async=False, is_proto_plus_type=False) %}
201201
{% set service_name = service.name %}
202202
{% set await_prefix = "await " if is_async else "" %}
203203
{% set async_class_prefix = "Async" if is_async else "" %}
@@ -217,8 +217,14 @@ def _get_http_options():
217217
if CLIENT_LOGGING_SUPPORTED and _LOGGER.isEnabledFor(logging.DEBUG): # pragma: NO COVER
218218
request_url = "{host}{uri}".format(host=self._host, uri=transcoded_request['uri'])
219219
method = transcoded_request['method']
220+
try:
221+
request_payload = {% if is_proto_plus_type %}type(request).to_json(request){% else %}json_format.MessageToJson(request){% endif %}
222+
223+
except:
224+
{# TODO(https://github.com/googleapis/gapic-generator-python/issues/2282): Remove try/except and correctly parse request payload. #}
225+
request_payload = None
220226
http_request = {
221-
"payload": type(request).to_json(request),
227+
"payload": request_payload,
222228
"requestMethod": method,
223229
"requestUrl": request_url,
224230
"headers": dict(metadata),
@@ -470,8 +476,13 @@ class _{{ name }}(_Base{{ service.name }}RestTransport._Base{{name}}, {{ async_m
470476
resp = json_format.Parse(content, resp)
471477
resp = {{ await_prefix }}self._interceptor.post_{{ name|snake_case }}(resp)
472478
if CLIENT_LOGGING_SUPPORTED and _LOGGER.isEnabledFor(logging.DEBUG): # pragma: NO COVER
479+
try:
480+
response_payload = json_format.MessageToJson(resp)
481+
except:
482+
{# TODO(https://github.com/googleapis/gapic-generator-python/issues/2283): Remove try/except once unit tests are updated. #}
483+
response_payload = None
473484
http_response = {
474-
"payload": json_format.MessageToJson(response),
485+
"payload": response_payload,
475486
"headers": dict(response.headers),
476487
"status": response.status_code,
477488
}

‎packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,10 @@ from .transports.base import {{ service.name }}Transport, DEFAULT_CLIENT_INFO
5353
from .transports.grpc_asyncio import {{ service.grpc_asyncio_transport_name }}
5454
from .client import {{ service.client_name }}
5555

56-
try: # pragma: NO COVER
56+
try:
5757
from google.api_core import client_logging # type: ignore
58-
CLIENT_LOGGING_SUPPORTED = True
59-
except ImportError:
58+
CLIENT_LOGGING_SUPPORTED = True # pragma: NO COVER
59+
except ImportError: # pragma: NO COVER
6060
CLIENT_LOGGING_SUPPORTED = False
6161

6262
_LOGGER = std_logging.getLogger(__name__)
@@ -257,9 +257,12 @@ class {{ service.async_client_name }}:
257257
extra = {
258258
"serviceName": "{{ service.meta.address.proto }}",
259259
"universeDomain": getattr(self._client._transport._credentials, "universe_domain", ""),
260-
"credentialType": f"{type(self._client._transport._credentials).__module__}.{type(self._client._transport._credentials).__qualname__}",
261-
"credentialInfo": getattr(self.transport._credentials, "get_cred_info", lambda: None)(),
262-
},
260+
"credentialsType": f"{type(self._client._transport._credentials).__module__}.{type(self._client._transport._credentials).__qualname__}",
261+
"credentialsInfo": getattr(self.transport._credentials, "get_cred_info", lambda: None)(),
262+
} if hasattr(self._client._transport, "_credentials") else {
263+
"serviceName": "{{ service.meta.address.proto }}",
264+
"credentialsType": None,
265+
}
263266
)
264267

265268
{% for method in service.methods.values() %}

‎packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ try:
4040
except AttributeError: # pragma: NO COVER
4141
OptionalRetry = Union[retries.Retry, object, None] # type: ignore
4242

43-
try: # pragma: NO COVER
43+
try:
4444
from google.api_core import client_logging # type: ignore
45-
CLIENT_LOGGING_SUPPORTED = True
46-
except ImportError:
45+
CLIENT_LOGGING_SUPPORTED = True # pragma: NO COVER
46+
except ImportError: # pragma: NO COVER
4747
CLIENT_LOGGING_SUPPORTED = False
4848

4949
_LOGGER = std_logging.getLogger(__name__)
@@ -619,7 +619,10 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
619619
"universeDomain": getattr(self._transport._credentials, "universe_domain", ""),
620620
"credentialsType": f"{type(self._transport._credentials).__module__}.{type(self._transport._credentials).__qualname__}",
621621
"credentialsInfo": getattr(self.transport._credentials, "get_cred_info", lambda: None)(),
622-
},
622+
} if hasattr(self._transport, "_credentials") else {
623+
"serviceName": "{{ service.meta.address.proto }}",
624+
"credentialsType": None,
625+
}
623626
)
624627

625628

‎packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,10 @@ try:
4141
except AttributeError: # pragma: NO COVER
4242
OptionalRetry = Union[retries.Retry, object, None] # type: ignore
4343

44-
try: # pragma: NO COVER
44+
try:
4545
from google.api_core import client_logging # type: ignore
46-
CLIENT_LOGGING_SUPPORTED = True
47-
except ImportError:
46+
CLIENT_LOGGING_SUPPORTED = True # pragma: NO COVER
47+
except ImportError: # pragma: NO COVER
4848
CLIENT_LOGGING_SUPPORTED = False
4949

5050
_LOGGER = logging.getLogger(__name__)
@@ -235,7 +235,7 @@ class {{service.name}}RestTransport(_Base{{ service.name }}RestTransport):
235235
{% endif %}
236236
"""
237237

238-
{{ shared_macros.rest_call_method_common(body_spec, method.name, service)|indent(8) }}
238+
{{ shared_macros.rest_call_method_common(body_spec, method.name, service, False, method.output.ident.is_proto_plus_type)|indent(8) }}
239239

240240
{% if not method.void %}
241241
# Return the response
@@ -259,8 +259,14 @@ class {{service.name}}RestTransport(_Base{{ service.name }}RestTransport):
259259
{# TODO(https://github.com/googleapis/gapic-generator-python/issues/2279): Add logging support for rest streaming. #}
260260
{% if not method.server_streaming %}
261261
if CLIENT_LOGGING_SUPPORTED and _LOGGER.isEnabledFor(logging.DEBUG): # pragma: NO COVER
262+
try:
263+
response_payload = {% if method.output.ident.is_proto_plus_type %}{{ method.output.ident }}.to_json(response){% else %}json_format.MessageToJson(resp){% endif %}
264+
265+
except:
266+
{# TODO(https://github.com/googleapis/gapic-generator-python/issues/2283): Remove try/except once unit tests are updated. #}
267+
response_payload = None
262268
http_response = {
263-
"payload": {% if method.output.ident.is_proto_plus_type %}{{ method.output.ident }}.to_json(resp){% else %}json_format.MessageToJson(resp){% endif %},
269+
"payload": response_payload,
264270
"headers": dict(response.headers),
265271
"status": response.status_code,
266272
}

‎packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest_asyncio.py.j2‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,10 @@ from .base import DEFAULT_CLIENT_INFO as BASE_DEFAULT_CLIENT_INFO
6161

6262
import logging
6363

64-
try: # pragma: NO COVER
64+
try:
6565
from google.api_core import client_logging # type: ignore
66-
CLIENT_LOGGING_SUPPORTED = True
67-
except ImportError:
66+
CLIENT_LOGGING_SUPPORTED = True # pragma: NO COVER
67+
except ImportError: # pragma: NO COVER
6868
CLIENT_LOGGING_SUPPORTED = False
6969

7070
_LOGGER = logging.getLogger(__name__)
@@ -199,7 +199,7 @@ class Async{{service.name}}RestTransport(_Base{{ service.name }}RestTransport):
199199
{% endif %}
200200
"""
201201

202-
{{ shared_macros.rest_call_method_common(body_spec, method.name, service, is_async=True)|indent(8) }}
202+
{{ shared_macros.rest_call_method_common(body_spec, method.name, service, True, method.output.ident.is_proto_plus_type)|indent(8) }}
203203

204204
{% if not method.void %}
205205
# Return the response
@@ -220,8 +220,14 @@ class Async{{service.name}}RestTransport(_Base{{ service.name }}RestTransport):
220220
{# TODO(https://github.com/googleapis/gapic-generator-python/issues/2279): Add logging support for rest streaming. #}
221221
{% if not method.server_streaming %}
222222
if CLIENT_LOGGING_SUPPORTED and _LOGGER.isEnabledFor(logging.DEBUG): # pragma: NO COVER
223+
try:
224+
response_payload = {% if method.output.ident.is_proto_plus_type %}{{ method.output.ident }}.to_json(response){% else %}json_format.MessageToJson(resp){% endif %}
225+
226+
except:
227+
{# TODO(https://github.com/googleapis/gapic-generator-python/issues/2283): Remove try/except once unit tests are updated. #}
228+
response_payload = None
223229
http_response = {
224-
"payload": {% if method.output.ident.is_proto_plus_type %}{{ method.output.ident }}.to_json(response){% else %}json_format.MessageToJson(response){% endif %},
230+
"payload": response_payload,
225231
"headers": str(dict(response.headers)),
226232
"status": "OK", # need to obtain this properly
227233
}

‎packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_macros.j2‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,6 +1171,7 @@ def test_{{ method_name }}_rest_required_fields(request_type={{ method.input.ide
11711171

11721172
response_value._content = json_return_value.encode('UTF-8')
11731173
req.return_value = response_value
1174+
req.return_value.headers = {"header-1": "value-1", "header-2": "value-2"}
11741175

11751176
{% if method.client_streaming %}
11761177
response = client.{{ method_name }}(iter(requests))
@@ -1263,6 +1264,7 @@ def test_{{ method_name }}_rest_flattened():
12631264
{% endif %}
12641265
response_value._content = json_return_value.encode('UTF-8')
12651266
req.return_value = response_value
1267+
req.return_value.headers = {"header-1": "value-1", "header-2": "value-2"}
12661268

12671269
{% if method.server_streaming %}
12681270
with mock.patch.object(response_value, 'iter_content') as iter_content:
@@ -1796,6 +1798,7 @@ def test_initialize_client_w_{{transport_name}}():
17961798
response_value.status_code = 400
17971799
response_value.request = mock.Mock()
17981800
req.return_value = response_value
1801+
req.return_value.headers = {"header-1": "value-1", "header-2": "value-2"}
17991802
{{ await_prefix }}client.{{ method_name }}(request)
18001803

18011804
{% endif %}{# if 'grpc' in transport #}
@@ -1846,6 +1849,7 @@ def test_initialize_client_w_{{transport_name}}():
18461849
response_value.request = Request()
18471850
{% endif %}
18481851
req.return_value = response_value
1852+
req.return_value.headers = {"header-1": "value-1", "header-2": "value-2"}
18491853
{{ await_prefix }}client.{{ method_name }}(request)
18501854
{% endif %}{# if 'grpc' in transport #}
18511855
{% endmacro %}
@@ -2029,6 +2033,7 @@ def test_initialize_client_w_{{transport_name}}():
20292033
{% endif %}{# is_async #}
20302034
{% endif %}{# method.server_streaming #}
20312035
req.return_value = response_value
2036+
req.return_value.headers = {"header-1": "value-1", "header-2": "value-2"}
20322037
response = {{ await_prefix }}client.{{ method_name }}(request)
20332038
{% if "next_page_token" in method_output.fields.values()|map(attribute='name', default="") and not method.paged_result_field %}
20342039
{# TODO(https://github.com/googleapis/gapic-generator-python/issues/2199): The following assert statement is added to force
@@ -2139,6 +2144,7 @@ def test_initialize_client_w_{{transport_name}}():
21392144
{% endif %}
21402145

21412146
req.return_value = response_value
2147+
req.return_value.headers = {"header-1": "value-1", "header-2": "value-2"}
21422148

21432149
response = {{ await_prefix }}client.{{ method_name }}(request)
21442150

@@ -2232,6 +2238,7 @@ def test_initialize_client_w_{{transport_name}}():
22322238

22332239
req.return_value = mock.Mock()
22342240
req.return_value.status_code = 200
2241+
req.return_value.headers = {"header-1": "value-1", "header-2": "value-2"}
22352242
{% if not method.void %}
22362243
return_value = {% if method.output.ident.is_proto_plus_type %}{{ method.output.ident }}.to_json({{ method.output.ident }}()){% else %}json_format.MessageToJson({{ method.output.ident }}()){% endif %}
22372244

‎packages/gapic-generator/tests/integration/goldens/asset/google/cloud/asset_v1/services/asset_service/async_client.py‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,10 @@
4747
from .transports.grpc_asyncio import AssetServiceGrpcAsyncIOTransport
4848
from .client import AssetServiceClient
4949

50-
try: # pragma: NO COVER
50+
try:
5151
from google.api_core import client_logging # type: ignore
52-
CLIENT_LOGGING_SUPPORTED = True
53-
except ImportError:
52+
CLIENT_LOGGING_SUPPORTED = True # pragma: NO COVER
53+
except ImportError: # pragma: NO COVER
5454
CLIENT_LOGGING_SUPPORTED = False
5555

5656
_LOGGER = std_logging.getLogger(__name__)
@@ -261,9 +261,12 @@ def __init__(self, *,
261261
extra = {
262262
"serviceName": "google.cloud.asset.v1.AssetService",
263263
"universeDomain": getattr(self._client._transport._credentials, "universe_domain", ""),
264-
"credentialType": f"{type(self._client._transport._credentials).__module__}.{type(self._client._transport._credentials).__qualname__}",
265-
"credentialInfo": getattr(self.transport._credentials, "get_cred_info", lambda: None)(),
266-
},
264+
"credentialsType": f"{type(self._client._transport._credentials).__module__}.{type(self._client._transport._credentials).__qualname__}",
265+
"credentialsInfo": getattr(self.transport._credentials, "get_cred_info", lambda: None)(),
266+
} if hasattr(self._client._transport, "_credentials") else {
267+
"serviceName": "google.cloud.asset.v1.AssetService",
268+
"credentialsType": None,
269+
}
267270
)
268271

269272
async def export_assets(self,

0 commit comments

Comments
 (0)