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

chore(core): update CDK Metadata to report construct-level details #13423

Merged
merged 7 commits into from
Mar 10, 2021

Conversation

njlynch
Copy link
Contributor

@njlynch njlynch commented Mar 5, 2021

See CDK RFC 253 (aws/aws-cdk-rfcs#254) for background and details.

Currently -- if a user has not opted out -- an AWS::CDK::Metadata resource
is added to each generated stack template with details about each loaded module
and version that matches an Amazon-specific allow list.

This modules list is used to:

  • Track what library versions customers are using so they can be contacted in
    the event of a severe (security) issue with a library.
  • Get business metrics on the adoption of CDK and its libraries.

This modules list is sometimes inaccurate (a module may be loaded into memory
without actually being used) and too braod to support CDK v2.

This feature (mostly) implements the specification proposed in RFC 253 to
include metadata about what constructs are present in each stack, rather than
modules loaded into memory. The allow-list is still used to ensure only CDK/AWS
constructs are reported on.

Implementation notes:

  • The format of the Analytics property has changed slightly since the RFC. See
    the service-side code for justification and latest spec.
  • How to handle the jsii runtime information was left un-spec'd. I've chosen to
    create a psuedo-Construct to add to the list as the simplest solution.
  • runtime-info.test.ts leaps through some serious hoops to work equally well
    for both v1 and v2, and to fail somewhat gracefully locally if tsc was used
    to compile the module instead of jsii. Critques of this approach welcome!
  • I removed an annoyance from resolve-version-lib.js that produced error
    messages when running unit tests.

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

See CDK RFC 253 (aws/aws-cdk-rfcs#254) for background and details.

Currently -- if a user has not opted out -- an AWS::CDK::Metadata resource
is added to each generated stack template with details about each loaded module
and version that matches an Amazon-specific allow list.

This modules list is used to:

- Track what library versions customers are using so they can be contacted in
  the event of a severe (security) issue with a library.
- Get business metrics on the adoption of CDK and its libraries.

This modules list is sometimes inaccurate (a module may be loaded into memory
without actually being used) and too braod to support CDK v2.

This feature implements (mostly) the specification proposed in RFC 253 to
include metadata about what constructs are present in each stack, rather than
modules loaded into memory. The allow-list is still used to ensure only CDK/AWS
constructs are reported on.

Implementation notes:
- The format of the Analytics property has changed slightly since the RFC. See
  the service-side code for justification and latest spec.
- How to handle the jsii runtime information was left un-spec'd. I've chosen to
  create a psuedo-Construct to add to the list as the simplest solution.
- `runtime-info.test.ts` leaps through some serious hoops to work equally well
  for both v1 and v2, and to fail somewhat gracefully locally if `tsc` was used
  to compile the module instead of `jsii`. Critques of this approach welcome!
- I removed an annoyance from `resolve-version-lib.js` that produced error
  messages when running unit tests.
@njlynch njlynch added the pr-linter/exempt-readme The PR linter will not require README changes label Mar 5, 2021
@njlynch njlynch requested review from rix0rrr, nija-at and a team March 5, 2021 18:07
@njlynch njlynch self-assigned this Mar 5, 2021
@gitpod-io
Copy link

gitpod-io bot commented Mar 5, 2021

@github-actions github-actions bot added the @aws-cdk/core Related to core CDK functionality label Mar 5, 2021
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Mar 5, 2021
@rix0rrr
Copy link
Contributor

rix0rrr commented Mar 8, 2021

I feel this ought to be a chore. Is this something a user cares about? (Maybe, maybe not. Probably not I'd say).

rix0rrr
rix0rrr previously requested changes Mar 8, 2021
packages/@aws-cdk/core/lib/private/metadata-resource.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/core/lib/private/metadata-resource.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/core/lib/private/runtime-info.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/core/lib/private/runtime-info.ts Outdated Show resolved Hide resolved
@njlynch njlynch changed the title feat(core): update CDK Metadata to report construct-level details chore(core): update CDK Metadata to report construct-level details Mar 9, 2021
@njlynch njlynch requested review from rix0rrr and a team March 9, 2021 14:50
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.

Pleasure to read. Nothing blocking.

@eladb eladb added the pr/do-not-merge This PR should not be merged at this time. label Mar 9, 2021
@eladb
Copy link
Contributor

eladb commented Mar 9, 2021

added do-not-merge to allow other reviewers to chime in

@njlynch njlynch removed pr/do-not-merge This PR should not be merged at this time. pr-linter/exempt-readme The PR linter will not require README changes labels Mar 10, 2021
@njlynch njlynch dismissed rix0rrr’s stale review March 10, 2021 11:16

Comments addressed; confirmed offline.

@mergify
Copy link
Contributor

mergify bot commented Mar 10, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

* 1.90.0!aws-cdk-lib.{Stack,Construct,service.Resource},0.42.1!aws-cdk-lib-experiments.NewStuff
*
* The whole thing is then either included directly as plaintext as:
* v2:plaintext:{prefixEncodedList}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* v2:plaintext:{prefixEncodedList}

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 9b1a73b
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify
Copy link
Contributor

mergify bot commented Mar 10, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 8dca507 into master Mar 10, 2021
@mergify mergify bot deleted the njlynch/constructs-metadata branch March 10, 2021 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants