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

metrics: Validate parameters for a Metric.dimensions #3116

Closed
4 tasks
cybergoof opened this issue Jun 28, 2019 · 1 comment
Closed
4 tasks

metrics: Validate parameters for a Metric.dimensions #3116

cybergoof opened this issue Jun 28, 2019 · 1 comment
Assignees
Labels
@aws-cdk/aws-cloudwatch Related to Amazon CloudWatch effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@cybergoof
Copy link

  • I'm submitting a ...

    • πŸͺ² bug report
    • [ x ] πŸš€ feature request
    • πŸ“š construct library gap
    • ☎️ security issue or vulnerability => Please see policy
    • ❓ support request => Please see note at the top of this template.
  • What is the current behavior?
    When creating a new Metric for a CloudWatch dashboard, an 'undefined' is accepted for the dimensions value, but CloudFormation will reject it.

  • What is the expected behavior (or behavior of feature suggested)?
    CDK should evaluate the name/value of a dimension and reject invalid values.

  • What is the motivation / use case for changing the behavior or adding this feature?
    Input validation at the CDK build process is better than waiting for CloudFormation.

  • Please tell us about your environment:

    • CDK CLI Version: 0.35.0
    • Module Version: 0.35.0
    • OS: [all ]
    • Language: [TypeScript]
  • Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. associated pull-request, stackoverflow, gitter, etc)

        const dashboard = new cloudwatch.Dashboard(this, name, { dashboardName: name});
        const newMetric = new Metric({
            dimensions: {
                AutoScalingGroupName: undefined,
            },
            metricName: "CPUUtilization",
            namespace: "AWS/EC2",
            statistic: "avg",
        });
        dashboard.addWidgets(
            new GraphWidget({
                left: [
                    newMetric,
                ],
                title: `Average CPU`,
           }),
        );
    }

The above will build. But the following error is given from CloudFormation

