Skip to content

Commit f8ee385

Browse files
0marperez0marperez
authored
misc: increase business metrics checks (#1474)
1 parent fb5a0f3 commit f8ee385

File tree

4 files changed

+74
-32
lines changed

4 files changed

+74
-32
lines changed

‎aws-runtime/aws-http/api/aws-http.api‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ public final class aws/sdk/kotlin/runtime/http/interceptors/AwsBusinessMetric :
145145
public static final field S3_EXPRESS_BUCKET Laws/sdk/kotlin/runtime/http/interceptors/AwsBusinessMetric;
146146
public static fun getEntries ()Lkotlin/enums/EnumEntries;
147147
public fun getIdentifier ()Ljava/lang/String;
148+
public fun toString ()Ljava/lang/String;
148149
public static fun valueOf (Ljava/lang/String;)Laws/sdk/kotlin/runtime/http/interceptors/AwsBusinessMetric;
149150
public static fun values ()[Laws/sdk/kotlin/runtime/http/interceptors/AwsBusinessMetric;
150151
}

‎aws-runtime/aws-http/common/src/aws/sdk/kotlin/runtime/http/interceptors/BusinessMetricsInterceptor.kt‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,19 @@ import aws.smithy.kotlin.runtime.client.ProtocolRequestInterceptorContext
1313
import aws.smithy.kotlin.runtime.http.interceptors.HttpInterceptor
1414
import aws.smithy.kotlin.runtime.http.request.HttpRequest
1515
import aws.smithy.kotlin.runtime.http.request.toBuilder
16+
import aws.smithy.kotlin.runtime.telemetry.logging.Logger
17+
import aws.smithy.kotlin.runtime.telemetry.logging.logger
18+
import kotlin.coroutines.coroutineContext
1619

1720
/**
1821
* Appends business metrics to the `User-Agent` header.
1922
*/
2023
public class BusinessMetricsInterceptor : HttpInterceptor {
2124
override suspend fun modifyBeforeTransmit(context: ProtocolRequestInterceptorContext<Any, HttpRequest>): HttpRequest {
25+
val logger = coroutineContext.logger<BusinessMetricsInterceptor>()
26+
2227
context.executionContext.getOrNull(BusinessMetrics)?.let { metrics ->
23-
val metricsString = formatMetrics(metrics)
28+
val metricsString = formatMetrics(metrics, logger)
2429
val currentUserAgentHeader = context.protocolRequest.headers[USER_AGENT]
2530
val modifiedRequest = context.protocolRequest.toBuilder()
2631

@@ -34,10 +39,22 @@ public class BusinessMetricsInterceptor : HttpInterceptor {
3439

3540
/**
3641
* Makes sure the metrics do not exceed the maximum size and truncates them if so.
42+
* Makes sure that metric identifiers are not > 2 chars in length. Skips them if so.
3743
*/
38-
private fun formatMetrics(metrics: MutableSet<BusinessMetric>): String {
39-
if (metrics.isEmpty()) return ""
40-
val metricsString = metrics.joinToString(",", "m/") { it.identifier }
44+
private fun formatMetrics(metrics: MutableSet<BusinessMetric>, logger: Logger): String {
45+
val allowedMetrics = metrics.filter {
46+
if (it.identifier.length > 2) {
47+
logger.warn {
48+
"Business metric '${it.identifier}' will be skipped due to length being > 2. " +
49+
"This is likely a bug. Please raise an issue at https://github.com/awslabs/aws-sdk-kotlin/issues/new/choose"
50+
}
51+
false
52+
} else {
53+
true
54+
}
55+
}
56+
if (allowedMetrics.isEmpty()) return ""
57+
val metricsString = allowedMetrics.joinToString(",", "m/") { it.identifier }
4158
val metricsByteArray = metricsString.encodeToByteArray()
4259

4360
if (metricsByteArray.size <= BUSINESS_METRICS_MAX_LENGTH) return metricsString
@@ -65,4 +82,7 @@ private fun formatMetrics(metrics: MutableSet<BusinessMetric>): String {
6582
public enum class AwsBusinessMetric(public override val identifier: String) : BusinessMetric {
6683
S3_EXPRESS_BUCKET("J"),
6784
DDB_MAPPER("d"),
85+
;
86+
87+
override fun toString(): String = identifier
6888
}

‎aws-runtime/aws-http/common/test/aws/sdk/kotlin/runtime/http/interceptors/BusinessMetricsInterceptorTest.kt‎

Lines changed: 47 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,12 @@ import aws.smithy.kotlin.runtime.businessmetrics.BusinessMetric
1010
import aws.smithy.kotlin.runtime.businessmetrics.SmithyBusinessMetric
1111
import aws.smithy.kotlin.runtime.businessmetrics.emitBusinessMetric
1212
import aws.smithy.kotlin.runtime.client.ProtocolRequestInterceptorContext
13-
import aws.smithy.kotlin.runtime.collections.get
1413
import aws.smithy.kotlin.runtime.http.*
1514
import aws.smithy.kotlin.runtime.http.request.HttpRequest
1615
import aws.smithy.kotlin.runtime.net.url.Url
1716
import aws.smithy.kotlin.runtime.operation.ExecutionContext
1817
import kotlinx.coroutines.test.runTest
19-
import kotlin.test.Test
20-
import kotlin.test.assertFailsWith
21-
import kotlin.test.assertFalse
22-
import kotlin.test.assertTrue
18+
import kotlin.test.*
2319

2420
class BusinessMetricsInterceptorTest {
2521
@Test
@@ -32,6 +28,23 @@ class BusinessMetricsInterceptorTest {
3228
assertFalse(userAgentHeader.endsWith("m/"))
3329
}
3430

31+
@Test
32+
fun noValidBusinessMetrics() = runTest {
33+
val executionContext = ExecutionContext()
34+
35+
val invalidBusinessMetric = object : BusinessMetric {
36+
override val identifier: String = "All work and no play makes Jack a dull boy".repeat(1000)
37+
}
38+
39+
executionContext.emitBusinessMetric(invalidBusinessMetric)
40+
41+
val interceptor = BusinessMetricsInterceptor()
42+
val request = interceptor.modifyBeforeTransmit(interceptorContext(executionContext))
43+
val userAgentHeader = request.headers[USER_AGENT]!!
44+
45+
assertFalse(userAgentHeader.endsWith("m/"))
46+
}
47+
3548
@Test
3649
fun businessMetrics() = runTest {
3750
val executionContext = ExecutionContext()
@@ -66,49 +79,57 @@ class BusinessMetricsInterceptorTest {
6679
}
6780

6881
@Test
69-
fun truncateBusinessMetrics() = runTest {
82+
fun businessMetricsMaxLength() = runTest {
7083
val executionContext = ExecutionContext()
7184
executionContext.attributes[aws.smithy.kotlin.runtime.businessmetrics.BusinessMetrics] = mutableSetOf()
7285

73-
for (i in 0..1024) {
86+
for (i in 0..BUSINESS_METRICS_MAX_LENGTH) {
7487
executionContext.emitBusinessMetric(
7588
object : BusinessMetric {
7689
override val identifier: String = i.toString()
7790
},
7891
)
7992
}
8093

81-
val rawMetrics = executionContext[aws.smithy.kotlin.runtime.businessmetrics.BusinessMetrics]
82-
val rawMetricsString = rawMetrics.joinToString(",", "m/")
83-
val rawMetricsByteArray = rawMetricsString.encodeToByteArray()
84-
85-
assertTrue(rawMetricsByteArray.size >= BUSINESS_METRICS_MAX_LENGTH)
86-
8794
val interceptor = BusinessMetricsInterceptor()
8895
val request = interceptor.modifyBeforeTransmit(interceptorContext(executionContext))
8996
val userAgentHeader = request.headers[USER_AGENT]!!
90-
val truncatedMetrics = "m/" + userAgentHeader.substringAfter("m/")
97+
val metrics = "m/" + userAgentHeader.substringAfter("m/")
9198

92-
assertTrue(truncatedMetrics.encodeToByteArray().size <= BUSINESS_METRICS_MAX_LENGTH)
93-
assertFalse(truncatedMetrics.endsWith(","))
99+
assertTrue(metrics.encodeToByteArray().size <= BUSINESS_METRICS_MAX_LENGTH)
100+
assertFalse(metrics.endsWith(","))
94101
}
95102

96103
@Test
97-
fun malformedBusinessMetrics() = runTest {
104+
fun invalidBusinessMetric() = runTest {
98105
val executionContext = ExecutionContext()
99-
val reallyLongMetric = "All work and no play makes Jack a dull boy".repeat(1000)
100106

101-
executionContext.attributes.emitBusinessMetric(
102-
object : BusinessMetric {
103-
override val identifier: String = reallyLongMetric
104-
},
105-
)
107+
val validMetric = AwsBusinessMetric.S3_EXPRESS_BUCKET
108+
val invalidMetric = object : BusinessMetric {
109+
override val identifier: String = "All work and no play makes Jack a dull boy".repeat(1000)
110+
}
111+
112+
executionContext.attributes.emitBusinessMetric(validMetric)
113+
executionContext.attributes.emitBusinessMetric(invalidMetric)
106114

107115
val interceptor = BusinessMetricsInterceptor()
116+
val request = interceptor.modifyBeforeTransmit(interceptorContext(executionContext))
117+
val userAgentHeader = request.headers[USER_AGENT]!!
108118

109-
assertFailsWith<IllegalStateException>("Business metrics are incorrectly formatted:") {
110-
interceptor.modifyBeforeTransmit(interceptorContext(executionContext))
111-
}
119+
assertTrue(
120+
userAgentHeader.contains(validMetric.identifier),
121+
)
122+
assertFalse(
123+
userAgentHeader.contains(invalidMetric.identifier),
124+
)
125+
}
126+
127+
@Test
128+
fun businessMetricToString() {
129+
val businessMetricToString = AwsBusinessMetric.S3_EXPRESS_BUCKET.toString()
130+
val businessMetricIdentifier = AwsBusinessMetric.S3_EXPRESS_BUCKET.identifier
131+
132+
assertEquals(businessMetricIdentifier, businessMetricToString)
112133
}
113134
}
114135

‎gradle/libs.versions.toml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ coroutines-version = "1.9.0"
1111
atomicfu-version = "0.25.0"
1212

1313
# smithy-kotlin codegen and runtime are versioned separately
14-
smithy-kotlin-runtime-version = "1.3.26"
15-
smithy-kotlin-codegen-version = "0.33.26"
14+
smithy-kotlin-runtime-version = "1.3.27"
15+
smithy-kotlin-codegen-version = "0.33.27"
1616

1717
# codegen
1818
smithy-version = "1.51.0"

0 commit comments

Comments
 (0)