chore: Update deprecations in instrumentation-express/examples

[jessitron]

Which problem is this PR solving?

• The example code for instrumentation-express is out of date.

Short description of the changes

• This updates the README to make the install work
• the opentelemetry library versions to the latest
• and the tracing code to switch all deprecated imports to their replacement.

[jessitron]

😢 I can't fix "npm ci" at the top level of this project, that isn't me

[trentm]

@jessitron I can try to help sort out updating deps so that npm ci works.

Starting with your latest changes, I found I was getting this error from npm when trying to run npm install ... commands to try to update versions of deps:

% npm install @opentelemetry/[email protected]
npm ERR! Cannot read properties of null (reading 'isDescendantOf')

...

I didn't dive in to try to figure out what that was about. Instead I started over with the examples/package.json from "main" and roughly did the following:

npm ci   # run this at the top-dir to install everything

cd plugins/node/opentelemetry-instrumentation-express/examples
npm uninstall @opentelemetry/instrumentation-express
npm install @opentelemetry/[email protected]
npm install @opentelemetry/resources
npm install @opentelemetry/sdk-trace-base @opentelemetry/sdk-trace-node @opentelemetry/semantic-conventions
npm install @opentelemetry/exporter-jaeger @opentelemetry/exporter-zipkin
npm install @opentelemetry/exporter-trace-otlp-proto

Note that I'm working towards using npm workspaces, and not using the npm install --workspaces=false that you've added to the README. I can understand wanting to use npm install --workspaces=false to just get a local build. I think that should be possible to work, but might be working "against the grain".

[codecov]

Codecov Report

Merging #1842 (a308e53) into main (c8bebc7) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1842   +/-   ##
=======================================
  Coverage   91.51%   91.51%           
=======================================
  Files         145      145           
  Lines        7427     7427           
  Branches     1486     1486           
=======================================
  Hits         6797     6797           
  Misses        630      630           

[trentm]

Here are a few attempts of my steps to get the express example built and working with a fresh git clone.
I think it might be nice to pick one of these and add some of these exact commands to the README to help newer users get the example working.

attempt 0

I didn't actually play with and try to get the npm install --workspaces=false way. I don't think it is necessarily a bad way to do it, but I think it will end up fighting the workspaces setup a bit.

attempt 1

Getting example working with a full build of all packages in the repo. This
unfortunately takes a long time.

# Get a build of the Express example and its dependencies.
git clone [email protected]:open-telemetry/opentelemetry-js-contrib.git
cd opentelemetry-js-contrib
npm ci
npm run compile
cd plugins/node/opentelemetry-instrumentation-express/examples
npm run compile

# Start an Zipkin to use viewing traces.
docker run --name example-zipkin --rm -d -p 9411:9411 openzipkin/zipkin

# Run the express server, configured to export to Zipkin.
npm run zipkin:server

# In a separate terminal, run the client that calls the express server.
npm run zipkin:client

Visit the Zipkin UI to search for spans for the client requests.

When finished then clean up:

docker kill example-zipkin

attempt 2 (requires a patch)

I was able to reduce the build time, somewhat by trying to install (via npm ci) and compile only in the required subdirs:

git clone [email protected]:open-telemetry/opentelemetry-js-contrib.git
cd opentelemetry-js-contrib/plugins/node/opentelemetry-instrumentation-express
npm ci
npm run compile
cd examples
npm ci
npm run compile

However, that requires this patch to work:

diff --git a/plugins/node/opentelemetry-instrumentation-express/package.json b/plugins/node/opentelemetry-instrumentation-express/package.json
index 91065940..edfb4477 100644
--- a/plugins/node/opentelemetry-instrumentation-express/package.json
+++ b/plugins/node/opentelemetry-instrumentation-express/package.json
@@ -12,7 +12,7 @@
     "clean": "rimraf build/*",
     "lint": "eslint . --ext .ts",
     "lint:fix": "eslint . --ext .ts --fix",
-    "precompile": "tsc --version && lerna run version:update --scope @opentelemetry/instrumentation-express --include-dependencies",
+    "precompile": "tsc --version && npm run version:update",
     "prewatch": "npm run precompile",
     "version:update": "node ../../../scripts/version-update.js",
     "compile": "tsc -p .",

and I'm not yet sure of the implications of doing this in general. Doing this probably should be done on a separate PR that looks at doing it generally for all the packages in this repo.

attempt 3

This installs all deps at the top-level (npm ci), but only runs the compile step in the express instrumentation and examples subdir. This is much faster and works:

git clone [email protected]:open-telemetry/opentelemetry-js-contrib.git
cd opentelemetry-js-contrib
npm ci
cd plugins/node/opentelemetry-instrumentation-express
npm run compile
cd examples
npm run compile

What do you think about documenting these steps in the README?

[JamieDanielson]

Thanks for this much-needed update!

I understand the frustrations around working with a monorepo. Perhaps we can document both workspaces=false for a simple local build and also one of Trent's suggestions around getting the true dependencies working with workspaces. I can add that as a follow-up PR.

[JamieDanielson]

Hm looks like the updated merge from main failed some status checks, think we should be able to run npm ci to fix it. @jessitron let me know if this is something you can update or if you want any assistance in updating.

[JamieDanielson]

Not entirely sure why but I'm unable to push to this PR from my local env.
I think I fixed this with a merge commit over here if you want to cherry-pick it (ae10db5). Alternatively if anyone else has access to push to this branch (@trentm ) can you help with this? I just did Accept Combination on the merge conflict for the example package.json, and another npm install at the root.

[trentm]

can you help with this?

Hopefully that should do it. I pulled your earlier merge, sync'd the package-lock.json file (npm install --package-lock-only), then merged from main again (which had some conflicts), then sync'd package-lock again (not sure why this last small change). The ways of package-lock are mysterious (and not a little frightening).

[trentm]

Assuming tests pass, I'm happy with this now.