@carbon/telemetry pollutes our production code with unused development dependencies and impacts load performance

schuetza commented 3 years ago

What package(s) are you using?

[x ] carbon-components
[ x] carbon-components-vue

Detailed description

With v10.27.0 @carbon/telemetry was added, and has really bad impact on our product. For our application, we are using @carbon/vue, which picked telemetry with 2.37.1.

After updating to the new version, we found

many new dependencies added as production dependencies in our package-lock.json
and a growth of our distribution package of over 200% (from 26MB to 47MB, see below)

We already tried to address that, see https://ibm-cloud.slack.com/archives/CAM5P6NR1/p1612864854039900 https://github.com/carbon-design-system/carbon-components-vue/issues/1142

but the only answer was that we can opt out by using CARBON_TELEMETRY_DISABLED. But setting CARBON_TELEMETRY_DISABLED doesn't prevent any of the issues we have:

1) The raised footprint of the generated chunk-vendors.js layer slows down our performance. 2) We must ship components/code we never use, which need to be maintained and monitored for vulnerabilities, security flaws etc. 3) We need to get open-source clearance for all these newly added packages, for components that we don't require at all

Such a utility module must not impact the product's code in such heavy manner.

Is this issue related to a specific component? n/a

What did you expect to happen? What happened instead? What would you like to see changed?

Expecting

same build behavior when doing minor version updates
a clean production dependency tree without unused libs/code

Happend:

1) The raised footprint of the generated chunk-vendors.js layer slows down our performance. 2) We must ship components/code we never use, which need to be maintained and monitored for vulnerabilities, security flaws etc. 3) We need to get open-source clearance for all these newly added packages, for components that we don't require at all

What needs to change:

Telemetry code running only as node installation hook during development phase must not contribute to the app's production package. Therefore redesign telemetry copmonent to be a development package or drop it.

What browser are you working in? n/a

What version of the Carbon Design System are you using? see package.json below

What offering/product do you work on? Any pressing ship or release dates we should be aware of? IBM Digital Business Automation Cloud Portal app

Steps to reproduce the issue

1.Have an app built on carbo/vue 2.37.0

Check build sizes and dependency tree with node ls --prod
update to carbon/vue 2.37.1+
repeat step 2

Please create a reduced test case in CodeSandbox n/a

Style and vanilla JS: https://codesandbox.io/s/github/carbon-design-system/carbon/tree/main/packages/components/examples/codesandbox

React: https://codesandbox.io/s/github/carbon-design-system/carbon/tree/main/packages/react/examples/codesandbox

Additional information

For comparison, here are some examples of our app, before and after moving to version @carbon/vue 2.38.2, and therefore before and after added telemetry. Especially check the changed size of the chunk-vendors layer: 16Mb before, 37Mb afterwards and the quite small dependencies tree before and the very large one aftrwerwards:

Before update:

$ cat package.json { "name": "com.ibm.automation.cloud.instance.ui.dbaoc", "dependencies": { "@carbon/vue": "2.37.0", "core-js": "3.12.1", "lodash": "4.17.21", "vue": "2.6.12", "vue-i18n": "8.24.4", "vue-router": "3.5.1" }, ... }

$ du -h dist 68K dist/img 604K dist/css 25M dist/js 300K dist/vendor/jquery-ui/i18n 300K dist/vendor/jquery-ui 636K dist/vendor 26M dist

