Add @opentelemetry/api as peerDependencies on @opentelemetry/sdk

[mayurkale22]

This is needed to set global TracerProvider and global MeterProvider correctly, especially when you use only @opentelemetry/api module to instrument your application and want to provide ways for users to register OpenTelemetry or any other SDK, if they wish to.

Looks like in the current state, every module will have their own copy of @opentelemetry/api. That means it is not really a global thing.

My current test setup:

@company/database (The database is instrumented with api)
  node_modules
    |-- @opentelemetry/api

@opentelemetry/node (Current opentelemetry strcuture)
  node_modules
    |-- @opentelemetry/api 
    |-- @opentelemetry/core
    |-- ...

sample-app (The app uses the database module and node sdk)
  node_modules 
    |-- @company/database
    |-- @opentelemetry/node

The alternative option is to use global objects, but in practice should generally be avoided.

[dyladan]

Even if we could peer-dep it in our SDK modules, the issue I see here is that not everyone can have it as a peer dep. @company/database needs to depend on it directly at the very least because they need to be able to call api methods and assume they're there. The same goes for the end-user application. In that case, there is no way to guarantee that the end-user application and @company/database have the same copy. If they specify different versions for instance, the module resolution will make it so that they have different versions.

At the very least, this means that any end-user app needs to specify the same version of the API as their dependencies do in order to have any hope that it works. If @company/database depends on 0.5.0 but the end user application depends on 0.5.1, the resolution algorithm will nest 0.5.0 inside the node_modules of @company/database and will put 0.5.1 in the root node_modules.

The only way I see around this is to declare the API as an object property on the global namespace. Then the API methods exported by the module delegate to the global version to ensure every copy of the API dependency is calling the same API instance.

[dyladan]

One more option I just thought of.

Have the API module not implement the actual API, but just be the way it is accessed. You then have the API module peer-depend on another module that is the actual instance of the API, and proxies calls to it. If there is no API actually installed, the proxy calls no-op. You then have the SDK depend on that second module which is the actual instance of the API. In this case, the API delegate module can be duplicated as many times as needed and only the SDK depended on by the SDK is the real API.

[dyladan]

I think we should discuss this wednesday

[dyladan]

I believe we will need to use the global object. This article does a good job of explaining why. https://derickbailey.com/2016/03/09/creating-a-true-singleton-in-node-js-with-es6-symbols/

[mayurkale22]

I think we should discuss this wednesday

Agreed. Added to the agenda for the upcoming SIG meeting.

[pauldraper]

Even if we could peer-dep it in our SDK modules, the issue I see here is that not everyone can have it as a peer dep. @company/database needs to depend on it directly at the very least because they need to be able to call api methods and assume they're there. The same goes for the end-user application. In that case, there is no way to guarantee that the end-user application and @company/database have the same copy. If they specify different versions for instance, the module resolution will make it so that they have different versions.

This is the entire purpose of peerDependencies. They are the solution to "only one library."

• Libraries -- whether OpenTelemetry SDK or @company/database -- declare @opentracing/api as a peerDependency (with whatever version requirements they have).

• Applications (i.e. containing code that calls setGlobalTracerProvider and setGlobalMeterProvider) declares @opentracing/api as a dependency.

npm then installs a single copy of @opentracing/api, and warns if the dependency is forgotten or incompatible with library requirements.

This is how many ecosystems operate: react component libraries, webpack plugins, etc.

You could mess with global, but IMO that'd just be a clever way to avoid a well-established pattern.

[dyladan]

@pauldraper then in this situation, you end up with a broken install:

1. db-driver peer-depends on @opentelemetry/api to provide ootb tracing to otel users
2. end-user application depends on db-driver
3. end-user application is not an opentelemetry user, and does not depend on opentelemetry api

Now, the db-driver package will assume the end-user has installed the API, which of course they haven't because they are not opentelemetry users. When db-driver requires the API, the application crashes.

[pauldraper]

npm warns the user of their broken install.

WARN [email protected] requires a peer of @opentelemetry/api@^1.0.0 but none is installed. You must install peer dependencies yourself.

And the user would add @opentelemetry/api to their package.json. (Though never use it themselves.)

But I now gather your preference/requirement is that @opentelemetry/api never appears directly in application package.json downstream. That is not a requirement for react or webpack or many other things because the user using them directly; opentelemetry is passive.

---

In summary,

Options

A. OT users depend on @opentelemetry/api. SDK libraries peer depend on @opentelemetry/api. #885 (comment) Cons: Not just OT users, but all downstream applications must have @opentelemetry/api as a dependency.

B. OT users depend on @opentelemetry/global. Instrumentations depend on @opentelemetry/api, which peer depends on @opentelemetry/global and binds to it if present. #885 (comment) Cons: npm issues warnings for non-OT downstream applications if they don't install @opentelemetry/global. (That said, npm warnings are already spammy, so maybe that doesn't matter.)

