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

Add Micrometer integration to ktor client #36

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

spoonman01
Copy link
Contributor

@spoonman01 spoonman01 commented Feb 7, 2023

  • Services using kevin-jvm can pass their own Micrometer registry and the ktor client will also registry a Timer for any Platform API request
  • Move version to 0.2.9

NOTE:

I went back and forth trying to add micrometer support to platform request, by either changing the kevin-jvm library or by changing banking-service.
Here's what I've tried or considered to have metrics for a web clients for platform request:

  1. Wrapping library calls in banking-service with a timer -> many things to change, manually reporting metrics, but most importantly the actual metrics (in ms) were unreliable
  2. Using a web client configured in banking-service, instead of using the ktor client from kevin-jvm -> kevin-jvm does not expose publicly the endpoints or all the DTOs, meaning it would take some work to replicate objects, authenticatio, exception handling, and testing again every request. Also did not want to re-invent everything and spend a lot of time there if we are soon going to create a common library to import in projects for web-clients.
  3. (This PR) Change kevin-jvm to optionally receive a Micrometer registry, then adding it as a plugin to the ktor client -> have to add a dependency, ktor path parameter is bad and I had to add "attributes" to all requests

I do not like much any of this approaches, maybe the best option is to wait and use a common web-client in some time.
But by adding Micrometer to kevin-jvm, i just had to change 2 lines in banking-service and the metrics were correctly updated with http_client_requests and it worked pretty good. Down below the response to the metrics where you can see the http_client_requests values from ktor (@Mantas-Ragauskas also please see the hibernate metrics that I have added and tell me what you think)

So, my question is, does this PR make sense or should we wait to add Spring's webclient in banking-service (where adding metrics is easy) and replicate kevin-jvm requests there ???

* Services using kevin-jvm can pass their own Micrometer registry and the ktor client will also registry a Timer for any Platform API request
* Move version to 0.2.9
@spoonman01 spoonman01 self-assigned this Feb 7, 2023
@spoonman01 spoonman01 marked this pull request as draft April 18, 2023 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant