ref(build): Use rollup to build AWS lambda layer (#5146)

Our current system for building the AWS lambda layer involves a script which manually traces the serverless package's dependencies, including other monorepo packages, and symlinks them into a `node_modules` folder in a directory set up to mimic an installation from npm. There are a few disadvantages to this system:
- The signals we rely on to figure out what is and isn't a dependency aren't exhaustive, and we're not experts at this, both of which have made getting the right files and only the right files into the layer a brittle process, which has broken down more than once and caused issue for our users.
- The script is complicated, which makes it harder to figure out exactly what's causing it when something does go wrong.
- We symlink in entire packages at a time, regardless of whether or not we're using most of the code. This is true even of the serverless package itself, where we include all of the GCP code along with the AWS code.

This refactors our lambda layer build process to use the new bundling config functions we use for CDN bundles, to create a bundle which can take the place of the the index file, but which contains everything it needs internally. This has the following advantages:

- This puts Rollup in charge of figuring out dependencies instead of us. 
- The config is in line with existing configs and is therefore much easier to reason about and maintain.
- It lets us easily exclude anything GCP-related in the SDK. Between that, Rollup's treeshaking, and terser's minifying, the layer will now take up much less of the finite size allotted to each lambda function.
- Removing extraneous files means less to cache and retrieve from the cache in each GHA job.

Key changes:

- The layer now builds in `packages/serverless/build/aws` rather than at the top level of the repo. (It is now moved to the top level only in our GHA workflow creating the lambda zip.)
- In that workflow, the process to determine the SDK version has been simplified.
- The bundle builds based not on the main index file but on a new bundle-specific index file, which only includes AWS code.
- There is new rollup config just for the layer, which uses the bundle-building functions to make the main bundle and the npm-package-building functions to create a separate module for the optional script which automatically starts up the SDK in AWS.
- The old build script has been replaced by one which runs rollup with that config, and then symlinks the built auto-startup script into its legacy location, so as to be backwards compatible with folks who run it using an environment variable pointing to the old path.
- The building of the layer has temporarily been shifted from `build:awslambda-layer` to `build:bundle`, so that it will run during the first phase of the repo-level `yarn build`. (The goal is to eventually do everything in one phase, for greater parallelization and shorter overall build time.)

h/t to @antonpirker for all his help testing and thinking through this with me.

.github/workflows/build.yml

@@ -31,7 +31,7 @@ env:
     ${{ github.workspace }}/packages/ember/instance-initializers
     ${{ github.workspace }}/packages/gatsby/*.d.ts
     ${{ github.workspace }}/packages/core/src/version.ts
-    ${{ github.workspace }}/dist-serverless
+    ${{ github.workspace }}/packages/serverless
     ${{ github.workspace }}/packages/utils/cjs
     ${{ github.workspace }}/packages/utils/esm
 
@@ -124,52 +124,50 @@ jobs:
         # this file) to a constant and skip rebuilding all of the packages each time CI runs.
         if: steps.cache_built_packages.outputs.cache-hit == ''
         run: yarn build
-      - name: Save SDK version for later
-        run: |
-          echo "Saving SDK_VERSION for later"
-          cat packages/core/src/version.ts | awk -F"'" '{print $2}' > dist-serverless/version
-          [ ! -z $(cat dist-serverless/version) ] && echo SDK_VERSION=$(cat dist-serverless/version) || (echo "Version extraction failed" && exit 1)
     outputs:
       # this needs to be passed on, because the `needs` context only looks at direct ancestors (so steps which depend on
       # `job_build` can't see `job_install_deps` and what it returned)
       dependency_cache_key: ${{ needs.job_install_deps.outputs.dependency_cache_key }}
 
-  # job_build_aws_lambda_layer:
-  #   name: Build AWS Lambda Layer
-  #   needs: job_build
-  #   runs-on: ubuntu-latest
-  #   steps:
-  #     - name: Check out current commit (${{ env.HEAD_COMMIT }})
-  #       uses: actions/checkout@v2
-  #       with:
-  #         ref: ${{ env.HEAD_COMMIT }}
-  #     - name: Set up Node
-  #       uses: actions/setup-node@v1
-  #       with:
-  #         node-version: ${{ env.DEFAULT_NODE_VERSION }}
-  #     - name: Check dependency cache
-  #       uses: actions/cache@v2
-  #       with:
-  #         path: ${{ env.CACHED_DEPENDENCY_PATHS }}
-  #         key: ${{ needs.job_build.outputs.dependency_cache_key }}
-  #     - name: Check build cache
-  #       uses: actions/cache@v2
-  #       with:
-  #         path: ${{ env.CACHED_BUILD_PATHS }}
-  #         key: ${{ env.BUILD_CACHE_KEY }}
-  #     - name: Get SDK version
-  #       run: |
-  #         export SDK_VERSION=$(cat dist-serverless/version)
-  #         echo "SDK_VERSION=$SDK_VERSION" | tee -a $GITHUB_ENV
-  #     - uses: actions/upload-artifact@v3
-  #       with:
-  #         name: ${{ env.HEAD_COMMIT }}
-  #         path: |
-  #           dist-serverless/*
-  #     - uses: getsentry/action-build-aws-lambda-extension@v1
-  #       with:
-  #         artifact_name: ${{ env.HEAD_COMMIT }}
-  #         zip_file_name: sentry-node-serverless-${{ env.SDK_VERSION }}.zip
+  job_pack_aws_lambda_layer:
+    name: Pack and Upload AWS Lambda Layer
+    needs: [job_get_metadata, job_build]
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }})
+        uses: actions/checkout@v2
+        with:
+          ref: ${{ env.HEAD_COMMIT }}
+      - name: Set up Node
+        uses: actions/setup-node@v1
+        with:
+          node-version: ${{ env.DEFAULT_NODE_VERSION }}
+      - name: Check dependency cache
+        uses: actions/cache@v2
+        with:
+          path: ${{ env.CACHED_DEPENDENCY_PATHS }}
+          key: ${{ needs.job_build.outputs.dependency_cache_key }}
+      - name: Check build cache
+        uses: actions/cache@v2
+        with:
+          path: ${{ env.CACHED_BUILD_PATHS }}
+          key: ${{ env.BUILD_CACHE_KEY }}
+      - name: Get SDK version
+        # `jq` reads JSON files, and `tee` pipes its input to the given location and to stdout. (Adding `-a` is the
+        # equivalent of using >> rather than >.)
+        run: |
+          export SDK_VERSION=$(cat packages/core/package.json | jq --raw-output '.version')
+          echo "SDK_VERSION=$SDK_VERSION" | tee -a $GITHUB_ENV
+      - name: Move dist-serverless to root directory (requirement for zipping action)
+        run: |
+          mv ./packages/serverless/build/aws/dist-serverless .
+      - name: Create and upload final zip file
+        uses: getsentry/action-build-aws-lambda-extension@v1
+        with:
+          artifact_name: ${{ env.HEAD_COMMIT }}
+          zip_file_name: sentry-node-serverless-${{ env.SDK_VERSION }}.zip
+          build_cache_paths: ${{ env.CACHED_BUILD_PATHS }}
+          build_cache_key: ${{ env.BUILD_CACHE_KEY }}
 
   job_size_check:
     name: Size Check

.gitignore

@@ -14,6 +14,9 @@ scratch/
 *.js.map
 *.pyc
 *.tsbuildinfo
+# side effects of running AWS lambda layer zip action locally
+dist-serverless/
+sentry-node-serverless-*.zip
 # transpiled transformers
 jest/transformers/*.js
 # node tarballs

packages/serverless/.eslintrc.js

@@ -6,4 +6,18 @@ module.exports = {
   rules: {
     '@sentry-internal/sdk/no-async-await': 'off',
   },
+  overrides: [
+    {
+      files: ['scripts/**/*.ts'],
+      parserOptions: {
+        project: ['../../tsconfig.dev.json'],
+      },
+    },
+    {
+      files: ['test/**'],
+      parserOptions: {
+        sourceType: 'module',
+      },
+    },
+  ],
 };

