Ensure that the REST Client registers Vert.x HTTP Client metrics

[geoand]

• Closes: https://github.com/quarkusio/quarkus/issues/50048

[quarkus-bot[bot]]

---

:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.

---

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit c2c33dc48fa2d576c5b0557b7cb2a7036aab2d94.

:white_check_mark: The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

[!WARNING] There are other workflow runs running, you probably need to wait for their status before merging.

[github-actions[bot]]

🙈 The PR is closed and the preview is expired.

[geoand]

Lovely, this seems to break something else...

[geoand]

It seems that with this change, there are now two metrics being created, so I need to figure out what's going on

[quarkus-bot[bot]]

---

:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.

---

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit c2c33dc48fa2d576c5b0557b7cb2a7036aab2d94.

Failing Jobs

Status	Name	Step	Failures	Logs	Raw logs	Build scan
:x:	JVM Tests - JDK 17	Build	Failures	Logs	Raw logs	:mag:
:x:	JVM Tests - JDK 21	Build	Failures	Logs	Raw logs	:mag:
:x:	JVM Tests - JDK 25	Build	Failures	Logs	Raw logs	:mag:
:x:	JVM Tests - JDK 17 Windows	Build	Failures	Logs	Raw logs	:mag:
:hourglass:	JVM Integration Tests - JDK 17		:warning: Check →	Logs	Raw logs	:construction:
:heavy_check_mark:	JVM Integration Tests - JDK 17 Windows			Logs	Raw logs	:mag:
:heavy_check_mark:	JVM Integration Tests - JDK 21			Logs	Raw logs	:mag:
:heavy_check_mark:	JVM Integration Tests - JDK 25			Logs	Raw logs	:mag:

Full information is available in the Build summary check run. You can consult the Develocity build scans.

Failures

:gear: JVM Tests - JDK 17 #

- Failing: extensions/micrometer/deployment 
! Skipped: extensions/liquibase/liquibase-mongodb/deployment extensions/micrometer-opentelemetry/deployment extensions/micrometer-registry-prometheus/deployment and 7 more

:package: extensions/micrometer/deployment

:x: io.quarkus.micrometer.deployment.binder.RestClientUriParameterTest.testOverride line 61 - History - More details - Source on GitHub

org.opentest4j.AssertionFailedError: expected: </example/{id}> but was: </example/bar>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1145)
	at io.quarkus.micrometer.deployment.binder.RestClientUriParameterTest.testOverride(RestClientUriParameterTest.java:61)

---

:gear: JVM Tests - JDK 21 #

- Failing: extensions/micrometer/deployment 
! Skipped: extensions/liquibase/liquibase-mongodb/deployment extensions/micrometer-opentelemetry/deployment extensions/micrometer-registry-prometheus/deployment and 7 more

:package: extensions/micrometer/deployment

:x: io.quarkus.micrometer.deployment.binder.RestClientUriParameterTest.testOverride line 61 - History - More details - Source on GitHub

org.opentest4j.AssertionFailedError: expected: </example/{id}> but was: </example/bar>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1145)
	at io.quarkus.micrometer.deployment.binder.RestClientUriParameterTest.testOverride(RestClientUriParameterTest.java:61)

---

:gear: JVM Tests - JDK 25 #

- Failing: extensions/micrometer/deployment 
! Skipped: extensions/liquibase/liquibase-mongodb/deployment extensions/micrometer-opentelemetry/deployment extensions/micrometer-registry-prometheus/deployment and 7 more

:package: extensions/micrometer/deployment

:x: io.quarkus.micrometer.deployment.binder.RestClientUriParameterTest.testOverride line 61 - History - More details - Source on GitHub

org.opentest4j.AssertionFailedError: expected: </example/{id}> but was: </example/bar>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1145)
	at io.quarkus.micrometer.deployment.binder.RestClientUriParameterTest.testOverride(RestClientUriParameterTest.java:61)

---

:gear: JVM Tests - JDK 17 Windows #

- Failing: extensions/micrometer/deployment 
! Skipped: extensions/liquibase/liquibase-mongodb/deployment extensions/micrometer-opentelemetry/deployment extensions/micrometer-registry-prometheus/deployment and 7 more

:package: extensions/micrometer/deployment

:x: io.quarkus.micrometer.deployment.binder.RestClientUriParameterTest.testOverride line 61 - History - More details - Source on GitHub

org.opentest4j.AssertionFailedError: expected: </example/{id}> but was: </example/bar>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1145)
	at io.quarkus.micrometer.deployment.binder.RestClientUriParameterTest.testOverride(RestClientUriParameterTest.java:61)

[geoand]

Here is an outline of the problem:

Whenever a Vert.x HTTP Client is created in Quarkus, and if HttpClientOptions.setMetricsName is set, then Quarkus and Vert.x collaborate (via io.quarkus.micrometer.runtime.binder.vertx.VertxHttpClientMetrics) to generate various metrics. One of the metrics created is http.client.requests, but there are others as well that are very useful to determine the health of the system.

In the REST Client, we have a custom way to generate the same http.client.requests metric. The reason this one is needed is that at the REST Client level, we have information that is simply not available at the Vert.x HTTP Client level - such as the templated path.

The problem currently is that I have not found a way for either of these ways to influence the other (or outright turn it off). I want to avoid removing http.client.requests from the Vert.x HTTP Client metrics tracking, because that would mean that non-REST Client usages of the client would lose an important metric.

I am still looking for a workaround that would not require any changes to Vert.x

[geoand]

@cescoffier @vietj I believe that what I need to overcome this issue is to be able to set some information on HttpClientRequest, which is then propagated to io.vertx.core.spi.observability.HttpRequest.