C. OT users depend on @opentelemetry/global. Instrumentations depend on @opentelemetry/api, which does not depend on @opentelemetry/global but binds to it if present. Cons: There is no npm checking for version compatibility. (Though @opentelemetry/api itself could do that.)

D. Instrumentations depend on @opentelemetry/api, which uses global or window. #885 (comment) Cons: There is no npm checking for version compatibility. (Though @opentelemetry/api itself could do that.)

I think all these are workable. I slightly prefer the simplicity of D.

---

P.S. https://derickbailey.com/2016/03/09/creating-a-true-singleton-in-node-js-with-es6-symbols/ is a strange read. global[Symbol.for('mything')] is no different than global.__mything. Maybe you prefer the ergonomics of symbol, but it took a very circuitous route to get something equivalent to the "obvious" answer.

[dyladan]

A. OT users depend on @opentelemetry/api. SDK libraries peer depend on @opentelemetry/api. #885 (comment) Cons: Not just OT users, but all downstream applications must have @opentelemetry/api as a dependency.

I would like to avoid putting this burden on users of libraries that want to support opentelemetry, because it may slow adoption due to many libraries not wanting to put this random burden on their users.

If I was a db library author, this would prevent me from supporting opentelemetry.

B. OT users depend on @opentelemetry/global. Instrumentations depend on @opentelemetry/api, which peer depends on @opentelemetry/global and binds to it if present. #885 (comment) Cons: npm issues warnings for non-OT downstream applications if they don't install @opentelemetry/global. (That said, npm warnings are already spammy, so maybe that doesn't matter.)

I do not want to add to the spam

C. OT users depend on @opentelemetry/global. Instrumentations depend on @opentelemetry/api, which does not depend on @opentelemetry/global but binds to it if present. Cons: There is no npm checking for version compatibility. (Though @opentelemetry/api itself could do that.)

This is probably OK, but I would probably change the names so that end-users depend on api instead of some cryptically named global package, and instrumentations depend on something like instrumentation-api or similar.

D. Instrumentations depend on @opentelemetry/api, which uses global or window. #885 (comment) Cons: There is no npm checking for version compatibility. (Though @opentelemetry/api itself could do that.)

This is the solution I recommended, but I could be talked into C.

P.S. derickbailey.com/2016/03/09/creating-a-true-singleton-in-node-js-with-es6-symbols is a strange read. global[Symbol.for('mything')] is no different than global.__mything. Maybe you prefer the ergonomics of symbol, but it took a very circuitous route to get something equivalent to the "obvious" answer.

The advantage of using a symbol is that it is then not an enumerable property on the global object, which prevents it from being listed using something like Object.keys(global) or similar. It also is far less likely to be accidentally collided with (although I concede that something like __opentelemetry_api is already very unlikely).

Also, if you use a string key then your property can be accessed from the global namespace without the global keyword which is not desireable. example:

// file1.ts
global.__otel = "opentelemetry"

// file2.ts
console.log(__otel) // opentelemetry

With symbol this is not possible

// file1.ts
const key = Symbol.for("io.opentelemetry.js.api");
global[key] = "opentelemetry";

// file2.ts
// There is no way to accidentally access the property here without
// using `Symbol.for` and `global` deliberately.

[pauldraper]

some cryptically named global package

I would move the setGlobal... functions out of api to this hypothetical package. IMO that name is descriptive of its contents.

It also is far less likely to be accidentally collided with

The likelihood of colliding by choosing same property name is the same as the likelihood of colliding with with the same symbol name.

not an enumerable property on the global object

True. Could be also done with Object.defineProperty.

your property can be accessed from the global namespace without the global keyword

True. You should have written that article. :)

[dyladan]

We talked about this again at the SIG meeting today. I am going to come up with a PR using the global method that we can talk about next week. Whoever is interested in joining a PR review meeting for it early next week comment here and i'll invite you.

[owais]

I ran into the same issue a couple of days ago when trying to run an Otel workshop. Node was neither parsing parsing propagation headers from incoming requests, nor injecting them into outgoing ones. Took quite some time to figure out what was going on. http plugins were using the global propagator from @opentelemtry/api but they had their own copy of it and module caching is FS path sensitive. As a result my app was setting the global propagator in one package and other packages were trying to find it somewhere else. Moving to proper global objects should fix it. Thanks @dyladan

[dyladan]

@owais any way you can test the #943 PR against your workshop code to see if it is fixed?

[dyladan]

No longer needed after #943

[owais]

Hey @dyladan, sorry for not replying. I somehow missed the notification. Would you still like me to test it?

[dyladan]

If you can that would be great!