chore: use npm workspaces and a package-lock.json

[trentm]

• use npm workspaces instead of lerna for monorepo install handling (requires at least npm@7, so node v14 users will have to update their npm)
• updates to lerna@6
• add package-lock.json for more reliable dev and testing

Refs: open-telemetry/opentelemetry-js#4238

clean start in your git clones

The change from lerna to npm workspaces means that most dependencies will be
installed in the top-level node_modules dir. To avoid surprising conflicts
from node_modules and possible package-lock files in subdirs from local dev,
you should start fresh:

Either do a full git clean -ffdx (it is the only way to be sure) -- which will delete all non-git files -- or do this:

rm -rf node_modules \
  packages/*/{package-lock.json,node_modules} \
  plugins/node/*/{package-lock.json,node_modules} \
  plugins/node/*/examples/{package-lock.json,node_modules} \
  plugins/web/*/{package-lock.json,node_modules} \
  plugins/web/*/examples/{package-lock.json,node_modules} \
  propagators/*/{package-lock.json,node_modules} \
  detectors/node/*/{package-lock.json,node_modules} \
  metapackages/*/{package-lock.json,node_modules}

Then install and build, test and lint:

npm ci             # or 'npm install'
npm run compile

npm test
npm run lint

smaller install, faster CI builds

One benefit is that size of a git clone after npm ci is down 10X -- from
~8.6GB to ~800MB. As a result, clean builds should be faster.

• Compare a recent "unit-test" CI run against "main": https://github.com/open-telemetry/opentelemetry-js-contrib/actions/runs/6670946547/usage

  browser-test (14) 35m 14s
  unit-test (14) 43m 49s
  unit-test (16) 16m 42s
  unit-test (18) 18m 57s
  

• To a recent build of the PR: https://github.com/open-telemetry/opentelemetry-js-contrib/actions/runs/6791291162/usage

  browser-test (14) 14m 22s
  unit-test (14) 9m 7s
  unit-test (16) 9m 7s
  unit-test (18) 11m 35s
  

reviewer notes

• s/prepare/prepublishOnly/g is to avoid running the compile step in packages before npm install for all packages has completed. It resulted in various build errors. Using prepublishOnly is recommended (https://docs.npmjs.com/cli/v10/using-npm/scripts#prepare-and-prepublish) and is what the core repo was already doing.
• I've updated lint.yml and release-please.yml to use node 18 (was node 14), so it has an npm which supports workspaces. I assume that is okay.
• I don't know how the release-please PRs are created. I haven't tested anything around that.

[codecov]

Codecov Report

Merging #1771 (40ef62e) into main (1b0caa6) will not change coverage.
The diff coverage is n/a.

❗ Current head 40ef62e differs from pull request most recent head 5732850. Consider uploading reports for the commit 5732850 to get more accurate results

@@           Coverage Diff           @@
##             main    #1771   +/-   ##
=======================================
  Coverage   91.44%   91.44%           
=======================================
  Files         143      143           
  Lines        7273     7273           
  Branches     1458     1458           
=======================================
  Hits         6651     6651           
  Misses        622      622           

[david-luna]

I guess you removed the caches to test the CI and not mess with the existing ones. I think we can prefix it so there is no collision with the existing ones.

[trentm]

I guess you removed the caches to test the CI and not mess with the existing ones.

No. npm ci deletes node_modules and does a clean re-install, so the caching doesn't help. I'm copying what the equiv PR for the core repo did. See cache discussion there: open-telemetry/opentelemetry-js#4238 (comment)

t2t2 did mention on that thread that we could enable npm cache on the setup-node action:

You can however enable npm cache in actions/setup-node which will cache the local packages cache (npm-cache)

So that is worth trying out.

[RichiCoder1]

Howdy! Saw this PR while browsing.

Unless you have a good reason not too, dropping container.image and using setup-node w/ cache: npm should net you a decent perf win. There's a non-zero overhead to using container images, and setup-node natively caches both node installs and npm caches now.

[trentm]

Unless you have a good reason not too, dropping container.image and using setup-node w/ cache: npm should net you a decent perf win.

@RichiCoder1 Thanks. I'll try that out on the .github/workflows/lint.yml workflow a little later and compare job run times.
First I need to figure out this unit-test (18) test failure.

[codecov]

Codecov Report

Merging #1771 (f382586) into main (4c8b374) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1771   +/-   ##
=======================================
  Coverage   91.42%   91.42%           
=======================================
  Files         143      143           
  Lines        7303     7303           
  Branches     1461     1461           
=======================================
  Hits         6677     6677           
  Misses        626      626           

Files	Coverage Δ
...umentation-user-interaction/src/instrumentation.ts	92.25% <ø> (ø)

[trentm]

The unit tests finally passed with node v18. This took longer than it should have.

tl;dr: The following assert was failing in Node v18 because the output from the ConsoleSpanExporter was marked-up with ANSI color escape codes, which broke the stdout.includes("name: 'GET'") check.

opentelemetry-js-contrib/metapackages/auto-instrumentations-node/test/register.test.ts

Lines 38 to 39 in 1b0caa6

	//Check a span has been generated for the GET request done in app.js
	assert.ok(stdout.includes("name: 'GET'"));

The output was colored because nx is by default adding the FORCE_COLOR=true environment variable.
Only Node v18 was affected because in v18.17.0, FORCE_COLOR started being honoured for non-TTY streams (stdout is not a TTY when run piped into anything, as in CI).
Setting FORCE_COLOR=0 (or false), is the only way to prevent that with envvars.

Notes:

• It wasn't about adding Node adding color support for CI systems in tty: support more CI services in getColorDepth nodejs/node#40385. That is in recent node v16 versions as well. The Node change allowing FORCE_COLOR to ignore isatty is lib: support FORCE_COLOR for non TTY streams nodejs/node#48034
• Setting node's NODE_DISABLE_COLORS=1 or NO_COLOR=1 doesn't work, because FORCE_COLOR wins.
• The nx code that is defaulting FORCE_COLOR is here: https://github.com/nrwl/nx/blob/15.9.7/packages/nx/src/tasks-runner/forked-process-task-runner.ts#L130-L140
• This failure doesn't show in any previous runs is because we were using lerna@5, which didn't use nx.

[trentm]

@legendecas or someone else: Could you please remove the pkg:instrumentation-aws-sdk label? This is resulting in all the TAV tests failing due to the already known #1477 issue. I might look at that issue, but not in this PR.

Other than that, I think everything is basically working here now. I think we could merge this now, or wait for the renovate PR to update to the 0.45.1/1.18.1 release first.

[pichlermarc]

Wow, this must've been a chunk of work. Thank you so much for working on this @trentm.
We've desperately needed this change for a while now, so I'm very happy to see this PR!

It's a huge change but it looks good to me, very high quality overall. Tested locally and everything seems to work for me. @dyladan would also appreciate your review as you're more familiar with the release automation in this repo. 🙂

[pichlermarc]

Waiting for one more CI run to finish. If there are no more comments until after the SIG meeting today (18:00 GMT) I'll merge this.

[pichlermarc]

Ah, I think this still needs one more branch update and update to its package-lock.json as the most recently merged PR modified dependencies - then we can merge this.

[trentm]

I think this still needs one more branch update and update to its package-lock.json

I can do that now.

[trentm]

Ready to merge now.