Skip to content

Commit 7a927d6

Browse files
westarlewestarle
committed
test(logging): address PR review feedback
- Removed the `envVarTest` profile and reused the existing `slf4j2_logback` profile for running the actionable error logs test. - Refactored `ITActionableErrorsLogging` to use `TestAppender` to align with existing test patterns in `ITLogging`. - Replaced hardcoded observability attribute string literals with static constants from `ObservabilityAttributes`.
1 parent 4773130 commit 7a927d6

File tree

2 files changed

+73
-92
lines changed

2 files changed

+73
-92
lines changed

‎sdk-platform-java/java-showcase/gapic-showcase/pom.xml‎

Lines changed: 0 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -74,16 +74,6 @@
7474
</execution>
7575
</executions>
7676
</plugin>
77-
<plugin>
78-
<groupId>org.apache.maven.plugins</groupId>
79-
<artifactId>maven-failsafe-plugin</artifactId>
80-
<configuration>
81-
<!-- These tests require an Env Var to be set. Use -PenvVarTest to ONLY run these tests -->
82-
<excludes>
83-
<exclude>**/ITActionableErrorsLogging.java</exclude>
84-
</excludes>
85-
</configuration>
86-
</plugin>
8777
</plugins>
8878
</build>
8979

@@ -302,7 +292,6 @@
302292
<testExclude>**/com/google/showcase/v1beta1/it/*.java</testExclude>
303293
<testExclude>**/com/google/showcase/v1beta1/it/logging/ITLoggingDisabled.java</testExclude>
304294
<testExclude>**/com/google/showcase/v1beta1/it/logging/ITLogging1x.java</testExclude>
305-
<testExclude>**/com/google/showcase/v1beta1/it/logging/ITActionableErrorsLogging.java</testExclude>
306295
</testExcludes>
307296
</configuration>
308297
</plugin>
@@ -419,51 +408,6 @@
419408
</plugins>
420409
</build>
421410
</profile>
422-
<profile>
423-
<id>envVarTest</id>
424-
<dependencies>
425-
<dependency>
426-
<groupId>org.slf4j</groupId>
427-
<artifactId>slf4j-api</artifactId>
428-
<scope>test</scope>
429-
</dependency>
430-
</dependencies>
431-
<build>
432-
<plugins>
433-
<plugin>
434-
<groupId>org.apache.maven.plugins</groupId>
435-
<artifactId>maven-compiler-plugin</artifactId>
436-
<configuration>
437-
<testExcludes combine.self="override" />
438-
<testIncludes>
439-
<testInclude>**/ITActionableErrorsLogging.java</testInclude>
440-
</testIncludes>
441-
</configuration>
442-
</plugin>
443-
<plugin>
444-
<groupId>org.apache.maven.plugins</groupId>
445-
<artifactId>maven-failsafe-plugin</artifactId>
446-
<configuration>
447-
<!-- Clear excludes so it runs -->
448-
<excludes combine.self="override" />
449-
<includes>
450-
<include>**/ITActionableErrorsLogging.java</include>
451-
</includes>
452-
<additionalClasspathElements>
453-
<additionalClasspathElement>${project.basedir}/src/test/slf4j-test-provider</additionalClasspathElement>
454-
</additionalClasspathElements>
455-
<classpathDependencyExcludes>
456-
<classpathDependencyExclude>ch.qos.logback:logback-classic</classpathDependencyExclude>
457-
<classpathDependencyExclude>ch.qos.logback:logback-core</classpathDependencyExclude>
458-
</classpathDependencyExcludes>
459-
<environmentVariables>
460-
<GOOGLE_SDK_JAVA_LOGGING>true</GOOGLE_SDK_JAVA_LOGGING>
461-
</environmentVariables>
462-
</configuration>
463-
</plugin>
464-
</plugins>
465-
</build>
466-
</profile>
467411
</profiles>
468412

469413
</project>

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

Lines changed: 73 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,17 @@
1919
import static com.google.common.truth.Truth.assertThat;
2020
import static org.junit.jupiter.api.Assertions.assertThrows;
2121

22+
import ch.qos.logback.classic.Level;
23+
import ch.qos.logback.classic.spi.ILoggingEvent;
2224
import com.google.api.client.http.LowLevelHttpRequest;
2325
import com.google.api.client.http.LowLevelHttpResponse;
2426
import com.google.api.client.testing.http.MockHttpTransport;
2527
import com.google.api.client.testing.http.MockLowLevelHttpRequest;
2628
import com.google.api.client.testing.http.MockLowLevelHttpResponse;
2729
import com.google.api.gax.core.NoCredentialsProvider;
28-
import com.google.api.gax.logging.TestLogger;
2930
import com.google.api.gax.rpc.ApiException;
3031
import com.google.api.gax.tracing.LoggingTracerFactory;
32+
import com.google.api.gax.tracing.ObservabilityAttributes;
3133
import com.google.protobuf.Any;
3234
import com.google.rpc.ErrorInfo;
3335
import com.google.rpc.Status;
@@ -36,19 +38,22 @@
3638
import com.google.showcase.v1beta1.EchoSettings;
3739
import com.google.showcase.v1beta1.it.util.TestClientInitializer;
3840
import java.io.IOException;
41+
import java.util.HashMap;
3942
import java.util.Map;
4043
import java.util.concurrent.TimeUnit;
4144
import org.junit.jupiter.api.AfterAll;
45+
import org.junit.jupiter.api.AfterEach;
4246
import org.junit.jupiter.api.BeforeAll;
4347
import org.junit.jupiter.api.BeforeEach;
4448
import org.junit.jupiter.api.Test;
4549
import org.slf4j.LoggerFactory;
50+
import org.slf4j.event.KeyValuePair;
4651

