Skip to content

Commit 457f88c

Browse files
westarlewestarle
committed
fix(gax): address PR3 feedback on actionable errors
1 parent 1f13fb6 commit 457f88c

File tree

3 files changed

+96
-47
lines changed

3 files changed

+96
-47
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,9 @@ public Map<String, Object> getAttemptAttributes() {
205205
attributes.put(ObservabilityAttributes.HTTP_URL_TEMPLATE_ATTRIBUTE, httpPathTemplate());
206206
}
207207
}
208+
if (!Strings.isNullOrEmpty(serviceName())) {
209+
attributes.put(ObservabilityAttributes.GCP_CLIENT_SERVICE_ATTRIBUTE, serviceName());
210+
}
208211
if (!Strings.isNullOrEmpty(destinationResourceId())) {
209212
attributes.put(
210213
ObservabilityAttributes.DESTINATION_RESOURCE_ID_ATTRIBUTE, destinationResourceId());

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

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import com.google.api.core.InternalApi;
3535
import com.google.api.gax.logging.LoggerProvider;
3636
import com.google.api.gax.logging.LoggingUtils;
37+
import com.google.common.annotations.VisibleForTesting;
3738
import com.google.rpc.ErrorInfo;
3839
import java.util.HashMap;
3940
import java.util.Map;
@@ -44,13 +45,13 @@
4445
*/
4546
@BetaApi
4647
@InternalApi
47-
public class LoggingTracer extends BaseApiTracer {
48+
class LoggingTracer extends BaseApiTracer {
4849
private static final LoggerProvider LOGGER_PROVIDER =
4950
LoggerProvider.forClazz(LoggingTracer.class);
5051

5152
private final ApiTracerContext apiTracerContext;
5253

53-
public LoggingTracer(ApiTracerContext apiTracerContext) {
54+
LoggingTracer(ApiTracerContext apiTracerContext) {
5455
this.apiTracerContext = apiTracerContext;
5556
}
5657

@@ -69,18 +70,14 @@ public void attemptPermanentFailure(Throwable error) {
6970
recordActionableError(error);
7071
}
7172

72-
private void recordActionableError(Throwable error) {
73+
@VisibleForTesting
74+
void recordActionableError(Throwable error) {
7375
if (error == null) {
7476
return;
7577
}
7678

7779
Map<String, Object> logContext = new HashMap<>(apiTracerContext.getAttemptAttributes());
7880

79-
if (apiTracerContext.serviceName() != null) {
80-
logContext.put(
81-
ObservabilityAttributes.GCP_CLIENT_SERVICE_ATTRIBUTE, apiTracerContext.serviceName());
82-
}
83-
8481
logContext.put(
8582
ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE,
8683
ObservabilityUtils.extractStatus(error));

‎sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/LoggingTracerTest.java‎

Lines changed: 88 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -59,28 +59,94 @@ void setUp() {
5959
}
6060

6161
@Test
62-
void testAttemptFailed_LogsError() {
62+
void testAttemptFailedDuration_LogsError() {
6363
ApiTracerContext context = ApiTracerContext.empty();
6464
LoggingTracer tracer = new LoggingTracer(context);
6565

66-
// Call attemptFailed with a generic exception
67-
Exception error = new RuntimeException("generic failure");
66+
Exception error = new RuntimeException("generic failure duration");
6867
tracer.attemptFailedDuration(error, java.time.Duration.ZERO);
6968

7069
assertEquals(1, testLogger.getMessageList().size());
71-
assertEquals("generic failure", testLogger.getMessageList().get(0));
70+
assertEquals("generic failure duration", testLogger.getMessageList().get(0));
71+
}
72+
73+
@Test
74+
void testAttemptFailedRetriesExhausted_LogsError() {
75+
ApiTracerContext context = ApiTracerContext.empty();
76+
LoggingTracer tracer = new LoggingTracer(context);
77+
78+
Exception error = new RuntimeException("generic failure retries exhausted");
79+
tracer.attemptFailedRetriesExhausted(error);
80+
81+
assertEquals(1, testLogger.getMessageList().size());
82+
assertEquals("generic failure retries exhausted", testLogger.getMessageList().get(0));
83+
}
84+
85+
@Test
86+
void testAttemptPermanentFailure_LogsError() {
87+
ApiTracerContext context = ApiTracerContext.empty();
88+
LoggingTracer tracer = new LoggingTracer(context);
89+
90+
Exception error = new RuntimeException("generic permanent failure");
91+
tracer.attemptPermanentFailure(error);
92+
93+
assertEquals(1, testLogger.getMessageList().size());
94+
assertEquals("generic permanent failure", testLogger.getMessageList().get(0));
95+
}
96+
97+
@Test
98+
void testRecordActionableError_logsErrorMessage() {
99+
ApiTracerContext context = ApiTracerContext.empty();
100+
LoggingTracer tracer = new LoggingTracer(context);
101+
102+
Exception error = new RuntimeException("test error message");
103+
tracer.recordActionableError(error);
104+
105+
assertEquals(1, testLogger.getMessageList().size());
106+
assertEquals("test error message", testLogger.getMessageList().get(0));
107+
}
108+
109+
@Test
110+
void testRecordActionableError_logsStatus() {
111+
ApiTracerContext context = ApiTracerContext.empty();
112+
LoggingTracer tracer = new LoggingTracer(context);
113+
114+
Exception error =
115+
ApiExceptionFactory.createException(
116+
"test error message",
117+
new RuntimeException("cause"),
118+
FakeStatusCode.of(StatusCode.Code.INVALID_ARGUMENT),
119+
false);
120+
121+
tracer.recordActionableError(error);
122+
123+
Map<String, ?> attributesMap = getAttributesMap();
124+
125+
assertTrue(attributesMap != null && !attributesMap.isEmpty());
126+
assertEquals(
127+
"INVALID_ARGUMENT",
128+
attributesMap.get(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE));
72129
}
73130

74131
@Test
75-
void testAttemptFailed_LogsErrorAndAttributes() {
132+
void testRecordActionableError_logsAttributes() {
76133
ApiTracerContext context =
77-
ApiTracerContext.empty().toBuilder()
78-
.setServiceName("test-service")
79-
.setServerAddress("test.example.com")
80-
.setServerPort(443)
81-
.setFullMethodName("test.service.v1.Method")
82-
.setTransport(ApiTracerContext.Transport.GRPC)
83-
.build();
134+
ApiTracerContext.empty().toBuilder().setServiceName("test-service").build();
135+
LoggingTracer tracer = new LoggingTracer(context);
136+
137+
Exception error = new RuntimeException("generic failure");
138+
tracer.recordActionableError(error);
139+
140+
Map<String, ?> attributesMap = getAttributesMap();
141+
142+
assertTrue(attributesMap != null && !attributesMap.isEmpty());
143+
assertEquals(
144+
"test-service", attributesMap.get(ObservabilityAttributes.GCP_CLIENT_SERVICE_ATTRIBUTE));
145+
}
146+
147+
@Test
148+
void testRecordActionableError_logsErrorInfo() {
149+
ApiTracerContext context = ApiTracerContext.empty();
84150
LoggingTracer tracer = new LoggingTracer(context);
85151

86152
ErrorInfo errorInfo =
@@ -103,41 +169,24 @@ void testAttemptFailed_LogsErrorAndAttributes() {
103169
false,
104170
errorDetails);
105171

106-
tracer.attemptFailedDuration(error, java.time.Duration.ZERO);
172+
tracer.recordActionableError(error);
107173

108-
assertEquals(1, testLogger.getMessageList().size());
109-
assertEquals("test error message", testLogger.getMessageList().get(0));
110-
111-
Map<String, ?> attributesMap;
112-
// SLF4J 1.x logs using MDC, SLF4J 2.x logs using KeyValuePairs
113-
// Depending on the classpath binding active, one will be populated by TestLogger
114-
if (!testLogger.getMDCMap().isEmpty()) {
115-
attributesMap = testLogger.getMDCMap();
116-
} else {
117-
attributesMap = testLogger.getKeyValuePairsMap();
118-
}
174+
Map<String, ?> attributesMap = getAttributesMap();
119175

120176
assertTrue(attributesMap != null && !attributesMap.isEmpty());
121-
assertEquals(
122-
"test-service", attributesMap.get(ObservabilityAttributes.GCP_CLIENT_SERVICE_ATTRIBUTE));
123-
assertEquals(
124-
"test.example.com", attributesMap.get(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE));
125-
126-
Object portValue = attributesMap.get(ObservabilityAttributes.SERVER_PORT_ATTRIBUTE);
127-
assertTrue("443".equals(String.valueOf(portValue)));
128-
129-
assertEquals(
130-
"test.service.v1.Method",
131-
attributesMap.get(ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE));
132-
assertEquals("grpc", attributesMap.get(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE));
133-
assertEquals(
134-
"INVALID_ARGUMENT",
135-
attributesMap.get(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE));
136177
assertEquals("TEST_REASON", attributesMap.get(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE));
137178
assertEquals(
138179
"test.domain.com", attributesMap.get(ObservabilityAttributes.ERROR_DOMAIN_ATTRIBUTE));
139180
assertEquals(
140181
"test_value",
141182
attributesMap.get(ObservabilityAttributes.ERROR_METADATA_ATTRIBUTE_PREFIX + "test_key"));
142183
}
184+
185+
private Map<String, ?> getAttributesMap() {
186+
if (!testLogger.getMDCMap().isEmpty()) {
187+
return testLogger.getMDCMap();
188+
} else {
189+
return testLogger.getKeyValuePairsMap();
190+
}
191+
}
143192
}

0 commit comments

Comments
 (0)