Make README example less pseudo-y

[bcomnes]

Still WIP (see the one question), but I took a stab at improving the README example.

• Breaks out the otel.js and the app.js into distinct code blocks
• Set up development trace/span providers.
• Set up basic host metrics and runtime metrics (stuff you you usually want)
• Default to the auto instrumentation for the fastify plugin, and clarify how to use the loader flags
• Make the manual plugin loading process a secondary example (seems more esoteric, please correct me if I'm wrong)
• Graceful shutdown example

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[bcomnes]

The one open question I ran into was, do you also need to hook the sdk.stop() method into a fastify lifecycle, so that metrics/spans/traces drain correctly? If so, we should have an example of that.

[bcomnes]

This is what the node-sdk recommends:

// You can also use the shutdown method to gracefully shut down the SDK before process shutdown
// or on some operating system signal.
const process = require("process");
process.on("SIGTERM", () => {
  sdk
    .shutdown()
    .then(
      () => console.log("SDK shut down successfully"),
      (err) => console.log("Error shutting down SDK", err)
    )
    .finally(() => process.exit(0));
});

[bcomnes]

Reading through the code, this plugin does NOT automatically close down the SDK on server stop. We should add the best way to do this. I'm guessing in the otel.js file as the example shows.

[metcoder95]

Reading through the code, this plugin does NOT automatically close down the SDK on server stop. We should add the best way to do this. I'm guessing in the otel.js file as the example shows.

Noup it currently does not; you'd like to open a PR for that? We can extend the documentation there as well.

The best is to hop into the onPreClose hook to stop the instrumentation and let telemetry get flushed.

[metcoder95]

I'd set this up as an alternative; often instrumentation is just imported manually first thing when booting up a service

[bcomnes]

The reason why the the loader flags are appropriate as the default suggestion is because fastify-cli. Rather than directly launching some entry point with node where you control load order, many fastify apps will just have an app.js that fastify-cli loads. In order to guarantee the otel instruments are the first thing that loads, you must load them in with the loader/reqiuire flag otherwise the fastify-cli might load in a module before the otel instruments can intercept it.

Additionally the otel tutorials and docs recommend the loader flag approach. I found that working through those docs, and then hopping over to this module and seeing two different approaches super confusing.

https://opentelemetry.io/docs/languages/js/getting-started/nodejs/
https://github.com/open-telemetry/opentelemetry-js/blob/main/doc/esm-support.md

(Speaking of loader flags, I need to add the experimental loader note for esm)

[metcoder95]

Fair enough; I'd just add a note that import is also an alternative

[Eomm]

all this code could be moved to an example.js file instead of the readme?

it seems too much here to me (and we can't test it too)

[bcomnes]

I agree. Will add in a runnable example in.

[bcomnes]

I reduced the scope of the example in the README. I will still set up an external example folder.

[bcomnes]

A few more updates:

• Reduce the overall scope of the example (remove host metrics and runtime metrics)
• Move FastifyOtelInstrumentation into the instrumentations array. It's my understanding this is the preferred way to set up instruments in otel, and it handles things like setting the setTracerProvider automatically.
• Add a graceful shutdown to the example
• Add more information about loading otel in ESM and add fastify examples
• Remove any use of the serverName setting, because this is probably a mistake to have that setting Don't set ATTR_SERVICE_NAME in fastify instrumentation  #70

Next steps I will set up a runnable example folder or two, and narrow down the README example more.