Some fixes to the spoof.go and exporter.go (#223)

* Some fixes to the spoof.go and exporter.go

While reviewing some other CL, I saw some avenues for improving
spoof.go, to log the URL that's being fetched, which would help in test
debugging and to use switch construct, rather than nested if's.

While testing the change, I noticed some shifty loggin from the
exporter, so I fixed that as well while I was there.

* Continuation of the previous cleanups.

metrics/exporter.go

@@ -121,7 +121,7 @@ func newStackdriverExporter(config *metricsConfig, logger *zap.SugaredLogger) (v
 		DefaultMonitoringLabels: &stackdriver.Labels{},
 	})
 	if err != nil {
-		logger.Error("Failed to create the Stackdriver exporter.", zap.Error(err))
+		logger.Error("Failed to create the Stackdriver exporter: ", zap.Error(err))
 		return nil, err
 	}
 	logger.Infof("Created Opencensus Stackdriver exporter with config %v", config)
@@ -172,7 +172,7 @@ func setMonitoredResourceFunc(config *metricsConfig, logger *zap.SugaredLogger)
 	if getMonitoredResourceFunc == nil {
 		gm := retrieveGCPMetadata()
 		metricsPrefix := config.domain + "/" + config.component
-		logger.Infof("metrics prefix", metricsPrefix)
+		logger.Infof("metrics prefix: %s", metricsPrefix)
 		if _, ok := metricskey.KnativeRevisionMetricsPrefixes[metricsPrefix]; ok {
 			getMonitoredResourceFunc = getKnativeRevisionMonitoredResource(gm)
 		} else {

test/cleanup.go

@@ -32,7 +32,7 @@ func CleanupOnInterrupt(cleanup func(), logger *logging.BaseLogger) {
 	signal.Notify(c, os.Interrupt)
 	go func() {
 		for range c {
-			logger.Infof("Test interrupted, cleaning up.")
+			logger.Info("Test interrupted, cleaning up.")
 			cleanup()
 			os.Exit(1)
 		}

test/spoof/spoof.go

@@ -78,7 +78,7 @@ var _ Interface = (*SpoofingClient)(nil)
 // https://github.com/kubernetes/apimachinery/blob/cf7ae2f57dabc02a3d215f15ca61ae1446f3be8f/pkg/util/wait/wait.go#L172
 type ResponseChecker func(resp *Response) (done bool, err error)
 
-// SpoofingClient is a minimal http client wrapper that spoofs the domain of requests
+// SpoofingClient is a minimal HTTP client wrapper that spoofs the domain of requests
 // for non-resolvable domains.
 type SpoofingClient struct {
 	Client          *http.Client
@@ -123,7 +123,7 @@ func New(kubeClientset *kubernetes.Clientset, logger *logging.BaseLogger, domain
 	return &sc, nil
 }
 
-// GetServiceEndpoint gets the endpoint IP or hostname to use for the service
+// GetServiceEndpoint gets the endpoint IP or hostname to use for the service.
 func GetServiceEndpoint(kubeClientset *kubernetes.Clientset) (*string, error) {
 	var err error
 
@@ -135,7 +135,7 @@ func GetServiceEndpoint(kubeClientset *kubernetes.Clientset) (*string, error) {
 		}
 
 		var endpoint string
-		endpoint, err = getEndpointFromService(ingress)
+		endpoint, err = endpointFromService(ingress)
 		if err != nil {
 			continue
 		}
@@ -145,21 +145,22 @@ func GetServiceEndpoint(kubeClientset *kubernetes.Clientset) (*string, error) {
 	return nil, err
 }
 
-// getEndpointFromService extracts the endpoint from the service's ingress.
+// endpointFromService extracts the endpoint from the service's ingress.
-func getEndpointFromService(svc *v1.Service) (string, error) {
+func endpointFromService(svc *v1.Service) (string, error) {
 	ingresses := svc.Status.LoadBalancer.Ingress
 	if len(ingresses) != 1 {
-		return "", fmt.Errorf("Expected exactly one ingress load balancer, instead had %d: %s", len(ingresses), ingresses)
+		return "", fmt.Errorf("Expected exactly one ingress load balancer, instead had %d: %v", len(ingresses), ingresses)
 	}
-	ingressToUse := ingresses[0]
+	itu := ingresses[0]
 
-	if ingressToUse.IP == "" {
+	switch {
-		if ingressToUse.Hostname == "" {
+	case itu.IP != "":
+		return itu.IP, nil
+	case itu.Hostname != "":
+		return itu.Hostname, nil
+	default:
 		return "", fmt.Errorf("Expected ingress loadbalancer IP or hostname for %s to be set, instead was empty", svc.Name)
 	}
-		return ingressToUse.Hostname, nil
-	}
-	return ingressToUse.IP, nil
 }
 
 // Do dispatches to the underlying http.Client.Do, spoofing domains as needed
@@ -212,7 +213,7 @@ func (sc *SpoofingClient) Poll(req *http.Request, inState ResponseChecker) (*Res
 		resp, err = sc.Do(req)
 		if err != nil {
 			if err, ok := err.(net.Error); ok && err.Timeout() {
-				sc.logger.Infof("Retrying for TCP timeout %v", err)
+				sc.logger.Infof("Retrying %s for TCP timeout %v", req.URL.String(), err)
 				return false, nil
 			}
 			return true, err