Skip to content

fix(gapic-generator): avoid build log payloads when no handlers are attached - #19065

Open
daniel-sanche wants to merge 4 commits into
mainfrom
fix_logging_has_handlers_gapic-generator_15335
Open

daniel-sanche wants to merge 4 commits into
mainfrom
fix_logging_has_handlers_gapic-generator_15335

Conversation

@daniel-sanche

Copy link
Copy Markdown
Contributor

Recently, api_core added opt-in debug logging. Currently, the logging interceptor builds log payloads on each rpc. If no loggers are attached, the logs are discarded. This can be costly (example: datastore query going from 6 to 20 seconds)

This PR adds an extra clause to the logging check, to skip creating logs if no handlers are attached

According to a benchmark, this takes a reference datastore query back from /9.23s back to baseline 1.38

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request updates the client logging logic across the GAPIC generator templates and generated golden files by adding a check to ensure that the logger has active handlers (_LOGGER.hasHandlers()) before logging is enabled. It also introduces corresponding unit tests to verify that both synchronous and asynchronous gRPC logging interceptors require handlers to be present. I have no additional feedback to provide as the changes are consistent and well-tested.

@daniel-sanche
daniel-sanche marked this pull request as ready for review November 13, 2026 04:45
@daniel-sanche
daniel-sanche requested a review from a team as a code owner November 13, 2026 04:45

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant