Add request/response debug logging by parthea · Pull Request #2266 · googleapis/gapic-generator-python

parthea · 2024-11-29T12:31:51Z

This PR was built on top of #2265

Example debug logs

{"timestamp": "2024-11-29 12:30:05,731", "severity": "DEBUG", "name": "google.showcase_v1beta1.services.echo.client", "message": "Created client `google.showcase_v1beta1.EchoClient`.", "serviceName": "google.showcase.v1beta1.Echo", "universeDomain": "googleapis.com", "credentialType": "google.oauth2.credentials.Credentials", "credentialInfo": {"credential_source": "REDACTED", "credential_type": "user credentials"}}

{"timestamp": "2024-11-29 12:30:07,220", "severity": "DEBUG", "name": "google.showcase_v1beta1.services.echo.client", "message": "Sending request via google.showcase_v1beta1.EchoClient.echo", "rpcName": "Echo", "serviceName": "google.showcase.v1beta1.Echo", "request": "{\n  \"content\": \"The hail in Wales falls mainly on the snails.\",\n  \"requestId\": \"some_value\",\n  \"otherRequestId\": \"\",\n  \"severity\": 0,\n  \"header\": \"\",\n  \"otherHeader\": \"\"\n}", "retryAttempt": 1, "metadata": "{'some-key-bin': b'some_value', 'x-goog-api-version': 'v1_20240408'}"}

…nt client is successfully constructed.

ohmayr · 2024-12-02T22:54:56Z

gapic/templates/%namespace/%name_%version/%sub/services/%service/_shared_macros.j2

+                "serviceName": "{{ service.meta.address.proto }}",
+                "rpcName": "{{ method_name }}",
+                "retryAttempt": 1,
+                "request": type(request).to_json(request),


Does this request / metadata include request method and request url? We can extract that information out from the http_options defined above.

No, I didn't include request method or url. We need to add those fields to google-api-core first
https://github.com/googleapis/python-api-core/blob/d18d9b5131162b44eebcc0859a7aca1198a2ac06/google/api_core/client_logging.py#L13-L24

Do we want the fields to just be part of metadata or request? In that case, adding these fields in google-api-core may not be required.

Adding a TODO comment here linking to an issue would be helpful to understand what's missing.

The structured logging fields include httpRequest which includes requestMethod and requestUrl.

So we should populate those subfields of httpRequest, which is already registered in api-core.

ohmayr · 2024-12-02T23:00:00Z

gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2

        if CLIENT_LOGGING_SUPPORTED and _LOGGER.isEnabledFor(std_logging.DEBUG):  # pragma: NO COVER
            # TODO: Make this performant when logging is not enabled

+            credential_info = None


Suggested change

credential_info = None

ohmayr · 2024-12-02T23:00:10Z

gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2

        if "async" not in str(self._transport):
            if CLIENT_LOGGING_SUPPORTED and _LOGGER.isEnabledFor(std_logging.DEBUG):  # pragma: NO COVER

+                credential_info = None


Suggested change

credential_info = None

ohmayr · 2024-12-02T23:01:31Z

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2

+try:
+    from google.api_core import client_logging
+    CLIENT_LOGGING_SUPPORTED = True
+except ImportError:
+    CLIENT_LOGGING_SUPPORTED = False


Suggested change

try:

from google.api_core import client_logging

CLIENT_LOGGING_SUPPORTED = True

except ImportError:

CLIENT_LOGGING_SUPPORTED = False

try: # pragma: NO COVER

from google.api_core import client_logging. # type: ignore

CLIENT_LOGGING_SUPPORTED = True

except ImportError:

CLIENT_LOGGING_SUPPORTED = False

ohmayr · 2024-12-02T23:02:22Z

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2

+import logging
+_LOGGER = logging.getLogger(__name__)


Suggested change

import logging

_LOGGER = logging.getLogger(__name__)

import logging as std_logging

_LOGGER = std_logging.getLogger(__name__)

We don't need std_logging here because we don't directly import any generated types in this file

parthea · 2024-12-03T12:30:15Z

The REST specific changes were moved to a new PR: #2270

vchudnov-g · 2024-12-03T00:03:22Z

gapic/templates/%namespace/%name_%version/%sub/services/%service/_shared_macros.j2

        is_async (bool): Used to determine the code path i.e. whether for sync or async call. #}
-{% macro rest_call_method_common(body_spec, method_name, service_name, is_async=False) %}
+{% macro rest_call_method_common(body_spec, method_name, service, is_async=False) %}
+{% set service_name = service.name %}


nit: Does using service_name instead of service.name in the template buy us anything? They read the same, they're the same number of characters. Does service change in the course of this macro?

vchudnov-g · 2024-12-04T20:37:04Z

gapic/templates/%namespace/%name_%version/%sub/services/%service/_shared_macros.j2

+                "serviceName": "{{ service.meta.address.proto }}",
+                "rpcName": "{{ method_name }}",
+                "retryAttempt": 1,
+                "request": type(request).to_json(request),


The structured logging fields include httpRequest which includes requestMethod and requestUrl.

So we should populate those subfields of httpRequest, which is already registered in api-core.

vchudnov-g · 2024-12-04T20:48:41Z

Is this PR obsolete in favor of #2270 and #2271 ?

ohmayr and others added 8 commits November 27, 2024 15:44

feat: add support for debug logging

323df6d

Resolve issue where initialize_logging is not defined

14dd73b

remove logging for sync client in async rest code path

c4696b3

Remove log for sync client in async code path

6f2bbee

Created client google.showcase_v1beta1.EchoClient instead of EchoClie…

e227a64

…nt client is successfully constructed.

Add logging for 'serviceName'

a715854

Add universe domain, credential type, credential info

1140abc

feat: add debug log when sending request

18cda88

product-auto-label bot added the size: s Pull request size is small. label Nov 29, 2024

parthea added 2 commits November 29, 2024 15:36

fix build

2115834

fix build

bcf2b5b

parthea changed the title ~~Add debug log when sending request~~ Add request/response debug logging Nov 29, 2024

add response logging

93ef3ff

product-auto-label bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Nov 29, 2024

parthea and others added 12 commits November 29, 2024 17:47

update response logging

9ed9b86

Merge branch 'main' into initialize-logging

a1198d5

add isEnabledFor check

c5e325b

skip test coverage

60dd6f2

minor fixes

3b818fa

skip coverage

61f4b4b

fix metadata test case

001390d

add metadata test

98ab1f0

update creds info logic

cf8cbb8

ignore missing type for client_logging

44bb2f0

use alias for std logging lib

1fc665d

add rest/grpc transport logging

ca19b36

product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Dec 2, 2024

remove blank space

1466400

Merge branch 'initialize-logging' into add-debug-log-sending-request

458c475

ohmayr reviewed Dec 2, 2024

View reviewed changes

Base automatically changed from initialize-logging to main December 3, 2024 11:32

parthea mentioned this pull request Dec 3, 2024

feat: add debug log when sending requests via REST #2270

Merged

vchudnov-g reviewed Dec 4, 2024

View reviewed changes

ohmayr closed this Dec 11, 2024

Conversation

parthea commented Nov 29, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

parthea commented Dec 3, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vchudnov-g commented Dec 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants