Merge pull request #2632 from iwankgb/readPerf_zeros

Handling zeros in readPerfStat

info/v1/container.go

@@ -827,6 +827,13 @@ type AcceleratorStats struct {
 
 // PerfStat represents value of a single monitored perf event.
 type PerfStat struct {
+	PerfValue
+
+	// CPU that perf event was measured on.
+	Cpu int `json:"cpu"`
+}
+
+type PerfValue struct {
 	// Indicates scaling ratio for an event: time_running/time_enabled
 	// (amount of time that event was being measured divided by
 	// amount of time that event was enabled for).
@@ -843,9 +850,6 @@ type PerfStat struct {
 
 	// Name is human readable name of an event.
 	Name string `json:"name"`
-
-	// CPU that perf event was measured on.
-	Cpu int `json:"cpu"`
 }
 
 // MemoryBandwidthStats corresponds to MBM (Memory Bandwidth Monitoring).
@@ -876,22 +880,7 @@ type ResctrlStats struct {
 
 // PerfUncoreStat represents value of a single monitored perf uncore event.
 type PerfUncoreStat struct {
-	// Indicates scaling ratio for an event: time_running/time_enabled
-	// (amount of time that event was being measured divided by
-	// amount of time that event was enabled for).
-	// value 1.0 indicates that no multiplexing occurred. Value close
-	// to 0 indicates that event was measured for short time and event's
-	// value might be inaccurate.
-	// See: https://lwn.net/Articles/324756/
-	ScalingRatio float64 `json:"scaling_ratio"`
-
-	// Value represents value of perf event retrieved from OS. It is
-	// normalized against ScalingRatio and takes multiplexing into
-	// consideration.
-	Value uint64 `json:"value"`
-
-	// Name is human readable name of an event.
-	Name string `json:"name"`
+	PerfValue
 
 	// Socket that perf event was measured on.
 	Socket int `json:"socket"`

info/v2/conversion_test.go

@@ -19,8 +19,9 @@ import (
 	"testing"
 	"time"
 
-	"github.com/google/cadvisor/info/v1"
 	"github.com/stretchr/testify/assert"
+
+	"github.com/google/cadvisor/info/v1"
 )
 
 var (
@@ -198,30 +199,38 @@ func TestContainerStatsFromV1(t *testing.T) {
 		}},
 		PerfStats: []v1.PerfStat{
 			{
-				ScalingRatio: 1,
-				Value:        123,
-				Name:         "instructions",
+				PerfValue: v1.PerfValue{
+					ScalingRatio: 1,
+					Value:        123,
+					Name:         "instructions",
+				},
 			},
 			{
-				ScalingRatio: 0.3333333,
-				Value:        123456,
-				Name:         "cycles",
+				PerfValue: v1.PerfValue{
+					ScalingRatio: 0.3333333,
+					Value:        123456,
+					Name:         "cycles",
+				},
 			},
 		},
 		PerfUncoreStats: []v1.PerfUncoreStat{
 			{
-				ScalingRatio: 1.0,
-				Value:        123456,
-				Name:         "uncore_imc_0/cas_count_write",
-				Socket:       0,
-				PMU:          "17",
+				PerfValue: v1.PerfValue{
+					ScalingRatio: 1.0,
+					Value:        123456,
+					Name:         "uncore_imc_0/cas_count_write",
+				},
+				Socket: 0,
+				PMU:    "17",
 			},
 			{
-				ScalingRatio: 1.0,
-				Value:        654321,
-				Name:         "uncore_imc_0/cas_count_write",
-				Socket:       1,
-				PMU:          "17",
+				PerfValue: v1.PerfValue{
+					ScalingRatio: 1.0,
+					Value:        654321,
+					Name:         "uncore_imc_0/cas_count_write",
+				},
+				Socket: 1,
+				PMU:    "17",
 			},
 		},
 		ReferencedMemory: uint64(1234),

integration/tests/api/perf_test.go

@@ -47,11 +47,12 @@ func TestPerfEvents(t *testing.T) {
 }
 
 func waitForContainerInfo(fm framework.Framework, containerID string) {
-	framework.RetryForDuration(func() error {
+	err := framework.RetryForDuration(func() error {
 		_, err := fm.Cadvisor().Client().DockerContainer(containerID, &v1.ContainerInfoRequest{NumStats: 1})
 		if err != nil {
 			return err
 		}
 		return nil
 	}, 5*time.Second)
+	assert.NoError(fm.T(), err)
 }

metrics/prometheus_fake.go

@@ -625,44 +625,56 @@ func (p testSubcontainersInfoProvider) GetRequestedContainersInfo(string, v2.Req
 					},
 					PerfStats: []info.PerfStat{
 						{
-							ScalingRatio: 1.0,
-							Value:        123,
-							Name:         "instructions",
-							Cpu:          0,
+							PerfValue: info.PerfValue{
+								ScalingRatio: 1.0,
+								Value:        123,
+								Name:         "instructions",
+							},
+							Cpu: 0,
 						},
 						{
-							ScalingRatio: 0.5,
-							Value:        456,
-							Name:         "instructions",
-							Cpu:          1,
+							PerfValue: info.PerfValue{
+								ScalingRatio: 0.5,
+								Value:        456,
+								Name:         "instructions",
+							},
+							Cpu: 1,
 						},
 						{
-							ScalingRatio: 0.66666666666,
-							Value:        321,
-							Name:         "instructions_retired",
-							Cpu:          0,
+							PerfValue: info.PerfValue{
+								ScalingRatio: 0.66666666666,
+								Value:        321,
+								Name:         "instructions_retired",
+							},
+							Cpu: 0,
 						},
 						{
-							ScalingRatio: 0.33333333333,
-							Value:        789,
-							Name:         "instructions_retired",
-							Cpu:          1,
+							PerfValue: info.PerfValue{
+								ScalingRatio: 0.33333333333,
+								Value:        789,
+								Name:         "instructions_retired",
+							},
+							Cpu: 1,
 						},
 					},
 					PerfUncoreStats: []info.PerfUncoreStat{
 						{
-							ScalingRatio: 1.0,
-							Value:        1231231512.0,
-							Name:         "cas_count_read",
-							Socket:       0,
-							PMU:          "uncore_imc_0",
+							PerfValue: info.PerfValue{
+								ScalingRatio: 1.0,
+								Value:        1231231512.0,
+								Name:         "cas_count_read",
+							},
+							Socket: 0,
+							PMU:    "uncore_imc_0",
 						},
 						{
-							ScalingRatio: 1.0,
-							Value:        1111231331.0,
-							Name:         "cas_count_read",
-							Socket:       1,
-							PMU:          "uncore_imc_0",
+							PerfValue: info.PerfValue{
+								ScalingRatio: 1.0,
+								Value:        1111231331.0,
+								Name:         "cas_count_read",
+							},
+							Socket: 1,
+							PMU:    "uncore_imc_0",
 						},
 					},
 					ReferencedMemory: 1234,

perf/collector_libpfm.go

@@ -99,31 +99,47 @@ func (c *collector) UpdateStats(stats *info.ContainerStats) error {
 }
 
 func readPerfStat(file readerCloser, name string, cpu int) (*info.PerfStat, error) {
+	value, err := getPerfValue(file, name)
+	if err != nil {
+		return nil, err
+	}
+
+	stat := info.PerfStat{
+		PerfValue: value,
+		Cpu:       cpu,
+	}
+
+	return &stat, nil
+}
+
+func getPerfValue(file readerCloser, name string) (info.PerfValue, error) {
 	buf := make([]byte, 32)
 	_, err := file.Read(buf)
 	if err != nil {
-		return nil, err
+		return info.PerfValue{}, err
 	}
 	perfData := &ReadFormat{}
 	reader := bytes.NewReader(buf)
 	err = binary.Read(reader, binary.LittleEndian, perfData)
 	if err != nil {
-		return nil, err
+		return info.PerfValue{}, err
 	}
 
 	scalingRatio := 1.0
-	if perfData.TimeEnabled != 0 {
+	if perfData.TimeRunning != 0 && perfData.TimeEnabled != 0 {
 		scalingRatio = float64(perfData.TimeRunning) / float64(perfData.TimeEnabled)
 	}
 
-	stat := info.PerfStat{
-		Value:        uint64(float64(perfData.Value) / scalingRatio),
-		Name:         name,
-		ScalingRatio: scalingRatio,
-		Cpu:          cpu,
+	value := perfData.Value
+	if scalingRatio != float64(0) {
+		value = uint64(float64(value) / scalingRatio)
 	}
 
-	return &stat, nil
+	return info.PerfValue{
+		Value:        value,
+		Name:         name,
+		ScalingRatio: scalingRatio,
+	}, nil
 }
 
 func (c *collector) setup() error {