Skip to content

Commit 0f3624e

Browse files
authored
feat!: rename SdkLogMode to LogMode and enable its resolution from environment (#831)
1 parent 6d780ca commit 0f3624e

File tree

20 files changed

+451
-298
lines changed

20 files changed

+451
-298
lines changed
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"id": "19e30aaf-5796-40b3-8a83-a5f04641981c",
3+
"type": "feature",
4+
"description": "Enable resolving LogMode from environment",
5+
"issues": [
6+
"https://github.com/awslabs/aws-sdk-kotlin/issues/432"
7+
]
8+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"id": "86dde223-330a-49e2-b162-05fdb41c2848",
3+
"type": "feature",
4+
"description": "BREAKING: rename SdkLogMode to LogMode"
5+
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ object RuntimeTypes {
174174
object SmithyClient : RuntimeTypePackage(KotlinDependency.SMITHY_CLIENT) {
175175
val SdkClient = symbol("SdkClient")
176176
val AbstractSdkClientBuilder = symbol("AbstractSdkClientBuilder")
177-
val SdkLogMode = symbol("SdkLogMode")
177+
val LogMode = symbol("LogMode")
178178
val SdkClientConfig = symbol("SdkClientConfig")
179179
val SdkClientFactory = symbol("SdkClientFactory")
180180
val SdkClientOption = symbol("SdkClientOption")

codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/ServiceClientConfigGenerator.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class ServiceClientConfigGenerator(
4242
*/
4343
private fun detectDefaultProps(context: CodegenContext, shape: ServiceShape): List<ConfigProperty> = buildList {
4444
add(RuntimeConfigProperty.ClientName)
45-
add(RuntimeConfigProperty.SdkLogMode)
45+
add(RuntimeConfigProperty.LogMode)
4646
if (context.protocolGenerator?.applicationProtocol?.isHttpProtocol == true) {
4747
add(RuntimeConfigProperty.HttpClientEngine)
4848
add(RuntimeConfigProperty.HttpInterceptors)

codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/util/RuntimeConfigProperty.kt

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,12 @@ object RuntimeConfigProperty {
118118
additionalImports = listOf(RuntimeTypes.Core.Retries.StandardRetryStrategy)
119119
}
120120

121-
val SdkLogMode = ConfigProperty {
121+
val LogMode = ConfigProperty {
122122
symbol = buildSymbol {
123-
name = RuntimeTypes.SmithyClient.SdkLogMode.name
124-
namespace = RuntimeTypes.SmithyClient.SdkLogMode.namespace
125-
defaultValue = "SdkLogMode.Default"
126-
nullable = false
123+
name = RuntimeTypes.SmithyClient.LogMode.name
124+
namespace = RuntimeTypes.SmithyClient.LogMode.namespace
127125
}
126+
propertyType = ConfigPropertyType.RequiredWithDefault("LogMode.Default")
128127

129128
baseClass = RuntimeTypes.SmithyClient.SdkClientConfig
130129
builderBaseClass = buildSymbol {

codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/rendering/ServiceClientConfigGeneratorTest.kt

Lines changed: 30 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,9 @@ public class Config private constructor(builder: Builder) : HttpAuthConfig, Http
5353
public val endpointProvider: EndpointProvider = requireNotNull(builder.endpointProvider) { "endpointProvider is a required configuration property" }
5454
override val idempotencyTokenProvider: IdempotencyTokenProvider = builder.idempotencyTokenProvider ?: IdempotencyTokenProvider.Default
5555
override val interceptors: kotlin.collections.List<aws.smithy.kotlin.runtime.http.interceptors.HttpInterceptor> = builder.interceptors
56+
override val logMode: LogMode = builder.logMode ?: LogMode.Default
5657
override val retryPolicy: RetryPolicy<Any?> = builder.retryPolicy ?: StandardRetryPolicy.Default
5758
override val retryStrategy: RetryStrategy = builder.retryStrategy ?: StandardRetryStrategy()
58-
override val sdkLogMode: SdkLogMode = builder.sdkLogMode
5959
override val tracer: Tracer = builder.tracer ?: DefaultTracer(LoggingTraceProbe, clientName)
6060
"""
6161
contents.shouldContainWithDiff(expectedProps)
@@ -106,17 +106,6 @@ public class Config private constructor(builder: Builder) : HttpAuthConfig, Http
106106
*/
107107
override var interceptors: kotlin.collections.MutableList<aws.smithy.kotlin.runtime.http.interceptors.HttpInterceptor> = kotlin.collections.mutableListOf()
108108
109-
/**
110-
* The policy to use for evaluating operation results and determining whether/how to retry.
111-
*/
112-
override var retryPolicy: RetryPolicy<Any?>? = null
113-
114-
/**
115-
* The [RetryStrategy] implementation to use for service calls. All API calls will be wrapped by the
116-
* strategy.
117-
*/
118-
override var retryStrategy: RetryStrategy? = null
119-
120109
/**
121110
* Configure events that will be logged. By default clients will not output
122111
* raw requests or responses. Use this setting to opt-in to additional debug logging.
@@ -127,7 +116,18 @@ public class Config private constructor(builder: Builder) : HttpAuthConfig, Http
127116
* performance considerations when dumping the request/response body. This is primarily a tool for
128117
* debug purposes.
129118
*/
130-
override var sdkLogMode: SdkLogMode = SdkLogMode.Default
119+
override var logMode: LogMode? = null
120+
121+
/**
122+
* The policy to use for evaluating operation results and determining whether/how to retry.
123+
*/
124+
override var retryPolicy: RetryPolicy<Any?>? = null
125+
126+
/**
127+
* The [RetryStrategy] implementation to use for service calls. All API calls will be wrapped by the
128+
* strategy.
129+
*/
130+
override var retryStrategy: RetryStrategy? = null
131131
132132
/**
133133
* The tracer that is responsible for creating trace spans and wiring them up to a tracing backend (e.g.,
@@ -148,7 +148,7 @@ public class Config private constructor(builder: Builder) : HttpAuthConfig, Http
148148
"import ${KotlinDependency.CORE.namespace}.client.IdempotencyTokenConfig",
149149
"import ${KotlinDependency.CORE.namespace}.client.IdempotencyTokenProvider",
150150
"import ${KotlinDependency.CORE.namespace}.client.SdkClientConfig",
151-
"import ${KotlinDependency.CORE.namespace}.client.SdkLogMode",
151+
"import ${KotlinDependency.CORE.namespace}.client.LogMode",
152152
)
153153
expectedImports.forEach {
154154
contents.shouldContainWithDiff(it)
@@ -237,10 +237,8 @@ public class Config private constructor(builder: Builder) {
237237
val testCtx = model.newTestContext()
238238
val writer = createWriter()
239239
val customIntegration = object : KotlinIntegration {
240-
private val overriddenLogMode = RuntimeConfigProperty.SdkLogMode.toBuilder().apply {
241-
symbol = symbol!!.toBuilder().apply {
242-
defaultValue("SdkLogMode.LogRequest") // replaces SdkLogMode.Default
243-
}.build()
240+
private val overriddenLogMode = RuntimeConfigProperty.LogMode.toBuilder().apply {
241+
propertyType = ConfigPropertyType.RequiredWithDefault("LogMode.LogRequest") // replaces LogMode.Default
244242
}.build()
245243

246244
override fun additionalServiceConfigProps(ctx: CodegenContext): List<ConfigProperty> = listOf(
@@ -255,85 +253,20 @@ public class Config private constructor(builder: Builder) {
255253
ServiceClientConfigGenerator(serviceShape, detectDefaultProps = true).render(renderingCtx, renderingCtx.writer)
256254
val contents = writer.toString()
257255

258-
val expectedBuilderProps = """
259-
/**
260-
* A reader-friendly name for the client.
261-
*/
262-
override var clientName: String = "Test"
263-
264-
/**
265-
* Override the default HTTP client engine used to make SDK requests (e.g. configure proxy behavior, timeouts, concurrency, etc).
266-
* NOTE: The caller is responsible for managing the lifetime of the engine when set. The SDK
267-
* client will not close it when the client is closed.
268-
*/
269-
override var httpClientEngine: HttpClientEngine? = null
270-
271-
/**
272-
* Register new or override default [HttpAuthScheme]s configured for this client. By default, the set
273-
* of auth schemes configured comes from the service model. An auth scheme configured explicitly takes
274-
* precedence over the defaults and can be used to customize identity resolution and signing for specific
275-
* authentication schemes.
276-
*/
277-
override var authSchemes: kotlin.collections.List<aws.smithy.kotlin.runtime.http.auth.HttpAuthScheme> = emptyList()
278-
279-
public var customProp: Int? = null
280-
281-
/**
282-
* The endpoint provider used to determine where to make service requests. **This is an advanced config
283-
* option.**
284-
*
285-
* Endpoint resolution occurs as part of the workflow for every request made via the service client.
286-
*
287-
* The inputs to endpoint resolution are defined on a per-service basis (see [EndpointParameters]).
288-
*/
289-
public var endpointProvider: EndpointProvider? = null
290-
291-
/**
292-
* Override the default idempotency token generator. SDK clients will generate tokens for members
293-
* that represent idempotent tokens when not explicitly set by the caller using this generator.
294-
*/
295-
override var idempotencyTokenProvider: IdempotencyTokenProvider? = null
296-
297-
/**
298-
* Add an [aws.smithy.kotlin.runtime.client.Interceptor] that will have access to read and modify
299-
* the request and response objects as they are processed by the SDK.
300-
* Interceptors added using this method are executed in the order they are configured and are always
301-
* later than any added automatically by the SDK.
302-
*/
303-
override var interceptors: kotlin.collections.MutableList<aws.smithy.kotlin.runtime.http.interceptors.HttpInterceptor> = kotlin.collections.mutableListOf()
304-
305-
/**
306-
* The policy to use for evaluating operation results and determining whether/how to retry.
307-
*/
308-
override var retryPolicy: RetryPolicy<Any?>? = null
309-
310-
/**
311-
* The [RetryStrategy] implementation to use for service calls. All API calls will be wrapped by the
312-
* strategy.
313-
*/
314-
override var retryStrategy: RetryStrategy? = null
315-
316-
/**
317-
* Configure events that will be logged. By default clients will not output
318-
* raw requests or responses. Use this setting to opt-in to additional debug logging.
319-
*
320-
* This can be used to configure logging of requests, responses, retries, etc of SDK clients.
321-
*
322-
* **NOTE**: Logging of raw requests or responses may leak sensitive information! It may also have
323-
* performance considerations when dumping the request/response body. This is primarily a tool for
324-
* debug purposes.
325-
*/
326-
override var sdkLogMode: SdkLogMode = SdkLogMode.LogRequest
327-
328-
/**
329-
* The tracer that is responsible for creating trace spans and wiring them up to a tracing backend (e.g.,
330-
* a trace probe). By default, this will create a standard tracer that uses the service name for the root
331-
* trace span and delegates to a logging trace probe (i.e.,
332-
* `DefaultTracer(LoggingTraceProbe, "<service-name>")`).
333-
*/
334-
override var tracer: Tracer? = null
335-
"""
336-
contents.shouldContainWithDiff(expectedBuilderProps)
256+
// Expect logMode config value to override default to LogMode.Request
257+
val expectedConfigValues = """
258+
override val clientName: String = builder.clientName
259+
override val httpClientEngine: HttpClientEngine = builder.httpClientEngine ?: DefaultHttpEngine().manage()
260+
override val authSchemes: kotlin.collections.List<aws.smithy.kotlin.runtime.http.auth.HttpAuthScheme> = builder.authSchemes
261+
public val customProp: Int? = builder.customProp
262+
public val endpointProvider: EndpointProvider = requireNotNull(builder.endpointProvider) { "endpointProvider is a required configuration property" }
263+
override val idempotencyTokenProvider: IdempotencyTokenProvider = builder.idempotencyTokenProvider ?: IdempotencyTokenProvider.Default
264+
override val interceptors: kotlin.collections.List<aws.smithy.kotlin.runtime.http.interceptors.HttpInterceptor> = builder.interceptors
265+
override val logMode: LogMode = builder.logMode ?: LogMode.LogRequest
266+
override val retryPolicy: RetryPolicy<Any?> = builder.retryPolicy ?: StandardRetryPolicy.Default
267+
override val retryStrategy: RetryStrategy = builder.retryStrategy ?: StandardRetryStrategy()
268+
override val tracer: Tracer = builder.tracer ?: DefaultTracer(LoggingTraceProbe, clientName)"""
269+
contents.shouldContainWithDiff(expectedConfigValues)
337270
}
338271

339272
@Test

docs/design/client-configuration.md

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public interface BazClient : SdkClient { // 1
7676
}
7777

7878
public class Config private constructor(builder: Builder) : SdkClientConfig, HttpClientConfig { // 6
79-
override val sdkLogMode: SdkLogMode = builder.sdkLogMode // 7
79+
override val logMode: LogMode = builder.logMode // 7
8080
override val httpClientEngine: HttpClientEngine? = builder.httpClientEngine
8181
val bazSpecificConfig: String? = builder.bazSpecificConfig
8282

@@ -93,7 +93,7 @@ public interface BazClient : SdkClient { // 1
9393
/**
9494
* Description
9595
*/
96-
override var sdkLogMode: SdkLogMode = SdkLogMode.Default
96+
override var logMode: LogMode = LogMode.Default
9797

9898
/**
9999
* Description
@@ -130,12 +130,12 @@ codegen.
130130
```kotlin
131131
// explicit using DSL syntax inherited from companion `SdkClientFactory`
132132
val c1 = BazClient { // this: BazClient.Config.Builder
133-
sdkLogMode = SdkLogMode.LogRequest
133+
logMode = LogMode.LogRequest
134134
}
135135

136136
// use of a builder explicitly, this could be passed around for example
137137
val c2 = BazClient.builder().apply { // this: BazClient.Builder
138-
config.sdkLogMode = SdkLogMode.LogRequest
138+
config.logMode = LogMode.LogRequest
139139
}.build()
140140

141141
// "vended" using common code
@@ -162,7 +162,7 @@ private object ClientVendingMachine {
162162
}
163163
else -> {
164164
// always available from generic bounds:
165-
config.sdkLogMode
165+
config.logMode
166166
}
167167
}
168168
return builder.build()
@@ -239,10 +239,10 @@ public interface SdkClient : Closeable {
239239
*/
240240
public interface SdkClientConfig {
241241
/**
242-
* Controls the events that will be logged by the SDK, see [Builder.sdkLogMode].
242+
* Controls the events that will be logged by the SDK, see [Builder.logMode].
243243
*/
244-
public val sdkLogMode: SdkLogMode
245-
get() = SdkLogMode.Default
244+
public val logMode: LogMode
245+
get() = LogMode.Default
246246

247247
public interface Builder<TConfig : SdkClientConfig> : Buildable<TConfig> {
248248
/**
@@ -255,7 +255,7 @@ public interface SdkClientConfig {
255255
* performance considerations when dumping the request/response body. This is primarily a tool for
256256
* debug purposes.
257257
*/
258-
public var sdkLogMode: SdkLogMode
258+
public var logMode: LogMode
259259
}
260260
}
261261
```

docs/design/per-op-config.md

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@ interface ServiceClient : SdkClient {
2626
}
2727

2828
class Config private constructor(builder: Builder) {
29-
val logMode: SdkLogMode = builder.logMode ?: SdkLogMode.Default
29+
val logMode: LogMode = builder.logMode ?: LogMode.Default
3030
val httpEngine: HttpClientEngine = builder.httpEngine
3131

3232
class Builder {
33-
var logMode: SdkLogMode? = null
33+
var logMode: LogMode? = null
3434
val httpEngine: HttpClientEngine? = null
3535
fun build(): Config = Config(this)
3636
}
@@ -66,14 +66,14 @@ suspend fun main() {
6666
client.operationA { /* ... */ }
6767

6868
client.withConfig {
69-
logMode = SdkLogMode.LogRequestWithBody
69+
logMode = LogMode.LogRequestWithBody
7070
}.use { // for one-off or explicitly-scoped operation(s), a use {} block is most convenient
7171
it.operationA { /* ... */ }
7272
}
7373

7474
launch {
7575
doBackgroundRoutine(client.withConfig {
76-
logMode = SdkLogMode.LogResponse
76+
logMode = LogMode.LogResponse
7777
})
7878

7979
doBackgroundRoutine2(client.withConfig {
@@ -125,7 +125,7 @@ suspend fun main() {
125125
val requestA = OperationARequest { /* ... */ }
126126
client.operationA(requestA)
127127
client.operationA(requestA, client.config.copy {
128-
logMode = SdkLogMode.LogRequestWithBody
128+
logMode = LogMode.LogRequestWithBody
129129
})
130130

131131
val requestB = OperationBRequest { /* ... */ }
@@ -134,7 +134,7 @@ suspend fun main() {
134134
// existing builder extension - build request here
135135
}
136136
client.operationB(requestB) {
137-
logMode = SdkLogMode.LogResponseWithBody
137+
logMode = LogMode.LogResponseWithBody
138138
}
139139
}
140140
```
@@ -175,7 +175,7 @@ suspend fun main() {
175175
it.operationA {
176176
requestValue = 2
177177
withConfig {
178-
logMode = SdkLogMode.LogRequestWithBody
178+
logMode = LogMode.LogRequestWithBody
179179
}
180180
}
181181
}
@@ -214,7 +214,7 @@ suspend fun main() {
214214
client.operationA { /* ... */ }
215215

216216
client.withConfig({
217-
logMode = SdkLogMode.LogRequestWithBody + SdkLogMode.LogResponseWithBody
217+
logMode = LogMode.LogRequestWithBody + LogMode.LogResponseWithBody
218218
}) {
219219
client.operationA { /* ... */ }
220220
}

runtime/protocol/http-client/common/src/aws/smithy/kotlin/runtime/http/operation/SdkOperationExecution.kt

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
package aws.smithy.kotlin.runtime.http.operation
77

88
import aws.smithy.kotlin.runtime.InternalApi
9-
import aws.smithy.kotlin.runtime.client.SdkLogMode
10-
import aws.smithy.kotlin.runtime.client.sdkLogMode
9+
import aws.smithy.kotlin.runtime.client.LogMode
10+
import aws.smithy.kotlin.runtime.client.logMode
1111
import aws.smithy.kotlin.runtime.http.HttpHandler
1212
import aws.smithy.kotlin.runtime.http.auth.SignHttpRequest
1313
import aws.smithy.kotlin.runtime.http.interceptors.InterceptorExecutor
@@ -346,18 +346,18 @@ private class HttpCallMiddleware : Middleware<SdkHttpRequest, HttpCall> {
346346
* default middleware that logs requests/responses
347347
*/
348348
private suspend fun httpTraceMiddleware(request: SdkHttpRequest, next: Handler<SdkHttpRequest, HttpCall>): HttpCall {
349-
val logMode = request.context.sdkLogMode
349+
val logMode = request.context.logMode
350350
val logger = coroutineContext.getLogger("httpTraceMiddleware")
351351

352-
if (logMode.isEnabled(SdkLogMode.LogRequest) || logMode.isEnabled(SdkLogMode.LogRequestWithBody)) {
353-
val formattedReq = dumpRequest(request.subject, logMode.isEnabled(SdkLogMode.LogRequestWithBody))
352+
if (logMode.isEnabled(LogMode.LogRequest) || logMode.isEnabled(LogMode.LogRequestWithBody)) {
353+
val formattedReq = dumpRequest(request.subject, logMode.isEnabled(LogMode.LogRequestWithBody))
354354
logger.debug { "HttpRequest:\n$formattedReq" }
355355
}
356356

357357
var call = next.call(request)
358358

359-
if (logMode.isEnabled(SdkLogMode.LogResponse) || logMode.isEnabled(SdkLogMode.LogResponseWithBody)) {
360-
val (resp, formattedResp) = dumpResponse(call.response, logMode.isEnabled(SdkLogMode.LogResponseWithBody))
359+
if (logMode.isEnabled(LogMode.LogResponse) || logMode.isEnabled(LogMode.LogResponseWithBody)) {
360+
val (resp, formattedResp) = dumpResponse(call.response, logMode.isEnabled(LogMode.LogResponseWithBody))
361361
call = call.copy(response = resp)
362362
logger.debug { "HttpResponse:\n$formattedResp" }
363363
} else {

0 commit comments

Comments
 (0)