Conversation
|
@BridgeAR this is because we believe #711 might not address other dependencies that are used in the layer. appsec is likely one but there may be others. we dont know exactly which ones though as there isn't test coverage there yet. Until we can figure that out, I think itd be best to not try to cut out dependencies. If the rms have no effect I can remove them but the main thing is removing the --ignore-optional flag |
BridgeAR
left a comment
BridgeAR
left a comment
There was a problem hiding this comment.
If we want to land this, I think we should fix these first.
| RUN rm -rf /nodejs/node_modules/hdr-histogram-js/build | ||
| RUN rm -rf /nodejs/node_modules/protobufjs/dist | ||
| RUN rm -rf /nodejs/node_modules/protobufjs/cli |
There was a problem hiding this comment.
These should be changed to match our vendored path in dd-trace.
| RUN rm -rf /nodejs/node_modules/dd-trace/prebuilds | ||
| RUN rm -rf /nodejs/node_modules/dd-trace/dist |
There was a problem hiding this comment.
I am not sure where these come from. They do not belong to dd-trace (if they did, they do not since a very long time).
| RUN rm -rf /nodejs/node_modules/jsonpath-plus/src/jsonpath.d.ts | ||
| RUN rm -rf /nodejs/node_modules/jsonpath-plus/src/jsonpath-browser.js | ||
| RUN rm -rf /nodejs/node_modules/jsonpath-plus/src/dist/index-browser-umd.min.cjs | ||
| RUN rm -rf /nodejs/node_modules/jsonpath-plus/src/dist/index-browser-umd.cjs | ||
| RUN rm -rf /nodejs/node_modules/jsonpath-plus/src/dist/index-browser-esm.min.js | ||
| RUN rm -rf /nodejs/node_modules/jsonpath-plus/src/dist/index-browser-esm.js |
There was a problem hiding this comment.
This should be changed to match our vendored path in dd-trace.
There was a problem hiding this comment.
theres only an index.js file in each vendored package, so these files wouldnt exist there either. I think its okay to just remove these lines
There was a problem hiding this comment.
Yeah, vendoring should remove the need for any removal in a vendored dependency.
| RUN rm -rf /nodejs/node_modules/@datadog/pprof/prebuilds/darwin-arm64 | ||
| RUN rm -rf /nodejs/node_modules/@datadog/pprof/prebuilds/darwin-x64 | ||
| RUN rm -rf /nodejs/node_modules/@datadog/pprof/prebuilds/win32-ia32 | ||
| RUN rm -rf /nodejs/node_modules/@datadog/pprof/prebuilds/win32-x64 |
There was a problem hiding this comment.
We should be able to remove more binaries, similar to https://github.com/DataDog/datadog-lambda-js/pull/711/changes#diff-dd2c0eb6ea5cfc6c4bd4eac30934e2d5746747af48fef6da689e85b752f39557
4 participants
What does this PR do?
Reverts size optimizations while keeping latest dd-trace version.
Motivation
size optimizations may be removing necessary dependencies and functionality (namely profiling) needed in lambda layer. This is blocking release so for now the plan is to revert those optimizations, and address the dependency issues after releasing.
Testing Guidelines
Additional Notes
Types of Changes
Check all that apply