Http4k OpenTelemetry Metrics deviate from OTel Semantic Conventions

[krissrex]

Background

OpenTelemetry has a recommended way of naming metrics: the Semantic Conventions.

They also have a shared implementation of metric factories used by their intrumentors: client and server metric factories in open-telemetry/opentelemetry-java-instrumentation instrumentation-api-semconv module.
I don't believe it is published as a dependency we can use, but the settings used could be copied.

Problem

Http4k defines http.server.request.latency, where semconv says http.server.duration.

http4k/http4k-core/src/main/kotlin/org/http4k/metrics/MetricsDefaults.kt

Lines 16 to 31 in 4185ce5

	val server = MetricsDefaults(
	"http.server.request.count" to "Total number of server requests",
	"http.server.request.latency" to "Timing of server requests"
	) {
	it.copy(
	labels = mapOf(
	"method" to it.request.method.toString(),
	"status" to it.response.status.code.toString(),
	"path" to it.routingGroup.replace('/', '_').replaceRegexes().replace('.', '_').replace(notAlphaNumUnderscore, "")
	)
	)
	}
	val client = MetricsDefaults(
	"http.client.request.count" to "Total number of client requests",
	"http.client.request.latency" to "Timing of client requests"

This is defined in the http4k-core, then used in http4k-opentelemetry:

http4k/http4k-opentelemetry/src/main/kotlin/org/http4k/filter/metricsFiltersOpenTelemetryExtensions.kt

Lines 17 to 23 in 4185ce5

	fun RequestTimer(
	meter: Meter = Http4kOpenTelemetry.meter,
	name: String = defaults.timerDescription.first,
	description: String = defaults.timerDescription.second,
	labeler: HttpTransactionLabeler = defaults.labeler,
	clock: Clock = systemUTC()
	): Filter {

Change

I doubt we can use the same implementation as instrumentation-api-semconv.
Instead, we have to reproduce the conventions in http4k.

Rename

I suggest renaming the metric to http.server.duration and http.client.duration.

Change unit to seconds

Semconv says seconds, we use ms.

Note that the unit recommended is s, but the reference instrumentation-api-semconv may use ms for backwards-compatible reasons.

http4k/http4k-opentelemetry/src/main/kotlin/org/http4k/filter/metricsFiltersOpenTelemetryExtensions.kt

Lines 24 to 25 in 4185ce5

	val meterInstance = meter.histogramBuilder(name)
	.setDescription(description).setUnit("ms").build()

Explicit buckets

I suggest we use the defined histogram buckets from SemConv: [ 0, 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10 ].

Http4k would do it here:

http4k/http4k-opentelemetry/src/main/kotlin/org/http4k/filter/metricsFiltersOpenTelemetryExtensions.kt

Lines 24 to 25 in 4185ce5

	val meterInstance = meter.histogramBuilder(name)
	.setDescription(description).setUnit("ms").build()

For the reference implementation, this is done in
https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/15337549e3160b6db4acf8ea5ce39fb95a6bd63d/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpMetricsUtil.java#L25-L29
and
https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/15337549e3160b6db4acf8ea5ce39fb95a6bd63d/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpMetricsUtil.java#L37-L41

Label

I also suggest assuring that all attached labels are correct according to SemConv, and all required labels are present.
These are for server:

• http.route (if possible)
• http.request.method
• http.response.status_code (if possible)
• server.address
• server.port (when not 80 or 443)
• url.scheme (http/https)

For client:

• http.request.method
• http.response.status_code (if possible)
• server.address
• server.port (when not 80 or 443)

Possible issues

This is a downstream breaking change, because OpenTelemetry metric consumers will now create a new metric,
and any usage of latency in dashboards, alarms etc. will no longer work untill migrated to duration.

An option is to not remove the old metric, but rather introduce another one for duration, in a backwards compatible manner.
(This will increase the amount of metrics, which for some may incur costs for ingestion and storage.)

[jenarros-lw]

+1

Although it is a breaking change, introducing duration first before removing latency in a separate release will allow migrating any dependencies on the latency metric to duration without disruption.

[krissrex]

The bucket advice API seems to be in incubation still, and is not available right now.
Thus explicit buckets has to wait.

[stevesea]

version 1.20.0 of the otel semconv was http.server.duration

the latest semconv standard (1.22) now recommends http.server.request.duration https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-metrics.md
you could look at OTEL_SEMCONV_STABILITY_OPT_IN to decide which metrics to create, as described at that URL