You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "JiriOndrusek (via GitHub)" <gi...@apache.org> on 2023/06/01 13:45:19 UTC
[GitHub] [camel-quarkus] JiriOndrusek opened a new pull request, #4951: Expand micrometer test coverage #4582
JiriOndrusek opened a new pull request, #4951:
URL: https://github.com/apache/camel-quarkus/pull/4951
fixes https://github.com/apache/camel-quarkus/issues/4582
1. I added a test for jmx access to metrics (JVM only). For that porpose I had to add a dependency `io.micrometer:micrometer-registry-jmx` into CQ BOM.
2. Prometheus metrics ignores negative values for counter. I put that information into `limitations.adoc`, because there is Camel header `decrement` which is meant to be used with counter metrics. Possibly better place for such information is Camel itself.
<!-- Uncomment and fill this section if your PR is not trivial
[ ] An issue should be filed for the change unless this is a trivial change (fixing a typo or similar). One issue should ideally be fixed by not more than one commit and the other way round, each commit should fix just one issue, without pulling in other changes.
[ ] Each commit in the pull request should have a meaningful and properly spelled subject line and body. Copying the title of the associated issue is typically enough. Please include the issue number in the commit message prefixed by #.
[ ] The pull request description should explain what the pull request does, how, and why. If the info is available in the associated issue or some other external document, a link is enough.
[ ] Phrases like Fix #<issueNumber> or Fixes #<issueNumber> will auto-close the named issue upon merging the pull request. Using them is typically a good idea.
[ ] Please run mvn process-resources -Pformat (and amend the changes if necessary) before sending the pull request.
[ ] Contributor guide is your good friend: https://camel.apache.org/camel-quarkus/latest/contributor-guide.html
-->
--
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 #4951: Expand micrometer test coverage #4582
Posted by "jamesnetherton (via GitHub)" <gi...@apache.org>.
jamesnetherton commented on code in PR #4951:
URL: https://github.com/apache/camel-quarkus/pull/4951#discussion_r1221853558
##########
extensions/micrometer/runtime/src/main/doc/limitations.adoc:
##########
@@ -0,0 +1,8 @@
+=== Exposing Micrometer statisctis in Jmx
+
+The `Exposing Micrometer statisctis in Jmx` action is not available in native mode as JMX is not supported on GraalVM.
Review Comment:
```suggestion
The `Exposing Micrometer statistics in JMX` action is not available in native mode as JMX is not supported on GraalVM.
```
##########
extensions/micrometer/runtime/src/main/doc/limitations.adoc:
##########
@@ -0,0 +1,8 @@
+=== Exposing Micrometer statisctis in Jmx
Review Comment:
```suggestion
=== Exposing Micrometer statistics in JMX
```
--
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] JiriOndrusek commented on pull request #4951: Expand micrometer test coverage #4582
Posted by "JiriOndrusek (via GitHub)" <gi...@apache.org>.
JiriOndrusek commented on PR #4951:
URL: https://github.com/apache/camel-quarkus/pull/4951#issuecomment-1580970664
> I am not sure if we want to be supporting `micrometer-registry-jmx` directly in this project. Otherwise, we have to maintain the dependencies, tests and docs explaining how to configure it.
>
> Support for the different Micrometer registry implementations lives here:
>
> https://github.com/quarkiverse/quarkus-micrometer-registry/
>
> So maybe a better way would be to add `io.quarkiverse.micrometer.registry:quarkus-micrometer-registry-jmx` only to the test BOM. And just replace the table in our [usage docs](https://camel.apache.org/camel-quarkus/next/reference/extensions/micrometer.html#extensions-micrometer-usage) with a link to [here](https://quarkiverse.github.io/quarkiverse-docs/quarkus-micrometer-registry/dev/index.html) .
>
> It's a bit of an open question whether we even need to test the JMX bits at all, given it's already tested elsewhere.
>
> WDYT @ppalaga?
The dependency is changed and moved.
Table in the doc is replaced but I had to keep onel dependency for prometheus which is not part of the quarkiverse doc.
--
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] JiriOndrusek commented on a diff in pull request #4951: Expand micrometer test coverage #4582
Posted by "JiriOndrusek (via GitHub)" <gi...@apache.org>.
JiriOndrusek commented on code in PR #4951:
URL: https://github.com/apache/camel-quarkus/pull/4951#discussion_r1222566881
##########
extensions/micrometer/runtime/src/main/doc/limitations.adoc:
##########
@@ -0,0 +1,8 @@
+=== Exposing Micrometer statisctis in Jmx
Review Comment:
fixed
##########
extensions/micrometer/runtime/src/main/doc/limitations.adoc:
##########
@@ -0,0 +1,8 @@
+=== Exposing Micrometer statisctis in Jmx
+
+The `Exposing Micrometer statisctis in Jmx` action is not available in native mode as JMX is not supported on GraalVM.
Review Comment:
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] jamesnetherton merged pull request #4951: Expand micrometer test coverage #4582
Posted by "jamesnetherton (via GitHub)" <gi...@apache.org>.
jamesnetherton merged PR #4951:
URL: https://github.com/apache/camel-quarkus/pull/4951
--
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 pull request #4951: Expand micrometer test coverage #4582
Posted by "jamesnetherton (via GitHub)" <gi...@apache.org>.
jamesnetherton commented on PR #4951:
URL: https://github.com/apache/camel-quarkus/pull/4951#issuecomment-1572698859
> I see it managed in quarkus-bom
That's just the plain `micrometer-registry-jmx` dependency. Not the Quarkus extension that lives here:
https://github.com/quarkiverse/quarkus-micrometer-registry/
--
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] ppalaga commented on pull request #4951: Expand micrometer test coverage #4582
Posted by "ppalaga (via GitHub)" <gi...@apache.org>.
ppalaga commented on PR #4951:
URL: https://github.com/apache/camel-quarkus/pull/4951#issuecomment-1573477625
> > I see it managed in quarkus-bom
>
> That's just the plain `micrometer-registry-jmx` dependency. Not the Quarkus extension that lives here:
>
> https://github.com/quarkiverse/quarkus-micrometer-registry/
I am blind, sorry!
--
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] JiriOndrusek commented on pull request #4951: Expand micrometer test coverage #4582
Posted by "JiriOndrusek (via GitHub)" <gi...@apache.org>.
JiriOndrusek commented on PR #4951:
URL: https://github.com/apache/camel-quarkus/pull/4951#issuecomment-1580977571
I had to rebase because of the conflict.
--
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] ppalaga commented on pull request #4951: Expand micrometer test coverage #4582
Posted by "ppalaga (via GitHub)" <gi...@apache.org>.
ppalaga commented on PR #4951:
URL: https://github.com/apache/camel-quarkus/pull/4951#issuecomment-1572306947
> So maybe a better way would be to add `io.quarkiverse.micrometer.registry:quarkus-micrometer-registry-jmx` only to the test BOM.
I see it managed in quarkus-bom: https://repo1.maven.org/maven2/io/quarkus/quarkus-bom/3.1.0.Final/quarkus-bom-3.1.0.Final.pom
> It's a bit of an open question whether we even need to test the JMX bits at all, given it's already tested elsewhere.
JMX support is underway for "GraalVM for JDK 17 / JDK 20 (June 13, 2023)", so these tests may come in handy for native testing, no?
--
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] ppalaga commented on pull request #4951: Expand micrometer test coverage #4582
Posted by "ppalaga (via GitHub)" <gi...@apache.org>.
ppalaga commented on PR #4951:
URL: https://github.com/apache/camel-quarkus/pull/4951#issuecomment-1581112153
Thanks @JiriOndrusek!
--
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 pull request #4951: Expand micrometer test coverage #4582
Posted by "jamesnetherton (via GitHub)" <gi...@apache.org>.
jamesnetherton commented on PR #4951:
URL: https://github.com/apache/camel-quarkus/pull/4951#issuecomment-1572153297
I am not sure if we want to be supporting `micrometer-registry-jmx` directly in this project. Otherwise, we have to maintain the dependencies, tests and docs explaining how to configure it.
Support for the different Micrometer registry implementations lives here:
https://github.com/quarkiverse/quarkus-micrometer-registry/
So maybe a better way would be to add `io.quarkiverse.micrometer.registry:quarkus-micrometer-registry-jmx` only to the test BOM. And just replace the table in our [usage docs](https://camel.apache.org/camel-quarkus/next/reference/extensions/micrometer.html#extensions-micrometer-usage) with a link to [here](https://quarkiverse.github.io/quarkiverse-docs/quarkus-micrometer-registry/dev/index.html) .
It's a bit of an open question whether we even need to test the JMX bits at all, given it's already tested elsewhere.
WDYT @ppalaga?
--
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] JiriOndrusek commented on pull request #4951: Expand micrometer test coverage #4582
Posted by "JiriOndrusek (via GitHub)" <gi...@apache.org>.
JiriOndrusek commented on PR #4951:
URL: https://github.com/apache/camel-quarkus/pull/4951#issuecomment-1572167714
I also think that supporting `micrometer-registry-jmx` is not necessary and we might want to avoid it. That is the reason of my comment in the summary (I should have make it more clear, apologies)
--
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