-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat: enable additional metadata collection (under feature flag) #33232
base: main
Are you sure you want to change the base?
Conversation
// Enhanced CDK Analytics Telemetry | ||
addConstructMetadata(this, props); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The majority of the code changes are auto generated so you'll see hundreds of addConstructMetadata
method call across different L2 resources.
This method comes from this change https://github.com/aws/aws-cdk/pull/33232/files#diff-81f821b1205e7040fc3103bf7c0114060a6d5c43ebd2994aa4ed5906e42c9c5fR33. The main code change that needs to be reviewed is in packages/aws-cdk-lib/core
as well as tools/@aws-cdk/construct-metadata-updater
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only support collecting metadata from the constructor for now .. right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR only supports constructor collection. I'll work on the method collections but I don't think that blocks us reviewing and merging this PR.
tools/@aws-cdk/construct-metadata-updater/lib/metadata-updater.ts
Outdated
Show resolved
Hide resolved
are you going to add the GH workflow in a separate PR ? |
Yes, it's going to be a separate PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me overall.
*/ | ||
export function redactMetadata(fqn: string, data: any): any { | ||
// A valid fqn is consists of 3 parts, i.e. `aws-cdk-lib.aws-lambda.Function`. | ||
const fqnParts = fqn.replace(/[-_]/g, '-').split('.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: is there an example we need to replace _
with -
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, for example, IAM's fqn
is something like "fqn": "aws-cdk-lib.aws_iam.CfnRole",
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Added |
Issue # (if applicable)
#33198
Note
The majority of the code changes are auto generated so you'll see hundreds of
addConstructMetadata
method call across different L2 resources.This method comes from this change https://github.com/aws/aws-cdk/pull/33232/files#diff-81f821b1205e7040fc3103bf7c0114060a6d5c43ebd2994aa4ed5906e42c9c5fR33. The main code change that needs to be reviewed is in
packages/aws-cdk-lib/core
as well astools/@aws-cdk/construct-metadata-updater
Reason for this change
This discussion aims to expand the scope of usage data collected by the AWS CDK to better inform CDK development and improve communication for customer-impacting topics. Currently, for those that opt in, the CDK collects usage data on your CDK version and which L2 constructs you use.
Description of changes
Consider the following example
At synthesis, usage data is collected, compressed, and stored in the
AWS::CDK::Metadata resource
. Based on current behavior, the following is an example of the usage data that will be collected from our example app:With this proposal, the following usage data will be collected. The * value replaces property values that will be redacted from data collection:
Describe any new or updated permissions being added
No
Description of how you validated changes
Many new unit tests added to verify different behaviour of various functions and methods introduced. One integ test file is added to test the deployability.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license