Skip to content

Commit

Permalink
fix: count "yes" and "no" as part of getVariant (#53)
Browse files Browse the repository at this point in the history
## Description

This PR fixes a behavior issue related to counting metrics: when counting variant metrics, we should also count "yes" and "no" / enabled and disabled.

This PR changes one of the existing tests to align with this behavior and adds an extra test to specifically about this.

## Commits / changes

* fix: fix old test

* fix: add new test

* fix: count toggle enabled stats as part of `getVariant`
  • Loading branch information
thomasheartman authored Jun 16, 2023
1 parent 77b57f0 commit 381aa55
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 2 deletions.
7 changes: 6 additions & 1 deletion src/main/kotlin/io/getunleash/UnleashClient.kt
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,12 @@ class UnleashClient(
}

override fun getVariant(toggleName: String): Variant {
val variant = refreshPolicy.readToggleCache()[toggleName]?.variant ?: disabledVariant
val variant =
if (isEnabled(toggleName)) {
refreshPolicy.readToggleCache()[toggleName]?.variant ?: disabledVariant
} else {
disabledVariant
}
return metricsReporter.logVariant(toggleName, variant)
}

Expand Down
24 changes: 23 additions & 1 deletion src/test/kotlin/io/getunleash/metrics/MetricsTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,28 @@ class MetricsTest {
assertThat(toggles).containsEntry("unleash_android_sdk_demo", EvaluationCount(100, 0))
}


@Test
fun `getVariant calls also records "yes" and "no"`() {
val reporter = TestReporter()
val client = UnleashClient(config, context, metricsReporter = reporter)
repeat(100) {
// toggle doesn't exist
client.getVariant("some-non-existing-toggle")
// toggle with variants
client.getVariant("asdasd")
// toggle without variants
client.getVariant("cache.buster")
}
val toggles = reporter.getToggles()

assertThat(toggles).containsAllEntriesOf(mutableMapOf(
"some-non-existing-toggle" to EvaluationCount(0, 100, mutableMapOf("disabled" to 100)),
"asdasd" to EvaluationCount(100, 0, mutableMapOf("123" to 100)),
"cache.buster" to EvaluationCount(100, 0, mutableMapOf("disabled" to 100)),
))
}

@Test
fun `reporting resets period`() {
val reporter = TestReporter()
Expand Down Expand Up @@ -147,7 +169,7 @@ class MetricsTest {
"unleash-android-proxy-sdk" to EvaluationCount(0, 100),
"non-existing-toggle" to EvaluationCount(0, 100),
"Test_release" to EvaluationCount(100, 0),
"demoApp.step4" to EvaluationCount(0, 0, mutableMapOf("red" to 100))
"demoApp.step4" to EvaluationCount(100, 0, mutableMapOf("red" to 100))
))
server.enqueue(MockResponse())
// No activity since last report, bucket should be empty
Expand Down

0 comments on commit 381aa55

Please sign in to comment.