Add traffic_rule label to router metrics

[krithika369]

This PR introduces a new label traffic_rule to the following Prometheus metrics produced by the Turing router:

• RouteRequestDurationMs
• TuringComponentRequestDurationMs, where the component is route

Eg:

mlp_turing_route_request_duration_ms_bucket{route="control",status="failure",traffic_rule="traffic-split-default",le="2"} 0
mlp_turing_route_request_duration_ms_count{route="control",status="success",traffic_rule="traffic-split-b"} 1

mlp_turing_turing_comp_request_duration_ms_bucket{component="route",status="success",traffic_rule="traffic-split-default",le="2"} 0
mlp_turing_route_request_duration_ms_count{route="route-b",status="success",traffic_rule="traffic-split-b"} 1

Changes

• engines/router/missionctl/fiberapi/traffic_splitting_strategy.go - Set traffic-rule label in SelectRoute, to hold the matched traffic rule. Further, this function is refactored to only pick the first traffic rule matched as realistically, we may not have enough time to (sequentially) execute multiple matches. This also simplifies the matched rule metrics logging (otherwise, when there are multiple matched rules, we need more intricate wiring within Fiber to know which rule was ultimately executed).
• engines/router/missionctl/fiberapi/interceptors.go - Associate traffic_rule label to the RouteRequestDurationMs metric
• engines/router/missionctl/mission_control.go, engines/router/missionctl/mission_control_upi.go - Associate traffic_rule label to the TuringComponentRequestDurationMs metric for the route step.

Known issues

There is a limitation as of the current implementation in this PR (and the relevant Fiber PR): Issue #281

This will be addressed separately, with other improvements to timeout scenarios.

TODO

• Update the dependency once the related PR is merged: Add Response Labels gojek/fiber#9

[leonlnj]

I might have some misconception here,

I would had thought because of the orthogonality checks on the rules, there should always be at most one rule that can be matched given a request, and that should be the reason of the removal (and probably previously), here instead of

we may not have enough time to (sequentially) execute multiple matches

didnt catch the timing part since to me there should only be 1 rule matched for any request.

[krithika369]

Ahh you're right about your assumption but it's missing one detail - the idea of overlapping rules at different priorities (kind of similar to XP experiments). This is the relevant validation.

So, if there were 2 rules:

• {"a": [1]}
• {"a": [1], "b": [2]}
  A request that matches # 2, will also match # 1. In this case, we will just go with the order in which the rules are defined, to pick the appropriate rule (per this example, # 2 will never be applied because it comes after the broader rule).

[leonlnj]

Left a comment, the rest LGTM!

[krithika369]

Thanks for the review, @leonlnj. @caraml-dev/turing-dev, if there are no other comments, could one of you hit the "approve" button to satisfy the CI ? :)

[deadlycoconuts]

LGTM! Thanks for this! 🚀

[terryyylim]

Left a clarifying question, the rest LGTM, thanks @krithika369 !

[terryyylim]

Originally when we return orderedRoutes[1:], that value was never used right?

[krithika369]

Yes that's correct.

[krithika369]

Thanks for the reviews, all! Merging.