You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "jamesnetherton (via GitHub)" <gi...@apache.org> on 2023/08/22 09:03:11 UTC
[GitHub] [camel-quarkus] jamesnetherton opened a new pull request, #5210: Micrometer: Message History factory and JMX MicrometerMessageHistory is not covered
jamesnetherton opened a new pull request, #5210:
URL: https://github.com/apache/camel-quarkus/pull/5210
Clone of https://github.com/apache/camel-quarkus/pull/5051, rebased from latest `main` + some minor tweaks.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] jamesnetherton commented on a diff in pull request #5210: Micrometer: Message History factory and JMX MicrometerMessageHistory is not covered
Posted by "jamesnetherton (via GitHub)" <gi...@apache.org>.
jamesnetherton commented on code in PR #5210:
URL: https://github.com/apache/camel-quarkus/pull/5210#discussion_r1301516610
##########
integration-tests/micrometer/src/test/java/org/apache/camel/quarkus/component/micrometer/it/MicrometerTest.java:
##########
@@ -207,4 +207,46 @@ public void testJMXQuarkusDomain(String domain) throws Exception {
.getValue();
assertTrue(classes > 1);
}
+
+ @Test
+ @DisabledOnIntegrationTest("https://github.com/apache/camel-quarkus/issues/5209")
+ public void testJMXHistory() throws Exception {
+ //send a message to init history for the route with id `jmxHistory`
+ RestAssured.get("/micrometer/sendJmxHistory")
+ .then()
+ .statusCode(200);
+
+ String contextManagementName = RestAssured.get("/micrometer/getContextManagementName")
+ .then()
+ .statusCode(200)
+ .extract().body().asString();
+
+ // get the message history service using JMX
+ String name = String.format("org.apache.camel:context=%s,type=services,name=MicrometerMessageHistoryService",
+ contextManagementName);
+
+ MBeanServer mBeanServer = ManagementFactory.getPlatformMBeanServer();
+
+ ObjectName on = ObjectName.getInstance(name);
+
+ //return json result
+ String json = (String) mBeanServer.invoke(on, "dumpStatisticsAsJson", null, null);
+
+ assertTrue(json.contains("jmxHistory"));
+
+ }
+
Review Comment:
Test renamed as requested.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] jamesnetherton commented on a diff in pull request #5210: Micrometer: Message History factory and JMX MicrometerMessageHistory is not covered
Posted by "jamesnetherton (via GitHub)" <gi...@apache.org>.
jamesnetherton commented on code in PR #5210:
URL: https://github.com/apache/camel-quarkus/pull/5210#discussion_r1301391709
##########
integration-tests/micrometer/src/test/java/org/apache/camel/quarkus/component/micrometer/it/MicrometerTest.java:
##########
@@ -207,4 +207,46 @@ public void testJMXQuarkusDomain(String domain) throws Exception {
.getValue();
assertTrue(classes > 1);
}
+
+ @Test
+ @DisabledOnIntegrationTest("https://github.com/apache/camel-quarkus/issues/5209")
+ public void testJMXHistory() throws Exception {
+ //send a message to init history for the route with id `jmxHistory`
+ RestAssured.get("/micrometer/sendJmxHistory")
+ .then()
+ .statusCode(200);
+
+ String contextManagementName = RestAssured.get("/micrometer/getContextManagementName")
+ .then()
+ .statusCode(200)
+ .extract().body().asString();
+
+ // get the message history service using JMX
+ String name = String.format("org.apache.camel:context=%s,type=services,name=MicrometerMessageHistoryService",
+ contextManagementName);
+
+ MBeanServer mBeanServer = ManagementFactory.getPlatformMBeanServer();
+
+ ObjectName on = ObjectName.getInstance(name);
+
+ //return json result
+ String json = (String) mBeanServer.invoke(on, "dumpStatisticsAsJson", null, null);
+
+ assertTrue(json.contains("jmxHistory"));
+
+ }
+
Review Comment:
Yeah, if I understand the intent correctly, there's a check to ensure the relevant MBeans are published and metrics are being recorded. And another test to get it directly from the `CamelContext`. I can rename the test methods.
Maybe after #5209 these could be combined into a single test. Checking that the bits are available in JMX is probably enough for the native / non-native case.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] aldettinger merged pull request #5210: Micrometer: Message History factory and JMX MicrometerMessageHistory is not covered
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger merged PR #5210:
URL: https://github.com/apache/camel-quarkus/pull/5210
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] aldettinger commented on a diff in pull request #5210: Micrometer: Message History factory and JMX MicrometerMessageHistory is not covered
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on code in PR #5210:
URL: https://github.com/apache/camel-quarkus/pull/5210#discussion_r1301362792
##########
integration-tests/micrometer/src/test/java/org/apache/camel/quarkus/component/micrometer/it/MicrometerTest.java:
##########
@@ -207,4 +207,46 @@ public void testJMXQuarkusDomain(String domain) throws Exception {
.getValue();
assertTrue(classes > 1);
}
+
+ @Test
+ @DisabledOnIntegrationTest("https://github.com/apache/camel-quarkus/issues/5209")
+ public void testJMXHistory() throws Exception {
+ //send a message to init history for the route with id `jmxHistory`
+ RestAssured.get("/micrometer/sendJmxHistory")
+ .then()
+ .statusCode(200);
+
+ String contextManagementName = RestAssured.get("/micrometer/getContextManagementName")
+ .then()
+ .statusCode(200)
+ .extract().body().asString();
+
+ // get the message history service using JMX
+ String name = String.format("org.apache.camel:context=%s,type=services,name=MicrometerMessageHistoryService",
+ contextManagementName);
+
+ MBeanServer mBeanServer = ManagementFactory.getPlatformMBeanServer();
+
+ ObjectName on = ObjectName.getInstance(name);
+
+ //return json result
+ String json = (String) mBeanServer.invoke(on, "dumpStatisticsAsJson", null, null);
+
+ assertTrue(json.contains("jmxHistory"));
+
+ }
+
Review Comment:
Is it correct reading that `testJMXHistory` is checking that the service could be accessed through JMX ?
And that `testHistory` is ensuring that the service could be accessed through the camel context ?
If yes, what about renaming both tests ala:
+ micrometerHistoryShouldBeAvailableThroughJMX
+ micrometerHistoryShouldBeAvailableThroughCamelContext
If make sense.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org