Does this make sense?

[geoand]

@cescoffier @vietj I believe that what I need to overcome this issue is to be able to set some information on HttpClientRequest, which is then propagated to io.vertx.core.spi.observability.HttpRequest.

Does this make sense?

Without this, I don't see a way of being able to avoid having duplicate metrics for the REST client

[geoand]

@cescoffier @vietj I believe that what I need to overcome this issue is to be able to set some information on HttpClientRequest, which is then propagated to io.vertx.core.spi.observability.HttpRequest.

Does this make sense?

bump :)

[vietj]

@geoand observability.HttpRequest is not the same object than HttpClientRequest, you would like to pass some objects ? that is not clear. We are thinking of changing the metrics SPI to pass a Context like for tracing, would that be sufficient (e.g. use context local data).

[geoand]

We are thinking of changing the metrics SPI to pass a Context like for tracing, would that be sufficient (e.g. use context local data).

Anything that will allow me to set arbitrary data on HttpClientRequest and then obtain it when attempting to create the metrics. would do for me. It sounds like your proposal is exactly that 😉

[vietj]

that would ba a reasonnable breaking change in 5.1, in the meantime we could add an internal hook for you to work with if that's really needed that we would remove after

[geoand]

that would ba a reasonnable breaking change in 5.1

Cool!

in the meantime we could add an internal hook for you to work with if that's really needed that we would remove after

Much appreciated!

[vietj]

@geoand here is the implementation for Vert.x 5.1 with the Vert.x HTTP metrics SPI modifications, do you mind having a look to ensure we are on the expected track ? https://github.com/eclipse-vertx/vert.x/pull/5826/files

[geoand]

Thanks a lot @vietj!

I am probably missing something, so maybe you can help me understand how I would use that change in the following scenario:

Let's say I want to make five HTTP requests and for each one I want to have some kind of contextual information in the request, say the key is foo and the value is bar1, bar2 ... (where the number is different for each request). Now, then the metrics are created, I want to be able to query the context for foo and bar%d.

[vietj]

@geoand that will not work for multiple concurrent requests I think because they might share the same context unless you have a way to create a key from the request on one side and find it from the metrics SPI.

but let's go back to the initial use case I think, you mention "templated path" can you elaborate about how it is created or computed ?

[geoand]

but let's go back to the initial use case I think, you mention "templated path" can you elaborate about how it is created or computed ?

This is REST Client specific thing, there is no way the HTTP client can know about it. An example could be:

@Path("/extensions")
@RegisterRestClient(configKey = "extensions-api")
public interface ExtensionsService {
 
   @GET
   @Path("/id/{id}")
    Set<Extension> getById(String id);

}

In this example we have the /id/{id} template, which the REST Client makes sure is handled as is in the metrics. What I essentially want to do is able to make sure that the REST Client and the underlying HTTP client don't duplicate metrics, so I want to be able to set some information on the HTTP client request that I can then access during the metrics creation in order to decide whether I actually want to create the metrics or not.

[vietj]

I am thinking of two possible options:

1. we provide access to the underlying http client request object created by the client metrics (like HttpServerRequestInternal#metric())
2. or a method to send a message to the client metrics implementation

would one of that work ?

[geoand]

we provide access to the underlying http client request object created by the client metrics (like HttpServerRequestInternal#metric())

I don't think this would help in my case, because I would need to be able to set something on metrics object when I create the Http client request so I can later decide whether to actually record the metric when time comes

or a method to send a message to the client metrics implementation

What would this look like?

[vietj]

we provide access to the underlying http client request object created by the client metrics (like HttpServerRequestInternal#metric())

I don't think this would help in my case, because I would need to be able to set something on metrics object when I create the Http client request so I can later decide whether to actually record the metric when time comes

That's because the metrics object is created when it is sent, is that correct ?

or a method to send a message to the client metrics implementation

What would this look like?

I don't think that would actually help in this case, I was thinking of something like HttpServerRequest#routed method

[geoand]

we provide access to the underlying http client request object created by the client metrics (like HttpServerRequestInternal#metric())

I don't think this would help in my case, because I would need to be able to set something on metrics object when I create the Http client request so I can later decide whether to actually record the metric when time comes

That's because the metrics object is created when it is sent, is that correct ?

Exactly, it's created by ClientMetrics#requestBegin and therefore I have no opportunity to know which client request created it

[vietj]

@geoand how about we introduce a new method to create the request metric and make it available in the request before it is sent and provide access to this metric on the request:

so we would change:

  default M requestBegin(String uri, Req request) {
    return null;
  }

to

  default M requestInit() {
    return null;
  }

  default void requestBegin(M requestMetric, String uri, Req request) {
  }

[geoand]

If we do that, when would requestInit be called?

[vietj]

If we do that, when would requestInit be called?

that would be created when the HttpClientRequestImpl is instantiated (so almost immediately).

[geoand]

I see. I am still not exactly sure how I would use this, but if you put together a prototype, I can certainly try it out

[vietj]

@geoand that allow to retrieve the metrics object created by the metrics implementation and modify its state, e.g. we could imagine something like

ClientRequestMetric metric = (ClientRequestMetric)request.metric();
metric.label = "some-label";

that would then be used by the metrics implementation when doing actual reporting.

[geoand]

Ah okay, that should work!

[geoand]

@vietj I used https://github.com/eclipse-vertx/vert.x/pull/5835 to verify that this approach would work for Quarkus

[vietj]

@vietj I used eclipse-vertx/vert.x#5835 to verify that this approach would work for Quarkus

so that confirms it allows you to implement the use case and we could merge it in vertx ?

[geoand]

Exactly