microsoft / ApplicationInsights-Java

Application Insights for Java
http://aka.ms/application-insights
Other
297 stars 199 forks source link

Handled exceptions being logged as exceptions in agent v3 #1472

Closed petergphillips closed 3 years ago

petergphillips commented 3 years ago

Expected behavior

Exceptions that are thrown from Spring Boot controllers and then handled by a Spring boot exception handler shouldn't be logged as exception in Application Insights.

Actual behavior

Application Insights exceptions is unnecessarily noisy as there are lots of client errors in the logs.

To Reproduce

Throw an exception from a Spring Boot controller that is then caught by a method annotated with org.springframework.web.bind.annotation.ExceptionHandler. Even though the exception is then handled by our code it is still logged as an exception. It appears that the io.opentelemetry.javaagent.shaded.instrumentation.api.tracer.BaseTracer intercepts the exception in the endExceptionally method after our controller is called and before the Spring Boot exception handler then kicks in.

I had a look to see if we could exclude the exception processor from running, or some way to disable that logging, but didn't see any way in configuration to disable it.

Sample Application

Project to replicate the issue is https://github.com/ministryofjustice/prison-estate.git. Need Java 11. Replace with your key for app insights.

To run the application:

git checkout pgp-DT-1442-insights-v3.0.2
./gradlew clean assemble && cp applicationinsights.json build/libs
APPLICATIONINSIGHTS_CONNECTION_STRING=InstrumentationKey=<your instrumentation key> java -javaagent:build/libs/applicationinsights-agent-3.0.2.jar -Dspring.profiles.active=dev -jar build/libs/prison-estate-*.jar

Then make a request that returns a 404 e.g. curl http://localhost:8080/prisons/id/ABC. Our PrisonEstateExceptionHandler will intercept the exception thrown from the service and return a 404 back to the client. This is normal behaviour and shouldn't cause an exception in exceptions.

System information

SDK Version: 2.6.2 Java Agent Version: 3.0.2 OS type and version: Debian 14 Application Server type and version (if applicable): Spring Boot 2.4.2 Using spring-boot? yes

trask commented 3 years ago

hey @petergphillips, we're thinking about making those "InProc" dependencies (internal controller spans) optional, which would make both the InProc dependency for the controller method and the associated exception record (which is parented to the InProc dependency) go away

konjp commented 3 years ago

@trask Any update on this?

In meantime, is it possible to disable it, because in our case it is just noise in application insights:

target : OperationHandler.handle, name : OperationHandler.handle, type : InProc, itemType : dependency ?

trask commented 3 years ago

@konjp is OperationHandler.handle a spring controller method, or something other framework?

konjp commented 3 years ago

Sorry, not to reply earlier. Yes it is spring controller, it happens when calling /actuator/health endpoint.

trask commented 3 years ago

PR #1866 introduces a new preview option to disable capturing of these "InProc" controller spans:

{
  "preview": {
    "captureControllerSpans": false
  }
}

here's a SNAPSHOT build if you want to try it out before the next release: applicationinsights-agent-3.2.0-BETA.3-SNAPSHOT.jar.zip