Skip to content

Commit

Permalink
Refactor metrics recording into the Client
Browse files Browse the repository at this point in the history
It makes sense to take the metrics at the site where the OpenStack call
is made, and also tidies up the 'business logic' functions when metrics
recording is the responsibility of the network client implementation.
  • Loading branch information
macaptain committed Jul 16, 2021
1 parent e10d1bf commit 5ca1033
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 46 deletions.
71 changes: 58 additions & 13 deletions pkg/cloud/services/networking/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,17 @@ func (c networkClient) ListFloatingIP(opts floatingips.ListOptsBuilder) ([]float
}

func (c networkClient) CreateFloatingIP(opts floatingips.CreateOptsBuilder) (*floatingips.FloatingIP, error) {
return floatingips.Create(c.serviceClient, opts).Extract()
mc := metrics.NewMetricPrometheusContext("floating_ip", "create")
fip, err := floatingips.Create(c.serviceClient, opts).Extract()
if mc.ObserveRequest(err) != nil {
return nil, err
}
return fip, nil
}

func (c networkClient) DeleteFloatingIP(id string) error {
return floatingips.Delete(c.serviceClient, id).ExtractErr()
mc := metrics.NewMetricPrometheusContext("floating_ip", "delete")
return mc.ObserveRequest(floatingips.Delete(c.serviceClient, id).ExtractErr())
}

func (c networkClient) GetFloatingIP(id string) (*floatingips.FloatingIP, error) {
Expand All @@ -142,7 +148,12 @@ func (c networkClient) GetFloatingIP(id string) (*floatingips.FloatingIP, error)
}

func (c networkClient) UpdateFloatingIP(id string, opts floatingips.UpdateOptsBuilder) (*floatingips.FloatingIP, error) {
return floatingips.Update(c.serviceClient, id, opts).Extract()
mc := metrics.NewMetricPrometheusContext("floating_ip", "update")
fip, err := floatingips.Update(c.serviceClient, id, opts).Extract()
if mc.ObserveRequest(err) != nil {
return nil, err
}
return fip, nil
}

