Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rest-client extension does not record any metrics about client calls #13847

Closed
oscarfh opened this issue Dec 11, 2020 · 11 comments · Fixed by #13884
Closed

Rest-client extension does not record any metrics about client calls #13847

oscarfh opened this issue Dec 11, 2020 · 11 comments · Fixed by #13884

Comments

@oscarfh
Copy link
Contributor

oscarfh commented Dec 11, 2020

I followed the official documentation to create an app that uses the rest-client extension to make http calls.
I also added the micrometer lib to the project, so I get metrics.
My expectation is that I can get out-of-the-box metrics about rate, latency (percentiles), errors, etc on the clients calls.

But when I execute my example (and call localhost:8080/country/name/{name}), I do not see any client related metrics, even though I can see the result of the client call on my browser.

Steps to reproduce:

  1. Clone https://github.com/oscarfh/quarkus-rest-metrics
  2. Run ./mvnw compile quarkus:dev
  3. go to http://localhost:8080/country/name/germany
  4. go to http://0.0.0.0:8080/metrics
  5. Notice that there is no metric about the rest client call performed when you executed item 3
@oscarfh oscarfh added the kind/bug Something isn't working label Dec 11, 2020
@ghost
Copy link

ghost commented Dec 11, 2020

/cc @jmartisk, @phillip-kruger

@jmartisk
Copy link
Contributor

jmartisk commented Dec 14, 2020

We don't currently support any metrics associated with rest clients.

There are metrics for the server side (statistics how often each REST method is called and how long it takes) if you use the SmallRye Metrics extension. It's enabled by setting quarkus.smallrye-metrics.jaxrs.enabled.

With the Micrometer extension, we support a lower-level approach of gathering metrics on the HTTP (Vert.x) layer, enabled by quarkus.micrometer.binder.vertx.enabled. But it's, again, for the server side.

@gsmet
Copy link
Member

gsmet commented Dec 14, 2020

/cc @ebullient

@oscarfh
Copy link
Contributor Author

oscarfh commented Dec 14, 2020

@jmartisk Do you think this should exist?
I was planning to create an interceptor (or something similar) that intercepts calls to any bean annotated with @RegisterRestClient and creates the metrics.
An Alternative would be to extend the rest-client extension to wrap the resteasy proxy with a custom object that would register the metrics. I analyzed the code and I don't know if that is feasible.

What do you think? Is any of these a viable solution?

(I am creating an interceptor. If you agree please let me know whether I should add to the rest-client or the micrometer extension)

@ebullient
Copy link
Member

You can certainly create a client interceptor to emit metrics as a near term (use the appropriate API for the metrics extension you're using).

We can also add support for http client metrics to the micrometer extension (which will capture outbound vert.x requests). I'll get started on that after publishing a few things today. If you'd like to help, that's cool. ;)

@oscarfh
Copy link
Contributor Author

oscarfh commented Dec 14, 2020

@ebullient I will get something started.

@ebullient
Copy link
Member

Ok. For the micrometer extension, I'll be emulating this for clients: https://github.com/quarkusio/quarkus/blob/master/extensions/micrometer/runtime/src/main/java/io/quarkus/micrometer/runtime/binder/vertx/VertxHttpServerMetrics.java (to emit metrics akin to what Spring actuator emits, as a reasonable baseline)

@oscarfh
Copy link
Contributor Author

oscarfh commented Dec 14, 2020

I am trying this right now, but I cannot get it called, because it seems that using the resteasy does not create an HttpClientImpl

@ebullient
Copy link
Member

Y. I double checked -- resteasy client goes straight out using apache http client, that's why I hadn't added the vert.x based instrumentation. So we'll want client request/response filters.. And I think we may be able to keep those in the rest-client extension in this case .. I'll probably crib from what you get working..

@oscarfh
Copy link
Contributor Author

oscarfh commented Dec 14, 2020

I just got the client filters to work, see oscarfh/quarkus-rest-metrics@4b500e5

I will start working on a PR to get the rest-client extension to create these filters.

@ebullient
Copy link
Member

ContainerRequestFilter / ContainerResponseFilter are server side?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants