feat: add new instrumentations into auto-instrumentations-node

[rauno56]

Which problem is this PR solving?

As @pizzaz93 pointed out, some of the new instrumentation packages were missing from the auto-instrumentation-node package.

Short description of the changes

Added all contrib packages to the metapackage.

Checklist

• Ran npm run test-all-versions for the edited package(s) on the latest commit if applicable.

[codecov]

Codecov Report

Merging #981 (83cb5da) into main (72430f8) will increase coverage by 0.24%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #981      +/-   ##
==========================================
+ Coverage   96.08%   96.32%   +0.24%     
==========================================
  Files          14       20       +6     
  Lines         893     1060     +167     
  Branches      191      216      +25     
==========================================
+ Hits          858     1021     +163     
- Misses         35       39       +4     

Impacted Files	Coverage Δ
...tapackages/auto-instrumentations-node/src/utils.ts	98.18% <100.00%> (ø)
...etry-instrumentation-router/src/instrumentation.ts	96.15% <100.00%> (ø)
...instrumentation-router/src/enums/AttributeNames.ts	100.00% <0.00%> (ø)
.../opentelemetry-instrumentation-router/src/utils.ts	100.00% <0.00%> (ø)
...ntelemetry-instrumentation-router/src/constants.ts	100.00% <0.00%> (ø)
...etry-instrumentation-router/src/enums/LayerType.ts	100.00% <0.00%> (ø)

[Flarna]

Isn't it needed to update also utils.ts?

[rauno56]

It is! I don't even have any excuses. Added a test for it for the future.

[blumamir]

Liked the new test.
LGTM

[Flarna]

Not related to this PR. It seems the dependencies are not updated automatically here if an instrumentation is published. Don't know if we can automate this or somehow tell lerna to do this.

[rauno56]

Not related to this PR. It seems the dependencies are not updated automatically here if an instrumentation is published. Don't know if we can automate this or somehow tell lerna to do this.

Don't you think open ranges for those dependencies takes care of that?

[Flarna]

I think this could work. But I guess the range needs to be * as other ranges may skip parts - in special for the 0.x ranges.
But this removes quite some control from end users.

[rauno56]

Not sure I understand what you mean exactly. What do you think would be the ideal version specifiers for the instrumentation deps?

[Flarna]

I think the optimum would be that this module is automatically updated whenever another module is released. And the version should be pinned. I assume here that updates of dependencies like @opentelemtry/instrumentation are done atomic here.
This gives end users an easy way to select the instrumentation version.

If ranges like ^0.28.0 are used you get only new patch versions, for ^1.2.0 aditionally minors. only if you use * as range you get latest.
But if this is done users get new instrumenations automatically except if they use a lock file.

[rauno56]

I don't have a strong preference, but * feels a bit too broad to me during a major bump from 0 to 1. On the other hand as long as the meta package is unstable it should be fine either way.

[rauno56]

Going through release-please logs and release PR, the dependencies does seem to be updated, @Flarna. That was exactly what you had in mind, right?

[Flarna]

Yes, that seems ok. But maybe remove the ^ prefix?
So pinning @opentelemetry/auto-instrumentations-node results in pinning all instrumentations.

[rauno56]

I've now learned that pinning is probably even better idea than I thought because the version @opentelemetry/instrumentation package has to match for all of the packages for the tests to run, otherwise we'll get a type mismatch because of the concrete classes used in the instrumentation package. Ideally those should be interfaces instead, but that's a fix for the future. Do you think we could get away with ~ range as well?

This is also something to keep in mind in the future whenever we update any of the deps in the meta-pacakges.

[rauno56]

@Flarna, had to revert some of the changes in 098c2ed to bring @opentelemetry/instrumentation to the same version.

[Flarna]

Suggested change

	"@opentelemetry/instrumentation": "0.27.0",
	"@opentelemetry/instrumentation": "0.28.0",

[Flarna]

Suggested change

	"@opentelemetry/instrumentation-grpc": "0.27.0",
	"@opentelemetry/instrumentation-grpc": "0.28.0",

[rauno56]

That's what I meant by ☝️. Updating this leaves @opentelemetry/instrumentation out of sync with other packages and tests fail because of a type mismatch.

[Flarna]

Suggested change

	"@opentelemetry/instrumentation-http": "0.27.0",
	"@opentelemetry/instrumentation-http": "0.28.0",

[Flarna]

There should be nothing left using 0.27.0. All the instrumentations here (expect http and grpc) should be linked by lerna and main should be on 0.28.0 meanwhile.
http/grpc from npm in version 0.28.0 should match.

[rauno56]

I'll try it out. I guess I must've had something left behind for the whole dep tree to end up in a mess.

[rauno56]

I do agree that after your suggested edits install should bring in 0.28 but I'm not seeing it, so unless I manage to get everything to upgrade in union, I will continue holding http and grpc packages back.

[Flarna]

Maybe some leftover package-lock.json files? maybe try git clean -fdx.

Maybe #1012 helps also as I found npm is not dedup in all cases.

[rauno56]

Maybe some leftover package-lock.json files? maybe try git clean -fdx.

The CI also has the same issue:

Argument of type 'AmqplibInstrumentation | AwsLambdaInstrumentation | AwsInstrumentation | BunyanInstrumentation | ... 26 more ... | WinstonInstrumentation' is not assignable to parameter of type 'Instrumentation'.
  Type 'AmqplibInstrumentation' is not assignable to type 'Instrumentation'.

[rauno56]

There should be nothing left using 0.27.0. All the instrumentations here (expect http and grpc) should be linked by lerna and main should be on 0.28.0 meanwhile.
http/grpc from npm in version 0.28.0 should match.

I could achieve that by updating all instrumentations here to use ^0.28.
^0.28 and ^0.27 have no overlap because of the different behavior of the tilde in the 0.x range.

[blumamir]

@rauno56 - This PR is still relevant, right? just needs to be updated (mostly version numbers I guess)

[rauno56]

Yep .. I'll rebase and let's get it merged.

[github-actions]

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.