Add mandatory rpc and grpc tags for grpc integration

[rarguelloF]

What does this PR do?

Adds the following tags to the grpc integration (both to client and server side interceptors):

• rpc.grpc.full_method => equivalent to /$package.$service/$method from the original protobuf definitions (not from Ruby class / method names)
• rpc.grpc.status_code => equivalent to the numeric value of the grpc response status code (https://grpc.github.io/grpc/core/md_doc_statuscodes.html)

Other changes are:

• Includes .proto files used for grpc tests and update them to use actual protoc generated code.
• Adds a script and docs for generating protoc Ruby files locally.
• Adds Contrib::GRPC::Formatting module for extracting grpc resource attributes from the full method string that comes in the client interceptor, and from the ruby grpc method object received on the server side one.

Motivation

Unify span tags across tracers.

Additional Notes

How to test the change?

[zarirhamza]

Looks mostly good, but just ensure the tag names are correct and some failing tests

[marcotc]

For lint failures, you can run bundle exec rubocop -a locally to fix most of them.

[codecov-commenter]

Codecov Report

Merging #2620 (c6c052a) into master (170ede5) will decrease coverage by 0.01%.
The diff coverage is 98.32%.

@@            Coverage Diff             @@
##           master    #2620      +/-   ##
==========================================
- Coverage   98.08%   98.08%   -0.01%     
==========================================
  Files        1160     1166       +6     
  Lines       63676    63821     +145     
  Branches     2849     2858       +9     
==========================================
+ Hits        62457    62597     +140     
- Misses       1219     1224       +5     

Impacted Files	Coverage Δ
spec/test_service_pb.rb	66.66% <66.66%> (ø)
...atadog/tracing/contrib/grpc/support/grpc_helper.rb	98.18% <92.85%> (-1.82%)	⬇️
...tracing/contrib/grpc/datadog_interceptor/server.rb	95.55% <93.33%> (+0.43%)	⬆️
lib/datadog/tracing/contrib/ext.rb	100.00% <100.00%> (ø)
...tracing/contrib/grpc/datadog_interceptor/client.rb	98.18% <100.00%> (+0.26%)	⬆️
lib/datadog/tracing/contrib/grpc/formatting.rb	100.00% <100.00%> (ø)
...ng/contrib/grpc/datadog_interceptor/server_spec.rb	100.00% <100.00%> (ø)
...adog/tracing/contrib/grpc/integration_test_spec.rb	100.00% <100.00%> (ø)
...ib/grpc/support/gen/grpc-1.19.0/test_service_pb.rb	100.00% <100.00%> (ø)
...upport/gen/grpc-1.19.0/test_service_services_pb.rb	100.00% <100.00%> (ø)
... and 7 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ivoanjo]

Since this PR is about a tracing integration, I'm humbly going to remove myself from the list of reviewers, since the folks working on tracing have a lot more context here than me :)

[TonyCTHsu]

Can this be replaced by requiring this file?

[rarguelloF]

I think so, thanks for catching this up!

EDIT: actually it's not possible due to circular import issues. There are 2 files in the gen folders, one called test_service and the other is test_service_services (this would look better if using actual service names, for example if the service was called auth, then you would have auth_pb.rb and auth_services_pb.rb). The first one contains the messages/payloads only and the other contains the rpc services. The spec/test_service_pb.rb file is a hack because the autogenerated code from test_service_services_pb.rb adds the line require 'test_service_pb' to import the messages/payloads, but assumes a project structure that doesn't match this project's one so it does not work without that file.

I will add a comment to the test_service_pb.rb explaining this so it's clear for future readers, and if you think there's an alternative to doing this please let me know 🙏

[TonyCTHsu]

Array#find would be iterating through the entire collection until the result is found. Do you know how large would the collection be? Is there better way to get the value?

[rarguelloF]

Do you know how large would the collection be?

This collection would contain all the methods (or rpcs in gRPC terminology) in that service. I can imagine most services having a magnitude of 10s or at most 100s (even though I already consider this case being weird). I guess potentially it could be anything but it doesn't seem a realistic scenario to me.

Is there better way to get the value?

Let me try to give a little bit of extra context:

• Assume you create an RPC service MyService with a method named MyMethod in your original .proto file - please note protobuf doesn't enforce any specific syntax, you could do my_method, MyMethod, myMethod, and so on.
• What we have grpc_method_object.name.to_s is always the method name with underscore syntax: my_method, regardless of what you did in the previous step. This is enforced by the protoc autogenerated RPC service Ruby class (if you name it myMethod or MyMethod` you will get an exception).
• In this section of code, you don't know what was the original choice in step 1, but you have the underscore version my_method. So if you loop over all the available original method names for the service (inside owner.rpc_descs) and transform them to underscore using ::GRPC::GenericService.underscore and matches my_method, then you know that is method you are looking for.

So given this situation, I don't think there's an alternative to this, but if you think there's one please let me know 🙏

[TonyCTHsu]

Is there a reason to invoke instance_variable_defined? instead of leveraging public method rpc_descs ?

[rarguelloF]

I added this check because for the case of the previous grpc Ruby class (manual) implementation this was not defined, and this is added by the protoc Ruby generated one.

Even though I just checked the grpc gem fails to start if you provide a class without this or this having no items.

Would you prefer to remove this check? (in that case I think we probably need to update some unit tests that are relying on a non-autogenerated grpc test service class if I recall correctly)

[marcotc]

Is this setting all gRPC metadata key/value pairs as span tags?
Isn't metadata arbitrary user data, meaning it could have PII?
If so, we can't simply tag it like this. Is this a requirement from some internal doc?

Also, the keys will become Datadog tag names, we should make sure they don't conflict our tag namespace, likely by namespacing them to metadata.METADATA_KEY = METADATA_VALUE.

[rarguelloF]

Hey @marcotc 👋 !
This code was already here before my changes (I just moved the lines around)
I think the concerns that you raise make sense but I think they would be out of scope of this PR. If you wanna discuss this offline please let me know!

[marcotc]

@rarguelloF, can you rebase this PR?

I'll merge it right after.