QosException reasons are included in SafeLoggable args (#930)

QosException reasons are included in SafeLoggable args for better observability when exceptions are wrapped and logged.
palantir · Dec 14, 2022 · cb06ccc · cb06ccc
1 parent 9ba9250
commit cb06ccc
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 4 deletions.
diff --git a/changelog/@unreleased/pr-930.v2.yml b/changelog/@unreleased/pr-930.v2.yml
@@ -0,0 +1,6 @@
+type: improvement
+improvement:
+  description: QosException reasons are included in SafeLoggable args for better observability
+    when exceptions are wrapped and logged.
+  links:
+  - https://github.com/palantir/conjure-java-runtime-api/pull/930
diff --git a/errors/build.gradle b/errors/build.gradle
@@ -10,6 +10,7 @@ dependencies {
     implementation "com.palantir.safe-logging:preconditions"
 
     testImplementation project(":extras:jackson-support")
+    testImplementation "com.palantir.safe-logging:preconditions-assertj"
     testImplementation "org.assertj:assertj-core"
     testImplementation "org.apache.commons:commons-lang3"
     testImplementation 'org.junit.jupiter:junit-jupiter'

diff --git a/errors/src/main/java/com/palantir/conjure/java/api/errors/QosException.java b/errors/src/main/java/com/palantir/conjure/java/api/errors/QosException.java
@@ -222,7 +222,7 @@ public String getLogMessage() {
 
         @Override
         public List<Arg<?>> getArgs() {
-            return Collections.singletonList(SafeArg.of("retryAfter", retryAfter.orElse(null)));
+            return List.of(SafeArg.of("retryAfter", retryAfter.orElse(null)), SafeArg.of("reason", getReason()));
         }
     }
 
@@ -269,7 +269,7 @@ public String getLogMessage() {
 
         @Override
         public List<Arg<?>> getArgs() {
-            return Collections.singletonList(UnsafeArg.of("redirectTo", redirectTo));
+            return List.of(UnsafeArg.of("redirectTo", redirectTo), SafeArg.of("reason", getReason()));
         }
     }
 
@@ -307,7 +307,7 @@ public String getLogMessage() {
 
         @Override
         public List<Arg<?>> getArgs() {
-            return Collections.emptyList();
+            return Collections.singletonList(SafeArg.of("reason", getReason()));
         }
     }
 }
diff --git a/errors/src/test/java/com/palantir/conjure/java/api/errors/QosExceptionTest.java b/errors/src/test/java/com/palantir/conjure/java/api/errors/QosExceptionTest.java
@@ -16,9 +16,11 @@
 
 package com.palantir.conjure.java.api.errors;
 
+import static com.palantir.logsafe.testing.Assertions.assertThatLoggableExceptionThrownBy;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
+import com.palantir.logsafe.SafeArg;
 import com.palantir.logsafe.exceptions.SafeIllegalArgumentException;
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -53,8 +55,12 @@ public Class visit(QosException.Unavailable exception) {
 
     @Test
     public void testReason() {
-        QosReason reason = QosReason.of("reason");
+        QosReason reason = QosReason.of("custom-reason");
         assertThat(QosException.throttle(reason).getReason()).isEqualTo(reason);
+        assertThatLoggableExceptionThrownBy(() -> {
+                    throw QosException.throttle(reason);
+                })
+                .containsArgs(SafeArg.of("reason", reason));
     }
 
     @Test