Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error in span does not cause entire request to report error in APM #723

Open
alexrhogue opened this issue Oct 24, 2019 · 15 comments
Open

Error in span does not cause entire request to report error in APM #723

alexrhogue opened this issue Oct 24, 2019 · 15 comments
Labels
community local-root Issues requesting API access to the local root span. question Further information is requested

Comments

@alexrhogue
Copy link

alexrhogue commented Oct 24, 2019

I am having an issue with getting an error in a span to propagate up and cause the entire trace to be marked as a failure.

I want to provide my team with the ability to notice a critical error in an http request (even if the code catches that error and handles it graceful). To achieve this I am create a span and causing it to error out. This works. However, the trace created by the express plugin (and I assume the http plugins as well) does not seem to care about a failed.

I think I must be missing something. But I can't find any clear examples or documentation on how to achieve this.

example code:

  tracer.trace('noticeError', (span, onError) => {
        onError(new Error('notice error'));
    });

Environment

  • Operation system:
    OSX 10.14.3
  • Node version:
    v11.15.0
  • Tracer version:
    dd-trace@0.15.4
  • Agent version:
    6.14.1
@alexrhogue alexrhogue added the bug Something isn't working label Oct 24, 2019
@rochdev rochdev added community question Further information is requested and removed bug Something isn't working labels Oct 24, 2019
@rochdev
Copy link
Member

rochdev commented Oct 24, 2019

I'm not sure to understand exactly the expected behavior. Are you trying to add an error to a specific span, or add an error to the root span of the tracer? Or are you trying to add an error to the root span whenever there is another span that has an error?

@alexrhogue
Copy link
Author

alexrhogue commented Oct 24, 2019

Thanks for the quick reply. I am trying to add an error to the root span of the tracer.

@rochdev
Copy link
Member

rochdev commented Oct 24, 2019

Are errors reported as 200 status code? You can control which status codes result in a trace-level error. Otherwise, I'm afraid I'll have to provide a workaround since the tracer considers a handled error as a success from the request perspective.

@rochdev
Copy link
Member

rochdev commented Oct 24, 2019

Another question: do you expect any error on any span to flag the trace with an error, or only specific ones?

@alexrhogue
Copy link
Author

  1. The errors are reported as 200 status code
  2. Just a specific one

@rochdev
Copy link
Member

rochdev commented Oct 25, 2019

Ok, then it should be possible to flag the request and use that on the request span in a hook. It would look like this:

// right after tracer.init()
tracer.use('express', {
  hooks: {
    request: (span, req, res) => {
      // assuming somewhere in your app you set req.error = true
      if (req.error) {
        span.setTag('error', req.error)
      }
    }
  }
})

You could also set the error to an actual Error object instead of true but I wouldn't recommend it. This would give the impression that the error was thrown by the request (i.e. actually bubbled up) and would be redundant since you will be able to find the real error on one of the spans in the trace. It's up to you however what works best for your use case.

Please let me know if that helps!

@alexrhogue
Copy link
Author

Trying that right now. Thank you.

@alexrhogue
Copy link
Author

Seems to be working great. Request is showing up as an error. And I can inspect the spans to find the actual error. Only issue I see is that some code paths where I may want to 'noticeError' may not currently have access to the req.

I'm currently attempting to search through the spans in the express middleware to find my error span. I'll keep you posted!

Thanks again.

@rochdev
Copy link
Member

rochdev commented Oct 25, 2019

It's also possible to access the request span from any span at any time during the request lifecycle, but there is no public API to do it yet so you would have to rely on private properties.

If this is something you are comfortable with, it would look like this:

// anywhere during the request lifecycle
const span = tracer.scope().active()

if (span) {
  span.context()._trace.started[0].setTag('error', true)
}

@alexrhogue
Copy link
Author

I think that is exactly what I need. Having to use private props is not ideal but I think I can work with it for now. Should I open another issue to request that as a first party feature?

@rochdev
Copy link
Member

rochdev commented Oct 25, 2019

Sure thing! This is definitely something that is on our radar but we want to make sure the API will cover all uses cases, which is why it hasn't landed yet.

@bengl bengl added the local-root Issues requesting API access to the local root span. label May 18, 2021
@ajwootto
Copy link

ajwootto commented Dec 13, 2021

I found this issue when trying to figure out how to get APM Error tracking working. I may be missing something, but since the Error Tracking view only shows errors that are attached to root-level spans, it basically never shows any error that occurred during a request, even if that request returned a 500.

When the built-in error handling of express (or in my case, NestJS) catches the uncaught error and sets up the 500 response, that means there's no "automatically" captured error that will show up in error tracking. It seems like the only way to use error tracking correctly in this case is with the above workaround to manually attach the error to the request span in a custom error handling middleware?

@rochdev does that sound right?

@rochdev
Copy link
Member

rochdev commented Dec 13, 2021

@ajwootto Sounds right. If there is an error object available we could capture that is available and we don't currently capture it's definitely something we should add. Otherwise it has to be done in a hook.

@ajwootto
Copy link

ajwootto commented Dec 13, 2021

I actually am having trouble getting this working even with the above workaround. It seems that something later up in the chain is overriding the error tag to just be "true".

Edit:

I managed to get it working eventually. While the errors now show up in error tracking, this does feel like I'm using APM "incorrectly" in order to make the error tracking work. I was under the impression that it would be somewhat of a replacement for Sentry or Rollbar and automatically catch and group any errors causing 5xx errors in our API, and that unfortunately does not seem to be the case without "tricking" it into having the error present on the root span.

@billoneil
Copy link

Here is a workaround my team just implemented that should work without messing with the spans directly. This might be an option for the library to expose this functionality. #1944 (comment)

Haven't pushed it to production yet but early testing seems to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community local-root Issues requesting API access to the local root span. question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants