I think we probably need to match the second section against the service namespace as well. In fact, we can construct the entire authority exactly except for the port. So I think the regex can just be: authority=~"%s.%s.svc.%s:\d+" or something like that

In an authority authors.default.svc.cluster.local:7001 we have authors from the apex service name and default from namespace. svc is a given. will an authority always have cluster and local as part of the address?

cluster.local is harded as the cluster domain in a number of places but there is currently ongoing work to replace that hardcoded value with the cluster domain from the Linkerd configmap.

Thanks for the context. Given this ongoing work, should I avoid hardcoding cluster.local into the authority address, and if so, what's the best way to extract the cluster domain so I can ensure I'm checking for the right string?

I'm now matching to authority=~"^authors.default.svc.+" using the apex service and the service namespace.

if you look at controller/cmd/destination/main.go you can see how the cluster domain is loaded from the global config at startup, and then passed into destination.newServer. We could do the same thing in the public-api main and pass the cluster domain into NewServer.

Great tip, thanks. To keep this PR trafficsplit-specific, I'd prefer to do that in a separate PR after this is merged, and leave the current regex matching as is. WDYT?

I'm a bit confused on what this is doing.

I refactored this, let me know if it's more clear. buildTrafficSplitRequestLabels handles four different query scenarios, and needs to output different labels for each:

stat ts -A (namespace is not specified)
stat ts -n foo
stat ts --from resource/name
stat ts --to resource/name

Ok this is super subtle, but I don't think we want this API to return rows for traffic splits that don't have any corresponding metrics in prometheus, when a --to or --from flag is specified.

By way of example, in my default namespace, I'm running the books app, with an additional books service (books-v2). When I query for authorities, I see:

$ bin/linkerd stat au
NAME                                      MESHED   SUCCESS      RPS   LATENCY_P50   LATENCY_P95   LATENCY_P99   TCP_CONN
                                               -   100.00%   0.5rps           3ms           9ms          10ms          -
authors.default.svc.cluster.local:7001         -    77.09%   6.0rps           7ms          35ms          70ms          -
books-v2.default.svc.cluster.local:7002        -    75.00%   1.2rps          13ms         125ms         185ms          -
books.default.svc.cluster.local:7002           -    81.70%   6.5rps           9ms          86ms          97ms          -
webapp.default.svc.cluster.local:7000          -    78.70%   6.4rps          32ms          95ms         171ms          -

If I then add a --from flag, that filters the rows returned to only those that are called by the from resource. For instance:

$ bin/linkerd stat au --from deploy/webapp
NAME                                     MESHED   SUCCESS      RPS   LATENCY_P50   LATENCY_P95   LATENCY_P99   TCP_CONN
authors.default.svc.cluster.local:7001        -   100.00%   3.1rps          12ms          47ms          86ms          -
books.default.svc.cluster.local:7002          -    77.96%   7.0rps          12ms          83ms          97ms          -

Or:

$ bin/linkerd stat au --from deploy/books
NAME                                     MESHED   SUCCESS      RPS   LATENCY_P50   LATENCY_P95   LATENCY_P99   TCP_CONN
authors.default.svc.cluster.local:7001        -    46.60%   3.2rps           5ms          11ms          18ms          -

But that's not the behavior that we're seeing with traffic splits. If I query for all traffic splits in my default namespace, I see:

$ bin/linkerd stat ts
NAME          APEX    LEAF       WEIGHT   SUCCESS      RPS   LATENCY_P50   LATENCY_P95   LATENCY_P99
books-split   books   books        800m    79.63%   6.4rps          12ms          75ms         120ms
books-split   books   books-v2     200m    77.00%   1.7rps          20ms          33ms          39ms

If I add a --to flag, however, to filter to only those traffic splits that are hitting svc/books-v2, I see:

$ bin/linkerd stat ts --to svc/books-v2
NAME          APEX    LEAF       WEIGHT   SUCCESS      RPS   LATENCY_P50   LATENCY_P95   LATENCY_P99
books-split   books   books        800m         -        -             -             -             -
books-split   books   books-v2     200m    77.11%   1.4rps          10ms          62ms          92ms

Notice how the books row still shows up, even though it doesn't have any stats. For parity with how stat works for other resources, I think we should drop that row.

And the good news is that there's a very straightforward fix for this, by changing this func as follows:

diff --git a/controller/api/public/stat_summary.go b/controller/api/public/stat_summary.go
index 3763c2b9..5f0e900d 100644
--- a/controller/api/public/stat_summary.go
+++ b/controller/api/public/stat_summary.go
@@ -310,6 +310,13 @@ func (s *grpcServer) trafficSplitResourceQuery(ctx context.Context, req *pb.Stat
 				Apex:      tsStats.apex,
 				Leaf:      name,
 			}
+			stats, ok := tsBasicStats[currentLeaf]
+			if !ok && !req.SkipStats {
+				switch req.Outbound.(type) {
+				case *pb.StatSummaryRequest_ToResource, *pb.StatSummaryRequest_FromResource:
+					continue
+				}
+			}
 
 			trafficSplitStats := &pb.TrafficSplitStats{
 				Apex:   tsStats.apex,
@@ -324,7 +331,7 @@ func (s *grpcServer) trafficSplitResourceQuery(ctx context.Context, req *pb.Stat
 					Type:      req.GetSelector().GetResource().GetType(),
 				},
 				TimeWindow: req.TimeWindow,
-				Stats:      tsBasicStats[currentLeaf],
+				Stats:      stats,
 				TsStats:    trafficSplitStats,
 			}
 			rows = append(rows, &row)

Thanks for testing this out and thinking about this. You make a good point that there's not exact parity here between ts and other resources.

My preference is to continue showing a row for each leaf within a trafficsplit even if there are no metrics returned, because in the case of traffic splitting, the absence of traffic is important information to validate that the split is working.

Also, if you are running watch linkerd stat ts as you apply weight changes within a split, showing a row for each leaf maintains a consistent display even if a leaf service you are phasing out goes down to 0 (or up from 0), which is more readable than a row popping in or out of view. WDYT?

You're right that we'll need to be specific in the descriptions about what --from and --to means in the case of linkerd stat ts. This is a very wordy attempt that needs to be more concise:

linkerd stat ts —from deploy/authors
Show all trafficsplit leaf services in the default namespace, and metrics for any traffic coming to those services from the authors deployment.

Personally, I also think it's important to show the traffic split branches with no metrics even though this is inconsistent with how other resources behave.

Ok, this all sounds good to me. I just wanted to clarify that if a traffic split has ever received any traffic that matches the query pattern, then it will still be displayed, even if it currently is not receiving any traffic. My proposed change would only exclude rows that have never seen a single request matching the query pattern.

But I agree that the nature of trafficsplits is different enough from other resources to warrant showing all branches in the stat output.