Proposal: Package Structure

[open-telemetry]

Proposal: Package Structure

[SergeyKanzhelev]

opentelemetry/instrumentation-*
mongodb, redis, ioredis, express, hapi etc.

There is a discussion on how to call it: open-telemetry/opentelemetry-specification#30

[mayurkale22]

Are transports interestingly different because of the need to propagate context and so are a different group?

Absolutely. To me, transport sounds more familiar when it comes to HTTP/2 and GRPC protocols. Having said that, I am fine with re-using opentelemetry/instrumentation-[transport] name.

[draffensperger]

What's the motivation for having a separate opentelemetry/transport-* package group rather than opentelemetry/instrumentation-[transport] like opentelementry/instrumentation-http, etc.? Are transports interestingly different because of the need to propagate context and so are a different group?

I'm fine with it, just wanted to clarify!

[rochdev]

opentelemetry or opentelemetry/api or opentelemetry/core

It's a bit unclear where the no-op implementations are stored. Should it be a different package or should it be part of this one?

My vote goes to core if it contains any behavior, or api if it's only interfaces.

opentelemetry/context-propagation or opentelemetry/context-cls

As discussed in opentracing/opentracing-javascript#112 (comment), this package should be split into multiple different packages.

For example:

• opentelemetry/context-base: The base implementation that doesn't propagate the context but handles things like binding.
• opentelemetry/context-async-listener: Implementation based on @datadog/async-listener for Node <8. If our fixes for promise binding + memory leak + interoperability can be merged back into async-listener then that should be used instead.
• opentelemetry/context-async-hooks: Implementation based on async_hooks for Node >=8.
• opentelemetry/context-zones: Implementation based on Zones for the browser.

My vote goes to context-* for the various implementations.

opentelemetry/cls-tracer or opentelemetry/automatic-tracer

My vote definitely goes to automatic-tracer as cls would be an implementation detail (which would probably be incorrect too).

opentelemetry/propagation-*

Where would the base classes/interfaces go?

opentelemetry/instrumentation-* / opentelemetry/transport-*

As discussed above, separating these may require patching the same module multiple times.

In general, I think the structure looks good with a few changes. The most important part I think will be to ensure that each of them can be used on its own, and have many hooks to allow altering the behavior based on vendor needs. This is one area where both OpenCensus and OpenTracing were lacking, which I think is the main reason that prevented adoption.

[rochdev]

Since a lot of the packages above could be used for the browser as well, should the name of this repo be changed to something like opentelemetry-js?

[rochdev]

opentelemetry/context-propagation or opentelemetry/context-cls

Thinking about this a bit more, I think this could be externalized as an external project since context propagation is a generic cross-cutting concern. Maybe opencontext?

The benefits would be that users could use this to store anything, so they could use it as a replacement for continuation-local-storage and cls-hooked. The former is no longer supported with new versions of Node and the latter only supports specific versions and has some very difficult to fix issues. None of them support the browser.

I think providing a universal implementation could be beneficial outside the realm of OpenTelemetry.

[draffensperger]

I like the idea of calling this opentelemetry-js. It will require more complex CI checks since we would want to test out the browser packages in multiple browser and the Node packages on multiple Node versions, but it would enable us to keep everything in sync much better than separate repos.

[mayurkale22]

opentelemetry or opentelemetry/api or opentelemetry/core

IMO this package will have api interfaces and no-op implementations. This might look like this,

opentelemetry/core
   |__ trace/
      |__ propagation/
      |__ samplers/
   |__ metrics/
      |__ stats/
   |__ tags/
      |__ propagation/
   |__ resources/
   |__ utils/

I am in favor of core, but I think other languages(Java, Python and Go) are going with api.

[draffensperger]

Would you expect the core implementation to be Node-specific or would it be shareable with the browser (similar to the OpenTracing tracer)?

The benefit of making it cross-platform would be code sharing, but then we need separate hooks for the platform specific parts. Alternatively, as long as the api keeps a clean set of types that core implements, we could have a core-web that is optimized for the web platform but implements the same interfaces, and then core could use Node-specific utilities as well.

It seems like with OpenTracing, the API package had shared code but was pretty lean and allowed for plugging it into either Node or web needs (see e.g. https://github.com/elastic/apm-agent-nodejs and https://github.com/elastic/apm-agent-rum-js both of which integrate with OpenTracing)

[rochdev]

I would try to keep as much code as possible shareable, and add more modules for platform specific code. Then some parent module can compose the correct modules together and register them with each other.

For example, say there is a web-tracer. It would use automatic-tracer and register context-zones on it as the context propagator.

I think this approach could allow more different combinations of modules, even unrelated to the platform. This is just a theory however, so I'd like to know if anyone has used this kind of approach for this use case specifically.

[draffensperger]

@crdgonzalezca is doing some experimental work to build a Zone-based tracer for the browser. You can check out some of his very early stage work here: census-instrumentation/opencensus-web#87

Since we haven't built it yet, I'm not really sure what it will take to make Zone.js do what we want. I like the idea of sharing code between the browser and Node, but I think it will require some discipline to make sure Node-specific stuff doesn't creep in since likely the Node use case will be more fully built out sooner than the web-specific one.

I think we could mitigate this by setting up a web build of the intended-to-be-shared packages very early on with some simple integration test that exercises the webpack compile+build process and makes sure it can get spans correctly in the browser.

[rochdev]

@crdgonzalezca is doing some experimental work to build a Zone-based tracer for the browser.

There was also some work done for context propagation specifically in https://github.com/opentracing/opentracing-javascript/pull/113/files#diff-e02a2e69cef841e50802298da59382a3

I think we could mitigate this by setting up a web build of the intended-to-be-shared packages very early on with some simple integration test that exercises the webpack compile+build process and makes sure it can get spans correctly in the browser.

I definitely agree with that. Otherwise, the more we wait, the more difficult it will be to support the browser because of things we won't have thought about. In general I find that tests are a lot more difficult to share between Node and the browser than the actual code, but I'm not sure that it's necessary either.

[mayurkale22]

As mentioned in the SIG meeting, these are the new package names I would like to advocate.

1. opentelemetry/basic-tracer -> opentelemetry-basic-sdk
   By default basic-sdk contains support for all the telemetry signals i.e. tracing, metrics, logs etc. It is possible to have separate packages for each telemetry signal, but currently not covered under default SDKs(basic-sdk and node-sdk) implementation.

2. opentelemetry/node-tracer -> opentelemetry-node-sdk
   This package should help users/vendors to load specific plugin(s). Mostly applicable for advanced usres.

3. opentelemetry-node
   This package should load all the provided plugins, we expect most of the applications to use this package.

The rest should stay as it is : opentelemetry-types, opentelemetry-core, opentelemetry-exporter-, opentelemetry-plugin-.

/cc @OlivierAlbertini @vmarchaud @rochdev @hekike @draffensperger @bg451 @markwolff @danielkhan

[draffensperger]

What about having opentelemetry-basic-sdk just re-export the exported symbols from opentelemetry/basic-tracer (and perhaps in the future opentelemetry/basic-metrics and opentelemetry/basic-logs, etc.)?

That way if someone wants to use e.g. just traces or just metrics as a power user they still can

[mayurkale22]

I'm going to close this issue via #408. Do add a comment if you think this is incorrect.