packages/serverless/package.json

@@ -9,9 +9,9 @@
   "engines": {
     "node": ">=10"
   },
-  "main": "build/cjs/index.js",
-  "module": "build/esm/index.js",
-  "types": "build/types/index.d.ts",
+  "main": "build/npm/cjs/index.js",
+  "module": "build/npm/esm/index.js",
+  "types": "build/npm/types/index.d.ts",
   "publishConfig": {
     "access": "public"
   },
@@ -38,8 +38,9 @@
     "read-pkg": "^5.2.0"
   },
   "scripts": {
-    "build": "run-p build:rollup build:types && yarn build:extras",
-    "build:awslambda-layer": "node scripts/build-awslambda-layer.js",
+    "build": "run-p build:rollup build:types build:bundle && yarn build:extras",
+    "build:awslambda-layer": "echo 'WARNING: AWS lambda layer build emporarily moved to \\`build:bundle\\`.'",
+    "build:bundle": "yarn ts-node scripts/buildLambdaLayer.ts",
     "build:dev": "run-p build:rollup build:types",
     "build:extras": "yarn build:awslambda-layer",
     "build:rollup": "rollup -c rollup.npm.config.js",
@@ -48,7 +49,7 @@
     "build:dev:watch": "run-s build:watch",
     "build:rollup:watch": "rollup -c rollup.npm.config.js --watch",
     "build:types:watch": "tsc -p tsconfig.types.json --watch",
-    "build:npm": "ts-node ../../scripts/prepack.ts && npm pack ./build",
+    "build:npm": "ts-node ../../scripts/prepack.ts --bundles && npm pack ./build/npm",
     "circularDepCheck": "madge --circular src/index.ts",
     "clean": "rimraf build dist-awslambda-layer coverage sentry-serverless-*.tgz",
     "fix": "run-s fix:eslint fix:prettier",

packages/serverless/rollup.aws.config.js

@@ -0,0 +1,44 @@
+import { makeBaseBundleConfig, makeBundleConfigVariants, makeBaseNPMConfig } from '../../rollup/index.js';
+
+export default [
+  // The SDK
+  ...makeBundleConfigVariants(
+    makeBaseBundleConfig({
+      // this automatically sets it to be CJS
+      bundleType: 'node',
+      entrypoints: ['src/index.awslambda.ts'],
+      jsVersion: 'es6',
+      licenseTitle: '@sentry/serverless',
+      outputFileBase: () => 'index',
+      packageSpecificConfig: {
+        output: {
+          dir: 'build/aws/dist-serverless/nodejs/node_modules/@sentry/serverless/build/npm/cjs',
+          sourcemap: false,
+        },
+      },
+    }),
+    // We only need one copy of the SDK, and we pick the minified one because there's a cap on how big a lambda function
+    // plus its dependencies can be, and we might as well take up as little of that space as is necessary. We'll rename
+    // it to be `index.js` in the build script, since it's standing in for the index file of the npm package.
+    { variants: ['.min.js'] },
+  ),
+
+  // This builds a wrapper file, which our lambda layer integration automatically sets up to run as soon as node
+  // launches (via the `NODE_OPTIONS="-r @sentry/serverless/dist/awslambda-auto"` variable). Note the inclusion in this
+  // path of the legacy `dist` folder; for backwards compatibility, in the build script we'll copy the file there.
+  makeBaseNPMConfig({
+    entrypoints: ['src/awslambda-auto.ts'],
+    packageSpecificConfig: {
+      // Normally `makeNPMConfigVariants` sets both of these values for us, but we don't actually want the ESM variant,
+      // and the directory structure is different than normal, so we have to do it ourselves.
+      output: {
+        format: 'cjs',
+        dir: 'build/aws/dist-serverless/nodejs/node_modules/@sentry/serverless/build/npm/cjs',
+        sourcemap: false,
+      },
+      // We only want `awslambda-auto.js`, not the modules that it imports, because they're all included in the bundle
+      // we generate above
+      external: ['./index'],
+    },
+  }),
+];

packages/serverless/rollup.npm.config.js

@@ -2,6 +2,11 @@ import { makeBaseNPMConfig, makeNPMConfigVariants } from '../../rollup/index.js'
 
 export default makeNPMConfigVariants(
   makeBaseNPMConfig({
+    // TODO: `awslambda-auto.ts` is a file which the lambda layer uses to automatically init the SDK. Does it need to be
+    // in the npm package? Is it possible that some people are using it themselves in the same way the layer uses it (in
+    // which case removing it would be a breaking change)? Should it stay here or not?
     entrypoints: ['src/index.ts', 'src/awslambda-auto.ts'],
+    // packages with bundles have a different build directory structure
+    hasBundles: true,
   }),
 );