Align OpenTelemetry HTTP metrics with semantic conventions#5188
Align OpenTelemetry HTTP metrics with semantic conventions#5188
Conversation
Per https://opentelemetry.io/docs/specs/semconv/http/http-metrics/#http-server: - `http.server.request.duration`: unit `s` (was `ms`); record seconds; add recommended ExplicitBucketBoundariesAdvice - `http.server.active_requests`: unit `{request}` (was `1`) - `http.server.request.total`: unit `{request}` (was `1`); note in docstring that this metric is not part of the semantic conventions Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Aligns Tapir’s OpenTelemetry HTTP server metrics (OpenTelemetryMetrics) with the OpenTelemetry HTTP server semantic conventions (names/units/buckets), so exported metrics match the spec and common dashboards/collectors.
Changes:
- Renames documented metric names to
http.server.*and updates docstrings to reference the actual emitted metric/attribute names. - Updates metric units for active requests / request totals to
{request}. - Updates request duration histogram to record seconds (from nanoseconds) and adds explicit bucket boundary advice per the HTTP semconv recommendations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| copy(metrics = metrics :+ requestTotal(meter, labels)) | ||
|
|
||
| /** Registers a `request_duration_seconds{path, method, status, phase}` histogram (assuming default labels). */ | ||
| /** Registers a `http.server.request.duration` histogram (assuming default labels). */ |
There was a problem hiding this comment.
The addRequestsDuration Scaladoc no longer mentions the unit, but the implementation now records values in seconds (setUnit("s") and nanos-to-seconds conversion). Please document the unit here as well to avoid consumers assuming milliseconds (especially since older versions used ms).
| /** Registers a `http.server.request.duration` histogram (assuming default labels). */ | |
| /** Registers a `http.server.request.duration` histogram (assuming default labels), recording durations in seconds (`s`). */ |
| .setExplicitBucketBoundariesAdvice( | ||
| java.util.Arrays.asList[java.lang.Double](0.005d, 0.01d, 0.025d, 0.05d, 0.075d, 0.1d, 0.25d, 0.5d, 0.75d, 1d, 2.5d, 5d, 7.5d, 10d) | ||
| ) |
There was a problem hiding this comment.
The explicit bucket boundaries list is hard-coded inline, which hurts readability and makes it harder to reuse/update in the future. Consider extracting the boundaries into a named val/private constant (e.g., httpServerRequestDurationBucketsSeconds) and referencing it here.
| m.eval { | ||
| val requestStart = Instant.now() | ||
| def duration = Duration.between(requestStart, Instant.now()).toMillis.toDouble | ||
| def duration = Duration.between(requestStart, Instant.now()).toNanos.toDouble / 1e9 | ||
| EndpointMetric() |
There was a problem hiding this comment.
Instant.now()/Duration.between uses the wall clock, so NTP/time adjustments can produce negative or skewed durations that end up recorded in the histogram. For elapsed-time measurement, prefer a monotonic source (e.g., capture System.nanoTime() at request start and compute (System.nanoTime() - startNanos) / 1e9d).
- Scaladoc on `addRequestsDuration` now notes durations are in seconds - Extract explicit bucket boundaries into `HttpServerRequestDurationBucketsSeconds` constant - Measure duration with `System.nanoTime` (monotonic) instead of `Instant.now` to avoid wall-clock skew producing negative/bad values Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Brings the metrics registered by
OpenTelemetryMetricsin line with the OpenTelemetry HTTP server semantic conventions:http.server.request.duration: unit changed frommstos; values now recorded in seconds (computed from nanoseconds to preserve sub-ms precision); added the spec-recommendedExplicitBucketBoundariesAdvice([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]).http.server.active_requests: unit changed from1to{request}.http.server.request.total: unit changed from1to{request}. Docstring now notes this metric is not defined by the semantic conventions — the count is implicitly provided by the duration histogram. Kept becauseaddRequestsTotalis part of the public API.http.response.status_code,http.route) instead of the previous placeholders.No attribute keys changed; existing tests don't assert on units or exact duration values, so no test changes were needed.
Test plan
opentelemetryMetrics/test(Scala 2.13)opentelemetryMetrics3/test(Scala 3)http.server.request.durationin seconds with the recommended buckets🤖 Generated with Claude Code