feat: client metrics

[monosoul]

Subsystem
Client

Motivation
Provide a plugin to instrument Ktor client similarly to how the server lib is instrumented with Micrometer.

Solution
This is basically just a PoC to discuss if the approach is okay or not. If it's okay, then I'll proceed with adding tests etc. If not, then we can discuss it here.

The idea here is to provide a client instrumentation similar to how it's done in OkHttpMetricsEventListener - introduce a timer for client requests.

The implementation is heavily inspired by the Logging plugin.

Since using an expanded URI as the value for the URI tag can lead to cardinality blow up, I had to introduce support for URI templates. This is done in 0e542bd. While those methods could potentially be added as extension functions in the plugin module, it feels like they belong to the core lib more.
( Probably a better option there would be to have a set of explicit builder functions like:

client.get(uriTemplate: String, vararg pathParameters: Pair<String, Any>, block: HttpRequestBuilder.() -> Unit = {})

Then it wouldn't be necessary to rebuild the URL. This is the path Spring followed with their client for example: https://github.com/spring-projects/spring-framework/blob/main/spring-web/src/main/java/org/springframework/web/client/RestTemplate.java#L400 )

The URI template is stored as a call attribute with the first call to HttpRequestBuilder.pathParameters and is later used in MicrometerMetrics plugin.

Here's how using the new methods would look like:

client.get("https://blog.jetbrains.com/kotlin/{year}/{month}/{name}/") {
    pathParameters(
         "year" to 2023,
         "month" to "11",
         "name" to "kotlin-1-9-20-released",
    )
}.bodyAsText()

The plugin is implemented in 2e9cab1 and provides configuration to make it flexible enough to cover most of the use cases.

The timer will be started when request has been sent and will be stopped either when an exception has occurred, or when the response has been received.

[e5l]

Hey @monosoul, thanks for the PR!

@marychatte, could you please check?

[marychatte]

Thanks a lot for the PR! And sorry for the big delay in the review
Good work, I like the idea of plugin. And I added some comments

[marychatte]

Can we remove url.build() here and make it call only once? Maybe we can use HttpRequestBuilder.attributes to save parameters and build later

[monosoul]

I guess one option here would be to just update URLBuilder to support path parameters in addition to query parameters

[marychatte]

Why does it need to be saved as a String? Later, it's used to parse, but maybe we can use functions URL.encodedQuery or URL.encodedPath

[monosoul]

Since we use that value as a metric tag, we don't really want to have an encoded url there.

[marychatte]

What about pathSegment = /{year}_{month}/ for example?

[monosoul]

Should we care about cases like that? Could be managed like that:

client.get("https://blog.jetbrains.com/kotlin/{year_month}/{name}/") {
    pathParameter("year_month", "2023_11")
    pathParameter("name", "kotlin-1-9-20-released")
}.bodyAsText()

[marychatte]

We also replace query parameters here, so the names pathParameters(...) and pathParameter(...) can be confusing. And it's also better to add in example query parameter values for clarity

[monosoul]

The thing is, nothing prevents you from specifying the URL for the client like that: https://www.google.com/search?client=firefox-b-d&q={query}, in which case to me it feels like it makes sense to substitute query parameters while doing so for the path parameters.

[marychatte]

We need to copy fragment and trailingQuery too

[monosoul]

b5d7684

[marychatte]

Can you please add in Kdoc example of plugin usage?

[marychatte]

Can we extract it so it does not have copypaste?

[monosoul]

Something like this: f802824 ?

[marychatte]

Can we add FailureHook so there is no need to write try-catch?

[monosoul]

This part was inspired by the logging plugin. From what I see, ResponseHook attaches to HttpReceivePipeline, SendHook attaches to HttpSendPipeline and ReceiveHook attaches to HttpResponsePipeline. If I get that right, I'd have to introduce 3 different failure hooks then to handle this, which doesn't seem to be very different from just handling the exception 3 times 😅

[marychatte]

Can we extract regex here so as not to compile it every time?

[monosoul]

Unless you know a way to parameterize the regex, no, we can't

[marychatte]

Can we, instead of creating new Hooks, use functions onRequest {... }, onResponse {... } etc?

[monosoul]

Only if we wouldn't need to explicitly call proceed() to handle exceptions

[PierrickPuimeanChieze]

Quick question here : why use directly MeterRegistry, instead of using ObservationRegistry ?
It is really a naive question, as I am starting working with Micrometer, but from my understanding, an Observation is a generic concept on which you can graft Timing, Tracing and Logging. But maybe there is some caveats to using it that I am not aware of...