DataDog / dd-trace-rb

Datadog Tracing Ruby Client
https://docs.datadoghq.com/tracing/
Other
310 stars 375 forks source link

Exceptions captured with `rescue_from :all` still appear in Error Tracking when using Grape integration #3253

Open llxff opened 1 year ago

llxff commented 1 year ago

Expected behavior

When using Grape's rescue_from :all to capture exceptions, I expect these exceptions to be handled internally and not to appear in Datadog's Error Tracking.

Actual behavior Despite using rescue_from :all, exceptions are still being sent to Datadog's Error Tracking. It seems that the error is assigned to the span before the rescue_from block is called, so the error still appears in the Error Tracking dashboard.

Additional context

If Grape's instrumentation is removed, the context in rack spans is lost, e.g.:

with Grape's instrumentation:

Screenshot 2023-11-10 at 16 29 10

without Grape's instrumentation:

Screenshot 2023-11-10 at 16 30 03
marcotc commented 1 year ago

Hey @llxff, thank you for bringing this issue up.

I looked into the implementation of https://github.com/ruby-grape/grape-on-rack/blob/03e03b78c52752c17c07c2f08430d3fe1544da12/api/rescue_from.rb#L4C39-L4C42 and I see that it will return a status code 500 will be returned. I assume that's the behavior you are seeing in your application.

In that case, the Rack integration will consider a 500 response as an error response, and mark it as an error for Error Tracking.

How could the product best work for you here, ideally? e.g. Would you like the grape and rack spans to be marked with error, but error tracking not being notified of it? Would you like either only one of rack or grape to be marked with error? Would like that none of the spans were marked as error?

llxff commented 1 year ago

Hi Marco, thank you for checking that!

Generally, rescue_from is used for handling exceptions and preparing contextual responses for clients. The status code is not necessarily 500, it could be anything depending on context: e.g. 422 if the error is related to invalid parameters or 401 if the error is related to authentication.

Ideally, the span should not be marked with an error if the exception is handled. In the end, you can log the exception if you need an alert in Datadog, right?