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

Copying Stack tags to the Datadog monitor (#226) #227

Closed
wants to merge 1 commit into from

Conversation

elruwen
Copy link
Contributor

@elruwen elruwen commented Nov 9, 2022

Cloudformation supports tags at a stack level. They are normally passed on to all Cloudformation resources (some resources don't support that yet...). Datadog monitors got tags, too. With this change, the Cloudformation Stack tags are copied to the Datadog monitor. Additionally, the AWS autogenerated tags like stack name and logical id are also set. If a tag is already set explicitly at a monitor, the monitor value is being used. Since Datadog uses a : to separate key and value, all : are replaced with _.

In order to maintain backwards compatibility, this feature is switched off by default. It can be enabled account-wide via the type configuration, the same way Datadog credentials are being set.

Requirements for Contributing to this repository

  • Fill out the template below. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • The pull request must only fix one issue, or add one feature, at the time.
  • The pull request must update the test suite to demonstrate the changed functionality.
  • After you create the pull request, all status checks must be pass before a maintainer reviews your contribution. For more details, please see CONTRIBUTING.

What does this PR do?

This PR fixes #226

Description of the Change

See commit message

Alternate Designs

No alternate design.

Possible Drawbacks

There should be none, since the feature is implemented in a backwards-compatible way.

Verification Process

  • created a monitor via cfn with version 4.4.0
  • deploy my new version
  • updated the monitor text via cfn -> tags showed up
  • added to the stack a new monitor -> tags showed up

Additional Notes

I didn't add a test because there were none and I didn't work with python cloudformation resources before.

Release Notes

Copying Stack tags to the Datadog monitor (disabled by default)

Review checklist (to be filled by reviewers)

  • Feature or bug fix MUST have appropriate tests (unit, integration, etc...)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have one changelog/ label attached. If applicable it should have the backward-incompatible label attached.
  • PR should not have do-not-merge/ label attached.
  • If Applicable, issue must have kind/ and severity/ labels attached at least.

Cloudformation supports tags at a stack level. They are normally passed on to all Cloudformation resources (some resources don't support that yet...). Datadog monitors got tags, too. With this change, the Cloudformation Stack tags are copied to the Datadog monitor. Additionally, the AWS autogenerated tags like stack name and logical id are also set. If a tag is already set explicitly at a monitor, the monitor value is being used.
Since Datadog uses a : to separate key and value, all : are replaced with _.

In order to maintain backwards compatibility, this feature is switched off by default. It can be enabled account-wide via the type configuration, the same way Datadog credentials are being set.
@elruwen elruwen requested a review from a team as a code owner November 9, 2022 22:58
@skarimo
Copy link
Member

skarimo commented Dec 8, 2022

Hi, thanks for this PR. After a bit of testing we reverted this change. We realized that systemTags and others are not passed within the request context during the execution of the read_handler. This complicates things a bit further because it is difficult to properly handle resource drift with this implementation.

I am going to close this PR however, we are evaluating alternative approaches such as the native tagging support for cloudformation tags, caveat being, this will require a schema change for the tags attribute and that we are blocked waiting on new cloudformation-cli-python-plugin release.

@skarimo skarimo closed this Dec 8, 2022
@elruwen
Copy link
Contributor Author

elruwen commented Dec 13, 2022

I personally would be happy with the drift, but I can understand if you are not.

@carlosmesas
Copy link

@skarimo I agree with @elruwen , I think a simple integration could suffice in many cases.

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.

Monitors: Pass on Stack tags
3 participants