opentelemetry-go-contrib icon indicating copy to clipboard operation
opentelemetry-go-contrib copied to clipboard

OtelEcho swallows the error and does not pass it to other middlewares

Open hcelaloner opened this issue 2 years ago • 1 comments

Background

Hi, we were trying to use OpenTelemetry echo middleware provided in this repository in our project. However, using it causes misfunction in other middlewares. For instance, while trying we were using another middleware to send errors to our current apm tool and it stopped recording errors.

It seems that the current implementation of echo middleware swallows the error and does not pass it to other middleware (https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/instrumentation/github.com/labstack/echo/otelecho/echo.go#L101).

Proposed Solution

  • Return the error received from the next middleware call.
  • Also don't call the registered HTTP error handler since it is automatically called if the error received from the next middleware call is returned.

hcelaloner avatar Apr 21 '22 06:04 hcelaloner

Submitted a PR with the proposed solution but I guess with this solution middleware is not able to detect the correct HTTP status code returned since this time HTTP error handler is called later. So the only way to handle is to leave it as it is? If that is the case adding test cases may be a good idea? Or should we register it as the last middleware if we want to see capture in other middleware?

hcelaloner avatar Apr 21 '22 07:04 hcelaloner

FYI, the Echo project had a similar issue with their Recover middleware. They decided to make a flag to allow the user to let the error drop through.

Edit: Although in hindsight, they don't need to know about the status code, so they don't need to call the error handler...

The way we handle this is just to ensure this middleware is first. Not an ideal solution though :/

markhildreth-gravity avatar Mar 23 '23 19:03 markhildreth-gravity