-
Notifications
You must be signed in to change notification settings - Fork 8
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
Make metrics collection in throttle and middleware packages more flexible #11
Conversation
labels := reqInfo.makeLabels() | ||
labels[httpRequestMetricsLabelStatusCode] = strconv.Itoa(status) | ||
c.Durations.With(labels).Observe(time.Since(startTime).Seconds()) | ||
func (pm *HTTPRequestPrometheusMetrics) IncInFlightRequests(requestInfo HTTPRequestInfoMetrics) { |
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.
Let's add header comments for public functions
method string | ||
routePattern string | ||
userAgentType string | ||
func (pm *HTTPRequestPrometheusMetrics) DecInFlightRequests(requestInfo HTTPRequestInfoMetrics) { |
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.
Let's add header comments for public functions
// HTTPRequestInfoMetrics represents a request info for collecting metrics. | ||
type HTTPRequestInfoMetrics struct { | ||
Method string | ||
RoutePattern 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.
Are we safe here due to the combinatorics of different HTTP routes?
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.
No, it's supposed to be the route pattern, not the final URL.
…ible Now middlewares receive interfaces instead of the concrete Prometheus implementations.
4f031ed
to
0d21b9c
Compare
Now middlewares receive interfaces instead of the concrete Prometheus implementations.
This change is a breaking change. However, given the library’s current limited usage, we have decided not to bump the major version at this time.