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

Add support for Cloud Watch Alarm Tags #6598

Merged
merged 8 commits into from Apr 25, 2021
Merged

Conversation

gswai123
Copy link
Contributor

@gswai123 gswai123 commented Apr 1, 2021

This enhancement uses the augment function to add Tags to CloudWatch Alarm resources in Custodian, allowing users to filter by Alarm Tags using the Value Filter. The PR also includes a "Tag" action for CloudWatch alarms which allows users to add tags to alarm resources.

Issue Link: #6454

@gswai123 gswai123 requested a review from kapilt as a code owner April 1, 2021 19:37
@gswai123 gswai123 marked this pull request as draft April 1, 2021 19:42
Copy link
Collaborator

@kapilt kapilt left a comment

Choose a reason for hiding this comment

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

lgtm, thanks

@kapilt kapilt marked this pull request as ready for review April 1, 2021 20:43
}


@Alarm.action_registry.register('tag')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we may also need remove-tag, mark-for-op and marked-for-op. Would it be better to just use universal_taggble?

@kapilt
Copy link
Collaborator

kapilt commented Apr 1, 2021 via email

@PratMis
Copy link
Collaborator

PratMis commented Apr 5, 2021

Only thing I can see here is that, Tags are not getting captured with source: config. AWS claims to be capturing the tags re: config cmdb(https://github.com/awslabs/aws-config-resource-schema/blob/e7586bdfd24f31427ccc8df21d760670bbcd1d93/config/properties/resource-types/AWS::CloudWatch::Alarm.properties.json), but it doesn't show up in reality

Response captured with config as source:

{'resourceId': 'c7n-test-alarm-tags-filter', 'configuration': {'Statistic': 'Average', 'Period': 3600.0, 'MetricName': 'CPUUtilization', 'ComparisonOperator': 'GreaterThanThreshold', 'AlarmConfigurationUpdatedTimestamp': datetime.datetime(2021, 3, 24, 18, 1, 32, 583000, tzinfo=tzutc()), 'AlarmName': 'c7n-test-alarm-tags-filter', 'Threshold': 10.0, 'Okactions': [], 'InsufficientDataActions': [], 'EvaluationPeriods': 5.0, 'ActionsEnabled': True, 'Namespace': 'AWS/EC2', 'Metrics': [], 'AlarmActions': [], 'AlarmArn': 'arn:aws:cloudwatch:us-east-1:644160558196:alarm:c7n-test-alarm-tags-filter', 'Dimensions': []}, 'supplementaryConfiguration': {}}

vs boto response:

[
  {
    "AlarmName": "c7n-test-alarm-tags-filter",
    "AlarmArn": "arn:aws:cloudwatch:us-east-1:644160558196:alarm:c7n-test-alarm-tags-filter",
    "AlarmConfigurationUpdatedTimestamp": "2021-03-24T18:01:32.583000+00:00",
    "ActionsEnabled": true,
    "OKActions": [],
    "AlarmActions": [],
    "InsufficientDataActions": [],
    "StateValue": "INSUFFICIENT_DATA",
    "StateReason": "Unchecked: Initial alarm creation",
    "StateUpdatedTimestamp": "2021-03-24T18:01:32.583000+00:00",
    "MetricName": "CPUUtilization",
    "Namespace": "AWS/EC2",
    "Statistic": "Average",
    "Dimensions": [],
    "Period": 3600,
    "EvaluationPeriods": 5,
    "Threshold": 10.0,
    "ComparisonOperator": "GreaterThanThreshold",
    "Tags": [
      {
        "Key": "OwnerName",
policies:
        "Value": "SomeName"
      },
      {
        "Key": "pratyush",
        "Value": "test"
      },
      {
        "Key": "some-tag",
        "Value": "some-value"
      }
    ]
  }
]

@kapilt
Copy link
Collaborator

kapilt commented Apr 5, 2021

re config, the last few prs on config support, i'm noticing there's pretty large variance on the number of important resource data they fail to capture. any issues with config data capture, are definitely something to take up directly with the service team via tam, its not something we can really handle. we can note those deficiets in the resource doc string is probably the best we can do, but its probably something that can age out.

@PratMis
Copy link
Collaborator

PratMis commented Apr 5, 2021

re config, the last few prs on config support, i'm noticing there's pretty large variance on the number of important resource data they fail to capture. any issues with config data capture, are definitely something to take up directly with the service team via tam, its not something we can really handle. we can note those deficiets in the resource doc string is probably the best we can do, but its probably something that can age out.

Yup, making a note of these so that I can bring it up to our TAM. Thanks @kapilt

Copy link
Collaborator

@kapilt kapilt left a comment

Choose a reason for hiding this comment

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

lgtm, thanks

@kapilt kapilt merged commit a4394b4 into cloud-custodian:master Apr 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants