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.

Sign up for GitHub

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

add noiseDelta to telemetry test #1955

Merged
merged 1 commit into from
Apr 2, 2019
+29 −19
Merged

add noiseDelta to telemetry test #1955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 22 additions & 13 deletions agent/functional_tests/tests/functionaltests_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,9 @@ func telemetryTest(t *testing.T, taskDefinition string) {
// Try to let the container use 25% cpu, but bound it within valid range
cpuShare, expectedCPUPercentage := calculateCpuLimits(0.25)

// account for docker stats / CloudWatch noise
statsNoiseDelta := 5.0

// Try to use a new cluster for this test, ensure no other task metrics for this cluster
newClusterName := "ecstest-telemetry-" + uuid.New()
_, err := ECS.CreateCluster(&ecsapi.CreateClusterInput{
Expand Down Expand Up @@ -511,11 +514,12 @@ func telemetryTest(t *testing.T, taskDefinition string) {
time.Sleep(waitMetricsInCloudwatchDuration)

cwclient := cloudwatch.New(session.New(), aws.NewConfig().WithRegion(*ECS.Config.Region))
_, err = VerifyMetrics(cwclient, params, true)

_, err = VerifyMetrics(cwclient, params, true, statsNoiseDelta)
assert.NoError(t, err, "Before task running, verify metrics for CPU utilization failed")

params.MetricName = aws.String("MemoryUtilization")
_, err = VerifyMetrics(cwclient, params, true)
_, err = VerifyMetrics(cwclient, params, true, statsNoiseDelta)
assert.NoError(t, err, "Before task running, verify metrics for memory utilization failed")

tdOverrides := make(map[string]string)
Expand All @@ -531,19 +535,21 @@ func telemetryTest(t *testing.T, taskDefinition string) {
params.EndTime = aws.Time(RoundTimeUp(time.Now(), time.Minute).UTC())
params.StartTime = aws.Time((*params.EndTime).Add(-waitMetricsInCloudwatchDuration).UTC())
params.MetricName = aws.String("CPUUtilization")
metrics, err := VerifyMetrics(cwclient, params, false)
metrics, err := VerifyMetrics(cwclient, params, false, 0.0)
assert.NoError(t, err, "Task is running, verify metrics for CPU utilization failed")
// Also verify the cpu usage is around expectedCPUPercentage +/- 5%
assert.InDelta(t, expectedCPUPercentage*100.0, *metrics.Average, 5)
// Also verify the cpu usage is around expectedCPUPercentage
// +/- StatsNoiseDelta percentage
assert.InDelta(t, expectedCPUPercentage*100.0, *metrics.Average, statsNoiseDelta)

params.MetricName = aws.String("MemoryUtilization")
metrics, err = VerifyMetrics(cwclient, params, false)
metrics, err = VerifyMetrics(cwclient, params, false, 0.0)
assert.NoError(t, err, "Task is running, verify metrics for memory utilization failed")
memInfo, err := system.ReadMemInfo()
require.NoError(t, err, "Acquiring system info failed")
totalMemory := memInfo.MemTotal / bytePerMegabyte
// Verify the memory usage is around 1024/totalMemory +/- 5%
assert.InDelta(t, float32(1024*100)/float32(totalMemory), *metrics.Average, 5)
// Verify the memory usage is around 1024/totalMemory
// +/- StatsNoiseDelta percentage
assert.InDelta(t, float32(1024*100)/float32(totalMemory), *metrics.Average, statsNoiseDelta)

err = testTask.Stop()
require.NoError(t, err, "Failed to stop the telemetry task")
Expand All @@ -555,11 +561,11 @@ func telemetryTest(t *testing.T, taskDefinition string) {
params.EndTime = aws.Time(RoundTimeUp(time.Now(), time.Minute).UTC())
params.StartTime = aws.Time((*params.EndTime).Add(-waitMetricsInCloudwatchDuration).UTC())
params.MetricName = aws.String("CPUUtilization")
_, err = VerifyMetrics(cwclient, params, true)
_, err = VerifyMetrics(cwclient, params, true, statsNoiseDelta)
assert.NoError(t, err, "Task stopped: verify metrics for CPU utilization failed")

params.MetricName = aws.String("MemoryUtilization")
_, err = VerifyMetrics(cwclient, params, true)
_, err = VerifyMetrics(cwclient, params, true, statsNoiseDelta)
assert.NoError(t, err, "Task stopped, verify metrics for memory utilization failed")
}

Expand All @@ -570,6 +576,9 @@ func telemetryTestWithStatsPolling(t *testing.T, taskDefinition string) {
// Try to let the container use 25% cpu, but bound it within valid range
cpuShare, expectedCPUPercentage := calculateCpuLimits(0.25)

// account for docker stats / CloudWatch noise
statsNoiseDelta := 5.0

// Try to use a new cluster for this test, ensure no other task metrics for this cluster
newClusterName := "ecstest-telemetry-polling-" + uuid.New()
_, err := ECS.CreateCluster(&ecsapi.CreateClusterInput{
Expand Down Expand Up @@ -621,13 +630,13 @@ func telemetryTestWithStatsPolling(t *testing.T, taskDefinition string) {
params.EndTime = aws.Time(RoundTimeUp(time.Now(), time.Minute).UTC())
params.StartTime = aws.Time((*params.EndTime).Add(-waitMetricsInCloudwatchDuration).UTC())
params.MetricName = aws.String("CPUUtilization")
metrics, err := VerifyMetrics(cwclient, params, false)
metrics, err := VerifyMetrics(cwclient, params, false, 0.0)
assert.NoError(t, err, "Task is running, verify metrics for CPU utilization failed")
// Also verify the cpu usage is around expectedCPUPercentage +/- 5%
assert.InDelta(t, expectedCPUPercentage*100.0, *metrics.Average, 5)
assert.InDelta(t, expectedCPUPercentage*100.0, *metrics.Average, statsNoiseDelta)

params.MetricName = aws.String("MemoryUtilization")
metrics, err = VerifyMetrics(cwclient, params, false)
metrics, err = VerifyMetrics(cwclient, params, false, 0.0)
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this one zero instead of statsNoiseDelta?

Copy link
Member Author

@fierlion fierlion Mar 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

golang won't let me overload a function and I didn't want to use variadic args (...).

The existing VerifyMetrics signature uses the idleCluster bool:

func VerifyMetrics(cwclient *cloudwatch.CloudWatch, params *cloudwatch.GetMetricStatisticsInput, idleCluster bool)

So now if idleCluster = true, we need a corresponding noiseDelta float. There are no checks for idleCluster = false so the 0.0 is effectively a nil placeholder wherever we have false. If there's a better way of doing this, I'm open to it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use statsNoiseDelta as a constant within the function? I don't think we gain any flexibility by passing it around.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated.

assert.NoError(t, err, "Task is running, verify metrics for memory utilization failed")
memInfo, err := system.ReadMemInfo()
require.NoError(t, err, "Acquiring system info failed")
Expand Down
13 changes: 7 additions & 6 deletions agent/functional_tests/util/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,9 @@ func DeleteCluster(t *testing.T, clusterName string) {

// VerifyMetrics whether the response is as expected
// the expected value can be 0 or positive
func VerifyMetrics(cwclient *cloudwatch.CloudWatch, params *cloudwatch.GetMetricStatisticsInput, idleCluster bool) (*cloudwatch.Datapoint, error) {
// noiseDelta should be significantly less than the percentage of cpu/memory we
// use for non-idle workload.
func VerifyMetrics(cwclient *cloudwatch.CloudWatch, params *cloudwatch.GetMetricStatisticsInput, idleCluster bool, noiseDelta float64) (*cloudwatch.Datapoint, error) {
resp, err := cwclient.GetMetricStatistics(params)
if err != nil {
return nil, fmt.Errorf("Error getting metrics of cluster: %v", err)
Expand All @@ -353,14 +355,13 @@ func VerifyMetrics(cwclient *cloudwatch.CloudWatch, params *cloudwatch.GetMetric
if *datapoint.SampleCount != 1.0 {
return nil, fmt.Errorf("Incorrect SampleCount %f, expected 1", *datapoint.SampleCount)
}

if idleCluster {
if *datapoint.Average != 0.0 {
return nil, fmt.Errorf("non-zero utilization for idle cluster")
if *datapoint.Average >= noiseDelta {
return nil, fmt.Errorf("utilization is >= expected noise delta for idle cluster")
}
} else {
if *datapoint.Average == 0.0 {
return nil, fmt.Errorf("utilization is zero for non-idle cluster")
if *datapoint.Average < noiseDelta {
return nil, fmt.Errorf("utilization is < expected noise delta for non-idle cluster")
}
}
return datapoint, nil
Expand Down