Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling zeros in readPerfStat #2632

Merged
merged 7 commits into from
Aug 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 8 additions & 19 deletions info/v1/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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).
Expand Down Expand Up @@ -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"`
Expand Down
43 changes: 26 additions & 17 deletions info/v2/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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),
Expand Down
3 changes: 2 additions & 1 deletion integration/tests/api/perf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
64 changes: 38 additions & 26 deletions metrics/prometheus_fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
34 changes: 25 additions & 9 deletions perf/collector_libpfm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the same fix should be provided for uncore perf events, see this line

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 {
Expand Down
Loading