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

Instrumentation examples should use new NodeSDK style

Open AWinterman opened this issue 2 years ago • 8 comments

It is confusing as a new comer to see contradictory or different examples about how one should go about configuring the collector. Examples should be unified in the new way of doing things.

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

AWinterman avatar Apr 27 '22 23:04 AWinterman

Setup/configuration of collector is not JS specific and should be most likely discussed in the collector repository.

Regarding SDK setup there is no new/old way.

  • Using (still experimental) NodeSDK tries to give you as much as OTel can.
  • Using BasicTracerProvider/NodeTracerProvider gives you only traces.

It depends on the use case what fits better. e.g. in AWS lamda or similar FaaS setups time series like metrics are often useless as the process hosting the function is frozen most of the time.

Flarna avatar Apr 28 '22 05:04 Flarna

This is the open telemetry js repository, correct? I'm referring specifically to the js sdk, which is us specific.

I'm aware there is an old and a new way. That fact is not acknowledged in most documentation, which is confusing for beginners.

How should a new comer distinguish the new and old way? How are they to know which examples pertain to which way?

AWinterman avatar Apr 28 '22 06:04 AWinterman

There is no new and old way. There are different ways and you have to choose one depending on what you would like to archive.

I agree that documentation is by far not perfect. It's a community driven project and PRs to improve doc are very welcome. In special from newcomers because experienced developers tend to overlook the beginner topics or use wording not easy to understand for newcomers.

Flarna avatar Apr 28 '22 08:04 Flarna

Of course. I'm happy to help out there

  • Can trace setup be used interchangeably in the two ways?
  • Is one deprecated?

Perhaps all that's needed is additional examples of the new way?

AWinterman avatar Apr 28 '22 12:04 AWinterman

Yes, for trace you can use NodeTracerProvider or NodeSdk. NodeSdk uses NodeTracerProvider internally - it's just a higher level entry point which covers more then tracing. In browser the entry point to setup tracing is WebTracerProvider (there is no BrowserSdk)

NodeTracerProvider and WebTracerProvider are based on BasicTracerProvider. It's even fine to use this one to setup tracing. It gives even more control - and therefore is slightly harder to use.

Please note also that NodeTracerProvider, WebTracerProvider and BasicTracerProvider have reached GA state already (contained in packages of version >=1.0.0). NodeSdk is still experimental as it includes metrics which are experimental.

NodeSdk is simpler to use and targets a "just works" setup for "default" setups. As development is ongoing users of NodeSdk may face breaking changes in upcoming releases until it's GA.

Flarna avatar Apr 29 '22 13:04 Flarna

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 Jul 04 '22 06:07 github-actions[bot]

As somebody new to OTel, and just trying to get a basic setup working, this has been a source of confusion for me. Examples I've seen have instrumentation config, but it doesn't appear to be valid any longer. The specific snippet (linked above) from the example code I'm talking about is:

const tracerProvider = new NodeTracerProvider({
  resource: Resource.default().merge(
    new Resource({
      [SemanticResourceAttributes.SERVICE_NAME]: 'aws-otel-integ-test',
    }),
  ),
  idGenerator: new AWSXRayIdGenerator(),
  instrumentations: [
    new HttpInstrumentation(),
    new AwsInstrumentation({
      suppressInternalInstrumentation: true,
    }),
  ],
});```

kevin-mitchell avatar Jul 15 '22 15:07 kevin-mitchell

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 Sep 19 '22 06:09 github-actions[bot]

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

github-actions[bot] avatar Jul 24 '23 06:07 github-actions[bot]