Probably not needed, unless you want to add an else to the --output json below. The JSON output already contains the extra effective_* and actual_* fields.

Ah nice! thanks for pointing that out, I had no idea.

I think checking latency is a bit unreliable. If the intent here is to check that retries aren't happening, can we just check if effective_success and actual_success is zero (since --failure-rate=1.0)?

Good point. Testing for latency is pretty finicky, however, the behaviour we are testing is timeouts. The intent for this test case was to ensure that we aren't already hitting the retry timeout that we will eventually test for. My thinking here was, if we are already seeing timeouts surpassing the threshold even before applying the service profile with the intended timeout, we can't reliably prove that the service profile is doing its job after it's applied.

Another interesting thing here is with --failure-rate=1.0, we will still see effective_success and actual_success be zero even after we've applied the service profile and the service is retrying requests. Similar to your suggestion, I think comparing actual_rps and effective_rps would suffice. 🤔

Similar to above, checking latency can be unreliable. If the intent here is to check retries happened, but requests continue to fail because of --failure-rate=1.0, we can try to see if this works:

• Configure slow cooker to output a fixed number of requests via the -totalRequests flag
• Make sure effective_success and actual_success are zero
• Use linkerd top to check the count column for retried requests

From my understanding of the initial ticket, I thought we were trying to specifically test the timeouts behavior. Maybe, since its unreliable we should just test that retries are being performed?

Maybe, since its unreliable we should just test that retries are being performed?

SGTM, unless others have a way to do this. It isn't easy to reliably test timeouts without context. So far, I can't find anything in the service profile and route code that will be useful here, based on the little that I know.

TIOLI, you could make jsonRouteStats (from the cmd package) public and re-use that.

curious: was this change necessary to match against returned data from a routes command?

Yea, I had to use the full deployment name in order to get stuff back. I can make a change to the routes command to accept both deploy and deployment

ws?

🆒

tioli: since routeStatAssertion is only used in one place, i'd probably just pass upstream, downstream, and namespace directly into assertRouteStat