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

feat: client metrics #3988

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ktor-client/ktor-client-core/api/ktor-client-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -1118,6 +1118,8 @@ public final class io/ktor/client/request/HttpRequestBuilder : io/ktor/http/Http
public fun getHeaders ()Lio/ktor/http/HeadersBuilder;
public final fun getMethod ()Lio/ktor/http/HttpMethod;
public final fun getUrl ()Lio/ktor/http/URLBuilder;
public final fun pathParameter (Ljava/lang/String;Ljava/lang/Object;)V
public final fun pathParameters (Ljava/lang/Iterable;)V
public final fun setAttributes (Lkotlin/jvm/functions/Function1;)V
public final fun setBody (Ljava/lang/Object;)V
public final fun setBodyType (Lio/ktor/util/reflect/TypeInfo;)V
Expand Down Expand Up @@ -1150,6 +1152,7 @@ public final class io/ktor/client/request/HttpRequestJvmKt {

public final class io/ktor/client/request/HttpRequestKt {
public static final fun getResponseAdapterAttributeKey ()Lio/ktor/util/AttributeKey;
public static final fun getUriPatternAttributeKey ()Lio/ktor/util/AttributeKey;
public static final fun headers (Lio/ktor/http/HttpMessageBuilder;Lkotlin/jvm/functions/Function1;)Lio/ktor/http/HeadersBuilder;
public static final fun invoke (Lio/ktor/client/request/HttpRequestBuilder$Companion;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Lkotlin/jvm/functions/Function1;)Lio/ktor/client/request/HttpRequestBuilder;
public static final fun invoke (Lio/ktor/client/request/HttpRequestBuilder$Companion;Lkotlin/jvm/functions/Function1;)Lio/ktor/client/request/HttpRequestBuilder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,54 @@ public class HttpRequestBuilder : HttpMessageBuilder {
return attributes.getOrNull(ENGINE_CAPABILITIES_KEY)?.get(key) as T?
}

/**
* Substitute URI path and/or query parameters with provided values
*
* Example:
* ```
* 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()
* ```
*
* @param parameters pairs of parameter name and value to substitute
* @see pathParameter
*/
public fun pathParameters(parameters: Iterable<Pair<String, Any>>) {
val currentUrl = url.build()
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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

// save original URI pattern
attributes.computeIfAbsent(UriPatternAttributeKey) { "$currentUrl" }
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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


// replace path/query parameter in the URI pattern
url(
currentUrl.substitutePathParameters(parameters),
)
}

/**
* Substitute URI path and/or query parameter with provided value
*
* Example:
* ```
* client.get("https://blog.jetbrains.com/kotlin/{year}/{month}/{name}/") {
* pathParameter("year", 2023)
* pathParameter("month", "11")
* pathParameter("name", "kotlin-1-9-20-released")
* }.bodyAsText()
* ```
*
* @param key parameter name
* @param value parameter value
* @see pathParameters
*/
public fun pathParameter(key: String, value: Any) {
pathParameters(listOf(key to value))
}

public companion object
}

Expand Down Expand Up @@ -354,3 +402,31 @@ public class SSEClientResponseAdapter : ResponseAdapter {
}
}
}

/**
* Attribute key for URI pattern
*/
public val UriPatternAttributeKey: AttributeKey<String> = AttributeKey("UriPatternAttributeKey")

private fun Url.substitutePathParameters(substitutions: Iterable<Pair<String, Any>>) =
URLBuilder().also { builder ->
val substitutionsMap = substitutions.associate { (key, value) -> "{$key}" to "$value" }

builder.host = host
builder.protocol = protocol
builder.port = port
builder.pathSegments =
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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()

pathSegments.map { pathSegment ->
substitutionsMap[pathSegment] ?: pathSegment
}
parameters.forEach { parameterName, parameterValues ->
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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.

parameterValues.forEach { parameterValue ->
builder.parameters.append(
name = parameterName,
value = substitutionsMap[parameterValue] ?: parameterValue,
)
}
}
builder.user = user
builder.password = password
Copy link
Member

Choose a reason for hiding this comment

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

We need to copy fragment and trailingQuery too

Copy link
Author

Choose a reason for hiding this comment

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

}.build()
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
*/

