Plugins refactoring - new instrumentation package for plugins

[obecny]

!Important! -> the only thing that left is test coverage, but I want this to be reviewed before adding many more tests as especially the tests for node will require more effort, so would rather avoid massive refactoring if there is anything here that will need to be changed before that.

This is continuation of plugin refactoring. #1449

This new plugin base is completely decoupled from the SDK and relies only on the API. After #1448 plugins using this base class will be able to be fully standalone, and will support instrumenting modules very early in the load process, even before the tracer provider is set up.

• Solves load order issue
• Solves yarn pnp issue by using a more direct dependency model
• Removes need for plugin manager
• One instrumentation can patch multiple modules
• Can use different plugins for different versions of a module
• Supports tracers and meters
• Support for patching multiple files before they are loaded by main module. For example graphql doesn't allow to patch main file as it defines all methods as read only so they cannot be override. This will fix the problem allowing to patch files and methods before.

For some time we will allow for 2 plugins types until all plugins will be moved to use new class.

FYI
After this change the plugins http and https can be merged into one plugin too

[codecov]

Codecov Report

Merging #1540 into master will decrease coverage by 0.12%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master    #1540      +/-   ##
==========================================
- Coverage   93.18%   93.05%   -0.13%     
==========================================
  Files         156      158       +2     
  Lines        4854     4882      +28     
  Branches      986      988       +2     
==========================================
+ Hits         4523     4543      +20     
- Misses        331      339       +8     

Impacted Files	Coverage Δ
...entelemetry-instrumentation/src/instrumentation.ts	63.63% <63.63%> (ø)
packages/opentelemetry-api/src/trace/NoopLogger.ts	100.00% <100.00%> (ø)

[dyladan]

This looks really solid. I left some comments mostly to do with the README since I think writing instrumentation will be difficult for people their first time. The actual implementation is more or less exactly what I was thinking.

[dyladan]

LGTM right now. Are you planning on getting reviews on this and merging, then doing tests in a follow up PR?

[obecny]

LGTM right now. Are you planning on getting reviews on this and merging, then doing tests in a follow up PR?

Ideally I would add more tests in following up PR after getting more reviews too. It will be easier also to add tests then and make some adjustments if required as the PR is already not the smallest one so might be harder to make reviews with more code on board.

[obecny]

@dyladan just to be sure, the NoopLogger can stay in api right ?

[dyladan]

@dyladan just to be sure, the NoopLogger can stay in api right ?

I think that's fine, but it is a little weird to me that the logger is only used in the SDK but has a noop in the API. I think I am fine either way on this one. The biggest strike against for me is that it is harder to remove things from the API later if we decide it was a bad idea, but easy to add it later if it causes a problem not having it.

[obecny]

@dyladan just to be sure, the NoopLogger can stay in api right ?

I think that's fine, but it is a little weird to me that the logger is only used in the SDK but has a noop in the API. I think I am fine either way on this one. The biggest strike against for me is that it is harder to remove things from the API later if we decide it was a bad idea, but easy to add it later if it causes a problem not having it.

It will be used now in Instrumentation so all plugins will need that

[dyladan]

This looks good to me. It may need some modifications as we actually start writing plugins, but I think we can merge this and try to port one of the plugins or write a new one.

[mwear]

One comment, but otherwise, LGTM.