bbejeck commented on code in PR #17422:
URL: https://github.com/apache/kafka/pull/17422#discussion_r1802088202
##########
streams/src/test/java/org/apache/kafka/streams/StreamsConfigTest.java:
##########
@@ -1304,22 +1307,72 @@ public void
shouldDisableMetricCollectionForAllInternalClients() {
}
@Test
- public void shouldDisableMetricCollectionOnMainConsumerOnly() {
+ public void
shouldThrowConfigExceptionWhenMainConsumerMetricsDisabledStreamsMetricsPushEnabled()
{
+
props.put(StreamsConfig.mainConsumerPrefix(ConsumerConfig.ENABLE_METRICS_PUSH_CONFIG),
false);
+
+ final Exception exception = assertThrows(ConfigException.class, () ->
new StreamsConfig(props));
+
+ assertThat(
+ exception.getMessage(),
+ containsString("KafkaStreams has metrics push enabled" +
+ " but the main consumer metrics push is disabled.
Enable " +
+ " metrics push for the main consumer")
+ );
+ }
+
+ @Test
+ public void
shouldThrowConfigExceptionConsumerMetricsDisabledStreamsMetricsPushEnabled() {
+
props.put(StreamsConfig.consumerPrefix(ConsumerConfig.ENABLE_METRICS_PUSH_CONFIG),
false);
props.put(StreamsConfig.mainConsumerPrefix(ConsumerConfig.ENABLE_METRICS_PUSH_CONFIG),
false);
Review Comment:
~I thought we should have a test where `consumerPrefix` metrics is set to
`true`, but users get confused and turn off `mainConsumerPrefix` metrics~
NM I was referring to something else, this should be fixed
--
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]
