Skip to content

Commit

Permalink
Remove pod-level metrics from web and CLI (#304)
Browse files Browse the repository at this point in the history
This PR updates the web UI to remove the pod detail page, and to remove the links to that page from pod names in metrics tables. It also removes the `pods` option from `conduit stat`, and the `sourcePod` and `targetPod` fields from the controller API proto's `MetricMetadata` message.

I've updated the `conduit stat` tests to reflect these changes, and manually verified the web UI changes.

Closes #261 

Signed-off-by: Eliza Weisman <[email protected]>
  • Loading branch information
hawkw authored Feb 9, 2018
1 parent 81d4b7b commit 2015d99
Show file tree
Hide file tree
Showing 14 changed files with 141 additions and 419 deletions.
22 changes: 10 additions & 12 deletions cli/cmd/stat.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,10 @@ var statCmd = &cobra.Command{
Long: `Display runtime statistics about mesh resources.
Valid resource types include:
* pods (aka pod, po)
* deployments (aka deployment, deploy)
* paths (aka path, pa)
The optional [TARGET] option can be either a name for a deployment or pod resource`,
The optional [TARGET] option can be a name for a deployment.`,
RunE: func(cmd *cobra.Command, args []string) error {
var friendlyNameForResourceType string

Expand All @@ -41,7 +40,7 @@ The optional [TARGET] option can be either a name for a deployment or pod resour
friendlyNameForResourceType = args[0]
target = args[1]
default:
return errors.New("please specify a resource type: pods, deployments or paths")
return errors.New("please specify a resource type: deployments or paths")
}

validatedResourceType, err := k8s.CanonicalKubernetesNameFromFriendlyName(friendlyNameForResourceType)
Expand All @@ -50,7 +49,13 @@ The optional [TARGET] option can be either a name for a deployment or pod resour
case "paths", "path", "pa":
validatedResourceType = ConduitPaths
default:
return fmt.Errorf("invalid resource type %s, only %v are allowed as resource types", friendlyNameForResourceType, []string{k8s.KubernetesPods, k8s.KubernetesDeployments, ConduitPaths})
return fmt.Errorf("invalid resource type %s, only %v are allowed as resource types", friendlyNameForResourceType, []string{k8s.KubernetesDeployments, ConduitPaths})
}
} else {
switch friendlyNameForResourceType {
case "pods", "pod", "po":
return fmt.Errorf("invalid resource type %s, only %v are allowed as resource types", friendlyNameForResourceType, []string{k8s.KubernetesDeployments, ConduitPaths})
default:
}
}
client, err := newPublicAPIClient()
Expand All @@ -76,7 +81,6 @@ func init() {
}

var resourceTypeToAggregationType = map[string]pb.AggregationType{
k8s.KubernetesPods: pb.AggregationType_TARGET_POD,
k8s.KubernetesDeployments: pb.AggregationType_TARGET_DEPLOY,
ConduitPaths: pb.AggregationType_PATH,
}
Expand Down Expand Up @@ -130,9 +134,7 @@ func writeStatsToBuffer(resp *pb.MetricResponse, w *tabwriter.Writer) {

metadata := *metric.Metadata
var name string
if metadata.TargetPod != "" {
name = metadata.TargetPod
} else if metadata.TargetDeploy != "" {
if metadata.TargetDeploy != "" {
name = metadata.TargetDeploy
} else if metadata.Path != "" {
name = metadata.Path
Expand Down Expand Up @@ -191,10 +193,6 @@ func buildMetricRequest(aggregationType pb.AggregationType) (*pb.MetricRequest,
if err != nil {
return nil, err
}

if target != "all" && aggregationType == pb.AggregationType_TARGET_POD {
filterBy.TargetPod = target
}
if target != "all" && aggregationType == pb.AggregationType_TARGET_DEPLOY {
filterBy.TargetDeploy = target
}
Expand Down
22 changes: 11 additions & 11 deletions cli/cmd/stat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func TestRequestStatsFromApi(t *testing.T) {
t.Run("Returns string output containing the data returned by the API", func(t *testing.T) {
mockClient := &public.MockConduitApiClient{}

podName := "pod-1"
deployName := "deployment-1"
metricDatapoints := []*pb.MetricDatapoint{
{
Value: &pb.MetricValue{
Expand All @@ -32,7 +32,7 @@ func TestRequestStatsFromApi(t *testing.T) {
{
Name: pb.MetricName_SUCCESS_RATE,
Metadata: &pb.MetricMetadata{
TargetPod: podName,
TargetDeploy: deployName,
},
Datapoints: metricDatapoints,
},
Expand All @@ -41,13 +41,13 @@ func TestRequestStatsFromApi(t *testing.T) {
Metrics: series,
}

stats, err := requestStatsFromApi(mockClient, k8s.KubernetesPods)
stats, err := requestStatsFromApi(mockClient, k8s.KubernetesDeployments)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

if !strings.Contains(stats, podName) {
t.Fatalf("Expected response to contain [%s], but was [%s]", podName, stats)
if !strings.Contains(stats, deployName) {
t.Fatalf("Expected response to contain [%s], but was [%s]", deployName, stats)
}
})

Expand Down Expand Up @@ -98,8 +98,8 @@ func TestRenderStats(t *testing.T) {
t.Run("Prints stats correctly for busy example", func(t *testing.T) {
allSeries := make([]*pb.MetricSeries, 0)
for i := 0; i < 10; i++ {
seriesForPodX := generateMetricSeriesFor(fmt.Sprintf("pod-%d", i), int64(i))
allSeries = append(allSeries, seriesForPodX...)
seriesForDeployX := generateMetricSeriesFor(fmt.Sprintf("deployment-%d", i), int64(i))
allSeries = append(allSeries, seriesForDeployX...)
}

//shuffles
Expand Down Expand Up @@ -173,7 +173,7 @@ func TestSortStatsKeys(t *testing.T) {
})
}

func generateMetricSeriesFor(podName string, seed int64) []*pb.MetricSeries {
func generateMetricSeriesFor(deployName string, seed int64) []*pb.MetricSeries {
metricDatapoints := []*pb.MetricDatapoint{
{
Value: &pb.MetricValue{
Expand Down Expand Up @@ -211,21 +211,21 @@ func generateMetricSeriesFor(podName string, seed int64) []*pb.MetricSeries {
{
Name: pb.MetricName_REQUEST_RATE,
Metadata: &pb.MetricMetadata{
TargetPod: podName,
TargetDeploy: deployName,
},
Datapoints: metricDatapoints,
},
{
Name: pb.MetricName_SUCCESS_RATE,
Metadata: &pb.MetricMetadata{
TargetPod: podName,
TargetDeploy: deployName,
},
Datapoints: metricDatapoints,
},
{
Name: pb.MetricName_LATENCY,
Metadata: &pb.MetricMetadata{
TargetPod: podName,
TargetDeploy: deployName,
},
Datapoints: latencyHistogram,
},
Expand Down
22 changes: 11 additions & 11 deletions cli/cmd/testdata/stat_busy_output.golden
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
NAME REQUEST_RATE SUCCESS_RATE P50_LATENCY P99_LATENCY
pod-0 0.0rps 0.00% 1ms 9ms
pod-1 0.1rps 10.00% 2ms 10ms
pod-2 0.2rps 20.00% 3ms 11ms
pod-3 0.3rps 30.00% 4ms 12ms
pod-4 0.4rps 40.00% 5ms 13ms
pod-5 0.5rps 50.00% 6ms 14ms
pod-6 0.6rps 60.00% 7ms 15ms
pod-7 0.7rps 70.00% 8ms 16ms
pod-8 0.8rps 80.00% 9ms 17ms
pod-9 0.9rps 90.00% 10ms 18ms
NAME REQUEST_RATE SUCCESS_RATE P50_LATENCY P99_LATENCY
deployment-0 0.0rps 0.00% 1ms 9ms
deployment-1 0.1rps 10.00% 2ms 10ms
deployment-2 0.2rps 20.00% 3ms 11ms
deployment-3 0.3rps 30.00% 4ms 12ms
deployment-4 0.4rps 40.00% 5ms 13ms
deployment-5 0.5rps 50.00% 6ms 14ms
deployment-6 0.6rps 60.00% 7ms 15ms
deployment-7 0.7rps 70.00% 8ms 16ms
deployment-8 0.8rps 80.00% 9ms 17ms
deployment-9 0.9rps 90.00% 10ms 18ms
10 changes: 0 additions & 10 deletions controller/api/public/grpc_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,18 +378,10 @@ func formatQuery(query string, req *pb.MetricRequest, sumBy string) (string, err
}

if metadata := req.FilterBy; metadata != nil {
if metadata.TargetPod != "" {
filterLabels = append(filterLabels, fmt.Sprintf("%s=\"%s\"", targetPodLabel, metadata.TargetPod))
sumLabels = append(sumLabels, targetPodLabel)
}
if metadata.TargetDeploy != "" {
filterLabels = append(filterLabels, fmt.Sprintf("%s=\"%s\"", targetDeployLabel, metadata.TargetDeploy))
sumLabels = append(sumLabels, targetDeployLabel)
}
if metadata.SourcePod != "" {
filterLabels = append(filterLabels, fmt.Sprintf("%s=\"%s\"", sourcePodLabel, metadata.SourcePod))
sumLabels = append(sumLabels, sourcePodLabel)
}
if metadata.SourceDeploy != "" {
filterLabels = append(filterLabels, fmt.Sprintf("%s=\"%s\"", sourceDeployLabel, metadata.SourceDeploy))
sumLabels = append(sumLabels, sourceDeployLabel)
Expand Down Expand Up @@ -459,9 +451,7 @@ func filterQueryRsp(rsp *telemPb.QueryResponse, end int64) {

func extractMetadata(metric *telemPb.Sample) pb.MetricMetadata {
return pb.MetricMetadata{
TargetPod: metric.Labels[targetPodLabel],
TargetDeploy: metric.Labels[targetDeployLabel],
SourcePod: metric.Labels[sourcePodLabel],
SourceDeploy: metric.Labels[sourceDeployLabel],
Path: metric.Labels[pathLabel],
}
Expand Down
4 changes: 3 additions & 1 deletion controller/api/public/http_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ type handler struct {
}

func (h *handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
log.Debugf("Serving %s %s", req.Method, req.URL.Path)
log.WithFields(log.Fields{
"req.Method": req.Method, "req.URL": req.URL, "req.Form": req.Form,
}).Debugf("Serving %s %s", req.Method, req.URL.Path)
// Validate request method
if req.Method != http.MethodPost {
writeErrorToHttpResponse(w, fmt.Errorf("POST required"))
Expand Down
Loading

0 comments on commit 2015d99

Please sign in to comment.