perf(gax-grpc): skip metadata materialization when logging is disabled#12810
perf(gax-grpc): skip metadata materialization when logging is disabled#12810
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates GrpcLoggingInterceptor to check if logging is enabled before recording RPC and response headers, preventing unnecessary metadata materialization. The review feedback suggests ensuring test isolation in GrpcLoggingInterceptorTest by resetting the global logging state to true in the tearDown method, as the current implementation could negatively impact other tests.
|
|
||
| @AfterEach | ||
| void tearDown() throws Exception { | ||
| setLoggingEnabled(false); |
There was a problem hiding this comment.
The tearDown method sets the global logging state to false. Since LoggingUtils likely stores this state in a static field, this change will persist and potentially affect other tests in the same JVM. If other tests (such as testInterceptor_basic) expect logging to be enabled by default, they may fail or skip important logic when executed after this test. It is recommended to reset the state to true (or the original value) to ensure test isolation.
| setLoggingEnabled(false); | |
| setLoggingEnabled(true); |
84c3a05 to
a6ad914
Compare
Summary
GrpcLoggingInterceptor was eagerly converting gRPC metadata into maps even when gax logging was disabled. That meant every RPC still paid metadata iteration/allocation cost before the logging gate.
This change guards request and response header materialization with
LoggingUtils.isLoggingEnabled()so the disabled path avoids that work entirely.