chore: upstream mocha instrumentation testing plugin from ext-js

[blumamir]

Read more about it here.
Basically, it's a mocha plugin that implements many of the repeated boilerplate code needed for instrumentation testing, in a consistent, idiomatic fashion.
It makes writing instrumentation tests an easier task, with less code, and less room for doing things wrong, by ensuring that there is only a single instance of the instrumentation under test, and cleaning state between tests runs.

This PR is the first step for upstreaming Aspecto's opensource instrumentations from ext-js repo to the contrib. I upstream it first in order to create smaller, more reviewable PRs, with less changes in each.

The next step will be to upstream the aws-sdk instrumentation which relies on this mocha plugin for it's unit tests

Which problem is this PR solving?

• When writing unit test for instrumentation, there is a lot of boilerplate things to do: create trace provider, memory exporter and span processor, context manager, create the instance of the instrumentation under test. These components need to be initialized, configured, cleaned between tests, enabled\disabled and managed in each instrumentation in the repo.
  For one .spec file this is relatively easy, but when instrumentation has multiple .spec.ts files, each file has to set these up again and again, and due to the way mocha runs test files, it's very easy to mess it up and cause spans to be written to the wrong memory exporter, update the wrong instrumentation instance, use provider from other file etc.
  This PR is a suggestion for mocha plugin and documented workflow which aims to solve many of these issues by using singleton instances of these components, similar to how the SDK is expected to be setup and run in an instrumented application.

Short description of the changes

• move source files in @opentelemetry/test-utils from root to src directory and adjust package.json and tsconfig.json accordingly, so the structure matches the rest of the packages in the repo
• add mocha plugin to test-utils package which can be used in instrumentation tests, along with documentation on how to use in the package README.
• refactor mongodb instrumentation tests to demonstrate the use of the plugin in an instrumentation from contrib

[codecov]

Codecov Report

Merging #621 (6babb6f) into main (1b7ae9c) will decrease coverage by 1.62%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #621      +/-   ##
==========================================
- Coverage   96.68%   95.06%   -1.63%     
==========================================
  Files          13       71      +58     
  Lines         634     5042    +4408     
  Branches      124      490     +366     
==========================================
+ Hits          613     4793    +4180     
- Misses         21      249     +228     

Impacted Files	Coverage Δ
...ce-detector-gcp/test/detectors/GcpDetector.test.ts	100.00% <0.00%> (ø)
...detector-aws/test/detectors/AwsEcsDetector.test.ts	100.00% <0.00%> (ø)
...opentelemetry-instrumentation-mongodb/src/types.ts	100.00% <0.00%> (ø)
...e/opentelemetry-instrumentation-redis/src/utils.ts	92.98% <0.00%> (ø)
...metry-instrumentation-mysql/src/instrumentation.ts	93.38% <0.00%> (ø)
...pentelemetry-instrumentation-mongodb/test/utils.ts	91.17% <0.00%> (ø)
...ce-detector-aws/src/detectors/AwsLambdaDetector.ts	100.00% <0.00%> (ø)
...de/opentelemetry-instrumentation-pg/src/version.ts	100.00% <0.00%> (ø)
...ource-detector-aws/src/detectors/AwsEc2Detector.ts	97.91% <0.00%> (ø)
...ages/auto-instrumentations-node/test/utils.test.ts	96.87% <0.00%> (ø)
... and 48 more

[obecny]

I like the idea, just one concern towards the name for registerInstrumentation, other than that lgtm

[blumamir]

I like the idea, just one concern towards the name for registerInstrumentation, other than that lgtm

Fixed
Thanks for the review :)

[dyladan]

Did you mean to apply it to the mongodb instrumentation or was that just a test?

[blumamir]

Did you mean to apply it to the mongodb instrumentation or was that just a test?

I did, to demonstrate how to use it.
Is it ok?

[dyladan]

Yes it's fine, just wanted to make sure it was intended

[dyladan]

Looks like node 16 is failing. Any idea why that's happening?

[blumamir]

Looks like node 16 is failing. Any idea why that's happening?

After merging with the lerna hoisting PR, now mocha is hoisted to the root node_modules, but it depends on the test-utils package which is not hosted. thus it fails to require it.

Adding mocha to the --no-hoist list will resolve the issue, but add an additional ~300MB of node_modules files which will increase CI time.
Still thinking of how to approach it

[willarmiros]

What's strange that this seems to be an issue with lerna hoisted modules, but it only fails with Node 16. Do we know why that is?

Also, regarding the fix for adding mocha to the --no-hoist list, I wonder what the behavior before hoisting was with mocha, and whether the added CI time is still much better than before and we could just live with it for now.

Alternatively, is there any way we could force the test-utils package to be hoisted? Perhaps adding it as a devDependency to the root package.json so it's always included with those root-level dependencies?

[blumamir]

What's strange that this seems to be an issue with lerna hoisted modules, but it only fails with Node 16. Do we know why that is?

Not actually. Tried to run it locally on node 14 and it also failed locally, so the mystery is even greater :/

Also, regarding the fix for adding mocha to the --no-hoist list, I wonder what the behavior before hoisting was with mocha, and whether the added CI time is still much better than before and we could just live with it for now.

Good question, I don't have the number on CI time, but in term of size, mocha is using 300 MB of node_modules out of the 1GB for non-hoisting setup, and hoisting reduced about 10 minutes from the CI time, so guess it will add about 30% of that back, about 3 minutes per run. If I don't find any other solution, we can consider that.

Alternatively, is there any way we could force the test-utils package to be hoisted? Perhaps adding it as a devDependency to the root package.json so it's always included with those root-level dependencies?

That's a great idea. On ext-js repo, we use lerna with yarn workspaces which hoist the intra-repo dependency to the root node_modules automatically as you suggested.
Unfortunately, npm fails when it tries to fetch the (private) package from the registry before lerna has the chance to link it properly. I remember there was a discussion on publishing it to npm. Believe it can solve the issue. @dyladan do you support publishing test-utils to npm?

[blumamir]

Fixed the issues, now tests are green.
Had to include mocha in the --nohoist list for it to work correctly.

If there are no additional comments after changes, this one is ready to get merged finally :)

[willarmiros]

LGTM! Ready to merge whenever possible @obecny @vmarchaud @dyladan