From 8b5c8633fdec8371ecb5a125608b27fe47f134fd Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Wed, 19 Feb 2025 16:02:45 +0000 Subject: [PATCH 1/4] feat: add client debug logging support for unary-stream REST calls --- .../%sub/services/%service/transports/rest.py.j2 | 8 +++++--- .../%sub/services/%service/transports/rest_asyncio.py.j2 | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 index d419a2d3c3..4f72e3d614 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 @@ -258,17 +258,20 @@ class {{service.name}}RestTransport(_Base{{ service.name }}RestTransport): resp = self._interceptor.post_{{ method.name|snake_case }}(resp) response_metadata = [(k, str(v)) for k, v in response.headers.items()] resp, _ = self._interceptor.post_{{ method.name|snake_case }}_with_metadata(resp, response_metadata) - {# TODO(https://github.com/googleapis/gapic-generator-python/issues/2279): Add logging support for rest streaming. #} - {% if not method.server_streaming %} if CLIENT_LOGGING_SUPPORTED and _LOGGER.isEnabledFor(logging.DEBUG): # pragma: NO COVER + {# Logging of response iterator is in google-api-core #} + {% if not method.server_streaming %} try: response_payload = {% if method.output.ident.is_proto_plus_type %}{{ method.output.ident }}.to_json(response){% else %}json_format.MessageToJson(resp){% endif %} except: {# TODO(https://github.com/googleapis/gapic-generator-python/issues/2283): Remove try/except once unit tests are updated. #} response_payload = None + {% endif %}{# if not method.server_streaming #} http_response = { + {% if not method.server_streaming %} "payload": response_payload, + {% endif %}{# if not method.server_streaming #} "headers": dict(response.headers), "status": response.status_code, } @@ -282,7 +285,6 @@ class {{service.name}}RestTransport(_Base{{ service.name }}RestTransport): "httpResponse": http_response, }, ) - {% endif %}{# if not method.server_streaming #} return resp {% endif %}{# method.void #} diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest_asyncio.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest_asyncio.py.j2 index 8664b7077d..74d17823d6 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest_asyncio.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest_asyncio.py.j2 @@ -219,17 +219,20 @@ class Async{{service.name}}RestTransport(_Base{{ service.name }}RestTransport): resp = await self._interceptor.post_{{ method.name|snake_case }}(resp) response_metadata = [(k, str(v)) for k, v in response.headers.items()] resp, _ = await self._interceptor.post_{{ method.name|snake_case }}_with_metadata(resp, response_metadata) - {# TODO(https://github.com/googleapis/gapic-generator-python/issues/2279): Add logging support for rest streaming. #} - {% if not method.server_streaming %} if CLIENT_LOGGING_SUPPORTED and _LOGGER.isEnabledFor(logging.DEBUG): # pragma: NO COVER + {# Logging of response iterator is in google-api-core #} + {% if not method.server_streaming %} try: response_payload = {% if method.output.ident.is_proto_plus_type %}{{ method.output.ident }}.to_json(response){% else %}json_format.MessageToJson(resp){% endif %} except: {# TODO(https://github.com/googleapis/gapic-generator-python/issues/2283): Remove try/except once unit tests are updated. #} response_payload = None + {% endif %}{# if not method.server_streaming #} http_response = { + {% if not method.server_streaming %} "payload": response_payload, + {% endif %}{# if not method.server_streaming #} "headers": dict(response.headers), "status": "OK", # need to obtain this properly } @@ -243,7 +246,6 @@ class Async{{service.name}}RestTransport(_Base{{ service.name }}RestTransport): }, ) - {% endif %}{# if not method.server_streaming #} return resp {% endif %}{# method.void #} From 6478506ae70e9a450497cb58785861d7f1218c8d Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Mon, 31 Mar 2025 09:54:46 +0000 Subject: [PATCH 2/4] add comment --- .../%sub/services/%service/transports/rest.py.j2 | 7 ++++++- .../%sub/services/%service/transports/rest_asyncio.py.j2 | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 index 4f72e3d614..fa3698e1ec 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 @@ -259,7 +259,9 @@ class {{service.name}}RestTransport(_Base{{ service.name }}RestTransport): response_metadata = [(k, str(v)) for k, v in response.headers.items()] resp, _ = self._interceptor.post_{{ method.name|snake_case }}_with_metadata(resp, response_metadata) if CLIENT_LOGGING_SUPPORTED and _LOGGER.isEnabledFor(logging.DEBUG): # pragma: NO COVER - {# Logging of response iterator is in google-api-core #} + {# Logging of the streaming response payload is in `google-api-core` because the response #} + {# type `rest_streaming.ResponseIterator` is an iterator. #} + {# Each response 'item' is logged at the time that it is fetched which happens in google-api-core. #} {% if not method.server_streaming %} try: response_payload = {% if method.output.ident.is_proto_plus_type %}{{ method.output.ident }}.to_json(response){% else %}json_format.MessageToJson(resp){% endif %} @@ -269,6 +271,9 @@ class {{service.name}}RestTransport(_Base{{ service.name }}RestTransport): response_payload = None {% endif %}{# if not method.server_streaming #} http_response = { + {# Logging of the streaming response payload is in `google-api-core` because the response #} + {# type `rest_streaming.ResponseIterator` is an iterator. #} + {# Each response 'item' is logged at the time that it is fetched which happens in google-api-core. #} {% if not method.server_streaming %} "payload": response_payload, {% endif %}{# if not method.server_streaming #} diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest_asyncio.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest_asyncio.py.j2 index 74d17823d6..cf18bad5aa 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest_asyncio.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest_asyncio.py.j2 @@ -220,7 +220,9 @@ class Async{{service.name}}RestTransport(_Base{{ service.name }}RestTransport): response_metadata = [(k, str(v)) for k, v in response.headers.items()] resp, _ = await self._interceptor.post_{{ method.name|snake_case }}_with_metadata(resp, response_metadata) if CLIENT_LOGGING_SUPPORTED and _LOGGER.isEnabledFor(logging.DEBUG): # pragma: NO COVER - {# Logging of response iterator is in google-api-core #} + {# Logging of the streaming response payload is in `google-api-core` because the response #} + {# type `rest_streaming_async.AsyncResponseIterator` is an iterator. #} + {# Each response 'item' is logged at the time that it is fetched which happens in google-api-core. #} {% if not method.server_streaming %} try: response_payload = {% if method.output.ident.is_proto_plus_type %}{{ method.output.ident }}.to_json(response){% else %}json_format.MessageToJson(resp){% endif %} @@ -230,6 +232,9 @@ class Async{{service.name}}RestTransport(_Base{{ service.name }}RestTransport): response_payload = None {% endif %}{# if not method.server_streaming #} http_response = { + {# Logging of the streaming response payload is in `google-api-core` because the response #} + {# type `rest_streaming_async.AsyncResponseIterator` is an iterator. #} + {# Each response 'item' is logged at the time that it is fetched which happens in google-api-core. #} {% if not method.server_streaming %} "payload": response_payload, {% endif %}{# if not method.server_streaming #} From 0dda26a70555745d4376cb1b797ab41c7b04dfc8 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Fri, 9 May 2025 09:53:49 -0400 Subject: [PATCH 3/4] Update gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 Co-authored-by: Victor Chudnovsky --- .../%sub/services/%service/transports/rest.py.j2 | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 index fa3698e1ec..01b39c9d76 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 @@ -271,9 +271,7 @@ class {{service.name}}RestTransport(_Base{{ service.name }}RestTransport): response_payload = None {% endif %}{# if not method.server_streaming #} http_response = { - {# Logging of the streaming response payload is in `google-api-core` because the response #} - {# type `rest_streaming.ResponseIterator` is an iterator. #} - {# Each response 'item' is logged at the time that it is fetched which happens in google-api-core. #} + {# Not logging server streaming here. See comment above. #} {% if not method.server_streaming %} "payload": response_payload, {% endif %}{# if not method.server_streaming #} From 58d6a89f9dee5522f59b7df271ace97cc4adde0d Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Fri, 9 May 2025 14:07:49 +0000 Subject: [PATCH 4/4] update comments --- .../%sub/services/%service/transports/rest.py.j2 | 9 +++++---- .../services/%service/transports/rest_asyncio.py.j2 | 11 +++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 index 01b39c9d76..f22dcc2fc2 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2 @@ -259,9 +259,10 @@ class {{service.name}}RestTransport(_Base{{ service.name }}RestTransport): response_metadata = [(k, str(v)) for k, v in response.headers.items()] resp, _ = self._interceptor.post_{{ method.name|snake_case }}_with_metadata(resp, response_metadata) if CLIENT_LOGGING_SUPPORTED and _LOGGER.isEnabledFor(logging.DEBUG): # pragma: NO COVER - {# Logging of the streaming response payload is in `google-api-core` because the response #} - {# type `rest_streaming.ResponseIterator` is an iterator. #} - {# Each response 'item' is logged at the time that it is fetched which happens in google-api-core. #} + {# TODO(https://github.com/googleapis/gapic-generator-python/issues/2389): #} + {# Depending how we want to log (a) receiving a streaming response vs (b) exposing the next streamed item to the user, we could possibly want to log something here #} + {# (a) should always happen in api-core #} + {# (b) could happen in api-core, or it could happen here when we iterate to the next streamed item that was previously received. #} {% if not method.server_streaming %} try: response_payload = {% if method.output.ident.is_proto_plus_type %}{{ method.output.ident }}.to_json(response){% else %}json_format.MessageToJson(resp){% endif %} @@ -271,7 +272,7 @@ class {{service.name}}RestTransport(_Base{{ service.name }}RestTransport): response_payload = None {% endif %}{# if not method.server_streaming #} http_response = { - {# Not logging server streaming here. See comment above. #} + {# Not logging response payload for server streaming here. See comment above. #} {% if not method.server_streaming %} "payload": response_payload, {% endif %}{# if not method.server_streaming #} diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest_asyncio.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest_asyncio.py.j2 index cf18bad5aa..cde618e750 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest_asyncio.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest_asyncio.py.j2 @@ -220,9 +220,10 @@ class Async{{service.name}}RestTransport(_Base{{ service.name }}RestTransport): response_metadata = [(k, str(v)) for k, v in response.headers.items()] resp, _ = await self._interceptor.post_{{ method.name|snake_case }}_with_metadata(resp, response_metadata) if CLIENT_LOGGING_SUPPORTED and _LOGGER.isEnabledFor(logging.DEBUG): # pragma: NO COVER - {# Logging of the streaming response payload is in `google-api-core` because the response #} - {# type `rest_streaming_async.AsyncResponseIterator` is an iterator. #} - {# Each response 'item' is logged at the time that it is fetched which happens in google-api-core. #} + {# TODO(https://github.com/googleapis/gapic-generator-python/issues/2389): #} + {# Depending how we want to log (a) receiving a streaming response vs (b) exposing the next streamed item to the user, we could possibly want to log something here #} + {# (a) should always happen in api-core #} + {# (b) could happen in api-core, or it could happen here when we iterate to the next streamed item that was previously received. #} {% if not method.server_streaming %} try: response_payload = {% if method.output.ident.is_proto_plus_type %}{{ method.output.ident }}.to_json(response){% else %}json_format.MessageToJson(resp){% endif %} @@ -232,9 +233,7 @@ class Async{{service.name}}RestTransport(_Base{{ service.name }}RestTransport): response_payload = None {% endif %}{# if not method.server_streaming #} http_response = { - {# Logging of the streaming response payload is in `google-api-core` because the response #} - {# type `rest_streaming_async.AsyncResponseIterator` is an iterator. #} - {# Each response 'item' is logged at the time that it is fetched which happens in google-api-core. #} + {# Not logging response payload for server streaming here. See comment above. #} {% if not method.server_streaming %} "payload": response_payload, {% endif %}{# if not method.server_streaming #}