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.

Sign up for GitHub

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

Jump to bottom

Add client metrics to rest client extension #13884

Merged
merged 1 commit into from
Dec 17, 2020
Merged

Add client metrics to rest client extension #13884

merged 1 commit into from
Dec 17, 2020

Conversation

oscarfh
Copy link
Contributor

@oscarfh oscarfh commented Dec 14, 2020 •
edited by ebullient
Loading

This is my take (with a lot of support from @ebullient ) on how to add metrics to the rest-clients automatically (#13884 )

Please let me know of the improvements, or also feel free to make it yourselves.

Resolves #13847

@ghost ghost added the area/rest-client label Dec 14, 2020
@ebullient
Copy link
Member

I'd like to see if we can streamline things.. but this is a good start. Catch you on Zulip. ;)

@ebullient ebullient self-assigned this Dec 14, 2020
gsmet
gsmet previously requested changes Dec 15, 2020
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I will let @ebullient drive this, just a note that this needs to be squashed before merging. Please dismiss when done!

ebullient
ebullient requested changes Dec 15, 2020
View reviewed changes
Copy link
Member

@ebullient ebullient left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made a PR against the original branch w/ updates to move the http client metrics extension closer to http server metrics collection (as reactive rest is going to influence both paths across multiple extensions.. )

extensions/rest-client/runtime/pom.xml Outdated Show resolved Hide resolved
@ghost ghost added area/amazon-lambda area/codestarts area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/hibernate-orm Hibernate ORM area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/liquibase area/metrics area/narayana Transactions / Narayana area/persistence OBSOLETE, DO NOT USE area/qute The template engine area/resteasy-classic area/rest area/smallrye labels Dec 15, 2020
@ebullient ebullient removed area/amazon-lambda area/codestarts area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/hibernate-orm Hibernate ORM area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/liquibase area/narayana Transactions / Narayana labels Dec 15, 2020
kenfinnigan
kenfinnigan approved these changes Dec 15, 2020
View reviewed changes
Copy link
Member

@kenfinnigan kenfinnigan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of notes about imports that don't appear to be used, but otherwise looks good!

...st-client/deployment/src/main/java/io/quarkus/restclient/deployment/RestClientProcessor.java Outdated Show resolved Hide resolved
...ions/rest-client/runtime/src/main/java/io/quarkus/restclient/runtime/RestClientRecorder.java Outdated Show resolved Hide resolved
@ghost ghost added area/codestarts area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/platform Issues related to definition and interaction with Quarkus Platform area/vertx labels Dec 15, 2020
@ebullient ebullient removed area/codestarts area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/platform Issues related to definition and interaction with Quarkus Platform area/vertx labels Dec 15, 2020
@ebullient ebullient dismissed gsmet’s stale review December 17, 2020 18:11

Resolved over a few days

@ebullient ebullient merged commit 1561bde into quarkusio:master Dec 17, 2020
@ghost ghost added this to the 1.11 - master milestone Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebullient ebullient ebullient approved these changes

@kenfinnigan kenfinnigan kenfinnigan approved these changes

@gsmet gsmet gsmet left review comments

Assignees

@ebullient ebullient

Labels
area/metrics area/rest-client
Projects
None yet
Milestone
1.11.0.Beta2
Development

Successfully merging this pull request may close these issues.

Rest-client extension does not record any metrics about client calls
4 participants
@oscarfh @ebullient @kenfinnigan @gsmet