kotlin.sourceSets {
jvmMain {
dependencies {
api(libs.micrometer)
implementation(project(":ktor-client:ktor-client-core"))
}
}
jvmTest {
dependencies{
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,274 @@
/*
* Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
*/

package io.ktor.client.plugins.metrics.micrometer

import io.ktor.client.*
import io.ktor.client.call.*
import io.ktor.client.plugins.api.*
import io.ktor.client.request.*
import io.ktor.client.statement.*
import io.ktor.util.*
import io.ktor.util.pipeline.*
import io.ktor.utils.io.*
import io.micrometer.core.instrument.*
import io.micrometer.core.instrument.binder.http.Outcome
import io.micrometer.core.instrument.logging.*

public const val URI_PATTERN: String = "URI_PATTERN"

private const val TAG_TARGET_SCHEME = "target.scheme"
private const val TAG_TARGET_HOST = "target.host"
private const val TAG_TARGET_PORT = "target.port"
private const val TAG_URI = "uri"
private const val TAG_METHOD = "method"
private const val TAG_STATUS = "status"
private const val TAG_EXCEPTION = "exception"
private const val TAG_VALUE_UNKNOWN = "UNKNOWN"

private val EMPTY_EXCEPTION_TAG: Tag = Tag.of(TAG_EXCEPTION, "null")

private val QUERY_PART_REGEX = "\\?.*$".toRegex()

private val ClientCallTimer = AttributeKey<Timer.Sample>("CallTimer")

/**
* A configuration for the [MicrometerMetrics] plugin.
*/
@KtorDsl
public class MicrometerMetricsConfig {

/**
* Specifies the base name (prefix) of Ktor metrics used for monitoring HTTP client requests.
* For example, the default "ktor.http.client.requests" values results in the following metrics:
* - "ktor.http.client.requests.count"
* - "ktor.http.client.requests.seconds.max"
*
* If you change it to "custom.metric.name", the mentioned metrics will look as follows:
* - "custom.metric.name.count"
* - "custom.metric.name.seconds.max"
* @see [MicrometerMetrics]
*/
public var metricName: String = "ktor.http.client.requests"

/**
* Extra tags to add to the metrics
*/
public var extraTags: Iterable<Tag> = emptyList()

/**
* Whether to drop the query part of the URL in the tag or not. Default: `true`
*/
public var dropQueryPartInUriTag: Boolean = true

/**
* Whether to use expanded URL when the pattern is unavailable or not. Default: `true`
*
* Note that setting this option to `true` without using URI templates with [HttpRequestBuilder.pathParameters]
* might lead to cardinality blow up.
*/
public var useExpandedUrlWhenPatternUnavailable: Boolean = true

/**
* Specifies the meter registry for your monitoring system.
* The example below shows how to create the `PrometheusMeterRegistry`:
* ```kotlin
* install(MicrometerMetrics) {
* registry = PrometheusMeterRegistry(PrometheusConfig.DEFAULT)
* }
* ```
* @see [MicrometerMetrics]
*/
public var registry: MeterRegistry = LoggingMeterRegistry()
set(value) {
field.close()
field = value
}
}

/**
* A client's plugin that provides the capability to meter HTTP calls with micrometer
Copy link
Member

Choose a reason for hiding this comment

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

Can you please add in Kdoc example of plugin usage?

*
*/
public val MicrometerMetrics: ClientPlugin<MicrometerMetricsConfig> =
createClientPlugin("MicrometerMetrics", ::MicrometerMetricsConfig) {
val metricName = pluginConfig.metricName
val extraTags = pluginConfig.extraTags
val dropQueryPartInUriTag = pluginConfig.dropQueryPartInUriTag
val useExpandedUrlWhenPatternUnavailable = pluginConfig.useExpandedUrlWhenPatternUnavailable
val meterRegistry = pluginConfig.registry

fun Timer.Builder.addDefaultTags(request: HttpRequestBuilder) =
tags(
Tags.of(
Tag.of(TAG_TARGET_SCHEME, request.url.protocol.name),
Tag.of(TAG_TARGET_HOST, request.host),
Tag.of(TAG_TARGET_PORT, "${request.port}"),
Tag.of(TAG_METHOD, request.method.value),
Tag.of(
TAG_URI,
request.attributes.getOrNull(UriPatternAttributeKey)
.let { it ?: request.url.takeIf { useExpandedUrlWhenPatternUnavailable }?.toString() }
?.removeHostPart(request.host)
?.let { it.takeUnless { dropQueryPartInUriTag } ?: it.removeQueryPart() }
?: TAG_VALUE_UNKNOWN
),
)
).tags(extraTags)

fun Timer.Builder.addDefaultTags(request: HttpRequest) =
tags(
Tags.of(
Tag.of(TAG_TARGET_SCHEME, request.url.protocol.name),
Tag.of(TAG_TARGET_HOST, request.url.host),
Tag.of(TAG_TARGET_PORT, "${request.url.port}"),
Tag.of(TAG_METHOD, request.method.value),
Tag.of(
TAG_URI,
request.attributes.getOrNull(UriPatternAttributeKey)
.let { it ?: request.url.takeIf { useExpandedUrlWhenPatternUnavailable }?.toString() }
Copy link
Member

Choose a reason for hiding this comment

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

Can we extract it so it does not have copypaste?

Copy link
Author

@monosoul monosoul Jun 20, 2024

Choose a reason for hiding this comment

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

Something like this: f802824 ?

?.removeHostPart(request.url.host)
?.let { it.takeUnless { dropQueryPartInUriTag } ?: it.removeQueryPart() }
?: TAG_VALUE_UNKNOWN
),
)
).tags(extraTags)

on(SendHook) { request ->
val timer = Timer.start(meterRegistry)
request.attributes.put(ClientCallTimer, timer)

try {
proceed()
} catch (cause: Throwable) {
timer.stop(
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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 😅

Timer.builder(metricName)
.addDefaultTags(request)
.tags(
Tags.of(
Outcome.CLIENT_ERROR.asTag(),
Tag.of(TAG_STATUS, TAG_VALUE_UNKNOWN),
cause.toTag(),
)
)
.register(meterRegistry)
)
throw cause
}
}

on(ReceiveHook) { call ->
val timer = call.attributes.getOrNull(ClientCallTimer)

try {
proceed()
} catch (cause: Throwable) {
timer?.stop(
Timer.builder(metricName)
.addDefaultTags(call.request)
.tags(
Tags.of(
Outcome.CLIENT_ERROR.asTag(),
Tag.of(TAG_STATUS, TAG_VALUE_UNKNOWN),
cause.toTag(),
)
)
.register(meterRegistry)
)
throw cause
}
}

on(ResponseHook) { response ->
val timer = response.call.attributes.getOrNull(ClientCallTimer)

try {
proceed()
timer?.stop(
Timer.builder(metricName)
.addDefaultTags(response.request)
.tags(
Tags.of(
Outcome.forStatus(response.status.value).asTag(),
Tag.of(TAG_STATUS, "${response.status.value}"),
EMPTY_EXCEPTION_TAG,
)
)
.register(meterRegistry)
)
} catch (cause: Throwable) {
timer?.stop(
Timer.builder(metricName)
.addDefaultTags(response.request)
.tags(
Tags.of(
Outcome.CLIENT_ERROR.asTag(),
Tag.of(TAG_STATUS, TAG_VALUE_UNKNOWN),
cause.toTag(),
)
)
.register(meterRegistry)
)
throw cause
}
}
}

private fun Throwable.toTag(): Tag =
Tag.of(
TAG_EXCEPTION,
cause?.javaClass?.simpleName ?: javaClass.simpleName,
)

private fun String.removeHostPart(host: String) = replace("^.*$host[^/]*".toRegex(), "")
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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


private fun String.removeQueryPart() = replace(QUERY_PART_REGEX, "")

private object ResponseHook : ClientHook<suspend ResponseHook.Context.(response: HttpResponse) -> Unit> {
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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


class Context(private val context: PipelineContext<HttpResponse, Unit>) {
suspend fun proceed() = context.proceed()
}

override fun install(
client: HttpClient,
handler: suspend Context.(response: HttpResponse) -> Unit
) {
client.receivePipeline.intercept(HttpReceivePipeline.State) {
handler(Context(this), subject)
}
}
}

private object SendHook : ClientHook<suspend SendHook.Context.(response: HttpRequestBuilder) -> Unit> {

class Context(private val context: PipelineContext<Any, HttpRequestBuilder>) {
suspend fun proceed() = context.proceed()
}

override fun install(
client: HttpClient,
handler: suspend Context.(request: HttpRequestBuilder) -> Unit
) {
client.sendPipeline.intercept(HttpSendPipeline.Monitoring) {
handler(Context(this), context)
}
}
}

private object ReceiveHook : ClientHook<suspend ReceiveHook.Context.(call: HttpClientCall) -> Unit> {

class Context(private val context: PipelineContext<HttpResponseContainer, HttpClientCall>) {
suspend fun proceed() = context.proceed()
}

override fun install(
client: HttpClient,
handler: suspend Context.(call: HttpClientCall) -> Unit
) {
client.responsePipeline.intercept(HttpResponsePipeline.Receive) {
handler(Context(this), context)
}
}
}
Loading