Add grpc client and server plugin

[nunofgs]

Hi DataDog team!

This PR adds automatic instrumentation support for grpc, both client and server. The client plugin is implemented using client interceptors and should be pretty resilient to upstream changes.

The server plugin however, has no such support and I took a stab at implementing — finding any injection point as best I could. It is fully functional but I cannot guarantee that it'll be as resilient as grpc-node evolves.

Let me know what you think!

• Fixes Add support for gRPC #311

Note: require-dir was removed since it doesn't support nested folders. There was already a file (src/plugins/index.js) with all of the supported plugins which I believe is even more appropriate to use.

[rochdev]

A lot of constructs incompatible with Node 4 were used and should be refactored to support it. You can use https://node.green/ when not sure if a feature is supported or not.

[rochdev]

Thanks for the PR! I did a first pass of review but in general the instrumentation itself looks good.

I scheduled this for 0.12.0 since we don't plan to release it before at least a couple weeks, so we should be able to include this feature in the release.

[rochdev]

It is fully functional but I cannot guarantee that it'll be as resilient as grpc-node evolves.

We have tests that run nightly so we should be able to find anything that breaks pretty quickly.

[rochdev]

Just noticed that the tests are not running in CI and should be added to .circleci/config.yml

[nunofgs]

Thanks for the in-depth review @rochdev and willingness to accept the PR :) !

I'll update the branch with your comments addressed shortly.

[nunofgs]

Just noticed that the tests are not running in CI and should be added to .circleci/config.yml

Missed that, thank you. Added.

[rochdev]

Ignore the node-mongodb-core errors, it's just noise because of an issue we are having with CircleCI. Postgres will need to be fixed however, but I'm not sure why it's failing. It must be related to adding grpc to the list of native modules. If you can't figure it out let me know and I will take a look when the PR is otherwise ready to merge.

[nunofgs]

Postgres will need to be fixed however, but I'm not sure why it's failing. It must be related to adding grpc to the list of native modules. If you can't figure it out let me know and I will take a look when the PR is otherwise ready to merge.

Found it. prev wasn't being used here. I ended up changing the implementation a bit but I can go back to an Array.reduce() if you prefer.

[rochdev]

Found it. prev wasn't being used here. I ended up changing the implementation a bit but I can go back to an Array.reduce() if you prefer.

Your implementation is a lot cleaner than Array.reduce so that's definitely fine. However, the leading and trailing slashes are no longer included for libpq. While this should not affect anything, I prefer to still have them to make it clear it's the actual name and not just part of a name. For example, what if some library would be called something-libpq? This also applies to grpc.

[nunofgs]

@rochdev sorry for taking so long to get back to this. Had a busy week.

I've updated the PR with support for cancellation and bidi streams (as well as tests of course). I believe all actionable comments should be addressed.

Mind doing another review?

[rochdev]

Overall looks good. I'll do one last pass when everything is resolved.

[rochdev]

@nunofgs One thing I'm wondering is about the context propagation. We are currently working on an HTTP2 integration which will handle context propagation, but right now you are doing it directly in gRPC. What do you think should be the expected behavior? Should we just drop propagation from gRPC at that point or is there still value to have it there?

[nunofgs]

@nunofgs One thing I'm wondering is about the context propagation. We are currently working on an HTTP2 integration which will handle context propagation, but right now you are doing it directly in gRPC. What do you think should be the expected behavior? Should we just drop propagation from gRPC at that point or is there still value to have it there?

🤔 gRPC uses http2 as transport protocol but it supports others and it's also possible to write your own. It might be useful to keep the gRPC context propagation for those cases.

[rishabh]

Let's also go through the plugin checklist and make sure we got everything.

[rochdev]

Plugin Checklist

• Unit tests.
• TypeScript definitions.
• TypeScript tests.
• API documentation.
• CircleCI jobs/workflows.
• Plugin is exported.

[rishabh]

Let's add some tests to ensure that we aren't modifying any metadata (except for the context propagation).

[rishabh]

I think we also need to add tests to check if the listener runs in the parent context for stream requests.

took ownership of the PR

[ebenoist]

👍