Merging stable API packages

[legendecas]

• This only affects the JavaScript OpenTelemetry library

Multiple stable API packages for each signal can create confusion for end-user and maintenance burdens for duplications. I can see one of the main concerns of merging the API packages, specifically @opentelemetry/api and @opentelemetry/api-metrics, is that unused signals can burden the final bundle size.

I verified tree-shaking on the API packages and found that we can actually drop the unused API package code pieces in the bundle with common tools, webpack, and rollup: https://github.com/legendecas/tree-shaking-otel-api

I talked to other maintainers and many of us agreed that merging the API packages can alleviate the cognitive load on end-users, but we'd also like to address concerns about having a unified API package.

/cc @open-telemetry/javascript-approvers your opinions and insights on this idea are much appreciated!

[dyladan]

The @open-telemetry/javascript-maintainers discussed this in slack and it was generally agreed that the DX of a single API package is significantly better and less confusing than multiple API packages. If the tree shaking is effective, I don't see a strong advantage of separate packages.

We agreed to create this issue in order to gather other opinions as we don't want to make such a change without at least considering everyone's opinion.

/cc @MSNev as you have had the opposite opinion in the past. is the tree shaking argument sufficient for you?

[MSNev]

I am against combining (and linking) all of the logs / metrics (and eventually logs) into a single combined common namespace for the api as legendecas calls out in their tree-shaking-otel-api test

/**
 * If this line is uncommented, all apis (including DiagAPI, TraceAPI,
 * PropagationAPI) are included in the final bundle.
 */
// import { context } from '@opentelemetry/api';

That is because if everything is linked to the same namespace tree-shaking cannot safely determine what can be dropped.

This does not preclude "including" metrics within the api package -- just as long as it has its' own namespace eg something link api-metrics and it's NOT linked or used by anything by the default api namespace.
So api-metrics can reference and use anything in api but api cannot reference or use anything in api-metrics

This is one of the things I wanted to have done already (via the sandbox) where the plan (still) is to have the main branch

• create browser "bundles" (I was using rollup)
• run browser and worker tests for all targeted packages

That way we would have somewhere to have solid numbers on tests which would break on any size regressions (based on the size of the generated browser bundles.

So....
If you are just going to "include" the api-metrics namespace in the "@opentelemetry/api" npm package then I don't have any direct issue as lon as it conforms to the above ( 'api' does not import api-metrics) and all metrics are in their own separate namsepace.

[dyladan]

So api-metrics can reference and use anything in api but api cannot reference or use anything in api-metrics

Some version of this rule is required no matter what to avoid circular dependencies.

If you are just going to "include" the api-metrics namespace in the "@opentelemetry/api" npm package then I don't have any direct issue as lon as it conforms to the above ( 'api' does not import api-metrics) and all metrics are in their own separate namsepace.

If I am understanding this right, you think this would be ok:

import { metrics, trace } from '@opentelemetry/api';

const meter = metrics.getMeter(name, version);
const tracer = trace.getTracer(name, version);

As long as metrics never references trace (and likely vice versa is also never going to happen) and the patch suggested by @legendecas is applied.

[MSNev]

Yes, it should be

[dyladan]

#3329 did not mark the API packages. I believe they should also be safe to mark because simply loading the files does not cause any global state change. A function must be called in order to modify global state.

[legendecas]

I'm going to merge the two api packages. I'll try to find a way to add a regression test to guarantee that using a single signal should not include the other signals in the final bundle.

I can mark the API package as side-effect-free too.