From 4a51f8a18661da14e6e4098b3c5972468dd311b6 Mon Sep 17 00:00:00 2001 From: Nick Sardo Date: Thu, 4 May 2017 18:04:34 -0700 Subject: [PATCH 1/4] Add metric capture on GETs --- .../providers/gce/gce_backendservice.go | 4 +- pkg/cloudprovider/providers/gce/gce_cert.go | 8 ++- .../providers/gce/gce_clusters.go | 20 +++++-- pkg/cloudprovider/providers/gce/gce_disks.go | 7 ++- .../providers/gce/gce_firewall.go | 11 +++- .../providers/gce/gce_forwardingrule.go | 11 ++-- .../providers/gce/gce_healthchecks.go | 57 +++++++++---------- .../providers/gce/gce_instancegroup.go | 21 +++++-- .../providers/gce/gce_instances.go | 11 ++-- pkg/cloudprovider/providers/gce/gce_routes.go | 4 +- .../providers/gce/gce_staticip.go | 6 +- .../providers/gce/gce_targetproxy.go | 19 +++++-- pkg/cloudprovider/providers/gce/gce_urlmap.go | 8 ++- 13 files changed, 118 insertions(+), 69 deletions(-) diff --git a/pkg/cloudprovider/providers/gce/gce_backendservice.go b/pkg/cloudprovider/providers/gce/gce_backendservice.go index d1446d4c4a1..02b20e8ede9 100644 --- a/pkg/cloudprovider/providers/gce/gce_backendservice.go +++ b/pkg/cloudprovider/providers/gce/gce_backendservice.go @@ -32,7 +32,9 @@ func newBackendServiceMetricContext(request string) *metricContext { // GetBackendService retrieves a backend by name. func (gce *GCECloud) GetBackendService(name string) (*compute.BackendService, error) { - return gce.service.BackendServices.Get(gce.projectID, name).Do() + mc := newBackendServiceMetricContext("get") + v, err := gce.service.BackendServices.Get(gce.projectID, name).Do() + return v, mc.Observe(err) } // UpdateBackendService applies the given BackendService as an update to an existing service. diff --git a/pkg/cloudprovider/providers/gce/gce_cert.go b/pkg/cloudprovider/providers/gce/gce_cert.go index dba3667c88c..77af58bbe3c 100644 --- a/pkg/cloudprovider/providers/gce/gce_cert.go +++ b/pkg/cloudprovider/providers/gce/gce_cert.go @@ -32,7 +32,9 @@ func newCertMetricContext(request string) *metricContext { // GetSslCertificate returns the SslCertificate by name. func (gce *GCECloud) GetSslCertificate(name string) (*compute.SslCertificate, error) { - return gce.service.SslCertificates.Get(gce.projectID, name).Do() + mc := newCertMetricContext("get") + v, err := gce.service.SslCertificates.Get(gce.projectID, name).Do() + return v, mc.Observe(err) } // CreateSslCertificate creates and returns a SslCertificate. @@ -69,6 +71,8 @@ func (gce *GCECloud) DeleteSslCertificate(name string) error { // ListSslCertificates lists all SslCertificates in the project. func (gce *GCECloud) ListSslCertificates() (*compute.SslCertificateList, error) { + mc := newCertMetricContext("list") // TODO: use PageToken to list all not just the first 500 - return gce.service.SslCertificates.List(gce.projectID).Do() + v, err := gce.service.SslCertificates.List(gce.projectID).Do() + return v, mc.Observe(err) } diff --git a/pkg/cloudprovider/providers/gce/gce_clusters.go b/pkg/cloudprovider/providers/gce/gce_clusters.go index ad55c08e16d..19062a93e14 100644 --- a/pkg/cloudprovider/providers/gce/gce_clusters.go +++ b/pkg/cloudprovider/providers/gce/gce_clusters.go @@ -16,19 +16,29 @@ limitations under the License. package gce +import "time" + +func newClustersMetricContext(request, zone string) *metricContext { + return &metricContext{ + start: time.Now(), + attributes: []string{"clusters" + request, unusedMetricLabel, zone}, + } +} + func (gce *GCECloud) ListClusters() ([]string, error) { + mc := newClustersMetricContext("list", "") allClusters := []string{} for _, zone := range gce.managedZones { clusters, err := gce.listClustersInZone(zone) if err != nil { - return nil, err + return nil, mc.Observe(err) } // TODO: Scoping? Do we need to qualify the cluster name? allClusters = append(allClusters, clusters...) } - return allClusters, nil + return allClusters, mc.Observe(nil) } func (gce *GCECloud) Master(clusterName string) (string, error) { @@ -36,14 +46,16 @@ func (gce *GCECloud) Master(clusterName string) (string, error) { } func (gce *GCECloud) listClustersInZone(zone string) ([]string, error) { + mc := newClustersMetricContext("list_zone", zone) // TODO: use PageToken to list all not just the first 500 list, err := gce.containerService.Projects.Zones.Clusters.List(gce.projectID, zone).Do() if err != nil { - return nil, err + return nil, mc.Observe(err) } + result := []string{} for _, cluster := range list.Clusters { result = append(result, cluster.Name) } - return result, nil + return result, mc.Observe(nil) } diff --git a/pkg/cloudprovider/providers/gce/gce_disks.go b/pkg/cloudprovider/providers/gce/gce_disks.go index e574dffbfed..4e62eaae95a 100644 --- a/pkg/cloudprovider/providers/gce/gce_disks.go +++ b/pkg/cloudprovider/providers/gce/gce_disks.go @@ -319,6 +319,7 @@ func (gce *GCECloud) GetAutoLabelsForPD(name string, zone string) (map[string]st // Returns a GCEDisk for the disk, if it is found in the specified zone. // If not found, returns (nil, nil) func (gce *GCECloud) findDiskByName(diskName string, zone string) (*GCEDisk, error) { + mc := newDiskMetricContext("get", zone) disk, err := gce.service.Disks.Get(gce.projectID, zone, diskName).Do() if err == nil { d := &GCEDisk{ @@ -327,12 +328,12 @@ func (gce *GCECloud) findDiskByName(diskName string, zone string) (*GCEDisk, err Kind: disk.Kind, Type: disk.Type, } - return d, nil + return d, mc.Observe(err) } if !isHTTPErrorCode(err, http.StatusNotFound) { - return nil, err + return nil, mc.Observe(err) } - return nil, nil + return nil, mc.Observe(nil) } // Like findDiskByName, but returns an error if the disk is not found diff --git a/pkg/cloudprovider/providers/gce/gce_firewall.go b/pkg/cloudprovider/providers/gce/gce_firewall.go index 8e1a5f012bb..607e51140e3 100644 --- a/pkg/cloudprovider/providers/gce/gce_firewall.go +++ b/pkg/cloudprovider/providers/gce/gce_firewall.go @@ -34,7 +34,14 @@ func newFirewallMetricContext(request string, region string) *metricContext { // GetFirewall returns the Firewall by name. func (gce *GCECloud) GetFirewall(name string) (*compute.Firewall, error) { - return gce.service.Firewalls.Get(gce.projectID, name).Do() + region, err := GetGCERegion(gce.localZone) + if err != nil { + return nil, err + } + + mc := newFirewallMetricContext("get", region) + v, err := gce.service.Firewalls.Get(gce.projectID, name).Do() + return v, mc.Observe(err) } // CreateFirewall creates the given firewall rule. @@ -45,7 +52,6 @@ func (gce *GCECloud) CreateFirewall(name, desc string, sourceRanges netsets.IPNe } mc := newFirewallMetricContext("create", region) - // TODO: This completely breaks modularity in the cloudprovider but // the methods shared with the TCPLoadBalancer take v1.ServicePorts. svcPorts := []v1.ServicePort{} @@ -90,7 +96,6 @@ func (gce *GCECloud) UpdateFirewall(name, desc string, sourceRanges netsets.IPNe } mc := newFirewallMetricContext("update", region) - // TODO: This completely breaks modularity in the cloudprovider but // the methods shared with the TCPLoadBalancer take v1.ServicePorts. svcPorts := []v1.ServicePort{} diff --git a/pkg/cloudprovider/providers/gce/gce_forwardingrule.go b/pkg/cloudprovider/providers/gce/gce_forwardingrule.go index 4ab006020a4..0eedf0559cb 100644 --- a/pkg/cloudprovider/providers/gce/gce_forwardingrule.go +++ b/pkg/cloudprovider/providers/gce/gce_forwardingrule.go @@ -35,7 +35,6 @@ func newForwardingRuleMetricContext(request, region string) *metricContext { // targetProxyLink is the SelfLink of a TargetHttp(s)Proxy. func (gce *GCECloud) CreateGlobalForwardingRule(targetProxyLink, ip, name, portRange string) (*compute.ForwardingRule, error) { mc := newForwardingRuleMetricContext("create", "") - rule := &compute.ForwardingRule{ Name: name, IPAddress: ip, @@ -59,7 +58,6 @@ func (gce *GCECloud) CreateGlobalForwardingRule(targetProxyLink, ip, name, portR // targetProxyLink is the SelfLink of a TargetHttp(s)Proxy. func (gce *GCECloud) SetProxyForGlobalForwardingRule(fw *compute.ForwardingRule, targetProxyLink string) error { mc := newForwardingRuleMetricContext("set_proxy", "") - op, err := gce.service.GlobalForwardingRules.SetTarget( gce.projectID, fw.Name, &compute.TargetReference{Target: targetProxyLink}).Do() if err != nil { @@ -73,7 +71,6 @@ func (gce *GCECloud) SetProxyForGlobalForwardingRule(fw *compute.ForwardingRule, // DeleteGlobalForwardingRule deletes the GlobalForwardingRule by name. func (gce *GCECloud) DeleteGlobalForwardingRule(name string) error { mc := newForwardingRuleMetricContext("delete", "") - op, err := gce.service.GlobalForwardingRules.Delete(gce.projectID, name).Do() if err != nil { if isHTTPErrorCode(err, http.StatusNotFound) { @@ -90,11 +87,15 @@ func (gce *GCECloud) DeleteGlobalForwardingRule(name string) error { // GetGlobalForwardingRule returns the GlobalForwardingRule by name. func (gce *GCECloud) GetGlobalForwardingRule(name string) (*compute.ForwardingRule, error) { - return gce.service.GlobalForwardingRules.Get(gce.projectID, name).Do() + mc := newForwardingRuleMetricContext("get", "") + v, err := gce.service.GlobalForwardingRules.Get(gce.projectID, name).Do() + return v, mc.Observe(err) } // ListGlobalForwardingRules lists all GlobalForwardingRules in the project. func (gce *GCECloud) ListGlobalForwardingRules() (*compute.ForwardingRuleList, error) { + mc := newForwardingRuleMetricContext("list", "") // TODO: use PageToken to list all not just the first 500 - return gce.service.GlobalForwardingRules.List(gce.projectID).Do() + v, err := gce.service.GlobalForwardingRules.List(gce.projectID).Do() + return v, mc.Observe(err) } diff --git a/pkg/cloudprovider/providers/gce/gce_healthchecks.go b/pkg/cloudprovider/providers/gce/gce_healthchecks.go index b957af7e83a..bf168a029f5 100644 --- a/pkg/cloudprovider/providers/gce/gce_healthchecks.go +++ b/pkg/cloudprovider/providers/gce/gce_healthchecks.go @@ -31,17 +31,17 @@ func newHealthcheckMetricContext(request string) *metricContext { // GetHttpHealthCheck returns the given HttpHealthCheck by name. func (gce *GCECloud) GetHttpHealthCheck(name string) (*compute.HttpHealthCheck, error) { - return gce.service.HttpHealthChecks.Get(gce.projectID, name).Do() + mc := newHealthcheckMetricContext("get_legacy") + v, err := gce.service.HttpHealthChecks.Get(gce.projectID, name).Do() + return v, mc.Observe(err) } // UpdateHttpHealthCheck applies the given HttpHealthCheck as an update. func (gce *GCECloud) UpdateHttpHealthCheck(hc *compute.HttpHealthCheck) error { mc := newHealthcheckMetricContext("update_legacy") - op, err := gce.service.HttpHealthChecks.Update(gce.projectID, hc.Name, hc).Do() if err != nil { - mc.Observe(err) - return err + return mc.Observe(err) } return gce.waitForGlobalOp(op, mc) @@ -50,11 +50,9 @@ func (gce *GCECloud) UpdateHttpHealthCheck(hc *compute.HttpHealthCheck) error { // DeleteHttpHealthCheck deletes the given HttpHealthCheck by name. func (gce *GCECloud) DeleteHttpHealthCheck(name string) error { mc := newHealthcheckMetricContext("delete_legacy") - op, err := gce.service.HttpHealthChecks.Delete(gce.projectID, name).Do() if err != nil { - mc.Observe(err) - return err + return mc.Observe(err) } return gce.waitForGlobalOp(op, mc) @@ -63,11 +61,9 @@ func (gce *GCECloud) DeleteHttpHealthCheck(name string) error { // CreateHttpHealthCheck creates the given HttpHealthCheck. func (gce *GCECloud) CreateHttpHealthCheck(hc *compute.HttpHealthCheck) error { mc := newHealthcheckMetricContext("create_legacy") - op, err := gce.service.HttpHealthChecks.Insert(gce.projectID, hc).Do() if err != nil { - mc.Observe(err) - return err + return mc.Observe(err) } return gce.waitForGlobalOp(op, mc) @@ -75,15 +71,20 @@ func (gce *GCECloud) CreateHttpHealthCheck(hc *compute.HttpHealthCheck) error { // ListHttpHealthChecks lists all HttpHealthChecks in the project. func (gce *GCECloud) ListHttpHealthChecks() (*compute.HttpHealthCheckList, error) { + mc := newHealthcheckMetricContext("list_legacy") // TODO: use PageToken to list all not just the first 500 - return gce.service.HttpHealthChecks.List(gce.projectID).Do() + v, err := gce.service.HttpHealthChecks.List(gce.projectID).Do() + return v, mc.Observe(err) } // Legacy HTTPS Health Checks // GetHttpsHealthCheck returns the given HttpsHealthCheck by name. func (gce *GCECloud) GetHttpsHealthCheck(name string) (*compute.HttpsHealthCheck, error) { - return gce.service.HttpsHealthChecks.Get(gce.projectID, name).Do() + mc := newHealthcheckMetricContext("get_legacy") + v, err := gce.service.HttpsHealthChecks.Get(gce.projectID, name).Do() + mc.Observe(err) + return v, err } // UpdateHttpsHealthCheck applies the given HttpsHealthCheck as an update. @@ -103,8 +104,7 @@ func (gce *GCECloud) DeleteHttpsHealthCheck(name string) error { mc := newHealthcheckMetricContext("delete_legacy") op, err := gce.service.HttpsHealthChecks.Delete(gce.projectID, name).Do() if err != nil { - mc.Observe(err) - return err + return mc.Observe(err) } return gce.waitForGlobalOp(op, mc) @@ -115,8 +115,7 @@ func (gce *GCECloud) CreateHttpsHealthCheck(hc *compute.HttpsHealthCheck) error mc := newHealthcheckMetricContext("create_legacy") op, err := gce.service.HttpsHealthChecks.Insert(gce.projectID, hc).Do() if err != nil { - mc.Observe(err) - return err + return mc.Observe(err) } return gce.waitForGlobalOp(op, mc) @@ -124,26 +123,27 @@ func (gce *GCECloud) CreateHttpsHealthCheck(hc *compute.HttpsHealthCheck) error // ListHttpsHealthChecks lists all HttpsHealthChecks in the project. func (gce *GCECloud) ListHttpsHealthChecks() (*compute.HttpsHealthCheckList, error) { + mc := newHealthcheckMetricContext("list_legacy") // TODO: use PageToken to list all not just the first 500 - return gce.service.HttpsHealthChecks.List(gce.projectID).Do() + v, err := gce.service.HttpsHealthChecks.List(gce.projectID).Do() + return v, mc.Observe(err) } // Generic HealthCheck // GetHealthCheck returns the given HealthCheck by name. func (gce *GCECloud) GetHealthCheck(name string) (*compute.HealthCheck, error) { - return gce.service.HealthChecks.Get(gce.projectID, name).Do() + mc := newHealthcheckMetricContext("get") + v, err := gce.service.HealthChecks.Get(gce.projectID, name).Do() + return v, mc.Observe(err) } // UpdateHealthCheck applies the given HealthCheck as an update. func (gce *GCECloud) UpdateHealthCheck(hc *compute.HealthCheck) error { mc := newHealthcheckMetricContext("update") - op, err := gce.service.HealthChecks.Update(gce.projectID, hc.Name, hc).Do() - if err != nil { - mc.Observe(err) - return err + return mc.Observe(err) } return gce.waitForGlobalOp(op, mc) @@ -152,12 +152,9 @@ func (gce *GCECloud) UpdateHealthCheck(hc *compute.HealthCheck) error { // DeleteHealthCheck deletes the given HealthCheck by name. func (gce *GCECloud) DeleteHealthCheck(name string) error { mc := newHealthcheckMetricContext("delete") - op, err := gce.service.HealthChecks.Delete(gce.projectID, name).Do() - if err != nil { - mc.Observe(err) - return err + return mc.Observe(err) } return gce.waitForGlobalOp(op, mc) @@ -166,11 +163,9 @@ func (gce *GCECloud) DeleteHealthCheck(name string) error { // CreateHealthCheck creates the given HealthCheck. func (gce *GCECloud) CreateHealthCheck(hc *compute.HealthCheck) error { mc := newHealthcheckMetricContext("create") - op, err := gce.service.HealthChecks.Insert(gce.projectID, hc).Do() if err != nil { - mc.Observe(err) - return err + return mc.Observe(err) } return gce.waitForGlobalOp(op, mc) @@ -178,6 +173,8 @@ func (gce *GCECloud) CreateHealthCheck(hc *compute.HealthCheck) error { // ListHealthChecks lists all HealthCheck in the project. func (gce *GCECloud) ListHealthChecks() (*compute.HealthCheckList, error) { + mc := newHealthcheckMetricContext("list") // TODO: use PageToken to list all not just the first 500 - return gce.service.HealthChecks.List(gce.projectID).Do() + v, err := gce.service.HealthChecks.List(gce.projectID).Do() + return v, mc.Observe(err) } diff --git a/pkg/cloudprovider/providers/gce/gce_instancegroup.go b/pkg/cloudprovider/providers/gce/gce_instancegroup.go index 4454d19b4c4..abfaf0dc45f 100644 --- a/pkg/cloudprovider/providers/gce/gce_instancegroup.go +++ b/pkg/cloudprovider/providers/gce/gce_instancegroup.go @@ -70,18 +70,24 @@ func (gce *GCECloud) DeleteInstanceGroup(name string, zone string) error { // ListInstanceGroups lists all InstanceGroups in the project and // zone. -func (gce *GCECloud) ListInstanceGroups(zone string) (*compute.InstanceGroupList, error) { +func (gce *GCECloud) ListInstanceGroups(zone string) (v *compute.InstanceGroupList, err error) { + mc := newInstanceGroupMetricContext("list", zone) + defer mc.Observe(err) // TODO: use PageToken to list all not just the first 500 - return gce.service.InstanceGroups.List(gce.projectID, zone).Do() + v, err = gce.service.InstanceGroups.List(gce.projectID, zone).Do() + return } // ListInstancesInInstanceGroup lists all the instances in a given // instance group and state. -func (gce *GCECloud) ListInstancesInInstanceGroup(name string, zone string, state string) (*compute.InstanceGroupsListInstances, error) { +func (gce *GCECloud) ListInstancesInInstanceGroup(name string, zone string, state string) (v *compute.InstanceGroupsListInstances, err error) { + mc := newInstanceGroupMetricContext("list_instances", zone) + defer mc.Observe(err) // TODO: use PageToken to list all not just the first 500 - return gce.service.InstanceGroups.ListInstances( + v, err = gce.service.InstanceGroups.ListInstances( gce.projectID, zone, name, &compute.InstanceGroupsListInstancesRequest{InstanceState: state}).Do() + return } // AddInstancesToInstanceGroup adds the given instances to the given @@ -182,6 +188,9 @@ func (gce *GCECloud) AddPortToInstanceGroup(ig *compute.InstanceGroup, port int6 } // GetInstanceGroup returns an instance group by name. -func (gce *GCECloud) GetInstanceGroup(name string, zone string) (*compute.InstanceGroup, error) { - return gce.service.InstanceGroups.Get(gce.projectID, zone, name).Do() +func (gce *GCECloud) GetInstanceGroup(name string, zone string) (v *compute.InstanceGroup, err error) { + mc := newInstanceGroupMetricContext("get", zone) + defer mc.Observe(err) + v, err = gce.service.InstanceGroups.Get(gce.projectID, zone, name).Do() + return } diff --git a/pkg/cloudprovider/providers/gce/gce_instances.go b/pkg/cloudprovider/providers/gce/gce_instances.go index 5e46f82d8a8..008ab6cbfaa 100644 --- a/pkg/cloudprovider/providers/gce/gce_instances.go +++ b/pkg/cloudprovider/providers/gce/gce_instances.go @@ -36,10 +36,10 @@ import ( "k8s.io/kubernetes/pkg/cloudprovider" ) -func newInstancesMetricContext(request string) *metricContext { +func newInstancesMetricContext(request, zone string) *metricContext { return &metricContext{ start: time.Now(), - attributes: []string{"instances_" + request, unusedMetricLabel, unusedMetricLabel}, + attributes: []string{"instances_" + request, unusedMetricLabel, zone}, } } @@ -164,7 +164,7 @@ func (gce *GCECloud) AddSSHKeyToAllInstances(user string, keyData []byte) error }) } - mc := newInstancesMetricContext("add_ssh_key") + mc := newInstancesMetricContext("add_ssh_key", "") op, err := gce.service.Projects.SetCommonInstanceMetadata( gce.projectID, project.CommonInstanceMetadata).Do() @@ -197,6 +197,7 @@ func (gce *GCECloud) GetAllZones() (sets.String, error) { // TODO: Parallelize, although O(zones) so not too bad (N <= 3 typically) for _, zone := range gce.managedZones { + mc := newInstancesMetricContext("list_zone", zone) // We only retrieve one page in each zone - we only care about existence listCall := gce.service.Instances.List(gce.projectID, zone) @@ -216,7 +217,7 @@ func (gce *GCECloud) GetAllZones() (sets.String, error) { res, err := listCall.Do() if err != nil { - return nil, err + return nil, mc.Observe(err) } if len(res.Items) != 0 { zones.Insert(zone) @@ -338,7 +339,9 @@ func (gce *GCECloud) getInstanceByName(name string) (*gceInstance, error) { // Avoid changing behaviour when not managing multiple zones for _, zone := range gce.managedZones { name = canonicalizeInstanceName(name) + mc := newInstancesMetricContext("get", zone) res, err := gce.service.Instances.Get(gce.projectID, zone, name).Do() + mc.Observe(err) if err != nil { glog.Errorf("getInstanceByName: failed to get instance %s; err: %v", name, err) diff --git a/pkg/cloudprovider/providers/gce/gce_routes.go b/pkg/cloudprovider/providers/gce/gce_routes.go index 17572ab2435..b20e3c82eb1 100644 --- a/pkg/cloudprovider/providers/gce/gce_routes.go +++ b/pkg/cloudprovider/providers/gce/gce_routes.go @@ -42,6 +42,7 @@ func (gce *GCECloud) ListRoutes(clusterName string) ([]*cloudprovider.Route, err pageToken := "" page := 0 for ; page == 0 || (pageToken != "" && page < maxPages); page++ { + mc := newRoutesMetricContext("list_page") listCall := gce.service.Routes.List(gce.projectID) prefix := truncateClusterName(clusterName) @@ -50,6 +51,7 @@ func (gce *GCECloud) ListRoutes(clusterName string) ([]*cloudprovider.Route, err listCall = listCall.PageToken(pageToken) } res, err := listCall.Do() + mc.Observe(err) if err != nil { glog.Errorf("Error getting routes from GCE: %v", err) return nil, err @@ -110,7 +112,7 @@ func (gce *GCECloud) CreateRoute(clusterName string, nameHint string, route *clo } func (gce *GCECloud) DeleteRoute(clusterName string, route *cloudprovider.Route) error { - mc := newRoutesMetricContext("create") + mc := newRoutesMetricContext("delete") deleteOp, err := gce.service.Routes.Delete(gce.projectID, route.Name).Do() if err != nil { return mc.Observe(err) diff --git a/pkg/cloudprovider/providers/gce/gce_staticip.go b/pkg/cloudprovider/providers/gce/gce_staticip.go index f81973d8987..36aa119e898 100644 --- a/pkg/cloudprovider/providers/gce/gce_staticip.go +++ b/pkg/cloudprovider/providers/gce/gce_staticip.go @@ -59,6 +59,8 @@ func (gce *GCECloud) DeleteGlobalStaticIP(name string) error { } // GetGlobalStaticIP returns the global static IP by name. -func (gce *GCECloud) GetGlobalStaticIP(name string) (address *compute.Address, err error) { - return gce.service.GlobalAddresses.Get(gce.projectID, name).Do() +func (gce *GCECloud) GetGlobalStaticIP(name string) (*compute.Address, error) { + mc := newStaticIPMetricContext("get") + v, err := gce.service.GlobalAddresses.Get(gce.projectID, name).Do() + return v, mc.Observe(err) } diff --git a/pkg/cloudprovider/providers/gce/gce_targetproxy.go b/pkg/cloudprovider/providers/gce/gce_targetproxy.go index 875a973f050..383d79c2550 100644 --- a/pkg/cloudprovider/providers/gce/gce_targetproxy.go +++ b/pkg/cloudprovider/providers/gce/gce_targetproxy.go @@ -32,7 +32,9 @@ func newTargetProxyMetricContext(request string) *metricContext { // GetTargetHttpProxy returns the UrlMap by name. func (gce *GCECloud) GetTargetHttpProxy(name string) (*compute.TargetHttpProxy, error) { - return gce.service.TargetHttpProxies.Get(gce.projectID, name).Do() + mc := newTargetProxyMetricContext("get") + v, err := gce.service.TargetHttpProxies.Get(gce.projectID, name).Do() + return v, mc.Observe(err) } // CreateTargetHttpProxy creates and returns a TargetHttpProxy with the given UrlMap. @@ -79,20 +81,24 @@ func (gce *GCECloud) DeleteTargetHttpProxy(name string) error { // ListTargetHttpProxies lists all TargetHttpProxies in the project. func (gce *GCECloud) ListTargetHttpProxies() (*compute.TargetHttpProxyList, error) { + mc := newTargetProxyMetricContext("list") // TODO: use PageToken to list all not just the first 500 - return gce.service.TargetHttpProxies.List(gce.projectID).Do() + v, err := gce.service.TargetHttpProxies.List(gce.projectID).Do() + return v, mc.Observe(err) } // TargetHttpsProxy management // GetTargetHttpsProxy returns the UrlMap by name. func (gce *GCECloud) GetTargetHttpsProxy(name string) (*compute.TargetHttpsProxy, error) { - return gce.service.TargetHttpsProxies.Get(gce.projectID, name).Do() + mc := newTargetProxyMetricContext("get") + v, err := gce.service.TargetHttpsProxies.Get(gce.projectID, name).Do() + return v, mc.Observe(err) } // CreateTargetHttpsProxy creates and returns a TargetHttpsProxy with the given UrlMap and SslCertificate. func (gce *GCECloud) CreateTargetHttpsProxy(urlMap *compute.UrlMap, sslCert *compute.SslCertificate, name string) (*compute.TargetHttpsProxy, error) { - mc := newTargetProxyMetricContext("delete") + mc := newTargetProxyMetricContext("create") proxy := &compute.TargetHttpsProxy{ Name: name, UrlMap: urlMap.SelfLink, @@ -133,7 +139,6 @@ func (gce *GCECloud) SetSslCertificateForTargetHttpsProxy(proxy *compute.TargetH // DeleteTargetHttpsProxy deletes the TargetHttpsProxy by name. func (gce *GCECloud) DeleteTargetHttpsProxy(name string) error { mc := newTargetProxyMetricContext("delete") - op, err := gce.service.TargetHttpsProxies.Delete(gce.projectID, name).Do() if err != nil { if isHTTPErrorCode(err, http.StatusNotFound) { @@ -146,6 +151,8 @@ func (gce *GCECloud) DeleteTargetHttpsProxy(name string) error { // ListTargetHttpsProxies lists all TargetHttpsProxies in the project. func (gce *GCECloud) ListTargetHttpsProxies() (*compute.TargetHttpsProxyList, error) { + mc := newTargetProxyMetricContext("list") // TODO: use PageToken to list all not just the first 500 - return gce.service.TargetHttpsProxies.List(gce.projectID).Do() + v, err := gce.service.TargetHttpsProxies.List(gce.projectID).Do() + return v, mc.Observe(err) } diff --git a/pkg/cloudprovider/providers/gce/gce_urlmap.go b/pkg/cloudprovider/providers/gce/gce_urlmap.go index 862bae4070d..ac9c6e13cff 100644 --- a/pkg/cloudprovider/providers/gce/gce_urlmap.go +++ b/pkg/cloudprovider/providers/gce/gce_urlmap.go @@ -32,7 +32,9 @@ func newUrlMapMetricContext(request string) *metricContext { // GetUrlMap returns the UrlMap by name. func (gce *GCECloud) GetUrlMap(name string) (*compute.UrlMap, error) { - return gce.service.UrlMaps.Get(gce.projectID, name).Do() + mc := newUrlMapMetricContext("get") + v, err := gce.service.UrlMaps.Get(gce.projectID, name).Do() + return v, mc.Observe(err) } // CreateUrlMap creates an url map, using the given backend service as the default service. @@ -80,6 +82,8 @@ func (gce *GCECloud) DeleteUrlMap(name string) error { // ListUrlMaps lists all UrlMaps in the project. func (gce *GCECloud) ListUrlMaps() (*compute.UrlMapList, error) { + mc := newUrlMapMetricContext("list") // TODO: use PageToken to list all not just the first 500 - return gce.service.UrlMaps.List(gce.projectID).Do() + v, err := gce.service.UrlMaps.List(gce.projectID).Do() + return v, mc.Observe(err) } From 14d2cf85a6f2a4892751fa26d72f83e2c9f54f6c Mon Sep 17 00:00:00 2001 From: Nick Sardo Date: Thu, 4 May 2017 18:06:10 -0700 Subject: [PATCH 2/4] Undo capture of list clusters --- pkg/cloudprovider/providers/gce/gce_clusters.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/cloudprovider/providers/gce/gce_clusters.go b/pkg/cloudprovider/providers/gce/gce_clusters.go index 19062a93e14..18c3abc858d 100644 --- a/pkg/cloudprovider/providers/gce/gce_clusters.go +++ b/pkg/cloudprovider/providers/gce/gce_clusters.go @@ -26,19 +26,18 @@ func newClustersMetricContext(request, zone string) *metricContext { } func (gce *GCECloud) ListClusters() ([]string, error) { - mc := newClustersMetricContext("list", "") allClusters := []string{} for _, zone := range gce.managedZones { clusters, err := gce.listClustersInZone(zone) if err != nil { - return nil, mc.Observe(err) + return nil, err } // TODO: Scoping? Do we need to qualify the cluster name? allClusters = append(allClusters, clusters...) } - return allClusters, mc.Observe(nil) + return allClusters, nil } func (gce *GCECloud) Master(clusterName string) (string, error) { From 48d58a15ec3c51a578708d4521e983f5b016d215 Mon Sep 17 00:00:00 2001 From: Nick Sardo Date: Thu, 4 May 2017 18:07:53 -0700 Subject: [PATCH 3/4] Add missing underscore --- pkg/cloudprovider/providers/gce/gce_clusters.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cloudprovider/providers/gce/gce_clusters.go b/pkg/cloudprovider/providers/gce/gce_clusters.go index 18c3abc858d..4524d483f69 100644 --- a/pkg/cloudprovider/providers/gce/gce_clusters.go +++ b/pkg/cloudprovider/providers/gce/gce_clusters.go @@ -21,7 +21,7 @@ import "time" func newClustersMetricContext(request, zone string) *metricContext { return &metricContext{ start: time.Now(), - attributes: []string{"clusters" + request, unusedMetricLabel, zone}, + attributes: []string{"clusters_" + request, unusedMetricLabel, zone}, } } From 63841dadb14d13e334a7f0a7c7d81af024c80d4d Mon Sep 17 00:00:00 2001 From: Nick Sardo Date: Thu, 4 May 2017 18:26:45 -0700 Subject: [PATCH 4/4] missed a file --- pkg/cloudprovider/providers/gce/gce_disks.go | 2 +- .../providers/gce/gce_firewall.go | 7 +---- .../providers/gce/gce_instancegroup.go | 27 +++++++------------ .../providers/gce/gce_instances.go | 5 ++-- 4 files changed, 15 insertions(+), 26 deletions(-) diff --git a/pkg/cloudprovider/providers/gce/gce_disks.go b/pkg/cloudprovider/providers/gce/gce_disks.go index 4e62eaae95a..2217bbb8fd0 100644 --- a/pkg/cloudprovider/providers/gce/gce_disks.go +++ b/pkg/cloudprovider/providers/gce/gce_disks.go @@ -328,7 +328,7 @@ func (gce *GCECloud) findDiskByName(diskName string, zone string) (*GCEDisk, err Kind: disk.Kind, Type: disk.Type, } - return d, mc.Observe(err) + return d, mc.Observe(nil) } if !isHTTPErrorCode(err, http.StatusNotFound) { return nil, mc.Observe(err) diff --git a/pkg/cloudprovider/providers/gce/gce_firewall.go b/pkg/cloudprovider/providers/gce/gce_firewall.go index 607e51140e3..1826f20eb67 100644 --- a/pkg/cloudprovider/providers/gce/gce_firewall.go +++ b/pkg/cloudprovider/providers/gce/gce_firewall.go @@ -34,12 +34,7 @@ func newFirewallMetricContext(request string, region string) *metricContext { // GetFirewall returns the Firewall by name. func (gce *GCECloud) GetFirewall(name string) (*compute.Firewall, error) { - region, err := GetGCERegion(gce.localZone) - if err != nil { - return nil, err - } - - mc := newFirewallMetricContext("get", region) + mc := newFirewallMetricContext("get", "") v, err := gce.service.Firewalls.Get(gce.projectID, name).Do() return v, mc.Observe(err) } diff --git a/pkg/cloudprovider/providers/gce/gce_instancegroup.go b/pkg/cloudprovider/providers/gce/gce_instancegroup.go index abfaf0dc45f..e5ee03be020 100644 --- a/pkg/cloudprovider/providers/gce/gce_instancegroup.go +++ b/pkg/cloudprovider/providers/gce/gce_instancegroup.go @@ -40,7 +40,6 @@ func (gce *GCECloud) CreateInstanceGroup(name string, zone string) (*compute.Ins op, err := gce.service.InstanceGroups.Insert( gce.projectID, zone, &compute.InstanceGroup{Name: name}).Do() - if err != nil { mc.Observe(err) return nil, err @@ -59,7 +58,6 @@ func (gce *GCECloud) DeleteInstanceGroup(name string, zone string) error { op, err := gce.service.InstanceGroups.Delete( gce.projectID, zone, name).Do() - if err != nil { mc.Observe(err) return err @@ -70,31 +68,28 @@ func (gce *GCECloud) DeleteInstanceGroup(name string, zone string) error { // ListInstanceGroups lists all InstanceGroups in the project and // zone. -func (gce *GCECloud) ListInstanceGroups(zone string) (v *compute.InstanceGroupList, err error) { +func (gce *GCECloud) ListInstanceGroups(zone string) (*compute.InstanceGroupList, error) { mc := newInstanceGroupMetricContext("list", zone) - defer mc.Observe(err) // TODO: use PageToken to list all not just the first 500 - v, err = gce.service.InstanceGroups.List(gce.projectID, zone).Do() - return + v, err := gce.service.InstanceGroups.List(gce.projectID, zone).Do() + return v, mc.Observe(err) } // ListInstancesInInstanceGroup lists all the instances in a given // instance group and state. -func (gce *GCECloud) ListInstancesInInstanceGroup(name string, zone string, state string) (v *compute.InstanceGroupsListInstances, err error) { +func (gce *GCECloud) ListInstancesInInstanceGroup(name string, zone string, state string) (*compute.InstanceGroupsListInstances, error) { mc := newInstanceGroupMetricContext("list_instances", zone) - defer mc.Observe(err) // TODO: use PageToken to list all not just the first 500 - v, err = gce.service.InstanceGroups.ListInstances( + v, err := gce.service.InstanceGroups.ListInstances( gce.projectID, zone, name, &compute.InstanceGroupsListInstancesRequest{InstanceState: state}).Do() - return + return v, mc.Observe(err) } // AddInstancesToInstanceGroup adds the given instances to the given // instance group. func (gce *GCECloud) AddInstancesToInstanceGroup(name string, zone string, instanceNames []string) error { mc := newInstanceGroupMetricContext("add_instances", zone) - if len(instanceNames) == 0 { return nil } @@ -121,10 +116,10 @@ func (gce *GCECloud) AddInstancesToInstanceGroup(name string, zone string, insta // the instance group. func (gce *GCECloud) RemoveInstancesFromInstanceGroup(name string, zone string, instanceNames []string) error { mc := newInstanceGroupMetricContext("remove_instances", zone) - if len(instanceNames) == 0 { return nil } + instances := []*compute.InstanceReference{} for _, ins := range instanceNames { instanceLink := makeHostURL(gce.projectID, zone, ins) @@ -152,7 +147,6 @@ func (gce *GCECloud) RemoveInstancesFromInstanceGroup(name string, zone string, // AddPortToInstanceGroup adds a port to the given instance group. func (gce *GCECloud) AddPortToInstanceGroup(ig *compute.InstanceGroup, port int64) (*compute.NamedPort, error) { mc := newInstanceGroupMetricContext("add_port", ig.Zone) - for _, np := range ig.NamedPorts { if np.Port == port { glog.V(3).Infof("Instance group %v already has named port %+v", ig.Name, np) @@ -188,9 +182,8 @@ func (gce *GCECloud) AddPortToInstanceGroup(ig *compute.InstanceGroup, port int6 } // GetInstanceGroup returns an instance group by name. -func (gce *GCECloud) GetInstanceGroup(name string, zone string) (v *compute.InstanceGroup, err error) { +func (gce *GCECloud) GetInstanceGroup(name string, zone string) (*compute.InstanceGroup, error) { mc := newInstanceGroupMetricContext("get", zone) - defer mc.Observe(err) - v, err = gce.service.InstanceGroups.Get(gce.projectID, zone, name).Do() - return + v, err := gce.service.InstanceGroups.Get(gce.projectID, zone, name).Do() + return v, mc.Observe(err) } diff --git a/pkg/cloudprovider/providers/gce/gce_instances.go b/pkg/cloudprovider/providers/gce/gce_instances.go index 008ab6cbfaa..917b570a5fd 100644 --- a/pkg/cloudprovider/providers/gce/gce_instances.go +++ b/pkg/cloudprovider/providers/gce/gce_instances.go @@ -197,7 +197,7 @@ func (gce *GCECloud) GetAllZones() (sets.String, error) { // TODO: Parallelize, although O(zones) so not too bad (N <= 3 typically) for _, zone := range gce.managedZones { - mc := newInstancesMetricContext("list_zone", zone) + mc := newInstancesMetricContext("list", zone) // We only retrieve one page in each zone - we only care about existence listCall := gce.service.Instances.List(gce.projectID, zone) @@ -214,11 +214,12 @@ func (gce *GCECloud) GetAllZones() (sets.String, error) { // Just a minimal set of fields - we only care about existence listCall = listCall.Fields("items(name)") - res, err := listCall.Do() if err != nil { return nil, mc.Observe(err) } + mc.Observe(nil) + if len(res.Items) != 0 { zones.Insert(zone) }