mirror of
https://github.com/google/nomulus
synced 2025-12-23 06:15:42 +00:00
Increment proxy metrics by reciprocal of proxy metrics ratio (#2780)
This is necessary so that the total number of requests/responses adds up correctly even though some fraction of them are only being recorded. It uses stochastic rounding so that the totals add up correctly even when the reciprocal of the ratio isn't an integer. This is a follow-up to PR #2772.
This commit is contained in:
@@ -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.
|
||||
*
|
||||
* <p>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.
|
||||
*
|
||||
* <p>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));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user