Review feedback

probe/awsecs/client.go

@@ -33,7 +33,7 @@ func newClient(cluster string) (*ecsClient, error) {
 // returns a map from deployment ids to service names
 // cannot fail as it will attempt to deliver partial results, though that may end up being no results
 func (c ecsClient) getDeploymentMap() map[string]string {
-	results := make(map[string]string)
+	results := map[string]string{}
 	lock := sync.Mutex{} // lock mediates access to results
 
 	group := sync.WaitGroup{}
@@ -43,21 +43,20 @@ func (c ecsClient) getDeploymentMap() map[string]string {
 		func(page *ecs.ListServicesOutput, lastPage bool) bool {
 			// describe each page of 10 (the max for one describe command) concurrently
 			group.Add(1)
+			serviceArns := page.ServiceArns
 			go func() {
 				defer group.Done()
 
 				resp, err := c.client.DescribeServices(&ecs.DescribeServicesInput{
 					Cluster:  &c.cluster,
-					Services: page.ServiceArns,
+					Services: serviceArns,
 				})
 				if err != nil {
-					// rather than trying to propagate errors up, just log a warning here
 					log.Warnf("Error describing some ECS services, ECS service report may be incomplete: %v", err)
 					return
 				}
 
 				for _, failure := range resp.Failures {
-					// log the failures but still continue with what succeeded
 					log.Warnf("Failed to describe ECS service %s, ECS service report may be incomplete: %s", failure.Arn, failure.Reason)
 				}
 
@@ -75,7 +74,6 @@ func (c ecsClient) getDeploymentMap() map[string]string {
 	group.Wait()
 
 	if err != nil {
-		// We want to still return partial results if we have any, so just log a warning
 		log.Warnf("Error listing ECS services, ECS service report may be incomplete: %v", err)
 	}
 	return results
@@ -99,11 +97,10 @@ func (c ecsClient) getTaskDeployments(taskArns []string) (map[string]string, err
 	}
 
 	for _, failure := range resp.Failures {
-		// log the failures but still continue with what succeeded
 		log.Warnf("Failed to describe ECS task %s, ECS service report may be incomplete: %s", failure.Arn, failure.Reason)
 	}
 
-	results := make(map[string]string)
+	results := make(map[string]string, len(resp.Tasks))
 	for _, task := range resp.Tasks {
 		results[*task.TaskArn] = *task.StartedBy
 	}
@@ -112,7 +109,7 @@ func (c ecsClient) getTaskDeployments(taskArns []string) (map[string]string, err
 
 // returns a map from task ARNs to service names
 func (c ecsClient) getTaskServices(taskArns []string) (map[string]string, error) {
-	deploymentMapChan := make(chan map[string]string)
+	deploymentMapChan := chan map[string]string{}
 	go func() {
 		deploymentMapChan <- c.getDeploymentMap()
 	}()
@@ -125,7 +122,7 @@ func (c ecsClient) getTaskServices(taskArns []string) (map[string]string, error)
 		return nil, err
 	}
 
-	results := make(map[string]string)
+	results := map[string]string{}
 	for taskArn, depID := range taskDeployments {
 		// Note not all tasks map to a deployment, or we could otherwise mismatch due to races.
 		// It's safe to just ignore all these cases and consider them "non-service" tasks.

probe/awsecs/reporter.go

@@ -18,29 +18,38 @@ type taskInfo struct {
 
 // return map from cluster to map of task arns to task infos
 func getLabelInfo(rpt report.Report) map[string]map[string]*taskInfo {
-	results := make(map[string]map[string]*taskInfo)
+	results := map[string]map[string]*taskInfo{}
 	log.Debug("scanning for ECS containers")
 	for nodeID, node := range rpt.Container.Nodes {
 
-		taskArn, taskArnOk := node.Latest.Lookup(docker.LabelPrefix + "com.amazonaws.ecs.task-arn")
-		cluster, clusterOk := node.Latest.Lookup(docker.LabelPrefix + "com.amazonaws.ecs.cluster")
-		family, familyOk := node.Latest.Lookup(docker.LabelPrefix + "com.amazonaws.ecs.task-definition-family")
-
-		if taskArnOk && clusterOk && familyOk {
-			taskMap, ok := results[cluster]
-			if !ok {
-				taskMap = make(map[string]*taskInfo)
-				results[cluster] = taskMap
-			}
-
-			task, ok := taskMap[taskArn]
-			if !ok {
-				task = &taskInfo{containerIDs: make([]string, 0), family: family}
-				taskMap[taskArn] = task
-			}
-
-			task.containerIDs = append(task.containerIDs, nodeID)
+		taskArn, ok := node.Latest.Lookup(docker.LabelPrefix + "com.amazonaws.ecs.task-arn")
+		if !ok {
+			continue
 		}
+
+		cluster, ok := node.Latest.Lookup(docker.LabelPrefix + "com.amazonaws.ecs.cluster")
+		if !ok {
+			continue
+		}
+
+		family, ok := node.Latest.Lookup(docker.LabelPrefix + "com.amazonaws.ecs.task-definition-family")
+		if !ok {
+			continue
+		}
+
+		taskMap, ok := results[cluster]
+		if !ok {
+			taskMap = map[string]*taskInfo{}
+			results[cluster] = taskMap
+		}
+
+		task, ok := taskMap[taskArn]
+		if !ok {
+			task = &taskInfo{containerIDs: []string{}, family: family}
+			taskMap[taskArn] = task
+		}
+
+		task.containerIDs = append(task.containerIDs, nodeID)
 	}
 	log.Debug("Got ECS container info: %v", results)
 	return results
@@ -51,7 +60,7 @@ type Reporter struct {
 }
 
 // Tag needed for Tagger
-func (r Reporter) Tag(rpt report.Report) (report.Report, error) {
+func (Reporter) Tag(rpt report.Report) (report.Report, error) {
 	rpt = rpt.Copy()
 
 	clusterMap := getLabelInfo(rpt)
@@ -75,7 +84,7 @@ func (r Reporter) Tag(rpt report.Report) (report.Report, error) {
 		}
 
 		// Create all the services first
-		unique := make(map[string]bool)
+		unique := map[string]bool{}
 		for _, serviceName := range taskServices {
 			if !unique[serviceName] {
 				serviceID := report.MakeECSServiceNodeID(serviceName)
@@ -99,7 +108,6 @@ func (r Reporter) Tag(rpt report.Report) (report.Report, error) {
 				serviceID := report.MakeECSServiceNodeID(serviceName)
 				parentsSets = parentsSets.Add(report.ECSService, report.MakeStringSet(serviceID))
 			}
-
 			for _, containerID := range info.containerIDs {
 				if containerNode, ok := rpt.Container.Nodes[containerID]; ok {
 					rpt.Container.Nodes[containerID] = containerNode.WithParents(parentsSets)
@@ -107,7 +115,6 @@ func (r Reporter) Tag(rpt report.Report) (report.Report, error) {
 					log.Warnf("Got task info for non-existent container %v, this shouldn't be able to happen", containerID)
 				}
 			}
-
 		}
 
 	}

prog/main.go

@@ -285,7 +285,7 @@ func main() {
 	flag.StringVar(&flags.probe.kubernetesConfig.Username, "probe.kubernetes.username", "", "Username for basic authentication to the API server")
 
 	// AWS ECS
-	flag.BoolVar(&flags.probe.ecsEnabled, "probe.ecs", false, "collect ecs-related attributes for containers on this node")
+	flag.BoolVar(&flags.probe.ecsEnabled, "probe.ecs", false, "Collect ecs-related attributes for containers on this node")
 
 	// Weave
 	flag.StringVar(&flags.probe.weaveAddr, "probe.weave.addr", "127.0.0.1:6784", "IP address & port of the Weave router")