-
Notifications
You must be signed in to change notification settings - Fork 564
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
Support http.server.request_count metric in otelhttp. #2482
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2482 +/- ##
=====================================
Coverage 74.3% 74.3%
=====================================
Files 144 144
Lines 6563 6568 +5
=====================================
+ Hits 4877 4882 +5
Misses 1543 1543
Partials 143 143
|
@@ -202,6 +206,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |||
|
|||
// Add metrics | |||
attributes := append(labeler.Get(), semconv.HTTPServerMetricAttributesFromHTTPRequest(h.operation, r)...) | |||
h.counters[RequestCount].Add(ctx, 1, attributes...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this necessary? Can't the same information be extracted from the count of the server latency histogram?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using Dynatrace, at least in their UI I can't seem to find a way to get the count out of the http.server.duration
metric, is it required to be available by opentelemetry?
Can this keep to be considered? I have no good way of getting the request count on my provider without this metric, which seems to be required by the spec. |
I ended up deleting this PR unintentionally because I purged all my repositories. My list got way too long. https://github.com/open-telemetry/opentelemetry-go-contrib/pull/2482/files still fortunately shows the diff. |
I'm not seeing this metric being required in the spec. |
Hmm you're right, I think I was confusing it with another thing. It isn't even mentioned there. The only thing I'm afraid is adding it to my code, and in the future this being added to the library and it counting the request 2 times. |
Fixes #2481