feat: route stat name #6310

Inode1 · 2025-06-13T22:09:39Z

What type of PR is this?

What this PR does / why we need it:

Add support for determining route stat names in BackendTrafficPolicy

Supported: xRoute
Supported Formatters: %ROUTE_KIND%, %ROUTE_NAME%, %ROUTE_NAMESPACE%, %ROUTE_RULE_NAME%, %ROUTE_RULE_NUMBER%

Which issue(s) this PR fixes:

Fixes #6190

Release Notes: Yes

codecov · 2025-06-14T20:36:55Z

Codecov Report

❌ Patch coverage is 88.88889% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.15%. Comparing base (a9fa022) to head (1e6d51f).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/gatewayapi/backendtrafficpolicy.go	83.33%	3 Missing and 2 partials ⚠️
api/v1alpha1/validation/envoyproxy_validate.go	90.90%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6310      +/-   ##
==========================================
- Coverage   72.31%   71.15%   -1.16%     
==========================================
  Files         231      274      +43     
  Lines       34084    34857     +773     
==========================================
+ Hits        24647    24802     +155     
- Misses       7664     8260     +596     
- Partials     1773     1795      +22

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Inode1 · 2025-06-14T21:38:52Z

/retest

arkodg · 2025-06-16T18:02:35Z

seeing the API being added to BTP, is the use case to enable this only on some routes or all ? If all is the most common use case, , then lets start with a field in EnvoyProxy and later one in BTP

Inode1 · 2025-06-17T07:14:03Z

seeing the API being added to BTP, is the use case to enable this only on some routes or all ? If all is the most common use case, , then lets start with a field in EnvoyProxy and later one in BTP

Hi, I thought we agreed to add stats only in BTP), i.e., only for a specific route using the template. I don't quite understand how the settings of EnvoyProxy and BTP will work together. It seems that in the discussion it was decided that this should be opt-in, as enabling it for all routes through EnvoyProxy could lead to a large number of metrics, which might affect the performance of Envoy (and it's unclear how to disable the option at the EnvoyProxy level for a specific route). Additionally, with BTP, you can specify more specific things, such as the name of the route (routeStatPrefix: name=/route), as well as more concrete business-related aspects, like tenant, for example.

arkodg · 2025-06-17T18:36:11Z

okay, lets start with BTP first

arkodg · 2025-06-17T18:38:16Z

api/v1alpha1/backendtrafficpolicy_types.go

+	// %ROUTE_RULE_NAME%: name of the Gateway API xRoute section
+	// %ROUTE_RULE_NUMBER%: sequence number of the Gateway API xRoute resource
+	// Example: %ROUTE_KIND%/%ROUTE_NAMESPACE%/%ROUTE_NAME%/rule/%ROUTE_RULE_NUMBER% => httproute/my-ns/my-route/rule/0
+	//


Suggested change

//

// Disabled by default.

//

arkodg · 2025-06-17T19:02:56Z

api/v1alpha1/backendtrafficpolicy_types.go

+type BackendMetrics struct {
+	// RouteStatName defines the value of the Route stat_prefix, determining how the route stats are named.
+	// For more details, see envoy docs: https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#config-route-v3-route
+	// The supported operators for this pattern are:


routes can be attached to multiple gateways/parentRefs which will need to be represented here in case the user doesnt want to aggregate them into 1, and prefers per parent per route metrics
https://gateway-api.sigs.k8s.io/reference/spec/#httproutespec

As far as I understand your point, I thought this function was supposed to set the stats for the route in the gateways.

https://github.com/envoyproxy/gateway/blob/main/internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-different-ns.out.yaml should help understand how 1 route can attach to multiple gateways
and this introduces two uses cases for metrics granularity that is mentioned in #6310 (comment)

Do we want to check and validate this somehow?)

https://github.com/envoyproxy/gateway/blob/main/internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-different-ns.out.yaml should help understand how 1 route can attach to multiple gateways and this introduces two uses cases for metrics granularity that is mentioned in #6310 (comment)

It seems I understand what this is about. In the function https://github.com/envoyproxy/gateway/blob/26f9b20a7d5f3060f841787e677c0ed127adc759/internal/gatewayapi/backendtrafficpolicy.go#L664, there is processing for the Gateway, but here we have the IR, and we need to somehow map it to a resource to get the template parameters. We could do this via "resources", but that would be expensive — we'd have to go through all resources for every route, which is O(n²). Maybe we should reconsider the metadata field for all routes?)

you should have access to it indirectly via the IR listener name

gateway/internal/gatewayapi/helpers.go

Line 377 in 26f9b20

