search

pinpoint-apm / pinpoint

APM, (Application Performance Management) tool for large-scale distributed systems.
https://pinpoint-apm.gitbook.io/
Apache License 2.0
13.29k stars 3.75k forks source link

Whether a failed sql execution can marked the span as failed. #7518

Open tankilo opened 3 years ago

tankilo commented 3 years ago

Prerequisites

Please check the FAQ, and search existing issues for similar questions before creating a new issue.YOU MAY DELETE THIS PREREQUISITES SECTION.

Supposed that when a tomcat deal with a http request, many sqls are executed and some of them failed. But these failed sql are not important, so the application still return 200 http status code to the front user. Then in pinpoint, the span of the tomcat request is marked as failed.

image

This is done by com.navercorp.pinpoint.bootstrap.context.SpanEventRecorder#recordException(java.lang.Throwable)

image

I think if user has already catched the exception and prevent it from spreading to the outer method. Then the span shouldn't be marked as a failed call.

tankilo commented 3 years ago

Currently , we make a change if the application return a http status code that no included in profiler.http.status.code.errors list, we will override the error status of the trace root as succeful. With this change , if the user want to mark the tomcat request as failed, it must return http status code that included in profiler.http.status.code.errors list.

RoySRose commented 3 years ago

I think PR would be nice. But how about the other way around. I think most of the users want it to be marked as a failure when there is an SQL exception. So it would be nice to make a list of SQLs(or SQL exceptions) that should not be marked as a failure.

With your proposed method. A request that runs several jobs(ex, multiple queries, query+ another request... ) can't be distinguished.