HTTP duration metrics should match HTTP span duration #69
Conversation
| @@ -56,6 +56,8 @@ operations. By adding HTTP attributes to metric events it allows for finely tune | |||
|
|
|||
| This metric is required. | |||
|
|
|||
| When this metric is reported alongside an HTTP server span, the metric value SHOULD be the same as the HTTP server span duration. | |||
There was a problem hiding this comment.
While I 100% agree with this statement, I think that the current shape of the API does not allow us to implement it. We can't retrieve start/end timestamps from a Span without converting it to SpanData first (because of the read/write API split), which is very wasteful. In the Java instrumentation we've decided to use current time at start/end of the operation (unless the start/end timestamps are explicitly provided by the instrumentation), and they are always a bit off compared to the span timestamps.
Also see open-telemetry/opentelemetry-java-instrumentation#5905 for a bit more context
There was a problem hiding this comment.
While I 100% agree with this statement, I think that the current shape of the API does not allow us to implement it.
Correct, and that's exactly one of the reason why "SHOULD" is used rather than "MUST".
There was a problem hiding this comment.
Fair enough. It does seem a bit weird that we're adding a recommendation that can't be fulfilled at the moment, but I suppose that leaves us an option to fix that in the future.
There was a problem hiding this comment.
ya, good point, I guess it's more "aspirational" and hopefully will drive improvement in the SDK to make it a reality
The purpose of this PR is to clarify one of the easier questions that came up as part of open-telemetry/opentelemetry-specification#3520.
I don't think I worded it great, if anyone has suggestions 🙇♂️