-
Notifications
You must be signed in to change notification settings - Fork 284
outbound: Report per-route-backend request count metrics #2377
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
Conversation
When performing policy-based routing, proxies may dispatch requests through per-route backend configurations. In order to illustrate how routing rules apply and how backend distributions are being honored, this change adds two new metrics: * `outbound_http_route_backend_requests_total`; and * `outbound_grpc_route_backend_requests_total` Each of these metrics includes labels that identify a routes parent (i.e. a Service), the route resource being used, and the backend resource being used. This implementation does NOT implement any form of metrics eviction for these new metrics. This is tolerable for the short term as the cardinality of services and routes is generally much less than the cardinality of individual endpoints (where we do require timeout/eviction for metrics).
hawkw
left a comment
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.
looks good to me, the use of ExtractParam for metrics is very cool and makes me want to rewrite the rest of our metrics code (eventually). no blockers from me!
| impl<T> svc::ExtractParam<RequestCount, Http<T>> for ExtractMetrics { | ||
| fn extract_param(&self, params: &Http<T>) -> RequestCount { | ||
| RequestCount(self.metrics.http_requests_total( | ||
| params.params.concrete.parent_ref.clone(), | ||
| params.params.route_ref.clone(), | ||
| params.params.concrete.backend_ref.clone(), | ||
| )) | ||
| } | ||
| } | ||
|
|
||
| impl<T> svc::ExtractParam<RequestCount, Grpc<T>> for ExtractMetrics { | ||
| fn extract_param(&self, params: &Grpc<T>) -> RequestCount { | ||
| RequestCount(self.metrics.grpc_requests_total( | ||
| params.params.concrete.parent_ref.clone(), | ||
| params.params.route_ref.clone(), | ||
| params.params.concrete.backend_ref.clone(), | ||
| )) | ||
| } | ||
| } |
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.
neat, this could be a nice pattern to use for other metrics stuff eventually!
| assert_eq!( | ||
| lines, | ||
| vec![ | ||
| r#"outbound_http_route_backend_requests_total{parent_group="core",parent_kind="Service",parent_namespace="ns",parent_name="papa",parent_port="7979",parent_section_name="",route_group="policy.linkerd.io",route_kind="HTTPRoute",route_namespace="ns",route_name="default",backend_group="core",backend_kind="Service",backend_namespace="ns",backend_name="default",backend_port="8080",backend_section_name=""} 1"#, |
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.
nit, not actually important: it seems like it might be nicer to use multi-line string literals here with \ to escape whitespace? for example, this is the same string:
| r#"outbound_http_route_backend_requests_total{parent_group="core",parent_kind="Service",parent_namespace="ns",parent_name="papa",parent_port="7979",parent_section_name="",route_group="policy.linkerd.io",route_kind="HTTPRoute",route_namespace="ns",route_name="default",backend_group="core",backend_kind="Service",backend_namespace="ns",backend_name="default",backend_port="8080",backend_section_name=""} 1"#, | |
| "outbound_http_route_backend_requests_total{\ | |
| parent_group=\"core\",\ | |
| parent_kind=\"Service\",\ | |
| parent_namespace=\"ns\",\ | |
| parent_name=\"papa\",\ | |
| parent_port=\"7979\",\ | |
| parent_section_name=\"\",\ | |
| route_group=\"policy.linkerd.io\",\ | |
| route_kind=\"HTTPRoute\", | |
| route_namespace=\"ns\",\ | |
| route_name=\"default\",\ | |
| backend_group=\"core\",\ | |
| backend_kind=\"Service\",\ | |
| backend_namespace=\"ns\",\ | |
| backend_name=\"default\",\ | |
| backend_port=\"8080\",\ | |
| backend_section_name=\"\"\ | |
| } 1", |
but the label values are a little easier to read...
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.
yeah this test could definitely be prettier in a few ways. going to leave the mush of labels for now.
This proxy release adds new `outbound_http_route_backend_requests_total` and `outbound_grpc_route_backend_requests_total` metrics, which can be used to track how routing rules and backend distributions apply to requests. These metrics contain labels describing the route's parent (i.e. a Service), the route resource being used, and the backend resource being used by each request. In addition, this proxy release adds new `INFO`-level logs for failure accrual, and updates logging contexts with Kubernetes resource metadata. --- * outbound: Emit INFO-level logs on failure accrual changes (linkerd/linkerd2-proxy#2373) * app: Rename Metrics types as InboundMetrics and OutboundMetrics (linkerd/linkerd2-proxy#2376) * outbound: Configure balancers with service metadata (linkerd/linkerd2-proxy#2374) * outbound: Report per-route-backend request count metrics (linkerd/linkerd2-proxy#2372) * outbound: Report per-route-backend request count metrics (linkerd/linkerd2-proxy#2377)
This proxy release adds new `outbound_http_route_backend_requests_total` and `outbound_grpc_route_backend_requests_total` metrics, which can be used to track how routing rules and backend distributions apply to requests. These metrics contain labels describing the route's parent (i.e. a Service), the route resource being used, and the backend resource being used by each request. In addition, this proxy release adds new `INFO`-level logs for failure accrual, and updates logging contexts with Kubernetes resource metadata. --- * outbound: Emit INFO-level logs on failure accrual changes (linkerd/linkerd2-proxy#2373) * app: Rename Metrics types as InboundMetrics and OutboundMetrics (linkerd/linkerd2-proxy#2376) * outbound: Configure balancers with service metadata (linkerd/linkerd2-proxy#2374) * outbound: Report per-route-backend request count metrics (linkerd/linkerd2-proxy#2372) * outbound: Report per-route-backend request count metrics (linkerd/linkerd2-proxy#2377)
This branch adds documentation for the new metrics added in linkerd/linkerd2-proxy#2377 and linkerd/linkerd2-proxy#2380.
This branch adds documentation for the new metrics added in linkerd/linkerd2-proxy#2377 and linkerd/linkerd2-proxy#2380.
the documentation of our proxy metrics has not kept pace with all of the exciting telemetry that has been introduced since #1599 documented the state of our authorization policy metrics. this commit reworks the documentation to exhaustively document the families of metrics exported by the proxy. this commit does not introduce mention of the _inbound_ metrics that have been added, but does rename this section to "_Endpoint Metrics_" in order to be compatible with the future addition of `inbound_http_route_*`, `inbound_http_route_backend_*`, `inbound_grpc_route*`, and `inbound_grpc_route_backend_*` metrics. see: * linkerd/linkerd2-proxy#2377 * linkerd/linkerd2-proxy#2380 * linkerd/linkerd2-proxy#3086 * linkerd/linkerd2-proxy#3308 * linkerd/linkerd2-proxy#3334 Signed-off-by: katelyn martin <[email protected]>
* feat(proxy-metrics): document outbound policy routing metrics the documentation of our proxy metrics has not kept pace with all of the exciting telemetry that has been introduced since #1599 documented the state of our authorization policy metrics. this commit reworks the documentation to exhaustively document the families of metrics exported by the proxy. this commit does not introduce mention of the _inbound_ metrics that have been added, but does rename this section to "_Endpoint Metrics_" in order to be compatible with the future addition of `inbound_http_route_*`, `inbound_http_route_backend_*`, `inbound_grpc_route*`, and `inbound_grpc_route_backend_*` metrics. see: * linkerd/linkerd2-proxy#2377 * linkerd/linkerd2-proxy#2380 * linkerd/linkerd2-proxy#3086 * linkerd/linkerd2-proxy#3308 * linkerd/linkerd2-proxy#3334 Signed-off-by: katelyn martin <[email protected]> * chore(markdownlint): allow duplicate "labels" headers Signed-off-by: katelyn martin <[email protected]> --------- Signed-off-by: katelyn martin <[email protected]>
When performing policy-based routing, proxies may dispatch requests through per-route backend configurations. In order to illustrate how routing rules apply and how backend distributions are being honored, this change adds two new metrics:
outbound_http_route_backend_requests_total; andoutbound_grpc_route_backend_requests_totalEach of these metrics includes labels that identify a routes parent (i.e. a Service), the route resource being used, and the backend resource being used.
This implementation does NOT implement any form of metrics eviction for these new metrics. This is tolerable for the short term as the cardinality of services and routes is generally much less than the cardinality of individual endpoints (where we do require timeout/eviction for metrics).