From d7d3de213a78219ab0e15cad7730854b67c96860 Mon Sep 17 00:00:00 2001 From: kaiyan-sheng Date: Thu, 24 Jan 2019 11:58:29 -0700 Subject: [PATCH] Remove hardcoded 300s and add error messages when data collection is missing (#10142) * Remove hardcoded 300s and add error messages * Add logger instance in metricset instead recreating new ones * Add check on period to be multiple of 60s * Change to info msg for period setting * Update metricbeat/docs/modules/aws.asciidoc --- metricbeat/docs/modules/aws.asciidoc | 21 ++- .../metricbeat/module/aws/_meta/docs.asciidoc | 21 ++- x-pack/metricbeat/module/aws/ec2/ec2.go | 148 ++++++++++-------- x-pack/metricbeat/module/aws/ec2/ec2_test.go | 128 +++++++++------ 4 files changed, 181 insertions(+), 137 deletions(-) diff --git a/metricbeat/docs/modules/aws.asciidoc b/metricbeat/docs/modules/aws.asciidoc index 04fae450d5d..f85ed8c8aa0 100644 --- a/metricbeat/docs/modules/aws.asciidoc +++ b/metricbeat/docs/modules/aws.asciidoc @@ -7,8 +7,9 @@ This file is generated! See scripts/docs_collector.py beta[] -This module periodically fetches monitoring metrics from AWS Cloudwatch using GetMetricData API. Note: extra AWS charges on -GetMetricData API requests will be generated by this module. +This module periodically fetches monitoring metrics from AWS Cloudwatch using +https://docs.aws.amazon.com/AmazonCloudWatch/latest/APIReference/API_GetMetricData.html[GetMetricData API] for running +EC2 instances. Note: extra AWS charges on GetMetricData API requests will be generated by this module. The default metricset is `ec2`. @@ -30,7 +31,7 @@ see https://docs.aws.amazon.com/IAM/latest/UserGuide/id_credentials_temp.html[Te `sts get-session-token` AWS CLI can be used to generate temporary credentials. For example. with MFA-enabled: ---- -aws> sts get-session-token --serial-number arn:aws:iam::1234:mfa/test@elastic.co --token-code 456789 --duration-seconds 129600 +aws> sts get-session-token --serial-number arn:aws:iam::1234:mfa/your-email@example.com --token-code 456789 --duration-seconds 129600 ---- Specific permissions needs to be added into the IAM user's policy to allow Metricbeat collecting AWS monitoring metrics. Please @@ -43,11 +44,12 @@ $0.01/1000 metrics requested using GetMetricData. Please see https://aws.amazon. for more details. To avoid unnecessary charges, `period` is preferred to be set to `300s` or multiples of `300s`, such as `600s` and `900s`. -For more granular monitoring data you can enable detailed monitoring on the instance to get metrics every 1 minute. With this -detailed monitoring enabled, `period` in aws module configuration can be any number larger than `60s`. Since AWS sends metric -data to CloudWatch in 1-minute periods, setting metricbeat module `period` less than `60s` will cause extra API requests which -means extra charges on AWS. To avoid unnecessary charges, `period` is preferred to be set to `60s` or multiples of `60s`, -such as `120s` and `180s`. +For more granular monitoring data you can enable detailed monitoring on the instance to get metrics every 1 minute. Please see +https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-cloudwatch-new.html[Enabling Detailed Monitoring] for instructions +on how to enable detailed monitoring. With detailed monitoring enabled, `period` in aws module configuration can be any number +larger than `60s`. Since AWS sends metric data to CloudWatch in 1-minute periods, setting metricbeat module `period` less +than `60s` will cause extra API requests which means extra charges on AWS. To avoid unnecessary charges, `period` is +preferred to be set to `60s` or multiples of `60s`, such as `120s` and `180s`. Here is an example of aws metricbeat module configuration: @@ -63,6 +65,9 @@ metricbeat.modules: default_region: '${AWS_REGION:us-west-1}' ---- +This module only collects metrics for EC2 instances that are in `running` state and exist more than 10 minutes to make sure +there are monitoring metrics exist in Cloudwatch already. + The AWS module comes with a predefined dashboard. For example: image::./images/metricbeat-aws-ec2-overview.png[] diff --git a/x-pack/metricbeat/module/aws/_meta/docs.asciidoc b/x-pack/metricbeat/module/aws/_meta/docs.asciidoc index bf19bf650dd..735254f1a8a 100644 --- a/x-pack/metricbeat/module/aws/_meta/docs.asciidoc +++ b/x-pack/metricbeat/module/aws/_meta/docs.asciidoc @@ -1,5 +1,6 @@ -This module periodically fetches monitoring metrics from AWS Cloudwatch using GetMetricData API. Note: extra AWS charges on -GetMetricData API requests will be generated by this module. +This module periodically fetches monitoring metrics from AWS Cloudwatch using +https://docs.aws.amazon.com/AmazonCloudWatch/latest/APIReference/API_GetMetricData.html[GetMetricData API] for running +EC2 instances. Note: extra AWS charges on GetMetricData API requests will be generated by this module. The default metricset is `ec2`. @@ -21,7 +22,7 @@ see https://docs.aws.amazon.com/IAM/latest/UserGuide/id_credentials_temp.html[Te `sts get-session-token` AWS CLI can be used to generate temporary credentials. For example. with MFA-enabled: ---- -aws> sts get-session-token --serial-number arn:aws:iam::1234:mfa/test@elastic.co --token-code 456789 --duration-seconds 129600 +aws> sts get-session-token --serial-number arn:aws:iam::1234:mfa/your-email@example.com --token-code 456789 --duration-seconds 129600 ---- Specific permissions needs to be added into the IAM user's policy to allow Metricbeat collecting AWS monitoring metrics. Please @@ -34,11 +35,12 @@ $0.01/1000 metrics requested using GetMetricData. Please see https://aws.amazon. for more details. To avoid unnecessary charges, `period` is preferred to be set to `300s` or multiples of `300s`, such as `600s` and `900s`. -For more granular monitoring data you can enable detailed monitoring on the instance to get metrics every 1 minute. With this -detailed monitoring enabled, `period` in aws module configuration can be any number larger than `60s`. Since AWS sends metric -data to CloudWatch in 1-minute periods, setting metricbeat module `period` less than `60s` will cause extra API requests which -means extra charges on AWS. To avoid unnecessary charges, `period` is preferred to be set to `60s` or multiples of `60s`, -such as `120s` and `180s`. +For more granular monitoring data you can enable detailed monitoring on the instance to get metrics every 1 minute. Please see +https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-cloudwatch-new.html[Enabling Detailed Monitoring] for instructions +on how to enable detailed monitoring. With detailed monitoring enabled, `period` in aws module configuration can be any number +larger than `60s`. Since AWS sends metric data to CloudWatch in 1-minute periods, setting metricbeat module `period` less +than `60s` will cause extra API requests which means extra charges on AWS. To avoid unnecessary charges, `period` is +preferred to be set to `60s` or multiples of `60s`, such as `120s` and `180s`. Here is an example of aws metricbeat module configuration: @@ -54,6 +56,9 @@ metricbeat.modules: default_region: '${AWS_REGION:us-west-1}' ---- +This module only collects metrics for EC2 instances that are in `running` state and exist more than 10 minutes to make sure +there are monitoring metrics exist in Cloudwatch already. + The AWS module comes with a predefined dashboard. For example: image::./images/metricbeat-aws-ec2-overview.png[] diff --git a/x-pack/metricbeat/module/aws/ec2/ec2.go b/x-pack/metricbeat/module/aws/ec2/ec2.go index 15d9e84114e..045c50d169d 100644 --- a/x-pack/metricbeat/module/aws/ec2/ec2.go +++ b/x-pack/metricbeat/module/aws/ec2/ec2.go @@ -42,9 +42,12 @@ func init() { // interface methods except for Fetch. type MetricSet struct { *aws.MetricSet - moduleConfig *aws.Config - awsConfig *awssdk.Config - regionsList []string + moduleConfig *aws.Config + awsConfig *awssdk.Config + regionsList []string + durationString string + periodInSec int + logger *logp.Logger } // metricIDNameMap is a translating map between createMetricDataQuery id @@ -72,6 +75,7 @@ var metricIDNameMap = map[string][]string{ // any MetricSet specific configuration options if there are any. func New(base mb.BaseMetricSet) (mb.MetricSet, error) { cfgwarn.Beta("The aws ec2 metricset is beta.") + ec2Logger := logp.NewLogger(aws.ModuleName) moduleConfig := aws.Config{} if err := base.Module().UnpackConfig(&moduleConfig); err != nil { @@ -80,7 +84,7 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) { if moduleConfig.Period == "" { err := errors.New("period is not set in AWS module config") - logp.Error(err) + ec2Logger.Error(err) } metricSet, err := aws.NewMetricSet(base) @@ -103,19 +107,38 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) { } awsConfig.Region = moduleConfig.DefaultRegion - svcEC2 := ec2.New(awsConfig) regionsList, err := getRegions(svcEC2) if err != nil { err = errors.Wrap(err, "getRegions failed") - logp.Error(err) + ec2Logger.Error(err.Error()) + } + + // Calculate duration based on period + durationString, periodSec, err := convertPeriodToDuration(moduleConfig.Period) + if err != nil { + ec2Logger.Error(err.Error()) + return nil, err + } + + // Check if period is set to be multiple of 60s or 300s + remainder300 := periodSec % 300 + remainder60 := periodSec % 60 + if remainder300 != 0 || remainder60 != 0 { + err := errors.New("period needs to be set to 60s (or a multiple of 60s) if detailed monitoring is " + + "enabled for EC2 instances or set to 300s (or a multiple of 300s) if EC2 instances has basic monitoring. " + + "To avoid data missing or extra costs, please make sure period is set correctly in config.yml") + ec2Logger.Info(err) } return &MetricSet{ - MetricSet: metricSet, - moduleConfig: &moduleConfig, - awsConfig: &awsConfig, - regionsList: regionsList, + MetricSet: metricSet, + moduleConfig: &moduleConfig, + awsConfig: &awsConfig, + regionsList: regionsList, + durationString: durationString, + periodInSec: periodSec, + logger: ec2Logger, }, nil } @@ -129,32 +152,37 @@ func (m *MetricSet) Fetch(report mb.ReporterV2) { instanceIDs, instancesOutputs, err := getInstancesPerRegion(svcEC2) if err != nil { err = errors.Wrap(err, "getInstancesPerRegion failed, skipping region "+regionName) - logp.Error(err) + m.logger.Errorf(err.Error()) report.Error(err) continue } svcCloudwatch := cloudwatch.New(*m.awsConfig) for _, instanceID := range instanceIDs { - //Calculate duration based on period - detailedMonitoring := instancesOutputs[instanceID].Monitoring.State - durationString, periodSec := convertPeriodToDuration(m.moduleConfig.Period, detailedMonitoring) init := true getMetricDataOutput := &cloudwatch.GetMetricDataOutput{NextToken: nil} for init || getMetricDataOutput.NextToken != nil { init = false - output, err := getMetricDataPerRegion(durationString, periodSec, instanceID, getMetricDataOutput.NextToken, svcCloudwatch) + output, err := getMetricDataPerRegion(m.durationString, m.periodInSec, instanceID, getMetricDataOutput.NextToken, svcCloudwatch) if err != nil { err = errors.Wrap(err, "getMetricDataPerRegion failed, skipping region "+regionName+" for instance "+instanceID) - logp.Error(err) + m.logger.Error(err.Error()) report.Error(err) continue } getMetricDataOutput.MetricDataResults = append(getMetricDataOutput.MetricDataResults, output.MetricDataResults...) } - event, err := createCloudWatchEvents(getMetricDataOutput, instanceID, instancesOutputs[instanceID], regionName) + + event, info, err := createCloudWatchEvents(getMetricDataOutput, instanceID, instancesOutputs[instanceID], regionName) + if info != "" { + m.logger.Info(info) + } + if err != nil { - report.Error(err) + m.logger.Error(err.Error()) + event.Error = err + report.Event(event) + continue } report.Event(event) } @@ -166,7 +194,7 @@ func getRegions(svc ec2iface.EC2API) (regionsList []string, err error) { req := svc.DescribeRegionsRequest(input) output, err := req.Send() if err != nil { - logp.Error(errors.Wrap(err, "Failed DescribeRegions")) + err = errors.Wrap(err, "Failed DescribeRegions") return } for _, region := range output.Regions { @@ -175,19 +203,20 @@ func getRegions(svc ec2iface.EC2API) (regionsList []string, err error) { return } -func createCloudWatchEvents(getMetricDataOutput *cloudwatch.GetMetricDataOutput, instanceID string, instanceOutput ec2.Instance, regionName string) (event mb.Event, err error) { - machineType, err := instanceOutput.InstanceType.MarshalValue() - if err != nil { - err = errors.Wrap(err, "instance.InstanceType.MarshalValue failed") - logp.Error(err) - } - +func createCloudWatchEvents(getMetricDataOutput *cloudwatch.GetMetricDataOutput, instanceID string, instanceOutput ec2.Instance, regionName string) (event mb.Event, info string, err error) { event.Service = metricsetName event.RootFields = common.MapStr{} mapOfRootFieldsResults := make(map[string]interface{}) mapOfRootFieldsResults["service.name"] = metricsetName mapOfRootFieldsResults["cloud.provider"] = metricsetName mapOfRootFieldsResults["cloud.instance.id"] = instanceID + + machineType, err := instanceOutput.InstanceType.MarshalValue() + if err != nil { + err = errors.Wrap(err, "instance.InstanceType.MarshalValue failed") + return + } + mapOfRootFieldsResults["cloud.machine.type"] = machineType mapOfRootFieldsResults["cloud.availability_zone"] = *instanceOutput.Placement.AvailabilityZone mapOfRootFieldsResults["cloud.image.id"] = *instanceOutput.ImageId @@ -195,9 +224,10 @@ func createCloudWatchEvents(getMetricDataOutput *cloudwatch.GetMetricDataOutput, resultRootFields, err := eventMapping(mapOfRootFieldsResults, schemaRootFields) if err != nil { - err = errors.Wrap(err, "Error trying to apply schema in AWS EC2 metricbeat module.") - logp.Error(err) + err = errors.Wrap(err, "Error trying to apply schema schemaRootFields in AWS EC2 metricbeat module.") + return } + event.RootFields = resultRootFields mapOfMetricSetFieldResults := make(map[string]interface{}) for _, output := range getMetricDataOutput.MetricDataResults { @@ -208,12 +238,17 @@ func createCloudWatchEvents(getMetricDataOutput *cloudwatch.GetMetricDataOutput, mapOfMetricSetFieldResults[metricKey[0]] = fmt.Sprint(output.Values[0]) } + if len(mapOfMetricSetFieldResults) <= 3 { + info = "Missing Cloudwatch data for instance " + instanceID + ". This is expected for a new instance during the " + + "first data collection. If this shows up multiple times, please recheck the period setting in config." + return + } + resultMetricSetFields, err := eventMapping(mapOfMetricSetFieldResults, schemaMetricSetFields) if err != nil { - err = errors.Wrap(err, "Error trying to apply schema in AWS EC2 metricbeat module.") - logp.Error(err) + err = errors.Wrap(err, "Error trying to apply schema schemaMetricSetFields in AWS EC2 metricbeat module.") + return } - event.RootFields = resultRootFields event.MetricSetFields = resultMetricSetFields return } @@ -236,69 +271,44 @@ func getInstancesPerRegion(svc ec2iface.EC2API) (instanceIDs []string, instances req := svc.DescribeInstancesRequest(describeInstanceInput) output, err := req.Send() if err != nil { - logp.Error(errors.Wrap(err, "Error DescribeInstances")) + err = errors.Wrap(err, "Error DescribeInstances") return nil, nil, err } for _, reservation := range output.Reservations { for _, instance := range reservation.Instances { - instanceID := *instance.InstanceId - instanceIDs = append(instanceIDs, instanceID) - instancesOutputs[instanceID] = instance + instanceIDs = append(instanceIDs, *instance.InstanceId) + instancesOutputs[*instance.InstanceId] = instance } } } return } -func convertPeriodToDuration(period string, detailedMonitoring ec2.MonitoringState) (duration string, periodInSeconds int) { +func convertPeriodToDuration(period string) (string, int, error) { // Amazon EC2 sends metrics to Amazon CloudWatch with 5-minute default frequency. // If detailed monitoring is enabled, then data will be available in 1-minute period. // Set starttime double the default frequency earlier than the endtime in order to make sure // GetMetricDataRequest gets the latest data point for each metric. numberPeriod, err := strconv.Atoi(period[0 : len(period)-1]) if err != nil { - logp.Error(errors.Wrap(err, "Error converting string to int. Use default duration instead.")) - // If failed converting string to int, then set default duration to "-600s" with basic monitoring and "-120s" with - // detailed monitoring. - numberPeriod = 300 - if detailedMonitoring == ec2.MonitoringStateEnabled { - numberPeriod = 60 - } - duration = "-" + strconv.Itoa(numberPeriod*2) + "s" - periodInSeconds = numberPeriod - return duration, periodInSeconds + return "", 0, err } unitPeriod := period[len(period)-1:] - // if detailed monitoring is enabled, then period can be larger or equal than 1min. - // if detailed monitoring is disabled, then period can be larger or equal than 5min. switch unitPeriod { case "s": - if detailedMonitoring == ec2.MonitoringStateDisabled && numberPeriod < 300 { - numberPeriod = 300 - } else if detailedMonitoring == ec2.MonitoringStateEnabled && numberPeriod < 60 { - numberPeriod = 60 - } - duration = "-" + strconv.Itoa(numberPeriod*2) + unitPeriod - periodInSeconds = numberPeriod + duration := "-" + strconv.Itoa(numberPeriod*2) + unitPeriod + return duration, numberPeriod, nil case "m": - if detailedMonitoring == ec2.MonitoringStateDisabled && numberPeriod < 5 { - numberPeriod = 5 - } else if detailedMonitoring == ec2.MonitoringStateEnabled && numberPeriod < 1 { - numberPeriod = 1 - } - duration = "-" + strconv.Itoa(numberPeriod*2) + unitPeriod - periodInSeconds = numberPeriod * 60 + duration := "-" + strconv.Itoa(numberPeriod*2) + unitPeriod + periodInSec := numberPeriod * 60 + return duration, periodInSec, nil default: - numberPeriod = 300 - if detailedMonitoring == ec2.MonitoringStateEnabled { - numberPeriod = 60 - } - duration = "-" + strconv.Itoa(numberPeriod*2) + "s" - periodInSeconds = numberPeriod + err = errors.New("invalid period in config. Please reset period in config") + duration := "-" + strconv.Itoa(numberPeriod*2) + "s" + return duration, numberPeriod, err } - return } func getMetricDataPerRegion(durationString string, periodInSec int, instanceID string, nextToken *string, svc cloudwatchiface.CloudWatchAPI) (*cloudwatch.GetMetricDataOutput, error) { diff --git a/x-pack/metricbeat/module/aws/ec2/ec2_test.go b/x-pack/metricbeat/module/aws/ec2/ec2_test.go index 3ff6c43a9cb..c563ed18567 100644 --- a/x-pack/metricbeat/module/aws/ec2/ec2_test.go +++ b/x-pack/metricbeat/module/aws/ec2/ec2_test.go @@ -10,16 +10,17 @@ import ( "fmt" "testing" - "github.com/elastic/beats/metricbeat/mb" - "github.com/elastic/beats/x-pack/metricbeat/module/aws" - awssdk "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/cloudwatch" "github.com/aws/aws-sdk-go-v2/service/cloudwatch/cloudwatchiface" "github.com/aws/aws-sdk-go-v2/service/ec2" "github.com/aws/aws-sdk-go-v2/service/ec2/ec2iface" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" + "github.com/elastic/beats/metricbeat/mb" + "github.com/elastic/beats/x-pack/metricbeat/module/aws" + "github.com/elastic/beats/libbeat/common" ) @@ -50,15 +51,13 @@ func (m *MockEC2Client) DescribeRegionsRequest(input *ec2.DescribeRegionsInput) } func (m *MockEC2Client) DescribeInstancesRequest(input *ec2.DescribeInstancesInput) ec2.DescribeInstancesRequest { - monitoringState := ec2.Monitoring{ - State: ec2.MonitoringState(ec2.MonitoringStateDisabled), - } instance := ec2.Instance{ InstanceId: awssdk.String("i-123"), InstanceType: ec2.InstanceTypeT2Medium, - Placement: &ec2.Placement{AvailabilityZone: awssdk.String("us-west-1a")}, - ImageId: awssdk.String("image-123"), - Monitoring: &monitoringState, + Placement: &ec2.Placement{ + AvailabilityZone: awssdk.String("us-west-1a"), + }, + ImageId: awssdk.String("image-123"), } return ec2.DescribeInstancesRequest{ Request: &awssdk.Request{ @@ -72,17 +71,45 @@ func (m *MockEC2Client) DescribeInstancesRequest(input *ec2.DescribeInstancesInp } func (m *MockCloudWatchClient) GetMetricDataRequest(input *cloudwatch.GetMetricDataInput) cloudwatch.GetMetricDataRequest { - id := "cpu1" - label := "CPUUtilization" - value := 0.25 + id1 := "cpu1" + label1 := "CPUUtilization" + value1 := 0.25 + + id2 := "status1" + label2 := "StatusCheckFailed" + value2 := 0.0 + + id3 := "status2" + label3 := "StatusCheckFailed_System" + value3 := 0.0 + + id4 := "status3" + label4 := "StatusCheckFailed_Instance" + value4 := 0.0 + return cloudwatch.GetMetricDataRequest{ Request: &awssdk.Request{ Data: &cloudwatch.GetMetricDataOutput{ MetricDataResults: []cloudwatch.MetricDataResult{ { - Id: &id, - Label: &label, - Values: []float64{value}, + Id: &id1, + Label: &label1, + Values: []float64{value1}, + }, + { + Id: &id2, + Label: &label2, + Values: []float64{value2}, + }, + { + Id: &id3, + Label: &label3, + Values: []float64{value3}, + }, + { + Id: &id4, + Label: &label4, + Values: []float64{value4}, }, }, }, @@ -125,61 +152,57 @@ func TestGetMetricDataPerRegion(t *testing.T) { fmt.Println("failed getMetricDataPerRegion: ", err) t.FailNow() } - assert.Equal(t, 1, len(getMetricDataOutput.MetricDataResults)) + assert.Equal(t, 4, len(getMetricDataOutput.MetricDataResults)) assert.Equal(t, "cpu1", *getMetricDataOutput.MetricDataResults[0].Id) assert.Equal(t, "CPUUtilization", *getMetricDataOutput.MetricDataResults[0].Label) assert.Equal(t, 0.25, getMetricDataOutput.MetricDataResults[0].Values[0]) + + assert.Equal(t, "status1", *getMetricDataOutput.MetricDataResults[1].Id) + assert.Equal(t, "StatusCheckFailed", *getMetricDataOutput.MetricDataResults[1].Label) + assert.Equal(t, 0.0, getMetricDataOutput.MetricDataResults[1].Values[0]) + + assert.Equal(t, "status2", *getMetricDataOutput.MetricDataResults[2].Id) + assert.Equal(t, "StatusCheckFailed_System", *getMetricDataOutput.MetricDataResults[2].Label) + assert.Equal(t, 0.0, getMetricDataOutput.MetricDataResults[2].Values[0]) + + assert.Equal(t, "status3", *getMetricDataOutput.MetricDataResults[3].Id) + assert.Equal(t, "StatusCheckFailed_Instance", *getMetricDataOutput.MetricDataResults[3].Label) + assert.Equal(t, 0.0, getMetricDataOutput.MetricDataResults[3].Values[0]) } -func TestConvertPeriodToDurationWithDetailedMonitoring(t *testing.T) { +func TestConvertPeriodToDuration(t *testing.T) { period1 := "300s" - duration1, periodSec1 := convertPeriodToDuration(period1, "enabled") + duration1, periodSec1, err := convertPeriodToDuration(period1) + assert.NoError(t, nil, err) assert.Equal(t, "-600s", duration1) assert.Equal(t, 300, periodSec1) period2 := "30ss" - duration2, periodSec2 := convertPeriodToDuration(period2, "enabled") - assert.Equal(t, "-120s", duration2) - assert.Equal(t, 60, periodSec2) + duration2, periodSec2, err := convertPeriodToDuration(period2) + expectedErr := errors.New("Invaid period in config. Please reset period in config.") + assert.Error(t, expectedErr, err) + assert.Equal(t, "", duration2) + assert.Equal(t, 0, periodSec2) period3 := "10m" - duration3, periodSec3 := convertPeriodToDuration(period3, "enabled") + duration3, periodSec3, err := convertPeriodToDuration(period3) + assert.NoError(t, nil, err) assert.Equal(t, "-20m", duration3) assert.Equal(t, 600, periodSec3) period4 := "30s" - duration4, periodSec4 := convertPeriodToDuration(period4, "enabled") - assert.Equal(t, "-120s", duration4) - assert.Equal(t, 60, periodSec4) + duration4, periodSec4, err := convertPeriodToDuration(period4) + assert.NoError(t, nil, err) + assert.Equal(t, "-60s", duration4) + assert.Equal(t, 30, periodSec4) period5 := "60s" - duration5, periodSec5 := convertPeriodToDuration(period5, "enabled") + duration5, periodSec5, err := convertPeriodToDuration(period5) + assert.NoError(t, nil, err) assert.Equal(t, "-120s", duration5) assert.Equal(t, 60, periodSec5) } -func TestConvertPeriodToDurationWithBasicMonitoring(t *testing.T) { - period1 := "300s" - duration1, periodSec1 := convertPeriodToDuration(period1, "disabled") - assert.Equal(t, "-600s", duration1) - assert.Equal(t, 300, periodSec1) - - period2 := "30ss" - duration2, periodSec2 := convertPeriodToDuration(period2, "disabled") - assert.Equal(t, "-600s", duration2) - assert.Equal(t, 300, periodSec2) - - period3 := "10m" - duration3, periodSec3 := convertPeriodToDuration(period3, "disabled") - assert.Equal(t, "-20m", duration3) - assert.Equal(t, 600, periodSec3) - - period5 := "60s" - duration5, periodSec5 := convertPeriodToDuration(period5, "disabled") - assert.Equal(t, "-600s", duration5) - assert.Equal(t, 300, periodSec5) -} - func TestCreateCloudWatchEvents(t *testing.T) { mockModuleConfig := aws.Config{ Period: "300s", @@ -212,21 +235,22 @@ func TestCreateCloudWatchEvents(t *testing.T) { assert.Equal(t, "i-123", instanceID) svcCloudwatchMock := &MockCloudWatchClient{} - detailedMonitoring := instancesOutputs[instanceID].Monitoring.State //Calculate duration based on period - durationString, periodSec := convertPeriodToDuration(mockModuleConfig.Period, detailedMonitoring) + durationString, periodSec, err := convertPeriodToDuration(mockModuleConfig.Period) + assert.NoError(t, nil, err) assert.Equal(t, "-600s", durationString) assert.Equal(t, 300, periodSec) getMetricDataOutput, err := getMetricDataPerRegion(durationString, periodSec, instanceID, nil, svcCloudwatchMock) assert.NoError(t, err) - assert.Equal(t, 1, len(getMetricDataOutput.MetricDataResults)) + assert.Equal(t, 4, len(getMetricDataOutput.MetricDataResults)) assert.Equal(t, "cpu1", *getMetricDataOutput.MetricDataResults[0].Id) assert.Equal(t, "CPUUtilization", *getMetricDataOutput.MetricDataResults[0].Label) assert.Equal(t, 0.25, getMetricDataOutput.MetricDataResults[0].Values[0]) - event, err := createCloudWatchEvents(getMetricDataOutput, instanceID, instancesOutputs[instanceID], mockModuleConfig.DefaultRegion) + event, info, err := createCloudWatchEvents(getMetricDataOutput, instanceID, instancesOutputs[instanceID], mockModuleConfig.DefaultRegion) assert.NoError(t, err) + assert.Equal(t, "", info) assert.Equal(t, expectedEvent.RootFields, event.RootFields) assert.Equal(t, expectedEvent.MetricSetFields["cpu"], event.MetricSetFields["cpu"]) }