Nagarjunanm commented on code in PR #20559:
URL: https://github.com/apache/druid/pull/20559#discussion_r4316314009
##########
extensions-contrib/kafka-emitter/src/test/java/org/apache/druid/emitter/kafka/KafkaEmitterTest.java:
##########
@@ -673,4 +696,462 @@ public String getFeed()
return "testFeed";
}
}
+
+ // ==================== Sampling Tests ====================
+
+ @Test
+ public void testShouldSampleOut_noSamplingWhenRateIs100()
+ {
+ final KafkaEmitter emitter = initKafkaEmitter(newSamplingConfig(100,
ImmutableSet.of("query/time"), null));
+ ServiceMetricEvent event =
ServiceMetricEvent.builder().setMetric("query/time", 100).build("svc", "host");
+ Assertions.assertFalse(emitter.shouldSampleOut(event));
+ }
+
+ @Test
+ public void testShouldSampleOut_noSamplingWhenSampledMetricsEmpty()
+ {
+ final KafkaEmitter emitter = initKafkaEmitter(newSamplingConfig(100, null,
null));
Review Comment:
Thanks for pointing it out. With rate 100 the test couldn't tell which
setting kept the event. Its name was also misleading: an empty `sampledMetrics`
actually means every metric is eligible for sampling, not none. I've removed
it. The two cases it was mixing are already covered on their own:
`testShouldSampleOut_rate100OverridesAllFilters` (rate 100 keeps everything)
and `testShouldSampleOut_metricOnlyFilterNonMatchingMetric` (rate 0, metric not
in the list, so it's kept).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
