Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Confusing documentation mixes different packages and entrypoints #3230

Closed
1 of 2 tasks
Tracked by #2
ringerc opened this issue Sep 5, 2022 · 6 comments
Closed
1 of 2 tasks
Tracked by #2

Confusing documentation mixes different packages and entrypoints #3230

ringerc opened this issue Sep 5, 2022 · 6 comments
Labels
document Documentation-related help wanted Extra attention is needed stale up-for-grabs Good for taking. Extra help will be provided by maintainers

Comments

@ringerc
Copy link

ringerc commented Sep 5, 2022

  • This only affects the JavaScript OpenTelemetry library
  • This may affect other libraries, but I would like to get opinions here first

The opentelemetry docs here could really use a bit of a prune to remove references to obsolete packages, and to clarify the correct way to set up autoinstrumention.

There are multiple overlapping "main" npm packages used:

The docs for sdk-node don't mention sdk-trace-base or sdk-trace-node.

It's hard to know whether the correct way to init the opentelemetry js provider is with NodeTracerProvider:

const { NodeTracerProvider } = require('@opentelemetry/sdk-trace-node');
const provider = new NodeTracerProvider();
provider.register();

or with NodeSDK:

const opentelemetry = require("@opentelemetry/sdk-node");
const { JaegerExporter } = require("@opentelemetry/exporter-jaeger");
const { getNodeAutoInstrumentations } = require("@opentelemetry/auto-instrumentations-node");
const sdk = new opentelemetry.NodeSDK({
    traceExporter: new JaegerExporter(),
    instrumentations: [getNodeAutoInstrumentations()],
})

Neither package references the other. All have recent releases.

Then there are multiple autoinstrumentation interfaces. Both these provide different autoinstrumentation APIs:

and again, it's far from clear which should be used. sdk-node links to auto-instrumentations-node, but the instructions for auto-instrumentations-node use NodeTracerProvider from sdk-trace-node not NodeSDK from sdk-node. Is it still correct to use getNodeAutoInstrumentations()?

Some stale links too. E.g. https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/metapackages/auto-instrumentations-node links to https://github.com/open-telemetry/opentelemetry-js/tree/main/packages/opentelemetry-instrumentation-http which is 404.

Another point of new-user bewilderment has been that the otel SDK is documented as requiring that it must be loaded before other components in the app. But node --require can only run cjs modules, which don't support await, so it's unclear how it's possible to wait for the otel sdk to be ready before loading other application components.

@Flarna Flarna added help wanted Extra attention is needed up-for-grabs Good for taking. Extra help will be provided by maintainers labels Sep 5, 2022
@Flarna
Copy link
Member

Flarna commented Sep 5, 2022

Actually there is no single right way. It's up to the user to decide which variant to use.

Some points one has to think about (there are likely more points):

  • Which backend do I want to use to collect/analyse/.. my data?
  • Do I need metrics or only traces?
  • Are autoinstrumentations needed or do I use only manual created spans?
  • Do I want to depend on all auto instrumentations available or do I pick that ones I really need/want?
  • Do I want to reduce the number of experimental packages?
  • Do I need automatic context propagation?

If one don't want to invest the time to get familiar with OTel to decide this I recommend to use an OTel variant packaged by some company.

That said I agree that doc could be improved. But as usual the experts knowing all the fine grained details how to use miss the newbie view. I add the "help wanted" and "up-for-grabs" labels to indicate this.

Another point of new-user bewilderment has been that the otel SDK is documented as requiring that it must be loaded before other components in the app. But node --require can only run cjs modules, which don't support await, so it's unclear how it's possible to wait for the otel sdk to be ready before loading other application components.

In short instrumenting ESM modules is not yet supported. Better stick of CJS if you want working autoinstrumentation. Work is ongoing to improve this but as long as the looder hooks on node side are experimental and have breaking changes every now and than it's not the likely that this should be considered as production ready.

@ringerc
Copy link
Author

ringerc commented Sep 5, 2022

Some guidance in the docs on how to select appropriate packages and entrypoint APIs would definitely be helpful. Right now it's exceedingly confusing, unclear which packages are current vs obsolete, etc.

I am investing the time to become familiar with opentelemetry, and I use OpenTelemetry in other languages with far fewer issues. My node experience is definitely weak, but even it has been a lot harder to get anything up and running on node than on any of the other stacks I've instrumented.

Some clarity in the package docs on what is deprecated vs not, and how the various packages inter-related would be most welcome.

Another example of confusion between the APIs:

It looks a bit like NodeSDK is supposed to be a high-level wrapper and convenience interface, but none of the other docs are written in terms of NodeSDK, and there's no mapping showing how the NodeSDK interface relates to the underlying APIs. The docs at https://www.npmjs.com/package/@opentelemetry/sdk-node do not identify it as experimental, though it's in the "experimental" dir in git.

So from a usability perspective, I'm trying to share new user experiences of how confusing this is to encounter, and some things that might help. I know that I find it immensely valuable to have people who're working with my own code and documentation give me their first impressions, before they've figured it all out and understood it, as it helps me smooth the UX and fix pitfalls I don't see anymore because I already know it all.

@cartermp
Copy link
Contributor

@ringerc I think your issues may be best sorted out in the OTel docs, where there is a lot of what you mention here: https://opentelemetry.io/docs/instrumentation/js/instrumentation/

Happy to clarify more stuff there as well

@legendecas legendecas added the document Documentation-related label Sep 13, 2022
@github-actions
Copy link

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
Copy link

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 github-actions bot added the stale label Feb 27, 2023
@github-actions
Copy link

This issue was closed because it has been stale for 14 days with no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
document Documentation-related help wanted Extra attention is needed stale up-for-grabs Good for taking. Extra help will be provided by maintainers
Projects
None yet
Development

No branches or pull requests

4 participants