The dashboard body is invalid, there are 1 validation errors: [ { "dataPath": "/widgets/0/properties/metrics/0", "message": "Should NOT have more than 3 items" } ] (Service: AmazonCloudWatch; Status Code: 400; Error Code: InvalidParameterInput; 
@cybergoof cybergoof added the needs-triage This issue or PR still needs to be triaged. label Jun 28, 2019
@NGL321 NGL321 added bug This issue is a bug. @aws-cdk/aws-cloudformation Related to AWS CloudFormation @aws-cdk/aws-cloudwatch Related to Amazon CloudWatch and removed needs-triage This issue or PR still needs to be triaged. labels Jul 1, 2019
@eladb eladb self-assigned this Aug 12, 2019
@eladb eladb assigned rix0rrr and unassigned eladb Sep 3, 2019
@rix0rrr rix0rrr added feature-request A feature should be added or improved. and removed bug This issue is a bug. labels Sep 16, 2019
@rix0rrr rix0rrr added the effort/small Small work item – less than a day of effort label Jan 23, 2020
@rix0rrr rix0rrr changed the title Evaluate parameters for a Metric.dimensions{} metrics: Validate parameters for a Metric.dimensions Jan 23, 2020
@eladb eladb removed the @aws-cdk/aws-cloudformation Related to AWS CloudFormation label Mar 19, 2020
@rix0rrr rix0rrr added good first issue Related to contributions. See CONTRIBUTING.md p2 labels Aug 12, 2020
OksanaH added a commit to OksanaH/aws-cdk that referenced this issue Apr 25, 2021
OksanaH added a commit to OksanaH/aws-cdk that referenced this issue Apr 25, 2021
@mergify mergify bot closed this as completed in 4a24d61 May 7, 2021
@github-actions
Copy link

github-actions bot commented May 7, 2021

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

john-tipper pushed a commit to john-tipper/aws-cdk that referenced this issue May 10, 2021
…ws#3116) (aws#14365)

As per aws#3116, the changes in this PR validate metric dimension values (length, and checking if the value is null or undefined) and throw errors if the values are not valid. I've also corrected a comment in the metric-types.ts to use the correct method name
eladb pushed a commit that referenced this issue May 11, 2021
* chore(cloudfront): remove the use of calculateFunctionHash (#14583)

`calculateFunctionHash()` was used to compute the 'refresh token' of the
custom resource for the EdgeFunction construct.

This method is private to the lambda module and is deemed to be changed.
Instead, use the lambda function version's logical id.

The logical id of the version includes computing the function hash (among
others) and is a more reliable determinant of whether the underlying
function version changed.


----

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

* feat(cloudwatch): validate parameters for a metric dimensions (closes #3116) (#14365)

As per #3116, the changes in this PR validate metric dimension values (length, and checking if the value is null or undefined) and throw errors if the values are not valid. I've also corrected a comment in the metric-types.ts to use the correct method name

* feat(appmesh): change HealthChecks to use protocol-specific union-like classes (#14432)

BREAKING CHANGE: HealthChecks require use of static factory methods

fixes #11640

----

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

Co-authored-by: Niranjan Jayakar <nija@amazon.com>
Co-authored-by: OksanaH <34384274+OksanaH@users.noreply.github.com>
Co-authored-by: Dominic Fezzie <fezzid@amazon.com>
Co-authored-by: Elad Ben-Israel <benisrae@amazon.com>
eladb pushed a commit that referenced this issue May 12, 2021
* chore(cloudfront): remove the use of calculateFunctionHash (#14583)

`calculateFunctionHash()` was used to compute the 'refresh token' of the
custom resource for the EdgeFunction construct.

This method is private to the lambda module and is deemed to be changed.
Instead, use the lambda function version's logical id.

The logical id of the version includes computing the function hash (among
others) and is a more reliable determinant of whether the underlying
function version changed.


----

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

* feat(cloudwatch): validate parameters for a metric dimensions (closes #3116) (#14365)

As per #3116, the changes in this PR validate metric dimension values (length, and checking if the value is null or undefined) and throw errors if the values are not valid. I've also corrected a comment in the metric-types.ts to use the correct method name

* feat(appmesh): change HealthChecks to use protocol-specific union-like classes (#14432)

BREAKING CHANGE: HealthChecks require use of static factory methods

fixes #11640

----

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

* feat(msk): Cluster L2 Construct (#9908)

L2 Construct for a MSK Cluster. 

I wrote this for internal use and thought I'd share it. I tried to follow the [example resource](https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/example-construct-library/lib/example-resource.ts) and [design guidelines](https://github.com/aws/aws-cdk/blob/master/DESIGN_GUIDELINES.md) as much as I could. Default properties were chosen either based on defaults when creating a cluster in the console or defaults set from CloudFormation.

Closes #9603

----

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

* feat(kms): allow specifying key spec and key usage (#14478)

This allows specifying key spec and key usage, so you can create asymmetric keys.

closes #5639


----

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

* chore: add `@types/jest` to a package that was missing it (#14609)

----

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

* chore: issue template for bugs to have an SSCCE example (#14615)

Ask customers to provide reproducible code snippets
to reduce the amount of triage time required.

----

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

* fix(cli): synth fails if there was an error when synthesizing the stack (#14613)

All stacks created inside a pipeline stage will be flagged for validation. After synth is done, the CLI will validate all flagged stacks plus the stacks that were explicitly specified.

----

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

* chore: annotate `aws-lambda-go` with `docker` requirement (#14618)

The `nozem` build tool needs to know that `docker` is required to
build/test this particular package.


----

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

* feat(elbv2): preserveClientIp for NetworkTargetGroup (#14589)

Allows users to configure client IP preservation for network target groups.

See: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-elasticloadbalancingv2-targetgroup-targetgroupattribute.html

----

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

* chore: npm-check-updates && yarn upgrade (#14620)

Ran npm-check-updates and yarn upgrade to keep the `yarn.lock` file up-to-date.

* chore(mergify): add @BenChaimberg to team roster

* chore(release): 1.103.0

* chore: mark "otaviomacedo" as core contributor (#14619)

Co-authored-by: Otavio Macedo <otaviomacedo@protonmail.com>

* chore(cli): add npm command to upgrade notice (#14621)

A colleague had to go look up the command to update the CDK CLI. It occurred to me that is probably common with developers who don't work with NPM on a daily basis, such as anyone who isn't developing in TypeScript or JavaScript.

Put the necessary command right in the upgrade notice.

----

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

* fix(lambda): custom resource fails to connect to efs filesystem (#14431)

Fixes: #14430

----

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

* feat(cfnspec): cloudformation spec v35.2.0 (#14610)

* feat: cloudformation spec v35.2.0

* add spec patches

Co-authored-by: AWS CDK Team <aws-cdk@amazon.com>
Co-authored-by: Elad Ben-Israel <benisrae@amazon.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* fix(lambda-nodejs): handler filename missing from error message (#14564)


----

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

* chore(build): clarify prerequisites in CONTRIBUTING and verify before build (#14642)

The build script at the top level of the repository performs a prerequisites check before beginning the build. This verification is not complete as it does not include the required checks for .NET and Python executables. Further, the prerequisites documentation in the contributing guide does not note the Docker requirement.

----

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

Co-authored-by: Niranjan Jayakar <nija@amazon.com>
Co-authored-by: OksanaH <34384274+OksanaH@users.noreply.github.com>
Co-authored-by: Dominic Fezzie <fezzid@amazon.com>
Co-authored-by: Curtis <curtis.eppel@cultureamp.com>
Co-authored-by: Shinya Tsuda <shinya@dacci.org>
Co-authored-by: Rico Huijbers <rix0rrr@gmail.com>
Co-authored-by: Otavio Macedo <otaviomacedo@protonmail.com>
Co-authored-by: Griffin Byatt <byatt.griffin@gmail.com>
Co-authored-by: Ben Chaimberg <chaimber@amazon.com>
Co-authored-by: AWS CDK Team <aws-cdk@amazon.com>
Co-authored-by: Mitchell Valine <valinm@amazon.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Jerry Kindall <52084730+Jerry-AWS@users.noreply.github.com>
Co-authored-by: Daniel Neilson <53624638+ddneilson@users.noreply.github.com>
Co-authored-by: Elad Ben-Israel <benisrae@amazon.com>
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
…ws#3116) (aws#14365)

As per aws#3116, the changes in this PR validate metric dimension values (length, and checking if the value is null or undefined) and throw errors if the values are not valid. I've also corrected a comment in the metric-types.ts to use the correct method name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-cloudwatch Related to Amazon CloudWatch effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
Development

No branches or pull requests

4 participants