Test: Aerospike module storage #186
Conversation
There was a problem hiding this comment.
Unfortunately, it can't do a trick. We need to know exact numbers for metrics because they can still be zero or duplicated. Judging by looks of things, we should either extend the metrics endpoint with an admit /collected-metrics handler or add Graphite to our tests. We should discuss this in the future.
| data class MetricDetail( | ||
| val name: String, | ||
| val description: String? = null, | ||
| val baseUnit: String? = null, | ||
| val measurements: List<Measurement>, | ||
| val availableTags: List<AvailableTag> | ||
| ) | ||
|
|
||
| data class Measurement( | ||
| val statistic: String, | ||
| val value: Number | ||
| ) | ||
|
|
||
| data class AvailableTag( | ||
| val tag: String, | ||
| val values: List<String> | ||
| ) |
There was a problem hiding this comment.
I think it's part of the model.request package
| "storage.aerospike.${applicationName}.max_retry" to "3", | ||
| "storage.aerospike.${applicationName}.namespace" to aerospikeNamespace, | ||
| "storage.aerospike.${applicationName}.prevent-u-u-i-d-duplication" to preventUuidDuplication.toString(), | ||
| "storage.default-ttl-seconds" to 1000L.toString() |
There was a problem hiding this comment.
It's basically the base config + getAerospikePreventUuidDuplicationConfig, so why not use them?
There was a problem hiding this comment.
getAerospikePreventUuidDuplicationConfig and storage.aerospike.${applicationName}.prevent-u-u-i-d-duplication different property
src/test/kotlin/org/prebid/cache/functional/testcontainers/PrebidCacheContainerConfig.kt
Show resolved
Hide resolved
| lateinit var apiKey: String | ||
| lateinit var applicationName: String |
There was a problem hiding this comment.
Nitpick. We can make apiKey, applicationName and config as constant
| val metrics = cacheApi.getMetrics() | ||
| assertSoftly { | ||
| metrics["pbc.module_storage.read.request"] shouldBe 1 | ||
| metrics["pbc.module_storage.read.request.duration"] shouldBe 2 |
There was a problem hiding this comment.
Not sure that measuring request.duration is a correct idea, because it can change from time to time and end up with flakes. We should measure that it exists as a separate // and: block or highlight it in general metrics block
| val payloadTransfer = PayloadTransfer.getDefaultJsonPayloadTransfer().apply { | ||
| key = payloadKey | ||
| application = applicationName | ||
| ttlseconds = 300L |
| should("throw an exception when post request have null key name") { | ||
| //given: default text payload with empty payloadKey | ||
| val payloadTransfer = PayloadTransfer.getDefaultTextPayloadTransfer().apply { | ||
| key = null | ||
| application = applicationName | ||
| } | ||
|
|
||
| // when: POST module-storage endpoint is called | ||
| val exception = shouldThrowExactly<ApiException> { cacheApi.postStorageCache(payloadTransfer, apiKey) } | ||
|
|
||
| // then: Bad request exception is thrown | ||
| assertSoftly { | ||
| exception.statusCode shouldBe BAD_REQUEST.value() | ||
| exception.responseBody shouldContain "\"path\":\"/storage\"" | ||
| exception.responseBody shouldContain "key must not be empty" | ||
| } | ||
|
|
||
| // and: pbc should populate corresponding metrics | ||
| val metrics = cacheApi.getMetrics() | ||
| assertSoftly { | ||
| metrics["pbc.module_storage.write.err.badRequest"] shouldBe 2 | ||
| metrics["pbc.module_storage.write.request"] shouldBe 1 | ||
| metrics["pbc.module_storage.write.request.duration"] shouldBe 2 | ||
| } | ||
| } | ||
|
|
||
| should("throw an exception when post request have empty key name") { | ||
| //given: default text payload with empty payloadKey | ||
| val payloadTransfer = PayloadTransfer.getDefaultTextPayloadTransfer().apply { | ||
| key = "" | ||
| application = applicationName | ||
| } | ||
|
|
||
| // when: POST module-storage endpoint is called | ||
| val exception = shouldThrowExactly<ApiException> { cacheApi.postStorageCache(payloadTransfer, apiKey) } | ||
|
|
||
| // then: Bad request exception is thrown | ||
| assertSoftly { | ||
| exception.statusCode shouldBe BAD_REQUEST.value() | ||
| exception.responseBody shouldContain "\"path\":\"/storage\"" | ||
| exception.responseBody shouldContain "key must not be empty" | ||
| } | ||
|
|
||
| // and: pbc should populate corresponding metrics | ||
| val metrics = cacheApi.getMetrics() | ||
| assertSoftly { | ||
| metrics["pbc.module_storage.write.err.badRequest"] shouldBe 2 | ||
| metrics["pbc.module_storage.write.request"] shouldBe 1 | ||
| metrics["pbc.module_storage.write.request.duration"] shouldBe 2 | ||
| } | ||
| } |
There was a problem hiding this comment.
We should combine them as parameterized tests, same for the others
There was a problem hiding this comment.
Then we couldn't measure metrics
No description provided.