refactor(rt): add retry logging; rename retry middleware (#528)

Author: aajtodd

runtime/protocol/http/common/src/aws/smithy/kotlin/runtime/http/middleware/Md5Checksum.kt

@@ -11,6 +11,7 @@ import aws.smithy.kotlin.runtime.http.HttpBody
 import aws.smithy.kotlin.runtime.http.HttpClientFeatureFactory
 import aws.smithy.kotlin.runtime.http.operation.SdkHttpOperation
 import aws.smithy.kotlin.runtime.http.request.header
+import aws.smithy.kotlin.runtime.util.InternalApi
 import aws.smithy.kotlin.runtime.util.encodeBase64String
 import aws.smithy.kotlin.runtime.util.md5
 
@@ -20,6 +21,7 @@ import aws.smithy.kotlin.runtime.util.md5
  *   - https://awslabs.github.io/smithy/1.0/spec/core/behavior-traits.html#httpchecksumrequired-trait
  *   - https://datatracker.ietf.org/doc/html/rfc1864.html
  */
+@InternalApi
 class Md5Checksum : Feature {
 
     companion object Feature : HttpClientFeatureFactory<Md5Checksum, Md5Checksum> {

runtime/protocol/http/common/src/aws/smithy/kotlin/runtime/http/middleware/RetryFeature.kt renamed to runtime/protocol/http/common/src/aws/smithy/kotlin/runtime/http/middleware/Retry.kt

@@ -11,31 +11,67 @@ import aws.smithy.kotlin.runtime.http.HttpBody
 import aws.smithy.kotlin.runtime.http.HttpClientFeatureFactory
 import aws.smithy.kotlin.runtime.http.operation.SdkHttpOperation
 import aws.smithy.kotlin.runtime.http.operation.deepCopy
+import aws.smithy.kotlin.runtime.http.operation.getLogger
 import aws.smithy.kotlin.runtime.http.request.HttpRequestBuilder
+import aws.smithy.kotlin.runtime.logging.Logger
+import aws.smithy.kotlin.runtime.retries.RetryDirective
 import aws.smithy.kotlin.runtime.retries.RetryPolicy
 import aws.smithy.kotlin.runtime.retries.RetryStrategy
+import aws.smithy.kotlin.runtime.util.InternalApi
+
+/**
+ * Retry requests with the given strategy and policy
+ * @param strategy the [RetryStrategy] to retry failed requests with
+ * @param policy the [RetryPolicy] used to determine when to retry
+ */
+@InternalApi
+class Retry(
+    private val strategy: RetryStrategy,
+    private val policy: RetryPolicy<Any?>
+) : Feature {
 
-class RetryFeature(private val strategy: RetryStrategy, private val policy: RetryPolicy<Any?>) : Feature {
     class Config {
         var strategy: RetryStrategy? = null
         var policy: RetryPolicy<Any?>? = null
     }
 
-    companion object Feature : HttpClientFeatureFactory<Config, RetryFeature> {
-        override val key: FeatureKey<RetryFeature> = FeatureKey("RetryFeature")
+    companion object Feature : HttpClientFeatureFactory<Config, Retry> {
+        override val key: FeatureKey<Retry> = FeatureKey("Retry")
 
-        override fun create(block: Config.() -> Unit): RetryFeature {
+        override fun create(block: Config.() -> Unit): Retry {
             val config = Config().apply(block)
             val strategy = requireNotNull(config.strategy) { "strategy is required" }
             val policy = requireNotNull(config.policy) { "policy is required" }
-            return RetryFeature(strategy, policy)
+            return Retry(strategy, policy)
+        }
+    }
+
+    /**
+     * Wrapper around [policy] that logs termination decisions
+     */
+    private class PolicyLogger(
+        private val policy: RetryPolicy<Any?>,
+        private val logger: Logger,
+    ) : RetryPolicy<Any?> {
+        override fun evaluate(result: Result<Any?>): RetryDirective = policy.evaluate(result).also {
+            if (it is RetryDirective.TerminateAndFail) {
+                logger.debug { "request failed with non-retryable error" }
+            }
         }
     }
 
     override fun <I, O> install(operation: SdkHttpOperation<I, O>) {
         operation.execution.finalize.intercept { req, next ->
             if (req.subject.isRetryable) {
-                strategy.retry(policy) {
+                var attempt = 1
+                val logger = req.context.getLogger("Retry")
+                val wrappedPolicy = PolicyLogger(policy, logger)
+
+                strategy.retry(wrappedPolicy) {
+                    if (attempt > 1) {
+                        logger.debug { "retrying request, attempt $attempt" }
+                    }
+
                     // Deep copy the request because later middlewares (e.g., signing) mutate it
                     val reqCopy = req.deepCopy()
 
@@ -44,6 +80,8 @@ class RetryFeature(private val strategy: RetryStrategy, private val policy: Retr
                         is HttpBody.Streaming -> body.reset()
                     }
 
+                    attempt++
+
                     next.call(reqCopy)
                 }
             } else {

runtime/runtime-core/common/src/aws/smithy/kotlin/runtime/client/SdkLogMode.kt

@@ -50,13 +50,6 @@ sealed class SdkLogMode(private val mask: Int) {
         override fun toString(): String = "LogResponseWithBody"
     }
 
-    /**
-     * Log retry attempts
-     */
-    object LogRetries : SdkLogMode(0x10) {
-        override fun toString(): String = "LogRetries"
-    }
-
     internal class Composite(mask: Int) : SdkLogMode(mask)
 
     operator fun plus(mode: SdkLogMode): SdkLogMode = Composite(mask or mode.mask)
@@ -76,7 +69,6 @@ sealed class SdkLogMode(private val mask: Int) {
             LogRequestWithBody,
             LogResponse,
             LogResponseWithBody,
-            LogRetries
         )
     }
     override fun toString(): String =

runtime/runtime-core/common/test/aws/smithy/kotlin/runtime/client/SdkLogModeTest.kt

@@ -31,7 +31,7 @@ class SdkLogModeTest {
     fun testToString() {
         val mode = SdkLogMode.allModes().reduce { acc, curr -> acc + curr }
         assertTrue { SdkLogMode.allModes().all { mode.isEnabled(it) } }
-        val expected = "SdkLogMode(LogRequest|LogRequestWithBody|LogResponse|LogResponseWithBody|LogRetries)"
+        val expected = "SdkLogMode(LogRequest|LogRequestWithBody|LogResponse|LogResponseWithBody)"
         assertEquals(expected, mode.toString())
     }
 }

smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/RuntimeTypes.kt

@@ -49,7 +49,7 @@ object RuntimeTypes {
         object Middlware {
             val Md5ChecksumMiddleware = runtimeSymbol("Md5Checksum", KotlinDependency.HTTP, "middleware")
             val MutateHeadersMiddleware = runtimeSymbol("MutateHeaders", KotlinDependency.HTTP, "middleware")
-            val RetryFeature = runtimeSymbol("RetryFeature", KotlinDependency.HTTP, "middleware")
+            val Retry = runtimeSymbol("Retry", KotlinDependency.HTTP, "middleware")
             val ResolveEndpoint = runtimeSymbol("ResolveEndpoint", KotlinDependency.HTTP, "middleware")
         }
 

smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/retries/StandardRetryMiddleware.kt

@@ -13,14 +13,11 @@ import software.amazon.smithy.kotlin.codegen.rendering.protocol.HttpFeatureMiddl
  * Adds retry wrappers around operation invocations.
  */
 class StandardRetryMiddleware : HttpFeatureMiddleware() {
-    companion object {
-        const val name: String = "RetryFeature"
-    }
 
-    override val name: String = Companion.name
+    override val name: String = RuntimeTypes.Http.Middlware.Retry.name
 
     override fun renderConfigure(writer: KotlinWriter) {
-        writer.addImport(RuntimeTypes.Http.Middlware.RetryFeature)
+        writer.addImport(RuntimeTypes.Http.Middlware.Retry)
         writer.addImport(RuntimeTypes.Core.Retries.Impl.StandardRetryPolicy)
 
         writer.write("strategy = config.retryStrategy")