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

(cfn2ts): "metric" function does not work cross account/region resources #18951

Open
jonnekaunisto opened this issue Feb 12, 2022 · 7 comments
Open
Labels
@aws-cdk/aws-sqs Related to Amazon Simple Queue Service effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@jonnekaunisto
Copy link
Contributor

jonnekaunisto commented Feb 12, 2022

What is the problem?

When using the metric function on an Interface of a resource in another region or account, a metric is returned with no account or region information, which means that the metric does not properly work on alarms or dashboards.

Reproduction Steps

For example when doing something like this:

const queue = Queue.fromQueueArn(this, 'crossAccountQueue', queueArnFromAnotherAccount);
queue.metricNumberOfMessagesSent()

What did you expect to happen?

The metric should be for the queue on the other account.

The source on dashboard should look like this ideally:

 [ "AWS/SQS", "NumberOfMessagesSent", "QueueName", "SomeQueue", { "label": "SYD", "accountId": "281919751090", "region": "ap-southeast-2", "stat": "Sum" } ],

What actually happened?

If you add the following metric to your dashboard it will try to get the metric from the account that the dashboard is in instead of where the queue is.

The source on dashboard looked like this:

 [ "AWS/SQS", "NumberOfMessagesSent", "QueueName", "SomeQueue", { "label": "SYD",  "stat": "Sum" } ],

CDK CLI Version

1.143.0

Framework Version

1.139.0

Node.js Version

16

OS

Mac OS

Language

Typescript

Language Version

3.6.4

Other information

The way the metric function is implemented currently is by using code generation here: https://github.com/aws/aws-cdk/blob/3721358fa1501e42b3514b8a8f15f05c9615f149/tools/%40aws-cdk/cfn2ts/lib/augmentation-generator.ts

The following code is generated by that:

function_base_1.FunctionBase.prototype.metric = function (metricName, props) {
    return new cloudwatch.Metric({
        namespace: 'AWS/Lambda',
        metricName,
        dimensionsMap: { FunctionName: this.functionName },
        ...props
    }).attachTo(this);
};

I believe changing it to this would fix it:

function_base_1.FunctionBase.prototype.metric = function (metricName, props) {
    return new cloudwatch.Metric({
        namespace: 'AWS/Lambda',
        metricName,
        dimensionsMap: { FunctionName: this.functionName },
        account: this.account, //actually we'd parse it from the arn
        region: this.region,
        ...props
    }).attachTo(this);
};
@jonnekaunisto jonnekaunisto added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 12, 2022
@github-actions github-actions bot added the @aws-cdk/aws-lambda Related to AWS Lambda label Feb 12, 2022
@NGL321 NGL321 added p1 and removed needs-triage This issue or PR still needs to be triaged. labels Feb 14, 2022
@kaizencc kaizencc added effort/small Small work item – less than a day of effort effort/medium Medium work item – several days of effort and removed effort/small Small work item – less than a day of effort labels Feb 22, 2022
@madeline-k madeline-k added @aws-cdk/core Related to core CDK functionality and removed @aws-cdk/aws-lambda Related to AWS Lambda labels Jan 28, 2023
@tim-finnigan tim-finnigan self-assigned this Feb 8, 2023
@tim-finnigan
Copy link

Hi @jonnekaunisto thanks for reporting this. Do you have any updates on your end as far as what you've tried or observed regarding this issue? We just received another issue that appears to be related: #24061. If these are overlapping issues then we likely want to de-duplicate them and close one for tracking purposes.

@tim-finnigan tim-finnigan added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Feb 8, 2023
@github-actions
Copy link

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Feb 11, 2023
@jonnekaunisto
Copy link
Contributor Author

Yeah it's pretty much the same issue

@github-actions github-actions bot removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Feb 11, 2023
@tim-finnigan
Copy link

Thanks for confirming, I've closed the newer issue as a duplicate. I think this one will need to get prioritized for a deep dive to address the root problem.

@tim-finnigan tim-finnigan removed their assignment Feb 22, 2023
@peterwoodworth
Copy link
Contributor

peterwoodworth commented May 15, 2023

We would need to add options for which account and region you're importing the resource from. Then we'd be able to configure the metric appropriately automatically. But as it is now, we support the account and region options as options for this method already, so either way you'll have to specify the account and region somewhere. So i'm not super sure what exactly is the ask here

@peterwoodworth peterwoodworth added p2 feature-request A feature should be added or improved. @aws-cdk/aws-sqs Related to Amazon Simple Queue Service and removed bug This issue is a bug. @aws-cdk/core Related to core CDK functionality p1 labels May 15, 2023
@peterwoodworth peterwoodworth added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label May 15, 2023
@github-actions
Copy link

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label May 18, 2023
@EHadoux
Copy link

EHadoux commented May 18, 2023

I think the issue is that one would like to be able to do, e.g.:

const queue = Queue.fromQueueArn(this, 'Queue', QUEUE_ARN);
queue.metricApproximateNumberOfMessagesVisible();

but has to do instead:

const queue = Queue.fromQueueArn(this, 'Queue', QUEUE_ARN);
new Metric({
  metricName: 'ApproximateNumberOfMessagesVisible',
  namespace: 'AWS/SQS',
  account: ACCOUNT_NUMBER,
  dimensionsMap: { QueueName: queue.queueName },
});

even though the queue should contain/know everything already as both the region and the account are coming from QUEUE_ARN.

@github-actions github-actions bot removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-sqs Related to Amazon Simple Queue Service effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

7 participants