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

Jump to bottom

feat(cloudwatch): support for metric math #5582

Merged
merged 48 commits into from
Jan 3, 2020
Merged

feat(cloudwatch): support for metric math #5582

merged 48 commits into from
Jan 3, 2020

Conversation

AhmedSedek
Copy link
Contributor

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@AhmedSedek AhmedSedek changed the title Dummy change Base PR for cloudwatch expressions Dec 30, 2019
@mergify
Copy link
Contributor

mergify bot commented Dec 30, 2019

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

@rix0rrr rix0rrr self-assigned this Dec 30, 2019
@rix0rrr rix0rrr changed the title Base PR for cloudwatch expressions feat(cloudwatch): add support for metric math Dec 30, 2019
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@eladb
Copy link
Contributor

eladb commented Dec 30, 2019

?

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

rix0rrr
rix0rrr reviewed Dec 30, 2019
View reviewed changes
packages/@aws-cdk/aws-cloudwatch/lib/metric.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-cloudwatch/lib/metric.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-cloudwatch/lib/metric.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-cloudwatch/lib/metric.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-cloudwatch/lib/metric.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-cloudwatch/lib/metric.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-cloudwatch/lib/metric.ts Outdated Show resolved Hide resolved
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

rix0rrr and others added 2 commits January 2, 2020 17:42
@rix0rrr @RomainMuller
Update packages/@aws-cdk/aws-applicationautoscaling/lib/step-scaling-…
8a2a67d 
…policy.ts

Co-Authored-By: Romain Marcadier-Muller <romain.muller@telecomnancy.net>
@rix0rrr @RomainMuller
Update packages/@aws-cdk/aws-applicationautoscaling/lib/target-tracki…
58f63b3 
…ng-scaling-policy.ts

Co-Authored-By: Romain Marcadier-Muller <romain.muller@telecomnancy.net>
@mergify mergify bot dismissed RomainMuller’s stale review January 2, 2020 16:43

Pull request has been modified.

rix0rrr and others added 2 commits January 2, 2020 17:48
@rix0rrr @RomainMuller
Update packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts
70bd8bb 
Co-Authored-By: Romain Marcadier-Muller <romain.muller@telecomnancy.net>
@rix0rrr
Revert quotes change
6c0ee40
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

eladb
eladb reviewed Jan 3, 2020
View reviewed changes
Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

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

  • Change title to "feat(cloudwatch): metric math"
  • Add PR description which briefly describes what's changed

packages/@aws-cdk/aws-cloudwatch/README.md Outdated

```ts
const metric = new Metric({
namespace: 'MyNamespace',
Copy link
Contributor

Choose a reason for hiding this comment

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

Use 2 space indentation

packages/@aws-cdk/aws-cloudwatch/README.md Outdated
Comment on lines 51 to 56
expression: "errors + faults",
usingMetrics: {
errors: myConstruct.metricErrors(),
faults: myConstruct.metricFaults(),
}
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think it might be possible to also provide the option to simply embed metric objects (via tokens) into the expression like so:

expression: `${myConstruct.metricErrors()} + ${myConstruct.metricFaults()}`

I feel this might be a much more intuitive API for most users... And less error prone.

@AhmedSedek AhmedSedek requested a review from skinny85 as a code owner January 3, 2020 09:47
@rix0rrr rix0rrr requested a review from RomainMuller January 3, 2020 09:47
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@rix0rrr rix0rrr mentioned this pull request Jan 3, 2020
Closed
5 tasks
@rix0rrr
Copy link
Contributor

rix0rrr commented Jan 3, 2020

Note that this PR fixes #1077, fixes #5449, fixes #5261, fixes #4716.

@rix0rrr rix0rrr merged commit a7f189e into aws:master Jan 3, 2020
@rix0rrr rix0rrr deleted the cloudwatch-expressions branch January 3, 2020 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eladb eladb eladb left review comments

@rix0rrr rix0rrr rix0rrr approved these changes

@RomainMuller RomainMuller RomainMuller approved these changes

@shivlaks shivlaks Awaiting requested review from shivlaks

@nija-at nija-at Awaiting requested review from nija-at

@skinny85 skinny85 Awaiting requested review from skinny85

Assignees

@rix0rrr rix0rrr

Labels
pr/do-not-merge This PR should not be merged at this time.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@AhmedSedek @aws-cdk-automation @eladb @rix0rrr @RomainMuller