Skip to content

Commit a73c78b

Browse files
westarlewestarle
committed
fix(test): provide libraryMetadata for ApiTracerContext.newBuilder in tests
1 parent 84e3b25 commit a73c78b

8 files changed

Lines changed: 149 additions & 52 deletions

File tree

‎sdk-platform-java/gax-java/gax/pom.xml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@
139139
<configuration>
140140
<argLine>@{argLine} -Djava.util.logging.SimpleFormatter.format="%1$tY %1$tl:%1$tM:%1$tS.%1$tL %2$s %4$s: %5$s%6$s%n"</argLine>
141141
<!-- These tests require an Env Var to be set. Use -PenvVarTest to ONLY run these tests -->
142-
<test>!EndpointContextTest#endpointContextBuild_universeDomainEnvVarSet+endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority,!LoggingEnabledTest</test>
142+
<test>!EndpointContextTest#endpointContextBuild_universeDomainEnvVarSet+endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority,!LoggingEnabledTest,!LoggingTracerTest</test>
143143
</configuration>
144144
</plugin>
145145
</plugins>
@@ -154,7 +154,7 @@
154154
<groupId>org.apache.maven.plugins</groupId>
155155
<artifactId>maven-surefire-plugin</artifactId>
156156
<configuration>
157-
<test>EndpointContextTest#endpointContextBuild_universeDomainEnvVarSet+endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority,LoggingEnabledTest</test>
157+
<test>EndpointContextTest#endpointContextBuild_universeDomainEnvVarSet+endpointContextBuild_multipleUniverseDomainConfigurations_clientSettingsHasPriority,LoggingEnabledTest,LoggingTracerTest</test>
158158
</configuration>
159159
</plugin>
160160
</plugins>

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

Lines changed: 23 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,6 @@ public LoggingTracer(ApiTracerContext apiTracerContext) {
5454
this.apiTracerContext = apiTracerContext;
5555
}
5656

