opentelemetry-js icon indicating copy to clipboard operation
opentelemetry-js copied to clipboard

Add sideEffects: false to package.json for packages to enable tree shaking

Open domasx2 opened this issue 3 years ago • 7 comments

Apologies if this is not supposed to be a bug type, wasn't sure which issue type fits best!

What version of OpenTelemetry are you using?

1.1.1

What version of Node are you using?

16.14.2

Please provide the code you used to setup the OpenTelemetry SDK

import { registerInstrumentations } from '@opentelemetry/instrumentation';
//import { DocumentLoadInstrumentation } from '@opentelemetry/instrumentation-document-load';
import { ConsoleSpanExporter, SimpleSpanProcessor } from '@opentelemetry/sdk-trace-base';
import { WebTracerProvider } from '@opentelemetry/sdk-trace-web';

const provider = new WebTracerProvider();
provider.addSpanProcessor(new SimpleSpanProcessor(new ConsoleSpanExporter()));

registerInstrumentations({
  instrumentations: [/*new DocumentLoadInstrumentation()*/],
});

What did you do?

I'm PoCing using opentelemetry-js in a library aimed at web application. It's rather hefty and produces a large js bundle when compiled with parcel or webpack. It seems that tree-shaking does not work with it.

What did you expect to see?

Smaller bundle size :)

What did you see instead?

Huge bundle, all of the code is included, even what isn't used by the library.

Additional context

It seems most packages do not have "sideEffects" property set. Manually going to node_modules and adding "sideEffects": false to every @opentelemetry/* package reduced bundle size by ~40kb! Please consider adding this property to every package.

More about it: https://webpack.js.org/guides/tree-shaking/

domasx2 avatar Mar 23 '22 15:03 domasx2

I think the problem for us to adopt the sideEffects property to be shipped with package.json is that we need a reliable tool to verify our settings are correct -- we can not declare a side-effectful file as side-effect-free, that will break consumers. Hand-tailored list of side-effect-free files is highly-likely to be broken from day-to-day maintenance.

legendecas avatar Mar 25 '22 03:03 legendecas

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] avatar May 30 '22 06:05 github-actions[bot]

We talked about this in the SIG meeting and agreed that this is safe for us. Even our modules which have side-effects do not execute those side-effects at load time and require a user to explicitly call a function. This will cause the static analysis of webpack to realize the module is used and not tree-shake it out.

dyladan avatar Jun 08 '22 16:06 dyladan

That would be awesome! Currently open telemetry packages are a no go for my company because it more than double our bundle size just for the sake of being able to trace 😥

ogxd avatar Jul 08 '22 12:07 ogxd

@ogxd if you have time a PR would be welcome

dyladan avatar Jul 08 '22 15:07 dyladan

I'll do that :) By the way, I notice that exporter-trace-otlp-http references otl-exporter-base which references otlp-transformer which references sdk-metrics-base. This sdk-metrics-base represents 50% of the bundle size according to this, while exporter-trace-otlp-http is only about traces. It seems that this dependency was added in 0.29.
I'm wondering if the tree shaking is able to get rid of that, I still see some metrics related things in my production bundle (OTEL_EXPORTER_OTLP_METRICS_TIMEOUT, AWS_DYNAMODB_ITEM_COLLECTION_METRICS, ...)

ogxd avatar Jul 11 '22 08:07 ogxd

I'm working on refactoring the otlp exporters right now. they're a mess

dyladan avatar Jul 11 '22 12:07 dyladan

I would like to pick this up if it's up for grabs.

pkanal avatar Oct 12 '22 15:10 pkanal

It's yours :slightly_smiling_face:

pichlermarc avatar Oct 12 '22 15:10 pichlermarc