Skip to content

Commit 27559f4

Browse files
authored
feat: log actionable errors on operation failure in LoggingTracer (#14460)
Implement operationFailed in LoggingTracer to log actionable errors for logical operation failures. b/564459813
1 parent 4e5a0d0 commit 27559f4

7 files changed

Lines changed: 179 additions & 68 deletions

File tree

‎java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/logging/ITActionableErrorsLogging.java‎

Lines changed: 78 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static org.junit.jupiter.api.Assertions.assertThrows;
2121

2222
import ch.qos.logback.classic.Level;
23+
import ch.qos.logback.classic.Logger;
2324
import ch.qos.logback.classic.spi.ILoggingEvent;
2425
import com.google.api.client.http.LowLevelHttpRequest;
2526
import com.google.api.client.http.LowLevelHttpResponse;
@@ -38,9 +39,11 @@
3839
import com.google.showcase.v1beta1.EchoSettings;
3940
import com.google.showcase.v1beta1.it.util.TestClientInitializer;
4041
import java.io.IOException;
42+
import java.time.Duration;
4143
import java.util.HashMap;
4244
import java.util.Map;
4345
import java.util.concurrent.TimeUnit;
46+
import org.awaitility.Awaitility;
4447
import org.junit.jupiter.api.AfterAll;
4548
import org.junit.jupiter.api.AfterEach;
4649
import org.junit.jupiter.api.BeforeAll;
@@ -76,9 +79,9 @@ static void destroyClients() throws InterruptedException {
7679
private TestAppender setupTestLogger(String loggerName, Level level) {
7780
TestAppender appender = new TestAppender();
7881
appender.start();
79-
org.slf4j.Logger logger = LoggerFactory.getLogger(loggerName);
80-
((ch.qos.logback.classic.Logger) logger).setLevel(level);
81-
((ch.qos.logback.classic.Logger) logger).addAppender(appender);
82+
Logger logger = (Logger) LoggerFactory.getLogger(loggerName);
83+
logger.setLevel(level);
84+
logger.addAppender(appender);
8285
return appender;
8386
}
8487

@@ -92,9 +95,31 @@ void setupTestLogger() {
9295
void teardownTestLogger() {
9396
if (testAppender != null) {
9497
testAppender.stop();
98+
Logger logger = (Logger) LoggerFactory.getLogger("com.google.api.gax.tracing.LoggingTracer");
99+
logger.detachAppender(testAppender);
100+
testAppender.clearEvents();
95101
}
96102
}
97103

104+
/**
105+
* Polls asynchronously until an ERROR logging event is appended or timeout is reached.
106+
*
107+
* <p>Logging of operation failure in TraceFinisher occurs asynchronously on a background executor
108+
* thread, so Awaitility is used to prevent test flakiness and race conditions.
109+
*
110+
* @return the first {@link ILoggingEvent} with ERROR level
111+
*/
112+
private ILoggingEvent getErrorLoggingEvent() {
113+
Awaitility.await()
114+
.atMost(Duration.ofSeconds(5))
115+
.until(() -> testAppender.events.stream().anyMatch(e -> e.getLevel() == Level.ERROR));
116+
return testAppender.events.stream()
117+
.filter(event -> event.getLevel() == Level.ERROR)
118+
.findFirst()
119+
.orElseThrow(
120+
() -> new AssertionError("Expected an ERROR log event in: " + testAppender.events));
121+
}
122+
98123
private Map<String, Object> getKvps(ILoggingEvent loggingEvent) {
99124
Map<String, Object> map = new HashMap<>();
100125
if (loggingEvent.getKeyValuePairs() != null) {
@@ -174,38 +199,40 @@ public LowLevelHttpResponse execute() throws IOException {
174199
.build();
175200
com.google.showcase.v1beta1.stub.EchoStub stub = echoStubSettings.createStub();
176201
EchoClient mockHttpJsonClient = EchoClient.create(stub);
202+
try {
203+
EchoRequest request = EchoRequest.newBuilder().build();
204+
assertThrows(ApiException.class, () -> mockHttpJsonClient.echo(request));
177205

178-
EchoRequest request = EchoRequest.newBuilder().build();
179-
assertThrows(ApiException.class, () -> mockHttpJsonClient.echo(request));
180-
181-
assertThat(testAppender.events.size()).isAtLeast(1);
182-
ILoggingEvent loggingEvent = testAppender.events.get(testAppender.events.size() - 1);
206+
ILoggingEvent loggingEvent = getErrorLoggingEvent();
207+
assertThat(loggingEvent.getLevel()).isEqualTo(Level.ERROR);
183208

184-
assertThat(loggingEvent.getMessage())
185-
.contains("This is a mock JSON error generated by the server");
209+
assertThat(loggingEvent.getMessage())
210+
.contains("This is a mock JSON error generated by the server");
186211

187-
Map<String, Object> kvps = getKvps(loggingEvent);
188-
assertThat(kvps).containsEntry(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE, "http");
189-
assertThat(kvps).containsEntry(ObservabilityAttributes.HTTP_METHOD_ATTRIBUTE, "POST");
190-
assertThat(kvps)
191-
.containsEntry(ObservabilityAttributes.HTTP_URL_TEMPLATE_ATTRIBUTE, "v1beta1/echo:echo");
192-
assertThat(kvps)
193-
.containsEntry(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE, "ABORTED");
194-
assertThat(kvps)
195-
.containsEntry(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE, "mock_error_reason");
196-
assertThat(kvps)
197-
.containsEntry(ObservabilityAttributes.ERROR_DOMAIN_ATTRIBUTE, "mock.googleapis.com");
198-
assertThat(kvps)
199-
.containsEntry(
200-
ObservabilityAttributes.ERROR_METADATA_ATTRIBUTE_PREFIX + "mock_key", "mock_value");
201-
202-
mockHttpJsonClient.close();
203-
mockHttpJsonClient.awaitTermination(
204-
TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS);
212+
Map<String, Object> kvps = getKvps(loggingEvent);
213+
assertThat(kvps).containsEntry(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE, "http");
214+
assertThat(kvps).containsEntry(ObservabilityAttributes.HTTP_METHOD_ATTRIBUTE, "POST");
215+
assertThat(kvps)
216+
.containsEntry(ObservabilityAttributes.HTTP_URL_TEMPLATE_ATTRIBUTE, "v1beta1/echo:echo");
217+
assertThat(kvps)
218+
.containsEntry(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE, "ABORTED");
219+
assertThat(kvps)
220+
.containsEntry(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE, "mock_error_reason");
221+
assertThat(kvps)
222+
.containsEntry(ObservabilityAttributes.ERROR_DOMAIN_ATTRIBUTE, "mock.googleapis.com");
223+
assertThat(kvps)
224+
.containsEntry(
225+
ObservabilityAttributes.ERROR_METADATA_ATTRIBUTE_PREFIX + "mock_key", "mock_value");
226+
} finally {
227+
mockHttpJsonClient.close();
228+
mockHttpJsonClient.awaitTermination(
229+
TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS);
230+
}
205231
}
206232

207233
@Test
208234
void testHttpJson_noLogEmittedForSuccess() {
235+
testAppender.clearEvents();
209236
EchoRequest request = EchoRequest.newBuilder().setContent("Success").build();
210237
httpjsonClient.echo(request);
211238
assertThat(testAppender.events.size()).isEqualTo(0);
@@ -218,22 +245,23 @@ void testHttpJson_clientLevelFailureAttributes() throws Exception {
218245
stubSettingsBuilder
219246
.echoSettings()
220247
.setRetrySettings(
221-
com.google.api.gax.retrying.RetrySettings.newBuilder()
222-
.setInitialRpcTimeoutDuration(java.time.Duration.ofMillis(0))
223-
.setTotalTimeoutDuration(java.time.Duration.ofMillis(0))
224-
.setMaxAttempts(1)
225-
.build());
248+
com.google.api.gax.retrying.RetrySettings.newBuilder().setMaxAttempts(1).build());
226249
stubSettingsBuilder.setTracerFactory(new LoggingTracerFactory());
227250
stubSettingsBuilder.setCredentialsProvider(NoCredentialsProvider.create());
228251
stubSettingsBuilder.setEndpoint("localhost:1");
229252

230-
try (com.google.showcase.v1beta1.stub.EchoStub stub = stubSettingsBuilder.build().createStub();
231-
EchoClient client = EchoClient.create(stub)) {
253+
com.google.showcase.v1beta1.stub.EchoStub stub = stubSettingsBuilder.build().createStub();
254+
EchoClient client = EchoClient.create(stub);
255+
try {
232256
assertThrows(ApiException.class, () -> client.echo(EchoRequest.newBuilder().build()));
233-
assertThat(testAppender.events.size()).isAtLeast(1);
234-
ILoggingEvent loggingEvent = testAppender.events.get(testAppender.events.size() - 1);
257+
ILoggingEvent loggingEvent = getErrorLoggingEvent();
258+
assertThat(loggingEvent.getLevel()).isEqualTo(Level.ERROR);
259+
assertThat(loggingEvent.getMessage()).isNotEmpty();
235260
Map<String, Object> kvps = getKvps(loggingEvent);
236261
assertThat(kvps).containsEntry(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE, "http");
262+
} finally {
263+
client.shutdownNow();
264+
client.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS);
237265
}
238266
}
239267

@@ -242,8 +270,8 @@ void testGrpc_logEmittedForLowLevelRequestFailure() {
242270
EchoRequest request = buildErrorRequest();
243271
assertThrows(ApiException.class, () -> grpcClient.echo(request));
244272

245-
assertThat(testAppender.events.size()).isAtLeast(1);
246-
ILoggingEvent loggingEvent = testAppender.events.get(testAppender.events.size() - 1);
273+
ILoggingEvent loggingEvent = getErrorLoggingEvent();
274+
assertThat(loggingEvent.getLevel()).isEqualTo(Level.ERROR);
247275
assertThat(loggingEvent.getMessage()).contains("This is a test error");
248276

249277
Map<String, Object> kvps = getKvps(loggingEvent);
@@ -264,6 +292,7 @@ void testGrpc_logEmittedForLowLevelRequestFailure() {
264292

265293
@Test
266294
void testGrpc_noLogEmittedForSuccess() {
295+
testAppender.clearEvents();
267296
EchoRequest request = EchoRequest.newBuilder().setContent("Success").build();
268297
grpcClient.echo(request);
269298
assertThat(testAppender.events.size()).isEqualTo(0);
@@ -276,22 +305,23 @@ void testGrpc_clientLevelFailureAttributes() throws Exception {
276305
stubSettingsBuilder
277306
.echoSettings()
278307
.setRetrySettings(
279-
com.google.api.gax.retrying.RetrySettings.newBuilder()
280-
.setInitialRpcTimeoutDuration(java.time.Duration.ofMillis(0))
281-
.setTotalTimeoutDuration(java.time.Duration.ofMillis(0))
282-
.setMaxAttempts(1)
283-
.build());
308+
com.google.api.gax.retrying.RetrySettings.newBuilder().setMaxAttempts(1).build());
284309
stubSettingsBuilder.setTracerFactory(new LoggingTracerFactory());
285310
stubSettingsBuilder.setCredentialsProvider(NoCredentialsProvider.create());
286311
stubSettingsBuilder.setEndpoint("localhost:1");
287312

288-
try (com.google.showcase.v1beta1.stub.EchoStub stub = stubSettingsBuilder.build().createStub();
289-
EchoClient client = EchoClient.create(stub)) {
313+
com.google.showcase.v1beta1.stub.EchoStub stub = stubSettingsBuilder.build().createStub();
314+
EchoClient client = EchoClient.create(stub);
315+
try {
290316
assertThrows(ApiException.class, () -> client.echo(EchoRequest.newBuilder().build()));
291-
assertThat(testAppender.events.size()).isAtLeast(1);
292-
ILoggingEvent loggingEvent = testAppender.events.get(testAppender.events.size() - 1);
317+
ILoggingEvent loggingEvent = getErrorLoggingEvent();
318+
assertThat(loggingEvent.getLevel()).isEqualTo(Level.ERROR);
319+
assertThat(loggingEvent.getMessage()).isNotEmpty();
293320
Map<String, Object> kvps = getKvps(loggingEvent);
294321
assertThat(kvps).containsEntry(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE, "grpc");
322+
} finally {
323+
client.shutdownNow();
324+
client.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS);
295325
}
296326
}
297327
}

‎java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/logging/TestAppender.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@
1818

1919
import ch.qos.logback.classic.spi.ILoggingEvent;
2020
import ch.qos.logback.core.AppenderBase;
21-
import java.util.ArrayList;
2221
import java.util.List;
22+
import java.util.concurrent.CopyOnWriteArrayList;
2323

2424
/** Logback appender used to set up tests. */
2525
public class TestAppender extends AppenderBase<ILoggingEvent> {
26-
public List<ILoggingEvent> events = new ArrayList<>();
26+
public List<ILoggingEvent> events = new CopyOnWriteArrayList<>();
2727

2828
@Override
2929
protected void append(ILoggingEvent eventObject) {

‎sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/logging/LoggingUtils.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
import com.google.api.core.InternalApi;
3434
import java.util.Map;
3535
import org.jspecify.annotations.NullMarked;
36+
import org.slf4j.Logger;
37+
import org.slf4j.event.Level;
3638

3739
@NullMarked
3840
@InternalApi
@@ -149,18 +151,19 @@ public static <RespT> void logRequest(
149151
}
150152

151153
/**
152-
* Logs an actionable error message with structured context at a specific log level.
154+
* Logs an actionable error message with structured context at a specified log level.
153155
*
154156
* @param logContext A map containing the structured logging context (e.g., RPC service, method,
155157
* error details).
156158
* @param loggerProvider The provider used to obtain the logger.
157159
* @param message The human-readable error message.
160+
* @param level The SLF4J log level at which to emit the error log.
158161
*/
159162
public static void logActionableError(
160-
Map<String, Object> logContext, LoggerProvider loggerProvider, String message) {
163+
Map<String, Object> logContext, LoggerProvider loggerProvider, String message, Level level) {
161164
if (loggingEnabled) {
162-
org.slf4j.Logger logger = loggerProvider.getLogger();
163-
Slf4jUtils.log(logger, org.slf4j.event.Level.DEBUG, logContext, message);
165+
Logger logger = loggerProvider.getLogger();
166+
Slf4jUtils.log(logger, level, logContext, message);
164167
}
165168
}
166169

‎sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/LoggingTracer.java‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import java.util.HashMap;
3939
import java.util.Map;
4040
import org.jspecify.annotations.NullMarked;
41+
import org.slf4j.event.Level;
4142

4243
/**
4344
* An {@link ApiTracer} that logs actionable errors using {@link LoggingUtils} when an RPC attempt
@@ -56,21 +57,37 @@ class LoggingTracer extends BaseApiTracer {
5657

5758
@Override
5859
public void attemptFailedDuration(Throwable error, java.time.Duration delay) {
59-
recordActionableError(error);
60+
recordActionableError(error, Level.DEBUG);
6061
}
6162

6263
@Override
6364
public void attemptFailedRetriesExhausted(Throwable error) {
64-
recordActionableError(error);
65+
recordActionableError(error, Level.DEBUG);
66+
}
67+
68+
/**
69+
* Records an actionable error log entry at ERROR level when the logical operation fails.
70+
*
71+
* @param error the exception that caused the logical operation to fail
72+
*/
73+
@Override
74+
public void operationFailed(Throwable error) {
75+
recordActionableError(error, Level.ERROR);
6576
}
6677

6778
@Override
6879
public void attemptPermanentFailure(Throwable error) {
69-
recordActionableError(error);
80+
recordActionableError(error, Level.DEBUG);
7081
}
7182

83+
/**
84+
* Records an actionable error log entry with the specified log level.
85+
*
86+
* @param error the exception that occurred
87+
* @param level the SLF4J log level at which to emit the error log
88+
*/
7289
@VisibleForTesting
73-
void recordActionableError(Throwable error) {
90+
void recordActionableError(Throwable error, Level level) {
7491
if (error == null) {
7592
return;
7693
}
@@ -98,6 +115,6 @@ void recordActionableError(Throwable error) {
98115
}
99116

100117
String message = error.getMessage() != null ? error.getMessage() : error.getClass().getName();
101-
LoggingUtils.logActionableError(logContext, LOGGER_PROVIDER, message);
118+
LoggingUtils.logActionableError(logContext, LOGGER_PROVIDER, message, level);
102119
}
103120
}

‎sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/logging/LoggingUtilsTest.java‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,10 @@ void testLogActionableError_loggingDisabled() {
101101
mock(LoggerProvider.class, Mockito.withSettings().withoutAnnotations());
102102

103103
LoggingUtils.logActionableError(
104-
Collections.<String, Object>emptyMap(), loggerProvider, "message");
104+
Collections.<String, Object>emptyMap(),
105+
loggerProvider,
106+
"message",
107+
org.slf4j.event.Level.DEBUG);
105108

106109
verify(loggerProvider, never()).getLogger();
107110
}
@@ -119,8 +122,29 @@ void testLogActionableError_success() {
119122
when(eventBuilder.addKeyValue(anyString(), any())).thenReturn(eventBuilder);
120123

121124
Map<String, Object> context = Collections.singletonMap("key", "value");
122-
LoggingUtils.logActionableError(context, loggerProvider, "message");
125+
LoggingUtils.logActionableError(
126+
context, loggerProvider, "message", org.slf4j.event.Level.DEBUG);
127+
128+
verify(loggerProvider).getLogger();
129+
}
130+
131+
@Test
132+
void testLogActionableError_withLevel_success() {
133+
LoggingUtils.setLoggingEnabled(true);
134+
LoggerProvider loggerProvider =
135+
mock(LoggerProvider.class, Mockito.withSettings().withoutAnnotations());
136+
Logger logger = mock(Logger.class, Mockito.withSettings().withoutAnnotations());
137+
when(loggerProvider.getLogger()).thenReturn(logger);
138+
139+
org.slf4j.spi.LoggingEventBuilder eventBuilder = mock(org.slf4j.spi.LoggingEventBuilder.class);
140+
when(logger.atError()).thenReturn(eventBuilder);
141+
when(eventBuilder.addKeyValue(anyString(), any())).thenReturn(eventBuilder);
142+
143+
Map<String, Object> context = Collections.singletonMap("key", "value");
144+
LoggingUtils.logActionableError(
145+
context, loggerProvider, "message", org.slf4j.event.Level.ERROR);
123146

124147
verify(loggerProvider).getLogger();
148+
verify(logger).atError();
125149
}
126150
}

‎sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/logging/TestLogger.java‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,24 @@ public List<String> getMessageList() {
5252
return messageList;
5353
}
5454

55+
/**
56+
* Returns the most recent log level recorded by this test logger.
57+
*
58+
* @return the SLF4J {@link Level} or null if no level has been recorded
59+
*/
60+
public Level getLevel() {
61+
return level;
62+
}
63+
64+
/**
65+
* Sets or clears the log level recorded by this test logger.
66+
*
67+
* @param level the SLF4J {@link Level} to set
68+
*/
69+
public void setLevel(Level level) {
70+
this.level = level;
71+
}
72+
5573
Map<String, Object> keyValuePairsMap = new HashMap<>();
5674

5775
public Map<String, String> getMDCMap() {
@@ -261,7 +279,7 @@ public void warn(Marker marker, String msg, Throwable t) {}
261279

262280
@Override
263281
public boolean isErrorEnabled() {
264-
return false;
282+
return true;
265283
}
266284

267285
@Override

0 commit comments

Comments
 (0)