Skip to content

perf(gax-grpc): skip metadata materialization when logging is disabled #12810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rahul2393 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

perf(gax-grpc): skip metadata materialization when logging is disabled #12810

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f44889e
perf(gax-grpc): skip metadata materialization when logging is disabled
rahul2393 Apr 15, 2026
a6ad914
fix tests
rahul2393 Apr 15, 2026
71a3a72
chore: generate libraries at Wed Apr 15 15:29:18 UTC 2026
cloud-java-bot Apr 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions ...-java/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcLoggingInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
package com.google.api.gax.grpc;

import static com.google.api.gax.logging.LoggingUtils.executeWithTryCatch;
import static com.google.api.gax.logging.LoggingUtils.isLoggingEnabled;
import static com.google.api.gax.logging.LoggingUtils.logRequest;
import static com.google.api.gax.logging.LoggingUtils.logResponse;
import static com.google.api.gax.logging.LoggingUtils.recordResponseHeaders;
Expand Down Expand Up @@ -70,19 +71,23 @@ public <ReqT, RespT> ClientCall<ReqT, RespT> interceptCall(

@Override
public void start(Listener<RespT> responseListener, Metadata headers) {
recordServiceRpcAndRequestHeaders(
method.getServiceName(),
method.getFullMethodName(),
null, // endpoint is for http request only
metadataHeadersToMap(headers),
logDataBuilder,
LOGGER_PROVIDER);
if (isLoggingEnabled()) {
recordServiceRpcAndRequestHeaders(
method.getServiceName(),
method.getFullMethodName(),
null, // endpoint is for http request only
metadataHeadersToMap(headers),
logDataBuilder,
LOGGER_PROVIDER);
}
SimpleForwardingClientCallListener<RespT> responseLoggingListener =
new SimpleForwardingClientCallListener<RespT>(responseListener) {
@Override
public void onHeaders(Metadata headers) {
recordResponseHeaders(
metadataHeadersToMap(headers), logDataBuilder, LOGGER_PROVIDER);
if (isLoggingEnabled()) {
recordResponseHeaders(
metadataHeadersToMap(headers), logDataBuilder, LOGGER_PROVIDER);
}
super.onHeaders(headers);
}

Expand Down
40 changes: 40 additions & 0 deletions ...a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/GrpcLoggingInterceptorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

package com.google.api.gax.grpc;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
Expand All @@ -44,6 +45,8 @@
import io.grpc.Metadata;
import io.grpc.MethodDescriptor;
import io.grpc.Status;
import java.lang.reflect.Method;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
Expand All @@ -58,6 +61,11 @@ class GrpcLoggingInterceptorTest {

private static final MethodDescriptor<String, Integer> method = FakeMethodDescriptor.create();

@AfterEach
void tearDown() throws Exception {
setLoggingEnabled(false);
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
setLoggingEnabled(false);
setLoggingEnabled(true);

}

@Test
void testInterceptor_basic() {
when(channel.newCall(Mockito.<MethodDescriptor<String, Integer>>any(), any(CallOptions.class)))
Expand Down Expand Up @@ -101,4 +109,36 @@ void testInterceptor_responseListener() {
Status status = Status.OK;
interceptor.currentListener.onClose(status, new Metadata());
}

@Test
void testInterceptor_skipsMetadataMaterializationWhenLoggingDisabled() throws Exception {
setLoggingEnabled(false);
when(channel.newCall(Mockito.<MethodDescriptor<String, Integer>>any(), any(CallOptions.class)))
.thenReturn(call);

GrpcLoggingInterceptor interceptor = new GrpcLoggingInterceptor();
Channel intercepted = ClientInterceptors.intercept(channel, interceptor);

@SuppressWarnings("unchecked")
ClientCall.Listener<Integer> listener = mock(ClientCall.Listener.class);

Metadata requestHeaders = mock(Metadata.class);
when(requestHeaders.keys()).thenThrow(new AssertionError("request headers should not be read"));
ClientCall<String, Integer> interceptedCall = intercepted.newCall(method, CallOptions.DEFAULT);

assertDoesNotThrow(() -> interceptedCall.start(listener, requestHeaders));

Metadata responseHeaders = mock(Metadata.class);
when(responseHeaders.keys())
.thenThrow(new AssertionError("response headers should not be read"));

assertDoesNotThrow(() -> interceptor.currentListener.onHeaders(responseHeaders));
}

private static void setLoggingEnabled(boolean enabled) throws Exception {
Class<?> loggingUtils = Class.forName("com.google.api.gax.logging.LoggingUtils");
Method method = loggingUtils.getDeclaredMethod("setLoggingEnabled", boolean.class);
method.setAccessible(true);
method.invoke(null, enabled);
}
}
Loading