Skip to content

Commit

Permalink
Fix bug: too many cloudwatch metrics
Browse files Browse the repository at this point in the history
Cloudwatch metrics were being added incorrectly. The most obvious
symptom of this was that too many metrics were being added. A simple
check against the name of the metric proved to be a sufficient fix. In
order to test the fix, a metric selection function was factored out.
  • Loading branch information
leon-barrett committed Oct 11, 2016
1 parent e96f7a9 commit d838221
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ continue sending logs to /var/log/telegraf/telegraf.log.

### Bugfixes

- [#1885](https://github.com/influxdata/telegraf/pull/1885): Fix over-querying of cloudwatch metrics
- [#1746](https://github.com/influxdata/telegraf/issues/1746): Fix handling of non-string values for JSON keys listed in tag_keys.
- [#1628](https://github.com/influxdata/telegraf/issues/1628): Fix mongodb input panic on version 2.2.
- [#1733](https://github.com/influxdata/telegraf/issues/1733): Fix statsd scientific notation parsing
Expand Down
24 changes: 15 additions & 9 deletions plugins/inputs/cloudwatch/cloudwatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,7 @@ func (c *CloudWatch) Description() string {
return "Pull Metric Statistics from Amazon CloudWatch"
}

func (c *CloudWatch) Gather(acc telegraf.Accumulator) error {
if c.client == nil {
c.initializeCloudWatch()
}

func SelectMetrics (c *CloudWatch) ([]*cloudwatch.Metric, error) {
var metrics []*cloudwatch.Metric

// check for provided metric filter
Expand All @@ -149,11 +145,11 @@ func (c *CloudWatch) Gather(acc telegraf.Accumulator) error {
} else {
allMetrics, err := c.fetchNamespaceMetrics()
if err != nil {
return err
return nil, err
}
for _, name := range m.MetricNames {
for _, metric := range allMetrics {
if isSelected(metric, m.Dimensions) {
if name == *metric.MetricName && isSelected(metric, m.Dimensions) {
metrics = append(metrics, &cloudwatch.Metric{
Namespace: aws.String(c.Namespace),
MetricName: aws.String(name),
Expand All @@ -163,16 +159,26 @@ func (c *CloudWatch) Gather(acc telegraf.Accumulator) error {
}
}
}

}
} else {
var err error
metrics, err = c.fetchNamespaceMetrics()
if err != nil {
return err
return nil, err
}
}
return metrics, nil
}

func (c *CloudWatch) Gather(acc telegraf.Accumulator) error {
if c.client == nil {
c.initializeCloudWatch()
}

metrics, err := SelectMetrics(c)
if err != nil {
return err
}
metricCount := len(metrics)
errChan := errchan.New(metricCount)

Expand Down
96 changes: 92 additions & 4 deletions plugins/inputs/cloudwatch/cloudwatch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import (
"github.com/stretchr/testify/assert"
)

type mockCloudWatchClient struct{}
type mockGatherCloudWatchClient struct{}

func (m *mockCloudWatchClient) ListMetrics(params *cloudwatch.ListMetricsInput) (*cloudwatch.ListMetricsOutput, error) {
func (m *mockGatherCloudWatchClient) ListMetrics(params *cloudwatch.ListMetricsInput) (*cloudwatch.ListMetricsOutput, error) {
metric := &cloudwatch.Metric{
Namespace: params.Namespace,
MetricName: aws.String("Latency"),
Expand All @@ -31,7 +31,7 @@ func (m *mockCloudWatchClient) ListMetrics(params *cloudwatch.ListMetricsInput)
return result, nil
}

func (m *mockCloudWatchClient) GetMetricStatistics(params *cloudwatch.GetMetricStatisticsInput) (*cloudwatch.GetMetricStatisticsOutput, error) {
func (m *mockGatherCloudWatchClient) GetMetricStatistics(params *cloudwatch.GetMetricStatisticsInput) (*cloudwatch.GetMetricStatisticsOutput, error) {
dataPoint := &cloudwatch.Datapoint{
Timestamp: params.EndTime,
Minimum: aws.Float64(0.1),
Expand Down Expand Up @@ -62,7 +62,7 @@ func TestGather(t *testing.T) {
}

var acc testutil.Accumulator
c.client = &mockCloudWatchClient{}
c.client = &mockGatherCloudWatchClient{}

c.Gather(&acc)

Expand All @@ -83,6 +83,94 @@ func TestGather(t *testing.T) {

}

type mockSelectMetricsCloudWatchClient struct{}

func (m *mockSelectMetricsCloudWatchClient) ListMetrics(params *cloudwatch.ListMetricsInput) (*cloudwatch.ListMetricsOutput, error) {
metrics := []*cloudwatch.Metric{}
// 4 metrics are available
metricNames := []string { "Latency", "RequestCount", "HealthyHostCount", "UnHealthyHostCount" }
// for 3 ELBs
loadBalancers := []string { "lb-1", "lb-2", "lb-3" }
// in 2 AZs
availabilityZones := []string { "us-east-1a", "us-east-1b" }
for _, m := range metricNames {
for _, lb := range loadBalancers {
// For each metric/ELB pair, we get an aggregate value across all AZs.
metrics = append(metrics, &cloudwatch.Metric {
Namespace: aws.String("AWS/ELB"),
MetricName: aws.String(m),
Dimensions: []*cloudwatch.Dimension {
&cloudwatch.Dimension {
Name: aws.String("LoadBalancerName"),
Value: aws.String(lb),
},
},
})
for _, az := range availabilityZones {
// We get a metric for each metric/ELB/AZ triplet.
metrics = append(metrics, &cloudwatch.Metric {
Namespace: aws.String("AWS/ELB"),
MetricName: aws.String(m),
Dimensions: []*cloudwatch.Dimension {
&cloudwatch.Dimension {
Name: aws.String("LoadBalancerName"),
Value: aws.String(lb),
},
&cloudwatch.Dimension {
Name: aws.String("AvailabilityZone"),
Value: aws.String(az),
},
},
})
}
}
}

result := &cloudwatch.ListMetricsOutput{
Metrics: metrics,
}
return result, nil
}

func (m *mockSelectMetricsCloudWatchClient) GetMetricStatistics(params *cloudwatch.GetMetricStatisticsInput) (*cloudwatch.GetMetricStatisticsOutput, error) {
return nil, nil
}

func TestSelectMetrics(t *testing.T) {
duration, _ := time.ParseDuration("1m")
internalDuration := internal.Duration{
Duration: duration,
}
c := &CloudWatch{
Region: "us-east-1",
Namespace: "AWS/ELB",
Delay: internalDuration,
Period: internalDuration,
RateLimit: 10,
Metrics: []*Metric{
&Metric {
MetricNames: []string { "Latency", "RequestCount" },
Dimensions: []*Dimension {
&Dimension {
Name: "LoadBalancerName",
Value: "*",
},
&Dimension {
Name: "AvailabilityZone",
Value: "*",
},
},
},
},
}
c.client = &mockSelectMetricsCloudWatchClient{}
metrics, err := SelectMetrics(c)
// We've asked for 2 (out of 4) metrics, over all 3 load balancers in all 2
// AZs. We should get 12 metrics.
assert.Equal(t, 12, len(metrics))
assert.Nil(t, err)
}

func TestGenerateStatisticsInputParams(t *testing.T) {
d := &cloudwatch.Dimension{
Name: aws.String("LoadBalancerName"),
Expand Down

0 comments on commit d838221

Please sign in to comment.