4752
public class ITActionableErrorsLogging {
4853

4954
private static EchoClient grpcClient;
5055
private static EchoClient httpjsonClient;
51-
private TestLogger testLogger;
56+
private TestAppender testAppender;
5257

5358
@BeforeAll
5459
static void createClients() throws Exception {
@@ -68,11 +73,36 @@ static void destroyClients() throws InterruptedException {
6873
TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS);
6974
}
7075

76+
private TestAppender setupTestLogger(String loggerName, Level level) {
77+
TestAppender appender = new TestAppender();
78+
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+
return appender;
83+
}
84+
7185
@BeforeEach
7286
void setupTestLogger() {
73-
testLogger = (TestLogger) LoggerFactory.getLogger("com.google.api.gax.tracing.LoggingTracer");
74-
testLogger.getMessageList().clear();
75-
testLogger.getKeyValuePairsMap().clear();
87+
testAppender = setupTestLogger("com.google.api.gax.tracing.LoggingTracer", Level.DEBUG);
88+
testAppender.clearEvents();
89+
}
90+
91+
@AfterEach
92+
void teardownTestLogger() {
93+
if (testAppender != null) {
94+
testAppender.stop();
95+
}
96+
}
97+
98+
private Map<String, Object> getKvps(ILoggingEvent loggingEvent) {
99+
Map<String, Object> map = new HashMap<>();
100+
if (loggingEvent.getKeyValuePairs() != null) {
101+
for (KeyValuePair kvp : loggingEvent.getKeyValuePairs()) {
102+
map.put(kvp.key, kvp.value);
103+
}
104+
}
105+
return map;
76106
}
77107

78108
private EchoRequest buildErrorRequest() {
@@ -147,19 +177,21 @@ public LowLevelHttpResponse execute() throws IOException {
147177
EchoRequest request = EchoRequest.newBuilder().build();
148178
assertThrows(ApiException.class, () -> mockHttpJsonClient.echo(request));
149179

150-
assertThat(testLogger.getMessageList().size()).isAtLeast(1);
151-
String loggedMessage = testLogger.getMessageList().get(testLogger.getMessageList().size() - 1);
180+
assertThat(testAppender.events.size()).isAtLeast(1);
181+
ILoggingEvent loggingEvent = testAppender.events.get(testAppender.events.size() - 1);
152182

153-
assertThat(loggedMessage).contains("This is a mock JSON error generated by the server");
183+
assertThat(loggingEvent.getMessage()).contains("This is a mock JSON error generated by the server");
154184

155-
Map<String, Object> kvps = testLogger.getKeyValuePairsMap();
156-
assertThat(kvps).containsEntry("rpc.system.name", "http");
157-
assertThat(kvps).containsEntry("http.request.method", "POST");
158-
assertThat(kvps).containsEntry("url.template", "v1beta1/echo:echo");
159-
assertThat(kvps).containsEntry("rpc.response.status_code", "ABORTED");
160-
assertThat(kvps).containsEntry("error.type", "mock_error_reason");
161-
assertThat(kvps).containsEntry("gcp.errors.domain", "mock.googleapis.com");
162-
assertThat(kvps).containsEntry("gcp.errors.metadata.mock_key", "mock_value");
185+
Map<String, Object> kvps = getKvps(loggingEvent);
186+
assertThat(kvps).containsEntry(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE, "http");
187+
assertThat(kvps).containsEntry(ObservabilityAttributes.HTTP_METHOD_ATTRIBUTE, "POST");
188+
assertThat(kvps).containsEntry(ObservabilityAttributes.HTTP_URL_TEMPLATE_ATTRIBUTE, "v1beta1/echo:echo");
189+
assertThat(kvps).containsEntry(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE, "ABORTED");
190+
assertThat(kvps).containsEntry(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE, "mock_error_reason");
191+
assertThat(kvps).containsEntry(ObservabilityAttributes.ERROR_DOMAIN_ATTRIBUTE, "mock.googleapis.com");
192+
assertThat(kvps)
193+
.containsEntry(
194+
ObservabilityAttributes.ERROR_METADATA_ATTRIBUTE_PREFIX + "mock_key", "mock_value");
163195

164196
mockHttpJsonClient.close();
165197
mockHttpJsonClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS);
@@ -169,7 +201,7 @@ public LowLevelHttpResponse execute() throws IOException {
169201
void testHttpJson_noLogEmittedForSuccess() {
170202
EchoRequest request = EchoRequest.newBuilder().setContent("Success").build();
171203
httpjsonClient.echo(request);
172-
assertThat(testLogger.getMessageList().size()).isEqualTo(0);
204+
assertThat(testAppender.events.size()).isEqualTo(0);
173205
}
174206

175207
@Test
@@ -188,9 +220,10 @@ void testHttpJson_clientLevelFailureAttributes() throws Exception {
188220
try (com.google.showcase.v1beta1.stub.EchoStub stub = stubSettingsBuilder.build().createStub();
189221
EchoClient client = EchoClient.create(stub)) {
190222
assertThrows(ApiException.class, () -> client.echo(EchoRequest.newBuilder().build()));
191-
assertThat(testLogger.getMessageList().size()).isAtLeast(1);
192-
Map<String, Object> kvps = testLogger.getKeyValuePairsMap();
193-
assertThat(kvps).containsEntry("rpc.system.name", "http");
223+
assertThat(testAppender.events.size()).isAtLeast(1);
224+
ILoggingEvent loggingEvent = testAppender.events.get(testAppender.events.size() - 1);
225+
Map<String, Object> kvps = getKvps(loggingEvent);
226+
assertThat(kvps).containsEntry(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE, "http");
194227
}
195228
}
196229

@@ -199,24 +232,27 @@ void testGrpc_logEmittedForLowLevelRequestFailure() {
199232
EchoRequest request = buildErrorRequest();
200233
assertThrows(ApiException.class, () -> grpcClient.echo(request));
201234

202-
assertThat(testLogger.getMessageList().size()).isAtLeast(1);
203-
String loggedMessage = testLogger.getMessageList().get(testLogger.getMessageList().size() - 1);
204-
assertThat(loggedMessage).contains("This is a test error");
205-
206-
Map<String, Object> kvps = testLogger.getKeyValuePairsMap();
207-
assertThat(kvps).containsEntry("rpc.system.name", "grpc");
208-
assertThat(kvps).containsEntry("rpc.method", "google.showcase.v1beta1.Echo/Echo");
209-
assertThat(kvps).containsEntry("rpc.response.status_code", "INVALID_ARGUMENT");
210-
assertThat(kvps).containsEntry("error.type", "TEST_REASON");
211-
assertThat(kvps).containsEntry("gcp.errors.domain", "test.googleapis.com");
212-
assertThat(kvps).containsEntry("gcp.errors.metadata.test_metadata", "test_value");
235+
assertThat(testAppender.events.size()).isAtLeast(1);
236+
ILoggingEvent loggingEvent = testAppender.events.get(testAppender.events.size() - 1);
237+
assertThat(loggingEvent.getMessage()).contains("This is a test error");
238+
239+
Map<String, Object> kvps = getKvps(loggingEvent);
240+
assertThat(kvps).containsEntry(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE, "grpc");
241+
assertThat(kvps)
242+
.containsEntry(ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE, "google.showcase.v1beta1.Echo/Echo");
243+
assertThat(kvps).containsEntry(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE, "INVALID_ARGUMENT");
244+
assertThat(kvps).containsEntry(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE, "TEST_REASON");
245+
assertThat(kvps).containsEntry(ObservabilityAttributes.ERROR_DOMAIN_ATTRIBUTE, "test.googleapis.com");
246+
assertThat(kvps)
247+
.containsEntry(
248+
ObservabilityAttributes.ERROR_METADATA_ATTRIBUTE_PREFIX + "test_metadata", "test_value");
213249
}
214250

215251
@Test
216252
void testGrpc_noLogEmittedForSuccess() {
217253
EchoRequest request = EchoRequest.newBuilder().setContent("Success").build();
218254
grpcClient.echo(request);
219-
assertThat(testLogger.getMessageList().size()).isEqualTo(0);
255+
assertThat(testAppender.events.size()).isEqualTo(0);
220256
}
221257

222258
@Test
@@ -235,9 +271,10 @@ void testGrpc_clientLevelFailureAttributes() throws Exception {
235271
try (com.google.showcase.v1beta1.stub.EchoStub stub = stubSettingsBuilder.build().createStub();
236272
EchoClient client = EchoClient.create(stub)) {
237273
assertThrows(ApiException.class, () -> client.echo(EchoRequest.newBuilder().build()));
238-
assertThat(testLogger.getMessageList().size()).isAtLeast(1);
239-
Map<String, Object> kvps = testLogger.getKeyValuePairsMap();
240-
assertThat(kvps).containsEntry("rpc.system.name", "grpc");
274+
assertThat(testAppender.events.size()).isAtLeast(1);
275+
ILoggingEvent loggingEvent = testAppender.events.get(testAppender.events.size() - 1);
276+
Map<String, Object> kvps = getKvps(loggingEvent);
277+
assertThat(kvps).containsEntry(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE, "grpc");
241278
}
242279
}
243-
}
280+
}

0 commit comments

Comments
 (0)