fix(grpc): allow custom error_handler for client interceptor

[rfwroo]

What does this PR do?

In #1583, the option of passing a custom error_handler to gRPC interceptors was added. This is useful for e.g. selectively marking certain gRPC status codes as an APM error.

However, the change was implemented only for gRPC server interceptors. This patch:

• updates DatadogInterceptor::Client to pass the error_handler into the Tracing.trace call
• updates DatadogInterceptor::Base to read the error_handler from the interceptor itself (i.e. what's passed here) instead of the default/global configuration. It's not clear why this wasn't needed for the Server interceptor.

Motivation:

To allow grpc.client spans to be selectively marked as errors based on gRPC status codes.

Additional Notes:

N/A

How to test the change?

Example:

interceptor = Datadog::Tracing::Contrib::GRPC::DatadogInterceptor::Client.new do |c|
  c.error_handler = proc { |span, error| span.set_error(error) unless error.is_a?(GRPC::NotFound) }
end

# then pass interceptor into gRPC stub

For Datadog employees:

• If this PR touches code that signs or publishes builds or packages, or handles
  credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
• This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

[TonyCTHsu]

👋 @rfwroo , this looks promising. Let me take some time to play with and give feedback about this 😄

[marcotc]

Thank you so much, @rfwroo! 🙇

[TonyCTHsu]

👋 @rfwroo , do you think it make sense to add a new configuration settings for the client's error handler instead of sharing the same error handler?

[rfwroo]

Hey @TonyCTHsu,

do you think it make sense to add a new configuration settings for the client's error handler instead of sharing the same error handler?

Assuming that you mean the global error handler (i.e. datadog_configuration[:error_handler] from here)?

Allowing a global error handler for each span kind sounds neat 👍

I could follow up with a separate PR if you're happy to merge this as-is?

[TonyCTHsu]

👋 @rfwroo, Sorry for my late reply.

Currently, there is only one error_handler configuration for the grpc integration. The error_handler method, is defined in the parent object DatadogInterceptor::Base and is only used by server interceptor

Applying global error handler to client interceptor is NOT actually adding a new function, it changes the existing behaviour for users already have error handler defined, which could be an undesired side-effect.

In order to have a smoother upgrade to other users, could you implement a new configuration option client_error_handler instead of sharing the same error handler? We could make it explicit that the error_handler would be only applying to server from the document.

[rfwroo]

@TonyCTHsu that sounds good.

We could make it explicit that the error_handler would be only applying to server from the document.

Would it make sense to also add a new server_error_handler configuration option, and deprecate the existing error_handler option instead?

[TonyCTHsu]

Would it make sense to also add a new server_error_handler configuration option, and deprecate the existing error_handler option instead?

We want to avoid changes for the existing user, so deprecating without removing it makes sense to me. 😄

[codecov-commenter]

Codecov Report

Merging #3095 (0e08b4b) into master (88f4425) will decrease coverage by 0.01%.
Report is 171 commits behind head on master.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3095      +/-   ##
==========================================
- Coverage   98.15%   98.15%   -0.01%     
==========================================
  Files        1323     1324       +1     
  Lines       75048    75103      +55     
  Branches     3422     3427       +5     
==========================================
+ Hits        73665    73718      +53     
- Misses       1383     1385       +2     

see 11 files with indirect coverage changes

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