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

👋 @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?

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?

👋 @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.

@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?

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 Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.15%. Comparing base (88f4425) to head (0e08b4b).
⚠️ Report is 5908 commits behind head on master.

@@            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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.