see-quick commented on code in PR #20672:
URL: https://github.com/apache/kafka/pull/20672#discussion_r2477919141
##########
streams/integration-tests/src/test/java/org/apache/kafka/streams/integration/KafkaStreamsTelemetryBase.java:
##########
@@ -249,28 +277,68 @@ public void shouldPushMetricsToBroker(final String
recordingLevel, final String
return "org.apache.kafka." + group + "." + name;
}).filter(name ->
!name.equals("org.apache.kafka.stream.thread.state"))// telemetry reporter
filters out string metrics
.sorted().toList();
- final List<String> actualMetrics = new
ArrayList<>(TelemetryPlugin.SUBSCRIBED_METRICS.get(mainConsumerInstanceId));
+ final List<String> actualMetrics = new
ArrayList<>(getSubscribedMetricsMap().get(mainConsumerInstanceId));
assertEquals(expectedMetrics, actualMetrics);
TestUtils.waitForCondition(
- () ->
!TelemetryPlugin.SUBSCRIBED_METRICS.getOrDefault(adminInstanceId,
Collections.emptyList()).isEmpty(),
+ () -> !getSubscribedMetricsMap().getOrDefault(adminInstanceId,
Collections.emptyList()).isEmpty(),
30_000,
"Never received subscribed metrics"
);
- final List<String> actualInstanceMetrics =
TelemetryPlugin.SUBSCRIBED_METRICS.get(adminInstanceId);
+ final List<String> actualInstanceMetrics =
getSubscribedMetricsMap().get(adminInstanceId);
final List<String> expectedInstanceMetrics = Arrays.asList(
"org.apache.kafka.stream.alive.stream.threads",
"org.apache.kafka.stream.client.state",
"org.apache.kafka.stream.failed.stream.threads",
"org.apache.kafka.stream.recording.level");
-
+
assertEquals(expectedInstanceMetrics, actualInstanceMetrics);
- TestUtils.waitForCondition(() -> TelemetryPlugin.processId != null,
+ TestUtils.waitForCondition(() -> getProcessId() != null,
30_000,
"Never received the process id");
- assertEquals(expectedProcessId, TelemetryPlugin.processId);
+ assertEquals(expectedProcessId, getProcessId());
+ }
+ }
+
+ @Test
+ public void shouldReceivePushInterval() throws Exception {
Review Comment:
Hmm, I thought we already had multiple intervalMs checks in the unit tests,
so there’s no need to add more here. Then I realised it might be better to add
a behaviour check (i.e., verifying that metrics are emitted at the correct
interval) => which would also implicitly cover the push interval. So I don’t
think we need to check it explicitly in this test case, since the behaviour
check already does that. Maybe I am missing something (so sorry for that)...
--
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]