func (c networkClient) ListPort(opts ports.ListOptsBuilder) ([]ports.Port, error) {
Expand Down Expand Up @@ -171,19 +182,30 @@ func (c networkClient) UpdatePort(id string, opts ports.UpdateOptsBuilder) (*por
}

func (c networkClient) CreateRouter(opts routers.CreateOptsBuilder) (*routers.Router, error) {
return routers.Create(c.serviceClient, opts).Extract()
mc := metrics.NewMetricPrometheusContext("router", "create")
router, err := routers.Create(c.serviceClient, opts).Extract()
if mc.ObserveRequest(err) != nil {
return nil, err
}
return router, nil
}

func (c networkClient) DeleteRouter(id string) error {
return routers.Delete(c.serviceClient, id).ExtractErr()
mc := metrics.NewMetricPrometheusContext("router", "delete")
return mc.ObserveRequest(routers.Delete(c.serviceClient, id).ExtractErr())
}

func (c networkClient) GetRouter(id string) (*routers.Router, error) {
return routers.Get(c.serviceClient, id).Extract()
}

func (c networkClient) UpdateRouter(id string, opts routers.UpdateOptsBuilder) (*routers.Router, error) {
return routers.Update(c.serviceClient, id, opts).Extract()
mc := metrics.NewMetricPrometheusContext("router", "update")
router, err := routers.Update(c.serviceClient, id, opts).Extract()
if mc.ObserveRequest(err) != nil {
return nil, err
}
return router, nil
}

func (c networkClient) ListSecGroup(opts groups.ListOpts) ([]groups.SecGroup, error) {
Expand All @@ -196,11 +218,17 @@ func (c networkClient) ListSecGroup(opts groups.ListOpts) ([]groups.SecGroup, er
}

func (c networkClient) CreateSecGroup(opts groups.CreateOptsBuilder) (*groups.SecGroup, error) {
return groups.Create(c.serviceClient, opts).Extract()
mc := metrics.NewMetricPrometheusContext("security_group", "create")
group, err := groups.Create(c.serviceClient, opts).Extract()
if mc.ObserveRequest(err) != nil {
return nil, err
}
return group, nil
}

func (c networkClient) DeleteSecGroup(id string) error {
return groups.Delete(c.serviceClient, id).ExtractErr()
mc := metrics.NewMetricPrometheusContext("security_group", "delete")
return mc.ObserveRequest(groups.Delete(c.serviceClient, id).ExtractErr())
}

func (c networkClient) GetSecGroup(id string) (*groups.SecGroup, error) {
Expand All @@ -220,11 +248,17 @@ func (c networkClient) ListSecGroupRule(opts rules.ListOpts) ([]rules.SecGroupRu
}

func (c networkClient) CreateSecGroupRule(opts rules.CreateOptsBuilder) (*rules.SecGroupRule, error) {
return rules.Create(c.serviceClient, opts).Extract()
mc := metrics.NewMetricPrometheusContext("security_group_rule", "create")
rule, err := rules.Create(c.serviceClient, opts).Extract()
if mc.ObserveRequest(err) != nil {
return nil, err
}
return rule, nil
}

func (c networkClient) DeleteSecGroupRule(id string) error {
return rules.Delete(c.serviceClient, id).ExtractErr()
mc := metrics.NewMetricPrometheusContext("security_group_rule", "delete")
return mc.ObserveRequest(rules.Delete(c.serviceClient, id).ExtractErr())
}

func (c networkClient) GetSecGroupRule(id string) (*rules.SecGroupRule, error) {
Expand All @@ -241,11 +275,17 @@ func (c networkClient) ListNetwork(opts networks.ListOptsBuilder) ([]networks.Ne
}

func (c networkClient) CreateNetwork(opts networks.CreateOptsBuilder) (*networks.Network, error) {
return networks.Create(c.serviceClient, opts).Extract()
mc := metrics.NewMetricPrometheusContext("network", "create")
net, err := networks.Create(c.serviceClient, opts).Extract()
if (mc.ObserveRequest(err)) != nil {
return nil, err
}
return net, nil
}

func (c networkClient) DeleteNetwork(id string) error {
return networks.Delete(c.serviceClient, id).ExtractErr()
mc := metrics.NewMetricPrometheusContext("network", "delete")
return mc.ObserveRequest(networks.Delete(c.serviceClient, id).ExtractErr())
}

func (c networkClient) GetNetwork(id string) (*networks.Network, error) {
Expand All @@ -266,7 +306,12 @@ func (c networkClient) ListSubnet(opts subnets.ListOptsBuilder) ([]subnets.Subne
}

func (c networkClient) CreateSubnet(opts subnets.CreateOptsBuilder) (*subnets.Subnet, error) {
return subnets.Create(c.serviceClient, opts).Extract()
mc := metrics.NewMetricPrometheusContext("subnet", "create")
subnet, err := subnets.Create(c.serviceClient, opts).Extract()
if mc.ObserveRequest(err) != nil {
return nil, err
}
return subnet, nil
}

func (c networkClient) DeleteSubnet(id string) error {
Expand Down
12 changes: 4 additions & 8 deletions pkg/cloud/services/networking/floatingip.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,8 @@ func (s *Service) GetOrCreateFloatingIP(openStackCluster *infrav1.OpenStackClust
fpCreateOpts.FloatingNetworkID = openStackCluster.Status.ExternalNetwork.ID
fpCreateOpts.Description = names.GetDescription(clusterName)

mc := metrics.NewMetricPrometheusContext("floating_ip", "create")
fp, err = s.client.CreateFloatingIP(fpCreateOpts)
if mc.ObserveRequest(err) != nil {
if err != nil {
record.Warnf(openStackCluster, "FailedCreateFloatingIP", "Failed to create floating IP %s: %v", ip, err)
return nil, err
}
Expand Down Expand Up @@ -102,9 +101,8 @@ func (s *Service) DeleteFloatingIP(openStackCluster *infrav1.OpenStackCluster, i
return nil
}

mc := metrics.NewMetricPrometheusContext("floating_ip", "delete")
err = s.client.DeleteFloatingIP(fip.ID)
if mc.ObserveRequest(err) != nil {
if err != nil {
record.Warnf(openStackCluster, "FailedDeleteFloatingIP", "Failed to delete floating IP %s: %v", ip, err)
return err
}
Expand All @@ -127,9 +125,8 @@ func (s *Service) AssociateFloatingIP(openStackCluster *infrav1.OpenStackCluster
PortID: &portID,
}

mc := metrics.NewMetricPrometheusContext("floating_ip", "update")
_, err := s.client.UpdateFloatingIP(fp.ID, fpUpdateOpts)
if mc.ObserveRequest(err) != nil {
if err != nil {
record.Warnf(openStackCluster, "FailedAssociateFloatingIP", "Failed to associate floating IP %s with port %s: %v", fp.FloatingIP, portID, err)
return err
}
Expand Down Expand Up @@ -159,9 +156,8 @@ func (s *Service) DisassociateFloatingIP(openStackCluster *infrav1.OpenStackClus
PortID: nil,
}

mc := metrics.NewMetricPrometheusContext("floating_ip", "update")
_, err = s.client.UpdateFloatingIP(fip.ID, fpUpdateOpts)
if mc.ObserveRequest(err) != nil {
if err != nil {
record.Warnf(openStackCluster, "FailedDisassociateFloatingIP", "Failed to disassociate floating IP %s: %v", fip.FloatingIP, err)
return err
}
Expand Down
10 changes: 3 additions & 7 deletions pkg/cloud/services/networking/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,8 @@ func (s *Service) ReconcileNetwork(openStackCluster *infrav1.OpenStackCluster, c
}
}

mc := metrics.NewMetricPrometheusContext("network", "create")
network, err := s.client.CreateNetwork(opts)
if mc.ObserveRequest(err) != nil {
if err != nil {
record.Warnf(openStackCluster, "FailedCreateNetwork", "Failed to create network %s: %v", networkName, err)
return err
}
Expand Down Expand Up @@ -155,9 +154,8 @@ func (s *Service) DeleteNetwork(openStackCluster *infrav1.OpenStackCluster, clus
return nil
}

mc := metrics.NewMetricPrometheusContext("network", "delete")
err = s.client.DeleteNetwork(network.ID)
if mc.ObserveRequest(err) != nil {
if err != nil {
record.Warnf(openStackCluster, "FailedDeleteNetwork", "Failed to delete network %s with id %s: %v", network.Name, network.ID, err)
return err
}
Expand Down Expand Up @@ -217,11 +215,9 @@ func (s *Service) createSubnet(openStackCluster *infrav1.OpenStackCluster, clust
DNSNameservers: openStackCluster.Spec.DNSNameservers,
Description: names.GetDescription(clusterName),
}
mc := metrics.NewMetricPrometheusContext("subnet", "create")

subnet, err := s.client.CreateSubnet(opts)

if mc.ObserveRequest(err) != nil {
if err != nil {
record.Warnf(openStackCluster, "FailedCreateSubnet", "Failed to create subnet %s: %v", name, err)
return nil, err
}
Expand Down
12 changes: 3 additions & 9 deletions pkg/cloud/services/networking/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/gophercloud/gophercloud/openstack/networking/v2/subnets"

infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha4"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/metrics"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/record"
capoerrors "sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/errors"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/names"
Expand Down Expand Up @@ -130,11 +129,8 @@ func (s *Service) createRouter(openStackCluster *infrav1.OpenStackCluster, clust
}
}

mc := metrics.NewMetricPrometheusContext("router", "create")

router, err := s.client.CreateRouter(opts)

if mc.ObserveRequest(err) != nil {
if err != nil {
record.Warnf(openStackCluster, "FailedCreateRouter", "Failed to create router %s: %v", name, err)
return nil, err
}
Expand Down Expand Up @@ -178,9 +174,8 @@ func (s *Service) setRouterExternalIPs(openStackCluster *infrav1.OpenStackCluste
})
}

mc := metrics.NewMetricPrometheusContext("router", "update")
_, err := s.client.UpdateRouter(router.ID, updateOpts)
if mc.ObserveRequest(err) != nil {
if err != nil {
record.Warnf(openStackCluster, "FailedUpdateRouter", "Failed to update router %s with id %s: %v", router.Name, router.ID, err)
return err
}
Expand Down Expand Up @@ -213,9 +208,8 @@ func (s *Service) DeleteRouter(openStackCluster *infrav1.OpenStackCluster, clust
}
}

mc := metrics.NewMetricPrometheusContext("router", "delete")
err = s.client.DeleteRouter(router.ID)
if mc.ObserveRequest(err) != nil {
if err != nil {
record.Warnf(openStackCluster, "FailedDeleteRouter", "Failed to delete router %s with id %s: %v", router.Name, router.ID, err)
return err
}
Expand Down
13 changes: 4 additions & 9 deletions pkg/cloud/services/networking/securitygroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/rules"

infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha4"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/metrics"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/record"
)

Expand Down Expand Up @@ -361,9 +360,8 @@ func (s *Service) deleteSecurityGroup(openStackCluster *infrav1.OpenStackCluster
// nothing to do
return nil
}
mc := metrics.NewMetricPrometheusContext("security_group", "delete")
err = s.client.DeleteSecGroup(group.ID)
if mc.ObserveRequest(err) != nil {
if err != nil {
record.Warnf(openStackCluster, "FailedDeleteSecurityGroup", "Failed to delete security group %s with id %s: %v", group.Name, group.ID, err)
return err
}
Expand Down Expand Up @@ -420,9 +418,8 @@ func (s *Service) reconcileGroupRules(desired, observed infrav1.SecurityGroup) (
s.logger.V(4).Info("Deleting rules not needed anymore for group", "name", observed.Name, "amount", len(rulesToDelete))
for _, rule := range rulesToDelete {
s.logger.V(6).Info("Deleting rule", "ruleID", rule.ID, "groupName", observed.Name)
mc := metrics.NewMetricPrometheusContext("security_group_rule", "delete")
err := s.client.DeleteSecGroupRule(rule.ID)
if mc.ObserveRequest(err) != nil {
if err != nil {
return infrav1.SecurityGroup{}, err
}
}
Expand Down Expand Up @@ -459,9 +456,8 @@ func (s *Service) createSecurityGroupIfNotExists(openStackCluster *infrav1.OpenS
}
s.logger.V(6).Info("Creating group", "name", groupName)

mc := metrics.NewMetricPrometheusContext("security_group", "create")
group, err := s.client.CreateSecGroup(createOpts)
if mc.ObserveRequest(err) != nil {
if err != nil {
record.Warnf(openStackCluster, "FailedCreateSecurityGroup", "Failed to create security group %s: %v", groupName, err)
return err
}
Expand Down Expand Up @@ -513,9 +509,8 @@ func (s *Service) createRule(r infrav1.SecurityGroupRule) (infrav1.SecurityGroup
SecGroupID: r.SecurityGroupID,
}
s.logger.V(6).Info("Creating rule", "Description", r.Description, "Direction", dir, "PortRangeMin", r.PortRangeMin, "PortRangeMax", r.PortRangeMax, "Proto", proto, "etherType", etherType, "RemoteGroupID", r.RemoteGroupID, "RemoteIPPrefix", r.RemoteIPPrefix, "SecurityGroupID", r.SecurityGroupID)
mc := metrics.NewMetricPrometheusContext("security_group_rule", "create")
rule, err := s.client.CreateSecGroupRule(createOpts)
if mc.ObserveRequest(err) != nil {
if err != nil {
return infrav1.SecurityGroupRule{}, err
}
return convertOSSecGroupRuleToConfigSecGroupRule(*rule), nil
Expand Down

0 comments on commit 5ca1033

Please sign in to comment.