$ du -ha dist/js/* 468K dist/js/admin-access-management.ec4b79d1.js 3.3M dist/js/admin-reports.8a3b9a58.js 724K dist/js/admin-system-operations.9059c77c.js 3.9M dist/js/app.283180c5.js 16M dist/js/chunk-vendors.27d0cff5.js


$ npm ls --prod
com.ibm.automation.cloud.instance.ui.dbaoc@1.0.0 
├─┬ @carbon/vue@2.37.0
│ ├─┬ @carbon/icons-vue@10.20.0
│ │ └── @carbon/icon-helpers@10.17.0
│ ├─┬ carbon-components@10.23.1
│ │ ├── flatpickr@4.6.1
│ │ ├── lodash.debounce@4.0.8
│ │ └─┬ warning@3.0.0
│ │   └─┬ loose-envify@1.4.0
│ │     └── js-tokens@4.0.0
│ ├── flatpickr@4.6.3
│ └── vue@2.6.12 deduped
├── core-js@3.12.1
├── lodash@4.17.21
├── vue@2.6.12
├── vue-i18n@8.24.4
└── vue-router@3.5.1

After update (no other changes):

package.json { "name": "com.ibm.automation.cloud.instance.ui.dbaoc" "dependencies": { "@carbon/vue": "2.38.2", "core-js": "3.12.1", "lodash": "4.17.21", "vue": "2.6.12", "vue-i18n": "8.24.4", "vue-router": "3.5.1" }, ... }

$ du -h dist 68K dist/img 604K dist/css 45M dist/js 300K dist/vendor/jquery-ui/i18n 300K dist/vendor/jquery-ui 636K dist/vendor 47M dist

$ du -ha dist/js/* 468K dist/js/admin-access-management.8b5d2bd7.js 3.3M dist/js/admin-reports.874e1cb5.js 724K dist/js/admin-system-operations.b017aac0.js 3.9M dist/js/app.c226d3a2.js 37M dist/js/chunk-vendors.49f119cd.js


$ npm ls --prod 
com.ibm.automation.cloud.instance.ui.dbaoc@1.0.0
├─┬ @carbon/vue@2.38.2
│ ├─┬ @carbon/icons-vue@10.27.0
│ │ └── @carbon/icon-helpers@10.18.0
│ ├─┬ @carbon/telemetry@0.0.0-alpha.6
│ │ ├── @babel/parser@7.14.3
│ │ ├─┬ @babel/traverse@7.14.2
│ │ │ ├─┬ @babel/code-frame@7.12.13
│ │ │ │ └─┬ @babel/highlight@7.14.0
│ │ │ │   ├── @babel/helper-validator-identifier@7.14.0 deduped
│ │ │ │   ├─┬ chalk@2.4.2
│ │ │ │   │ ├─┬ ansi-styles@3.2.1
│ │ │ │   │ │ └─┬ color-convert@1.9.3
│ │ │ │   │ │   └── color-name@1.1.3
│ │ │ │   │ ├── escape-string-regexp@1.0.5
│ │ │ │   │ └─┬ supports-color@5.5.0
│ │ │ │   │   └── has-flag@3.0.0
│ │ │ │   └── js-tokens@4.0.0 deduped
│ │ │ ├─┬ @babel/generator@7.14.3
│ │ │ │ ├── @babel/types@7.14.2 deduped
│ │ │ │ ├── jsesc@2.5.2
│ │ │ │ └── source-map@0.5.7
│ │ │ ├─┬ @babel/helper-function-name@7.14.2
│ │ │ │ ├─┬ @babel/helper-get-function-arity@7.12.13
│ │ │ │ │ └── @babel/types@7.14.2 deduped
│ │ │ │ ├─┬ @babel/template@7.12.13
│ │ │ │ │ ├── @babel/code-frame@7.12.13 deduped
│ │ │ │ │ ├── @babel/parser@7.14.3 deduped
│ │ │ │ │ └── @babel/types@7.14.2 deduped
│ │ │ │ └── @babel/types@7.14.2 deduped
│ │ │ ├─┬ @babel/helper-split-export-declaration@7.12.13
│ │ │ │ └── @babel/types@7.14.2 deduped
│ │ │ ├── @babel/parser@7.14.3 deduped
│ │ │ ├─┬ @babel/types@7.14.2
│ │ │ │ ├── @babel/helper-validator-identifier@7.14.0
│ │ │ │ └── to-fast-properties@2.0.0
│ │ │ ├─┬ debug@4.3.1
│ │ │ │ └── ms@2.1.2
│ │ │ └── globals@11.12.0
│ │ ├── ci-info@2.0.0
│ │ ├─┬ configstore@5.0.1
│ │ │ ├─┬ dot-prop@5.3.0
│ │ │ │ └── is-obj@2.0.0
│ │ │ ├── graceful-fs@4.2.6 deduped
│ │ │ ├─┬ make-dir@3.1.0
│ │ │ │ └── semver@6.3.0
│ │ │ ├─┬ unique-string@2.0.0
│ │ │ │ └── crypto-random-string@2.0.0
│ │ │ ├─┬ write-file-atomic@3.0.3
│ │ │ │ ├── imurmurhash@0.1.4
│ │ │ │ ├── is-typedarray@1.0.0
│ │ │ │ ├── signal-exit@3.0.3
│ │ │ │ └─┬ typedarray-to-buffer@3.1.5
│ │ │ │   └── is-typedarray@1.0.0 deduped
│ │ │ └── xdg-basedir@4.0.0
│ │ ├─┬ fast-glob@3.2.5
│ │ │ ├── @nodelib/fs.stat@2.0.5
│ │ │ ├─┬ @nodelib/fs.walk@1.2.7
│ │ │ │ ├─┬ @nodelib/fs.scandir@2.1.5
│ │ │ │ │ ├── @nodelib/fs.stat@2.0.5
│ │ │ │ │ └─┬ run-parallel@1.2.0
│ │ │ │ │   └── queue-microtask@1.2.3
│ │ │ │ └─┬ fastq@1.11.0
│ │ │ │   └── reusify@1.0.4
│ │ │ ├─┬ glob-parent@5.1.2
│ │ │ │ └─┬ is-glob@4.0.1
│ │ │ │   └── is-extglob@2.1.1
│ │ │ ├── merge2@1.4.1
│ │ │ ├─┬ micromatch@4.0.4
│ │ │ │ ├─┬ braces@3.0.2
│ │ │ │ │ └─┬ fill-range@7.0.1
│ │ │ │ │   └─┬ to-regex-range@5.0.1
│ │ │ │ │     └── is-number@7.0.0
│ │ │ │ └── picomatch@2.2.3 deduped
│ │ │ └── picomatch@2.2.3
│ │ ├─┬ fs-extra@9.1.0
│ │ │ ├── at-least-node@1.0.0
│ │ │ ├── graceful-fs@4.2.6
│ │ │ ├─┬ jsonfile@6.1.0
│ │ │ │ ├── graceful-fs@4.2.6 deduped
│ │ │ │ └── universalify@2.0.0 deduped
│ │ │ └── universalify@2.0.0
│ │ ├─┬ got@11.8.2
│ │ │ ├── @sindresorhus/is@4.0.1
│ │ │ ├─┬ @szmarczak/http-timer@4.0.5
│ │ │ │ └── defer-to-connect@2.0.1
│ │ │ ├─┬ @types/cacheable-request@6.0.1
│ │ │ │ ├── @types/http-cache-semantics@4.0.0
│ │ │ │ ├─┬ @types/keyv@3.1.1
│ │ │ │ │ └── @types/node@15.3.1 deduped
│ │ │ │ ├── @types/node@15.3.1
│ │ │ │ └── @types/responselike@1.0.0 deduped
│ │ │ ├─┬ @types/responselike@1.0.0
│ │ │ │ └── @types/node@15.3.1 deduped
│ │ │ ├── cacheable-lookup@5.0.4
│ │ │ ├─┬ cacheable-request@7.0.2
│ │ │ │ ├─┬ clone-response@1.0.2
│ │ │ │ │ └── mimic-response@1.0.1
│ │ │ │ ├─┬ get-stream@5.2.0
│ │ │ │ │ └─┬ pump@3.0.0
│ │ │ │ │   ├─┬ end-of-stream@1.4.4
│ │ │ │ │   │ └── once@1.4.0 deduped
│ │ │ │ │   └─┬ once@1.4.0
│ │ │ │ │     └── wrappy@1.0.2
│ │ │ │ ├── http-cache-semantics@4.1.0
│ │ │ │ ├─┬ keyv@4.0.3
│ │ │ │ │ └── json-buffer@3.0.1
│ │ │ │ ├── lowercase-keys@2.0.0 deduped
│ │ │ │ ├── normalize-url@6.0.1
│ │ │ │ └── responselike@2.0.0 deduped
│ │ │ ├─┬ decompress-response@6.0.0
│ │ │ │ └── mimic-response@3.1.0
│ │ │ ├─┬ http2-wrapper@1.0.3
│ │ │ │ ├── quick-lru@5.1.1
│ │ │ │ └── resolve-alpn@1.1.2
│ │ │ ├── lowercase-keys@2.0.0
│ │ │ ├── p-cancelable@2.1.1
│ │ │ └─┬ responselike@2.0.0
│ │ │   └── lowercase-keys@2.0.0 deduped
│ │ ├─┬ semver@7.3.5
│ │ │ └─┬ lru-cache@6.0.0
│ │ │   └── yallist@4.0.0
│ │ ├─┬ winston@3.3.3
│ │ │ ├─┬ @dabh/diagnostics@2.0.2
│ │ │ │ ├─┬ colorspace@1.1.2
│ │ │ │ │ ├─┬ color@3.0.0
│ │ │ │ │ │ ├─┬ color-convert@1.9.3
│ │ │ │ │ │ │ └── color-name@1.1.3
│ │ │ │ │ │ └─┬ color-string@1.5.5
│ │ │ │ │ │   ├── color-name@1.1.4 deduped
│ │ │ │ │ │   └─┬ simple-swizzle@0.2.2
│ │ │ │ │ │     └── is-arrayish@0.3.2
│ │ │ │ │ └── text-hex@1.0.0
│ │ │ │ ├── enabled@2.0.0
│ │ │ │ └── kuler@2.0.0
│ │ │ ├── async@3.2.0
│ │ │ ├── is-stream@2.0.0
│ │ │ ├─┬ logform@2.2.0
│ │ │ │ ├── colors@1.4.0
│ │ │ │ ├── fast-safe-stringify@2.0.7
│ │ │ │ ├── fecha@4.2.1
│ │ │ │ ├── ms@2.1.2 deduped
│ │ │ │ └── triple-beam@1.3.0 deduped
│ │ │ ├─┬ one-time@1.0.0
│ │ │ │ └── fn.name@1.1.0
│ │ │ ├─┬ readable-stream@3.6.0
│ │ │ │ ├── inherits@2.0.4
│ │ │ │ ├─┬ string_decoder@1.1.1
│ │ │ │ │ └── safe-buffer@5.1.2 deduped
│ │ │ │ └── util-deprecate@1.0.2
│ │ │ ├── stack-trace@0.0.10
│ │ │ ├── triple-beam@1.3.0
│ │ │ └─┬ winston-transport@4.4.0
│ │ │   ├─┬ readable-stream@2.3.7
│ │ │   │ ├── core-util-is@1.0.2
│ │ │   │ ├── inherits@2.0.4 deduped
│ │ │   │ ├── isarray@1.0.0
│ │ │   │ ├── process-nextick-args@2.0.1
│ │ │   │ ├── safe-buffer@5.1.2
│ │ │   │ ├── string_decoder@1.1.1 deduped
│ │ │   │ └── util-deprecate@1.0.2 deduped
│ │ │   └── triple-beam@1.3.0 deduped
│ │ └─┬ yargs@16.2.0
│ │   ├─┬ cliui@7.0.4
│ │   │ ├── string-width@4.2.2 deduped
│ │   │ ├─┬ strip-ansi@6.0.0
│ │   │ │ └── ansi-regex@5.0.0
│ │   │ └─┬ wrap-ansi@7.0.0
│ │   │   ├─┬ ansi-styles@4.3.0
│ │   │   │ └─┬ color-convert@2.0.1
│ │   │   │   └── color-name@1.1.4
│ │   │   ├── string-width@4.2.2 deduped
│ │   │   └── strip-ansi@6.0.0 deduped
│ │   ├── escalade@3.1.1
│ │   ├── get-caller-file@2.0.5
│ │   ├── require-directory@2.1.1
│ │   ├─┬ string-width@4.2.2
│ │   │ ├── emoji-regex@8.0.0
│ │   │ ├── is-fullwidth-code-point@3.0.0
│ │   │ └── strip-ansi@6.0.0 deduped
│ │   ├── y18n@5.0.8
│ │   └── yargs-parser@20.2.7
│ ├─┬ carbon-components@10.30.0
│ │ ├── @carbon/telemetry@0.0.0-alpha.6 deduped
│ │ ├── flatpickr@4.6.1
│ │ ├── lodash.debounce@4.0.8
│ │ └─┬ warning@3.0.0
│ │   └─┬ loose-envify@1.4.0
│ │     └── js-tokens@4.0.0
│ ├── flatpickr@4.6.9
│ └── vue@2.6.12 deduped
├── core-js@3.12.1
├── lodash@4.17.21
├── vue@2.6.12
├── vue-i18n@8.24.4
└── vue-router@3.5.1

joshblack commented 3 years ago

Hi there, @schuetza! 👋

Could you share more details about how this is impacting load performance through the generated chunks? The @carbon/telemetry package is not included in any bundled code so it should not be impacting that metric. Specifically, the package itself cannot be brought in through require or import because it has no entry point set in package.json. The intent for the package is to run in CI only but if something is going on with the setup we'd love to dig in further to find out what's going on.

schuetza commented 3 years ago

Thanks @joshblack for the quick answer.

re. how this is impacting load performance

The chunk-vendors.js is loaded as part of the app. Therefore, to boot up our app, the browser needs to load 37Mb instead of 16Mb. Additionally, browser needs to handle this large js file.

re. package is not included in any bundled code

Base on my limited understanding an how NPM works, if I look into the package.json of @carbon/telemetry, that is not true:

 "dependencies": {
    "@babel/parser": "^7.12.5",
    "@babel/traverse": "^7.12.5",
    "ci-info": "^2.0.0",
    "configstore": "^5.0.1",
    "fast-glob": "^3.2.4",
    "fs-extra": "^9.0.1",
    "got": "^11.8.0",
    "semver": "^7.3.2",
    "winston": "^3.3.3",
    "yargs": "^16.1.1"
  },

Since all the dependencies are declared as "dependencies" and not as "devDependencies", these must be packaged into production code.

After updating to carbon/vue to 2.37.1 and running npm install to update the package-lock.json, we can observe that many of the development libs are changed to production dependencies, like the following


@@ -8,7 +8,6 @@
       "version": "7.12.13",
       "resolved": "https://.../@babel/code-frame/-/code-frame-7.12.13.tgz",
       "integrity": "sha1-3PyCa+72XnXFDiHTg319lXmN1lg=",
-      "dev": true,
       "requires": {
         "@babel/highlight": "^7.12.13"
       }

This is also proven by the npm ls command output I attached above.

Beside that I can only report what I see: 1) package-lock has now many more production dependencies listed. (where the "dev":true property was removed) 2) our distribution package grew a lot 3) the wicked tool reports many new production dependencies 4) npm lists many more production dependencies

joshblack commented 3 years ago

@schuetza

The most helpful tool for figuring this out will be the output of something like webpack-bundle-analyzer or source-map-explorer. These tools can be used to see what modules are bundled as part of the output of a webpack build (or similar bundlers).

This may give us a hint as to whether @carbon/telemetry is being included, or if there is something else in play here.

Since all the dependencies are declared as "dependencies" and not as "devDependencies", these must be packaged into production code.

This will depend on how the application is bundled, I think. Items in the dependencies field aren't necessarily brought into the resulting bundle unless it is included in the entry point provided to the bundler (in the case of tools like webpack) or there is some other configuration at play.

Here's a quick flow using @vue/cli that brings in @carbon/telemetry as an example. At the end of the video, you can see that adding in the dependency does not change the resulting bundle.

https://user-images.githubusercontent.com/3901764/122796442-00733b80-d284-11eb-9719-d4af7e929f3b.mov

This doesn't mean that you aren't experiencing this problem, just that it may be dependant on project configuration or setup in order to re-create what you're running into 👍 I think figuring out what's in the bundles will be a great next step to figure out where the size increase comes from in the chunks that you mentioned.

schuetza commented 3 years ago

Thanks for the instructions. Re. the size it looks like I blamed the wrong component. The source-map-explorer is showing that 2.38.2 is pulling in a new carbon version, an that pulls icons/vue in, and that makes the size of carbon to grow. So in fact, there are two issues:

one with icons/vue growing the carbon layer
one with telemetry, pulling in dev dependencies as production dependencies, which in turn requires us to do OSS clearance for.

Do you want me to split this defect up into two?

For the size issue, I'll provide a Box folder to you so that you can inspect our changes (carbon/vue uplevel from 2.37.0 to 2.38.2) including the resulting package-lock, and the out put from the source-map-explorer. Let me know if you have issues to access that files.

joshblack commented 3 years ago

@schuetza sounds good! Thanks for clarifying.

For the telemetry note, unfortunately there is no way for us to list it as a devDependency and have it run on postinstall 😞 . This is why the package currently lists it as a dependency to make sure it gets included and the postinstall step can run.

For @carbon/icons-vue, most likely something is going on with tree-shaking that is causing the bundle to de-opt. Could you share more about how you're bringing in the icons?

schuetza commented 3 years ago

Re. the carbon/icons-vue:

In our vue.config.js file, this is pulled in via transpileDependencies

const webpack = require("webpack");
const fs = require("fs");
const CopyWebpackPlugin = require("copy-webpack-plugin");

module.exports = {
  transpileDependencies: ["@carbon/vue", "@carbon/icons-vue", "@carbon/icon-helpers"],
  configureWebpack: (config) => {        

  ...

Re. telemetry: I don't understand that and can't accept. We have other dev.dependencies, that run post-install steps, see extract of our pod build below. As you can see in the npm ls output above, they keep the production dependency tree clean.

23:15:27 > core-js@3.12.1 postinstall /home/bpmbuild/workspace/DBAoC-UI_Personal/bundles/com.ibm.automation.cloud.instance.ui.bpm/node_modules/core-js
23:15:27 > node -e "try{require('./postinstall')}catch(e){}"

23:15:27 > node-sass@5.0.0 install /home/bpmbuild/workspace/DBAoC-UI_Personal/bundles/com.ibm.automation.cloud.instance.ui.bpm/node_modules/node-sass
23:15:27 > node scripts/install.js

schuetza commented 3 years ago

Maybe there is a misunderstanding.For us it is OK to have the @carbon/telemetry added, but it should not add it's development dependencies to the tree.

joshblack commented 3 years ago

@schuetza the @carbon/telemetry package is listed as a dependency on packages like carbon-components, carbon-components-react, and @carbon/vue so that the postinstall step will work as expected for packages using it. If it was listed as a dev dependency then unfortunately it wouldn't be installed when the package itself was installed.

The @carbon/telemetry package itself will list dependencies that it needs, I don't believe it's specifying a development dependency in its package.json.

schuetza commented 3 years ago

Since we didn't get any solution or acceptance here, and we are not willing to use such a missused package tree (packaging dependencies on a web library), we started to work around this by locally overriding the telemetry package with a dummy package w/o any dependencies. However, we are happy if we can get a clean solution from carbon in the (near) future. So if you like, you can close this issue with 'won't fix' or keep it open if you think this is a valid issue.

joshblack commented 3 years ago

Just to reiterate what was shared above, this package is following a typical structure for a CLI package. It does not have a main, module, exports, etc. entrypoint and lists a bin entrypoint for the CLI. The dependencies listed by the package are necessary for the CLI to function.

None of the code for the CLI, or dependencies for the CLI, should be included in any bundle shipped to a user. If there is evidence of this happening, please provide steps for us to reproduce and we can investigate. It most likely is a specific bundler that is being impacted versus a general problem.

It seems that, in this case, the specific issue is that @carbon/telemetry is considered a production dependency instead of a development dependency. As a result, when installing production dependencies it will be included in a node_modules folder if you have carbon-components, carbon-components-react, etc listed under dependencies.

On our end, the only direction we can take is to list out dependencies under devDependencies and publish the package as a pre-built file that includes the dependencies. We'll definitely investigate this path and see how feasible it is 👍

I'll go ahead and mark this as closed. If folks are running into a bundle size issue that's being delivered to an end-user, please make an issue with steps to reproduce and we will 100% look into it.

carbon-design-system / carbon