57-
@Override
58-
public void attemptFailed(Throwable error, org.threeten.bp.Duration delay) {
59-
recordActionableError(error);
60-
}
61-
6257
@Override
6358
public void attemptFailedDuration(Throwable error, java.time.Duration delay) {
6459
recordActionableError(error);
@@ -75,47 +70,39 @@ public void attemptPermanentFailure(Throwable error) {
7570
}
7671

7772
private void recordActionableError(Throwable error) {
78-
Map<String, Object> logContext = new HashMap<>();
79-
80-
if (apiTracerContext.rpcSystemName() != null) {
81-
logContext.put(
82-
ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE, apiTracerContext.rpcSystemName());
83-
}
84-
if (apiTracerContext.fullMethodName() != null) {
85-
logContext.put(
86-
ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE, apiTracerContext.fullMethodName());
87-
}
88-
if (apiTracerContext.serverPort() != null) {
89-
logContext.put(ObservabilityAttributes.SERVER_PORT_ATTRIBUTE, apiTracerContext.serverPort());
90-
}
91-
if (apiTracerContext.libraryMetadata() != null
92-
&& !apiTracerContext.libraryMetadata().isEmpty()) {
93-
if (apiTracerContext.libraryMetadata().repository() != null) {
94-
logContext.put(
95-
ObservabilityAttributes.REPO_ATTRIBUTE,
96-
apiTracerContext.libraryMetadata().repository());
97-
}
73+
if (error == null) {
74+
return;
9875
}
9976

100-
if (error != null) {
77+
Map<String, Object> logContext = new HashMap<>(apiTracerContext.getAttemptAttributes());
78+
79+
if (apiTracerContext.serviceName() != null) {
10180
logContext.put(
102-
ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE,
103-
ObservabilityUtils.extractStatus(error));
81+
ObservabilityAttributes.GCP_CLIENT_SERVICE_ATTRIBUTE, apiTracerContext.serviceName());
10482
}
10583

84+
logContext.put(
85+
ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE,
86+
ObservabilityUtils.extractStatus(error));
87+
10688
ErrorInfo errorInfo = ObservabilityUtils.extractErrorInfo(error);
10789
if (errorInfo != null) {
108-
logContext.put("error.type", errorInfo.getReason());
109-
logContext.put("gcp.errors.domain", errorInfo.getDomain());
110-
for (Map.Entry<String, String> entry : errorInfo.getMetadataMap().entrySet()) {
111-
logContext.put("gcp.errors.metadata." + entry.getKey(), entry.getValue());
90+
if (errorInfo.getReason() != null && !errorInfo.getReason().isEmpty()) {
91+
logContext.put(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE, errorInfo.getReason());
92+
}
93+
if (errorInfo.getDomain() != null && !errorInfo.getDomain().isEmpty()) {
94+
logContext.put(ObservabilityAttributes.ERROR_DOMAIN_ATTRIBUTE, errorInfo.getDomain());
95+
}
96+
if (errorInfo.getMetadataMap() != null) {
97+
for (Map.Entry<String, String> entry : errorInfo.getMetadataMap().entrySet()) {
98+
logContext.put(
99+
ObservabilityAttributes.ERROR_METADATA_ATTRIBUTE_PREFIX + entry.getKey(),
100+
entry.getValue());
101+
}
112102
}
113103
}
114104

115-
String message = "Unknown Error";
116-
if (error != null) {
117-
message = error.getMessage() != null ? error.getMessage() : error.getClass().getName();
118-
}
105+
String message = error.getMessage() != null ? error.getMessage() : error.getClass().getName();
119106
LoggingUtils.logActionableError(logContext, LOGGER_PROVIDER, message);
120107
}
121108
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType op
5454

5555
@Override
5656
public ApiTracer newTracer(ApiTracer parent, ApiTracerContext context) {
57-
return new LoggingTracer(context);
57+
return new LoggingTracer(apiTracerContext.merge(context));
5858
}
5959

6060
@Override

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,4 +93,13 @@ public class ObservabilityAttributes {
9393

9494
/** The destination resource id of the request (e.g. projects/p/locations/l/topics/t). */
9595
public static final String DESTINATION_RESOURCE_ID_ATTRIBUTE = "gcp.resource.destination.id";
96+
97+
/** The type of error that occurred (e.g., from google.rpc.ErrorInfo.reason). */
98+
public static final String ERROR_TYPE_ATTRIBUTE = "error.type";
99+
100+
/** The domain of the error (e.g., from google.rpc.ErrorInfo.domain). */
101+
public static final String ERROR_DOMAIN_ATTRIBUTE = "gcp.errors.domain";
102+
103+
/** The prefix for error metadata (e.g., from google.rpc.ErrorInfo.metadata). */
104+
public static final String ERROR_METADATA_ATTRIBUTE_PREFIX = "gcp.errors.metadata.";
96105
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,20 @@ public class TestLogger implements Logger, LoggingEventAware {
4848
List<String> messageList = new ArrayList<>();
4949
Level level;
5050

51+
public List<String> getMessageList() {
52+
return messageList;
53+
}
54+
5155
Map<String, Object> keyValuePairsMap = new HashMap<>();
5256

57+
public Map<String, String> getMDCMap() {
58+
return MDCMap;
59+
}
60+
61+
public Map<String, Object> getKeyValuePairsMap() {
62+
return keyValuePairsMap;
63+
}
64+
5365
private String loggerName;
5466
private boolean infoEnabled;
5567
private boolean debugEnabled;

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

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,11 @@
3030

3131
package com.google.api.gax.logging;
3232

33-
import static org.mockito.ArgumentMatchers.anyString;
34-
import static org.mockito.Mockito.mock;
35-
import static org.mockito.Mockito.when;
36-
33+
import java.util.concurrent.ConcurrentHashMap;
34+
import java.util.concurrent.ConcurrentMap;
3735
import org.slf4j.ILoggerFactory;
3836
import org.slf4j.IMarkerFactory;
37+
import org.slf4j.Logger;
3938
import org.slf4j.spi.MDCAdapter;
4039
import org.slf4j.spi.SLF4JServiceProvider;
4140

@@ -45,12 +44,18 @@
4544
*/
4645
public class TestServiceProvider implements SLF4JServiceProvider {
4746

47+
private final ConcurrentMap<String, Logger> loggers = new ConcurrentHashMap<>();
48+
private final ILoggerFactory loggerFactory =
49+
new ILoggerFactory() {
50+
@Override
51+
public Logger getLogger(String name) {
52+
return loggers.computeIfAbsent(name, TestLogger::new);
53+
}
54+
};
55+
4856
@Override
4957
public ILoggerFactory getLoggerFactory() {
50-
// mock behavior when provider present
51-
ILoggerFactory mockLoggerFactory = mock(ILoggerFactory.class);
52-
when(mockLoggerFactory.getLogger(anyString())).thenReturn(new TestLogger("test-logger"));
53-
return mockLoggerFactory;
58+
return loggerFactory;
5459
}
5560

5661
@Override

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ void testNewTracer_WithContext_CreatesLoggingTracer() {
6363
@Test
6464
void testWithContext_ReturnsNewFactoryWithMergedContext() {
6565
LoggingTracerFactory factory = new LoggingTracerFactory();
66-
ApiTracerContext context = ApiTracerContext.newBuilder().setServerAddress("address").build();
66+
ApiTracerContext context =
67+
ApiTracerContext.empty().toBuilder().setServerAddress("address").build();
6768
ApiTracerFactory updatedFactory = factory.withContext(context);
6869

6970
assertNotNull(updatedFactory);

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

Lines changed: 87 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,18 @@
3030

3131
package com.google.api.gax.tracing;
3232

33+
import static org.junit.jupiter.api.Assertions.assertEquals;
34+
import static org.junit.jupiter.api.Assertions.assertTrue;
35+
3336
import com.google.api.gax.logging.TestLogger;
37+
import com.google.api.gax.rpc.ApiExceptionFactory;
38+
import com.google.api.gax.rpc.ErrorDetails;
39+
import com.google.api.gax.rpc.StatusCode;
40+
import com.google.api.gax.rpc.testing.FakeStatusCode;
41+
import com.google.protobuf.Any;
42+
import com.google.rpc.ErrorInfo;
43+
import java.util.Collections;
44+
import java.util.Map;
3445
import org.junit.jupiter.api.BeforeEach;
3546
import org.junit.jupiter.api.Test;
3647
import org.slf4j.LoggerFactory;
@@ -42,6 +53,9 @@ class LoggingTracerTest {
4253
@BeforeEach
4354
void setUp() {
4455
testLogger = (TestLogger) LoggerFactory.getLogger(LoggingTracer.class);
56+
testLogger.getMessageList().clear();
57+
testLogger.getMDCMap().clear();
58+
testLogger.getKeyValuePairsMap().clear();
4559
}
4660

4761
@Test
@@ -51,10 +65,79 @@ void testAttemptFailed_LogsError() {
5165

5266
// Call attemptFailed with a generic exception
5367
Exception error = new RuntimeException("generic failure");
54-
tracer.attemptFailed(error, org.threeten.bp.Duration.ZERO);
68+
tracer.attemptFailedDuration(error, java.time.Duration.ZERO);
69+
70+
assertEquals(1, testLogger.getMessageList().size());
71+
assertEquals("generic failure", testLogger.getMessageList().get(0));
72+
}
73+
74+
@Test
75+
void testAttemptFailed_LogsErrorAndAttributes() {
76+
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();
84+
LoggingTracer tracer = new LoggingTracer(context);
85+
86+
ErrorInfo errorInfo =
87+
ErrorInfo.newBuilder()
88+
.setReason("TEST_REASON")
89+
.setDomain("test.domain.com")
90+
.putMetadata("test_key", "test_value")
91+
.build();
92+
93+
ErrorDetails errorDetails =
94+
ErrorDetails.builder()
95+
.setRawErrorMessages(Collections.singletonList(Any.pack(errorInfo)))
96+
.build();
97+
98+
Exception error =
99+
ApiExceptionFactory.createException(
100+
"test error message",
101+
new RuntimeException("cause"),
102+
FakeStatusCode.of(StatusCode.Code.INVALID_ARGUMENT),
103+
false,
104+
errorDetails);
105+
106+
tracer.attemptFailedDuration(error, java.time.Duration.ZERO);
107+
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+
}
119+
120+
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)));
55128

56-
// To prevent failing due to disabled logging or other missing context,
57-
// we don't strictly assert the contents of the log here if the logger isn't enabled.
58-
// The main verification is that calling attemptFailed doesn't throw.
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));
136+
assertEquals("TEST_REASON", attributesMap.get(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE));
137+
assertEquals(
138+
"test.domain.com", attributesMap.get(ObservabilityAttributes.ERROR_DOMAIN_ATTRIBUTE));
139+
assertEquals(
140+
"test_value",
141+
attributesMap.get(ObservabilityAttributes.ERROR_METADATA_ATTRIBUTE_PREFIX + "test_key"));
59142
}
60143
}

0 commit comments

Comments
 (0)