-
Notifications
You must be signed in to change notification settings - Fork 604
chore: Add http_route label to HTTP server metrics #2330
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
base: master
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 21253676451Details
💛 - Coveralls |
26c6751 to
b810990
Compare
This commit adds the http_route label to HTTP server metrics, such as `http_server_request_duration_seconds_bucket`. This enables users to collect standard HTTP metrics per route rather than across all of Auth. Because `WithMetricAttributesFn` is only available in newer versions of otelhttp, this commit also upgrades the otelhttp package to v0.64.0. As a result, HTTP request metrics now use the newer `http_server_request_` naming convention rather than just `http_server_`.
This commit updates the semconv package to v1.38.0.
As described.
b810990 to
2aea070
Compare
| return | ||
| } | ||
|
|
||
| defer func() { |
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 think we should keep this defensive check here, it doesn't cost much.
| out := []attribute.KeyValue{ | ||
| routePatternAttr, | ||
| } | ||
|
|
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.
It's a nit, but the whitespace styling in places like this and the don't match the rest of the code base
| "go.opentelemetry.io/otel/trace" | ||
| ) | ||
|
|
||
| func getChiRoutePattern(r *http.Request) string { |
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.
Another nit on styling
What kind of change does this PR introduce?
This PR adds the
http_routelabel to standard HTTP server metrics collected by the otelhttp package. This enables users to collect HTTP metrics per route.Because
WithMetricAttributesFnis only available in newer versions of otelhttp, this commit also upgrades the otelhttp package to v0.63.0. As a result, HTTP request metrics now use the newerhttp_server_request_naming convention rather than justhttp_server_.What is the current behavior?
Currently, the
http_server_duration_milliseconds_bucketmetric does not include HTTP route labels. This means HTTP duration metrics are heavily skewed towards fast, frequently accessed API routes such as /user.What is the new behavior?
There are two changes introduced in this PR:
http_server_duration_milliseconds_bucketmetric is renamed tohttp_server_request_duration_seconds_buckethttp_server_request_duration_seconds_bucketmetric has an additional label,http_route, that matches thehttp_routelabel in thehttp_status_codes_totalmetric