document: fix express example

[rauno56]

Which problem is this PR solving?

The express example was erroring and used soon-to-be-deprecated API. I'm bringing that up to date.

---

Short description of the changes

Mainly just removing the usage of the old plugins API and replacing that with registerInstrumentations. I also added logger configuration because especially for the newcomers it will definitely be helpful to see those messages and know how to configure the logger. Other than that it's functionally the same.

---

A question for the original express instrumentation developer(@vmarchaud, I believe): Is it expected that if HTTP layer is not instrumented, express instrumentation is a no-op? Should we document that? Error? Somehow make HTTP instrumentation a dependency for express?

If that looks alright, I could work on others as well.

Fixes #376

[codecov]

Codecov Report

Merging #413 (57d39f1) into main (a2de775) will not change coverage.
The diff coverage is n/a.

❗ Current head 57d39f1 differs from pull request most recent head 2444d21. Consider uploading reports for the commit 2444d21 to get more accurate results

@@           Coverage Diff           @@
##             main     #413   +/-   ##
=======================================
  Coverage   94.21%   94.21%           
=======================================
  Files          12       12           
  Lines         432      432           
  Branches       48       48           
=======================================
  Hits          407      407           
  Misses         25       25           

[vmarchaud]

A question for the original express instrumentation developer(@vmarchaud, I believe): Is it expected that if HTTP layer is not instrumented, express instrumentation is a no-op? Should we document that? Error?

I indeed wrote the instrumentation and its expected that without the http instrumentation, the instrumentation does nothing. I don't think this is a problem right now (either in term of documentation or behavior) but maybe i don't see it, why do you think it should be documented ?

[...] Somehow make HTTP instrumentation a dependency for express?

Thats not really possible, instrumentation should not depend on each user, specially because you need to be able to configure them. If the express instrumentation load its http instrumentation itself, the user can't control its config.

[rauno56]

[...] why do you think it should be documented ?

Because the alternative - registering express instrumentation (without HTTP inst) and not seeing anything - is worse and will definitely frustrate people who expect at least something to show up in their tracing UI when they enable their first instrumentation. As a very fundamental use-case, it will disproportionally affect people less experienced with OTel, which is something we want to especially avoid as a relatively young effort.

Thats not really possible, instrumentation should not depend on each user, specially because you need to be able to configure them. If the express instrumentation load its http instrumentation itself, the user can't control its config.

The way to do that might be unprecedented or unobvious, but I'm sure we can figure something out! :) OR if this really is the case (for now) it should at least be documented IMHO. Do we have any other takes on that?

I'll write something up regardless, but leave the behaviour as it is for now. Please hold the merge until the commit has been added.

[Flarna]

I indeed wrote the instrumentation and its expected that without the http instrumentation, the instrumentation does nothing.

That's not really true. The express instrumentation creates only spans if there is a current active span but there is no check that this span was created by http instrumentation. It will create a child on whatever parent is present.

[rauno56]

That's not really true. The express instrumentation creates only spans if there is a current active span but there is no check that this span was created by http instrumentation. It will create a child on whatever parent is present.

True, good point! What could be the other possible expected sources for that parent span? Might there also exist a case where the spans created by express instrumentation are attached to an unexpected parent span if the HTTP instrumentation is not there?

Please hold the merge until the commit has been added.

I'm now done proposing the changes to the docs!