open-telemetry/opentelemetry-java-instrumentation

ClassCastException for v3_0.CountingHttpServletResponse

Closed this issue ยท 7 comments

ishg commented

Hello, running the Java agent (v0.7.1) with a Java 1.8 app results in the following error with jetty.server.Response:

[2020-08-24 20:13:46,734] [qtp1474515510-53] WARN server.HttpChannel: Could not send response error 500: java.lang.ClassCastException: io.opentelemetry.auto.instrumentation.servlet.v3_0.CountingHttpServletResponse cannot be cast to org.eclipse.jetty.server.Response
[2020-08-24 20:13:48,671] [qtp1474515510-121] WARN server.HttpChannel: /sso/health
java.lang.ClassCastException: io.opentelemetry.auto.instrumentation.servlet.v3_0.CountingHttpServletResponse cannot be cast to org.eclipse.jetty.server.Response
    at org.eclipse.jetty.server.handler.RequestLogHandler.handle(RequestLogHandler.java:104) ~[jetty-server-9.1.2.v20140210.jar:9.1.2.v20140210]
    at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1114) ~[jetty-server-9.1.2.v20140210.jar:9.1.2.v20140210]
    at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:479) ~[jetty-servlet-9.1.2.v20140210.jar:9.1.2.v20140210]
    at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:183) ~[jetty-server-9.1.2.v20140210.jar:9.1.2.v20140210]
...
...
[REDACTED]

I can provide the relevant Pom files after scrubbing if you'd like to know versions for specific libraries, but Jetty seems to be v9.1.2 which should be supported according to the list of frameworks in the main README.

Is there any guidance on how best to debug this or does this seem like a potential bug in the intstrumentation ?

To Note: This DOES break the application due to the ClassCastException and is not a graceful error (even though in the logs it says WARN)

Hello @ishg, thank you for this report. I believe that this information is enough for us to start our investigation. Sorry for this inconvenience.

ishg commented

Hi @iNikem! Any luck with investigating this issue? Is there any further information I can provide, or advisable possible testing steps I can take on my end? Thanks!

Sorry, @ishg , did not have time to think about a solution for this yet. I will try to look at it during next couple days.

@anuraaga @trask Looking at CountingHttpServletResponse I started to think if this is a good idea to use it in JettyHandlerAdvice. There we instrument Jetty specific API and we wrap a parameter of that API into our own class. And we return our own class back into Jetty pipeline. I think it is not unreasonable to assume, that Jetty may have some assumptions about Jetty-specific API's implementations, including concrete classes. So first idea is just to remove CountingHttpServletResponse wrapping from JettyHandlerAdvice.

Then second question is whether we should implement setting http.request_content_length and http.response_content_length semantic attributes based on Content-Length http header only, without wrapping and actual counting.

  1. Yeah if it doesn't interact with Jetty well, I think we do need to remove it

  2. With many streaming responses now adays, like gRPC, I think we'd lose a large section of content length recording if only going with user-agent. But I would generally use counting only if it's not extreme complexity - for example gRPC exposes an interface natively for it (we don't use it yet but should soon) https://grpc.github.io/grpc-java/javadoc/io/grpc/StreamTracer.html#outboundWireSize-long-. Armeria does too - for these cases we definitely should take advantage to record the content length even with no header. The servlet approach we've taken may be too high complexity for it but not sure ๐Ÿ˜…

@ishg should be fixed now and will be release as part of 0.8.0 version which should happen during the next few days.

ishg commented

Thank you all!