feat(trafficpolicy): add configurable Envoy route stat_prefix support#13600
feat(trafficpolicy): add configurable Envoy route stat_prefix support#13600mihir-dixit2k27 wants to merge 7 commits intokgateway-dev:mainfrom
Conversation
Add a new opt-in statPrefix field to TrafficPolicy CRD that enables per-xRoute Envoy route metric namespacing via the route-level stat_prefix field on Envoy RouteAction. The field supports template variable substitution resolved at apply-time: - %NAMESPACE% -> replaced with the xRoute namespace - %NAME% -> replaced with the xRoute name - %RULE_NAME% -> replaced with the HTTPRoute rule name (when set) If %RULE_NAME% is used on a route rule that has no name, the literal token is kept in the resolved string and a controller warning is logged. Changes: - api/v1alpha1/kgateway/traffic_policy_types.go: add StatPrefixConfig type and StatPrefix field with kubebuilder validation pattern - api/v1alpha1/kgateway/zz_generated.deepcopy.go: add DeepCopy methods for StatPrefixConfig and update TrafficPolicySpec.DeepCopyInto - trafficpolicy/stat_prefix.go: new file with statPrefixIR type, Equals, Validate, and constructStatPrefix (raw template, no substitution at construct time) - trafficpolicy/traffic_policy_plugin.go: wire statPrefix into trafficPolicySpecIr, Equals, Validate; add apply-time template resolution with %RULE_NAME% unresolved warning in handlePerRoutePolicies - trafficpolicy/constructor.go: call constructStatPrefix - trafficpolicy/merge.go: add mergeStatPrefix and register it - trafficpolicy/stat_prefix_test.go: 13 unit tests covering Equals, construct-time no-substitution contract, apply-time resolution, nil-safety, and TrafficPolicy integration Signed-off-by: Mihir Dixit <dixitmihir1@gmail.com>
There was a problem hiding this comment.
Pull request overview
Adds opt-in per-route Envoy stat_prefix configuration to TrafficPolicy so operators can correlate xRoute resources with route-level metrics, with template substitution based on live route context.
Changes:
- Introduces
spec.statPrefixinTrafficPolicyCRD (with templating tokens) and wires it through IR construction + merge/equality/validation. - Applies
RouteAction.stat_prefixduring route translation with%NAMESPACE%,%NAME%,%RULE_NAME%substitution and warning behavior for unnamed rules. - Adds unit tests covering IR storage and apply-time substitution behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/kgateway/extensions2/plugins/trafficpolicy/traffic_policy_plugin.go | Applies the resolved stat_prefix to Envoy routes during per-route policy handling. |
| pkg/kgateway/extensions2/plugins/trafficpolicy/stat_prefix.go | Adds statPrefixIR and IR construction from the CRD field (stores raw template). |
| pkg/kgateway/extensions2/plugins/trafficpolicy/stat_prefix_test.go | Adds tests for equality/validation, IR construction, and apply-time substitution. |
| pkg/kgateway/extensions2/plugins/trafficpolicy/merge.go | Ensures statPrefix participates in TrafficPolicy merge behavior. |
| pkg/kgateway/extensions2/plugins/trafficpolicy/constructor.go | Constructs stat prefix IR during TrafficPolicy translation. |
| api/v1alpha1/kgateway/traffic_policy_types.go | Adds the statPrefix API field and schema validation markers/docs. |
| api/v1alpha1/kgateway/zz_generated.deepcopy.go | Updates autogenerated deepcopy logic for the new API types/fields. |
Comments suppressed due to low confidence (1)
pkg/kgateway/extensions2/plugins/trafficpolicy/traffic_policy_plugin.go:685
handlePerRoutePoliciesonly checksout.GetAction() == nilbut then unconditionally usesout.GetRoute()as a*RouteAction. If the route action is a redirect/direct_response (i.e.,GetAction()!=nilbutGetRoute()==nil), settingaction.StatPrefixwill panic whenstatPrefixis configured. Add a guard that returns early whenout.GetRoute()is nil (or only apply statPrefix when the action is a RouteAction).
// A parent route rule with a delegated backend will not have RouteAction set
if out.GetAction() == nil {
return
}
action := out.GetRoute()
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: mihir-dixit2k27 <143348248+mihir-dixit2k27@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: mihir-dixit2k27 <143348248+mihir-dixit2k27@users.noreply.github.com>
…phens The previous kubebuilder validation pattern ^[a-zA-Z0-9_%]+$ rejected hyphens, but Kubernetes resource names frequently contain hyphens (e.g. my-route). Since template variables like %NAME% resolve to the xRoute name, patterns like %NAMESPACE%_%NAME% would produce valid strings such as default_my-route that the old pattern would reject at admission time. Update the pattern to ^[a-zA-Z0-9_%-]+$ and correct the doc comment: - Remove the misleading claim that post-substitution output must be alphanumeric+underscore only - Clarify that hyphens are expected (Kubernetes names) and percent signs are expected as template token delimiters Signed-off-by: Mihir Dixit <dixitmihir1@gmail.com>
josh-pritchard
left a comment
There was a problem hiding this comment.
Thanks for the contribution. The unit tests look good but a change like this to plugins also requires translator tests that will result in new golden files that are representative of the new feature.
| // string and a warning will be logged by the controller) | ||
| // This field is only honored for HTTPRoute and GRPCRoute targets. | ||
| // +optional | ||
| StatPrefix *StatPrefixConfig `json:"statPrefix,omitempty"` |
There was a problem hiding this comment.
Make sure to run a make generate locally as your missing generated clients and CRDs from name change.
…refix Addresses code review feedback: - Adds CEL validation (XValidation) on statPrefix to restrict % usage to predefined tokens - Adds a nil-check guard in statPrefixIR.Validate() - Updates API docs to use ASCII arrows (->) instead of unicode - Adds a gateway translator test case with expected golden values Signed-off-by: Mihir Dixit <dixitmihir1@gmail.com>
137ba38 to
3c272e9
Compare
|
Thanks for the thorough review and guidance @josh-pritchard ! I've addressed all the feedback in the latest commits: Added CEL XValidation to statPrefixto strictly restrict % characters to only the predefined template tokens. (Note: I wasn't able to run make generate locally to update the generated CRDs/deepcopy files because my WSL environment is currently on Go 1.25, and this project now strictly requires Go 1.26. If it's easier, could a maintainer run the codegen update for me? Alternatively, I can update my environment to Go 1.26 and push the generated files later today—just let me know your preference!) |
Signed-off-by: Mihir Dixit <dixitmihir1@gmail.com>
3614f8c to
0bb8bc8
Compare
Description
Motivation: Operators have no way to correlate Kubernetes xRoute resources with Envoy route-level metrics — all routes share the same default
stat_prefix.What changed: Adds an opt-in
statPrefixfield toTrafficPolicythat setsRouteAction.stat_prefixin Envoy, enabling per-route metric namespacing. Supports template variable substitution (%NAMESPACE%,%NAME%,%RULE_NAME%) resolved at apply-time using live route context — the raw template is stored in the IR and substitution happens inhandlePerRoutePolicies()viapCtx.In.Parent. If%RULE_NAME%is used on an unnamed rule, the literal token is preserved in the resolved string and a controller warning is logged.Related issues: Fixes #13564
Change Type
Changelog
Additional Notes
^[a-zA-Z0-9_%]+$(MaxLength=256) rejects invalid Envoy stat names at admission time, before they can reach the data plane.%RULE_NAME%is intentionally left as a literal string when the HTTPRoute rule has no name — empty-string substitution would silently corrupt metric names in production. A controller warning is logged to surface this to operators.