diff --git a/proxy/src/main/java/google/registry/proxy/metric/BackendMetrics.java b/proxy/src/main/java/google/registry/proxy/metric/BackendMetrics.java index f14372c7f..446782275 100644 --- a/proxy/src/main/java/google/registry/proxy/metric/BackendMetrics.java +++ b/proxy/src/main/java/google/registry/proxy/metric/BackendMetrics.java @@ -101,7 +101,7 @@ public class BackendMetrics extends BaseMetrics { if (random.nextDouble() > backendMetricsRatio) { return; } - requestsCounter.increment(protocol, certHash); + requestsCounter.incrementBy(roundRatioReciprocal(), protocol, certHash); requestBytes.record(bytes, protocol, certHash); } @@ -114,6 +114,28 @@ public class BackendMetrics extends BaseMetrics { } latencyMs.record(latency.getMillis(), protocol, certHash); responseBytes.record(response.content().readableBytes(), protocol, certHash); - responsesCounter.increment(protocol, certHash, response.status().toString()); + responsesCounter.incrementBy( + roundRatioReciprocal(), protocol, certHash, response.status().toString()); + } + + /** + * Returns the reciprocal of the backend metrics ratio, stochastically rounded to the nearest int. + * + *

This is necessary because if we are only going to record a metric, say, 1/20th of the time, + * then each time we do record it, we should increment it by 20 so that, modulo some randomness, + * the total figures still add up to the same amount. + * + *

The stochastic rounding is necessary to prevent introducing errors stemming from rounding a + * non-integer reciprocal consistently to the floor or ceiling. As an example, if the ratio is + * .03, then the reciprocal would be 33.3..., so two-thirds of the time it should increment by 33 + * and one-third of the time it should increment by 34, calculated randomly, so that the overall + * total adds up correctly. + */ + private long roundRatioReciprocal() { + double reciprocal = 1 / backendMetricsRatio; + return (long) + ((random.nextDouble() < reciprocal - Math.floor(reciprocal)) + ? Math.ceil(reciprocal) + : Math.floor(reciprocal)); } } diff --git a/proxy/src/test/java/google/registry/proxy/metric/BackendMetricsTest.java b/proxy/src/test/java/google/registry/proxy/metric/BackendMetricsTest.java index 7e158fd71..47d199137 100644 --- a/proxy/src/test/java/google/registry/proxy/metric/BackendMetricsTest.java +++ b/proxy/src/test/java/google/registry/proxy/metric/BackendMetricsTest.java @@ -112,9 +112,18 @@ class BackendMetricsTest { @Test void testSuccess_multipleResponses() { Random mockRandom = mock(Random.class); - metrics = new BackendMetrics(0.2, mockRandom); - // The third response won't be logged. - when(mockRandom.nextDouble()).thenReturn(.1, .04, .5, .15); + metrics = new BackendMetrics(0.3, mockRandom); + // The reciprocal of this metrics ratio is 3.3..., so depending on stochastic rounding each + // response will be recorded as either 3 or 4 (which we hard-code in test by mocking the RNG). + when(mockRandom.nextDouble()) + .thenReturn( + /* record response1 */ .1, + /* ... as 3 */ .5, + /* record response2 */ .04, + /* ... as 4 */ .2, + /* don't record response3 (skips stochastic rounding) */ .5, + /* record response4 */ .15, + /* ... as 3 */ .5); String content1 = "some response"; String content2 = "other response"; String content3 = "a very bad response"; @@ -130,9 +139,9 @@ class BackendMetricsTest { assertThat(BackendMetrics.requestsCounter).hasNoOtherValues(); assertThat(BackendMetrics.requestBytes).hasNoOtherValues(); assertThat(BackendMetrics.responsesCounter) - .hasValueForLabels(2, protocol, certHash, "200 OK") + .hasValueForLabels(7, protocol, certHash, "200 OK") .and() - .hasValueForLabels(1, protocol, certHash, "400 Bad Request") + .hasValueForLabels(3, protocol, certHash, "400 Bad Request") .and() .hasNoOtherValues(); assertThat(BackendMetrics.responseBytes)