Modify the function CollectUSEMetrics
Modify the function CollectUSEMetrics so that it takes
in a component instead of being implemented by every
component. This removes repetition that was originally
present
Change-Id: I3d5a6def8102609924a63b4a6d68c6d3807b5cfd
Reviewed-on: https://cos-review.googlesource.com/c/cos/tools/+/20670
Cloud-Build: GCB Service account <228075978874@cloudbuild.gserviceaccount.com>
Reviewed-by: Dexter Rivera <riverade@google.com>
Tested-by: Dexter Rivera <riverade@google.com>
diff --git a/src/pkg/nodeprofiler/cloudlogger/cloudlogger_test.go b/src/pkg/nodeprofiler/cloudlogger/cloudlogger_test.go
index 3aac11a..856d9ea 100644
--- a/src/pkg/nodeprofiler/cloudlogger/cloudlogger_test.go
+++ b/src/pkg/nodeprofiler/cloudlogger/cloudlogger_test.go
@@ -1,7 +1,6 @@
package cloudlogger
import (
- "fmt"
"io/ioutil"
"testing"
"time"
@@ -9,8 +8,8 @@
"cloud.google.com/go/logging"
"cos.googlesource.com/cos/tools.git/src/pkg/nodeprofiler/profiler"
"cos.googlesource.com/cos/tools.git/src/pkg/nodeprofiler/utils"
-
"github.com/google/go-cmp/cmp"
+ "github.com/google/go-cmp/cmp/cmpopts"
)
// fakeCPU is a struct that implements the Component interface.
@@ -36,24 +35,6 @@
return nil
}
-// Collect USEMetrics behavior with regards to type fakeCPU.
-func (f *fakeCPU) CollectUSEMetrics(outputs map[string]utils.ParsedOutput) error {
- metrics := f.Metrics
- metrics.Timestamp = time.Date(2021, time.July, 21, 9, 59, 30, 0, time.UTC)
- // setting an arbitrary start time.
- start := metrics.Timestamp
- if err := f.CollectUtilization(outputs); err != nil {
- return fmt.Errorf("failed to collect utilization for CPU: %v", err)
- }
- if err := f.CollectSaturation(outputs); err != nil {
- return fmt.Errorf("failed to collect saturation for CPU: %v", err)
- }
- // setting an arbitrary end time.
- end := time.Date(2021, time.July, 21, 10, 3, 0, 0, time.UTC)
- metrics.Interval = end.Sub(start)
- return nil
-}
-
// USEMetrics behavior with regards to type fakeCPU.
func (f *fakeCPU) USEMetrics() *profiler.USEMetrics {
return f.Metrics
@@ -87,24 +68,6 @@
return nil
}
-// Collect USEMetrics behavior with regards to type fakeMemCap.
-func (f *fakeMemCap) CollectUSEMetrics(outputs map[string]utils.ParsedOutput) error {
- metrics := f.Metrics
- metrics.Timestamp = time.Date(2021, time.July, 21, 9, 59, 30, 0, time.UTC)
- // setting an arbitrary start time.
- start := metrics.Timestamp
- if err := f.CollectUtilization(outputs); err != nil {
- return fmt.Errorf("failed to collect utilization for CPU: %v", err)
- }
- if err := f.CollectSaturation(outputs); err != nil {
- return fmt.Errorf("failed to collect saturation for CPU: %v", err)
- }
- // setting an arbitrary end time.
- end := time.Date(2021, time.July, 21, 10, 3, 0, 0, time.UTC)
- metrics.Interval = end.Sub(start)
- return nil
-}
-
// USEMetrics behavior with regards to type fakeMemCap.
func (f *fakeMemCap) USEMetrics() *profiler.USEMetrics {
return f.Metrics
@@ -138,24 +101,6 @@
return nil
}
-// Collect USEMetrics behavior with regards to type fakeStorageDevIO.
-func (f *fakeStorageDevIO) CollectUSEMetrics(outputs map[string]utils.ParsedOutput) error {
- metrics := f.Metrics
- metrics.Timestamp = time.Date(2021, time.July, 21, 9, 59, 30, 0, time.UTC)
- // setting an arbitrary start time.
- start := metrics.Timestamp
- if err := f.CollectUtilization(outputs); err != nil {
- return fmt.Errorf("failed to collect utilization for CPU: %v", err)
- }
- if err := f.CollectSaturation(outputs); err != nil {
- return fmt.Errorf("failed to collect saturation for CPU: %v", err)
- }
- // setting an arbitrary end time.
- end := time.Date(2021, time.July, 21, 10, 3, 0, 0, time.UTC)
- metrics.Interval = end.Sub(start)
- return nil
-}
-
// USEMetrics behavior with regards to type fakeStorageDevIO.
func (f *fakeStorageDevIO) USEMetrics() *profiler.USEMetrics {
return f.Metrics
@@ -461,7 +406,8 @@
if gotErr := err != nil; gotErr != test.wantErr {
t.Errorf("LogProfilerReport(%v, %v) = %v, wantErr %t", f, test.input, err, test.wantErr)
}
- if diff := cmp.Diff(test.wantOutput, f.logged); diff != "" {
+
+ if diff := cmp.Diff(f.logged, test.wantOutput, cmpopts.IgnoreFields(profiler.USEMetrics{}, "Timestamp", "Interval")); diff != "" {
t.Errorf("ran LogProfilerReport(fakeStructuredLogger,%+v), but got mismatch between got and want (-got, +want): \n diff %s", test.input, diff)
}
diff --git a/src/pkg/nodeprofiler/profiler/components.go b/src/pkg/nodeprofiler/profiler/components.go
index 40b16bf..86730b1 100644
--- a/src/pkg/nodeprofiler/profiler/components.go
+++ b/src/pkg/nodeprofiler/profiler/components.go
@@ -27,8 +27,6 @@
// It takes in a map of commands to their parsed output and uses that
// to specify which commands (and therefore output) it needs.
CollectErrors(cmdOutputs map[string]utils.ParsedOutput) error
- // CollectUSEMetrics collects USEMetrics for the component.
- CollectUSEMetrics(cmdOutputs map[string]utils.ParsedOutput) error
// USEMetrics returns the USEMetrics of the component.
USEMetrics() *USEMetrics
// Name returns the name of the component.
@@ -187,32 +185,6 @@
return nil
}
-// CollectUSEMetrics collects USE Metrics for the CPU component.
-func (c *CPU) CollectUSEMetrics(outputs map[string]utils.ParsedOutput) error {
- metrics := c.metrics
- metrics.Timestamp = time.Now()
- start := metrics.Timestamp
-
- var gotErr bool
- if err := c.CollectUtilization(outputs); err != nil {
- gotErr = true
- log.Errorf("failed to collect utilization for CPU: %v", err)
- }
- if err := c.CollectSaturation(outputs); err != nil {
- gotErr = true
- log.Errorf("failed to collect saturation for CPU: %v", err)
- }
- end := time.Now()
- metrics.Interval = end.Sub(start)
-
- if gotErr {
- err := "failed to collect all USE Metrics for CPU. " +
- "Please check the logs for more information"
- return fmt.Errorf(err)
- }
- return nil
-}
-
// MemCap holds information about the Memory capacity component:
// name and USE Metrics collected.
type MemCap struct {
@@ -358,32 +330,6 @@
return nil
}
-// CollectUSEMetrics collects USE Metrics for the MemCap component.
-func (m *MemCap) CollectUSEMetrics(outputs map[string]utils.ParsedOutput) error {
- metrics := m.metrics
- metrics.Timestamp = time.Now()
- start := metrics.Timestamp
-
- var gotErr bool
- if err := m.CollectUtilization(outputs); err != nil {
- gotErr = true
- log.Errorf("failed to collect utilization for Memory capacity: %v", err)
- }
- if err := m.CollectSaturation(outputs); err != nil {
- gotErr = true
- log.Errorf("failed to collect saturation for Memory capacity: %v", err)
- }
- end := time.Now()
- metrics.Interval = end.Sub(start)
-
- if gotErr {
- err := "failed to collect all USE metrics for Memory Capacity. " +
- "Please check the logs for more information"
- return fmt.Errorf(err)
- }
- return nil
-}
-
// StorageDevIO holds information about the Storage device I/O component:
// name and USE Metrics collected.
type StorageDevIO struct {
@@ -466,32 +412,6 @@
return nil
}
-// CollectUSEMetrics collects USE Metrics for the Storage Device I/O component.
-func (d *StorageDevIO) CollectUSEMetrics(outputs map[string]utils.ParsedOutput) error {
- metrics := d.metrics
- metrics.Timestamp = time.Now()
- start := metrics.Timestamp
-
- var gotErr bool
- if err := d.CollectUtilization(outputs); err != nil {
- gotErr = true
- log.Errorf("failed to collect utilization for Storage Device I/O: %v", err)
- }
- if err := d.CollectSaturation(outputs); err != nil {
- gotErr = true
- log.Errorf("failed to collect saturation for Storage Device I/O: %v", err)
- }
- end := time.Now()
- metrics.Interval = end.Sub(start)
-
- if gotErr {
- err := "failed to collect all USE metrics for Storage Device I/O. " +
- "Please check the logs for more information"
- return fmt.Errorf(err)
- }
- return nil
-}
-
// StorageCap holds information about the Storage Capacity component:
// name and USE Metrics collected.
type StorageCap struct {
@@ -601,30 +521,6 @@
return nil
}
-// CollectUSEMetrics collects USE Metrics for the Storage Capacity component
-func (s *StorageCap) CollectUSEMetrics(outputs map[string]utils.ParsedOutput) error {
- metrics := s.metrics
- metrics.Timestamp = time.Now()
- start := metrics.Timestamp
-
- var gotErr bool
- if err := s.CollectUtilization(outputs); err != nil {
- gotErr = true
- log.Errorf("failed to collect utilization for Storage capacity: %v", err)
- }
- if err := s.CollectSaturation(outputs); err != nil {
- gotErr = true
- log.Errorf("failed to collect saturation for Storage capacity: %v", err)
- }
- end := time.Now()
- metrics.Interval = end.Sub(start)
-
- if gotErr {
- return fmt.Errorf("got error when collecting USE Metrics for Storage Capacity")
- }
- return nil
-}
-
func (s *StorageCap) USEMetrics() *USEMetrics {
return s.metrics
}
@@ -633,6 +529,34 @@
return s.name
}
+// CollectUSEMetrics collects USE Metrics for the component specified. It does this by calling
+// the necessary methods to collect utilization, saturation and errors.
+func CollectUSEMetrics(component Component, outputs map[string]utils.ParsedOutput) error {
+
+ metrics := component.USEMetrics()
+ metrics.Timestamp = time.Now()
+ start := metrics.Timestamp
+
+ var gotErr bool
+ if err := component.CollectUtilization(outputs); err != nil {
+ gotErr = true
+ log.Errorf("failed to collect utilization for %q: %v", component.Name(), err)
+ }
+ if err := component.CollectSaturation(outputs); err != nil {
+ gotErr = true
+ log.Errorf("failed to collect saturation for %q: %v", component.Name(), err)
+ }
+ end := time.Now()
+ metrics.Interval = end.Sub(start)
+
+ if gotErr {
+ err := "failed to collect all USE metrics for %q. " +
+ "Please check the logs for more information"
+ return fmt.Errorf(err, component.Name())
+ }
+ return nil
+}
+
// GenerateUSEReport generates USE Metrics for all the components
// as well as an analysis string to help the diagnose performance issues.
func GenerateUSEReport(components []Component, cmds []Command) (USEReport, error) {
@@ -649,7 +573,7 @@
}
var failed []string
for _, s := range components {
- if err := s.CollectUSEMetrics(outputs); err != nil {
+ if err := CollectUSEMetrics(s, outputs); err != nil {
log.Errorf("failed to collect USE metrics for %q", s.Name())
failed = append(failed, s.Name())
}
