Skip to content

Commit 0f33445

Browse files
committed
fix(tracing): align ErrorType mapping with updated specification
- Removed heuristic exception mappings for CLIENT_REDIRECT_ERROR and CLIENT_UNKNOWN_ERROR to fall back to simple class name - Refined mapping for CLIENT_AUTHENTICATION_ERROR by removing FileNotFoundException - Added unmapped placeholders to ErrorType enum - Updated unit and integration tests to verify new fallback mechanisms
1 parent fa5b594 commit 0f33445

4 files changed

Lines changed: 47 additions & 54 deletions

File tree

gax-java/gax/src/main/java/com/google/api/gax/tracing/ErrorTypeUtil.java

Lines changed: 5 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import com.google.api.gax.rpc.WatchdogTimeoutException;
3535
import com.google.common.base.Strings;
3636
import com.google.common.collect.ImmutableSet;
37-
import java.io.FileNotFoundException;
3837
import java.net.BindException;
3938
import java.net.ConnectException;
4039
import java.net.NoRouteToHostException;
@@ -52,10 +51,14 @@ public enum ErrorType {
5251
CLIENT_TIMEOUT,
5352
CLIENT_CONNECTION_ERROR,
5453
CLIENT_REQUEST_ERROR,
54+
/** Placeholder for potential future request body errors. */
5555
CLIENT_REQUEST_BODY_ERROR,
56+
/** Placeholder for potential future response decode errors. */
5657
CLIENT_RESPONSE_DECODE_ERROR,
58+
/** Placeholder for potential future redirect errors. */
5759
CLIENT_REDIRECT_ERROR,
5860
CLIENT_AUTHENTICATION_ERROR,
61+
/** Placeholder for potential future unknown errors. */
5962
CLIENT_UNKNOWN_ERROR,
6063
INTERNAL;
6164

@@ -66,7 +69,7 @@ public String toString() {
6669
}
6770

6871
private static final Set<Class<? extends Throwable>> AUTHENTICATION_EXCEPTION_CLASSES =
69-
ImmutableSet.of(GeneralSecurityException.class, FileNotFoundException.class);
72+
ImmutableSet.of(GeneralSecurityException.class);
7073

7174
private static final Set<Class<? extends Throwable>> CLIENT_TIMEOUT_EXCEPTION_CLASSES =
7275
ImmutableSet.of(
@@ -193,16 +196,10 @@ private static String getClientSideError(Throwable error) {
193196
if (isClientAuthenticationError(error)) {
194197
return ErrorType.CLIENT_AUTHENTICATION_ERROR.toString();
195198
}
196-
if (isClientRedirectError(error)) {
197-
return ErrorType.CLIENT_REDIRECT_ERROR.toString();
198-
}
199199
// This covers CLIENT_REQUEST_ERROR for general illegal arguments in client requests.
200200
if (error instanceof IllegalArgumentException) {
201201
return ErrorType.CLIENT_REQUEST_ERROR.toString();
202202
}
203-
if (isClientUnknownError(error)) {
204-
return ErrorType.CLIENT_UNKNOWN_ERROR.toString();
205-
}
206203
return null;
207204
}
208205

@@ -228,40 +225,10 @@ private static boolean isClientConnectionError(Throwable e) {
228225
return hasErrorClassInCauseChain(e, CLIENT_CONNECTION_EXCEPTIONS);
229226
}
230227

231-
/**
232-
* Checks if the given Throwable represents a client-side redirect error. This is identified by
233-
* the presence of "redirect" in the exception message.
234-
*
235-
* @param e The Throwable to check.
236-
* @return true if the error is a client redirect error, false otherwise.
237-
*/
238-
private static boolean isClientRedirectError(Throwable e) {
239-
return e.getMessage() != null && e.getMessage().contains("redirect");
240-
}
241-
242-
/**
243-
* Checks if the given Throwable represents a client-side authentication error. This is identified
244-
* by exceptions related to the auth library.
245-
*
246-
* @param e The Throwable to check.
247-
* @return true if the error is a client authentication error, false otherwise.
248-
*/
249228
private static boolean isClientAuthenticationError(Throwable e) {
250229
return hasErrorClassInCauseChain(e, AUTHENTICATION_EXCEPTION_CLASSES);
251230
}
252231

253-
/**
254-
* Checks if the given Throwable represents an unknown client-side error. This is a general
255-
* fallback for exceptions whose class name contains "unknown", indicating an unclassified
256-
* client-side issue.
257-
*
258-
* @param e The Throwable to check.
259-
* @return true if the error is an unknown client error, false otherwise.
260-
*/
261-
private static boolean isClientUnknownError(Throwable e) {
262-
return e.getClass().getName().toLowerCase().contains("unknown");
263-
}
264-
265232
/**
266233
* Recursively checks the throwable and its cause chain for any of the specified error classes.
267234
*

gax-java/gax/src/test/java/com/google/api/gax/rpc/ErrorTypeUtilTest.java

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333

3434
import com.google.api.gax.rpc.testing.FakeStatusCode;
3535
import com.google.api.gax.tracing.ErrorTypeUtil;
36-
import java.io.FileNotFoundException;
3736
import java.io.IOException;
3837
import java.net.BindException;
3938
import java.net.ConnectException;
@@ -113,7 +112,8 @@ void testExtractErrorType_realUnknownHostException() {
113112
void testExtractErrorType_realSSLHandshakeException() throws Exception {
114113
// Emulating a reliable SSLHandshakeException (vs a generic SSLException) requires
115114
// complex keystore setups which are brittle. We instantiate it directly here.
116-
assertThat(ErrorTypeUtil.extractErrorType(new SSLHandshakeException("Cert path building failed")))
115+
assertThat(
116+
ErrorTypeUtil.extractErrorType(new SSLHandshakeException("Cert path building failed")))
117117
.isEqualTo(ErrorTypeUtil.ErrorType.CLIENT_CONNECTION_ERROR.toString());
118118
}
119119

@@ -135,16 +135,17 @@ void testExtractErrorType_realBindException() throws Exception {
135135
void testExtractErrorType_clientTimeout_others() {
136136
assertThat(ErrorTypeUtil.extractErrorType(new WatchdogTimeoutException("timeout", false)))
137137
.isEqualTo(ErrorTypeUtil.ErrorType.CLIENT_TIMEOUT.toString());
138-
assertThat(ErrorTypeUtil.extractErrorType(new DeadlineExceededException("timeout", null, new FakeStatusCode(StatusCode.Code.DEADLINE_EXCEEDED), false)))
138+
assertThat(
139+
ErrorTypeUtil.extractErrorType(
140+
new DeadlineExceededException(
141+
"timeout", null, new FakeStatusCode(StatusCode.Code.DEADLINE_EXCEEDED), false)))
139142
.isEqualTo(ErrorTypeUtil.ErrorType.CLIENT_TIMEOUT.toString());
140143
}
141144

142145
@Test
143146
void testExtractErrorType_clientAuthenticationError() {
144147
assertThat(ErrorTypeUtil.extractErrorType(new GeneralSecurityException("auth fail")))
145148
.isEqualTo(ErrorTypeUtil.ErrorType.CLIENT_AUTHENTICATION_ERROR.toString());
146-
assertThat(ErrorTypeUtil.extractErrorType(new FileNotFoundException("key not found")))
147-
.isEqualTo(ErrorTypeUtil.ErrorType.CLIENT_AUTHENTICATION_ERROR.toString());
148149
}
149150

150151
@Test
@@ -177,7 +178,19 @@ void testExtractErrorType_causeChainTraversal() {
177178

178179
@Test
179180
void testExtractErrorType_unknownException() {
180-
assertThat(ErrorTypeUtil.extractErrorType(new Exception("Unknown stuff")))
181-
.isEqualTo("Exception");
181+
assertThat(ErrorTypeUtil.extractErrorType(new Exception("Unknown stuff")))
182+
.isEqualTo("Exception");
183+
}
184+
185+
@Test
186+
void testExtractErrorType_redirectFallback() {
187+
assertThat(ErrorTypeUtil.extractErrorType(new Exception("redirect"))).isEqualTo("Exception");
188+
}
189+
190+
@Test
191+
void testExtractErrorType_unknownClassNameFallback() {
192+
class UnknownClientException extends Exception {}
193+
assertThat(ErrorTypeUtil.extractErrorType(new UnknownClientException()))
194+
.isEqualTo("UnknownClientException");
182195
}
183196
}

gax-java/gax/src/test/java/com/google/api/gax/tracing/SpanTracerTest.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -218,9 +218,7 @@ void testAttemptFailed_clientRedirectError() {
218218
tracer.attemptFailedRetriesExhausted(new RedirectException("redirect failed"));
219219

220220
verify(attemptHandle)
221-
.addAttribute(
222-
ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE,
223-
ErrorTypeUtil.ErrorType.CLIENT_REDIRECT_ERROR.toString());
221+
.addAttribute(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE, "RedirectException");
224222
verify(attemptHandle).end();
225223
}
226224

@@ -248,9 +246,7 @@ void testAttemptFailed_clientUnknownError() {
248246
tracer.attemptFailedRetriesExhausted(new UnknownClientException());
249247

250248
verify(attemptHandle)
251-
.addAttribute(
252-
ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE,
253-
ErrorTypeUtil.ErrorType.CLIENT_UNKNOWN_ERROR.toString());
249+
.addAttribute(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE, "UnknownClientException");
254250
verify(attemptHandle).end();
255251
}
256252

java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelErrorType.java

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import static org.junit.Assert.assertThrows;
3535

3636
import com.google.api.gax.core.FixedCredentialsProvider;
37+
import com.google.api.gax.httpjson.RestSerializationException;
3738
import com.google.api.gax.rpc.DeadlineExceededException;
3839
import com.google.api.gax.rpc.StatusCode.Code;
3940
import com.google.api.gax.rpc.UnavailableException;
@@ -383,7 +384,9 @@ void testTracing_clientAuthenticationError_GeneralSecurityException_grpc() throw
383384
void testTracing_clientAuthenticationError_FileNotFoundException_grpc() throws Exception {
384385
try (EchoClient client = createInterceptorClient(new FileNotFoundException("Key not found"))) {
385386
assertThrows(RuntimeException.class, () -> client.echo(EchoRequest.newBuilder().setContent("test").build()));
386-
verifyErrorTypeAttribute("CLIENT_AUTHENTICATION_ERROR");
387+
// Wrapping non-RuntimeExceptions in RuntimeException during interceptCall()
388+
// means the simple class name of the exception being recorded is "RuntimeException"
389+
verifyErrorTypeAttribute("RuntimeException");
387390
}
388391
}
389392

@@ -399,7 +402,9 @@ void testTracing_clientRequestError_IllegalArgumentException_grpc() throws Excep
399402
void testTracing_clientRedirectError_grpc() throws Exception {
400403
try (EchoClient client = createInterceptorClient(new RuntimeException("Too many redirects"))) {
401404
assertThrows(RuntimeException.class, () -> client.echo(EchoRequest.newBuilder().setContent("test").build()));
402-
verifyErrorTypeAttribute("CLIENT_REDIRECT_ERROR");
405+
// Heuristic mapping of "redirect" in message has been removed.
406+
// Expected result is now the simple class name of the exception.
407+
verifyErrorTypeAttribute("RuntimeException");
403408
}
404409
}
405410

@@ -409,7 +414,19 @@ void testTracing_clientUnknownError_grpc() throws Exception {
409414
class MyUnknownException extends RuntimeException {}
410415
try (EchoClient client = createInterceptorClient(new MyUnknownException())) {
411416
assertThrows(RuntimeException.class, () -> client.echo(EchoRequest.newBuilder().setContent("test").build()));
412-
verifyErrorTypeAttribute("CLIENT_UNKNOWN_ERROR");
417+
// Heuristic mapping of "Unknown" in class name has been removed.
418+
// Expected result is now the simple class name of the exception.
419+
verifyErrorTypeAttribute("MyUnknownException");
420+
}
421+
}
422+
423+
@Test
424+
void testTracing_clientRequestError_RestSerializationException_httpjson() throws Exception {
425+
try (EchoClient client = createInterceptorClient(new RestSerializationException("failed to serialize", null))) {
426+
assertThrows(RuntimeException.class, () -> client.echo(EchoRequest.newBuilder().setContent("test").build()));
427+
// RestSerializationException is not handled due to ambiguity (serialization vs deserialization).
428+
// Expected result is now its simple class name.
429+
verifyErrorTypeAttribute("RestSerializationException");
413430
}
414431
}
415432
}

0 commit comments

Comments
 (0)