func irListenerName(listener *ListenerContext) string {

thinking out loud, it may help to start off with an API within EnvoyProxy which is easier to build out its not tied to policy translation

arkodg · 2025-06-17T19:03:36Z

api/v1alpha1/backendtrafficpolicy_types.go

+	// Example: %ROUTE_KIND%/%ROUTE_NAMESPACE%/%ROUTE_NAME%/rule/%ROUTE_RULE_NUMBER% => httproute/my-ns/my-route/rule/0
+	//
+	// +optional
+	RouteStatName string `json:"routeStatName,omitempty"`


must be a ptr since its optional

Done, but can you explain why it's like that? controllergen automatically makes string fields with omitempty optional. I understand that we might want to distinguish between a field being unset and being set to an empty "" in order to apply some default, but that's not our case.

We don't want to generate additional stats if it's unset, so making it a ptr makes it opt in

Yeah "", may not always means empty, sometimes it could be a valid value

arkodg · 2025-06-17T19:06:21Z

...rnal/gatewayapi/testdata/gateway-with-two-listeners-with-tcproutes-with-sectionname.out.yaml

@@ -188,6 +188,10 @@ xdsIR:
            name: tcproute/default/tcproute-1/rule/-1/backend/0
            protocol: TCP
            weight: 1
+        metadata:


its expensive to add this from a mem usage perspective, why this this needed

I explained below why the metadata is needed.

can we do it conditionally only if the feature is enabled? Maybe that also related to Arko's other comment about EnvoyProxy level feature as opposed to BTP.

arkodg · 2025-06-17T19:09:47Z

internal/ir/xds.go

 	// settings of upstream connection
 	BackendConnection *BackendConnection `json:"backendConnection,omitempty" yaml:"backendConnection,omitempty"`
 	// DNS is used to configure how DNS resolution is handled for the route
 	DNS *DNS `json:"dns,omitempty" yaml:"dns,omitempty"`
+	// Metrics defines the schema for metric stat configuration.
+	Metrics *egv1a1.BackendMetrics `json:"metrics,omitempty" yaml:"metrics,omitempty"`


my suggestion would be to convert BackendMetrics into RouteStat name in the gateway api layer similar to what ClusterStatName is doing, add add that as a sub field here

I see your point, but Routes are processed before BTP is handled — see this line. So at the Route processing stage, I can't create RouteStats, and instead I store all the necessary route information in metadata. I could generate RouteStats in this place, but I still need additional route-related data like SectionName and RouteIndex, which are only available during Route processing.

recommend using a StatName field in HTTPRoute to address this

recommend using a StatName field in HTTPRoute to address this

Sorry, I didn't understand you. Should StatName already contain the fully parsed and built stat_prefix field, or will it contain information about the route (SectionName, RuleIndex)? If it's the former, then we can do it that way, but we'll have to drop SectionName and RuleIndex—in that case, the metadata won't be needed either. Are you okay with that?

you should have access to RULE_NAME and RULE_NUMBER during route processing
but yes, any values coming from the policy cannot be used (today, we can do better in the future with some post processing if really needed)

Inode1 · 2025-06-30T18:57:32Z

/retest

Inode1 · 2025-06-30T18:58:13Z

@arkodg PTAL

arkodg · 2025-07-17T21:50:07Z

internal/gatewayapi/backendtrafficpolicy.go

@@ -430,6 +431,16 @@ func (t *Translator) translateBackendTrafficPolicyForRouteWithMerge(
 		return fmt.Errorf("error merging policies: %w", err)
 	}

+	if policy.Spec.Telemetry != nil && policy.Spec.Telemetry.Metrics != nil {


looks like this needs to be removed

arkodg · 2025-07-17T21:51:23Z

internal/ir/xds.go

@@ -739,6 +739,8 @@ type HTTPClientTimeout struct {
 type HTTPRoute struct {
 	// Name of the HTTPRoute
 	Name string `json:"name" yaml:"name"`
+	// StatName is the name of the route used for statistics and metrics.
+	StatName string `json:"statName,omitempty" yaml:"statName,omitempty"`


should be a ptr since its optional

arkodg · 2025-10-29T00:48:36Z

hey @Inode1 do you have a use case for using %ROUTE_RULE_NUMBER% instead of %ROUTE_SECTION_NAME% which is already available in the IR today ? my preference would be to avoid adding RuleIndex to the IR unless we absolutely need it

Inode1 · 2025-11-03T05:11:47Z

hey @Inode1 do you have a use case for using %ROUTE_RULE_NUMBER% instead of %ROUTE_SECTION_NAME% which is already available in the IR today ? my preference would be to avoid adding RuleIndex to the IR unless we absolutely need it

No, to be honest, I don't need formatters tags at all, just a custom stats name string).

Done, delete ROUTE_RULE_NUMBER

arkodg · 2025-11-03T20:04:59Z

thanks @Inode1, will take a look at this PR next week, after v1.6 is out

arkodg · 2025-11-15T02:02:48Z

internal/gatewayapi/backendtrafficpolicy.go

@@ -955,6 +973,7 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(
 			setIfNil(&r.TCPKeepalive, tf.TCPKeepalive)
 			setIfNil(&r.Timeout, tf.Timeout)
 			setIfNil(&r.DNS, tf.DNS)
+			r.StatName = buildRouteStatName(routeStatName, r.Metadata)


shouldnt this be setIfNil ?

r.StatName is a value, not a pointer

the PR sets setIfNil(&r.StatName, buildRouteStatName(routeStatName, r.Metadata)) in applyTrafficFeatureToRoute

Yup, you are right. Done

arkodg · 2025-11-15T02:04:49Z

internal/gatewayapi/backendtrafficpolicy.go

@@ -1002,6 +1021,7 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(
 				continue
 			}

+			r.StatName = buildRouteStatName(routeStatName, r.Metadata)


shouldnt this use setIfNIl ?

arkodg

LGTM thanks

zirain · 2025-11-18T00:10:19Z

@Inode1 can you fix the DCO?

Signed-off-by: i.makarychev <[email protected]>

Signed-off-by: i.makarychev <[email protected]> Signed-off-by: ivan <ivan> Signed-off-by: i.makarychev <[email protected]>

Signed-off-by: i.makarychev <[email protected]>

Signed-off-by: i.makarychev <[email protected]> Signed-off-by: i.makarychev <[email protected]>

Signed-off-by: i.makarychev <[email protected]>

Inode1 requested a review from a team as a code owner June 13, 2025 22:09

Inode1 force-pushed the feat-per-route-metrics branch from 559f768 to 1484995 Compare June 14, 2025 19:41

Inode1 marked this pull request as draft June 14, 2025 19:43

Inode1 force-pushed the feat-per-route-metrics branch 3 times, most recently from 828420c to 04cd1df Compare June 14, 2025 20:28

Inode1 marked this pull request as ready for review June 14, 2025 21:39

arkodg reviewed Jun 17, 2025

View reviewed changes

Inode1 force-pushed the feat-per-route-metrics branch 2 times, most recently from 44d329f to 26f9b20 Compare June 19, 2025 06:55

Inode1 marked this pull request as draft June 19, 2025 08:48

Inode1 force-pushed the feat-per-route-metrics branch 2 times, most recently from 733185c to 47a2d99 Compare June 22, 2025 19:57

Inode1 marked this pull request as ready for review June 22, 2025 20:19

Inode1 requested a review from arkodg June 24, 2025 12:45

arkodg requested a review from guydc June 30, 2025 20:32

arkodg added this to the v1.5.0-rc.1 Release milestone Jun 30, 2025

arkodg reviewed Jul 17, 2025

View reviewed changes

arkodg modified the milestones: v1.6.0 Milestone, Backlog Oct 29, 2025

Inode1 force-pushed the feat-per-route-metrics branch 7 times, most recently from ad3f911 to 62eecee Compare November 2, 2025 17:53

arkodg mentioned this pull request Nov 15, 2025

allow the change altStatName for backend #7520

Open

arkodg reviewed Nov 15, 2025

View reviewed changes

Inode1 force-pushed the feat-per-route-metrics branch from dfea4b8 to 62439e0 Compare November 17, 2025 19:32

arkodg approved these changes Nov 17, 2025

View reviewed changes

arkodg requested review from a team November 17, 2025 21:27

zirain force-pushed the feat-per-route-metrics branch from 62439e0 to f7ae8bc Compare November 17, 2025 23:38

zirain approved these changes Nov 18, 2025

View reviewed changes

i.makarychev and others added 6 commits November 18, 2025 20:23

rebase

c9f3b92

Signed-off-by: i.makarychev <[email protected]>

rebase

57e649d

Signed-off-by: i.makarychev <[email protected]>

review fixes

d8ac8ce

Signed-off-by: i.makarychev <[email protected]> Signed-off-by: ivan <ivan> Signed-off-by: i.makarychev <[email protected]>

delete ruleIndex

4b51fa0

Signed-off-by: i.makarychev <[email protected]>

rebase

f88e5ee

Signed-off-by: i.makarychev <[email protected]> Signed-off-by: i.makarychev <[email protected]>

review fixes

1e6d51f

Signed-off-by: i.makarychev <[email protected]>

Inode1 force-pushed the feat-per-route-metrics branch from f7ae8bc to 1e6d51f Compare November 18, 2025 17:23

arkodg merged commit b086308 into envoyproxy:main Nov 18, 2025
35 checks passed

feat: route stat name #6310

feat: route stat name #6310

Uh oh!

Conversation

Inode1 commented Jun 13, 2025

Uh oh!

codecov bot commented Jun 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Inode1 commented Jun 14, 2025

Uh oh!

arkodg commented Jun 16, 2025

Uh oh!

Inode1 commented Jun 17, 2025

Uh oh!

arkodg commented Jun 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Inode1 Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Inode1 commented Jun 30, 2025

Uh oh!

Inode1 commented Jun 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arkodg commented Oct 29, 2025

Uh oh!

Inode1 commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

codecov bot commented Jun 14, 2025 •

edited

Loading

Inode1 Jun 18, 2025 •

edited

Loading

Inode1 commented Nov 3, 2025 •

edited

Loading