From cfc9823e914d7cfba8811b8789d1b7e8f8c30c1c Mon Sep 17 00:00:00 2001 From: 0marperez Date: Tue, 11 Jun 2024 14:42:19 -0400 Subject: [PATCH] pr feedback --- aws-runtime/aws-http/api/aws-http.api | 9 ++++ .../runtime/http/AwsUserAgentMetadata.kt | 1 + .../BusinessMetricsInterceptor.kt | 30 ++++++++------ .../BusinessMetricsInterceptorTest.kt | 41 +++++++++++++------ .../aws/sdk/kotlin/codegen/AwsRuntimeTypes.kt | 1 + .../codegen/BusinessMetricsIntegration.kt | 19 ++++++--- 6 files changed, 72 insertions(+), 29 deletions(-) diff --git a/aws-runtime/aws-http/api/aws-http.api b/aws-runtime/aws-http/api/aws-http.api index 3d64454ab83..e6355231cb9 100644 --- a/aws-runtime/aws-http/api/aws-http.api +++ b/aws-runtime/aws-http/api/aws-http.api @@ -48,6 +48,7 @@ public final class aws/sdk/kotlin/runtime/http/AwsUserAgentMetadata$Companion { public final class aws/sdk/kotlin/runtime/http/AwsUserAgentMetadataKt { public static final field AWS_APP_ID_ENV Ljava/lang/String; public static final field AWS_APP_ID_PROP Ljava/lang/String; + public static final field BUSINESS_METRICS_MAX_SIZE I } public final class aws/sdk/kotlin/runtime/http/ExecutionEnvMetadata { @@ -185,6 +186,14 @@ public final class aws/sdk/kotlin/runtime/http/interceptors/BusinessMetricsInter public fun readBeforeTransmit (Laws/smithy/kotlin/runtime/client/ProtocolRequestInterceptorContext;)V } +public final class aws/sdk/kotlin/runtime/http/interceptors/SdkBusinessMetric : java/lang/Enum, aws/smithy/kotlin/runtime/businessmetrics/BusinessMetric { + public static final field S3_EXPRESS_BUCKET Laws/sdk/kotlin/runtime/http/interceptors/SdkBusinessMetric; + public static fun getEntries ()Lkotlin/enums/EnumEntries; + public fun getIdentifier ()Ljava/lang/String; + public static fun valueOf (Ljava/lang/String;)Laws/sdk/kotlin/runtime/http/interceptors/SdkBusinessMetric; + public static fun values ()[Laws/sdk/kotlin/runtime/http/interceptors/SdkBusinessMetric; +} + public final class aws/sdk/kotlin/runtime/http/interceptors/UnsupportedSigningAlgorithmInterceptor : aws/smithy/kotlin/runtime/client/Interceptor { public fun ()V public fun modifyBeforeAttemptCompletion-gIAlu-s (Laws/smithy/kotlin/runtime/client/ResponseInterceptorContext;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; diff --git a/aws-runtime/aws-http/common/src/aws/sdk/kotlin/runtime/http/AwsUserAgentMetadata.kt b/aws-runtime/aws-http/common/src/aws/sdk/kotlin/runtime/http/AwsUserAgentMetadata.kt index a5bf36461f6..31de4c9921e 100644 --- a/aws-runtime/aws-http/common/src/aws/sdk/kotlin/runtime/http/AwsUserAgentMetadata.kt +++ b/aws-runtime/aws-http/common/src/aws/sdk/kotlin/runtime/http/AwsUserAgentMetadata.kt @@ -15,6 +15,7 @@ import kotlin.jvm.JvmInline internal const val AWS_EXECUTION_ENV = "AWS_EXECUTION_ENV" public const val AWS_APP_ID_ENV: String = "AWS_SDK_UA_APP_ID" private const val USER_AGENT_SPEC_VERSION = "2.1" +public const val BUSINESS_METRICS_MAX_SIZE: Int = 1024 // non-standard environment variables/properties public const val AWS_APP_ID_PROP: String = "aws.userAgentAppId" diff --git a/aws-runtime/aws-http/common/src/aws/sdk/kotlin/runtime/http/interceptors/BusinessMetricsInterceptor.kt b/aws-runtime/aws-http/common/src/aws/sdk/kotlin/runtime/http/interceptors/BusinessMetricsInterceptor.kt index 070e4c992a4..fe2c33b6661 100644 --- a/aws-runtime/aws-http/common/src/aws/sdk/kotlin/runtime/http/interceptors/BusinessMetricsInterceptor.kt +++ b/aws-runtime/aws-http/common/src/aws/sdk/kotlin/runtime/http/interceptors/BusinessMetricsInterceptor.kt @@ -4,8 +4,11 @@ */ package aws.sdk.kotlin.runtime.http.interceptors +import aws.sdk.kotlin.runtime.http.BUSINESS_METRICS_MAX_SIZE import aws.sdk.kotlin.runtime.http.middleware.USER_AGENT -import aws.smithy.kotlin.runtime.businessmetrics.businessMetrics +import aws.smithy.kotlin.runtime.InternalApi +import aws.smithy.kotlin.runtime.businessmetrics.BusinessMetric +import aws.smithy.kotlin.runtime.businessmetrics.BusinessMetrics import aws.smithy.kotlin.runtime.client.ProtocolRequestInterceptorContext import aws.smithy.kotlin.runtime.http.interceptors.HttpInterceptor import aws.smithy.kotlin.runtime.http.request.HttpRequest @@ -16,7 +19,7 @@ import aws.smithy.kotlin.runtime.http.request.toBuilder */ public class BusinessMetricsInterceptor : HttpInterceptor { override suspend fun modifyBeforeTransmit(context: ProtocolRequestInterceptorContext): HttpRequest { - context.executionContext.getOrNull(businessMetrics)?.let { metrics -> + context.executionContext.getOrNull(BusinessMetrics)?.let { metrics -> val metricsString = formatMetrics(metrics) val currentUserAgentHeader = context.protocolRequest.headers[USER_AGENT] val modifiedRequest = context.protocolRequest.toBuilder() @@ -36,18 +39,13 @@ private fun formatMetrics(metrics: MutableSet): String { if (metrics.isEmpty()) return "" val metricsString = metrics.joinToString(",", "m/") val metricsByteArray = metricsString.encodeToByteArray() - val maxSize = 1024 - if (metricsByteArray.size <= maxSize) return metricsString + if (metricsByteArray.size <= BUSINESS_METRICS_MAX_SIZE) return metricsString - val commaByte = ','.code.toByte() - var lastCommaIndex: Int? = null - - for (i in 0..1023) { - if (metricsByteArray[i] == commaByte) { - lastCommaIndex = i - } - } + val lastCommaIndex = metricsByteArray + .sliceArray(0 until 1024) + .indexOfLast { it == ','.code.toByte() } + .takeIf { it != -1 } lastCommaIndex?.let { return metricsByteArray.decodeToString( @@ -59,3 +57,11 @@ private fun formatMetrics(metrics: MutableSet): String { throw IllegalStateException("Business metrics are incorrectly formatted: $metricsString") } + +/** + * SDK specific business metrics + */ +@InternalApi +public enum class SdkBusinessMetric(public override val identifier: String) : BusinessMetric { + S3_EXPRESS_BUCKET("J"), +} diff --git a/aws-runtime/aws-http/common/test/aws/sdk/kotlin/runtime/http/interceptors/BusinessMetricsInterceptorTest.kt b/aws-runtime/aws-http/common/test/aws/sdk/kotlin/runtime/http/interceptors/BusinessMetricsInterceptorTest.kt index 7e77374c4cc..e37d64e8d7b 100644 --- a/aws-runtime/aws-http/common/test/aws/sdk/kotlin/runtime/http/interceptors/BusinessMetricsInterceptorTest.kt +++ b/aws-runtime/aws-http/common/test/aws/sdk/kotlin/runtime/http/interceptors/BusinessMetricsInterceptorTest.kt @@ -4,8 +4,10 @@ */ package aws.sdk.kotlin.runtime.http.interceptors +import aws.sdk.kotlin.runtime.http.BUSINESS_METRICS_MAX_SIZE import aws.sdk.kotlin.runtime.http.middleware.USER_AGENT -import aws.smithy.kotlin.runtime.businessmetrics.BusinessMetrics +import aws.smithy.kotlin.runtime.businessmetrics.SdkBusinessMetric +import aws.smithy.kotlin.runtime.businessmetrics.SmithyBusinessMetric import aws.smithy.kotlin.runtime.businessmetrics.emitBusinessMetric import aws.smithy.kotlin.runtime.client.ProtocolRequestInterceptorContext import aws.smithy.kotlin.runtime.collections.get @@ -15,6 +17,7 @@ import aws.smithy.kotlin.runtime.net.url.Url import aws.smithy.kotlin.runtime.operation.ExecutionContext import kotlinx.coroutines.test.runTest import kotlin.test.Test +import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertTrue @@ -32,7 +35,7 @@ class BusinessMetricsInterceptorTest { @Test fun businessMetrics() = runTest { val executionContext = ExecutionContext() - executionContext.emitBusinessMetric(BusinessMetrics.S3_EXPRESS_BUCKET) + executionContext.emitBusinessMetric(SdkBusinessMetric.S3_EXPRESS_BUCKET) val interceptor = BusinessMetricsInterceptor() val request = interceptor.modifyBeforeTransmit(interceptorContext(executionContext)) @@ -40,7 +43,7 @@ class BusinessMetricsInterceptorTest { assertTrue( userAgentHeader.endsWith( - "m/${BusinessMetrics.S3_EXPRESS_BUCKET.identifier}", + "m/${SdkBusinessMetric.S3_EXPRESS_BUCKET.identifier}", ), ) } @@ -48,8 +51,8 @@ class BusinessMetricsInterceptorTest { @Test fun multipleBusinessMetrics() = runTest { val executionContext = ExecutionContext() - executionContext.emitBusinessMetric(BusinessMetrics.S3_EXPRESS_BUCKET) - executionContext.emitBusinessMetric(BusinessMetrics.GZIP_REQUEST_COMPRESSION) + executionContext.emitBusinessMetric(SdkBusinessMetric.S3_EXPRESS_BUCKET) + executionContext.emitBusinessMetric(SmithyBusinessMetric.GZIP_REQUEST_COMPRESSION) val interceptor = BusinessMetricsInterceptor() val request = interceptor.modifyBeforeTransmit(interceptorContext(executionContext)) @@ -57,7 +60,7 @@ class BusinessMetricsInterceptorTest { assertTrue( userAgentHeader.endsWith( - "m/${BusinessMetrics.S3_EXPRESS_BUCKET.identifier},${BusinessMetrics.GZIP_REQUEST_COMPRESSION.identifier}", + "m/${SdkBusinessMetric.S3_EXPRESS_BUCKET.identifier},${SmithyBusinessMetric.GZIP_REQUEST_COMPRESSION.identifier}", ), ) } @@ -65,27 +68,41 @@ class BusinessMetricsInterceptorTest { @Test fun truncateBusinessMetrics() = runTest { val executionContext = ExecutionContext() - executionContext.attributes[aws.smithy.kotlin.runtime.businessmetrics.businessMetrics] = mutableSetOf() + executionContext.attributes[aws.smithy.kotlin.runtime.businessmetrics.BusinessMetrics] = mutableSetOf() for (i in 0..1024) { - executionContext.attributes[aws.smithy.kotlin.runtime.businessmetrics.businessMetrics].add(i.toString()) + executionContext.attributes[aws.smithy.kotlin.runtime.businessmetrics.BusinessMetrics].add(i.toString()) } - val rawMetrics = executionContext[aws.smithy.kotlin.runtime.businessmetrics.businessMetrics] + val rawMetrics = executionContext[aws.smithy.kotlin.runtime.businessmetrics.BusinessMetrics] val rawMetricsString = rawMetrics.joinToString(",", "m/") val rawMetricsByteArray = rawMetricsString.toByteArray() - val maxSize = 1024 - assertTrue(rawMetricsByteArray.size >= maxSize) + assertTrue(rawMetricsByteArray.size >= BUSINESS_METRICS_MAX_SIZE) val interceptor = BusinessMetricsInterceptor() val request = interceptor.modifyBeforeTransmit(interceptorContext(executionContext)) val userAgentHeader = request.headers[USER_AGENT]!! val truncatedMetrics = "m/" + userAgentHeader.substringAfter("m/") - assertTrue(truncatedMetrics.toByteArray().size <= maxSize) + assertTrue(truncatedMetrics.toByteArray().size <= BUSINESS_METRICS_MAX_SIZE) assertFalse(truncatedMetrics.endsWith(",")) } + + @Test + fun malformedBusinessMetrics() = runTest { + val executionContext = ExecutionContext() + + executionContext.attributes[aws.smithy.kotlin.runtime.businessmetrics.BusinessMetrics] = mutableSetOf( + "A".repeat(BUSINESS_METRICS_MAX_SIZE), + ) + + val interceptor = BusinessMetricsInterceptor() + + assertFailsWith("Business metrics are incorrectly formatted:") { + interceptor.modifyBeforeTransmit(interceptorContext(executionContext)) + } + } } private fun interceptorContext(executionContext: ExecutionContext): ProtocolRequestInterceptorContext = diff --git a/codegen/aws-sdk-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/AwsRuntimeTypes.kt b/codegen/aws-sdk-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/AwsRuntimeTypes.kt index 1b3195589cb..9261838407b 100644 --- a/codegen/aws-sdk-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/AwsRuntimeTypes.kt +++ b/codegen/aws-sdk-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/AwsRuntimeTypes.kt @@ -61,6 +61,7 @@ object AwsRuntimeTypes { val AddUserAgentMetadataInterceptor = symbol("AddUserAgentMetadataInterceptor") val UnsupportedSigningAlgorithmInterceptor = symbol("UnsupportedSigningAlgorithmInterceptor") val BusinessMetricsInterceptor = symbol("BusinessMetricsInterceptor") + val SdkBusinessMetric = symbol("SdkBusinessMetric") } object Retries { diff --git a/codegen/aws-sdk-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/BusinessMetricsIntegration.kt b/codegen/aws-sdk-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/BusinessMetricsIntegration.kt index b832577192d..7468ac52db0 100644 --- a/codegen/aws-sdk-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/BusinessMetricsIntegration.kt +++ b/codegen/aws-sdk-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/BusinessMetricsIntegration.kt @@ -6,6 +6,7 @@ package aws.sdk.kotlin.codegen import software.amazon.smithy.kotlin.codegen.core.KotlinWriter import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes +import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.Auth.Signing.AwsSigningCommon.AwsSigningAttributes import software.amazon.smithy.kotlin.codegen.integration.KotlinIntegration import software.amazon.smithy.kotlin.codegen.integration.SectionWriter import software.amazon.smithy.kotlin.codegen.integration.SectionWriterBinding @@ -24,19 +25,27 @@ class BusinessMetricsIntegration : KotlinIntegration { ) private val endpointBusinessMetricsSectionWriter = SectionWriter { writer, _ -> + writer.write("") writer.write( "if (endpoint.attributes.contains(#T)) request.context.#T(#T.SERVICE_ENDPOINT_OVERRIDE)", - RuntimeTypes.Core.BusinessMetrics.serviceEndpointOverride, + RuntimeTypes.Core.BusinessMetrics.ServiceEndpointOverride, RuntimeTypes.Core.BusinessMetrics.emitBusinessMetrics, - RuntimeTypes.Core.BusinessMetrics.BusinessMetrics, + RuntimeTypes.Core.BusinessMetrics.SmithyBusinessMetric, ) - writer.write( "if (endpoint.attributes.contains(#T)) request.context.#T(#T.ACCOUNT_ID_BASED_ENDPOINT)", - RuntimeTypes.Core.BusinessMetrics.accountIdBasedEndPointAccountId, + RuntimeTypes.Core.BusinessMetrics.AccountIdBasedEndpointAccountId, + RuntimeTypes.Core.BusinessMetrics.emitBusinessMetrics, + RuntimeTypes.Core.BusinessMetrics.SmithyBusinessMetric, + ) + writer.write( + "if (endpoint.attributes.contains(#T.SigningService) && endpoint.attributes[#T.SigningService] == \"s3express\") request.context.#T(#T.S3_EXPRESS_BUCKET)", + AwsSigningAttributes, + AwsSigningAttributes, RuntimeTypes.Core.BusinessMetrics.emitBusinessMetrics, - RuntimeTypes.Core.BusinessMetrics.BusinessMetrics, + AwsRuntimeTypes.Http.Interceptors.SdkBusinessMetric, ) + writer.write("") } override fun customizeMiddleware(